Skip to content

Order-dependent unit tests hide behind the domain-scoped Rust lane and ambush unrelated PRs #6198

Description

@M3gA-Mind

Summary

The Rust unit lane hides order-dependent test failures, and they surface only when a PR happens to touch the right domain. Three workers hit this cluster independently today. Each one costs a debugging session to attribute, because the failure appears on the PR that exposed it, not the one that caused it.

The mechanism is not one bug — it is a pattern: a test asserts against process-global state it did not install, so what it actually asserts depends on which sibling test ran first in the same binary.

Why it hides

ci-lite scopes the Rust coverage lane to the changed files' domain (scripts/ci/rust-coverage-changed.sh): a push touching src/openhuman/agent/** runs -- 'openhuman::agent', and a push touching something else never runs that filter at all. So a regression can sit green on main indefinitely and then fail on the next PR that widens the scope to include it.

That is the "scoped coverage hides orphaned tests" shape. The lane is not wrong to be scoped — but nothing currently runs the whole suite on a cadence, so nothing catches these between ambushes.

Confirmed instance (fixed in #6189)

memory_access_instruction_is_present_with_learning_disabled
(src/openhuman/agent/harness/session/builder/builder_tests_part_01_tests.rs)
the memory-access section must not be gated on learning.enabled

Evidence, all run under the product feature set (--features "$(bash scripts/ci/product-features.sh)"):

Where Scope Result
upstream/main 2e2d0a2b8 single test passes
upstream/main 2e2d0a2b8 single test, CI's exact features passes
upstream/main 2e2d0a2b8 --no-fail-fast -p openhuman --lib -- 'openhuman::agent' FAILS2389 passed; 1 failed
#6189 branch, before its test fix same full scope FAILS2390 passed; 1 failed
#6189 branch, after its test fix same full scope passes2391 passed; 0 failed

Row 3 is main with no PR commit present at all, reproducing the CI failure exactly.

Cause. The test passed None for the agent definition, so the factory resolved "orchestrator" from AgentDefinitionRegistry's static GLOBAL: OnceLock<_> (src/openhuman/agent/harness/definition_part_02.rs:24) — first-write-wins, never reset. The prompt section is gated on memory_recall being registered and visible after tool filtering (any_tool_offered), and the visible set derives from the resolved definition's tool scope. A registry another test installed first silently dropped the section.

Why only the read side flaked. The sibling memory_write_instruction_is_present_with_learning_disabled (builder_tests_part_03_tests.rs) already passed builtin_def("orchestrator"), so it never depended on the global.

Fixed in #6189 by passing builtin_def("orchestrator") — the helper whose own doc says it loads fresh from bundled TOML, "entirely independent of the global registry singleton (so tests can't be poisoned by another test's AgentDefinitionRegistry::init_global* call, and can't poison later ones)". Not #[ignore]d, and not weakened: verified still failing when MemoryAccessSection's registration is disabled in helpers.rs.

Also reported hit today, not yet individually traced

Reported by other workers in the same session; listing them so whoever owns this can confirm each is the same shape rather than assuming:

  • budget_gate
  • modules::ops
  • 4 × composio_*_raw_coverage_e2e

Latent, same shape, not currently failing

Worth auditing rather than waiting for the ambush — every one of these globals is a first-write-wins OnceLock with no reset:

  • AgentDefinitionRegistrystatic GLOBAL (harness/definition_part_02.rs:24)
  • CoreContextstatic DEFAULT_CONTEXT (core/runtime/context.rs:27), reached by CoreContext::current()'s fallback. core/runtime/context.rs:1159 already records this hazard in a test comment: "that global is process-wide and another test in the same binary may have set it, which would make a bare assert order-dependently flaky." Capabilities resolve through it, and capability_allowed defaults open only when it is unset.
  • memory::binding::for_workspace — the process-wide HashMap<PathBuf, _> driver cache behind CoreContext::memory_capabilities()
  • agent/learning/candidate.rs:129static GLOBAL_BUFFER
  • agent/file_state/ops.rs:12static GLOBAL
  • agent/tinyagents/orchestration.rs:41static STEERING_REGISTRY

I checked the CoreContext/capability path first on the confirmed instance and it was not the cause — scoping the test to its own CoreContext::for_test(.., None, ..) did not fix it. Recording that so the next person does not repeat it.

What would actually stop this

Options, roughly in increasing cost:

  1. Run the full --lib suite somewhere on a cadence (nightly, or on pushes to main) so an order-dependent failure is attributed to the commit that introduced it instead of the next PR that widens the lane's scope. This is the one that changes the economics.
  2. A lint or review rule: a test that resolves an agent definition, a core context, or a memory binding must supply its own. builtin_def and CoreContext::for_test exist; the failures are the call sites that did not use them.
  3. Audit the globals listed above for a test-only reset, so a fixture can establish known state rather than inheriting it.

I have not implemented any of these — flagging for whoever owns the lane.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_016ipSkHi47nsBmnxX5dC4dA

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

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions