feat(sessions): upload patch files from loadouts and friends - #914

In file @crates/minimald/src/rpc.rs around line 833:

The atomic swap deletes the existing `patches_dir` before calling `rename(staging_dir, patches_dir)`. If `rename` fails, the previously valid patch tree and its ready marker are already gone, and there is a window where no patch tree exists at all — breaking the atomicity the comment claims. The old tree should be preserved until the new one is installed, e.g. by renaming the old tree out of the way first so a failed `rename` can roll back.

Also found in 1 other location(s):
- crates/sessions/docs/COMPOSITION.md:685 -- The new invariant calls the unpack an atomic rename and says the marker never lies, but `serve_stream_workspace_patches` first removes the existing `patches/` directory and only then renames `patches.tmp/`. During a retry, `FinalizeSession` can concurrently observe the old ready marker before removal and materialize stale patches, or observe the gap; the swap is not atomic replacement. The documentation therefore gives operators an incorrect concurrency guarantee and should either document/serialize this race or the implementation should use a genuinely atomic swap.

In file @crates/minimald/src/rpc.rs around line 675:

Two concurrent `WorkspacePatchesTarZst` uploads for the same session corrupt each other's patch set. Both compute the same fixed `staging_dir` (`patches/.tmp`), so each upload's `remove_dir_all`/`create_dir_all`/writes interleave with the other's, and the final `remove_dir_all(&patches_dir)` + `rename` swaps race — one upload can wipe the tree the other just installed, install a mixed archive, or fail after the other already renamed. There is no per-session serialization or unique staging path. Consider using a unique staging directory per upload and serializing the final swap per session (e.g., a per-session mutex).

In file @crates/minimald/src/session.rs around line 808:

`finalize` converts every error from `tokio::fs::try_exists` into `false` via `unwrap_or(false)`. A permissions or filesystem I/O error when checking the patches-ready marker makes the function return the misleading `InvalidInput` error "patches upload never completed" instead of the real I/O error, preventing correct diagnosis or retry. Propagate the `try_exists` error and only treat `Ok(false)` as a missing upload.

In file @crates/minimald/src/session.rs around line 376:

A `Materializing` record that survives daemon restart is mapped to `SessionInner::Draft { pending: None }`, so `configure_loadout` and the attach shortcut accept it and recompose the session as if it were `Pending`. The record's persisted status stays `Materializing`, so the session drifts to an inconsistent state, and any stale `.patches_ready` marker left by a prior upload can satisfy the new composition's finalize check — materializing the old session's patches for a different composition instead of requiring the client to abort and re-create.

In file @crates/minimal/src/file_upload.rs around line 62:

If the future returned by `stream_tar_zstd` is cancelled while `add_dir_entries` or `archive.finish()` is awaiting, the `TarZstArchive` is dropped without finalization, and `async_tar::Builder` panics from its `Drop` impl. Unlike the previous spawned-task design, the archive builder now runs inline in the caller's future, so any task abort, timeout, or Ctrl-C shutdown triggers the drop-time panic. Consider either keeping the builder on a separate task that drains to completion on cancellation, or wrapping the archive in an `Abortable`/`CancellationToken` guard that finalizes the builder before dropping it.

In file @crates/minimald/src/session.rs around line 39:

`materialize_patches_into_home` can fail with an `io::Error` for a composition that `check_patch_mismatches` accepted. Two patches targeting destinations `foo` and `foo/bar` have distinct destinations so the composer's conflict check passes, but materialization tries to treat `foo` as both a file and a directory: whichever runs second fails. This leaves the session stuck in `Materializing` for a valid composition. The fix belongs in the composer: reject incoming patches whose destination is a prefix of (or equal to) an existing destination, and vice versa, so prefix collisions are caught at compose time rather than failing at finalize.

In file @crates/minimald/src/rpc.rs around line 777:

The per-entry size cap doesn't bound total memory: up to `write_concurrency` write tasks each hold a buffered body in memory, and the next entry body is read fully *before* the backpressure check waits for a slot. Peak live memory is therefore roughly `(available_parallelism() + 1) × MAX_PATCH_ENTRY_BYTES` — tens of GiB on a many-core host — which can trigger the allocator's OOM handler and abort the whole daemon. This defeats the stated purpose of `MAX_PATCH_ENTRY_BYTES`. The concurrency cap limits the number of writes, but since each accepted body is fully buffered into a `Vec` before being handed to a spawned task, the memory ceiling is the product of the cap and concurrency, not the cap alone. Consider enforcing a daemon-wide byte budget and applying backpressure *before* reading the next entry body, rather than multiplying a 1 GiB per-entry cap by CPU count.

In file @crates/minimald/src/session.rs around line 893:

