feat(web): clone repositories in the background instead of holding the palette open#11762

🔴 Critical sourceControl/SourceControlRepositoryService.ts:239

prepareClone returns remoteUrl unchanged, so a URL containing embedded Git credentials is propagated into ProjectCloneSnapshot and exposed to every caller of subscribeProjectClones with read scope. Strip or reject credential-bearing URLs before returning/storing the clone snapshot.

Fixed in 10e3397: prepareClone now returns a credential-free remoteUrl for snapshots/results and a separate cloneUrl that only git receives. Test added.

🔴 Critical src/ws.ts:2703

Concurrent projectCloneStart calls can clone different projects into the same destination path: both pass prepareClone, one then fails after the other populates the directory, and retrying that failed project calls discardClone on the shared path and deletes the successful checkout, including later uncommitted edits. Serialize clone starts and enforce destination-path ownership before allowing prepareClone/discardClone.

Fixed in 10e3397: start atomically claims the destination path in the tracker; a second start into a path an in-flight clone owns fails with 'A clone into this destination is already in progress.'

🟠 High src/ws.ts:1637

HTTP orchestration dispatches thread.create and bootstrap thread.turn.start without the clone-phase check, so clients can start work in an empty project directory while its tracked clone is still running. Apply the same projectCloneTracker gate used here to apps/server/src/orchestration/http.ts before normalization and dispatch.

🟡 Medium components/ProjectCloneToastCoordinator.tsx:169

The Remove project action permanently removes the clone toast even when removeClonedProject fails, leaving the failed/cancelled clone tracked without any retry or removal action. Because the snapshot and rendered key stay unchanged, the coordinator does not recreate the toast; keep it open until the removal succeeds so the cleanup effect can close it.

Fixed in 10e3397: the toast is no longer closed from the click; the server dropping the clone with the project closes it, so a failed removal keeps Retry / Remove in place.

🟠 High components/ChatView.tsx:2112

After a server restart, a clone-incomplete project is treated as sendable: activeProjectClone === null makes projectCloneSendBlockReason null, allowing thread.create or bootstrap thread.turn.start against the empty or partial checkout. ProjectCloneTracker keeps snapshots only in memory, so the failed or running clone marker is lost while the durable project remains. Persist clone state or otherwise distinguish an untracked cloned project from a successfully completed clone before allowing sends.

The tracker docs do describe that behavior, but the claimed loud failure does not appear guaranteed: once the tracker entry is gone, sends proceed, while downstream Git handling returns non-repository/skip results rather than rejecting. The concern remains valid.

🟠 High src/ws.ts:1643

thread.create can be accepted for a project whose clone has started but is not yet tracked, so it targets an empty checkout. The same registration gap lets project.delete miss the clone, after which the request registers and runs a clone for a project that no longer exists. Register the clone before dispatching project.create, with rollback if creation fails.

Fixed in 10e3397: the tracker registers the entry before dispatching project.create (rolled back if creation fails), and refuses a second start into a destination another clone owns.

🟠 High src/ws.ts:1684

Deleting a project through the authenticated HTTP orchestration dispatch endpoint leaves its active clone running and its checkout on disk. That endpoint calls orchestrationEngine.dispatch directly, so it bypasses the projectCloneTracker.discard cleanup performed here for WebSocket dispatchCommand; move the cleanup into shared dispatch handling or add it to the HTTP path as well.

🟠 High project/ProjectCloneTracker.ts:338

Concurrent retry calls can both pass the failed/cancelled phase check and start git clone fibers for the same destination. Because discardClone is awaited before the phase changes to running, the second call can delete the first retry's working tree and leave the project failed. Atomically reserve the entry (or serialize retries per project) before discarding and launching.

🔴 Critical hooks/useRemoveClonedProject.ts:34

force: true can delete a project after a stale or concurrent clone completion, including its newly created threads. This hook does not verify that the clone is still unfinished before dispatching project.delete, and the server emits thread.deleted events for forced non-empty deletions. Submit a non-forced delete or make the server atomically require an unfinished clone.

No longer relevant as of 10e3397fc74d63c2effee8ecdf2c79582f113648

Fixed in 10e3397: the remove action no longer forces the delete, so a project that gained threads in the meantime is refused by the decider instead of wiped.

🟠 High project/ProjectCloneTracker.ts:362

discard deletes a checkout that completed during Fiber.interrupt, so a project.delete completion race can erase a successful clone. The condition uses the pre-interrupt tracked.snapshot; reread the snapshot after awaiting the interrupt and retain the checkout when its phase is done.

