Skip to content

fix: a launch blocked on devpod's workspace lock says so while it waits - #601

Merged
blooop merged 3 commits into
mainfrom
fix/600-launch-says-devpod-lock-blocked
Sep 10, 2026
Merged

blooop merged 3 commits into
mainfrom
fix/600-launch-says-devpod-lock-blocked

Conversation

@blooop

@blooop blooop commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Fixes #600.

What was wrong

devpod's lock acquire is a blocking flock with no deadline. It logs Trying to lock workspace … every five seconds for as long as the holder lives, and the usual holder is a devpod up that outlived the dl that started it. dl <ws> rm has watched for that line since #484 and says what clears it. A launch ran its devpod up through a plain passthrough and read nothing, so the same wedge left dl <ws>, up, restart, recreate, reset, code and dotfiles sitting silent behind devpod's own log, with nothing on the terminal about the way out.

What changed

  • up_under_stage runs its up through devpod::run_watching, keeping leading_its_own_group(). On the first line says_it_is_blocked matches it says a new LaunchNotice::UpBlockedOnTheLock, once per launch. Every devpod line is still forwarded, each to the stream it came from, so the build's output is unchanged.
  • The rendering is modelled on delete_blocked and keeps its two judgements: it names another terminal, because this one is holding the launch, and it says the wait has no deadline. It adds the one thing rm's line does not owe: kill deletes the workspace, so launch again after it.
  • Runner gains watched (second commit). Running the first cut against the real orphan from the ticket, still on this host, showed devpod's lock line going by on stdout: devpod's logger splits by level, info to stdout and only error/fatal to stderr, and the line is an info. A stderr-only watch never fires against a real devpod, which also means rm's notice from an orphaned devpod up holds the workspace flock forever, and dl has no way out #484 never has. watched inherits stdin, reads both output streams a line at a time, writes each line straight back to its own stream, and hands every line to the caller to read and not own. session and watched share one body. run_watching_stderr becomes run_watching over it, and both the delete and the launch use it, so rm is fixed by the same change. It is a widening of the devlaunch-runner seam: a new required trait method, recorded in public-api.txt, and every test wrapper in the workspace gained the method.
  • The shared body honours SpawnSpec::own_group. session used to spawn every child with OwnGroup::No whatever the spec said, right while its only caller was an interactive ssh -t. A plain swap would have silently dropped the process group the Ctrl-C drain killpgs (SIGTERM runs the SIGINT drain #304) and recreated the very orphan the ticket is about. The group bookkeeping passthrough shares with it is one helper.
  • README, docs/cli.md's kill section, CHANGELOG, and the public-api snapshots (four core rows for the new arm, three runner rows for watched).

Tests, red before green

  • flows::launch unit tests with a fake devpod whose up writes the line: on stderr, on stdout, once for three repeats, still said when the up is then refused, not said for an ordinary build line. The stderr ones failed on the passthrough; the stdout one failed on the stderr-only watch.
  • flows::lifecycle: a delete with the line on stdout stalls once (failed before run_watching read both streams).
  • devlaunch-runner: watched hands over lines from both streams, keeps order within a stream, honours own group, kills on timeout; session honours own group when asked and stays in ours by default.
  • dl/tests/up_blocked.rs: a shim devpod whose up prints a build line on stderr and the lock line twice on stdout, then blocks. dl's streams go to files so the notice is judged while dl is still blocked, then a SIGINT drains at 130. Asserts the notice names kill and another terminal, appears exactly once, the stdout lines came back on stdout and the stderr line on stderr. Red with the shim on stdout before the second commit.

Verified on the host

  • Against the live orphan (devpod up … kinisi-ros-nb2-jf78, PPID 1 since yesterday): the new dl kinisi-ros-nb2-jf78 up forwards devpod's lines to stdout and prints the notice once on stderr. The first cut printed nothing.
  • A complete real devpod up of a scratch workspace of this repo through watched: returned on devpod's exit in about 30s, provisioning ran after it, workspace removed afterwards.
  • cargo test --workspace, clippy -D warnings, fmt, and the Python suite (774 passed) are green locally. The interrupt suite passing is the check that the up still comes down with dl.

Not done here

The optional second half of the ticket, acting on ContainerState::Busy before the up is attempted, is left for its own PR as the ticket suggests. Gap 3 (why the orphan exists) is out of scope per the ticket.

🤖 Generated with Claude Code

Summary by Sourcery

Make launches identify workspace-lock waits while they remain blocked and provide the command needed to recover.

Bug Fixes:

  • Report when a launch is waiting indefinitely on devpod’s workspace lock and explain how to clear the blockage.
  • Ensure workspace-lock detection works for devpod messages on either stdout or stderr, including existing delete operations.

Enhancements:

  • Preserve devpod output streams while allowing callers to observe live output and retain process-group handling for interrupt cleanup.

Build:

  • Bump the project version to 0.41.0.

Documentation:

  • Document lock notices for launches and update release/version documentation.

Tests:

  • Add coverage for launch and delete lock detection across both output streams, single-notice behavior, forwarding, process groups, timeouts, and interrupt handling.

Chores:

  • Extend the runner API and test support with watched process execution.

devpod's lock acquire is a blocking flock with no deadline behind it. It
logs `Trying to lock workspace …` every five seconds for as long as the
holder lives, and the usual holder is a `devpod up` that outlived the dl
that started it. `dl <ws> rm` has watched for that line since #484 and
says what clears it; a launch ran its `devpod up` as a plain passthrough
and read nothing, so the same wedge left every launch verb sitting
silent behind devpod's own log.

The launch's `up` now goes through `devpod::run_watching_stderr`, as the
delete's does, and says once, on the first matching line, that devpod is
waiting for another process to let go of the workspace and will wait for
as long as that takes, that `dl <ws> kill` in another terminal clears it,
and that `kill` deletes the workspace so the launch is typed again after.
Every devpod line is still forwarded in order.

One thing underneath had to move with it. `ProcessRunner::session`
spawned every child with `OwnGroup::No` whatever the spec said, which was
right while its only caller was an interactive `ssh -t`. `devpod up` is
spawned leading its own group so the interrupt drain can killpg it (#304),
and a plain swap would have dropped that silently. `session` now honours
`SpawnSpec::own_group`, and the group bookkeeping it shares with
`passthrough` is one helper rather than two copies.

Pinned at three seams: the launch flow with a fake devpod whose `up`
writes the line (said once, however many times; the launch still ends as
the `up` ended), the runner (a `session` child leads its own group when
asked and stays in ours by default), and the dl binary (a shim `up` that
prints the line and blocks; the notice is judged mid-block, then a Ctrl-C
drains at 130).

Fixes #600

@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 1 hour by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Reviewer's Guide

Launches now monitor devpod up stderr for workspace-lock waits and promptly provide one actionable, in-progress notice while preserving all devpod output and outcomes. The runner was adjusted to preserve dedicated process groups on the watched execution path so Ctrl-C cleanup remains reliable, with corresponding documentation, API snapshot, unit tests, and an end-to-end blocked-launch test.

Sequence diagram for a launch blocked on the workspace lock

sequenceDiagram
    participant User
    participant DL as dl
    participant Runner
    participant Devpod
    participant OtherProcess

    User->>DL: launch workspace
    DL->>Runner: session(own_group=true)
    Runner->>Devpod: devpod up
    Devpod->>OtherProcess: waits for workspace lock
    Devpod-->>Runner: Trying to lock workspace...
    Runner-->>DL: stderr line callback
    DL-->>User: UpBlockedOnTheLock notice
    Note over User,DL: In another terminal: dl <workspace> kill
    Devpod-->>Runner: forwards all stderr lines in order
    User->>DL: Ctrl-C
    DL->>Runner: terminate process group
    Runner->>Devpod: killpg
Loading

File-Level Changes

Change Details Files
Watch devpod up stderr for workspace-lock contention and emit a single actionable launch notice without changing forwarded output or launch outcomes.
  • Route launch-up execution through the existing stderr watcher while preserving its dedicated process group.
  • Detect repeated lock-wait lines and attach the workspace identity to a new launch notice.
  • Render guidance to use dl <workspace> kill from another terminal, then relaunch because kill deletes the workspace.
  • Cover detection, deduplication, non-lock output, refusal behavior, and a blocked binary-level launch.
rust/devlaunch-core/src/flows/launch.rs
rust/dl/src/render.rs
rust/dl/tests/up_blocked.rs
Honor process-group ownership for stderr-watching sessions so interrupted launches retain the same cleanup behavior as passthrough launches.
  • Pass SpawnSpec::own_group through ProcessRunner::session.
  • Extract shared process-group bookkeeping for passthrough and session execution.
  • Add tests for session children with and without their own process groups.
rust/devlaunch-runner/src/lib.rs
rust/devlaunch-runner/src/tests.rs
Document the new launch lock-wait behavior and update the public API snapshot.
  • Describe affected launch verbs, the kill-and-relaunch workflow, and process-group rationale.
  • Update generated API rows for the new notice and field.
README.md
docs/cli.md
CHANGELOG.md
rust/devlaunch-core/public-api.api.txt

Assessment against linked issues

Issue Objective Addressed Explanation
#600 Make launch-related commands watch devpod up stderr, detect the workspace-lock wait, and emit a single actionable notice explaining that the wait has no deadline and that dl <workspace> kill in another terminal clears the lock and deletes the workspace.
#600 Preserve correct launch interruption and process-group behavior while routing devpod up through stderr watching, so interrupted launches do not recreate the orphaned devpod up problem.
#600 Optionally use ContainerState::Busy to warn about an already-held workspace lock before attempting devpod up. The PR explicitly leaves this optional early-detection improvement for a separate PR. It does not act on ContainerState::Busy before invoking up, but this was identified by the issue as an optional, independent second half rather than the required lock-wait notification fix.

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 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.98678% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.85%. Comparing base (06cb1b6) to head (6862dc3).

Files with missing lines Patch % Lines
rust/devlaunch-core/src/flows/listing.rs 0.00% 7 Missing ⚠️
rust/devlaunch-core/src/flows/provision.rs 0.00% 4 Missing ⚠️
rust/devlaunch-core/src/flows/repo_manager.rs 0.00% 4 Missing ⚠️
rust/devlaunch-core/src/flows/workspace_clone.rs 0.00% 3 Missing ⚠️
rust/devlaunch-core/src/testing.rs 0.00% 3 Missing ⚠️
rust/devlaunch-test-support/src/fake_runner.rs 88.88% 2 Missing ⚠️
rust/devlaunch-runner/src/lib.rs 98.91% 1 Missing ⚠️
rust/dl/src/render.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
python 42.98% <ø> (ø)
rust 95.11% <88.98%> (-0.04%) ⬇️

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

Components Coverage Δ
shipped code (rust) 95.11% <88.98%> (-0.04%) ⬇️
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.

Found by running the first cut against the very orphan #600 reports, still
on this host: two of devpod's "Trying to lock workspace" lines went by on
stdout and dl said nothing. devpod's logger splits by level, `info` to
stdout and only `error` and `fatal` to stderr, and the lock line is an
`info`. The stderr-only watch the launch borrowed from `rm` was a watch on
a guess about severity, which also means `rm`'s own notice from #484 has
never fired against a real devpod.

`Runner` gains `watched`: stdin inherited, both output streams read a line
at a time and written straight back to the stream each came from, every
line handed to the caller to read and not own. `session` and `watched`
share one body, which is where the own-group handling now lives, so the
group the interrupt drain `killpg`s is kept on both. `devpod::run_watching`
replaces `run_watching_stderr` and both the delete and the launch go
through it, so `rm` is fixed by the same change.

Red before green at three seams: a launch unit test with the line on
stdout (zero notices before), the binary test's shim now printing the lock
line on stdout the way devpod does and a build line on stderr (no notice
before; both streams forwarded, each to its own, after), and a delete unit
test with the line on stdout. Runner tests pin `watched`: both streams
reach the callback, order within a stream is kept, own group honoured, a
timeout kills. The delete's deadline test matched on the recorded call
being a `Session`; it is a `Watched` now.

Also corrects the comments this route change had left stale: the runner's
module docs and `passthrough` still named `devpod up` as their child, and
`Call::own_group`'s docs spoke of a passthrough.

Verified against the live orphan (kinisi-ros-nb2-jf78: notice printed once,
devpod's lines forwarded to stdout) and with a complete real `devpod up`
of a scratch workspace (returned on exit, provisioning ran after).
The launch-blocked-on-devpod's-lock entry, which is the whole of what is
new since 0.40.0, and it fixes `rm`'s notice from #484 along with it: the
watch both verbs use now reads the stream devpod actually logs that line
on.

Cut on this branch rather than one of its own, as 0.39.0 and 0.40.0 were:
the merge that lands the fix is the push that publishes it. 0.41.0 is
untaken, which `version_untaken.py` says against the pull request's base
rather than against the moving tip.
@blooop
blooop merged commit debe1b9 into main Sep 10, 2026
15 checks passed
@blooop
blooop deleted the fix/600-launch-says-devpod-lock-blocked branch September 10, 2026 08:42
blooop added a commit that referenced this pull request Sep 10, 2026
devlaunch#602 item 2: "If every holder is `Attended`, there is nothing to
heal: keep today's notice and today's wait, which are right for that
case." The wait was kept; the notice was not. #601's line named `dl <ws>
kill` as the way out, this branch took that out of the blocked notice --
rightly, because it is said before dl knows whether it can clear the lock
and dl now does it unasked -- and put nothing back in the arms where dl
looks and deliberately does not act. `docs/cli.md` already claimed the
attended case "keeps the notice and the wait"; only the wait was true.

Behind a spared build the command is named with what it costs, since
`kill` deletes the workspace under somebody else's build. Behind an
orphan that sat through SIGKILL no command is named, because `kill` would
fail there for the reason the sweep just did, that the process is almost
certainly another user's; who can end it is said instead.

The comment on `up_blocked.rs`'s stderr assertion said the instruction was
gone from the whole of a launch's output, which this makes untrue. It now
says what the assertion actually pins: this run's sweep finds no holder,
so no arm that names the verb is reachable in it.
@blooop blooop mentioned this pull request Sep 10, 2026
JSmithRobotics pushed a commit to JSmithRobotics/devlaunch that referenced this pull request Sep 16, 2026
devlaunch#601 gave a blocked launch a voice: it printed, once, that devpod
was waiting for another process to let go of the workspace and that
'dl <ws> kill' in another terminal would clear it. The advice was right and
the launch still waited for as long as the holder lived, which for a
devpod up reparented to init is until the machine reboots (devlaunch#602).

Everything that line told the reader to do, dl now does. On devpod's lock
line the launch reaches the same sweep 'dl <ws> kill' runs, kills whatever
holds the workspace that nothing is waiting on, and reports it by pid and
command line.

The up is neither restarted nor abandoned, and that is the measurement the
fix rests on: devpod's acquire polls behind its five-second line, so the
blocked up takes the freed flock itself. Measured on a host, one second
after the holder was killed. So the sweep releases the lock and returns,
and the up it is watching goes on to build.

What it will not do:

- Signal a holder somebody is waiting on. The distinction is kill's own
  Parentage, reused rather than rewritten, so it cannot drift between the
  two verbs. Behind a live build the launch keeps blooop#601's notice and wait.
- Delete the workspace. kill deletes because you are finished with it; a
  launch is you asking for it, so a built workspace wedged behind an
  orphan gets its lock back and keeps its container and volumes.
- Unlink the flock, touch devpod's busy marker, or kill a container. The
  marker and the containers belong to the build this launch is running.

The escalation is factored into sweep_holders so workspace_kill and the
new release_the_lock share one implementation; what dl <ws> kill does is
unchanged, and its tests are untouched and still pass.

Verified on the host: both reproductions in the ticket, cold and warm.
Each second command ends connected, nothing is left holding either
workspace, and fuser on both locks names nobody.
JSmithRobotics pushed a commit to JSmithRobotics/devlaunch that referenced this pull request Sep 16, 2026
Two changes since 0.41.0, both of them merged today.

A launch blocked on devpod's workspace lock now clears the orphan holding
it and connects, rather than printing advice and waiting until the machine
reboots (blooop#602, blooop#603). Everything blooop#601's notice told the reader to do, dl
does: it reaches the same sweep `dl <ws> kill` runs, kills what nothing is
waiting on, and says what it killed. The blocked `up` takes the freed flock
itself, so nothing is restarted and no workspace is deleted to fix a lock.

And a page for the other kind of agent, the one driving `dl` as a tool,
with a `--help` pointer to it (blooop#604), corrected where it wrote down shapes
the code does not produce (blooop#607): `unsaved` has five, not two, and two of
the three it omitted are the ones that mean "do not delete this".

Cut on a branch of its own rather than on either of theirs: both had
already merged by the time the version moved, so there was no landing push
left to carry it. 0.42.0 is untaken.
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.

A launch blocked on devpod's workspace lock waits forever and says nothing, though rm says exactly the right thing

1 participant