feat: install from lockfile#48
Conversation
📝 WalkthroughWalkthroughAdds an ChangesInstall Command Feature
Sequence DiagramsequenceDiagram
participant CLI as runInstallCommand
participant Sync as runSync
participant Plan as getSyncPlan
participant Build as buildInstallResult
participant Validate as assertInstallLock
participant Fetch as createJobRunner
participant Finalize as finalizeSync
CLI->>Sync: invoke with install: true, lockOnly: false
Sync->>Sync: reject install && lockOnly
Sync->>Plan: request plan (install branch)
Plan->>Build: buildInstallResult (derive resolvedCommit from lock)
Build->>Plan: set status (missing/changed/up-to-date)
Sync->>Validate: assertInstallLock(plan)
Validate->>Sync: throw if lock missing/stale or repo/ref drift
Sync->>Fetch: createJobRunner using lock repo/ref
Fetch->>Fetch: fetch and materialize from lock values
Sync->>Finalize: finalize without rewriting docs-lock.json
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Pull request overview
Adds a new docs-cache install [id...] workflow to restore cache/targets directly from docs-lock.json without rewriting the lock file, enabling more deterministic installs (e.g., in CI or postinstall) while still failing when the lock is missing or out-of-date.
Changes:
- Introduces
installmode in the sync planner/executor to use lock data and skip lockfile writes. - Extends CLI parsing/dispatch to support
docs-cache install [id...]. - Adds tests for install behavior and CLI parsing, and updates README guidance.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/install.test.js | Adds install-mode integration tests (materialize from lock; fail on rules drift). |
| tests/cli-parse.test.js | Adds CLI parser test for install positional source filters. |
| src/types/sync.ts | Extends SyncOptions with install?: boolean. |
| src/commands/sync.ts | Implements install-mode planning, lock assertions, and skips lock rewrites during install. |
| src/cli/types.ts | Adds install to the CLI command union. |
| src/cli/parse-args.ts | Adds install command parsing and help registration. |
| src/cli/index.ts | Adds install command execution path and wiring into runSync. |
| README.md | Documents install usage and updates postinstall guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/sync.ts`:
- Around line 793-796: The drift detection in driftedSources uses source.ref
directly which misses the fallback to plan.defaults.ref used elsewhere; update
the filter in driftedSources to compute the effective ref (e.g., effectiveRef =
source.ref ?? plan.defaults?.ref or source.ref || plan.defaults?.ref) and
compare lockEntry.ref against that effectiveRef (keep the existing repo
comparison with source.repo), so inherited default refs are treated as matching
the lock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31970230-570c-4edb-9c3b-a1199d62b805
📒 Files selected for processing (3)
README.mdsrc/commands/sync.tstests/install.test.js
✅ Files skipped from review due to trivial changes (1)
- README.md
| const driftedSources = plan.sources.filter((source) => { | ||
| const lockEntry = plan.lockData?.sources[source.id]; | ||
| return lockEntry?.repo !== source.repo || lockEntry.ref !== source.ref; | ||
| }); |
There was a problem hiding this comment.
Compare drift against the effective ref, not the raw source field.
Line 795 uses source.ref directly, but this file falls back to plan.defaults.ref elsewhere. If a source inherits the default ref, this will report a matching lock as drifted and make docs-cache install fail on a valid lock.
Suggested fix
const driftedSources = plan.sources.filter((source) => {
const lockEntry = plan.lockData?.sources[source.id];
- return lockEntry?.repo !== source.repo || lockEntry.ref !== source.ref;
+ if (!lockEntry) {
+ return false;
+ }
+ const expectedRef = source.ref ?? plan.defaults.ref;
+ return lockEntry.repo !== source.repo || lockEntry.ref !== expectedRef;
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/sync.ts` around lines 793 - 796, The drift detection in
driftedSources uses source.ref directly which misses the fallback to
plan.defaults.ref used elsewhere; update the filter in driftedSources to compute
the effective ref (e.g., effectiveRef = source.ref ?? plan.defaults?.ref or
source.ref || plan.defaults?.ref) and compare lockEntry.ref against that
effectiveRef (keep the existing repo comparison with source.repo), so inherited
default refs are treated as matching the lock.
Summary
docs-cache install [id...]to restore cache/targets fromdocs-lock.jsonwithout rewriting the lock file.Validation
pnpm testpnpm typecheckrtk proxy pnpm lintSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests