Regenerate .surface from Go walker with ARG and alias support#366
Merged
Regenerate .surface from Go walker with ARG and alias support#366
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7e9b2b258
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous commit (f1c15cf) regenerated .surface using the shell script, which produces ARG entries and uses IsAvailableCommand() to filter commands. The Go test (TestSurfaceSnapshot) uses the surface package walker which only produces CMD/FLAG/SUB entries and filters by Hidden. This divergence caused the test to fail. Regenerate from the Go walker — the authoritative source for CI.
jeremy
force-pushed
the
fix-surface-snapshot-mcp
branch
from
b7e9b2b to
4ca600f
Compare
Bump github.com/basecamp/cli to v0.2.1 which adds: - ARG entries: positional arguments parsed from cmd.Use strings - Alias paths: CMD/FLAG/SUB/ARG trees under each alias name - Cobra lazy flag init: --help appears on all commands This brings the Go walker to full parity with the shell script, minus 614 entries the script incorrectly included: - 612 --version flags on subcommands (Cobra adds --version as a local flag on root, not persistent — shell script propagated it) - 2 hidden --assignee flags on recordings (hidden flags are not public surface)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeremy
changed the title
jeremy
added a commit
that referenced
this pull request
Mar 25, 2026
The .surface regeneration with alias support added 332 CMD entries for aliases that the smoke suite didn't account for. Add smoke_aliases.bats with group-level OOS markers that propagate to all leaf descendants, and add a chat show test to smoke_campfire.bats.
2 tasks
jeremy
added a commit
that referenced
this pull request
Mar 25, 2026
* Close smoke coverage gap from .surface alias expansion (#366) The .surface regeneration with alias support added 332 CMD entries for aliases that the smoke suite didn't account for. Add smoke_aliases.bats with group-level OOS markers that propagate to all leaf descendants, and add a chat show test to smoke_campfire.bats. * Move chat show to OOS — alias for chat line (ExactArgs(1)) chat show is an alias for chat line which requires a positional ID arg. Mark it OOS since chat line is already tested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
github.com/basecamp/clito v0.2.1, which extends thesurfacepackage to emitARGentries (positional arguments parsed fromcmd.Use) and walk alias command paths alongside primary names.surfacefrom the Go walker — now the authoritative source forTestSurfaceSnapshot— with full ARG + alias coverage--versionflags and 2 hidden--assigneeflags (see details below)What's preserved
basecamp account,basecamp campfire)What's removed (-615 lines)
--versionflags on subcommands: The shell script (check-cli-surface.sh:45) merges root flags into every subcommand via$ROOT_FLAGS, but Cobra adds--versionas a root-local flag (not persistent). The Go walker correctly omits it from subcommands. Exact parity with the shell script except for this known over-propagation bug; no other diff remained.--assigneeflags on recordings: These were hidden flags (MarkHidden) present in main's.surfacebaseline (from the shell-script regeneration). The Go walker correctly excludes hidden flags. These do not appear in the current shell-script output either — they were artifacts of an earlier generation.Test plan
TestSurfaceSnapshotpasses without-update-surface--versionentries differ (shell script has them, Go walker correctly omits them)make test— all passscripts/check-skill-drift.sh— passes