Skip to content

Say what Trips is, and correct the Runner impl count #409

Closed

Description

@blooop

Question

Doc-only, three places. Trips is a third fake devpod the conformance corpus does not cover decided that Trips is a recording harness rather than a fake devpod, that nothing joins test/fixtures/devpod/conformance.json, and that the argv guard it asked for already exists. What is left is that none of that is written where a reader lands.

1. Runner's doc line — rust/devlaunch-runner/src/lib.rs:384-385 says "Implemented once for real processes ([ProcessRunner]) and once for tests (devlaunch_test_support::FakeRunner)." There are nine impls. Replace the count with the shape, which is what a reader actually needs:

  • one production impl, ProcessRunner;
  • one shared fake, FakeRunner — call recorder, argv-prefix response table, and DevpodMachine behind them;
  • seven test-local wrappers over those two. Three of them route by program name so the programs they do not fake reach real processes: FakeDevpodRealGit (flows/listing.rs:1208), Devpod (flows/lifecycle.rs:3417), StubbedLfs (flows/workspace_clone.rs:4026). ScriptedRunner (core/testing.rs:88) adds the timing exclusion, FakeGit (flows/repo_manager.rs:1855) adds filesystem effects for git clone, and Trips (flows/provision.rs:2463) with Rebuilding (:4876) answer by call index.

Do not write "seven" as a number that will rot the way "once" did — say the structure and name the three routers, since those are the ones a reader currently learns by grepping.

2. Trips' doc comment (rust/devlaunch-core/src/flows/provision.rs:2359-2371) — say it is a recorder, not a fake devpod: it answers by call index (:2442) and never reads argv to decide an outcome, holds no workspace state, and therefore cannot be driven over a corpus row, which is given state → argv → exit + then state. Say that the fake devpod the rest of the suite meets is DevpodMachine, reached through FakeRunner, and that it is the corpus-covered one — so a reader does not conclude a third fake escaped #309.

3. Trip::script (:2347-2355) — say which argv it is re-parsing (setup_pass, :1939-1944), and that the expect is the guard: the payload moves or the flag is renamed, and this fails loudly at the read rather than returning a neighbouring element. Worth noting alongside that the shape itself is pinned as real devpod behaviour: --command is in SSH_VALUE_FLAGS (devlaunch-test-support/src/devpod.rs:180) and conformance.json:258 carries an ssh ... --command row measured against v0.26.1.

Optional one-liner while in there: &self.argv[at + 1] panics with an unnamed index message when --command is the last element. .get(at + 1).expect("a --command with a payload") says which of the two assumptions broke. Test-side code, no production change.

Testing the outcome

No behaviour changes, so no new test. pixi run cargo test --workspace, clippy --locked --all-targets -- -D warnings and fmt --check from rust/ are the whole gate. The claims above are checkable by hand: grep -rn "impl Runner for" rust/ returns nine, and three of those files hold a ProcessRunner field.

Out of scope

Adding corpus rows for Trips, adding a second argv guard, and retiring any fake (ruled out on evidence in #309).

Activity

  1. self-assigned this
    on Aug 24, 2026
  2. blooop commented on Aug 24, 2026

    @blooop
    OwnerAuthor

    Built in #415 (docs/runner-seam-and-trips), not merged.

    Counts confirmed against the tree: nine impl Runner for, three of which hold a
    real ProcessRunner (FakeDevpodRealGit, Devpod, StubbedLfs) — #405's
    numbers hold. One nuance: StubbedLfs routes on the lfs subcommand rather than
    the program name, and only its capture reaches real processes, so the doc says
    "hands it whatever they do not fake".

    Per the steer in the ticket, the Runner doc states the shape and names the grep
    as the enumeration rather than listing the seven wrappers or the three routers —
    both are counts that rot the way "once" did. What it names instead is the two
    shapes worth recognising: part real, and recorder. Trips, Trip::script and
    devlaunch-test-support/src/devpod.rs's module doc got the rest.

    One guard added, devlaunch-runner/tests/one_seam.rs: ProcessRunner is the
    only Runner outside test code, scanned rather than asserted in prose. Proven
    red first. Gate green from rust/ (test --workspace, clippy -D warnings, fmt).

  3. blooop commented on Aug 24, 2026

    @blooop
    OwnerAuthor

    breadcrumb: wf-review on #415 — request changes. Docs verified accurate and Trip::script is a clean improvement, but the new guard has two demonstrated bypasses (impl<'a> Runner for X, impl devlaunch_runner::Runner for X both pass green) and one demonstrated false positive (any attribute between #[cfg(test)] and mod tests { makes test wrappers read as production). Also: this ticket asks to "name the three routers" and the shipped doc names only FakeDevpodRealGit — worth reconciling here. #415

  4. blooop commented on Aug 24, 2026

    @blooop
    OwnerAuthor

    breadcrumb: the review's spec finding on this ticket is settled in the
    ticket's favour
    , in 1dead6d on #415.

    "Do not write 'seven' as a number that will rot the way 'once' did — say the
    structure and name the three routers" is one sentence, and the clause forbidding
    the count does not carry over to the names; the names are what the sentence asks
    for. No later steer superseded it. The PR body claimed the opposite and has been
    corrected. Runner's doc now names FakeDevpodRealGit, flows::lifecycle's
    Devpod and flows::workspace_clone's StubbedLfs in the "Part real" bullet,
    with StubbedLfs's nuance stated, and still writes no count for the wrappers.

    The same commit rebuilds tests/one_seam.rs around a token rather than a line,
    which is what the review's two blocking guard findings turned out to be: both
    bypasses (impl<'a> Runner for X, impl devlaunch_runner::Runner for X) and the
    #[cfg(test)] false positive were artefacts of the unit, plus a fourth hole
    where anything written below an inline test module read as test code. Seven of
    the nine tests run the scanner over source held as a string; four were red
    against the old logic first.

    Still open, not closed by this.

  5. blooop commented on Aug 28, 2026

    @blooop
    OwnerAuthor

    Shipped in PR #415. The Runner doc states the shape and, after the review settled the spec dispute this ticket's way (commit 1dead6d), names all three routers (FakeDevpodRealGit, Devpod, StubbedLfs). Trips is documented as a recorder, not a third fake devpod, and the count guard lives in devlaunch-runner/tests/one_seam.rs, rebuilt token-wise after the review's bypass findings.

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

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions