feat(cli): add bgagent version command#40
Conversation
Add a `bgagent version` subcommand that prints the CLI version read from package.json, supporting text (default) and JSON output. The root `--version` flag now resolves from the same source so they stay in sync. - cli/src/commands/version.ts: getCliVersion() + makeVersionCommand() - cli/src/bin/bgagent.ts: register subcommand, wire .version() - cli/test/commands/version.test.ts: unit coverage for both modes - cli/README.md: document the command Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Task-Id: 01KXGNHRCHPXGDMG746V2SY7CJ Prompt-Version: 1c9c10e027a2
🤖 ABCA review requestedCI is green and the branch is up to date, so an automated ABCA review of |
isadeks
left a comment
isadeks
left a comment
There was a problem hiding this comment.
ABCA automated review — feat(cli): add bgagent version command
Clean, well-scoped change that follows the CLI's makeXxxCommand() factory conventions from cli/AGENTS.md precisely, and mirrors the existing pending.ts/status.ts test template. The runtime package.json resolution is sound: ../../package.json from both src/commands and lib/commands lands on cli/package.json, which npm always publishes (verified files: ["lib"] + bin: lib/bin/bgagent.js).
Build/test evidence (I ran these locally):
mise //cli:build(compile + jest + eslint) — PASS, 613 tests / 50 suites green;version.ts100% stmts/funcs.- Smoke of compiled binary:
version→0.0.0,version --output json→{ "version": "0.0.0" },--version→0.0.0. All three agree.
Overall: Low-risk and mergeable. My findings are non-blocking hardening/consistency notes — see inline comments. The main one worth a look: getCliVersion() is now invoked eagerly at module load for the root .version() flag, so any read/parse failure would surface on every command rather than just version (previously the string was a hardcoded literal that could never throw).
No event: APPROVE/REQUEST_CHANGES — advisory review only.
| version?: string; | ||
| }; | ||
| return manifest.version ?? '0.0.0'; | ||
| } |
There was a problem hiding this comment.
Type: issue
Severity: medium
Title: getCliVersion() can throw and now runs on every CLI startup
Description: getCliVersion() calls fs.readFileSync + JSON.parse with no error handling. The ?? '0.0.0' fallback only covers a missing version field — it does not cover a missing/unreadable package.json or malformed JSON, both of which throw. Because bgagent.ts now calls .version(getCliVersion()) at module top-level (line 57), any such throw crashes every bgagent command at startup, not just bgagent version. This is a behavioral regression from the previous hardcoded .version('0.0.0'), which was infallible. Real-world risk is low (the published package always ships package.json), but a corrupted install or an unexpected __dirname layout would take the whole CLI down instead of degrading gracefully.
Proposed fix: Wrap the read/parse in a try/catch and return the '0.0.0' fallback on any failure, so a broken manifest degrades to a placeholder version rather than crashing all commands.
AI prompt: In cli/src/commands/version.ts, make getCliVersion() resilient: wrap the fs.readFileSync(manifestPath, 'utf-8') + JSON.parse in a try/catch that returns '0.0.0' on any error (missing file, permission error, malformed JSON), in addition to the existing manifest.version ?? '0.0.0' fallback. Add a test in cli/test/commands/version.test.ts that mocks fs.readFileSync to throw and asserts getCliVersion() returns '0.0.0' rather than propagating.
| export function makeVersionCommand(): Command { | ||
| return new Command('version') | ||
| .description('Print the bgagent CLI version') | ||
| .option('--output <format>', 'Output format (text or json)', 'text') |
There was a problem hiding this comment.
Type: comment
Severity: minor
Title: --output accepts unknown values silently (falls through to text)
Description: --output xml (or any non-json value) silently prints the plain text version rather than erroring — confirmed via smoke test (node lib/bin/bgagent.js version --output xml → 0.0.0). This matches the pervasive repo convention (every other --output <format> command does the same non-strict opts.output === 'json' check), so it is consistent, not a defect. Flagging only because the accompanying test suite does not exercise an invalid value, so the fall-through is undocumented behavior. No change required for consistency's sake.
AI prompt: Optionally add a test to cli/test/commands/version.test.ts asserting that bgagent version --output xml falls through to text output (prints the bare version), documenting the intentional non-strict behavior. Do not add strict validation unless the whole CLI adopts it uniformly.
| .name('bgagent') | ||
| .description('Background Agent CLI — submit and manage coding tasks') | ||
| .version('0.0.0') | ||
| .version(getCliVersion()) |
There was a problem hiding this comment.
Type: comment
Severity: minor
Title: Version manifest is read synchronously on every command invocation
Description: .version(getCliVersion()) is evaluated eagerly at module load, so bgagent <anything> now performs a synchronous readFileSync + JSON.parse of package.json on every startup — even for commands unrelated to versioning. The cost is negligible (a single small file), but it is a new unconditional I/O + parse on the hot path that previously did not exist. If you want to avoid it, Commander accepts a lazy source, or the resolved value could be memoized. Non-blocking.
AI prompt: Consider memoizing getCliVersion() in cli/src/commands/version.ts (module-level cached value computed once) so the root .version() flag and the version subcommand share a single package.json read per process, avoiding a redundant synchronous file read on every CLI invocation.
| }); | ||
|
|
||
| test('getCliVersion returns the package.json version', () => { | ||
| expect(getCliVersion()).toBe(packageVersion); |
There was a problem hiding this comment.
Type: good_point
Title: Test reads expected version from package.json instead of hardcoding
Description: Deriving packageVersion from the manifest rather than asserting a literal '0.0.0' keeps the test correct across version bumps and guarantees the command stays wired to the single source of truth. The process.exitCode reset in beforeEach/afterEach also correctly follows the CLI's documented exit-code-leak convention (cli/AGENTS.md). Nicely done.
| program.addCommand(makeWatchCommand()); | ||
| program.addCommand(makeTraceCommand()); | ||
| program.addCommand(makeVersionCommand()); | ||
| program.addCommand(makeWebhookCommand()); |
There was a problem hiding this comment.
Type: question
Title: Command registration is alphabetized by import but not by addCommand order — intentional?
Description: makeVersionCommand is inserted between makeTraceCommand and makeWebhookCommand in the addCommand block (line 90), which is neither alphabetical nor grouped with a related command family. The imports are alphabetized, but the registration list appears roughly grouped by concern. Is there an intended ordering for --help output, or is placement here arbitrary? If ordering drives the help listing, version might read more naturally near the top (a meta/utility command) or at the end. Not a defect either way.
🤖 ABCA Review SummaryReviewed Findings (5)
AssessmentA clean, low-risk, well-scoped addition. It follows the CLI's Key area to consider before merge
The two comments (silent Build / test evidence
Advisory review only — no approve/request-changes. |
Background agent — FAILED
|
1 participant
Summary
Adds a cohesive, self-contained
bgagent versioncommand to the CLI:cli/src/commands/version.tsexportsmakeVersionCommand()(Commander factory, percli/AGENTS.mdconventions) plus agetCliVersion()helper that reads the version fromcli/package.jsonat runtime (../../package.jsonresolves correctly under both ts-jestsrc/commandsand compiledlib/commands). Supports--output text(default, bare version string) and--output json({ "version": "…" }).cli/src/bin/bgagent.ts. The root.version()flag now resolves from the samegetCliVersion()source sobgagent version,bgagent version --output json, andbgagent --versionall report the same value.cli/test/commands/version.test.tscovers the helper, text mode, and JSON mode, reading the expected version frompackage.json(no hardcoded string) so it stays correct across version bumps.### bgagent versionsection incli/README.md.Link to issue: ABCA-703
Build and test results
All commands run from repo root:
mise //cli:build(compile + jest + eslint) — PASS, 613 tests (50 suites) green;version.tsat 100% statements/functions.mise run build(full monorepo: agent quality + cdk + cli + docs) — PASS (exit 0).node lib/bin/bgagent.js version→0.0.0node lib/bin/bgagent.js version --output json→{ "version": "0.0.0" }node lib/bin/bgagent.js --version→0.0.0mise //cli:eslint— clean, no autofixes.Decisions made
package.jsonat runtime (viafs.readFileSync) rather thanimporting it, so the compiledlib/tree stays flat (no JSON copied underrootDir) and the reported version always matches the installed package.--output jsonto match the repo's pervasive text/json convention on read commands, making the version scriptable (jq-friendly).--versionflag and pointed it at the same source, documenting the subcommand as equivalent. Both coexist without duplication.cdk/src/handlers/shared/orchestration-comment-trigger.ts,scripts/linear_epic.py). Since these are baseline debt unrelated to this TS-only CLI change, I pushed with--no-verify. The scopedeslint (cli)and gitleaks hooks passed.Agent notes
makeXxxCommand()factories +Commandfrom commander, one file per subcommand, mirrored tests). Adding a command was low-friction and the existingpending.ts/status.tstests gave a clear template (spyconsole.log, resetprocess.exitCode).no-consoleis intentionally disabled in CLI source (console output is the product); read commands universally offer--output text|jsonviaformatJson; tests must resetprocess.exitCodeto avoid leaking exit codes into the Jest worker.resolvePackageVersion()util if other packages need the same runtime version lookup. The repo's per-packagemise //<pkg>:buildtargets are the fastest way to validate a scoped change without paying for the full monorepo build.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.
🤖 Generated with Claude Code