AIDA-2282: Reject legacy CLI calls to the desktop app - #2014

🤖 Reviewed f687135c016aa34dba965bb220cf0a3ad15672df: no blocking correctness or security findings in the three-file diff. Minimalness, elegance, and correctness: 9/10 each for the stated scope.

The recognized command/global-option list matches the legacy CLI. Rejection happens before identity access, single-instance activation, or deep-link registration; stderr is static and later payload arguments are not inspected. The regression tests exercise the actual executable and bound its startup time.

Independent local validation on macOS at this head:

• cargo test -p Buzz --locked --no-fail-fast -- --test-threads=2: 721 passed, two existing tests ignored.
• 72 actual-executable invocations through both target/debug/Buzz and its case-insensitive lowercase buzz path: all recognized commands/switches, separate-value and equals-form global options, and a non-Unicode later payload. Every invocation returned 2, empty stdout, and the exact static stderr message.
• git diff --check passed; current required CI checks passed when inspected.

Remaining acceptance gap: normal cold/warm GUI startup and OS-delivered deep links were not exercised in this review. As documented in docs/deep-links.md, those require a coordinated bundle test because launching it changes the installed app's URL handler. The classifier tests establish argument pass-through, not complete GUI/OS acceptance. Human acceptance remains separate under AGENTS.md.

🤖 Blocking: one correctness gap must be fixed before merge. Reviewed at f687135c016aa34dba965bb220cf0a3ad15672df.

Blocking: the legacy gifs command is not recognized

is_legacy_cli_arg in src-tauri/src/legacy_cli_invocation.rs lists every top-level command of the legacy CLI except gifs. The legacy CLI defines Gifs(GifsCmd) (search, share) in crates/buzz-cli/src/lib.rs in block/buzz. It exists at this app's pinned Buzz revision 7e646124 and in the last old-desktop release desktop-v0.5.28, which bundles the CLI as binaries/buzz. So …/Contents/MacOS/buzz gifs search cats still falls through to native startup. With an instance already running, it is foregrounded and exits 0 without doing anything. That is the silent success this PR is meant to stop, and it contradicts the claim that the recognized list matches the legacy CLI.

Fix: add "gifs" to the classifier, add it to recognizes_legacy_commands_and_global_options, and add a representative gifs invocation to the real-binary test in src-tauri/tests/legacy_cli_invocation.rs.

Checked and fine

• All other top-level commands and all four global options (--relay, --private-key, --auth-tag, --format, including =value forms) match the legacy CLI. --version/-V don't exist there, but rejecting them is harmless.
• No legitimate launch argument is caught. Windows ("<exe>" "%1") and Linux (%u) deep-link registration, single-instance forwarding, the updater handoff, and the macOS relauncher (src-tauri/src/relaunch.rs) all pass through the original or buzz:// arguments, and checking only argv[1] keeps link contents and later values out of the match.
• main() -> ExitCode is valid under windows_subsystem = "windows". Note that a release GUI-subsystem launch on Windows may have no stderr handle, so the message can be dropped there; exit status 2 is the reliable signal, so don't promise visible stderr on every Windows launch.

🤖 Star Lord’s automated source review via Wes’s account, integrating Rocket (startup/lifecycle), Mantis (correctness/security), and Groot (CLI behavior).

Reviewed head: f687135
Base tip: 6ee523c
PR diff merge base: 859c995

[P2] Remove the internal tracker hyperlink from the public description. The “Why” paragraph publishes a private workspace’s Linear issue URL in this public repository. Remove that hyperlink and retain the self-contained problem statement (or replace it with a public issue). This is a public-material boundary finding about the coordination reference itself; it does not assert exposure of the ticket contents or credentials.

No actionable code defects found in the three-file patch. The first-argument guard precedes native initialization, uses a static diagnostic, and leaves the existing launch/deep-link owners in place. Legitimate commit attribution is not a finding.

Validation: source-only review; no tests, builds or app launches executed. One hosted-check snapshot reported CI required successful, with Windows native validation skipped. Contributor-reported packaged checks were not independently reproduced; native diagnostics, cold/warm OS deep-link behavior and human acceptance remain unverified. The linked issue could not be read (403); the description and commit message were inspected and there were no attached images. This COMMENT is not an approval or merge authorization.

🤖 Three actionable concerns: migration exit-code semantics, command-inventory drift, and executable-test isolation. Native/package acceptance remains unverified.

🤖 Star Lord’s automated source review via Wes’s account, integrating Rocket (startup/lifecycle), Mantis (correctness/security), and Groot (CLI behavior).

No additional changes requested in this follow-up. The earlier internal tracker hyperlink is removed, gifs now matches the pinned CLI command inventory, and the diagnostic/help point to the bundled CLI path. The original guard patch is unchanged by the rebase; the follow-up extends the existing executable regression with GIF payload suppression and the replacement path.

The existing exit-code, inventory-drift and test-isolation comments remain visible. We did not establish a concrete failing caller for the exit-code concern; inventory synchronization and regression-path isolation remain optional improvements, not newly demonstrated defects in this follow-up.

Reviewed head: f5f28e0
Base tip: e4177d0
PR diff merge base: 7f793e9

Source-only: no tests, builds or app launches executed, and CI was not assessed this cycle. Contributor-reported checks were not independently reproduced. Installed-package behavior, cold/warm OS deep links, rendered retry/focus behavior and human acceptance remain unverified. This COMMENT is not an approval.

🤖 Automated review at f5f28e047426f9fe85461bbcfefa05c912e6428f (base e4177d0a46c68a221bedf5df6a46a8b6ea50d60c): no blocking findings.

