fix: a warm launch says what it skipped (#560 sections 1 and 2) - #561
Merged
Conversation
Two reports from #560, and they share a root: a launch of a workspace devpod already knows skips both the git work and the setup pass, and the terminal said nothing about either. A warm launch of `owner/repo@branch` now says how far behind its checkout is, when it is behind. The count comes out of dl's own clone -- one `rev-list` against a local repository, no network -- so the claim is narrow on purpose: how the checkout stands against the `origin/<branch>` that clone last fetched, of whatever age, and never a claim about the remote now. A fetch on attach was the other candidate and would have traded away devlaunch#144's decision to fix a reporting problem. Silent when the counts agree, when the checkout is only ahead, when there is no clone on disk, for a bare workspace name, and for a warm resolution addressed by a recorded id rather than the derived one. Every `devpod up` now says whether it forwarded dotfiles, naming the repository and script it passed or saying that devpod's context options name none. dl reads DOTFILES_URL from `devpod context options` and from nowhere else, and it used to forward or omit the flags in silence either way -- which is what turned one report of "the dotfiles never landed" into a fortnight of three plausible causes and no observation to cut between them. `reset`'s help line and the README table stopped saying "Clean slate: remove everything, recreate", which reads as a promise about the checkout that reset does not keep: `rm` is the only verb that refreshes git state, and docs/cli.md and docs/workspaces.md now say so. docs/cli.md also gained the diagnosis for `inject agent ... exit status 126` (section 3 of the issue): devpod globs `uname -a` for `arm` to pick its agent binary, and `uname -a` carries the container hostname. The match is devpod's and is not fixed here. Claude-Session: https://claude.ai/code/session_01Dn4qJGhkW4KdwQnMuNSXsN
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideThis PR adds local, no-network stale-checkout diagnostics to warm and reset launches, emits an auditable dotfiles-forwarding notice for every devpod up, and documents the arm-related agent-injection diagnosis while intentionally deferring the stderr-handling code change. It adds unit, integration, fixture, golden, and API-snapshot updates covering the new notice paths and their required silent cases. Sequence diagram for a warm launch checkout freshness noticesequenceDiagram
participant User
participant Launch
participant WorkspaceClone
participant Git
participant Devpod
User->>Launch: run owner/repo@branch
Launch->>Devpod: status workspace
Devpod-->>Launch: workspace already running
Launch->>WorkspaceClone: checkout_freshness(repos_dir, workspace)
WorkspaceClone->>Git: ahead_behind(clone, refs/remotes/origin/branch)
Git-->>WorkspaceClone: rev-list counts
WorkspaceClone-->>Launch: ahead and behind counts
alt checkout is behind
Launch-->>User: CheckoutBehind notice
end
Launch->>Devpod: ssh workspace
Devpod-->>User: attach session
Sequence diagram for dotfiles forwarding during devpod upsequenceDiagram
participant Launch
participant Devpod
participant Notices
Launch->>Devpod: context options --output json
Devpod-->>Launch: DOTFILES_URL and script options
Launch->>Notices: dotfiles_notice()
alt DOTFILES_URL configured
Launch->>Devpod: up with --dotfiles and optional --dotfiles-script
Notices-->>Launch: DotfilesForwarded
else DOTFILES_URL not configured
Launch->>Devpod: up without dotfiles flags
Notices-->>Launch: DotfilesNotConfigured
end
Launch-->>Notices: render notice
Flow diagram for checkout refresh semanticsflowchart LR
Launch[Workspace command] --> Existing{Workspace devpod exists?}
Existing -->|yes| Container[Act on existing container]
Existing -->|no| Cold[Cold launch]
Container --> GitCheck[Optional local checkout freshness check]
GitCheck -->|behind| Notice[Report checkout behind last-fetched ref]
GitCheck -->|agreeing, ahead, missing, or bare name| Silent[No freshness notice]
Container --> Reset[restart, recreate, or reset]
Reset --> Unchanged[Checkout remains unchanged]
Cold --> Fetch[Fetch or clone git state]
Rm[rm] --> Delete[Delete workspace and clone]
Delete --> Fetch
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:
|
# Conflicts: # CHANGELOG.md # rust/devlaunch-core/public-api.api.txt
…d a script
`up_args` pushed `--dotfiles-script` on DOTFILES_SCRIPT alone, and
`dotfiles_notice` branched on DOTFILES_URL alone. So
devpod context set-options DOTFILES_SCRIPT=install.sh # and no DOTFILES_URL
put `--dotfiles-script install.sh` in the argv and printed
dotfiles: none set in devpod context options, so this up asked for none.
That is precisely the drift this notice exists to catch. Its own doc comment says
so -- derived from the same two readers rather than from the argv, because "the
disagreement worth catching is exactly a reader that stopped feeding the argv" --
and the notice was the half that was right.
Fixed at the argv rather than by teaching the notice a third state, because the
flag was meaningless: `--dotfiles-script` names a file *inside* what `--dotfiles`
cloned, so with no repository it asks devpod to run a script in a repository
nobody named. Nothing is lost by not sending it, and the two readings agree again
without a variant describing a configuration that cannot work.
The existing argv test sets both options, so it never saw this.
Claude-Session: https://claude.ai/code/session_01U4SpX4ixqQG5R8hErsPMot
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
1 participant
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.
Addresses sections 1 and 2 of #560 — A warm attach reports neither a stale checkout nor a skipped setup pass, and an
armin a workspace id kills the launch with exit 126, and gives section 3 the diagnosis it can have without a design decision. Section 3's code change is deliberately not here; see the last part of this description for why.The two sections in scope share the root the issue names: a launch of a workspace devpod already knows skips both the git work and the setup pass, both skips are deliberate and documented, and neither was visible at the moment it mattered.
1. A warm launch says how far behind its checkout is
dl owner/repo@branchagainst a workspace devpod already has now prints one line before the attach banner, when the checkout is behind:No network call, and the wording is scoped to what that buys. The count is
git rev-list --left-right --count HEAD...refs/remotes/origin/<branch>against dl's own clone. A fetch on attach would have traded away the decision #144 settled and #149 / #150 built, to fix a reporting problem, and it was never needed: the fact was local the whole time. So the sentence says how the checkout stands against a ref of whatever age the last fetch left it, and claims nothing about the remote now.The symmetric difference rather than
HEAD..<ref>, so a diverged checkout is two facts: local commits are named as the reader's own ((and 3 of its own it has not pushed)) instead of being counted as staleness.Five shapes stay silent, each pinned by a test:
dl <workspace-id>by bare name:Plan::Existingcarries no triple, so there is no branch to name and no clone path to derive, and the triple the picker recovers is allowed to name the tab and nothing elsemetadata.jsonrecorded rather than the derived one (#88's arm): the clone directory is a function of the derived id, and the recorded path is only readable by opening the store, which this arm may not do (#145)cold.opens.get() == 0is asserted on the reporting path, soa_launch_that_matches_its_own_record_attaches_and_reads_no_machinerykeeps meaning what it meant.And
resetsays it too, and still leaves the checkout alone. That was the half that cost the reporter a session:resetreads like the answer to an out-of-date checkout and is not one, because devpod's--resetremoves a devpod-managed source and dl hands devpod its own local folder.reset_reports_the_stale_checkout_it_is_about_to_rebuild_over_and_leaves_it_aloneasserts the line, the--resetflag, and that the clone is exactly as far behind afterwards as before, read out of the clone rather than taken on trust.reset's help line and the README table stopped calling themselves a clean slate;docs/cli.mdanddocs/workspaces.mdgained the sentence thatrmis the only verb which refreshes git state.2. Every
devpod upsays whether it forwarded dotfilesdl reads
DOTFILES_URLfromdevpod context optionsand from nowhere else, and it forwarded or omitted the two flags in silence either way. That silence is what made the report a fortnight rather than an afternoon: three plausible causes and no observation to cut between them.an_up_says_which_dotfiles_it_asked_devpod_for_and_the_argv_agreesasserts the sentence and thedevpod upargv in one test, so the printed line and the flags cannot drift.Said on the
upand only on theup: an attach that runs noupasked devpod for nothing either way, and a line there would suggest it had.docs/workspace-tools.mdgained the section on where the setting comes from, including thatcontext-options.jsonis dl's cache of devpod's answer rather than an input.This is one new line on every
up. Two goldens changed to carry it. The negative arm is the one that answers the reported case, so it is not gated on the positive.3. Not in this PR, and why
The issue's own framing is that section 3 can be picked up on its own, and both halves of it need something this PR should not decide alone.
The upstream PR is a link, from a branch in
blooop/devpodtoskevetter/devpod, in a repository this change does not touch. That is yours to open.The dl-side half rests on a premise that does not hold. The issue proposes matching devpod's stderr for
inject agentplusexit status 126, citingclients/devpod.rs:331as dl already reading devpod's stderr as it arrives. That reader isdevpod::run_watching_stderr, and its only caller isflows/lifecycle/delete.rs.devpod upgoes throughdevpod::run, which is a passthrough that captures nothing, deliberately: an image build's progress belongs on the user's terminal. Making the 126 legible as specified therefore means pipingdevpod up's stderr through dl on the main build path, which changes what devpod sees of the terminal and what it prints. That is a real trade on the path every cold launch takes, and it is not the trade the issue was weighing.What is here instead is the diagnosis, in
docs/cli.md: theuname -aglob, the one-line check for any running container, thedocker cpthat unblocks a container already in that state, and the note that a recreate wipes it. It also records that dl's own setup pass writes the workspace id into the container's hostname, so dl is one of the waysarmgets there.Testing
cargo test --workspace,cargo clippy --locked --all-targets -- -D warnings,cargo fmt --check, andpytest test/(707 passed) all clean.mainin this devcontainer and untouched by this change:flows::provision::tests::the_probe_reports_the_config_facts_when_it_actually_runsexpectsclaudemountsto be empty and this container's own mount table makes the scan answer/.rust/devlaunch-core/public-api.api.txtwas hand-edited, in sorted position for the four newLaunchNoticerows in each of the two paths the generator renders them at.cargo public-apineeds a nightly toolchain the devcontainer does not carry, so the CIpublic-apijob's diff is the first real check of it; if it disagrees, regenerating on a host is the fix.New fixtures in
launch_scenario.py:--stale-checkout(two commits onorigin.git'smain, fetched into the warm workspace's clone and not checked out) and--dotfiles(devpod context optionsnaming a repository, in devpod's own nested shape rather than the flat shape dl's cache holds).Divergence rows 33 and 34 record both new lines, since neither has a Python precedent.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Dn4qJGhkW4KdwQnMuNSXsN
Summary by Sourcery
Make warm launches and provisioning diagnostics explicit without adding network fetches or changing the established warm-attach behavior.
New Features:
devpod up.arm.Bug Fixes:
Enhancements:
rmis the only workspace verb that refreshes git state and thatresetrecreates container resources without changing the checkout.Documentation:
Tests: