fix: resolve consistency issues across CLI codebase#101
Merged
Conversation
Address 14+ inconsistencies found during a full repository review: - Use findProjectRoot() consistently in all project-aware commands instead of process.cwd() (adr create/list/show/update, plugin install) - Add try-catch error boundaries to all async command actions (review-context, session-context/*, adr/*, plugin/url, upgrade) - Remove duplicate getChangedFiles() from helpers/git.ts (dead code) - Fix login hint: "login --refresh" -> "login refresh" (subcommand) - Add .choices(ADR_DOMAINS) to adr list --domain option - Fix adr show checking wrong path (adrsDir vs root) - Extract shared EDITOR_LABELS to helpers/init-project.ts - Remove cursor from plugin url --editor (cursor uses bundle API) - Fix configureVscodeSettings creating empty .vscode/ without reason - Fix frontmatter regex for Windows \r\n line endings - Fix homedir fallback using "~" literal instead of os.homedir() - Standardize AbortSignal.timeout formatting (15000 -> 15_000) - Fix docs/astro.config.mjs softwareVersion (0.11.0 -> 0.16.0) - Remove dead template path constants from helpers/paths.ts - Rename local findProjectRoot in upgrade.ts to findPackageRoot - Document devDependencies design decision in package.json Add three new ADRs with companion rules to prevent recurrence: - ARCH-011: Consistent project root resolution (no-process-cwd rule) - ARCH-012: Command error boundaries (async-action-error-boundary rule) - ARCH-013: Version synchronization (docs-version-sync rule)
Deploying archgate-cli with
|
| Latest commit: |
1c0faef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f3b4be23.archgate-cli.pages.dev |
| Branch Preview URL: | https://fix-consistency-review.archgate-cli.pages.dev |
- Merge upstream installGit tests into consolidated test file (removed duplicate getChangedFiles import from upstream) - Fix adr list test: create .archgate/adrs/ dir (not just .archgate/) to match findProjectRoot() expectations
- Document that .simple-release.js already handles optionalDependencies sync during the release bump hook - Add optional-deps-version-sync rule to catch drift between package.json version and platform-specific npm package versions - Update ADR context to list all three version touch points
Replace non-standard _dependencies_note field in package.json with a proper "npm Distribution" section in CLAUDE.md explaining that the npm package is a thin shim and all runtime deps are bundled into the binary.
Add softwareVersion replacement to .simple-release.js bump hook so docs/astro.config.mjs is updated automatically during release, matching the existing optionalDependencies sync pattern. The ARCH-013 archgate rule remains as a CI safety net.
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
Full repository consistency review that fixes 14+ issues and adds 3 new ADRs with automated rules to prevent recurrence.
Code fixes (24 files, net -40 lines)
findProjectRoot()instead ofprocess.cwd(), so they work from subdirectories (adr create/list/show/update, plugin install)getChangedFiles()fromhelpers/git.ts(canonical version lives inengine/git-files.ts)login --refreshhint: Was showing--refresh(flag syntax) but it's a subcommand (login refresh)adr list --domain: Added.choices(ADR_DOMAINS)validation (was accepting any string)adr showpath check: Was checkingadrsDirexistence but reporting.archgate/missingEDITOR_LABELS: Was duplicated ininit.tsandplugin/install.tsplugin url --editor: Cursor uses a bundle API, not marketplace URLs.vscode/dir creation: No longer creates empty directory when no marketplace URL\n->\r?\nfor\r\nline endingshomedir()fallback: Was using literal"~"string that doesn't expand15000->15_000in binary-upgrade.tssoftwareVersion: Was0.11.0, now0.16.0matching package.jsontemplatesRemoteArchive,templatesZipFile, etc._dependencies_noteexplaining the shim architectureNew ADRs & automated rules (6 new files)
no-process-cwd-for-project-rootprocess.cwd()instead offindProjectRoot()async-action-error-boundarydocs-version-syncsoftwareVersionin docs diverging from package.jsonValidation
bun run validatepasses (lint, typecheck, format, 324 tests, 20 rules, build)Test plan
bun run validatepasses locally