fix: a launch killed mid setup pass was never provisioned again (0.39.0) - #598
Merged
Conversation
`devpod up` finishes, devpod writes its create result, and what runs next is dl's own `devpod ssh --command`: the hostname, the terminal title, the onboarding memo, `gh` and `claude`. A Ctrl-C there leaves the container running with none of it, and nothing about that container says so. `devpod status` answers `Running` and devpod's create record is complete, which is exactly what the fast-attach arm asks, so every later `dl <ws>` attaches in one round trip and provisions nothing. The workspace is permanently half built and the only recovery is knowing to type `dl <ws> up`. A pass now writes down that it is running before its trip, in `tool-verdicts/<ws>.pass`, and removes it when the pass answers. dl's signal handler `_exit`s without unwinding, so a record left standing is the one thing on the host that says a pass was cut short. It is anchored to the same `workspace_result.json` mtime the verdict marker is, so one left by a container since rebuilt describes nothing standing. Positive evidence, never the absence of it: a workspace brought up by VS Code, a hand-typed `devpod up`, or a build older than the record has no record either, and those go on attaching in one round trip. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC
…ended Found by all three review axes independently, and measured: with a probe that answers absent, the in-flight record stands during trip one and is gone during trip two. `remember` ran the close, and `remember` sits immediately after `setup_pass`. But a pass is up to three trips, and the two after the probe are the long ones: the lend streams the host's binaries, and the install fetches a `claude` of a few hundred megabytes. The probe is ~1.7s. So the record was open for the window nobody interrupts and shut for the window everybody does, and a Ctrl-C during a cold install left exactly the container the change exists to stop existing: running, create complete, no marker, no record, fast-attached forever with no tools. The close moves out of `remember` and out to `provision`, decided by what the pass answered. `Provisioning::reached_the_container` is that decision, exhaustive so a later arm has to be classified by somebody looking at it. Second finding, from the Types axis: `TripRefused` now leaves the record standing. It is the OS declining to make the trip, so nothing was learned about the container and nothing was done to it, which is the state the record describes. `a_pass_devpod_refused_is_a_pass_that_happened` asserted the opposite and is replaced: it conflated a trip that ran and failed with one that never happened, and erasing the record on the second is reachable from the recovery pass itself -- one refused trip and nothing could ever recover the workspace. The prose the same defect falsified goes with it: `begin_pass`, `end_pass`, and the two paragraphs in docs/workspace-tools.md that described a pass of one trip. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC
CLAUDE.md's standing rule: a second hand-maintained copy of a fact is allowed
only if a test named beside it diffs it against the first. `<workspace>.pass`
was spelled by hand in docs/workspace-tools.md against `in_flight`'s
`with_extension("pass")`, and the sibling claims in that same file each have a
contract module. This one had nothing.
It is the kind of claim a reader acts on rather than reads: the page tells
somebody a killed launch leaves that file beside the marker, so they go looking
for it to see whether a workspace is stuck, or to clear one by hand. A page
naming a file that is not there is worse than a page naming none.
Two assertions, both read out of `VerdictCache` rather than out of a constant,
so a rename moves them with it. Renaming the extension to `inflight` fails the
first with the new name in the message.
Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC
…he fix Two things. `### Fixed` went in above `### Changed`, where every released section in the file orders Added, Changed, Fixed. And the entry still said the record is removed "when the pass answers", which is what the placement did before the previous commit moved it: the sentence described the defect rather than the behaviour. The paragraph that replaces it says which trip the record closes after and why that is most of the fix, since a reader deciding whether to upgrade wants the window and not the mechanism. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC
The [Unreleased] section as it stands becomes the release: the `RemoteCommand` split, the version-collision guard, and the interrupted setup pass. **0.38.0 and not 0.37.1.** `LaunchVerb::Attach` carries a `RemoteCommand` rather than an `Option<String>`, which is a break for anyone driving `devlaunch-core` directly, and `Provision` grows a method. The method is defaulted so no implementer has to move, but the enum change is not available for a patch reading. What a `dl` user gets is the interrupted pass: a launch killed while the container was being provisioned no longer leaves a workspace that every later launch attaches to without provisioning. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC
Reviewer's GuideThe PR fixes permanently skipped provisioning after a launch is killed during dl’s multi-trip setup pass by persisting an anchored in-flight marker, checking it before fast attach, and rerunning the pass only when positive evidence identifies an interrupted run; it also adds unit, contract, and end-to-end coverage plus the 0.38.0 release and documentation updates. Sequence diagram for recovering an interrupted setup passsequenceDiagram
participant User
participant Launch
participant VerdictCache
participant Devpod
participant Container
User->>Launch: Attach workspace
Launch->>VerdictCache: pass_never_finished(workspace_id)
VerdictCache->>VerdictCache: Compare .pass mtime with workspace_result.json
alt Interrupted pass identified
Launch->>Launch: Emit SetupPassNeverFinished
Launch->>Devpod: provision_tools(..., PassOccasion::TopUp, ...)
Devpod->>Container: Run setup pass
Container-->>Devpod: Pass result
Devpod-->>Launch: provision_tools result
Launch->>VerdictCache: end_pass(workspace_id)
else No valid interrupted-pass evidence
Launch->>Launch: Skip provisioning
end
Launch->>Devpod: attach(workspace, RemoteCommand)
Devpod-->>User: Session
Entity relationship diagram for anchored pass evidenceerDiagram
WORKSPACE_RESULT {
string workspace_id
timestamp mtime
}
PASS_RECORD {
timestamp result_mtime
}
WORKSPACE_RESULT ||--o| PASS_RECORD : anchors
Flow diagram for anchored setup-pass lifecycleflowchart TD
A[Start setup pass] --> B[observe workspace_result.json]
B --> C[begin_pass writes workspace.pass]
C --> D[Run probe, lend, and install trips]
D --> E{Provisioning reached the container?}
E -->|Yes| F[end_pass removes workspace.pass]
E -->|No: TripRefused| G[Leave workspace.pass standing]
F --> H[Pass complete]
G --> I[Next fast attach detects interrupted pass]
I --> J[Run TopUp setup pass before attaching]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This reverts commit c608245.
CI's `public-api` job caught it: `LaunchNotice::SetupPassNeverFinished` is a variant of a public enum that `api` re-exports, so it is four rows of `public-api.api.txt` and I added none of them. The six `pass_never_finished` rows I did add came back clean, so the placement rule held; what I missed is that a notice is public surface at all. The snapshots are hand-maintained on this branch because `cargo-public-api` needs a nightly toolchain the devcontainer does not carry, which is exactly the gap this job exists to cover. Rows taken from the job's own diff. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC
#597 cut and merged 0.38.0 while this branch was in review, so the `release: 0.38.0` commit here re-cut a version that already exists. It is reverted and the entry refiled under 0.39.0. The merge is what makes this worth spelling out. `git merge origin/main` put my `### Fixed` block *inside* the shipped `## [0.38.0]` heading with no conflict -- #527's failure exactly, arriving the way it always does, which is silently. main would have claimed 0.38.0 fixed the interrupted setup pass, and 0.38.0 does not: it is built from 74a1d88. The 0.38.0, 0.37.0 and 0.36.0 sections are byte-identical to `git show origin/main:CHANGELOG.md`, which is the check that says this refiles the entry rather than editing the record. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC
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.
The bug
devpod upfinishes, devpod writes its create result, and what runs next is dl's own setup pass: the hostname, the terminal title, the onboarding memo,ghandclaude. Ctrl-C there leaves the container running with none of it.Nothing about that container says so.
devpod statusanswersRunningand devpod's create record is complete, which is exactly what the fast-attach arm asks, sodl <ws>attaches in one round trip and provisions nothing — and so does every launch after it. Reproduced at the binary boundary; the second run's entire devpod traffic was:Whether you get a whole workspace depends on which second the Ctrl-C landed in, and the recovery —
dl <ws> up— is undocumented folk knowledge.aidisdl::runin-process, so it is the same bug.The fix
A pass writes down that it is running before its first trip, in
tool-verdicts/<ws>.pass, and removes it after its last. dl's signal handler_exits without unwinding, so a record left standing is the one thing on the host that says a pass was cut short. The fast-attach arm asks, and runs the pass before handing over a shell.Positive evidence, never absence of it: a workspace brought up by VS Code, a hand-typed
devpod up, or an older build has no record either, and those keep attaching in one round trip. The record carries the sameworkspace_result.jsonmtime the verdict marker does, so one left by a since-rebuilt container is ignored.What the self-review changed
All three review axes independently found the same defect in the first commit, and Defects measured it: the record closed when the probe answered, but a pass is up to three trips and the two after the probe are the long ones. It was open for the window nobody interrupts and shut for the window everybody does.
28f46b0— the close moves out toprovision, decided byProvisioning::reached_the_container.TripRefusednow leaves the record standing: the OS declined the trip, so nothing was learned and nothing was done, which is the state the record describes. A test asserting the opposite is replaced.e33723d—<workspace>.passwas spelled by hand in the docs with no guard, where sibling claims in that file have contract modules. CLAUDE.md's standing rule; now guarded, and proven to fail on a rename.4eb231e— the changelog entry was misfiled above### Changedand still described the pre-fix placement.fbdfe3e— CI'spublic-apijob caught a public enum variant I had not written into the snapshot.Release: 0.39.0, and the 0.38.0 detour is on purpose
#597 — Cut 0.38.0 merged while this was in review, so the
release: 0.38.0commit here re-cut a version that already exists. It is reverted (671fbb1) and refiled under 0.39.0 (acc02f4).Worth reading
acc02f4: merging main put this branch's### Fixedblock inside the shipped## [0.38.0]heading with no conflict — #527's failure, arriving silently as it always does. main would have claimed 0.38.0 fixed this, and 0.38.0 is built from74a1d88. The 0.38.0, 0.37.0 and 0.36.0 sections are byte-identical togit show origin/main:CHANGELOG.md.Verification
Full Rust workspace,
clippy --locked --all-targets -D warnings,cargo fmt, 778 Python guards,prek. Every part of the fix is pinned by a test that fails without it, verified by mutation.rust/devlaunch-core/public-api.api.txtis hand-edited —cargo-public-apineeds a nightly toolchain the devcontainer does not carry — so thepublic-apijob is the real check on those rows, and it already caught one miss.🤖 Generated with Claude Code