CI: a source-only change that breaks (or fails to compile) an integration target under tests/ ships green

[M3gA-Mind]

Summary

A change confined to src/ that breaks an integration target under tests/ — including one that stops it compiling — passes every lane on a PR. Nothing in CI Lite builds integration test targets for a source-only change, so the break lands on main with a fully green PR.

Not hypothetical: found on #6214 today, which was green and mergeable with tests/raw_coverage/inference_agent_raw_coverage_e2e.rs uncompilable.

The three lanes, and why each misses it

Lane	Command	Builds tests/**?
Rust Quality (fmt, clippy)	cargo clippy -p openhuman --features "$(…)" -- -D warnings (ci-lite.yml:435, :445)	No — no --all-targets, so only lib + bins
Rust Core Coverage	scripts/ci/rust-coverage-changed.sh (ci-lite.yml:877)	Only if the changed files select one — see below
Test (test-reusable.yml, the only workflow that runs cargo test --test <target>)	reached via test.yml	Never on a PR — test.yml is on: workflow_dispatch: {}

Why the coverage lane misses it

rust-coverage-changed.sh is changed-files-driven and correct on its own terms:

• tests/raw_coverage/*.rs changed → --test raw_coverage_all (:343-352). Good.
• src/** changed → a --lib libtest filter, plus any target named by domain_integration_targets().

And domain_integration_targets() is, in full:

domain_integration_targets() {
  case "$1" in
    src/openhuman/agent/harness/session/* | src/openhuman/threads/goals/*)
      printf '%s\n' agent_turn_overrides_e2e
      ;;
  esac
}

One mapping, two directories, one target. Every other source path in the tree maps to --lib alone. The script's own header already names the class (:10): "integration targets that GUARD a domain but live outside --lib, so a PR that touches only the domain's src/ never runs the gate." The design knows about this; the mitigation is a hand-maintained allowlist that has not kept up, and cannot be expected to.

Worked example (#6214)

• Changed src/openhuman/agent/orchestration/tools/** — not in the mapping, so scope was --lib only.
• That changed ArchetypeDelegationTool.agent_id from String to a new DelegationTarget newtype.
• Two call sites in tests/raw_coverage/inference_agent_raw_coverage_e2e.rs use agent_id: "researcher".into(), which resolved via From<&str> for String.
• No tests/ file changed, so raw_coverage_all was never selected.

Measured, with cargo check -p openhuman --no-default-features --features "$(bash scripts/ci/product-features.sh)" --test raw_coverage_all:

Tree	Result
upstream/main (5e543a76b)	exit 0
main + 605fc7fa6 (#6214)	exit 101 — E0277: DelegationTarget: From<&str> is not satisfied

PR state at that commit: 21 SUCCESS, 0 failing, Rust Core Coverage SUCCESS.

Why this is worse than a coverage gap

Two different things are being conflated by the current setup:

1. Which assertions run. Genuinely a judgement call, and what rust-coverage-changed.sh exists to scope. Running every suite on every PR is not affordable, so scoping is right.
2. Whether the tree compiles at all. Not a judgement call. It should not be possible to merge a commit that does not build, and that is not a per-domain decision — it is the same answer for every path in the repo.

The current lanes only answer (1), so (2) goes unasked. And raw_coverage_all is an aggregated target (~76 suites via tests/raw_coverage_all.rs + build.rs), so a single type error in it blocks local verification of every suite inside it, not just the one that broke.

Suggested fix

Add a compile-only gate that is scope-independent:

cargo check -p openhuman --no-default-features \
  --features "$(bash scripts/ci/product-features.sh)" --all-targets

• Catches 100% of this class regardless of which files changed — no allowlist to maintain, nothing to keep up to date.
• Compile-only, so far cheaper than running the suites; it is a build the coverage lane largely does anyway.
• Orthogonal to rust-coverage-changed.sh, which keeps doing the job it is good at. This is not a replacement for scoped coverage.

One wrinkle to handle: naming a target whose required-features are unsatisfied is a hard error, not a skip — rust-coverage-changed.sh:160-190 already solves this with target_features_satisfied() and documents the incident where dropping memory-git from the product set took the lane down. --all-targets selects implicitly, so it skips rather than errors, but this should be confirmed against the current required-features set before the gate is made blocking.

Alternatively, extending domain_integration_targets() would narrow the hole but not close it — it is an allowlist, so it is only ever as good as the last person who remembered to update it, which is the failure mode being reported.

Related

• fix(agent): make load_skill's listing agree with the gate that refuses it #6214 — where this was found; the break itself is W2's to fix and is already reported on the PR.
• fix(agent): scope load_skill's advertised index to what the session can call #6215 — inherits the same break (it contains fix(agent): make load_skill's listing agree with the gate that refuses it #6214's commit).

[tinysweeper]

CI: source-only changes can merge despite breaking compilation of integration tests under tests/, because no lane builds those targets for such changes. A compile-only gate like cargo check --all-targets is needed.

Labelled priority: p2.

[M3gA-Mind]

This now has a merged example, which is the case I could not make when I filed it.

#6214 merged green carrying exactly this break. All 19 checks passed. main is at 10d2f1c67 and does not compile --test raw_coverage_all:

error[E0277]: the trait bound `DelegationTarget: From<&str>` is not satisfied
  --> tests/raw_coverage/inference_agent_raw_coverage_e2e.rs:3086:32

Verified on a tree with 0 top-level and 0 nested submodule drift, so it is the trait bound and not a dependency-resolution artifact — worth stating because a first attempt at this control elsewhere exited 101 on a missing nested tinyinference submodule and nearly got reported as the break.

Repaired in #6224. That PR does not close this issue — it fixes the one call site; the lane that let it through is unchanged.

The timeline is the argument. The break was identified, reported on the PR with a reproduction and an exact patch, and re-verified at three successive heads (605fc7fa6 → dbdad815c → 2ce124725) before the merge. It still merged, because every signal the merge gate actually consults said green. No amount of a human or an agent noticing compensates for a lane that cannot see the file — the observation has to be something CI does, or it does not count at merge time.

One correction to my original report, since it bears on the proposed fix: I described the coverage lane as --lib-scoped. More precisely, rust-coverage-changed.sh is changed-files-driven and does select raw_coverage_all when a tests/raw_coverage/*.rs file changes. The hole is narrower and worse than "it only runs --lib": for a source-only change it maps to a --lib filter plus domain_integration_targets(), and that function is two directories and one target in its entirety. #6214 changed only src/, so nothing selected the target. An allowlist cannot close this, because the failure mode is precisely that nobody remembered to extend it.

The cargo check --all-targets gate proposed above would have caught this one with no allowlist entry and no judgement call.