Source review (two independent reviewers):

• The 23 command names in src-tauri/src/legacy_cli_invocation.rs match the top-level Cmd enum of crates/buzz-cli exactly. This holds at the block/buzz revision this app pins and bundles (7e646124, from the buzz-audio-client rev used by scripts/build-buzz-cli.mjs) and at block/buzz main. Neither defines aliases. The four root options match Cli. -V/--version go beyond that CLI, but matching them is harmless.
• The guard runs before buzz_foundation_lib::run(), so it precedes native-MCP setup, identity initialization and single-instance IPC. Only argv[1] is checked, so buzz:// links, -psn_*, Cocoa launch options and literal -- keep the normal startup path. The locked deep-link and single-instance plugins pass the URL as argv[1] on Windows and Linux, and parse it after this guard. The diagnostic is static and never echoes payloads.

Native executable check (debug build of this head, headless):

• These each exited 2, with empty stdout and the exact diagnostic: messages send …, gifs search …, --help, --relay=x, --private-key=… messages list and --format compact channels list.
• These each passed the guard and entered run(): an unknown argument, a buzz:// link, -psn_0_12345, -- messages send, an unknown argument followed by messages, --help=something, bare send, and no arguments. This was shown with a pre-window startup exit. Bare send is correctly not rejected, since the CLI nests it under messages.
• Not covered: full app startup, OS-delivered deep links and packaged-app behavior.

MINOR (non-blocking): src-tauri/tests/legacy_cli_invocation.rs checks only the arguments that should be rejected at the executable level. A pass-through control would guard the "argv[1] only" wiring against a future change that scans every argument. For example, run an unknown first argument followed by messages, with a startup tripwire such as BUZZ_NATIVE_MCP_PORT that exits before the window opens.

CI: the failing browser journeys (Chromium 6/9 and WebKit 9/14 for thread navigation history, and WebKit nested-reply focus) fail the same way on main at this PR's base e4177d0a. Later main commits are green, so a rebase should clear them. Rust and tool integration, including the new tests, passed.

🤖 One non-blocking inventory-coverage gap remains; no current runtime defect demonstrated.

🤖 Star Lord’s automated source review via Wes’s account, integrating Rocket (startup/lifecycle), Mantis (correctness/security), and Groot (CLI behavior).

No additional changes requested in this follow-up. Exit status 1 now matches the bundled CLI’s bad-input contract. The executable tests stop at the existing pre-identity refusal if recognition regresses, and the bundled-CLI harness checks every advertised root command. The earlier guard and GIF repair remain intact through the rebase.

The existing suggestion to derive root-option coverage from help remains optional: the current four options match the pinned CLI, and no present runtime defect was demonstrated. No duplicate inline comment added.

Reviewed head: 289e60b
Base tip: f557d0a
PR diff merge base: f759d75

Source-only: no tests, builds or app launches executed. One hosted-check snapshot reported CI required successful, with Windows native validation and flake checks skipped. The author reports the local full macOS suite still crashes in window_state::tests::onboarding_snapshot_is_kept_until_next_launch_restore; we did not reproduce or diagnose that failure. Installed-package behavior, full GUI startup, cold/warm OS deep links, focus behavior and human acceptance remain unverified. Public description and commit messages were inspected; no attached images were present. This COMMENT is not an approval or merge authorization.

🤖 Automated review at 289e60b3eff61e1cf1927f604869232e5c1a609b (base f557d0a50486cd90bc0a377f8f80a06dcba5bf6a): no blocking findings.

I reviewed the one commit added since f5f28e04, "refine CLI migration failures". The other two commits are the same as before apart from a docs rebase.

• Exit status 1. This now matches the bundled CLI's bad-input status. main.rs, docs/buzz-help.md and both executable tests agree on it. The pass-through test still separates the two exit-1 paths by checking stderr: the native-MCP refusal versus the migration message.
• Pass-through controls. These cover the earlier suggestion. ordinary_launch_arguments_reach_the_pre_identity_startup_refusal sends no arguments, a buzz:// link, -psn_*, a Cocoa option followed by messages, an unknown argument followed by messages, and -- messages send. Each must reach the BUZZ_NATIVE_MCP_PORT=invalid refusal. That refusal runs at the top of run() (src-tauri/src/lib.rs), right after the static app context loads, so a regression stops before sign-out erase work, identity access and single-instance IPC. The HOME/XDG/APPDATA overrides plus that tripwire also cover the test-isolation concern. tempfile is already a regular dependency of the Buzz crate.
• Bundled-CLI contract. scripts/test-buzz-cli.mjs takes the root commands from the staged CLI's own --help output and confirms that the desktop executable rejects each one with status 1, empty stdout and the migration message. It guards against an empty list. It reuses the same isolated environment and tripwire. The script is the existing CI step "Bundled buzz CLI".

CI at this head: green. In the Rust and tool integration job log, both executable tests and the three unit tests pass, and the contract step prints Desktop rejects all 23 bundled CLI commands.

MINOR (non-blocking): I agree with the open comment on scripts/test-buzz-cli.mjs. The four root options (--relay, --private-key, --auth-tag, --format) are still a handwritten list that nothing checks against the bundled CLI. Taking --long names from the Options: section of the same help output would close that drift path.

Not covered: full GUI startup, cold and warm OS-delivered deep links, and installed-package behavior.

🤖 One integration blocker against current main; the previous root-option coverage gap is addressed.

🤖 One remaining command-inventory defect at a42c60d; add sidebar before merging.