Skip to content

fix(server): old isolated threads from the pre-v2 fork open as a multi-root workspace - #350

Merged
LoganRupe merged 2 commits into
LoganRupe:multi-repo-workspacesfrom
stefanvandervelden-zipline:fix/multi-repo/fork-thread-workspace-file
Oct 15, 2026
Merged

LoganRupe merged 2 commits into
LoganRupe:multi-repo-workspacesfrom
stefanvandervelden-zipline:fix/multi-repo/fork-thread-workspace-file

Conversation

@stefanvandervelden-zipline

@stefanvandervelden-zipline stefanvandervelden-zipline commented Oct 14, 2026 •

Copy link
Copy Markdown

Problem

After #341 moves an old isolated thread from the pre-v2 branch onto its whole thread folder, "Open in" still opens that folder as a plain directory instead of a multi-root workspace. VS Code then shows the worktrees as nested folders rather than repos, plus the .code-workspace file itself.

v2 opens the .code-workspace it writes next to a new isolated run's folder (#348). The pre-v2 branch kept its copy inside the folder instead (#294), so repaired threads have no file where v2 looks.

Fix

A new startup step, writeMultiRepoForkWorkspaceFiles, runs after the #341 repair. It applies to each v2 thread whose folder has v2's layout file but no .code-workspace next to it, and whose folder holds a single real .code-workspace (the pre-v2 copy). For those, it writes v2's file using the same generator new isolated runs use, built from the project's .code-workspace.

  • The pre-v2 copy is left in place, so the pre-v2 build can still open it if a database is ever taken back to that branch.
  • Folders v2 created only contain a symlink to the project's file, so they're left alone.
  • Once a thread has the file it's skipped, so this runs once per thread. It also covers threads fix(server): threads from the pre-v2 multi-repo fork reach every repository again #341 already repaired, which the repair itself won't revisit.

