Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,17 @@ Operator commands (`platform`, `repo`, `github set-token`) use **operator AWS cr

## Commands

### `bgagent version`

Print the installed CLI version (read from `package.json`). Equivalent to the root `bgagent --version` flag, but available as an explicit subcommand with a `--output json` mode for scripting.

```
bgagent version \
--output <text|json> Output format (default: text)
```

Text mode prints the bare version string (e.g. `0.0.0`); JSON mode prints `{ "version": "0.0.0" }`.

### `bgagent configure`

Save API endpoint and Cognito settings to `~/.bgagent/config.json`.
Expand Down
4 changes: 3 additions & 1 deletion cli/src/bin/bgagent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { makeSlackCommand } from '../commands/slack';
import { makeStatusCommand } from '../commands/status';
import { makeSubmitCommand } from '../commands/submit';
import { makeTraceCommand } from '../commands/trace';
import { getCliVersion, makeVersionCommand } from '../commands/version';
import { makeWatchCommand } from '../commands/watch';
import { makeWebhookCommand } from '../commands/webhook';
import { setVerbose } from '../debug';
Expand All @@ -53,7 +54,7 @@ const program = new Command();
program
.name('bgagent')
.description('Background Agent CLI — submit and manage coding tasks')
.version('0.0.0')
.version(getCliVersion())

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.option('--verbose', 'Enable debug output')
.hook('preAction', (_thisCommand, actionCommand) => {
// Resolve --verbose from the root program, not the subcommand
Expand Down Expand Up @@ -86,6 +87,7 @@ program.addCommand(makeRuntimeCommand());
program.addCommand(makeOpsCommand());
program.addCommand(makeWatchCommand());
program.addCommand(makeTraceCommand());
program.addCommand(makeVersionCommand());
program.addCommand(makeWebhookCommand());

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

program.addCommand(makeAdminCommand());

Expand Down
61 changes: 61 additions & 0 deletions cli/src/commands/version.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* MIT No Attribution
*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* the Software without restriction, including without limitation the rights to
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
* the Software, and to permit persons to whom the Software is furnished to do so.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/

import * as fs from 'fs';
import * as path from 'path';
import { Command } from 'commander';
import { formatJson } from '../format';

/**
* Resolve the CLI version from the package manifest.
*
* The manifest lives at ``cli/package.json``, two directories above this
* module in both the TypeScript source tree (``src/commands``) and the
* compiled output (``lib/commands``), so ``../../package.json`` resolves
* correctly under ts-jest and from the published ``lib`` bin alike. We read
* it at runtime rather than importing it so the version reported always
* matches the installed package and the compiled ``lib`` layout stays flat
* (no ``package.json`` copied under ``rootDir``).
*/
export function getCliVersion(): string {
const manifestPath = path.join(__dirname, '..', '..', 'package.json');
const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')) as {
version?: string;
};
return manifest.version ?? '0.0.0';
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/**
* `bgagent version [--output text|json]` — print the installed CLI version,
* read from ``package.json``. Mirrors the root ``--version`` flag as an
* explicit subcommand so scripts can query it uniformly (and in JSON).
*/
export function makeVersionCommand(): Command {
return new Command('version')
.description('Print the bgagent CLI version')
.option('--output <format>', 'Output format (text or json)', 'text')

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 xml0.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.

.action((opts: { output: string }) => {
const version = getCliVersion();
if (opts.output === 'json') {
console.log(formatJson({ version }));
return;
}
console.log(version);
});
}
64 changes: 64 additions & 0 deletions cli/test/commands/version.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* MIT No Attribution
*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* the Software without restriction, including without limitation the rights to
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
* the Software, and to permit persons to whom the Software is furnished to do so.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/

import * as fs from 'fs';
import * as path from 'path';
import { getCliVersion, makeVersionCommand } from '../../src/commands/version';

describe('version command', () => {
let consoleSpy: jest.SpiedFunction<typeof console.log>;

// The version reported must match the single source of truth: the
// package manifest. Reading it here (rather than hardcoding) keeps the
// test correct across version bumps.
const packageVersion = (
JSON.parse(
fs.readFileSync(
path.join(__dirname, '..', '..', 'package.json'),
'utf-8',
),
) as { version: string }
).version;

beforeEach(() => {
process.exitCode = undefined;
consoleSpy = jest.spyOn(console, 'log').mockImplementation();
});

afterEach(() => {
process.exitCode = undefined;
consoleSpy.mockRestore();
});

test('getCliVersion returns the package.json version', () => {
expect(getCliVersion()).toBe(packageVersion);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

});

test('prints the plain version in text mode (default)', async () => {
await makeVersionCommand().parseAsync(['node', 'version']);
expect(consoleSpy).toHaveBeenCalledTimes(1);
expect(consoleSpy).toHaveBeenCalledWith(packageVersion);
});

test('prints a JSON object with --output json', async () => {
await makeVersionCommand().parseAsync(['node', 'version', '--output', 'json']);
const out = consoleSpy.mock.calls.map((c) => c[0]).join('\n');
expect(JSON.parse(out)).toEqual({ version: packageVersion });
});
});
Loading