Skip to content

The Provision seam names none of the three things that decide a pass #397

Open

Description

@blooop

Finding

The seam between the two hottest modules in the repo mentions none of the three things that decide what happens across it, and the production adapter throws the answer away.

trait Provision (flows/launch.rs:974):

fn provision_tools(&self, runner: &dyn Runner, workspace_id: &str,
                   occasion: PassOccasion, title: Option<&str>)
    -> Result<(), DevpodMissing>;

provision::provision_tools (flows/provision.rs:1779) takes 8 parameters — the four above plus switches, host, verdicts — and answers with Provisioning, an 8-arm enum. The one production adapter closes the gap by holding the missing three as construction-time state and discarding the answer, at dl/src/launch.rs:181-183:

provisioned.map(|outcome| { let _: Provisioning = outcome; })

The comment there justifies discarding it for rendering, and that reasoning is sound: the workspace is up, and most arms are not worth a word. The cost is not rendering, it is testability.

What nothing can assert

  • launch.rs's 122 tests observe only (workspace_id, occasion, title) through RecordingProvision (flows/launch.rs:3269). Switches, host and the verdict cache are invisible.
  • provision.rs's 133 tests drive provision_tools directly and cannot see which verb produced which PassOccasion — that is decided in launch.rs:1131, :1170, :2822.
  • So verb → occasion → cache verdict → trip count has no single place it can be asserted, and the verdict cache is the feature that makes a launch do less.

verdict_cache.rs's own 11 tests write workspace_result.json by hand and backdate mtimes to compensate.

Shape

Keep the narrow seam — launch.rs:970-974 argues correctly that the occasion is a parameter and not a second method, and holding switches/host/verdicts in the adapter is what stops a launch getting provisioning wrong. Change two things:

  • bundle the request: provision(&self, runner, req: &PassRequest) -> Result<Provisioning, DevpodMissing>
  • return Provisioning rather than ()

The binary still renders nothing for most arms. A test can read them, and wf gains an answer it currently cannot get.

Proving it

A test in launch.rs that runs the same verb twice and asserts the second pass is CachedProvisioned — impossible to write today, which is the point.

Found by an architecture review.

Activity

  1. blooop commented on Aug 29, 2026

    @blooop
    OwnerAuthor

    Triage note: unblocking soon (gated on #340, in build under the #299 closeout), but the seam has drifted since this was written, so re-read before building. On main the trait is at flows/launch.rs:1094, returns Result<Option<ClaudeConfig>, DevpodMissing> rather than Result<(), DevpodMissing>, and has grown remembered_claude; the finding still holds in essence (the adapter still discards Provisioning at dl/src/launch.rs:~205 and still holds switches/host/verdicts as construction state), but the quoted signatures and the "return Provisioning rather than ()" instruction are stale as written. The proving test to keep: run the same verb twice, assert CachedProvisioned, which pins a real warm-path saving nothing can assert today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions