Skip to content

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

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

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