[matt2e]

This ports the advisory flake check from block/buzz-app (block/buzz-app#890) to this repository: pull requests repeat the tests they change ten times, with retries off, so a test that passes CI by luck is caught before it merges.

What it does

flake-select diffs the PR merge commit against its first parent (HEAD^1..HEAD, fetch-depth: 2) and maps changed hunks to tests per runner: a hunk inside a test body selects that test; a change anywhere else in a test file, or a new file, selects the whole file. One lane per runner then repeats only that selection 10× with retries disabled and writes a test × iteration table to its step summary. Lanes skip when nothing was selected, run only on pull_request, and are not read by any required gate in ci.yml — their own statuses carry the result.

Per runner

Runner	Selection granularity	Repeat mechanism
Rust (cargo-nextest)	crates/<crate>/<path>.rs::<module::test> from #[test]-style attribute spans (comments and literals blanked first). A hunk outside any test body selects the file's tests. #[ignore]d and Postgres / external_infra tests are excluded.	One cargo nextest run --profile flake per package per iteration, with target flags derived from the path (src/** → --lib --bins, tests/<stem>.rs → --test <stem>), a filterset of exact binary_id & test pairs, and JUnit parsed per iteration. New [profile.flake]: retries = 0, fail-fast = false.
node:test	Whole changed desktop/src/**/*.test.mjs, *.jsdom-test.mjs, desktop/scripts/*.test.mjs	10× loop, each file alone, with the flag sets from desktop/package.json test
Playwright smoke	file:line journeys from test()/it() spans (TypeScript AST) intersected with playwright test --list --project=smoke, deduplicated	Native --repeat-each=10 --retries=0, sharded min(10, ceil(journeys / 5)) ways; playwright-summary tabulates the JSON report
Flutter	Whole changed mobile/test/**/*_test.dart	10× flutter test --file-reporter=json loop with the mobile lane's --dart-define

A runner whose selection exceeds 64 KiB of entries, or more than 250 Playwright journeys, is skipped with a note rather than overrunning its timeout.

Differences from the buzz-app version

• Lanes live in a reusable _ci-flake.yml called from ci.yml, matching this repo's _ci-*.yml convention, instead of inline jobs.
• Playwright specs are parsed with the desktop app's own typescript package rather than rolldown/parseAst.
• No Playwright integration project lane: it needs the relay artifact chain. Integration-only specs list nothing for smoke and the selection stays empty.
• Playwright shards are capped at 10, the same bound as the regular smoke lane.
• Rust entries carry the file and in-file module path, and the filterset is anchored to exact (binary, test) pairs, so a same-named test in another module is not repeated.
• nextest target flags are derived from the changed file's path, so a buzz-relay change does not compile every test target of the crate.
• A journey-count cap (250) skips the Playwright lane for selections ten repetitions could not finish within its 45-minute timeout.

Heads-up

• The Unit Tests rust-cache moves from key: sherpa-cache-v1 to shared-key: unit-tests-sherpa-cache-v1 so the flake lane can restore it read-only. This changes the cache key once: the first Unit Tests run after merge builds cold.
• This PR itself changes no file the selector recognises (scripts/flake-check.test.mjs is a root script, run by the Desktop lane), so its own Flake Check lanes are expected to skip.

Verification

• node --test scripts/flake-check.test.mjs — all unit tests pass (also wired into the Desktop lane).
• actionlint on the changed workflows; biome check on the two scripts.
• scripts/test-rust-cache-contract.sh and its regression harness pass with the new shared key.
• Synthetic-diff selection runs for each runner, plus select against this PR's own merge-base (empty, as expected).
• 2-iteration repeats of a real selection on all four runners locally.

🤖 Generated with Claude Code

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: bad9d26142eaa9df8027b51a3b1bea0ca984309a...ba859ae1ecf54fdef66b6f9fdc38e6be05dc7b09
• Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability findings in the authorized PR range.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

• Review used read-only source and history inspection. Builds, tests, and repository scripts were not executed.

---

_{Generated by Codex Security Review |
Requested by: @matt2e |
Workflow run}

[jedwards27]

🤖 :bot: Jude’s code review agent

Verdict: REQUEST CHANGES before merge. I’m submitting this as a comment review per this repo’s agent-review guidance; the blocker is still concrete and author-actionable.

This PR’s stated contract is to repeat changed-test selections on pull requests without silently skipping intended targets. At exact head 8a69757b6f648a5b937d79e6c2c81d8a8e2f263a, I found two false-green paths where changed Rust test surfaces produce an empty flake selection and the repeat lanes skip.

Findings

High — Desktop Tauri Rust tests are silently excluded from changed-test repetition

scripts/flake-check.mjs only classifies Rust files under crates/<crate>/(src|tests)/** (scripts/flake-check.mjs:70-82). Desktop Tauri Rust files under desktop/src-tauri/** fall through to undefined, even though the normal CI path filter treats desktop/src-tauri/** as a required desktop-rust surface (.github/workflows/ci.yml:89-90) and the repo’s local Tauri test command runs cd desktop/src-tauri && cargo test --workspace (Justfile:231-232). There are real Tauri integration tests, e.g. desktop/src-tauri/tests/rodio_mixer_diagnostic.rs:53.

I verified the selector behavior at the reviewed SHA:

desktop/src-tauri/tests/rodio_mixer_diagnostic.rs -> undefined
desktop/src-tauri/src/lib.rs -> undefined
crates/buzz-core/src/filter.rs -> rust

And with a synthetic tree that changed only desktop/src-tauri/tests/rodio_mixer_diagnostic.rs, node scripts/flake-check.mjs select HEAD <tree> reported:

## Flake check selection

No Rust, node:test, Playwright or Flutter test files changed.

rust=

Because _ci-flake.yml skips the Rust repeat job when needs.flake-select.outputs.rust == '' (.github/workflows/_ci-flake.yml:67-70), a PR that changes or adds a Desktop Tauri Rust test gets a successful selection job and skipped repeat lanes. That is a false “nothing selected” green on a required test surface.

Author action: include desktop/src-tauri Rust test/source files in flake-check.mjs selection and repeat execution, with the correct manifest/workspace command and sidecar-stub setup if needed; or explicitly declare this surface out of scope and make the skip visible in the selection summary. Add a regression proving a changed Tauri Rust test produces a non-empty Rust selection through the real select path.

High — Rust integration-test helper changes can select no tests and skip the repeat lane

classify() treats crates/<crate>/tests/**/*.rs as Rust (scripts/flake-check.mjs:70-73), but selectRust() returns undefined when the changed file itself has no Rust test functions (scripts/flake-check.mjs:354-360), and the caller records no test functions then continues without producing a Rust selection (scripts/flake-check.mjs:518-520).

That is unsafe for shared integration-test helper modules. crates/buzz-agent/tests/common/mod.rs states that every integration test file includes it with mod common; and compiles it once per binary (crates/buzz-agent/tests/common/mod.rs:1-5); current importers include fake_llm.rs, anthropic_continuation.rs, regressions.rs, and permission_boundary.rs.

I verified the reachable behavior at the reviewed SHA by creating a synthetic tree that only appended a comment to crates/buzz-agent/tests/common/mod.rs, then running node scripts/flake-check.mjs select HEAD <tree>:

| crates/buzz-agent/tests/common/mod.rs | rust | no test functions |

rust=

So a PR can change a shared Rust integration-test harness/helper and get all flake repeat lanes skipped, even though the dependent integration binaries compile and exercise that helper.

Author action: make no-test Rust files under crates/<crate>/tests/** select the dependent runnable integration tests instead of producing an empty selection. A conservative minimal behavior would repeat all non-ignored/non-infra integration tests in the package, or all integration-test targets that could include the helper. Add a regression using crates/buzz-agent/tests/common/mod.rs or an equivalent fixture that proves a helper-only change yields a non-empty Rust selection through the real select path.

Review basis

• Local HEAD and remote refs/pull/8613/head both verified as 8a69757b6f648a5b937d79e6c2c81d8a8e2f263a before posting.
• Authenticated reviewer: jedwards27; PR author: matt2e.
• Commands used for the material findings included:
  • git rev-parse HEAD
  • git ls-remote origin refs/pull/8613/head
  • node --input-type=module -e import of classify() for the path probes above
  • synthetic-tree probes with git read-tree, git update-index --cacheinfo, git write-tree, then node scripts/flake-check.mjs select HEAD <tree>
• CI was not fully terminal when checked, but these blockers are selector-contract defects independent of the remaining in-progress jobs.

[jedwards27]

🤖 :bot: Jude’s code review agent reviewed exact head f02a4235a9fd0e60406e218acaab1b6537ca6446 against base 8216db1c712cefdc0e2af7191d501adae03ba392.

Requesting changes because this head has a PR-caused required Desktop gate failure.

Finding

High — Required Desktop JS gate fails because the new selector regression test reads stdout while CI redirects the child selector output to GitHub files

scripts/flake-check.test.mjs runs the real selector as a child process, but it does not control the child environment (scripts/flake-check.test.mjs:62-65). In GitHub Actions, that child inherits GITHUB_STEP_SUMMARY and GITHUB_OUTPUT; the production selector intentionally writes summaries to $GITHUB_STEP_SUMMARY (scripts/flake-check.mjs:444-447) and outputs to $GITHUB_OUTPUT (scripts/flake-check.mjs:724-726) when those variables are present. The new regression test then asserts the Tauri selection summary is present in result.stdout (scripts/flake-check.test.mjs:495-497), so under the same environment CI uses, result.stdout is empty.

That is already happening in live required CI: Desktop Domain / Desktop JS failed on this head with actual: '' at scripts/flake-check.test.mjs:495, which also fails the top-level required Desktop check.

Failure mode: every PR run of the Desktop JS lane inherits the GitHub output environment, so this regression test fails in CI even when the selector behavior itself is correct. That blocks merge and turns the protective test into the dungeon trap. Excellent craftsmanship, wrong victim.

Author action: make selectAppending() isolate or explicitly control child output routing. Either unset GITHUB_STEP_SUMMARY / GITHUB_OUTPUT for the child and keep stdout parsing, or set them to temporary files and read those files. Please also add a GitHub-like-env regression so this CI-only failure cannot return.

Verification owner: author fixes the test and reruns the required Desktop Domain / Desktop JS / top-level Desktop gates; I can re-review the next exact head.

Verification performed

• Confirmed authenticated reviewer jedwards27 is not PR author matt2e before submitting this review.
• Confirmed live PR head and local review checkout both equal f02a4235a9fd0e60406e218acaab1b6537ca6446 immediately before review submission.
• Confirmed required checks currently include failing Desktop and failing Desktop Domain / Desktop JS for this head.
• Ran the focused selector regression test with GitHub output env unset: node --test --test-name-pattern 'select, through its entry point' scripts/flake-check.test.mjs — pass.
• Ran the same focused test with GITHUB_OUTPUT and GITHUB_STEP_SUMMARY set to temp files — fail with the same empty-stdout assertion while the expected summary/output data is written to those files.
• Reviewed the two independent lane reports: both found the prior false-empty selector paths materially fixed and found no additional systems/product blocker beyond the required-gate failure above.

[jedwards27]

🤖 :bot: Jude’s code review agent — REQUEST CHANGES

Reviewed: 8216db1c712cefdc0e2af7191d501adae03ba392..f02a4235a9fd0e60406e218acaab1b6537ca6446 (exact head f02a4235a9fd0e60406e218acaab1b6537ca6446)
Risk: high — this changes CI/test-selection contracts. A selector false negative gives contributors a false green, and a required exact-head gate is already red.
Behavior/contracts traced: changed-file diff selection → flake-check summary/output → GitHub Actions step outputs → Rust/Tauri repeat lanes; Rust integration-target versus helper-module classification.

Findings

1. Required Desktop JS CI is red because the new regression test is not hermetic to GitHub Actions output env.
   selectAppending() asserts the child selector’s stdout (scripts/flake-check.test.mjs:48-74, scripts/flake-check.test.mjs:489-507). In GitHub Actions, though, GITHUB_STEP_SUMMARY and GITHUB_OUTPUT are set, and scripts/flake-check.mjs writes the summary/output there instead of stdout (scripts/flake-check.mjs:444-448, scripts/flake-check.mjs:703-726). The exact-head required checks show Desktop Domain / Desktop JS, Desktop Domain / Desktop, and Desktop failing. Locally, the same test passes with those env vars unset and fails in a CI-shaped env while writing the expected Tauri selection into the summary/output files. That is a PR-caused required-gate failure, not a reviewer tooling gap.

   Author action: make the test helper hermetic to GitHub Actions env — either clear GITHUB_STEP_SUMMARY/GITHUB_OUTPUT in the spawned child when asserting stdout, or provide temp files and assert their contents. Then get the Desktop JS/Desktop required gate green at the new exact head.
   Verification owner: author for fix + CI rerun; reviewer/CI for exact-head recheck.

2. A runnable Rust integration-test target with no local test functions is still treated as a helper, so the changed target is skipped and unrelated binaries are repeated.
   selectRust() returns undefined when the changed file has no locally parsed Rust test functions (scripts/flake-check.mjs:408-414), and select treats any tests/** file in that state as a helper/importer case (scripts/flake-check.mjs:640-654). But crates/buzz-acp/tests/pool_lifecycle_state.rs:1-4 is itself a real integration-test binary; it includes ../src/pool_lifecycle.rs, whose #[cfg(test)] module contains runnable tests (crates/buzz-acp/src/pool_lifecycle.rs:134-146, crates/buzz-acp/src/pool_lifecycle.rs:282-300). cargo test -p buzz-acp --test pool_lifecycle_state -- --list lists 9 tests for that target. A synthetic change to pool_lifecycle_state.rs, however, produces flake output that selects git_bootstrap, run_task, and stdio_contract and omits pool_lifecycle_state. That preserves a false-green path for integration targets whose test bodies come through #[path]/included modules.

   Author action: distinguish true helper modules from runnable integration targets. For tests/<stem>.rs and tests/<dir>/main.rs, select that target’s runnable tests — or introduce a whole-target repeat representation consumed by the runner — even when tests are discovered only through included modules. Keep importer fan-out for true helper modules. Add a regression around crates/buzz-acp/tests/pool_lifecycle_state.rs proving the selected entries include the pool_lifecycle_state target’s 9 tests and do not substitute unrelated integration binaries.
   Verification owner: author for selector/repeater fix and regression; reviewer/CI for the synthetic probe and flake lane rerun.

Validation

At clean local head f02a4235a9fd0e60406e218acaab1b6537ca6446, with refs/pull/8613/head also resolving to f02a4235a9fd0e60406e218acaab1b6537ca6446:

• gh api user → authenticated reviewer jedwards27; PR author is matt2e.
• gh pr view 8613 --json author,baseRefOid,headRefOid,statusCheckRollup,title,url → base 8216db1c712cefdc0e2af7191d501adae03ba392, head f02a4235a9fd0e60406e218acaab1b6537ca6446; required Desktop checks above are failing.
• git ls-remote origin refs/pull/8613/head → f02a4235a9fd0e60406e218acaab1b6537ca6446.
• env -u GITHUB_STEP_SUMMARY -u GITHUB_OUTPUT node --test scripts/flake-check.test.mjs → pass, 15/15.
• GITHUB_STEP_SUMMARY=$PWD/.scratch/.../summary.md GITHUB_OUTPUT=$PWD/.scratch/.../output.txt node --test scripts/flake-check.test.mjs → fail with AssertionError because tauri.summary is empty; the expected Tauri selection is written to summary.md/output.txt.
• cargo test -p buzz-acp --test pool_lifecycle_state -- --list → pass and lists 9 pool_lifecycle::tests::* tests.
• Synthetic selector run using node scripts/flake-check.mjs select HEAD <tree-with-crates/buzz-acp/tests/pool_lifecycle_state.rs-appended> → selects git_bootstrap, run_task, and stdio_contract; does not select pool_lifecycle_state.

No full local just ci run. The blocking issues above are exact-head required-gate failure plus a directly reproduced selector false negative.

Manual/native evidence: not applicable; this PR changes CI/tooling behavior, verified through exact-head GitHub checks and local selector probes.
Residual risk: the product/dev-experience lane could not complete because of an internal tool failure, so this review is limited to exact-head CI/selector contracts. That does not weaken the two author-actionable blockers above.

[jedwards27]

🤖 :bot: Jude’s code review agent

Verdict: REQUEST CHANGES before merge.

Reviewed exact head f02a4235a9fd0e60406e218acaab1b6537ca6446 against base 8216db1c712cefdc0e2af7191d501adae03ba392. Risk is high because this PR changes CI selection/repetition policy, and false-green behavior here is exactly what the PR is meant to eliminate.

Findings

High — Required Desktop JS gate fails in the GitHub Actions environment

The new Desktop JS gate runs node --test scripts/flake-check.test.mjs (.github/workflows/_ci-desktop.yml:55-56). That test spawns the production selector and reads result.stdout (scripts/flake-check.test.mjs:62-74), then asserts the selection summary is in stdout (scripts/flake-check.test.mjs:495-497).

In GitHub Actions, the child process inherits GITHUB_STEP_SUMMARY and GITHUB_OUTPUT. The production selector deliberately writes the summary to $GITHUB_STEP_SUMMARY (scripts/flake-check.mjs:444-447) and outputs to $GITHUB_OUTPUT (scripts/flake-check.mjs:724-726) when those are present. So the child’s stdout is empty under CI, and the new regression test fails even though it passes locally with those env vars unset.

Impact: this exact head already has failing required checks: Desktop Domain / Desktop JS, Desktop Domain / Desktop, and top-level Desktop.

Author action: isolate or explicitly control the child selector environment in selectAppending(): either unset GITHUB_STEP_SUMMARY / GITHUB_OUTPUT for the child when the test wants stdout, or set them to temp files and read/assert those files. Add a GitHub-like-env regression so this CI-only failure cannot return.

Verification owner: author reruns node --test scripts/flake-check.test.mjs both with and without GITHUB_STEP_SUMMARY/GITHUB_OUTPUT set, and CI proves the required Desktop JS/Desktop checks green on the new exact head.

High — Feature-gated Rust selections can still false-skip instead of being repeated

The selector can now produce real Rust/Tauri entries for previously missed surfaces, but the repeat path does not preserve the feature configuration used by the normal CI lanes. When a selected test is compiled only behind a feature, repeatRust() lists/runs cargo without that feature, finds no runnable candidates, then reports “nothing to repeat” as if every selected test were ignored or infrastructure-backed (scripts/flake-check.mjs:902-906). That is a false green on the PR’s core contract.

Root Rust example:

• Normal CI intentionally runs feature-flag tests with LaunchDarkly enabled: cargo nextest run -p buzz-feature-flags --features launchdarkly (Justfile:479-482).
• The LaunchDarkly module is behind #[cfg(feature = "launchdarkly")] (crates/buzz-feature-flags/src/lib.rs:13-15).
• The flake repeat lane calls node scripts/flake-check.mjs rust --iterations=10 $SELECTED with no feature args (.github/workflows/_ci-flake.yml:97-100).
• repeatRust() builds cargo args as -p <crate> plus target flags only (scripts/flake-check.mjs:846-851) and uses those same args for both cargo nextest list and cargo nextest run (scripts/flake-check.mjs:869-880, scripts/flake-check.mjs:922-933). There is no per-package --features launchdarkly path.

I verified the failure shape at this exact head: a synthetic change to crates/buzz-feature-flags/src/launchdarkly.rs selects 25 Rust test entries, including tests::launchdarkly_test_config_uses_null_event_processor; cargo test -p buzz-feature-flags tests::launchdarkly_test_config_uses_null_event_processor -- --list lists 0 tests, while adding --features launchdarkly lists that selected test. Without feature preservation, the repeat lane has selected work that cannot become runnable candidates.

Desktop Tauri has the same source-level contract break:

• The normal mesh lane runs cargo test --manifest-path desktop/src-tauri/Cargo.toml --features mesh-llm --lib (Justfile:234-236).
• The mesh module is behind #[cfg(feature = "mesh-llm")] (desktop/src-tauri/src/lib.rs:25-28).
• The flake Tauri lane invokes node scripts/flake-check.mjs tauri --iterations=10 $SELECTED with only CMAKE_POLICY_VERSION_MINIMUM set and no --features mesh-llm route (.github/workflows/_ci-flake.yml:161-165), while repeatRust() uses the same no-feature cargo path above.

Author action: preserve the normal CI feature configuration for selected Rust/Tauri tests when listing and running repetitions. At minimum, buzz-feature-flags selections need --features launchdarkly, and Tauri mesh selections need --features mesh-llm. Add regression coverage through the production repeat/listing seam proving a selected LaunchDarkly test and a selected Tauri mesh test become runnable candidates and cannot silently degrade to “ignored or infrastructure-backed.” Also make unmatched selected tests fail or explicitly classify why they are unmatched; do not summarize feature-missing selected tests as ignored/infra.

Verification owner: author adds the feature-gated repeat regressions and reruns the affected flake repeat lanes; reviewer rechecks focused selector/list probes plus CI on the new exact head.

Validation performed

• Confirmed authenticated reviewer jedwards27 differs from PR author matt2e.
• Confirmed local checkout, gh pr view, and git ls-remote origin refs/pull/8613/head all matched exact head f02a4235a9fd0e60406e218acaab1b6537ca6446 immediately before this review.
• Confirmed live required checks include failing Desktop Domain / Desktop JS, Desktop Domain / Desktop, and Desktop for this head.
• env -u GITHUB_STEP_SUMMARY -u GITHUB_OUTPUT node --test --test-name-pattern 'select, through its entry point' scripts/flake-check.test.mjs → pass at the reviewed SHA.
• GITHUB_STEP_SUMMARY=/tmp/... GITHUB_OUTPUT=/tmp/... node --test --test-name-pattern 'select, through its entry point' scripts/flake-check.test.mjs → fail at the reviewed SHA with actual: '' for the scripts/flake-check.test.mjs:495 assertion.
• Synthetic selector for crates/buzz-feature-flags/src/launchdarkly.rs → rust= contains 25 selected LaunchDarkly tests.
• cargo test -p buzz-feature-flags tests::launchdarkly_test_config_uses_null_event_processor -- --list → 0 tests; the same command with --features launchdarkly lists launchdarkly::tests::launchdarkly_test_config_uses_null_event_processor.

I did not run full just ci; the live required Desktop gate is already red at this exact head, and the feature-gated repeat defect is source/probe-backed. No native/manual app evidence is applicable for this CI tooling change.

[jedwards27]

🤖 :bot: Jude’s code review agent

Verdict: REQUEST CHANGES.

This integrated follow-up supersedes my earlier same-head review body; the verdict stays REQUEST CHANGES, and this adds the systems selector false-green blocker that was not in the first body.

Reviewed: 8216db1c712cefdc0e2af7191d501adae03ba392..f02a4235a9fd0e60406e218acaab1b6537ca6446 (exact head f02a4235a9fd0e60406e218acaab1b6537ca6446).

Risk: high — this PR changes the CI selector/repeater that decides which changed tests get repeated. A required-gate failure blocks every PR run; a false-green selector lets changed tests skip the extra signal this PR is meant to provide.

Findings

High — Required Desktop JS gate fails because the new regression test reads stdout while CI redirects selector output to GitHub env files

The PR adds node --test scripts/flake-check.test.mjs to the Desktop JS lane (.github/workflows/_ci-desktop.yml:55-56). In that CI environment, child processes inherit GITHUB_STEP_SUMMARY and GITHUB_OUTPUT. The selector intentionally writes its summary to $GITHUB_STEP_SUMMARY (scripts/flake-check.mjs:444-447) and its outputs to $GITHUB_OUTPUT (scripts/flake-check.mjs:724-726) when those variables exist.

But the new test helper spawns scripts/flake-check.mjs select without controlling those env vars and then parses only result.stdout (scripts/flake-check.test.mjs:62-74). The assertion for the Tauri selection table therefore sees '' in CI (scripts/flake-check.test.mjs:495-497). Live exact-head CI already shows this: Desktop Domain / Desktop JS failed in actions/runs/40429121923, job 121942775419, with actual: '', and that rolls up to the required Desktop failure.

Consequence: the PR’s own required Desktop gate is red under the same GitHub Actions output contract the selector is supposed to support. Contributors get noise instead of actionable flake signal, and the PR cannot merge.

Author action: make the test choose one deliberate output contract. Either strip GITHUB_STEP_SUMMARY / GITHUB_OUTPUT from the child and keep stdout assertions, or set both to temp files and assert against those files. Add/keep a GitHub-env-shaped regression so this CI-only path stays covered.

Verification owner: author fixes and reruns the required Desktop Domain / Desktop JS and top-level Desktop checks.

High — tests/<target>.rs integration wrapper changes are still treated as helper changes, so the changed target is omitted and unrelated sibling binaries repeat instead

crates/buzz-acp/tests/pool_lifecycle_state.rs is an integration-test target wrapper, not a shared helper: it compiles the production source via #[path = "../src/pool_lifecycle.rs"] mod pool_lifecycle; (crates/buzz-acp/tests/pool_lifecycle_state.rs:1-4). The included source contains runnable tests (crates/buzz-acp/src/pool_lifecycle.rs:138, :149, :282, :292, etc.).

At this head, appending a non-test-body line to that wrapper produces this selection summary:

| crates/buzz-acp/tests/pool_lifecycle_state.rs | rust | helper for (no importer found; every integration test): git_bootstrap, run_task, stdio_contract: 14 test(s) |

The rust= output contains tests from crates/buzz-acp/tests/git_bootstrap.rs, run_task.rs, and stdio_contract.rs; it does not contain any pool_lifecycle_state entry.

The cause is in the selector: any tests/** Rust file with no directly parsed test functions falls into the helper branch (scripts/flake-check.mjs:640-647). rustImporters() then removes the changed file from the candidate target list (scripts/flake-check.mjs:525-543, specifically :537) and falls back to every sibling target when no importer includes it. That is correct for true helpers like tests/common/mod.rs, but wrong for integration targets that are wrappers around source modules.

Consequence: this PR can report a false green for exactly the class of wrapper-only integration target changes it is supposed to repeat. The changed test target is skipped while unrelated sibling integration binaries are repeated.

Author action: distinguish integration targets from helper modules. A changed tests/<stem>.rs / tests/<stem>/main.rs target should select that target’s runnable tests, including tests brought in through a #[path] wrapper, while true shared helpers should keep importer scanning. Add a regression using crates/buzz-acp/tests/pool_lifecycle_state.rs that fails if pool_lifecycle_state is omitted.

Verification owner: author fixes selector + regression, then reruns the synthetic wrapper selection and the affected repeat path.

Validation performed

• Confirmed authenticated reviewer jedwards27 differs from PR author matt2e.
• Refreshed live PR state immediately before updating this review: head f02a4235a9fd0e60406e218acaab1b6537ca6446, base 8216db1c712cefdc0e2af7191d501adae03ba392, review decision CHANGES_REQUESTED, merge state BLOCKED.
• Confirmed required checks at this head include failing Desktop Domain / Desktop JS and failing top-level Desktop.
• In a clean exact-head checkout (git rev-parse HEAD = f02a4235a9fd0e60406e218acaab1b6537ca6446, git status --short empty):
  • env -u GITHUB_OUTPUT -u GITHUB_STEP_SUMMARY node --test --test-name-pattern 'select, through its entry point' scripts/flake-check.test.mjs — passes.
  • GITHUB_OUTPUT=$(mktemp) GITHUB_STEP_SUMMARY=$(mktemp) node --test --test-name-pattern 'select, through its entry point' scripts/flake-check.test.mjs — fails with the same empty-stdout assertion; the expected summary/output are written to the temp files.
  • A synthetic tree appending to crates/buzz-acp/tests/pool_lifecycle_state.rs selects git_bootstrap, run_task, and stdio_contract entries and prints contains_pool_lifecycle_state=false.
• Reviewed the Tauri mapping path: desktop/src-tauri/** is classified to the tauri runner/manifest, and the live PR skipped the Desktop Tauri flake lane because this PR did not change a Tauri test. No additional Tauri-manifest blocker found at this head.

Residual risk: I did not run full just ci locally; live exact-head required CI plus focused local repros are already sufficient for the blockers above. The next head needs fresh review because selector behavior and CI status are the product here.

[jedwards27]

🤖 :bot: Jude’s code review agent

Verdict: REQUEST CHANGES before merge. I’m submitting this as a comment review per this repo’s agent-review guidance; the finding below is still author-actionable.

Reviewed: 8216db1c712cefdc0e2af7191d501adae03ba392..29858b2affbc9490fc2b13e47ca5b79db7adb8d6 (exact head 29858b2affbc9490fc2b13e47ca5b79db7adb8d6)

Risk: medium/high CI correctness. This PR’s contract is that changed tests are repeated rather than silently skipped; the affected path can produce a green repeat lane after selecting zero runnable tests.

Behavior/contracts traced: changed-file selector output → _ci-flake.yml Rust/Tauri repeat jobs → scripts/flake-check.mjs cargo nextest list/run construction → normal CI feature lanes in Justfile / _ci-desktop.yml.

Blocking finding

Selected feature-gated Rust/Tauri tests can pass as “nothing to repeat”

The selector can emit tests that only exist in a non-default Cargo feature graph, but the repeat path lists/runs the default graph and treats an empty match as success.

Evidence:

• The Rust/Tauri repeat path builds only the root default cargo args or --manifest-path desktop/src-tauri/Cargo.toml; it does not preserve any per-package/per-workspace feature args when running cargo nextest list or cargo nextest run (scripts/flake-check.mjs:939-957, scripts/flake-check.mjs:998-1008).
• If nextest list finds no chosen candidates, the job emits Flake check (Rust): nothing to repeat and returns success (scripts/flake-check.mjs:975-982). Mixed selections also drop the feature-gated candidate and report only the default-graph tests (scripts/flake-check.mjs:970-982, scripts/flake-check.mjs:1021-1028).
• Normal CI has feature-specific test graphs this PR does not mirror:
  • buzz-feature-flags --features launchdarkly (Justfile:479-482), with the LaunchDarkly module gated behind #[cfg(feature = "launchdarkly")] (crates/buzz-feature-flags/src/lib.rs:13-15, crates/buzz-feature-flags/Cargo.toml:10-11) and tests such as launchdarkly_test_config_uses_null_event_processor (crates/buzz-feature-flags/src/launchdarkly.rs:366-367).
  • Desktop Tauri mesh tests via just desktop-tauri-test-mesh / --features mesh-llm (.github/workflows/_ci-desktop.yml:86-89, Justfile:234-236), with mesh-only code/tests gated behind mesh-llm (desktop/src-tauri/src/managed_agents/relay_mesh.rs:14-35, desktop/src-tauri/src/managed_agents/relay_mesh.rs:138-145).
• _ci-flake.yml invokes the Tauri repeat as node scripts/flake-check.mjs tauri ... with no --features mesh-llm path (.github/workflows/_ci-flake.yml:151-165). The same implementation handles root Rust repeats, so feature-gated root selections have the same false-empty shape.

I reproduced the control-flow failure at this head with a fake cargo standing in for nextest: a cfg-gated-only selected test returned exit code 0 with “nothing to repeat,” and a mixed ordinary + cfg-gated selection returned exit code 0 while the report listed only the ordinary test. That matches the source trace: selected tests can disappear between selection and repeat without failing the job.

Consequence: contributors can change a real feature-gated Rust/Tauri test, see a non-empty selector summary, and still get a successful flake repeat lane that repeated zero of the selected feature-gated tests. That violates the PR’s core developer contract more dangerously than an explicit skip, because it looks verified.

Author action: bind selected Rust/Tauri entries to the Cargo invocation shape that normally runs them, e.g. feature-aware repeat groups for buzz-feature-flags --features launchdarkly, buzz-push-gateway --features personal-dev-app-attest, and Desktop Tauri --features mesh-llm; and fail loudly when selected entries cannot be listed under the graph being repeated unless the selector can prove they are intentionally ignored/excluded infra tests. Add regressions proving a selected feature-gated root Rust test and a selected Tauri mesh test are either repeated under the correct feature graph or hard-fail instead of reporting “nothing to repeat.”

Verification owner: author for the fix/regressions and rerun of affected flake/CI checks; reviewer to re-check exact-head selector/repeat probes after the next push.

Validation

At matching local/live head 29858b2affbc9490fc2b13e47ca5b79db7adb8d6:

• gh pr view and git ls-remote origin refs/pull/8613/head both matched the reviewed head; authenticated reviewer jedwards27, PR author matt2e.
• node --test scripts/flake-check.test.mjs — PASS, 15/15.
• eval "$(./bin/hermit env --activate)" && actionlint .github/workflows/ci.yml .github/workflows/_ci-flake.yml .github/workflows/_ci-rust.yml .github/workflows/_ci-desktop.yml — PASS.
• scripts/test-rust-cache-contract.sh && scripts/test-rust-cache-contract-regressions.sh — PASS.
• git diff --check 8216db1c712cefdc0e2af7191d501adae03ba392...HEAD — PASS.
• gh pr checks query for non-success/non-skipped checks returned none at the final local check. Green CI does not cover this false-repeat path.

Manual/native evidence: not applicable; this is CI/tooling behavior. No local cargo-nextest run was required for the blocker because the defect is in the command construction and empty-candidate handling, and the control-flow reproduction demonstrates the false-green path.

Residual risk: I did not run full local just ci or the real 10× nextest lanes. The unresolved author action is the feature-graph repeat contract above, not general CI finality.

[jedwards27]

🤖 :bot: Jude’s code review agent — REQUEST CHANGES for exact head 280cc2385470423a32dcb2bb389ea248650667cb.

The earlier “feature-only tests absent from default graph” blocker is mostly fixed, but this head still has a false-green path for same-named tests that are present in multiple regular CI feature graphs.

Blocking defect: first listed feature graph can hide the changed branch

buzz-push-gateway is tested by normal CI in both the default graph and the personal-dev-app-attest graph (Justfile:471-472). The selected test tests::attestation_environment_is_explicit_and_build_gated has mutually exclusive assertions under #[cfg(feature = "personal-dev-app-attest")] and #[cfg(not(feature = "personal-dev-app-attest"))] (crates/buzz-push-gateway/src/config.rs:260-269).

repeatRust() knows about both graphs for this package (scripts/flake-check.mjs:112-115), but after the default graph lists the selected test it advances only choice.unlisted to later graphs (scripts/flake-check.mjs:959-1001). That means a selected test name found in the default graph is removed before the feature graph is even listed.

I reproduced this at the exact head with a synthetic diff inside the personal-dev-app-attest-only assertion. The selector chose:

rust=crates/buzz-push-gateway/src/config.rs::tests::attestation_environment_is_explicit_and_build_gated

A fake-cargo repeat probe where the same test was listed in both graphs exited successfully and recorded only default-graph calls:

cargo nextest list -p buzz-push-gateway --lib --bins
cargo nextest run --profile flake -p buzz-push-gateway --lib --bins -E ...

There was no --features personal-dev-app-attest list or run call, yet the flake summary reported success (0 of 2 iterations failed). A mutation proof confirmed the practical failure mode: after changing only the feature-gated assertion to the wrong expected value, the default graph still passed, while cargo test -p buzz-push-gateway --features personal-dev-app-attest ... -- --exact failed.

Impact: a contributor can edit a feature-specific branch inside an otherwise default-visible test, see the changed test selected, and get a green repeat lane that never compiled or ran the changed branch. That violates the PR’s core contract: changed tests should be repeated without misleading false greens.

Required author action: for packages with multiple regular CI feature graphs, either:

1. repeat selected tests in every graph that lists them; or
2. make selection cfg-aware enough to bind the changed hunk to the graph(s) that compile it, and fail loudly when uncertain.

Please add a regression for the buzz-push-gateway personal-dev-app-attest branch proving the feature graph is invoked, not just the default graph.

Other verification

Exact-head checks/evidence used for this review:

• Live PR head remained 280cc2385470423a32dcb2bb389ea248650667cb immediately before submitting this review.
• scripts/buzz-pr-review-preflight 8613 — head/base matched; required checks passed except required Security.
• git diff --check 8216db1c712cefdc0e2af7191d501adae03ba392...HEAD — pass.
• actionlint .github/workflows/ci.yml .github/workflows/_ci-flake.yml .github/workflows/_ci-rust.yml .github/workflows/_ci-desktop.yml — pass.
• scripts/test-rust-cache-contract.sh and scripts/test-rust-cache-contract-regressions.sh — pass.
• just desktop-install-ci && node --test scripts/flake-check.test.mjs — pass, 16/16.
• pnpm exec biome check scripts/flake-check.mjs scripts/flake-check.test.mjs — pass.
• Synthetic probes for the prior blockers (Tauri selections, helper-module importer expansion, #[path] integration wrappers, and pure feature-only tests absent from default) passed; the unresolved blocker is the same-name multi-graph case above.

The required Security check is currently failing on RUSTSEC-2026-0414 for pulldown-cmark 0.13.4. This PR does not change Cargo.lock, Cargo.toml, or deny.toml, so I am not classifying that as the PR-caused defect behind this request-changes review, but it remains a required gate to route before merge.