Skip to content

fix: allow review diffs for registered projects#4288

Open
StiensWout wants to merge 1 commit into
pingdotgg:mainfrom
StiensWout:fix/issue-4022-review-project-root
Open

fix: allow review diffs for registered projects#4288
StiensWout wants to merge 1 commit into
pingdotgg:mainfrom
StiensWout:fix/issue-4022-review-project-root

Conversation

@StiensWout

@StiensWout StiensWout commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • allow review diff previews for exact workspace roots of active registered projects
  • retain the existing configured-cwd and managed-worktree boundaries for all other paths
  • stop retrying failed diff previews against the server process cwd, which could silently render an unrelated empty result

Root cause

Packaged desktop servers use the user's home directory as ServerConfig.cwd. ReviewService only trusted that directory and T3's managed worktree directory, so in-place projects on another drive or outside the home directory were rejected before VCS detection. The web client then retried against ServerConfig.cwd, hiding the authorization error behind an empty diff from the wrong directory.

Impact

Working-tree and branch scopes now query the registered project's repository while preserving the arbitrary-cwd guard for unregistered paths. If validation genuinely fails, the original error remains visible instead of being replaced by an unrelated fallback result.

Closes #4022.

Validation

  • vp test run apps/server/src/review/ReviewService.test.ts (4 tests passed, including the outside-configured-root regression)
  • vp run --filter t3 typecheck
  • vp run --filter @t3tools/web typecheck
  • vp lint --report-unused-disable-directives apps/server/src/review/ReviewService.ts apps/server/src/review/ReviewService.test.ts apps/web/src/components/DiffPanel.tsx
  • vp fmt apps/server/src/review/ReviewService.ts apps/server/src/review/ReviewService.test.ts apps/web/src/components/DiffPanel.tsx --check
  • git diff --check
  • live isolated server started with an unrelated configured cwd and accepted registration of an external dirty Git fixture; browser interaction could not be completed because the T3 collaborative preview reported that no automation host was available

Note

Medium Risk
Expands the review diff authorization boundary to registered project paths; behavior change is intentional but affects path validation for a security-sensitive API.

Overview
Review diff previews now accept a cwd that matches an active registered project’s workspace root, even when that path is outside ServerConfig.cwd or the managed worktrees directory. Unregistered paths still hit the same rejection as before.

ReviewService.assertWorkspaceBoundCwd consults ProjectionSnapshotQuery.getActiveProjectByWorkspaceRoot after the existing root checks; lookup failures surface as VcsRepositoryDetectionError instead of a generic “outside workspace” message.

DiffPanel no longer retries branch/working-tree diff previews against the server process cwd when the API complains about the configured workspace root—that fallback could show an empty diff from the wrong repo. The UI uses a single branchDiffPreview query at the thread/project activeCwd.

Tests mock projection lookup and add a regression case for an external registered project root.

Reviewed by Cursor Bugbot for commit a14a504. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Allow diff preview requests for registered project workspace roots

  • ReviewService.assertWorkspaceBoundCwd now accepts a cwd that matches a registered project's workspace root via ProjectionSnapshotQuery.getActiveProjectByWorkspaceRoot, in addition to the existing within-root check.
  • DiffPanel.tsx removes the two-step retry logic (primary request + fallback using serverConfig.cwd), replacing it with a single request scoped to activeCwd.
  • Errors from the registered project lookup are surfaced as VcsRepositoryDetectionError with the operation tag ReviewService.assertWorkspaceBoundCwd.resolveRegisteredProject.
  • Behavioral Change: the client no longer retries with the environment cwd on workspace root validation failures; all diff preview requests use activeCwd directly.

Macroscope summarized a14a504.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9c72aefa-5c73-491e-9900-acfd6fc10ad5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jul 22, 2026
@macroscopeapp

macroscopeapp Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR changes runtime behavior by adding an early-return path that bypasses workspace boundary validation for registered projects, and removes fallback retry logic in the web component. These behavioral changes to validation gating warrant human review.

You can customize Macroscope's approvability policy. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

1 participant