Skip to content

fix: diff panel uses AI session CWD when terminal is linked (#3)#112

Merged
InbarR merged 1 commit into
InbarR:mainfrom
yoziv:bug/yoziv/fix-diff-panel-cwd-resolution
May 24, 2026
Merged

fix: diff panel uses AI session CWD when terminal is linked (#3)#112
InbarR merged 1 commit into
InbarR:mainfrom
yoziv:bug/yoziv/fix-diff-panel-cwd-resolution

Conversation

@yoziv
Copy link
Copy Markdown
Contributor

@yoziv yoziv commented May 18, 2026

Summary

Fixes #3 — Diff panel fails to resolve git root when the Copilot CLI changes directory internally.

Problem

When the Copilot CLI changes directory via its cwd command, the diff panel fails with:
Error invoking remote method 'diff:resolveGitRoot': No git repository found from: C:\Users

The shell's CWD (tracked via OSC 7/9;9/prompt regex) stays stale because the CLI changes directory internally without emitting escape sequences.

Solution

Instead of overwriting terminal.cwd (which was rejected by 4 adversarial reviewers due to ping-pong, WSL corruption, and session restore poisoning), the diff panel's CWD selector now checks the linked AI session's CWD first:

// Before: always reads shell CWD
const terminalCwd = useTerminalStore(s =>
  diffReviewTerminalId ? s.terminals.get(diffReviewTerminalId)?.cwd ?? '' : ''
);

// After: prefers AI session CWD, falls back to shell CWD
const terminalCwd = useTerminalStore(s => {
  const t = diffReviewTerminalId ? s.terminals.get(diffReviewTerminalId) : null;
  if (!t) return '';
  if (t.aiSessionId) {
    const sess = s.copilotSessions.find(x => x.id === t.aiSessionId)
              ?? s.claudeCodeSessions.find(x => x.id === t.aiSessionId);
    if (sess?.cwd) return sess.cwd;
  }
  return t.cwd ?? '';
});

Why this approach

  • Zero two-writers riskterminal.cwd stays purely shell CWD, untouched
  • Works for both Copilot and Claude Code sessions
  • No new IPC, no PTY writes, no race conditions
  • No session-restore poisoning — saved workspace CWD is always the shell's real CWD
  • Naturally reactive — when the session monitor updates workspace.cwd, the store re-renders the diff panel
  • Graceful fallback — when no AI session is linked, behaves exactly as before

Changes

File Change
src/renderer/components/DiffReview.tsx CWD selector checks AI session CWD first
tests/e2e/issue-3-diff-cwd-resolution.spec.ts Regression tests for linked and unlinked terminals

When the Copilot CLI changes directory internally (via cwd command),
the diff panel failed with 'No git repository found from: C:\Users'
because terminal.cwd (shell CWD) stays stale — the CLI doesn't emit
OSC sequences.

Instead of overwriting terminal.cwd (which causes ping-pong, WSL path
corruption, and session restore poisoning), the diff panel's CWD
selector now checks the linked AI session's CWD first, falling back
to shell CWD when no session is linked.

Closes #3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@InbarR
Copy link
Copy Markdown
Owner

InbarR commented May 22, 2026

Reviewed the change end-to-end.

Verdict: looks good to merge.

The selector correctly reads the linked AI session's cwd (from copilotSessions / claudeCodeSessions, keyed by terminal.aiSessionId) and falls back to the shell cwd. The "no two writers" reasoning matches the architecture - session monitor owns workspace.cwd, the OSC 7 pipeline owns terminal.cwd. The added Playwright spec exercises Windows-style paths and covers both linked and unlinked terminals.

CI: The failing Playwright (Windows) job is a pre-existing main-branch flake hitting unrelated specs (clawpilot-cwd-detection, task-70 image path click, ai-session-sort-and-group, task-61 rich text paste). The new issue-3-diff-cwd-resolution.spec.ts is not in the failure list - it passed. The same job fails on every recent PR for the same set of specs. Worth a separate cleanup pass on those flakes, but not blocking here.

Nit (not blocking): the new spec re-declares the selector inline instead of importing it - so a future refactor of the real selector won't break the test. The inline comment ("mirrors DiffReview.tsx's terminalCwd selector") makes the intent explicit, so this is fine as-is.

Heads-up: PR #114 from the same author rewrites this same selector with an explicit helper. If #114 lands first, this PR's inline change becomes redundant and should rebase on top.

Copy link
Copy Markdown
Owner

@InbarR InbarR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - small targeted fix for the diff panel CWD when a pane is linked to an AI session. The 18-line selector swap is correct for the happy path, and the e2e test covers the linked/unlinked transitions.

Noted that the stale-but-linked edge case (AI session went idle but aiSessionId not cleared) isn't handled here, but you've called it out as a follow-up - happy for that to land separately.

The Playwright (Windows) failures in CI look like a pre-existing base-branch issue: the same ~32 tests fail on multiple unrelated PRs touching different areas. Shouldn't block this PR.

Approving.

@yoziv yoziv closed this May 24, 2026
@yoziv yoziv reopened this May 24, 2026
@InbarR InbarR merged commit ecf2004 into InbarR:main May 24, 2026
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants