Improve context-store setup and cleanup UX#1137
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesContext Store Cleanup Commands and Interactive Setup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/cli.md`:
- Line 11: The Agent-Compatible Commands matrix is missing the newly added
context-store commands; update the table to include `context-store unregister`
and `context-store remove` as agent-compatible entries, documenting that both
accept `--json` for machine-friendly output and that `context-store remove`
additionally supports a non-interactive `--yes` flag for forced deletion; locate
the matrix generation/markdown row for "Shared context (beta)" and add these two
commands with the same formatting and notes used for other context-store
commands (e.g., matching how `context-store register`/`list` are represented).
In `@src/core/context-store/operations.ts`:
- Around line 183-206: In findContainingGitRepositoryRoot, don't swallow all
execFileAsync('git'...) failures; in the catch block perform a filesystem-only
parent walk from nearestParent looking for a ".git" entry (use
FileSystemUtils.canonicalizeExistingPath and path.dirname/path.join or
nearestExistingDirectory to ascend) and if you find a .git directory treat that
ancestor as the git root (compute comparableStoreRoot relative check as
currently done), otherwise throw a ContextStoreError that includes the original
error information instead of returning null; keep execFileAsync,
nearestExistingDirectory and FileSystemUtils calls as the primary utilities to
locate/normalize paths.
In `@src/core/context-store/registry.ts`:
- Around line 158-167: The equality check in contextStoreBackendsMatch treats
local_path as a raw string, so normalize both actual.local_path and
expected.local_path with the same canonicalization helper used by conflict
detection (e.g., canonicalizeLocalPath or the project's path canonicalizer)
before comparing; update contextStoreBackendsMatch to call that helper on both
local_path values and compare the canonicalized results while leaving
comparisons of actual.type, actual.remote, and actual.branch unchanged.
In `@test/commands/context-store.test.ts`:
- Around line 439-450: Tests currently compare raw temp paths (storeRoot)
against registry/cleanup identities which can differ when temp dirs are
symlinked; update assertions to canonicalize expected paths by calling
expectedExistingPath(storeRoot) and use that canonical value in the
parseJson(...) assertions (e.g., where parseJson(result) is asserted to contain
files.left_on_disk or registry identities). For the successful remove case,
capture the canonical path into a variable before invoking runCLI() (so the
folder can be deleted) and assert against that captured canonical value; ensure
you call expectedExistingPath(storeRoot) wherever the test currently uses raw
storeRoot for path identity comparisons (including the other two similar
blocks).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bdcad684-0a3b-4ede-ba8b-a215f088d4d8
📒 Files selected for processing (14)
docs/cli.mddocs/workspaces-beta/agent-cli-playbook.mddocs/workspaces-beta/user-guide.mdopenspec/initiatives/context-store-and-initiatives/tasks.mdopenspec/initiatives/context-store-and-initiatives/work-items/12-context-store-first-run-and-cleanup-ux/evidence.mdopenspec/initiatives/context-store-and-initiatives/work-items/12-context-store-first-run-and-cleanup-ux/plan.mdopenspec/initiatives/context-store-and-initiatives/work-items/12-context-store-first-run-and-cleanup-ux/tasks.mdsrc/commands/context-store.tssrc/core/completions/command-registry.tssrc/core/context-store/operations.tssrc/core/context-store/registry.tstest/commands/context-store.test.tstest/core/completions/command-registry.test.tstest/core/context-store/registry.test.ts
Contributor
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 `@test/core/context-store/registry.test.ts`:
- Around line 545-548: The assertion currently compares the raw storeRoot
string; canonicalize filesystem identity by applying fs.realpathSync.native() to
the expected storeRoot in the test. Update the assertion that checks the
objectContaining for id 'team-context' to use storeRoot:
fs.realpathSync.native(storeRoot) (or realpath the actual result if you prefer),
ensuring you use the same fs.realpathSync.native() call used elsewhere in this
test file so both expected and actual filesystem paths are compared by canonical
identity.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 57d11014-f510-4adb-9eaf-28405df2db6e
📒 Files selected for processing (6)
docs/cli.mdsrc/core/context-store/foundation.tssrc/core/context-store/operations.tssrc/core/context-store/registry.tstest/commands/context-store.test.tstest/core/context-store/registry.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/cli.md
- src/core/context-store/registry.ts
- test/commands/context-store.test.ts
- src/core/context-store/operations.ts
alfred-openspec
approved these changes
May 28, 2026
Contributor
|
Actionable comments posted: 0 |
linjinze999
pushed a commit
to linjinze999/OpenSpec
that referenced
this pull request
Jun 4, 2026
* Improve context-store setup and cleanup UX * Address CodeRabbit context-store feedback * Canonicalize cleanup registry test assertion
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
2 participants
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
context-store setupwith explicit non-interactive/JSON validationcontext-store unregisterandcontext-store removecleanup flows with docs, completions, and testsVerification
pnpm buildpnpm lintpnpm vitest run test/commands/context-store.test.ts test/core/context-store/registry.test.tspnpm testSummary by CodeRabbit
New Features
context-store unregister(forget registration) andcontext-store remove(forget + delete local store; non-interactive requires--yes)context-store setupsupports interactive no-arg guided setup and deterministic non-interactive JSON usageBehavior / Safety
Documentation
Tests