Skip to content

Use stored worktree path when creating task terminals#3074

Merged
charlesvien merged 3 commits into
mainfrom
posthog-code/fix-shell-external-worktree
Jul 2, 2026
Merged

Use stored worktree path when creating task terminals#3074
charlesvien merged 3 commits into
mainfrom
posthog-code/fix-shell-external-worktree

Conversation

@haacked

@haacked haacked commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Task terminals fail to open when a task reuses an existing worktree that lives outside the managed worktree directory. ShellService.getTaskEnv always recomputed the worktree path from the managed layout, and that derived path doesn't exist on disk for reused worktrees, so simple-git throws and the terminal never opens.

Changes

  • getTaskEnv now uses the worktree's stored path when it exists on disk, and only falls back to deriving the path from the managed layout for stale rows.
  • Workspace env construction failures are caught, so the shell still opens (without the POSTHOG_CODE_* vars) instead of failing terminal creation entirely.
  • Added test coverage for the stored path, the fallback path, and graceful degradation on env construction failure. Replaced the test file's hand-rolled repository stubs with the shared mock factories.

How did you test this?

Manually tested opening a task terminal for a task with an external worktree. Also ran the updated unit test suite (vitest run shell.test.ts, 5/5 passing), tsc --noEmit, and biome check on the changed files.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

haacked added 3 commits July 1, 2026 16:35
ShellService.getTaskEnv recomputed the worktree path from the managed
worktree layout, which fails for tasks that reuse an existing worktree
living outside the managed directory. The derived path doesn't exist,
so simple-git threw and the terminal never opened.

Use the authoritative path stored on the worktree row when it exists on
disk, deriving only as a fallback for stale rows. Also degrade
gracefully: if workspace env construction fails, start the shell
without the POSTHOG_CODE_* vars instead of failing shell creation.

Generated-By: PostHog Code
Task-Id: 7dc599d8-3621-4c77-b988-4dbb412c2618
…ries

Narrow the try/catch in getTaskEnv to the buildWorkspaceEnv call so the
repo lookups keep their early-return style. Replace hand-rolled repo
stubs in the shell tests with the shared mock repository factories and
scope the temp directory to the one test that needs a real path.

Generated-By: PostHog Code
Task-Id: 7dc599d8-3621-4c77-b988-4dbb412c2618
@haacked haacked requested a review from Copilot July 2, 2026 01:25
@haacked haacked added the Stamphog This will request an autostamp by stamphog on small changes label Jul 2, 2026
@haacked haacked requested a review from a team July 2, 2026 01:25
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit ee69cb5.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-scoped bugfix with solid test coverage: uses the authoritative stored worktree path when it exists on disk, falls back to derived path otherwise, and adds graceful degradation so shell creation survives env-build failures. No showstoppers.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes task terminal creation when a task reuses an existing worktree located outside the managed worktree directory by treating the stored worktree path as authoritative and making workspace env setup non-fatal.

Changes:

  • Prefer the stored worktree.path (when it exists on disk) over re-deriving the managed-layout path.
  • Gracefully degrade when workspace env construction fails by logging and spawning the shell without POSTHOG_CODE_* variables.
  • Add unit tests covering stored-path usage, derived-path fallback, and env-construction failure behavior (using shared repository mock factories).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/workspace-server/src/services/shell/shell.ts Uses stored worktree path when valid and wraps workspace env construction in a try/catch so terminal creation doesn’t fail.
packages/workspace-server/src/services/shell/shell.test.ts Adds test coverage for stored vs derived worktree paths and verifies shell creation continues when env building throws.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "test(shell): improve type accuracy and b..." | Re-trigger Greptile

Comment thread packages/workspace-server/src/services/shell/shell.test.ts
@charlesvien charlesvien merged commit 646f66c into main Jul 2, 2026
24 checks passed
@charlesvien charlesvien deleted the posthog-code/fix-shell-external-worktree branch July 2, 2026 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stamphog This will request an autostamp by stamphog on small changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants