Skip to content

fix(cli): reject unknown top-level commands#353

Open
xxb wants to merge 1 commit intochenhg5:mainfrom
xxb:fix/unknown-top-level-command
Open

fix(cli): reject unknown top-level commands#353
xxb wants to merge 1 commit intochenhg5:mainfrom
xxb:fix/unknown-top-level-command

Conversation

@xxb
Copy link
Copy Markdown
Contributor

@xxb xxb commented Mar 28, 2026

Summary

  • reject unknown top-level commands before they can fall through to the default bridge startup path
  • parse root CLI flags first, then dispatch the remaining args to known top-level subcommands
  • add regression coverage for unknown commands, root flags before subcommands, preserved subcommand help, and root help

Problem

Commands such as cc-connect bind --help were treated as normal startup because only a small set of top-level subcommands were explicitly dispatched. That let an invalid top-level command continue into the full bridge startup path, potentially launching a second instance and rebinding ~/.cc-connect/run/api.sock.

Testing

  • nix shell nixpkgs#go -c go test ./cmd/cc-connect
  • nix shell nixpkgs#go -c go test ./agent/codex -run TestSend_UsesStdinForMultilinePrompt -count=1 -v
  • nix shell nixpkgs#go -c go test ./... (currently still fails outside this change in platform/wecom.TestWecomInboundFileMime; one full-suite run also hit a transient agent/codex failure that passed on isolated rerun)

Copy link
Copy Markdown
Owner

@chenhg5 chenhg5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Good fix for CLI command validation.

Review notes:

  • CI unit-test failing (but author notes this is an existing failure in platform/wecom.TestWecomInboundFileMime, not related to this change)
  • Correct approach: explicit command map + validation for unknown commands
  • Good test coverage for edge cases
  • Fixes the issue where invalid commands like cc-connect bind --help would fall through to bridge startup

The change properly rejects unknown top-level commands before they can trigger unwanted behavior. Approved for merge once CI passes (or the unrelated test failure is addressed separately).

@xxb xxb force-pushed the fix/unknown-top-level-command branch from f24529f to 3648aaf Compare March 30, 2026 17:17
@xxb xxb force-pushed the fix/unknown-top-level-command branch from 3648aaf to 02cb26b Compare March 31, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants