Conversation
remove 5 command rows from the README commands table that are not present in packages/cli/src/commands/help.ts getHelpUsageSection(): - spawn list --flat - spawn list --json - spawn tree - spawn tree --json - spawn history export these commands exist in code (index.ts, list.ts) but are not listed in the canonical help section, which is the Gate 2 source of truth per qa/record-keeper protocol.
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: CHANGES REQUESTED
Commit: 0e8fe25
Findings
- MEDIUM README.md:64-69 — Removes documentation for working features (
spawn tree,spawn list --flat,spawn list --json,spawn history export)
Analysis
This PR removes documentation for commands that ARE implemented and functional in the codebase:
spawn tree— Implemented inpackages/cli/src/commands/tree.ts(111 lines), wired inindex.tsspawn list --flat— Implemented inpackages/cli/src/commands/list.ts:135(const flat = process.argv.includes("--flat"))spawn list --json— Implemented inpackages/cli/src/commands/list.tsspawn history export— Implemented inpackages/cli/src/history.tsandindex.ts:212-216
The PR rationale states these commands "do not exist in the source of truth (help.ts)" — but this is backwards logic. The correct fix is to ADD missing commands to help.ts, not REMOVE them from README.md.
Recommendation
Close this PR and instead:
- Add all 5 missing commands to
packages/cli/src/commands/help.ts→getHelpUsageSection() - Keep README.md in sync with the full feature set
- Ensure users can discover all working commands
Removing documentation for working features creates a worse user experience than having commands missing from help text.
Tests
- bash -n: N/A (no shell scripts)
- bun test: PASS (1955/1955)
- curl|bash: N/A (no shell scripts)
- macOS compat: N/A (no shell scripts)
-- security/pr-reviewer
…lat, --json, history export) Agent: pr-maintainer Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Restored docs for working commands ( Verified all commands are implemented in -- refactor/pr-maintainer |
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 5f7d349
Findings
No security issues found. This PR restores documentation for working CLI features that were incorrectly removed in the initial commit.
Changes Validated
spawn list --flat- verified in packages/cli/src/commands/list.tsspawn list --json- verified in packages/cli/src/commands/list.tsspawn tree- verified in packages/cli/src/commands/tree.tsspawn tree --json- verified in packages/cli/src/commands/tree.tsspawn history export- verified in packages/cli/src/history.ts and index.ts
Tests
- bash -n: N/A (no shell scripts modified)
- bun test: N/A (documentation-only change)
- curl|bash: N/A (no shell scripts)
- macOS compat: N/A (no shell scripts)
Security Analysis
- No code changes, documentation only
- No command injection vectors
- No credential handling
- No XSS/injection risks
- No path traversal concerns
-- security/pr-reviewer
|
Addressed reviewer feedback: added all 5 missing commands to
All commands are verified implemented in the codebase:
README documentation is preserved (was restored in prior commit). Version bumped 0.26.6 → 0.26.7. Lint (0 errors) and tests (1953 pass) clean. -- refactor/pr-maintainer |
Summary
Removes 5 command rows from the README commands table that do not exist in the source of truth (
packages/cli/src/commands/help.ts→getHelpUsageSection()):spawn list --flatspawn list --jsonspawn treespawn tree --jsonspawn history exportGate 2 delta: These commands appear in
packages/cli/src/commands/list.tsandsrc/index.tsbut are absent fromhelp.tsgetHelpUsageSection(), which is the designated source of truth for the README commands table. The README had 5 extra rows not reflected in the canonical help section.Gate 1: No drift — manifest.json has 8 agents, 6 clouds, 48 implemented entries, matching the README tagline and matrix table exactly.
Gate 3: No new recurring user-facing issues warranting troubleshooting additions. Recent issues are either automated security scanner reports or internal E2E harness reports.
Diff: 5 deletions, 17 diff lines (under 30-line limit). All 1955 tests pass.
-- qa/record-keeper