When `attach` auto-configures an unconfigured `Draft` session with an empty contribution and the resulting composition has patches, the record is promoted to `Materializing` but the attach returns `SessionPending` and stays blocked forever. The inline `configure_loadout` moved the session to `Materializing`, but `attach` only auto-finalizes when `c.patches().is_empty()` — otherwise it skips finalization and falls through to the `SessionPending` refusal. This attach path never receives the composition or a patches-upload request, so there is no way for the client to run the expected upload → `FinalizeSession` sequence. Every subsequent attach to the same session hits the same `Materializing` guard and also fails. Consider checking for a non-empty composition before calling `configure_loadout` and refusing the inline shortcut (or documenting that internal callers must never produce daemon-side patches through this path).

In file @crates/minimal/src/file_upload.rs around line 332:

When the source file shrinks between `metadata()` and the body copy, `add_file` pads the short read with NUL bytes to match the declared size and returns `Ok(())`. The daemon then unpacks a corrupted file containing the surviving prefix plus trailing zeros, and the client reports a successful upload. The padding keeps the tar framing valid but silently loses content integrity. Consider returning an error when `written < declared` instead of zero-padding so the upload fails loudly.

In file @crates/minimald/src/session.rs around line 1089:

`configure_loadout` sets the on-disk record to `Materializing` but stores the in-memory state as `SessionInner::Active` at line 654. Because `Session::context` only rejects `SessionInner::Draft`, `SessionHandle::context()` succeeds before patches are uploaded or `FinalizeSession` runs. The task-exec path calls `context()` directly, so tasks execute against an unmaterialized home directory, bypassing the lifecycle safety guarantee the `Materializing` status was added to enforce. Consider gating `context()` on the persisted record status being `Active`, as `attach()` already does.

In file @crates/minimal/src/lib.rs around line 1039:

`best_effort_destroy` awaits `Client::oneshot_rpc` with no timeout, so when cleanup runs after a network blip or wedged daemon, the half-open SSH channel can make the call hang indefinitely — blocking the caller from ever seeing the original upload/finalization error it's supposed to surface. Consider bounding the cleanup with `tokio::time::timeout` so best-effort teardown can't stall activation.

In file @crates/minvmd/examples/exec.rs around line 229:

The `Materialized` branch skips `FinalizeSession`, so the session stays in `Materializing` status. The daemon gates exec on `Active` status, and the auto-finalize shortcut only applies to sessions the daemon configured itself — not this client-configured session. The exec at line 247 is therefore rejected instead of running the command. This example must call `FinalizeSession` (after the empty patches upload) before attempting exec.

In file @crates/minimald/src/rpc.rs around line 905:

The unpacker treats every tar entry as a regular file — `tokio::fs::write` is used unconditionally, ignoring the header's entry type. Directory entries (e.g. `.claude/`) are written as empty regular files instead of being created with `mkdir`, so the subsequent `tokio::fs::create_dir_all(".claude")` for a child like `.claude/settings.json` hits a file-not-directory error and the whole unpack fails. Symlink entries are likewise replaced by empty regular files instead of being created with `symlink`. The fix is to branch on `entry.header().entry_type()` and handle directories and symlinks explicitly.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minimald/src/rpc.rs` around lines 821 - 916, Update the archive-entry
handling loop around the entry_type and write task to branch on each tar entry’s
type before reading and writing its body. Create directories with the async
directory API, preserve symlink targets using the platform-appropriate symlink
operation, and retain body reads plus file writes only for regular files; reject
unsupported entry types with a descriptive error while preserving existing path
validation and inflight-task cleanup.

In file @crates/minimal/src/lib.rs around line 999:

`approved_patches_from_verdict` returns each approved patch's `host_path` as a `PathBuf` that the CLI later tries to upload from the local machine. But `WirePatchVerdict::Approved::host_path` is the canonical absolute path on the **daemon**, not the client. Any approved daemon-side patch therefore makes activation fail with a file-not-found error when the client attempts to open a path that doesn't exist locally. The function should either skip daemon-sourced patches or resolve them back to a genuine client-local source (or stream from daemon storage) before returning them as upload sources.

In file @crates/minimald/src/session.rs around line 548:

`IsBusy` returns `false` for a `Materializing` session (composition finalized but patches still uploading), so an unforced daemon shutdown proceeds mid-upload. On restart, the startup reaper deletes the `Materializing` record, discarding the in-progress session without warning. The `Materializing` phase maps to `SessionInner::Active { host: None, composition: Some(_), .. }`, which `IsBusy` treats as idle because it only checks `host.is_some()`. Consider also checking whether a composition is held and the record status is still `Materializing`, or track the phase explicitly.

In file @crates/sessions/src/core/primitives.rs around lines 1015-1017:

`PatchDest::try_new` accepts paths like `"~/."` or `"~/./"` and produces a `PatchDest` pointing at the sandbox home root, bypassing the empty-destination rejection. After `~/` is stripped and `CurDir` components are dropped during normalization, `normalized` is empty, but the code constructs `SandboxRelPath::try_new(normalized)` anyway instead of re-checking for emptiness. An empty `SandboxRelPath` is valid (it has no components), so the error is never raised. Consider checking `normalized.as_str().is_empty()` after the normalization loop and returning `PatchError::EmptyDest`.