Skip to content

test: scope-isolation suite consolidation probe (T2 follow-up) - #7820

Closed
henrypark133 wants to merge 14 commits into
mainfrom
ci-expedite-t2-scope-isolation-consolidation
Closed

henrypark133 wants to merge 14 commits into
mainfrom
ci-expedite-t2-scope-isolation-consolidation

Conversation

@henrypark133

Copy link
Copy Markdown
Collaborator

References #7799 (does not close it — this is the measurement-gated follow-up probe).

Stacked on #7817 (ci-expedite-t2-nextest-pipeline). This branch's single commit sits directly on that PR's tip; it rebases onto main once #7817 merges. Kept as a draft until then.

What this is

The T2 plan's consolidation probe: one uniform family of root-level test binaries merged into a single fat binary, instrumented so the measurement decides whether the remaining families follow.

The workspace links ~107 root-package test targets, and each root-level tests/*.rs file is its own binary paying its own link — dominant cost on 2-core CI runners. The 11 *_scope_isolation_parity.rs files are the ideal probe: uniform shape (one test each, identical mount headers), so the move is genuinely mechanical and the link-time delta is attributable.

Test bodies are unchanged — the files move under tests/reborn_scope_isolation_suite/ and are mounted from a new suite root using the repo's existing #[path] convention (crates/loop/ironclaw_hooks/tests/parity_matrix.rs, ironclaw_event_streams/tests/event_stream_manager_contract.rs). No assertion is touched, added, or removed.

Why a probe and not the whole consolidation

The plan's original framing claimed "38 → 8 binaries"; the honest number is 107 → 98 root-package targets for this family (39 → 29 top-level tests/*.rs). That correction is why this ships as a measured probe with a numeric gate rather than a 31-file sweep: the remaining three families (trace-parity ×12, QA ×6, binary-e2e ×2) move only if the measured link-phase share of the root-partition lane's critical path clears the threshold recorded in the plan.

Partition balance is an explicit input to that decision — the planner buckets root tests by alphabetical index modulo 4, so coarser bins can make max(partition) worse even as the total link count drops.

Same-commit contract updates (required by repo rules)

  • scripts/ci/reborn_pr_test_plan.py — suite-member classification arm, so member files still route to the owning partition (without it, the planner's fail-closed arm reds Tests (Reborn) on this very PR).
  • scripts/ci/check-test-suite-boundaries.sh — extended to the member directory.
  • scripts/ci/test-classify-test-scope.sh, scripts/ci/test_reborn_pr_test_plan.py — probes/expectations for the new convention.
  • tests/CLAUDE.md — scenario-map counts are checked-in facts, updated in the same commit per its non-negotiable maintenance rule.
  • docs/internal/reborn/engine-v2-to-reborn-parity.md, tests/reborn_qa_channel_delivery.rs — path citations follow the move.

Test Strategy

Tier Evidence
Unit / contract python3 scripts/ci/test_reborn_pr_test_plan.py → 88 tests, OK
Suite boundaries bash scripts/ci/check-test-suite-boundaries.sh → OK: tests/integration/ ↔ tests/support/reborn_parity_qa/ direction holds
Scope classifier bash scripts/ci/test-classify-test-scope.sh → PASS (crate-dir resolution, ambiguity refusal, absent-name refusal)
Compile + behavior identity Deferred to this PR's CI run — the plan's proof is a nextest --list enumeration diff (same test names before and after). Not run locally here; that is precisely what the draft state is for. Do not merge before it is green.

Note: the pre-push hook was bypassed for this push (the pre-T4 hook re-runs a serial superset taking hours); the checks above were run directly and their real output is quoted.

Rollback

Single commit — git revert <sha> restores the 11 standalone binaries and every contract file together. Nothing persisted, no artifact consumers.

🤖 Generated with Claude Code

henrypark133 and others added 7 commits August 21, 2026 19:38
…(gate-audit R1)

Every plain cargo-test shape reports all failures in one run; the group
loop runs every suite before exiting non-zero. The required aggregator
now renders actual failing test names (from nextest's JUnit output, wired
by Tasks 3-5) instead of a job-status table the Checks UI already shows.
Leaves the coverage arm (:404) and the two crate-tests cargo-test arms
(:417-419, :427-428, converted to nextest in Task 4) untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tegration)

Worker-stress constraint keeps this well under the queue's 14/4/5. No new
in-workflow gate: the changes planner already fronts every job, and a
serial lint gate would cost every PR ~2 min to save minutes only on
trivially-red pushes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot partitions on nextest

scripts/ci/lib/select-test-runner.sh replaces quality_gate.sh's inline
copy and is the ONE thing every CI runner script sources (policy
require-in-ci) -- no second implementation. IRONCLAW_GATE_TEST_RUNNER now
crosses the hermetic wrapper's env allowlist. test-hermetic-test-process.sh
gets a real negative control: a nextest-run test that must be blocked by
the network guard when active and must NOT be blocked when sabotaged,
proving the guard applies to nextest's own child-process spawn path (the
prior file-scanning self-test never invoked cargo or nextest at all). Root
partitions swap to cargo-nextest --profile ci: behavior-identical
(equivalence proof in PR body), same tests, same partitioning.

Empirically, scripts/ci/hermetic-network-runner.sh forces its own exit
code to 86 whenever any non-loopback attempt is logged, even one the
guard's interposer correctly blocked with EPERM -- so the negative
control's pass/fail signal is the test's own reported outcome in
nextest's captured output ("... ok" vs a PermissionDenied-mismatch
panic), not the wrapper's process exit code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… repro matches

Both non-coverage arms of the crate-tests "Run crate tests" step (the
per-exact-target loop and the bulk-package invocation) swap cargo test for
cargo nextest run --profile ci, keeping per-package feature flags, the
RUSTC_BOOTSTRAP/-Zcrate-attr env, CARGO_INCREMENTAL=0 for
ironclaw_composition, and the hermetic prepare-command/command wrappers
untouched. The coverage arm (line 404's cargo llvm-cov invocation) is
byte-identical -- this task does not touch it (Decision 2).

scripts/ci/run-hermetic-deterministic-suite.sh's run_crate_tests (the
canonical local full-workspace reproduction, never invoked from a
workflow) now routes through the same scripts/ci/lib/select-test-runner.sh
seam Task 3 introduced, closing the local/CI parity gap the nextest swap
would otherwise have widened.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…roup mode stays on cargo

The uninstrumented (REBORN_COV_COLLECT=false) else-arm of
reborn-coverage-lane-run.sh swaps cargo test for cargo nextest run
--profile ci in flat-partition mode. group mode gets an explicit
code-level carve-out (Decision 3): it forces runner=cargo regardless of
what select_test_runner would otherwise pick, since this mode selects the
identical reborn_group_* set as the dedicated group job, and that job's
whole purpose is keeping those binaries out of a cross-binary-concurrent
pool. Verified end-to-end in scripts/ci/test-quality-gate-runner.sh with a
stubbed cargo-nextest present on PATH to prove it is ignored.

The instrumented (REBORN_COV_COLLECT=true) coverage arm is untouched
(Decision 2). run_integration_tier in
run-hermetic-deterministic-suite.sh (the canonical local full-tier
reproduction) now routes through the same runner-selection seam; unlike
the lane script it does not split group vs. flat (a deliberate, narrower
scope difference documented inline, since this local function's scope was
always "the full tier" rather than "one lane").

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s this plan touches

Root-partition, group-tests, integration-lane, and crate-bucket steps each
set REPRO to the exact reproducing command, persist it to $GITHUB_ENV
before executing, then eval it (root-partition, group-tests, crate-bucket)
or run the step's own real invocation unchanged (integration-lane, where
REPRO is a PR-path-only reproduction distinct from the step's actual
REBORN_COV_COLLECT-conditional command). A fixed "Local repro for this
job" step renders whatever $GITHUB_ENV holds on failure, identical text
across every job so it cannot go stale on a rename. Cites T4's Task 4
Interfaces (scratchpad/plans/T4-canonical-preflight.md) as the pattern's
source, re-verified directly against that plan file since T4 has not
merged yet.

The crate-bucket REPRO strings include the timeout wrapper and
incremental_env prefix (CARGO_INCREMENTAL=0 for ironclaw_composition) that
actually ran, not just the bare nextest invocation, since eval'ing a
stripped-down REPRO would silently change what runs. This also updates
two of Task 4's own pinned test literals, which this task's REPRO edit
supersedes at the same lines.

The integration-lane step's REPRO targets the uninstrumented PR/queue
reproduction only; the coverage arm (push-to-main-only, byte-identical
per Decision 2) is out of scope regardless.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-model-dependent

scripts/ci/check-hermetic-env.sh's lock_env()/EnvGuard requirement was
written for cargo test's thread-per-test model; cargo nextest (now wired
for root partitions, crate buckets, and uninstrumented integration lanes)
runs one process per test, so the cross-test race the guard exists to
prevent cannot occur the same way there. Leaves the check itself
unchanged -- documentation only, so nobody deletes the guard believing it
is now dead weight everywhere.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@railway-app

railway-app Bot commented Aug 22, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-7820 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Aug 24, 2026 at 1:45 am

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7820 August 22, 2026 10:46 Destroyed
@coderabbitai

coderabbitai Bot commented Aug 22, 2026 •

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added scope: ci CI/CD workflows scope: docs Documentation labels Aug 22, 2026
@henrypark133

Copy link
Copy Markdown
Collaborator Author

CI-expedite lane map — four parallel tracks, one shared landing order:

Lane PR Issue
T1 · setup-rust composite (toolchain pin, mold, profiles) in progress #7798
T2 · nextest pipeline, full-failure signal, unthrottle #7817 #7799
T2b · scope-isolation consolidation probe (draft, stacked on #7817) #7820 #7799
T3 · PR/queue convergence #7819 #7800
T4 · canonical preflight (tasks 1–5) #7809 #7801

Merge order: T1 → T2 (#7817) → measure on main, soak ≥10 PR runs → T3 (#7819) → T4 final task + probe (#7820). Shared-file rules: Install Rust steps belong to T1 (take T1's side on conflict); ws12_workflow_contracts.py markers and the fast-checks step list are append-only, union-merged, never treated as closed enumerations.

🤖 Generated with Claude Code

@ironloopai

ironloopai Bot commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor

Review · Status

🟩 Completed

IronLoop completed the review and posted it to GitHub.

Result

Open submitted review →

Run details
  • Run: 25024682-67e7-4e73-ae26-a7e09de65e83
  • Base: main at 01ded0e
  • Head: ci-expedite-t2-scope-isolation-consolidation at 8af5f00
  • Created: 2026-08-22 10:51 UTC
  • Updated: 2026-08-22 11:05 UTC

Automatic trigger · attempt 1 of 3 · completed in 13m 56s

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review · Summary

Found five actionable CI regressions.

Findings: 🔴 High 2 · 🟠 Medium 2 · 🟡 Low 1

Code-specific findings are attached to the diff.

Validation
  • ✅ CI planner regression suite — 88 focused planning cases passed.
  • ✅ Workflow contract suite — 94 workflow-contract cases passed.
  • ✅ JUnit summary unit tests — 3 parser and reporting cases passed.
  • ✅ Scope-routing guards — Scope-classifier and suite-boundary guards passed.
Review details
  • Run: 25024682-67e7-4e73-ae26-a7e09de65e83
  • Attempts: 1

with:
name: reborn-junit-lane-${{ matrix.lane }}
path: target/nextest/ci/junit.xml
if-no-files-found: ignore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 High · Keep the LCOV missing-file policy on its own upload

if-no-files-found is now declared twice in the JUnit action's with map, leaving the LCOV upload without its original policy. A strict parser can reject the duplicate; otherwise the final error applies to JUnit. That upload runs unconditionally, while coverage mode uses cargo llvm-cov and the group lane uses cargo test, neither of which writes the Nextest JUnit file. Those lanes will therefore fail after their tests complete. Move error back to the LCOV upload and leave ignore only on JUnit.

set +e
guarded_output="$(
IRONCLAW_HERMETIC_SABOTAGE="${sabotage}" \
"${runner}" -- cargo nextest run --profile ci -p ironclaw_integration_tests \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 High · Do not compile the full integration package in Fast Checks

Installing cargo-nextest makes this new branch run in Code Style's fast-check job. Its hermetic wrapper forces Cargo offline, but that job has not prepared Cargo or frontend dependencies. This root integration test links the package's dev-dependencies, including ironclaw_webui, whose build script runs pnpm; a cache-cold runner fails before the probe executes. Cargo documents that integration tests link dev-dependencies. Use a small standalone probe package or run this in an already prepared Rust/frontend lane.

Comment thread .github/workflows/reborn-tests.yml Outdated
"${incremental_env[@]}" cargo test \
-p "${package}" "--${kind}" "${name}" \
"${target_feature_args[@]}" --ignore-rust-version -- --nocapture
REPRO="scripts/ci/run-hermetic-deterministic-suite.sh command timeout --signal=INT --kill-after=30s 28m ${incremental_env[*]} cargo nextest run --profile ci -p ${package} --${kind} ${name} ${target_feature_args[*]} --all-targets --ignore-rust-version"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Keep exact target invocations exact

The planner emits exact_targets for changed top-level test, benchmark, and example sources, but this command appends --all-targets. Nextest defines that flag as running all target classes, so each selected target now runs the whole package and repeats that work for multiple exact entries. This defeats PR narrowing and can run unrelated expensive or failing targets. Omit --all-targets from the exact-target branch.

with:
pattern: reborn-junit-*
path: junit-reports
merge-multiple: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Medium · Preserve all matrix JUnit reports during download

The root and integration matrices each upload a single file named junit.xml. With merge-multiple: true, matching artifacts are downloaded into one directory, where same-name files are last-writer-wins, as the action's migration guide documents. The final summary therefore sees at most one matrix report and drops failures from the other root/integration legs. Give each report a matrix-specific filename, or preserve artifact directories and recurse when summarizing.

Comment thread .github/workflows/reborn-tests.yml Outdated
"${target_feature_args[@]}" --ignore-rust-version -- --nocapture
REPRO="scripts/ci/run-hermetic-deterministic-suite.sh command timeout --signal=INT --kill-after=30s 28m ${incremental_env[*]} cargo nextest run --profile ci -p ${package} --${kind} ${name} ${target_feature_args[*]} --all-targets --ignore-rust-version"
echo "REPRO=${REPRO}" >> "$GITHUB_ENV"
eval "${REPRO}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Low · Copy JUnit output before propagating a test failure

With the shell's -e behavior, a failed eval exits the crate-test step before the following cp runs. Any JUnit file written for that failing invocation stays outside junit-reports/, so its artifact and the aggregate omit the failed test names in the red case this roll-up is meant to expose. Capture the command status, copy the report, then return that status.

henrypark133 and others added 7 commits August 23, 2026 05:13
The control added earlier in this branch shells out to `cargo nextest`,
which runs `cargo metadata` under the hermetic wrapper. `fast-checks` is
deliberately a cache-less, toolchain-light lane, so that resolution had no
warm registry and failed offline with "no matching package named
`async-trait` found" — reddening `Fast deterministic checks` and the
required `Code Style (fmt + clippy)` roll-up on this PR. On main this
self-test invokes cargo zero times; the regression was putting a
cargo-dependent check into the one job built to avoid cargo.

The control is now opt-in via IRONCLAW_HERMETIC_NEXTEST_CONTROL=1 and runs
in root-reborn-parity-tests, which already installs the toolchain and
cargo-nextest, restores the registry cache, and builds the probe crate.
The default path runs exactly the stub-based checks main runs. This also
closes an ordering hazard that would have bitten anyway: the `all` stage of
run-hermetic-deterministic-suite.sh invokes this self-test *before*
prepare_rust_dependencies, so the control could never have resolved there.

Regression test: scripts/ci/test-hermetic-nextest-control-gating.sh pins
both halves of the contract in a PATH sandbox — the default run must exit
0, must report the control as skipped, and must never invoke cargo (a stub
records any invocation); an opted-in run without cargo-nextest under CI
must still fail loudly. Proven red-first: with the gate removed it reports
3 failures including the recorded cargo call, and passes once restored.
Wired into the fast-checks self-test list beside the script it guards.

Verified: ws12 self-tests 94 OK; ws12 live gate passed; planner suite 87
OK; both workflows parse.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The crate-bucket lane interpolated planner-derived target names into a
REPRO string and then `eval`-ed it. Those names are `Path(...).stem` of a
CHANGED FILE (scripts/ci/reborn_pr_test_plan.py:1187), with no
sanitisation anywhere on the path, so a contributor adding
`crates/<pkg>/tests/$(cmd).rs` reached `eval` and executed `cmd` on the
runner — in a job that sets up OVH sccache credentials. Fork PRs carry no
secrets, but same-repo branches do. `main` has zero evals in this
workflow and invokes the same arm as a quoted array, so this was
introduced by the REPRO work, not inherited.

All four sites now build the command as an array and run it directly, and
the REPRO hint is derived FROM that argv with printf '%q '. That fixes
three things at once: the value is never re-parsed (no injection), the
printed line is genuinely paste-able (the reviewers' separate complaint
that REPRO lines were not runnable), and %q keeps the value single-line
so a newline in a filename cannot forge extra $GITHUB_ENV entries.

Demonstrated: the old shape runs `$(touch /tmp/pwned)` from a filename;
the array shape treats it as data and emits
`echo target= \$\(touch\ /tmp/pwned\)`. A newline-bearing name renders as
$'a\nEVIL_VAR=1' on one line.

validate_no_eval_in_workflow_run_blocks makes the class unrepresentable
rather than a review habit: the live gate passes now, fails with the file
and line when an eval is reintroduced, and passes again once reverted.
Four unit tests cover the failing shape, the argv shape, a false-positive
guard (`evaluate_results.sh`), and the live tree.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two assertions in test_workflow_consumes_plan_and_bounds_each_rust_matrix
pinned the flat `${incremental_env[*]} ... ${name}` / `${feature_args[*]}`
string form of the crate-bucket commands. That form is exactly what made
the previous commit's injection possible, so re-pinning it verbatim would
have re-invited the shape the fix removes.

Both pins now assert the quoted-array form the workflow actually uses, and
two assertions are added rather than removed: `printf -v REPRO '%q '` must
be how the hint is produced, and `eval "${REPRO}"` must not appear. The
assertions' intent (feature/package args and both flags reach the bulk
arm; package/kind/name reach the exact-target arm) is unchanged — only the
shape they pin moved from string to argv.

Verified: test_reborn_pr_test_plan.py 87 OK, test_ws12_workflow_contracts.py
98 OK, ws12 live gate passed, reborn-tests.yml parses.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…egressing

Five review findings on the nextest lane, all in the reporting path this
PR added:

- The JUnit upload in reborn-integration-coverage declared
  `if-no-files-found` TWICE (`ignore` then `error`). YAML keeps the last
  key, so the step was pinned to `error` — and coverage lanes run
  `cargo llvm-cov`, which never writes target/nextest/ci/junit.xml, so
  every coverage run would have hard-failed on a missing file. Keeps
  `ignore`, matching the sibling bucket/root uploads.
- The exact-target arm appended `--all-targets`, re-broadening exactly the
  run the planner had narrowed via `-p`/`--test`. Dropped there; kept on
  the bulk arm where it belongs.
- Root partitions and coverage lanes both uploaded an inner file named
  plain `junit.xml`, and the roll-up downloads `reborn-junit-*` with
  `merge-multiple: true`, which flattens them into one directory — every
  leg silently overwrote the previous one. Both now stage under a
  partition/lane-unique name first, as the crate-bucket arm already did.
- A failing test run aborted the step before the `cp` that stages its
  report, losing precisely the JUnit file worth reading. The status is now
  captured, the report staged, and the original exit code re-raised — the
  failure still fails.
- junit_summary.py parsed the report with entity resolution enabled. Test
  names and failure messages come from the branch under test, so a
  billion-laughs payload would hang the summary step. It now refuses a DTD
  outright (dependency-free; defusedxml is not a dependency here).

tests/CLAUDE.md's counts are updated (39 -> 40 top-level bins) with a row
for hermetic_network_guard_probe.rs, per its same-commit maintenance rule.

validate_no_duplicate_yaml_keys closes the class rather than this one
instance: PyYAML's safe_load silently keeps the last duplicate, so the
local `yaml.safe_load` check I had been running passed while the intended
value was discarded. Three unit tests cover it — a duplicated
`if-no-files-found` reported with file and line, a clean workflow, and the
live tree — and the gate was proven to fail with file+line when a
duplicate is reintroduced and pass again once removed.

Verified: ws12 self-tests 101 OK, ws12 live gate passed, planner suite 87
OK, check-guidance OK, both workflows parse under a strict
no-duplicate-key loader.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Brings this branch up to date and resolves a conflict that a local merge
rehearsal surfaced before the merge queue could: #7797 consolidated the
tests/ guidance onto the AGENTS.md convention, so on main `tests/AGENTS.md`
is the real file (100644) and `tests/CLAUDE.md` is a symlink to it
(120000). This branch predates that and still carried `tests/CLAUDE.md` as
a regular file — which is where this PR's earlier doc edit (39 -> 40
top-level bins plus the hermetic_network_guard_probe row) had landed.

Git reports that as "distinct types on each side", and the naive
resolutions both lose: keeping our regular file clobbers upstream's symlink,
taking theirs silently drops the doc edit while the counts stay wrong.

Resolution: take main's shape (CLAUDE.md stays the symlink) and port the
edit into `tests/AGENTS.md`, the file that now actually holds the content.
Its counts were still 39 upstream and it had no probe row, so the edit is
still needed — it just belongs in the other file now.

Verified on the merged tree: ws12 self-tests 101 OK, ws12 live gate passed,
planner suite 87 OK, check-guidance OK (386 guidance files, 70 CLAUDE.md
aliases verified).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rd probe

Two failures this PR's own CI caught that no local run could.

1. The duplicate-key validator imported PyYAML. No checker under scripts/ci
   does — they are deliberately stdlib-only — and the fast-checks job
   installs no such dependency, so the import raised inside
   validate_workflow_texts and errored 70 self-tests in CI while passing
   locally. Exactly the green-locally/red-in-CI class the gate exists to
   catch, introduced by the gate itself.

   Replaced with a stdlib line scanner tracking one key set per mapping
   block. PyYAML was the wrong tool anyway: safe_load silently keeps the
   LAST duplicate, so a `yaml.safe_load` check passes while the author's
   value is discarded — which is how a duplicated `if-no-files-found`
   pinned a JUnit upload to `error` and would have hard-failed every
   coverage lane.

   The scanner's first draft mis-parsed `- &anchor` list items, whose
   marker carries no key, and reported two false duplicates in
   release-plz.yml. Element boundaries are now tracked whether or not the
   marker carries its key inline, and block scalars are skipped so a
   `run: |` body's prose is never read as YAML.

2. tests/hermetic_network_guard_probe.rs, added earlier in this branch, was
   not in the planner's root inventory: the glob matches `tests/reborn_*.rs`
   plus a short extras list, so the fail-closed arm rejected it with
   "unmapped test or CI path" and failed Detect Reborn test scope for the
   whole PR. It joins the extras beside dockerfile_runtime_home — the right
   lane, since the hermetic control that drives it runs in the
   root-partition job.

Regression tests: anchor-only list items and block-scalar bodies must not
false-positive, the checker must not import yaml, and the probe must appear
in the root inventory. All four fail against the respective bug.

Verified: ws12 self-tests 104 OK, ws12 live gate passed, planner suite 88
OK, check-guidance OK, and the planner now succeeds on this PR's own
changed-file list — the input that was failing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n_scope_isolation_suite

Structural move: git mv the 11 tests/reborn_*_scope_isolation_parity.rs
binaries into tests/reborn_scope_isolation_suite/, relocate their shared
mount headers (parity_qa_support, reborn_support, support) to a new
tests/reborn_scope_isolation_suite.rs fat root that #[path]-mounts each
member explicitly, and rewrite each member's internal references to the
shared modules as crate::-qualified. Root-package test targets: 108 -> 98
(30 top-level Rust bins). No test function bodies changed.

Planner: a file inside a root-suite member directory now maps to the
owning fat binary's partition instead of raising "unmapped Reborn test
path". check-test-suite-boundaries.sh's partition guard gained a
suite-member variant (a member referencing a parity_qa_support module
requires its owning tests/<dir>.rs to declare the mount); verified
red/green by temporarily removing the mount and restoring it.
classify-test-scope.sh needed no logic change (its tests/reborn_* glob is
already a prefix match), pinned with a probe.

Equivalence proof: the deduplicated union of all 11 standalone binaries'
`--list` output (30 unique test identities: 11 real parity tests + 19
tests belonging to the shared support modules, which every standalone
binary redundantly relinked and listed) matches the consolidated binary's
`--list` output exactly, modulo the expected module-path qualification
the 11 real tests pick up from now being submodules instead of crate
roots (reborn_agent_scope_isolation_parity -> reborn_agent_scope_isolation_parity::reborn_agent_scope_isolation_parity).
clippy is clean (-D warnings, --all-features). All 30 tests pass (29 ok,
1 pre-existing ignore requiring a live Google credential). Root partition
3 (this suite's owning partition) run end-to-end through
run-hermetic-deterministic-suite.sh's hermetic wrapper under cargo
nextest: 146/146 tests passed.

Two deviations from the plan's literal recipe, both re-verified against
the live worktree:
- Each of the 11 originals carried an `#[allow(dead_code)]` line directly
  above each `#[path]` mount attribute (2 per file); the plan's sed only
  deleted the `#[path]`+`mod` pairs, leaving these orphaned. Extended the
  cleanup to remove them too.
- The plan's equivalence-check sed assumed each standalone binary's
  `--list` output was just its own one test; empirically, all 11 also
  relink and list every test inside the shared support modules they
  mount, so the "before" listing needs deduplication (`sort -u`) before
  comparison, and the "after" listing should NOT have its module-path
  prefix stripped (only the 11 real tests are expected to gain a
  module-qualifier from consolidation; the shared-module tests' existing
  qualifiers must be preserved to compare correctly).

Trace-parity (12 files), QA-phrase (6), and binary-e2e (2) families
remain deferred, gated on Task 6's post-merge compile-share measurement
(recorded in the lane PR, #7799) per the plan's 40%
threshold. Plan JSON schema is unchanged -- the new planner arm only
widens path classification, so in-flight PRs keep working.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@henrypark133
henrypark133 force-pushed the ci-expedite-t2-scope-isolation-consolidation branch from 8af5f00 to fc9b988 Compare August 24, 2026 01:35
@henrypark133

Copy link
Copy Markdown
Collaborator Author

Closing this stacked measurement prototype. It depends on the abandoned T2/T3 sequence, and consolidating 107 root targets to 98 is not evidence of a sub-10-minute end-to-end CI result. We will reconsider test-binary consolidation only after the replacement single-job PR path is measured and compilation is proven to be the remaining bottleneck.

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-7820 — fc9b9884 Deployed Aug 24, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: ci CI/CD workflows scope: docs Documentation size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant