Skip to content

fix: a launch killed mid setup pass was never provisioned again (0.39.0) - #598

Merged
blooop merged 9 commits into
mainfrom
reliable_break
Sep 9, 2026
Merged

fix: a launch killed mid setup pass was never provisioned again (0.39.0)#598
blooop merged 9 commits into
mainfrom
reliable_break

Conversation

@blooop

@blooop blooop commented Sep 9, 2026

Copy link
Copy Markdown
Owner

The bug

devpod up finishes, devpod writes its create result, and what runs next is dl's own setup pass: the hostname, the terminal title, the onboarding memo, gh and claude. Ctrl-C there leaves the container running with none of it.

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 dl <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:

{"argv": ["status", "devlaunch-cold-8iyb", "--output", "json"]}
{"argv": ["ssh", "devlaunch-cold-8iyb", "--send-env", "GH_TOKEN"]}

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. aid is dl::run in-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 same workspace_result.json mtime 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 to provision, decided by Provisioning::reached_the_container. TripRefused now 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>.pass was 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 ### Changed and still described the pre-fix placement.
  • fbdfe3e — CI's public-api job 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.0 commit 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 ### Fixed block 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 from 74a1d88. The 0.38.0, 0.37.0 and 0.36.0 sections are byte-identical to git 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.txt is hand-edited — cargo-public-api needs a nightly toolchain the devcontainer does not carry — so the public-api job is the real check on those rows, and it already caught one miss.

🤖 Generated with Claude Code

`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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @blooop, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 5 days and 14 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Reviewer's Guide

The 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 pass

sequenceDiagram
    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
Loading

Entity relationship diagram for anchored pass evidence

erDiagram
    WORKSPACE_RESULT {
        string workspace_id
        timestamp mtime
    }
    PASS_RECORD {
        timestamp result_mtime
    }
    WORKSPACE_RESULT ||--o| PASS_RECORD : anchors
Loading

Flow diagram for anchored setup-pass lifecycle

flowchart 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]
Loading

File-Level Changes

Change Details Files
Track interrupted setup passes with host-side, container-anchored markers and use them to recover fast attaches.
  • Create <workspace>.pass before the first provisioning trip and remove it only after the pass finishes or a cached verdict proves completion.
  • Anchor markers to the current workspace_result.json mtime so stale records from rebuilt containers are ignored; treat missing or malformed records as no evidence.
  • Check the marker on the fast-attach path and rerun provisioning as a TopUp before attaching, while preserving the one-round-trip warm-attach path when no marker exists.
  • Add launch notices, cache APIs, provisioning classification, and public API updates for the recovery behavior.
rust/devlaunch-core/src/flows/provision/verdict_cache.rs
rust/devlaunch-core/src/flows/provision.rs
rust/devlaunch-core/src/flows/launch.rs
rust/dl/src/render.rs
rust/devlaunch-core/public-api.api.txt
Ensure marker lifetime accurately represents the entire multi-trip provisioning pass.
  • Split pass execution so the in-flight record remains open through probe, lend, and install trips.
  • Close the record only when a trip reached the container, including failed installs; retain it for TripRefused because no container state was observed or changed.
  • Add tests covering marker timing, interrupted recovery, stale anchors, unreadable records, cached completion, and refusal behavior.
rust/devlaunch-core/src/flows/provision.rs
rust/devlaunch-core/src/flows/provision/verdict_cache.rs
rust/devlaunch-core/src/flows/provision/interrupted_pass_contract.rs
Add end-to-end coverage for interruption and subsequent recovery.
  • Simulate a completed devpod up followed by termination during dl's setup SSH trip.
  • Verify the interrupted run leaves a marker, the next launch performs exactly one recovery pass and reports it, and the following launch returns to zero setup trips.
  • Extend the fake devpod harness to preserve workspace records and control blocked setup trips.
rust/dl/tests/resumed_setup.rs
Release version 0.38.0 and document the interrupted-pass recovery contract.
  • Update Rust, README, and release metadata from 0.37.0 to 0.38.0.
  • Document marker location, positive-evidence semantics, stale-container handling, and recovery behavior.
  • Add a changelog entry describing the bug, implementation, and trip-completion semantics.
CHANGELOG.md
README.md
rust/Cargo.toml
rust/Cargo.lock
docs/workspace-tools.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.39474% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.87%. Comparing base (1c6a2db) to head (acc02f4).

Files with missing lines Patch % Lines
rust/devlaunch-core/src/flows/provision.rs 92.92% 8 Missing ⚠️
rust/devlaunch-core/src/flows/launch.rs 95.06% 4 Missing ⚠️
...evlaunch-core/src/flows/provision/verdict_cache.rs 98.61% 1 Missing ⚠️
rust/dl/src/render.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
python 42.98% <ø> (ø)
rust 95.14% <95.39%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
shipped code (rust) 95.14% <95.39%> (+<0.01%) ⬆️
harness and tooling (python) 42.98% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
@blooop blooop changed the title fix: a launch killed mid setup pass was never provisioned again (0.38.0) fix: a launch killed mid setup pass was never provisioned again (0.39.0) Sep 9, 2026
@blooop
blooop merged commit e34d420 into main Sep 9, 2026
15 checks passed
@blooop
blooop deleted the reliable_break branch September 9, 2026 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant