docs(research): no divergence carries a test that survives its own revert (#338)

[tucktuck101]

Summary

Answers #338 by reverting divergences to the upstream merge-base and running the owning crate's tests. No divergence is protected by a test that survives its own revert — Rust puts unit tests in the file they test, so reverting the file deletes its own guard. Two divergences turn out to be genuinely protected by the windows-rust clippy job, and two (restore.rs, runtime_commands.rs) are genuinely exposed.

Related issue

Closes #338

Issue type

Task

---

Agent provenance

Field	Value
Harness / provider	Claude Code
Model	claude-opus-5[1m]
Session reference	N/A - the harness exposes no shareable run id
Initiating human	@tucktuck101

Objective

Add launchpad/Research/338-divergence-revert-tests.md recording, per diverged product-code file, whether any existing test fails when that divergence is reverted.

Impacted components

launchpad/Research/338-divergence-revert-tests.md

Approach and rejected alternatives

Mutation-style: revert one file to the merge-base, run the owning crate's tests, restore. This is the same principle as launchpad/scripts/mutation_harness.py, which the cohort already runs in launchpad-pr-check.yml.

Ran two of the nine and reasoned the other seven from their diffs and references. Rejected running all nine: desktop/src-tauri's test suite takes 666s per pass, the host was at 99% disk when this started, and the two executed cases established the structural finding that generalises. Every reasoned verdict is labelled as reasoned, and the one I most want executed is named.

Rejected reporting a simple protected/unprotected split. The interesting result is what protects each file — compiler, CI job, or nothing — and a binary would have hidden that two files are protected by something better than a test while two are protected by nothing at all.

Verification

Command run:

MB=f8692fa9b52ddcfeb4b95fb4862109983509f131
cp "$file" /tmp/keep.bak && git show $MB:$file > "$file"
cargo test -p buzz-persona          # with crates/buzz-persona/src/resolve.rs reverted
cargo test -p buzz-cli              # with crates/buzz-cli/src/commands/pack.rs reverted
cp /tmp/keep.bak "$file"            # restore
git status --short                  # confirm tree clean afterwards
grep -n "Clippy (workspace)" -A1 .github/workflows/ci.yml

Raw output:

=== crates/buzz-persona/src/resolve.rs reverted; cargo test -p buzz-persona ===
test defaults_merge_persona_overrides ... ok
test resolve_full_pipeline ... ok
test full_pipeline_load_and_validate ... ok

test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
EXIT=0
--- restored crates/buzz-persona/src/resolve.rs

=== crates/buzz-cli/src/commands/pack.rs reverted; cargo test -p buzz-cli ===
     |
  52 | pub fn cmd_inspect(path: &str) -> Result<(), CliError> {
     |        ^^^^^^^^^^^
help: remove the extra argument
     |
2022 -             PackCmd::Inspect { path, format } => commands::pack::cmd_inspect(path, format),
2022 +             PackCmd::Inspect { path, format } => commands::pack::cmd_inspect(path),
     |
For more information about this error, try `rustc --explain E0061`.
error: could not compile `buzz-cli` (lib) due to 1 previous error
--- restored crates/buzz-cli/src/commands/pack.rs

$ git status --short
(empty - tree clean, both files restored)

# the Windows guard, from ci.yml
      - name: Clippy (workspace)
        run: cargo clippy --workspace --all-targets --target $env:TARGET -- -D warnings

• Tests or checks were run and the raw output is pasted above
• The diff is confined to the scope of the linked issue
• No secrets, keys, tokens or hostnames were added to tracked files

Not verified

Seven of the nine files were reasoned, not executed. lib.rs, shell.rs, lifecycle.rs, restore.rs, runtime_commands.rs, runtime.rs and runtime/summary.rs were analysed from their diffs and from what references them.

The restore.rs / runtime_commands.rs verdict is the one I would most want executed, because the document's main recommendation rests on it. It is also the best-supported reasoning — the types are identical so it compiles, and no test in the managed_agents tree mentions the behaviour.

shell.rs / lifecycle.rs being protected was not demonstrated. I did not run a Windows or cross-target build. Cheap to confirm: cargo clippy --target x86_64-pc-windows-msvc with the cfg(unix) attributes removed.

A consistent revert was never tested. Every compile error here came from reverting one file while its counterpart stayed at the fork's version. Reverting pack.rs and lib.rs together — the realistic merge — was not attempted, and it is the scenario that matters. I expect it compiles and passes; that is the load-bearing untested claim.

Not addressed: whether these nine are the right nine (#339), whether desktop/src-tauri has integration tests outside src/ touching these paths, and the other 33 changed upstream files.

Security implications

None from the document. One finding in it is security-adjacent and worth a reviewer's attention: the restore.rs / runtime_commands.rs divergence controls which relay a managed agent dials, and nothing asserts it. A silent revert would send agents at key.relay_url instead of the configured relay. That is a connection-target regression with no test standing in its way.

Working-tree hygiene: two source files were temporarily modified during the experiment and both were restored; git status --short afterwards is empty, and the only committed change is the research document.

Escalations

1. Criterion 4's wording does not survive this finding. "Carries a test that fails if a merge reverts it" cannot be met by a test inside the diverging file. A guard must live outside the file it guards. Rewording the criterion is a human call.
2. Criterion 4 should probably admit non-test guards. The windows-rust clippy job protects two divergences at compile time — cheaper and stronger than a test. A register counting only tests would under-report the fork's safety and push effort at things the compiler already holds.
3. Two one-line divergences are genuinely exposed and a single test would close the real gap. I have not written it; that is work, and it belongs to whoever picks up criterion 4 with prd: the change agent — own the block/buzz → launchpad-26/buzz relationship, including conflict resolution #273's register.

[serina-mcfall]

Blocker — runtime.rs is not a pure extraction; it carries the relay behaviour fix, and the note concludes it has no behaviour to protect

The note classifies runtime.rs / runtime/summary.rs as a code move — "280 lines leave, 283 arrive" — and concludes:

So there is no behaviour here to protect with a test that is not already unprotected upstream.

and therefore scopes criterion 4 down to:

Two (restore.rs, runtime_commands.rs) are genuinely exposed, and they are one line each.

runtime.rs is where the behaviour change actually lives:

$ git show f8692fa9b:desktop/src-tauri/src/managed_agents/runtime.rs | grep -n effective_relay_url
501:    let effective_relay_url = runtime_key.relay_url.clone();
532:    command.env("BUZZ_RELAY_URL", &effective_relay_url);

$ git show origin/launchpad:desktop/src-tauri/src/managed_agents/runtime.rs | grep -n effective_relay_url
245:    let effective_relay_url = relay_url.to_string();
276:    command.env("BUZZ_RELAY_URL", &effective_relay_url);

runtime_key.relay_url.clone() → relay_url.to_string(). That is joshuavial's fix. spawn_agent_child is the function that decides the dialled address and writes it into BUZZ_RELAY_URL; restore.rs and runtime_commands.rs are only its two callers, passing &relay_url instead of &key.relay_url.

Reverting runtime.rs to the merge-base restores the bug, with exactly the properties you attribute to the other two files: same types, compiles cleanly, no test asserts it.

So the unprotected behavioural set is three files, and the primary one is the file the note dismissed. The headline recommendation — "a single test asserting that a managed agent dials the configured relay would close the real gap" — is scoped to the callers rather than to runtime.rs:245, which is the actual assertion target.

This propagates directly into #434 (which cites this note's verdict as settled input) and into #290's criterion-4 sizing.

Your universal claim itself survives. runtime.rs and summary.rs genuinely carry zero tests, at the merge-base and at head. "No divergence carries a test that survives its own revert" is not refuted — it is the reasoning offered for this one file that is wrong.

High — the one executed result is a partial paste, and it omits the binary containing the divergence's own test

Lines 60-68 say "Reverted, the crate builds and every test passes" and paste 13 passed; 0 failed as "the clearest possible demonstration".

cargo test -p buzz-persona emits three result lines. 13 passed is the tests/integration.rs binary. The crate carries ~128 lib unit tests plus 5 in e2e_env_flow.rs — and the divergence's own test is a lib unit test at resolve.rs:772 (resolved_pack_serializes_to_json_with_expected_fields).

So the one binary whose count would actually move under the revert (128 → 127) is the one not shown. The output is real; presented as the whole-crate result it is not, and a reader cannot check the structural point from it.

High — the universal is stated flat in the frontmatter; the sample size appears afterwards

The description and line 10 assert the universal over all nine. That only two of nine reverts were executed appears at line 41 — after the conclusion and after the summary table.

Rated High, not Blocker: the caveat is present, prominent and repeated at 208-231, and you enumerate all nine rather than sampling silently. But the Blocker above is precisely what the reasoning-not-execution gap cost — runtime.rs was one of the seven analysed from its diff.

What is right

• The nine files are the right nine — 43 changed paths outside launchpad/**, of which exactly 9 are .rs, matching your list one-for-one.
• Test counts added by each divergence: pack.rs +4, resolve.rs +1. Exact.
• The windows-rust protection claim: ci.yml:985 / :1023 cargo clippy --workspace --all-targets --target $env:TARGET -- -D warnings. Verbatim.
• The pub use re-exports at runtime.rs:74/75/77 match exactly.
• No test asserts the relay behaviour — the only relay_url hits in managed_agents tests assert the opposite concern (secret_exclusion_relay_url_absent), exactly as you say.

The structural insight is the genuinely valuable contribution here: a #[cfg(test)] mod tests block living inside the diverging file cannot, by construction, survive that file's own revert. That is correct, non-obvious, and worth keeping regardless of what happens to the runtime.rs classification.

---

Reviewed at head e956e4e6f. The runtime.rs diff was verified by me directly against the merge-base.

🤖 Review drafted by Claude Code (claude-opus-5) for @serina-mcfall.