feat(cli): OAC CLI package — npm-based install, update, and management#298
Open
darrenhinde wants to merge 11 commits intomainfrom
Open
feat(cli): OAC CLI package — npm-based install, update, and management#298darrenhinde wants to merge 11 commits intomainfrom
darrenhinde wants to merge 11 commits intomainfrom
Conversation
- Replace fs-extra with Bun.file/Bun.write/node:fs/promises across all 9 source files - Fix --help fast-path so all 8 subcommands are visible - Replace tsup/tsx/vitest with bun build/bun run/bun:test - Replace __dirname with import.meta.dir; use JSON import assertion for version - Fix hardcoded OAC_VERSION in add.ts; add hashesMatch import to status.ts - Replace computeFileHash with Bun.file().bytes() in sha256.ts - Rename checkNodeVersion → checkBunVersion; parallelise doctor checks - Fix TypeScript void-inference errors in update.ts, list.ts, status.ts by replacing .catch() with let/try-catch where result is used downstream - Add ManifestError named error class; remove unsafe type casts - Add 43 bun:test unit tests (sha256, manifest, installer, version) — 0 failures
Critical bug fixes:
- ide-detect.ts: replace Bun.file(dir).exists() with stat().isDirectory()
for all directory checks — Bun.file().exists() always returns false for
directories, breaking Cursor/Windsurf/OpenCode/Claude detection entirely
- bundled.ts: add registry.json exclusion anchor to findPackageRoot() to
prevent monorepo root from matching before the CLI package root
Standards cleanup (§4.1, §5.1, §6.1, §15.3, §21.1):
- status.ts: parallelise findModifiedFiles + detectIdes with Promise.all
- apply.ts: fix duplicate warn/limit messages; remove else after return in
reportWarnings; remove redundant mkdir before Bun.write
- version.ts: remove unnecessary (pkgJson as {version?:string}) cast
- manifest.ts: remove redundant mkdir before Bun.write; remove as unknown cast
- installer.ts: remove redundant mkdir calls before Bun.write throughout
- add.ts: add comment explaining why node:fs/promises rm is used
Tests (43 → 142, +99 new tests across 4 new files):
- ide-detect.test.ts: 26 tests covering all 4 IDEs, both claude indicators,
detectIdes parallel, isIdePresent — directly validates the directory fix
- bundled.test.ts: 27 tests for classifyBundledFile, findPackageRoot,
listBundledFiles, getBundledFilePath, bundledFileExists
- config.test.ts: 25 tests for readConfig/writeConfig round-trips,
createDefaultConfig, mergeConfig, isYoloMode, isAutoBackup
- installer-update.test.ts: 14 tests covering all 5 updateFiles decision
branches (install/update/skip/yolo/dry-run) plus isProjectRoot
- sha256.test.ts: +4 tests for empty file, large file, binary content
…solution - Remove duplicate --dry-run/--yolo/--verbose from parent program; Commander.js global option stealing caused all safety flags to be silently dropped in every subcommand action callback - Fix isProjectRoot() to use stat() for .git detection; Bun.file().exists() returns false for directories, breaking oac init in standard git repos - Add OAC_PACKAGE_ROOT env var override to getPackageRoot() for dev/monorepo mode; registry.json heuristic excluded the repo root causing oac init/add/update to throw when run from source - Fix build script: remove --banner flag that caused double shebang in dist/index.js - Rewrite bin/oac.js to invoke bun instead of node (dist is bun-only target) - Refactor installer.ts let accumulators to const using Promise.all + reduce - Add MVP planning docs (00-MVP-PLAN.md, master synthesis, project breakdown)
The get_registry_key() function was always adding 's' to the type, causing 'contexts' to become 'contextss' which doesn't exist in registry.json. Now handles: - Singular forms: context → contexts, agent → agents, skill → skills - Plural forms: contexts → contexts (unchanged), agents → agents - Config stays singular - Fallback for any type ending in 's' Fixes #257
…ot, update check Batch A (subtasks 01, 04-07, 09-11): - publishConfig.access: public in root + packages/cli package.json - engines.bun: >=1.0.0 added to root package.json - repository.directory set in both package.json files - version synced to 0.7.1; version.ts reads root package.json - scripts/sync-version.js: keeps packages/cli version in sync on bump - warn() output moved to stderr (console.error) in logger.ts - bin/oac.js: injects OAC_PACKAGE_ROOT, Windows bun.cmd support - bundled.ts: removed !hasRegistryJson guard from findPackageRoot() - manifest.ts: mkdir guard before writeManifest() - index.ts: SIGINT/SIGTERM signal handlers Batch B (subtasks 02, 03, 08, 12): - .npmignore: anchored /dist/, removed packages/ blanket exclusion - prepublishOnly: typecheck → build → dist existence check - packages/cli package.json: removed bin field, added private: true - update-check.ts: new module with fetchLatestNpmVersion, shouldShowUpdateNotice, checkForUpdate (24h cache, 3s timeout, stderr output, never throws) - doctor.ts: refactored to import fetchLatestNpmVersion from update-check.ts - index.ts: checkForUpdate() called after parseAsync; help() before update check Test gates: 202 pass, 14 fail (14 remaining are clean command gates for Batch C)
…tall
- clean.ts: new oac clean command with --force, --dry-run, --keep-opencode, --ide
Removes .oac/ and .opencode/ (or just .oac/ with --keep-opencode).
--ide also removes CLAUDE.md. --dry-run previews without removing.
Registered in index.ts alongside all other commands.
- Help examples: addHelpText('after', ...) on main program, init, and update.
Main program shows 7 examples including correct 'oac add agent:openagent' syntax.
init examples cover --dry-run, --verbose, --yolo with post-init doctor tip.
update examples cover --dry-run, --check, --yolo, --verbose with backup note.
- README: npm install section added before curl section in Quick Start.
Includes Bun prerequisite warning, global install, npx one-liner, update command.
Curl section heading renamed to 'Step 2: Install via curl' (no duplicate Step 1).
Tests: 216 pass, 0 fail
…t, exit codes B-2: oac clean --ide now removes all three IDE output files (.cursorrules and .windsurfrules added alongside CLAUDE.md) 2 new tests added to clean.test.ts B-3: oac update now writes manifest for successful files even on partial failure Removed the errors.length === 0 gate on writeManifest Warning message updated: 'manifest updated for successful files. Re-run to retry failures.' 3 new tests added in update.test.ts B-4: scripts/ dev tooling removed from npm package (46 files → 1 file) package.json files: 'scripts/' → 'scripts/sync-version.js' .npmignore: removed !scripts/ negation scripts/README.md excluded via files field negation W-3: oac clean sets process.exitCode = 1 when any removal fails hadError flag tracks failures; process.exitCode = 1 set after loop 1 new test added (chmod-based failure simulation) O-1: removed dead --verbose option from oac clean Option was registered but silently ignored; removed entirely Tests: 222 pass, 0 fail (was 216)
Remove duplicate "Step 2: Install via curl" heading and renumber "Step 2: Start Building" since old Step 1 no longer exists. Add review report and fix plan documents. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add path traversal guard in registry schema (reject absolute paths and ..) - Add containment check in add.ts before writing files outside project root - Validate OAC_PACKAGE_ROOT is absolute path before trusting it - Add depth limit (10) to findPackageRoot directory walk - Add schema validation to update-check cache reader - Wrap writeManifest in try/catch in update.ts - Normalize manifest keys to POSIX paths with traversal rejection - Fix empty HOME display bug in status.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@nextsystems/oac)init,update,add,apply,doctor,list,status,cleanCurrent State: IN PROGRESS
What's done
Remaining items from second code review (security hardening)
Critical (must fix before merge):
registry.ts—files[]array needs same path traversal refine aspathfieldmanifest.ts—removeFileFromManifestneeds same POSIX normalization asaddFileToManifestHigh:
3.
add.ts—removeCommandneeds containment check beforerm({ recursive: true })4.
update-check.ts—latestVersionfield type not validated in cache readerMedium:
5.
manifest.ts—includes('..')check is substring-based, should be segment-level (split('/').some(s => s === '..'))6.
update.ts— add warning about inconsistent state when manifest write fails7.
bundled.ts— off-by-one in depth limit (11 iterations, not 10)Not started:
8. Version bump 0.7.1 → 1.0.0 (when ready to ship)
Key files
packages/cli/— the full CLI packagebin/oac.js— Node.js entry pointdocs/planning/fix-plans/— all review findings and fix plans.npmignore— publish configTest plan
bun test— 222 tests passing, 0 failuresnpm pack --dry-run— verify published tarball contentsnpm install -g .— verify global install worksoac initin a test project🤖 Generated with Claude Code