daemon remote: a failed or interrupted bundle extract destroys the link's work tree

[beardthelion]

The remote bridge's bundle extractor deletes a link's live work tree before it has anything to put in its place, so a failure or a crash in that window leaves the link holding neither the old tree nor the new one. internal/dictation/download.go promotes a downloaded engine the same way.

extractBundle

extractBundle (internal/daemon/remote/bundle.go) clones into a staging dir, then:

if err := os.RemoveAll(dest); err != nil {
    return err
}
return os.Rename(cloneDest, dest)

The deferred os.RemoveAll(staging) then deletes the replacement on the way out, so an error between those two lines loses both copies. The doc comment above it claims the opposite ("the staging+rename keeps the live dest intact on error"), which is true only of a clone failure.

Two reachable triggers, both reproduced against the real function with passing controls (happy path replaces; clone failure leaves dest intact):

A removal that fails partway. With a subdirectory the daemon cannot delete, RemoveAll reports unlinkat .../proj-1/locked/keep.txt: permission denied after it has already removed .git and the tracked files. The prior extraction is gutted and the new clone is deleted with the staging dir.

Concurrent uploads of one link id. Serve handles every connection in its own goroutine, so two uploads for one id interleave. Over 15 rounds a watching reader saw dest absent, with errors including rename .../.staging-*/repo .../proj-1: directory not empty and unlinkat .../proj-1: directory not empty, one call's RemoveAll racing another's Rename.

A crash or power loss between the two calls has the same effect and leaves nothing to recover from.

Link ids and the staging namespace

Staging dirs are created as .staging-* in the bundle dir itself, and sanitizeLinkID accepts .staging-123, .git and ..foo. Link ids come from the client's --id flag and arrive over the wire, so an id can name another extract's in-flight staging dir, whose RemoveAll then deletes that clone mid-flight.

Dictation

downloadVerifyExtract has the same shape:

if err := os.RemoveAll(destDir); err != nil { ... }
if err := os.Rename(stageDir, destDir); err != nil { ... }

A failed promotion leaves no engine installed. Lower impact, since the engine is re-downloadable, but the same defect.

Impact

The bundle dir holds the only copy of a repository that was uploaded to a remote host. A user who has already removed the local original loses the work tree.

Why tests missed it

TestBridgeBundleUploadRoundTrip does two sequential, all-success uploads of proj-1. It never fails after the destructive step, never runs concurrent same-id uploads, and never uses a link id in the staging namespace.

Suggested fix

Move the live tree aside instead of deleting it, publish the new one, and put the old one back if that fails. Serialize extracts per link id, and keep link ids out of the staging namespace. A crash between the two renames still needs a repair pass at startup, since swapping a directory cannot be made atomic.

Affected: internal/daemon/remote/bundle.go, internal/dictation/download.go, at eeea3308 and current main. Found on linux/arm64, go1.26.6.

I have a branch with the fix and regression tests and will open a PR if this is approved.