Honor requested worktree authority in routed reads - #244
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds explicit workspace-routing contracts, validates workspace identity, records route evidence, propagates it through MCP workers and receipts, and updates output schema versions. Tests cover multiple worktrees, typed failures, identity mismatches, and schema metadata. ChangesWorkspace routing and evidence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCPCaller
participant WorkspaceRouter
participant RepositoryTool
participant ReceiptBuilder
MCPCaller->>WorkspaceRouter: Request explicit workspace route
WorkspaceRouter->>WorkspaceRouter: Resolve workspace and validate identity hints
WorkspaceRouter->>RepositoryTool: Execute with workspace route evidence
RepositoryTool->>ReceiptBuilder: Provide workspace route evidence
ReceiptBuilder-->>MCPCaller: Return receipt and _workspace metadata
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 |
Activity Summary
CodeRabbit reported that it could not review this exact head because its review limit was reached, so the repository fallback gate applies: @codex review please |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3798d94a34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
The single P1 from the prior review is repaired in @codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8b5be779d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
The outline schema-version collision is fixed on exact head @codex review please |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/contracts/output-schema-meta.ts (1)
78-88: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the exported common-schema helpers aligned with
_workspace.The new interface fields are not added by
withMcpCommonorwithCliPeerCommon. Their generated schemas therefore omit the workspace-route contract that these interfaces declare.Add the workspace evidence schema to both helpers, or remove the duplicate helpers and use one shared implementation. Add a contract test for the exported helpers.
🤖 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/contracts/output-schema-meta.ts` around lines 78 - 88, Update the exported withMcpCommon and withCliPeerCommon helpers to include the _workspace evidence schema declared by their corresponding interfaces, preferably by reusing one shared implementation where appropriate. Ensure generated schemas preserve the existing common fields and add contract coverage for both exported helpers.
🤖 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.
Outside diff comments:
In `@src/contracts/output-schema-meta.ts`:
- Around line 78-88: Update the exported withMcpCommon and withCliPeerCommon
helpers to include the _workspace evidence schema declared by their
corresponding interfaces, preferably by reusing one shared implementation where
appropriate. Ensure generated schemas preserve the existing common fields and
add contract coverage for both exported helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 24e5a9e1-8062-4639-8d81-400e25f36ae7
⛔ Files ignored due to path filters (8)
CHANGELOG.mdis excluded by!**/*.mddocs/BEARING.mdis excluded by!**/*.mddocs/MCP.mdis excluded by!**/*.mddocs/SETUP.mdis excluded by!**/*.mddocs/design/SURFACE_requested-worktree-authority.mdis excluded by!**/*.mddocs/invariants/versioned-output-schemas.mdis excluded by!**/*.mddocs/method/retro/SURFACE_requested-worktree-authority/retro.mdis excluded by!**/*.mddocs/method/retro/SURFACE_requested-worktree-authority/witness/verification.mdis excluded by!**/*.md
📒 Files selected for processing (14)
src/contracts/capabilities.tssrc/contracts/output-schema-meta.tssrc/contracts/output-schemas.tssrc/mcp/receipt.tssrc/mcp/repo-tool-job.tssrc/mcp/repo-tool-worker-context.tssrc/mcp/server-invocation.tssrc/mcp/server-tool-access.tssrc/mcp/workspace-router-model.tssrc/mcp/workspace-router-resolution.tssrc/mcp/workspace-router.tstest/unit/contracts/output-schemas.test.tstest/unit/mcp/per-call-workspace-route.test.tstest/unit/mcp/workspace-binding.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a202c5703
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed the exact-head CodeRabbit finding in @codex review please |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Both overlapping exact-head findings are repaired on @codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dc6024e22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review please — exact-head follow-up on 6cda22e. The two prior findings are repaired and resolved: authorization now revalidates the full resolved workspace identity, and workspace evidence is limited to routed output contracts. Local gate: 45 focused tests; 258 files / 2,055 tests isolated; build, release surface, lint, typecheck, and diff checks pass. CodeRabbit is rate-limited, so this request is the repository fallback review. |
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/contracts/output-schema-meta.ts`:
- Around line 27-33: Update workspaceRouteEvidenceSchema so requestedRoot and
resolvedRoot validate with path.isAbsolute, rejecting empty and relative paths
while preserving the existing strict object shape. Add schema tests covering
both invalid empty and relative values.
🪄 Autofix
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 Plus
Run ID: 3661e4a1-80f5-47ed-87c3-cc2f28c5c891
⛔ Files ignored due to path filters (4)
CHANGELOG.mdis excluded by!**/*.mddocs/design/SURFACE_requested-worktree-authority.mdis excluded by!**/*.mddocs/method/retro/SURFACE_requested-worktree-authority/retro.mdis excluded by!**/*.mddocs/method/retro/SURFACE_requested-worktree-authority/witness/verification.mdis excluded by!**/*.md
📒 Files selected for processing (7)
src/contracts/output-schema-meta.tssrc/contracts/output-schemas.tssrc/mcp/daemon-control-plane.tssrc/mcp/workspace-router.tstest/unit/contracts/output-schemas.test.tstest/unit/mcp/per-call-workspace-route.test.tstest/unit/mcp/workspace-binding.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/mcp/workspace-router.ts
- src/contracts/output-schemas.ts
- test/unit/contracts/output-schemas.test.ts
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review please — final exact-head follow-up on 1cb82f2. The absolute-root schema finding is repaired and resolved. Local gate: 46 focused tests; 258 files / 2,056 tests isolated; build, release surface, lint, typecheck, and diff checks pass. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== HEAD ==='
git rev-parse HEAD
echo '=== PR base ==='
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
printf '%s\n' "$base"
echo '=== Changed files ==='
git diff --name-status "$base"...HEAD
echo '=== Diff: workspace route schemas and routing ==='
git diff --unified=80 "$base"...HEAD -- \
src/contracts/output-schema-meta.ts \
src/contracts/output-schemas.ts \
src/mcp/workspace-router-model.ts \
src/mcp/workspace-router-resolution.ts \
src/mcp/workspace-router.ts \
src/mcp/receipt.ts \
src/mcp/server-invocation.ts \
src/mcp/repo-tool-worker-context.ts \
test/unit/contracts/output-schemas.test.ts \
test/unit/mcp/per-call-workspace-route.test.ts \
test/unit/mcp/workspace-binding.test.ts
echo '=== Relevant outlines ==='
ast-grep outline src/contracts/output-schema-meta.ts --items all
ast-grep outline src/contracts/output-schemas.ts --items all
ast-grep outline src/mcp/workspace-router.ts --items all
ast-grep outline src/mcp/workspace-router-resolution.ts --items all
echo '=== Root and route-evidence references ==='
rg -n -C 4 --glob '*.ts' \
'(requestedRoot|resolvedRoot|workspaceRouteEvidenceSchema|WorkspaceRouteEvidence|_workspace)' \
src test/unitLength of output: 183350 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Final exact-head commit ==='
git show --stat --oneline --decorate --no-renames 1cb82f2d
git show --format=fuller --no-ext-diff --unified=60 1cb82f2d -- \
src/contracts/output-schema-meta.ts \
test/unit/contracts/output-schemas.test.ts
echo '=== Routed capability definitions ==='
sed -n '1,150p' src/contracts/capabilities.ts
echo '=== Invocation routing set and argument-schema references ==='
rg -n -C 5 --glob '*.ts' \
'(workspaceRoutedRepoTools|WORKSPACE_ROUTED_MCP_TOOL_NAMES|WORKSPACE_ROUTED_CLI_COMMAND_NAMES|cwd.*z\.string|z\.string.*cwd)' \
src test/unit
echo '=== Receipt build call sites and schema validation call sites ==='
rg -n -C 4 --glob '*.ts' \
'(buildReceiptResult\(|MCP_OUTPUT_SCHEMAS\[|validateMcpOutput|attachMcpSchemaMeta\()' \
src test/unit
echo '=== Output-schema exports and version references ==='
sed -n '1,65p' src/contracts/output-schemas.ts
rg -n -C 3 --glob '*.ts' \
'(OUTPUT_SCHEMA_V3_VERSION|OUTLINE_OUTPUT_SCHEMA_VERSION|OutputSchemaVersion)' \
src test/unit
echo '=== Documentation precedence and root wording ==='
rg -n -C 4 \
'(requested root|resolved root|request root|session root|daemon default|precedence|absolute root|absolute path)' \
docs/BEARING.md docs/MCP.md docs/SETUP.md docs/design/SURFACE_requested-worktree-authority.md docs/invariants/versioned-output-schemas.mdLength of output: 69999 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Workspace authorization identity check ==='
ast-grep outline src/mcp/daemon-control-plane.ts --items all
rg -n -C 12 --glob '*.ts' \
'(getCapabilityProfile|authorizeWorkspace|workspaceAuthorizations|repoId.*worktreeRoot|gitCommonDir)' \
src/mcp/daemon-control-plane.ts src/mcp
echo '=== Route evidence construction and all invocation contexts ==='
sed -n '535,690p' src/mcp/workspace-router.ts
sed -n '170,215p' src/mcp/server-invocation.ts
sed -n '325,370p' src/mcp/server-invocation.ts
sed -n '90,120p' src/mcp/repo-tool-worker-context.ts
echo '=== Available documentation paths ==='
fd -t f -i 'BEARING.md' -i 'MCP.md' -i 'SETUP.md' -i 'SURFACE_requested-worktree-authority.md' -i 'versioned-output-schemas.md' . || true
echo '=== Static diff checks ==='
git diff --check "$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"...HEAD
git status --shortLength of output: 135130
The absolute-root schema now rejects relative route roots. The route producer emits absolute requested and resolved roots. Routed output schemas and receipts use the same route-evidence contract. Authorization revalidates repository ID, worktree root, and Git common directory.
Documentation verification was inconclusive because the changed If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit!
|
1 participant
Summary
cwdselects either worktree of the same repository even while another is activeValidation
pnpm test— 258 files, 2,051 testspnpm buildpnpm lintpnpm typecheckpnpm release:surface-gategit diff --checkCloses #238
Summary by CodeRabbit
New Features
Bug Fixes
Compatibility