🟠 High state/sourceControl.ts:48

cancelProjectClone requests run concurrently, so a late discardClone from an earlier cancellation can delete the checkout created by a retry. These commands lack the scheduler and per-environment serialization used by startProjectClone; route cancel and retry through the same serialized clone-action queue.

🟡 Medium project/ProjectCloneTracker.ts:339

retry returns true and marks the clone running even when discardClone fails, leaving partial files that cause the new cloneRepository attempt to reject on the nonempty destination. Because the cleanup error is ignored, the caller loses the actionable failure explaining how to recover. Propagate the discardClone error before updating the snapshot.

Fixed in 10e3397: retry propagates the discardClone error instead of marking the clone running over leftover files.

🟡 Medium routes/__root.tsx:226

In hosted-static mode, background clones from remote environments produce no progress, failure, retry, or completion toasts after the palette closes and the user navigates away. ProjectCloneToastCoordinator is mounted only for authenticated, so it never subscribes to those clone events; mount it for hosted-static as well.

Confirmed — this resolves the issue.

🟡 Medium sourceControl/SourceControlRepositoryService.ts:272

A clone that emits 64 MiB of progress before failing is reported as The repository could not be cloned. even when Git emitted a specific later fatal: reason. maxOutputBytes stops onStderrLine after the cap, so those final lines never reach stderrTail; decouple progress callbacks from the buffered-output limit or remove this cap for clone execution.

🟡 Medium hooks/useRemoveClonedProject.ts:51

A successful deletion redirects a newer draft page to / when the user navigates away while deleteProject is in flight. viewingThisProject is computed from the stale routeDraftId before the await, so line 51 still navigates based on the old page after the route has changed; re-check the current route (or otherwise guard stale actions) before calling navigate.

🟠 High sourceControl/SourceControlRepositoryService.ts:272

Each background clone can retain up to 128 MiB of Git output, so several concurrent clones can consume gigabytes of server memory and OOM the process. maxOutputBytes applies per stream, and these fibers have no concurrency limit; restore the previous 256 KiB cap or add a concurrency limit.

🟡 Medium state/projectClones.ts:29

useProjectClone fails to find the active clone when either identifier contains :, so the clone banner and send-blocking behavior are skipped. The key built at line 46 is split at the first delimiter, causing a:b to be parsed as environment a and project b:project; encode the two identifiers as an unambiguous composite key before parsing.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Two new exports have no consumer and fail vp run knip:check. The Check job reports both symbols as unused exports, so CI fails. Each symbol is referenced only inside its own module, so removing the export keyword keeps behavior and clears the failure.

• packages/contracts/src/projectClone.ts#L90-L90: remove export from projectCloneStageLabel; projectCloneProgressSummary at line 118 is its only caller. If a web component needs the label, add that consumer in this PR instead.
• apps/server/src/project/ProjectCloneTracker.ts#L112-L112: remove export from make; layer at line 402 is its only consumer, and server.ts and the tests use ProjectCloneTracker.layer.

✅ Confirmed as addressed by @juliusmarminge

Fixed in 22baf5c (both exports dropped); knip is green locally.

@juliusmarminge, thanks for the fix. Removing both exports addresses the reported knip failures.

✅ Review thread resolved.

You are interacting with an AI system.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Wait for project.meta.update before asserting dispatch order.

ProjectCloneTracker.runClone publishes the done snapshot in Effect.onExit before it runs onCloned. subscribeProjectClones forwards that stream directly, so Fiber.join(snapshots) can return before onCloned dispatches project.meta.update. The test has no other synchronization for that dispatch.

The proposed Effect.retry is not valid: Effect.sync turns the thrown assertion into a defect, which Effect.retry does not retry. The test also has no Schedule import. Signal the dispatch with a Deferred and await it:

✅ Addressed in commit 10e3397

🟠 High vcs/GitVcsDriverCore.ts:825

After maxOutputBytes is reached, a separator-free stderr stream still accumulates all remaining bytes in collectOutput's lineBuffer, so a large clone/error stream can exhaust server memory despite the 256 KiB cap. Bound or discard unterminated callback data after truncation instead of retaining it indefinitely.

🟠 High src/ws.ts:2681

Background cloning drops credentials from valid HTTPS URLs, so private repositories such as https://user:token@host/repo.git fail to clone. projectCloneTracker.start tracks only prepared.remoteUrl, even though prepareClone retains the credential-bearing prepared.cloneUrl for Git; preserve and pass cloneUrl through this path, matching the blocking clone flow.

