Add @base-org/account-cli package scaffolding by fan-zhang-sv · Pull Request #257 · base/account-sdk

fan-zhang-sv · 2026-03-19T22:16:19Z

Summary

Adds packages/account-cli/ as a new workspace package (@base-org/account-cli) with bin: base-account
Sets up build tooling (TypeScript, Vitest, Biome), Commander entry point with --json global flag, CLIError class, and formatOutput/formatError helpers
Establishes the two-layer architecture (commands/ + core/) documented in the README for incremental command additions

Test plan

yarn workspace @base-org/account-cli typecheck passes
yarn workspace @base-org/account-cli build succeeds
node packages/account-cli/dist/index.js --help shows expected output
No lint errors (npx biome lint packages/account-cli/)

Made with Cursor

cb-heimdall · 2026-03-19T22:16:24Z

✅ Heimdall Review Status

Requirement Status More Info

Reviews

✅ 2/1

Denominator calculation

Show calculation

1 if user is bot 0

1 if user is external 0

2 if repo is sensitive 0

From .codeflow.yml 1

Additional review requirements

Show calculation

Max	0

0

From CODEOWNERS 0

Global minimum 0

Max 1

1

1 if commit is unverified 0

Sum 1

Establish the account-cli workspace package with build tooling, entry point, type system, and output formatting — ready for commands to be added incrementally. Made-with: Cursor

Vitest exits with code 1 when no test files are found, which caused the Tests job to fail. Add tests for CLIError and the output formatting helpers. Made-with: Cursor

spencerstock

🔴 Changes requested · 72/100

The scaffolding for @base-org/account-cli is well-structured with clean separation of concerns (commands/core architecture), good test coverage for the error types and output helpers, and proper workspace integration.

However, there is one critical issue: TypeScript's tsc does not preserve the #!/usr/bin/env node shebang from the source file, so dist/index.js will lack it after build. This means the base-account binary entry won't work when the package is installed globally or via npx. A post-build shebang injection step or a switch to a bundler that handles this is needed.

Additionally, the formatOutput function has identical logic in both JSON and non-JSON branches, making the --json flag a no-op for output formatting. The license mismatch with the root package (MIT vs Apache-2.0) and missing LICENSE file should also be resolved before publishing.

Confidence: 72/100 · 🔴 1 critical · 🟡 2 suggestions

🔧 Fix with prompt

A reviewer gave these comments as feedback. Validate them and fix all the ones that need to be fixed.