Known limit: deleting an old thread still leaves its folder behind, because the pre-v2 copy is a regular file (as noted on #340). That's left for a separate change.

Verification

  • New tests in repairMultiRepoForkWorktrees.test.ts:
    • A pre-v2-shaped thread folder gets the file, keeps its copy unchanged, and a second run writes nothing.
    • A v2 thread folder that links the project's file is left alone.
  • Ran on a real database migrated from the pre-v2 branch, with 17 old isolated threads. The startup step wrote all 17 files, and "Open in" now opens each thread as a multi-root workspace with every repo's worktree.
  • One gotcha I hit: if the desktop app can't read the project folder (macOS Files & Folders privacy for ~/Documents), the step skips each thread and retries on the next start.

Written with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

…pace file

The fork kept an isolated run's .code-workspace inside the container, while
V2 opens the one written beside it. Threads moved over by the fork worktree
repair had none there, so "Open in" opened the bare container folder.

A startup pass now writes V2's file from the project's workspace file for
every container that holds the fork's own copy and has none beside it. The
fork's copy stays so the pre-V2 build can still open it. Containers V2
created only link the project's file in, so they are left alone.

Fork-only; not intended for upstream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@LoganRupe LoganRupe left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this, and for testing it on a real migrated database. The 17-thread run and the macOS privacy gotcha were useful to read.

I'd like to fix this somewhere other than startup, though. As written, the step never records that it has run. So every start re-checks every thread with a worktree path, one at a time, before the server takes commands. Threads it can never fix, like single-repo worktrees, get checked forever.

I think the bug sits in workspaceFile() (WorkspaceRepositories.ts:234). For an isolated run with no generated file, it returns null. Could it instead use the container's single .code-workspace, the same rule the non-isolated branch already uses? Your pre-v2 copy already points ./api and ./web at the worktrees, so "Open in" would work with no startup step and no extra file.

That change would also fix three other things I hit while reading:

  • holdsForkCopy matches more than fork containers. In a v2 run whose root repo commits a .code-workspace, that file is a real file in the container. So a v2 run with a missing generated file looks like a fork thread and gets <container>.code-workspace written. If the user deleted that file on purpose, it comes back on every start.
  • Deleting a repaired thread leaves the new <container>.code-workspace behind. The container survives because of the real file inside it, so remove() never reaches the cleanup branch.
  • A half-written target file is never repaired. The write isn't atomic and the step skips any thread whose target exists.

If there's a reason the fallback in workspaceFile() doesn't work, tell me, I may be missing something. If you'd rather keep the startup step, it needs a done marker and tests for the root-repo case above.

@stefanvandervelden-zipline

stefanvandervelden-zipline commented Oct 15, 2026 •

Copy link
Copy Markdown
Author

... Could it instead use the container's single .code-workspace ...

That was actually the first fix that Claude had suggested, but I thought that migrating to the same workspace file that normally gets generated for new isolated threads would be less likely to involve some form of whack-a-mole for bugs popping up for other places that might assume the new workspace file. Doing it the way I did just felt like a safer fix.

holdsForkCopy matches more than fork containers. In a v2 run whose root repo commits a .code-workspace, that file is a real file in the container. So a v2 run with a missing generated file looks like a fork thread and gets <container>.code-workspace written. If the user deleted that file on purpose, it comes back on every start.

Hmm, that's a good point. I feel like a workspace file like that would trip things up regardless of whether we migrate how I've currently designed it or with your suggestion of just re-using it, either way it would be treating that workspace file as the workspace file for the whole thread. Not sure how we could go about fixing this

@LoganRupe

Copy link
Copy Markdown
Owner

Fair on the root-repo case, I overstated that one. Whichever file ends up used, the paths resolve to the run's worktrees, so the output is correct. Ignore it.

On the whack-a-mole worry: I checked, and workspaceFile() is the only thing that reads the generated path. Everything that opens a workspace goes through it, and remove() only deletes the file. Your #294 copy is also built the same way as v2's file (repos pointed at the worktrees, other folders kept absolute). So a fallback there covers every reader.

That said, I'm happy to keep your approach if you prefer migrating to v2's file. My one ask is to narrow the candidates in SQL the way repairMultiRepoForkWorktrees does. Right now the query takes every live thread with a worktree path, and each one costs an exists() before the server takes commands, every start. Joining projection_threads and keeping only rows with json_array_length(worktrees_json) > 1 would limit it to fork-imported multi-repo threads, so a fresh v2 database touches no files at all.

…tten atomically

The startup step now only considers threads the pre-v2 fork recorded with
more than one worktree, so a database that never ran the fork touches no
files. The workspace file is written via a temp file and rename, so an
existing file is always complete and safe to skip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stefanvandervelden-zipline

Copy link
Copy Markdown
Author

Thanks for the review. I've pushed 4792bfc. Here's what I did with each point:

  • Fallback in workspaceFile(): I kept the startup step that migrates to v2's file, as discussed above.
  • Narrowing the candidates in SQL: applied as you suggested. writeMultiRepoForkWorkspaceFiles now joins projection_threads and only keeps rows with json_array_length(worktrees_json) > 1, using the same guard as repairMultiRepoForkWorktrees. A database that never ran the fork touches no files.
  • Done marker: I didn't add one, since the filter covers it. Only fork-imported multi-repo threads get looked at, and once a thread has its file, each start costs one exists() for it.
  • Half-written file: writeIsolatedRunWorkspaceFile now writes through writeFileStringAtomically. If the file exists, it's complete, so skipping it is safe. New v2 runs write through the same function, so they get this too.
  • Root-repo .code-workspace: no code change, since you withdrew it. The new filter excludes those runs anyway, and there's a new test for a thread with one worktree and a real file in its folder.
  • Deleting a repaired thread leaves <container>.code-workspace behind: I'm leaving this for a follow-up. It's the same issue as the container folder already being left behind by the fork's copy, and I'd rather fix both together.

@LoganRupe LoganRupe left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks, this looks good. Narrowing the query to fork threads was what I was after, and the atomic write is a nice extra for new runs. Happy for the delete cleanup to land in the follow-up.

@LoganRupe
LoganRupe merged commit 4f86515 into LoganRupe:multi-repo-workspaces Oct 15, 2026
@stefanvandervelden-zipline
stefanvandervelden-zipline deleted the fix/multi-repo/fork-thread-workspace-file branch October 15, 2026 05:22
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