Good catch, that was a real regression from the redaction change. Fixed in 72d113fea9: the tracker stores cloneUrl separately and passes it to git; only the snapshot and result carry the redacted URL. Test added.

🟡 Medium vcs/GitVcsDriverCore.ts:730

When the 256 KiB cap falls inside a multi-byte UTF-8 character, onLine receives a corrupted clone-progress or error line containing replacement characters instead of the original text. decoder.decode(..., { stream: false }) flushes the partial prefix as U+FFFD, while lineDecoder then sees only the continuation bytes and emits another replacement; keep the callback decoder's UTF-8 state continuous across the cap while retaining the capped decoder for the returned text.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Redact credential-bearing URLs from clone failure details.

cloneUrl remains raw, and onStderrLine stores non-progress stderr unchanged. The clone error uses that text as SourceControlRepositoryError.detail, which reaches clone failure feedback. If Git includes the remote URL in stderr, userinfo can leak. Redact URLs in stderrTail before creating detail, and add a failure-path test.

✅ Confirmed as addressed by @juliusmarminge

@juliusmarminge, thanks. The change addresses the credential exposure path. The failure-path test covers the required redaction behavior.

✅ Review thread resolved.

You are interacting with an AI system.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Make post-claim startup interruption-safe.

ProjectCloneTracker.start inserts a "running" entry with fiber: null, then runs clearRetention, createProject, publish, and launch. Effect.tapError from effect/Effect does not run when interruption exits createProject. Before launch, no clone fiber exists, so cancel returns false and retry rejects the still-running entry. The runClone Effect.onExit handler cannot clean it because runClone was never started.

Make the entire post-claim sequence, including clearRetention, uninterruptible through launch, and apply failure cleanup to the whole sequence. This prevents stale entries from interruption and removes the claim for every typed failure, not only createProject.

✅ Addressed in commits 72d113f to 7ae7d06

@juliusmarminge, verified. ProjectCloneTracker.start now runs the complete post-claim sequence uninterruptibly. Failure cleanup removes the claim for the complete sequence. The added test confirms that a createProject failure frees the destination for a new start.

You are interacting with an AI system.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Pass cloneUrl through ProjectCloneTracker.

prepareClone returns a credential-free remoteUrl and a credential-bearing cloneUrl. The tracker stores only remoteUrl, then passes it to cloneRepository, so authenticated clones can fail. Store prepared.cloneUrl and pass it as the clone input URL.

✅ Confirmed as addressed by @juliusmarminge

🔴 Critical sourceControl/SourceControlRepositoryService.ts:340

discardClone recursively deletes the entire destination directory, so files added by a user or another process after a failed or cancelled clone are permanently removed when Retry/Cancel runs. Limit cleanup to clone-owned contents, or re-check ownership before deleting the directory.

🟠 High sourceControl/SourceControlRepositoryService.ts:121

A clone URL with credentials only in its query string, such as https://host/repo.git?access_token=secret, is returned unchanged and broadcast to every subscription reader. Because url.username and url.password are empty, the early return bypasses redaction; remove the query string before publishing the URL so tokenized and signed clone URLs cannot leak.

Fixed in e6284abb30: the query string is dropped too (clone URLs have no legitimate query). Test added.

🟠 High sourceControl/SourceControlRepositoryService.ts:130

redactUrlCredentials leaves the suffix after the first raw @ in userinfo exposed, so a token such as part1@part2 is returned as https://part2@host/repo and leaks part2 in the clone error. The character class excludes @, causing the replacement to stop at the first delimiter; allow additional @ characters so the final userinfo delimiter is removed.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not treat every directory-read error as an empty destination.

Effect.orElseSucceed converts NotADirectory and permission errors into []. If another process replaces the clone destination with a file, the recursive remove below deletes that unrelated file. Only treat a confirmed missing path as empty. Propagate or reject other errors before removal.

✅ Confirmed as addressed by @juliusmarminge

Fixed in d907b88afa: only NotFound counts as an empty destination; any other read failure (a file in its place, permissions) fails discardClone before anything is removed. Test covers a file at the destination path.

@juliusmarminge, thanks. The change addresses the finding. discardClone now preserves non-NotFound read failures, so it does not remove a replaced file or proceed after a permission error. The destination-file test covers the critical case.

✅ Review thread resolved.

You are interacting with an AI system.

🟡 Medium components/ChatView.tsx:2165

Clicking Retry gives no feedback when retryProjectClone fails, so the project remains in its failed/cancelled state and the user cannot tell that a new clone was never started. Because the command uses reportFailure: false and the handler discards its result with void, await the result and surface the failure (for example, with an error banner or toast).