Skip to content

fix(cli): shell completions for the SSH helper describe the full CLI again - #2004

Merged
lidge-jun merged 1 commit into
devfrom
fix/stability-helper-completions
Oct 28, 2026
Merged

lidge-jun merged 1 commit into
devfrom
fix/stability-helper-completions

Conversation

@lidge-jun

Copy link
Copy Markdown
Contributor

goat __ssh-helper protocol --completions bash printed a completion script for the SSH helper alone instead of the full CLI, so bin.fast-path.test.ts > matches the full CLI for helper arguments: completions failed on dev and on every PR whose CI runs the server shards.

The cause is a merge overlap. #1857 added an __ssh-helper branch to bin.ts backed by cli/sshHelperCli.ts; #1925 then added the fast path (cli/helperMain.ts) ahead of it. After both landed, the old branch was reachable only when the arguments contained an Effect CLI built-in such as --completions or --help, exactly the case bin.ts says must go to the full CLI. This removes the old branch and its now-unused module, so built-ins fall through to the full CLI as intended.

The removed sshHelperCli.test.ts checked that --version and the SSH probe answer byte for byte like the full CLI. That check moves into the existing fast-path equality test, which now compares stdout writes as well as console output, covers --version and __ssh-helper protocol --shell, and refuses a vacuous comparison where the full CLI printed nothing.

Validation:

  • vp test run apps/server/src/bin.fast-path.test.ts: 1 failed / 5 passed on dev (98bd427), 8 passed with this change.
  • vp run check:changed: exit 0 (format, lint, knip, typecheck, 7 test files / 94 tests).

Model and harness: claude-opus-5-5 via the Codex harness in GOAT.

…again

bin.ts kept the pre-#1925 SSH helper branch after the fast path was added. It was only reachable for Effect CLI built-ins such as --completions, which it answered from a helper-only command tree. Remove the branch and its module; the fast-path test now also compares stdout for --version and the SSH probe.
@lidge-jun lidge-jun added the ci:affected goat-ci: run the tasks the changes reach label Oct 28, 2026
@lidge-jun

Copy link
Copy Markdown
Contributor Author

CI on exact head 17c5788: all jobs pass except two failures that reproduce identically on dev and are not touched by this PR:

  • goat-build: ReleaseVerificationError for the github.com/goat-si/goat link in apps/web/src/components/settings/MultipleAccountsNote.tsx:41 (from chase(t3): land the rest of cycle 261028 as one combined PR #1982); owned by the L-web lane.
  • goat-test-server related 1/4: SteeringCompletion.integration.test.ts, 2 tests ("promoting next past unclaimed/suspended compaction ..."). Single-file runs on dev e108855 and 98bd427 give the same 2 failed / 22 passed. A separate lane is fixing it.

Every other server shard passes, including bin.fast-path.test.ts, which this PR fixes. Independent gpt-6.1-sol review: PASS with no findings on 17c5788. Dev evidence run: https://github.com/goat-si/goat/actions/runs/40621796632 (queued).

@lidge-jun
lidge-jun merged commit 07aa973 into dev Oct 28, 2026
12 of 21 checks passed
@lidge-jun
lidge-jun deleted the fix/stability-helper-completions branch October 30, 2026 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:affected goat-ci: run the tasks the changes reach

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant