fix(dispatch): give every job of one workspace its own target worktree - #2155
Merged
Merged
Conversation
Starting a second Dispatch session on a workspace failed provisioning: dispatch __workspace_provision failed (exit 1): Error: dispatch workspace provisioning failed: dispatch worktree exists without the requested base commit The target names a job's checkout `<project label>-<short job id>`, and took the short id by filtering the job id to alphanumerics and slicing the first eight. Job ids are minted as `dispatch-<uuid>`, so the slice never reached the uuid: every job of a project produced the same eight characters — `dispatch` — and therefore the same directory. The second session landed on the first session's checkout. Provisioning inspects an occupied directory before it fetches anything, so the second job's base commit was usually not in the shared clone yet and it bailed with the message above; when the commit was already cached it bailed on the branch check instead. Either way one workspace could only ever run one dispatch at a time, and the error named neither the directory nor the job it collided with. Digest the job id instead of slicing it. A digest depends on the whole id, so no shared prefix, suffix, or length can collapse two jobs onto one directory. Jobs that already have a checkout keep it: the provision record pins the path it was first given, so this is inert on upgrade. The three worktree-rejection errors now name the directory they judged and the commit or branch they wanted, because an occupied checkout is exactly the case where "which directory, and whose?" is the question. Verified by reverting the fix: both new tests fail, the end-to-end one with the same refusal users hit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Starting a second Dispatch session on the same workspace failed provisioning:
The target names a job's checkout
<project label>-<short job id>, and built the short id by filtering the job id to alphanumerics and taking the first eight characters:Job ids are minted as
dispatch-<uuid>(DispatchSessionDriver.ts), so the slice never reached the uuid — it returneddispatchfor every job. Every session of a project resolved to the same directory, e.g.BitFun-dispatch. The old unit test asserted exactly this:So the second session landed on the first session's checkout.
existing_worktreeinspects an occupied directory before the fetch/bundle step, so the second job's base commit was usually not in the shared clone yet and it bailed with the message above; when the commit happened to be cached it bailed on the branch check instead. One workspace could only ever run one dispatch at a time.Fix: digest the job id instead of slicing it. A digest depends on the whole id, so no shared prefix, suffix, or length can collapse two jobs onto one directory.
Type and Areas
Type: bug fix
Areas: Rust core (CLI target-side dispatch workspace)
Motivation / Impact
Running several Dispatch sessions from one workspace — the normal way to parallelize work across a target — was impossible; the second one always failed with an error naming neither the directory nor the job it collided with.
Upgrade behavior is inert. A job that already has a checkout keeps it:
provision_in_storereusesrecord.workspace_pathahead of any naming rule ("a job keeps the directory it was first given, whatever the current naming rules would produce"). Only jobs that have not provisioned yet get the new name, and the old shared directory is reclaimed by the normal retention sweep when its owning job departs.Also, the three worktree-rejection errors now name the directory they judged and the commit or branch they expected. An occupied checkout is precisely the case where "which directory, and whose?" is the question, and the old messages answered neither.
No wire-protocol, schema, or user-facing string changes.
Verification
Three tests added/reworked:
a_second_session_on_one_workspace_provisions_alongside_the_first— end-to-end throughprovision_in_store, with the realdispatch-<uuid>id shape: first session delivers a bundle, second session provisions off the shared clone, and both land in distinct directories on their own managed branches. Re-provisioning the first is still idempotent.every_job_of_one_project_gets_its_own_worktree_directory— distinctness, label preservation, stability across calls, and ids that differ only past the old slice window.worktree_directories_are_named_after_the_project_not_the_job→worktree_directories_lead_with_the_project_label, which asserted the bug in its first case. It now checks the label half and leaves the suffix to the tests above.Reverted the fix to confirm the tests catch it. Both fail, and the end-to-end one reproduces the user-visible refusal:
(That path —
BitFun-dispatch— is the collision, and it is the same directory both sessions computed.)Reviewer Notes
prepare_baselinecreates its baseline worktree withrequest_id: job_idand a per-job claim, so two sessions on one workspace already got separate baselines there. The managed branch name also carries the full job id. The target directory name was the only collision.WORKTREE_SUFFIX_CHARSstays at 8, now 8 hex characters of SHA-256 instead of 8 characters of the id.hexis only a dev-dependency ofbitfun-cli, so the digest is rendered withformat!("{digest:x}")— the same formattingstore.rs::workspace_lock_pathalready uses — rather than adding a dependency.worktree_directory_names_stay_safe_path_componentspins that the digest still clearsDispatchStore::worktree_dir's path-component validation.