fix: scope project-path guards to the requested project (shared roots)#181
Conversation
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
There was a problem hiding this comment.
Code Review
This pull request refactors project path matching by extracting projectContainsPath and updates the broker:list-personas IPC handler to correctly handle multiple projects sharing the same root. It also optimizes terminal performance by coalescing multiple PTY chunks into a single write, and resolves a terminal focus issue by ensuring xterm is focused exactly once on the first mount of a new runtime. Feedback on the changes suggests addressing a potential bug in projectContainsPath where paths starting with double dots (e.g., ..foo) could be incorrectly flagged as being outside the root directory.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const pathFromRoot = relative(rootPath, resolved) | ||
| return pathFromRoot === '' || (!pathFromRoot.startsWith('..') && !isAbsolute(pathFromRoot)) |
There was a problem hiding this comment.
The check `pathFromRoot.startsWith('..')` can incorrectly return `true` (indicating the path is outside the root) if a subdirectory or file name starts with `..` (for example, a folder named `..foo` inside the project root). To prevent this false positive, we should ensure that `..` is either the entire path segment or followed by a path separator (`/` or `\\``).
const pathFromRoot = relative(rootPath, resolved)\n const isOutside = pathFromRoot === '..' || pathFromRoot.startsWith('../') || pathFromRoot.startsWith('..\\\\')\n return pathFromRoot === '' || (!isOutside && !isAbsolute(pathFromRoot))|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
Warning Review limit reached
More reviews will be available in 49 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR refactors path validation logic in the main process by extracting a ChangesPath Validation Refactor & Terminal Runtime Optimizations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
broker:list-personas rejected a valid cwd whenever that directory was a registered root of more than one project. The guard used getProjectIdForPath() -> findProjectForPath(), which returns the FIRST project (in store order) whose roots contain the path, then compared that to the passed projectId. When a directory is shared as a root across projects, every project except the first-listed one was wrongly rejected with "Path is outside project roots". Scope the check to the project actually requested: extract projectContainsPath(project, targetPath) in cli.ts and have the handler look up the passed projectId and test the cwd against that project's own roots. findProjectForPath now delegates to the helper, preserving first-match behavior for CLI/open flows. Adds a regression test for the shared-root case and unit coverage for the new helper. broker:spawn-agent does not use this guard and is unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
git:generate-commit-message inferred its project via getProjectIdForPath() (global first-match), so when the working dir was a root shared across projects the commit draft was generated by the first-listed project's broker rather than the one the user was working in — wrong attribution under shared roots. Thread projectId from the renderer instead: DiffPane passes activeProjectId through git-store -> preload -> IPC. The handler now looks up the passed project, verifies projectContainsPath(project, path), and generates the draft with that id. The broad assertPathWithinProjects(path) boundary guard is kept. Signature is generateCommitMessage(projectId, rootPath, input) to match the IPC handler shape. Adds handler tests for shared-root attribution and for rejecting a path outside the passed project. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
35d9e25 to
d647e05
Compare
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
1 similar comment
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #181 in AgentWorkforce/pear. |
Two related shared-root bugs where IPC handlers inferred the project from a path via global first-match (
getProjectIdForPath→findProjectForPath) instead of using the project the user is actually in. When a directory is a registered root of more than one project, first-match picks the wrong one.Bug 1 —
broker:list-personashard failureListing personas / spawning an agent threw
Path is outside project roots for project <id>: <cwd>for a valid cwd whenever that directory was a root of multiple projects.Field repro:
/Users/.../AgentWorkforce/workforceis a root of three projects (Proactive Agents, Relay Core, Workforce). From the Workforce project it failed, because the guard resolved the path to the first-listed owner (Proactive Agents) and that id ≠ the requested id.Fix: extract
projectContainsPath(project, targetPath)incli.ts; the handler looks up the passedprojectIdand tests the cwd against that project's roots.findProjectForPathnow delegates to the helper (first-match behavior preserved for CLI/open flows).Bug 2 —
git:generate-commit-messagewrong attributionSame root cause, milder symptom: the handler used
getProjectIdForPath(path), so under a shared root the commit draft was generated by the first-listed project's broker, not the one the user was working in (no error — just wrong context).Fix: thread
projectIdfrom the renderer (DiffPanealready hasactiveProjectId) through git-store → preload → IPC types → handler. The handler verifiesprojectContainsPath(project, path)and generates the draft with the passed id. Signature isgenerateCommitMessage(projectId, rootPath, input). The broadassertPathWithinProjects(path)boundary guard is kept.broker:spawn-agentwas audited and does not use this guard (it forwardsprojectIdtobrokerManager.spawnAgentand selects the session by id), so it is unaffected.Tests
list-personasshared-root regression: two projects share/tmp/shared;list-personas('project-2', '/tmp/shared')checksprojectContainsPathagainst project-2 and succeeds.generate-commit-messageshared-root attribution: uses the passedproject-2, and rejects a path outside the passed project.projectContainsPathunit tests: exact root, nested path, sibling-prefix non-match; existing first-match test kept.cli.test.ts14/14,ipc-handlers.test.ts8/8,tsc --noEmitclean.Note
Main-process change — requires a rebuild + restart of Pear to take effect.
🤖 Generated with Claude Code