- [critical] packages/account-cli/src/index.ts:1-1
  **critical:** The `#!/usr/bin/env node` shebang is in the TypeScript source, but `tsc` does **not** preserve shebangs when compiling. The `bin` entry in `package.json` points to `./dist/index.js`, which will lack the shebang after build.
  
  When installed via `npm install -g` or run via `npx`, `base-account` will fail to execute (the OS won't know to invoke Node).
  
  **Fix options:**
  1. Add a post-build script: `echo '#!/usr/bin/env node' | cat - dist/index.js > temp && mv temp dist/index.js`
  2. Switch to a bundler like `tsup` or `esbuild` that preserves/injects shebangs
  3. Use a small shell wrapper as the bin entry

- [suggestion] packages/account-cli/src/output/index.test.ts:22-25
  **suggestion:** This test only asserts `toHaveBeenCalledOnce()` without verifying the output content for the `json=false` case. Once the human-readable formatter is differentiated from JSON output, this test should assert on the actual format to prevent regressions.

- [suggestion] packages/account-cli/package.json:14-14
  **suggestion:** The root `package.json` uses `"license": "MIT"` while this package specifies `"license": "Apache-2.0"`. The `files` array also includes `LICENSE` but no LICENSE file exists in `packages/account-cli/`. Please verify the intended license and add the corresponding LICENSE file, or it will be missing from the published package.

_{SHA 15925ffc · gemini-3-pro-preview → claude-opus-4-6 · [bot-raiser]}

spencerstock · 2026-03-20T18:54:24Z

@@ -0,0 +1,10 @@
+#!/usr/bin/env node


_{[bot-raiser] critical}

critical: The #!/usr/bin/env node shebang is in the TypeScript source, but tsc does not preserve shebangs when compiling. The bin entry in package.json points to ./dist/index.js, which will lack the shebang after build.

When installed via npm install -g or run via npx, base-account will fail to execute (the OS won't know to invoke Node).

Fix options:

Add a post-build script: echo '#!/usr/bin/env node' | cat - dist/index.js > temp && mv temp dist/index.js

Switch to a bundler like tsup or esbuild that preserves/injects shebangs

Use a small shell wrapper as the bin entry

spencerstock · 2026-03-20T18:54:24Z

+  it('writes output to stdout when json=false', () => {
+    formatOutput({ value: 42 }, false);
+    expect(stdoutSpy).toHaveBeenCalledOnce();
+  });


_[bot-raiser]

suggestion: This test only asserts toHaveBeenCalledOnce() without verifying the output content for the json=false case. Once the human-readable formatter is differentiated from JSON output, this test should assert on the actual format to prevent regressions.

yup, this PR doesn't have non json formatter yet, this is just a placeholder test cases, will add it in human output formatter PR!

spencerstock · 2026-03-20T18:54:24Z

+  ],
+  "publishConfig": {
+    "access": "public"
+  },


_[bot-raiser]

suggestion: The root package.json uses "license": "MIT" while this package specifies "license": "Apache-2.0". The files array also includes LICENSE but no LICENSE file exists in packages/account-cli/. Please verify the intended license and add the corresponding LICENSE file, or it will be missing from the published package.

Add postbuild script to prepend #!/usr/bin/env node to dist/index.js since tsc strips shebangs. Align license to MIT to match root package and remove non-existent LICENSE from files array. Made-with: Cursor

spencerstock

🟡 Follow-up needed on prior feedback · 77/100 · Re-review

Previous comment assessment:

License (Comment 1): Resolved. The package.json now has "license": "MIT" (matching root), and the files array no longer includes LICENSE, avoiding shipping a missing file.
Shebang (Comment 2): Resolved. A postbuild script was added: printf '#!/usr/bin/env node\n' | cat - dist/index.js > dist/index.tmp && mv dist/index.tmp dist/index.js. This correctly prepends the shebang to the compiled output.
Test assertion for json=false (Comment 3): Not addressed. The test at line 23-24 of output/index.test.ts still only asserts toHaveBeenCalledOnce() without verifying the actual output content for the json=false case. Given that the implementation is still identical for both branches, this is understandable but the test gap remains.
Identical branches in formatOutput (Comment 4): Not addressed. Lines 4-8 of output/index.ts still have identical if/else branches — both emit JSON.stringify(data, null, 2). The human-readable path is indistinguishable from JSON output.

New issue sweep:

The postbuild script uses printf and cat which are POSIX utilities — this will fail on Windows without WSL/Git Bash. For a scaffolding PR this is acceptable but worth noting.
No other regressions or critical new issues found in the changed hunks.

Confidence: 77/100

_{SHA 075ebf00 · claude-opus-4-6}

fan-zhang-sv · 2026-03-20T21:49:01Z

@spencerstock thx! comment 3 and comment 4 is true, this PR is just for scaffolding, I'm leaning towards keeping it as-is for now, as we don't have clear picture on what human output is gonna be like yet. We might import some package to prettify the human output for commands, but will get to it once we start implementing actual commands i think

spencerstock

👍

stephancill · 2026-03-23T16:23:22Z

can we name this directory something like utils instead?

Made-with: Cursor

spencerstock

LGTM

fan-zhang-sv force-pushed the felix/account-cli-scaffolding branch from 8996555 to 0b157a4 Compare March 19, 2026 22:18

Add @base-org/account-cli package scaffolding

d907e87

Establish the account-cli workspace package with build tooling, entry point, type system, and output formatting — ready for commands to be added incrementally. Made-with: Cursor

fan-zhang-sv force-pushed the felix/account-cli-scaffolding branch from 0b157a4 to d907e87 Compare March 19, 2026 22:28

Add account-cli unit tests to fix CI

15925ff

Vitest exits with code 1 when no test files are found, which caused the Tests job to fail. Add tests for CLIError and the output formatting helpers. Made-with: Cursor

fan-zhang-sv force-pushed the felix/account-cli-scaffolding branch from f83df71 to 15925ff Compare March 20, 2026 16:51

spencerstock reviewed Mar 20, 2026

View reviewed changes

fan-zhang-sv force-pushed the felix/account-cli-scaffolding branch from c9f9bf0 to a80adbb Compare March 20, 2026 19:28

Fix shebang injection and license mismatch in account-cli

075ebf0

Add postbuild script to prepend #!/usr/bin/env node to dist/index.js since tsc strips shebangs. Align license to MIT to match root package and remove non-existent LICENSE from files array. Made-with: Cursor

fan-zhang-sv force-pushed the felix/account-cli-scaffolding branch from a80adbb to 075ebf0 Compare March 20, 2026 19:30

spencerstock reviewed Mar 20, 2026

View reviewed changes

fan-zhang-sv requested a review from spencerstock March 20, 2026 21:49

spencerstock previously approved these changes Mar 20, 2026

View reviewed changes

fan-zhang-sv enabled auto-merge (squash) March 20, 2026 21:50

stephancill reviewed Mar 23, 2026

View reviewed changes

Rename output/ to utils/ in account-cli

06a1557

Made-with: Cursor

fan-zhang-sv dismissed spencerstock’s stale review via 06a1557 March 23, 2026 16:33

stephancill approved these changes Mar 23, 2026

View reviewed changes

spencerstock approved these changes Mar 23, 2026

View reviewed changes

fan-zhang-sv merged commit 661ddea into master Mar 23, 2026
9 checks passed

fan-zhang-sv deleted the felix/account-cli-scaffolding branch March 23, 2026 17:03

Conversation

fan-zhang-sv commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

cb-heimdall commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Heimdall Review Status

Uh oh!

spencerstock left a comment

Choose a reason for hiding this comment

🔴 Changes requested · 72/100

Uh oh!

spencerstock Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

fan-zhang-sv Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

spencerstock Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

fan-zhang-sv Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

spencerstock Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

fan-zhang-sv Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

spencerstock left a comment

Choose a reason for hiding this comment

🟡 Follow-up needed on prior feedback · 77/100 · Re-review

Uh oh!

fan-zhang-sv commented Mar 20, 2026

Uh oh!

spencerstock left a comment

Choose a reason for hiding this comment

Uh oh!

stephancill Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spencerstock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fan-zhang-sv commented Mar 19, 2026 •

edited

Loading

cb-heimdall commented Mar 19, 2026 •

edited

Loading

stephancill Mar 23, 2026 •

edited

Loading