Conversation
Add missing `profile` entry to the static command catalog ("Auth &
Config" category). Extract `buildRootWithAllCommands()` helper in
commands_test.go, shared by TestCatalog and TestSurfaceSnapshot.
Review carefully before merging. Consider a major version bump. |
There was a problem hiding this comment.
Pull request overview
This PR closes feature parity gaps by bringing downstream CLI improvements into basecamp-cli, including editor-based content composition, skill installation, XDG state dir support, surface snapshots, and build/version/tooling refinements.
Changes:
- Add
$EDITORintegration (--edit) for composing comment/message content and supporting editor-driven workflows. - Improve operational/tooling parity: toolchain guard + coverage target, surface snapshot baseline enforcement, and
go install ...@versionversion fallback. - Align state/config and command surface parity: Linux/BSD resilience state under
XDG_STATE_HOME, catalog parity updates, and new STYLE/SKILL documentation.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/basecamp/SKILL.md | Extends embedded skill documentation with jq patterns and exit-code guidance. |
| internal/version/version.go | Adds runtime build-info fallback to populate version for go install ...@version installs. |
| internal/resilience/store.go | Updates default resilience state dir behavior to prefer XDG state on Linux/BSD. |
| internal/resilience/store_test.go | Adds tests for NewStore/defaultStateDir behavior, including XDG state handling. |
| internal/editor/editor.go | Introduces $EDITOR integration helper to open an editor and return composed content. |
| internal/editor/editor_test.go | Adds tests covering editor invocation, args handling, and empty-content aborts. |
| internal/commands/comment.go | Adds --edit flag to comment shortcut command. |
| internal/commands/messages.go | Adds --edit flag to message creation paths (group subcommand + shortcut). |
| internal/commands/edit_test.go | Adds tests for --edit mutual exclusion, stdin rejection, and empty abort behavior. |
| internal/commands/skill.go | Expands skill command to include skill install (file write + symlink/copy fallback). |
| internal/commands/skill_test.go | Adds tests for skill install behavior, idempotency, fallback copy, and result paths. |
| internal/commands/commands.go | Updates static command catalog entries (profile; skill actions). |
| internal/commands/commands_test.go | Refactors root builder helper and ensures catalog/registration parity includes profile. |
| internal/commands/surface_test.go | Adds .surface snapshot test with optional baseline regeneration flag. |
| internal/appctx/context.go | Routes resilience dir selection through resolveResilienceDir() to respect explicit cache overrides. |
| internal/appctx/resilience_dir_test.go | Adds tests for resilience dir resolution behavior. |
| Makefile | Adds toolchain mismatch guard and a convenience coverage target; wires guard into key targets. |
| go.mod | Bumps github.com/basecamp/cli to v0.1.1. |
| go.sum | Updates sums for github.com/basecamp/cli v0.1.1. |
| STYLE.md | Adds contributor conventions (commands, output, config resolution, catalog parity). |
| AGENTS.md | References STYLE.md for agent/dev context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be0f66ae9a
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
check-toolchain detects PATH go vs GOROOT go mismatches (mise environments); wired as prerequisite to build, test, vet, race-test, and tidy-check. coverage alias extends test-coverage with browser auto-open.
When Version is "dev" (no ldflags), probe runtime/debug.ReadBuildInfo for the module version. Enables correct version display for go install ...@v1.2.3 installs.
Update defaultStateDir() to prefer XDG_STATE_HOME on Linux/FreeBSD/ OpenBSD, falling back to ~/.local/state/basecamp/resilience/. macOS and Windows continue using the cache directory. Extract resolveResilienceDir() in appctx to respect explicit cache_dir overrides (flag/env/config file) while letting the default path use XDG_STATE_HOME.
Use github.com/basecamp/cli/surface.SnapshotString() to generate a checked-in .surface file. TestSurfaceSnapshot compares against the baseline; -update-surface flag regenerates it. Complements the script-based CI gate with an in-process test.
Writes embedded SKILL.md to ~/.agents/skills/basecamp/ and creates a relative symlink at ~/.claude/skills/basecamp. Falls back to direct file copy when symlink creation fails (e.g. Windows without developer mode). Idempotent — safe to run repeatedly.
New internal/editor package launches $EDITOR with initial content, supports editors with arguments (e.g. "code --wait"), and aborts on empty result. Add --edit flag to comment, message, and messages create commands. Mutually exclusive with --content; rejects piped stdin. Tests cover mutual exclusion, piped stdin rejection, and empty editor abort via the full command path.
STYLE.md documents command constructors, output conventions, config resolution, catalog maintenance, method ordering, and file organization patterns. SKILL.md gains a jq patterns library for common data extraction and an exit code remediation table mapping codes 0-8 to fix actions.
Summary
Closes the reverse gap between basecamp-cli and its derivative CLIs (fizzy-cli, hey-cli, amar) by adopting innovations that originated downstream.
8 atomic commits, each independently reviewable:
profileentry to static catalog; extract sharedbuildRootWithAllCommands()test helpermake coveragealias with browser auto-opendebug.ReadBuildInfo()forgo install ...@versionscenariosXDG_STATE_HOMEon Linux/BSD;resolveResilienceDir()respects explicit cache_dir overrides.surfacefile using sharedgithub.com/basecamp/cli/surfacepackage with-update-surfaceregenerationbasecamp skill installwrites SKILL.md to~/.agents/skills/basecamp/, symlinks into~/.claude/skills/basecampwith copy fallbackinternal/editorpackage with$EDITORintegration;--editflag oncomment,message,messages create(mutual exclusion with--content, piped stdin rejection, empty abort)Backward compatibility
All changes are additive — no existing behavior is altered:
skilldescription change: only the--helptext changed from "Print…" to "Manage…".basecamp skill(no subcommand) still prints SKILL.md to stdout via the preservedRunE. Scripts pipingbasecamp skilloutput are unaffected.--editflag validation:--editis a brand-new flag oncomment,message, andmessages create. The mutual exclusion with--content, piped stdin rejection, and empty editor abort are only reachable when--editis explicitly passed. No existing invocation can trigger these paths.skill installsubcommand: new subcommand on an existing command.basecamp skillwithout arguments still works identically.No major version bump needed.
Test plan
make checkpasses (fmt-check, vet, lint, test, test-e2e, naming, surface, provenance, tidy-check)make release-checkpasses (adds replace-check, vuln, race-test, surface-compat)go test ./...— all 26 packages passTestEditContentMutualExclusion,TestEditRejectsPipedStdin,TestEditEmptyAborts,TestEditWithoutContentAllowed,TestSkillInstallRunE,TestSkillInstallIdempotent,TestSkillInstallFallbackOnNonEmptyDir,TestSkillInstallResultMap,TestSurfaceSnapshot,TestDefaultStateDirXDG,TestResolveResilienceDir-update-surface)