Skip to content

ci: absent prerequisites SKIP with a named reason, and skipping is bounded - #3

Merged
stranske merged 1 commit into
mainfrom
claude/objective-dubinsky-3265d6
Aug 22, 2026
Merged

ci: absent prerequisites SKIP with a named reason, and skipping is bounded#3
stranske merged 1 commit into
mainfrom
claude/objective-dubinsky-3265d6

Conversation

@stranske

Copy link
Copy Markdown
Owner

The first CI run of the public repo came back red: 21 pytest failures, 8 module selftests, 2 capability gates — against 330 green locally. Not one was a defect in this tree.

What was actually wrong

Nothing, in the code. Every failing check needed something that exists only on the machine the system runs on. I reproduced the runner exactly — committed files only (git archive), empty HOME, no agent CLIs, no sibling checkouts, unauthenticated gh, fresh state — and got 305 passed / 21 failed / 4 skipped / the same 8 selftests / the same 2 gates, byte-for-byte with CI. That made the diagnosis measurement rather than guesswork:

Prerequisite Failures
the populated capability ledger 8 tests, 5 selftests, both gates
~/.codex/skills/.../audit_code_root.sh 8 tests
the version-capable Codex binary for exact profiles 2 tests, 2 selftests
an installed agent CLI / credential file for a seat 2 tests
(not a prerequisite — a stub leak) 1 test, 1 selftest

The ledger is the big one and worth stating plainly: it is machine-local state by design. A fresh bootstrap holds the 14 rows the code declares; this instance's holds 40. The other 26 are accumulated registration history and cannot be rebuilt from source.

The fix

Each genuinely-absent prerequisite now skips with a reason naming the missing thing, via detectors in the new env_prereq.py. Detection is of the prerequisite — never of $CI — so the same code is right on a runner, on the owner's box, and on a second instance with a different ORCH_STATE_DIR.

Assertions are untouched. A check that runs asserts exactly what it asserted before; only its applicability gate is new. Selftests gate the smallest section that needs the missing thing rather than the whole function, so CI keeps everything else — feedback.py's selftest still runs its several hundred other assertions.

One failure was not a prerequisite and is fixed by isolation, not a skip. test_experiment_arm_identity and exp_abcd's selftest replace subprocess.Popen wholesale, and model resolution spawns a catalog probe whenever the advertised-model cache is cold — which lands in the stub and dies on stdout.write with an int. Warm cache locally, cold on a fresh machine. Using adapters' own ORCH_MODEL_PROBE kill-switch makes both hermetic, so CI now runs more, not less.

The skip ceiling — the reason skipping was allowed at all

A suite that looks green while running less is the failure this repo exists to prevent, so verify.py now bounds skipping instead of permitting it:

  • ceilings on skipped tests / selftests / gates in .verify-floor.json (24 / 7 / 2 — exactly what a machine with none of the local prerequisites skips). One more than agreed is a red that names the key to raise.
  • the floor is now passed + skipped, so a check may move between passing and consciously-skipped but the two together may never shrink. --update-floor records passed + skipped and never re-measures the ceilings, so the floor cannot ratchet down behind a leak.
  • a selftest or gate that exits 0 having skipped is reported as SKIPPED, not ok — the silent-zero-exit rule's twin, and the reason the 74 of 81 number can be trusted.
  • every skip and its reason is printed, so green always states what did not run.
  • every pytest failure is printed, not the last 12 lines. The first run reported 21 failures and named 7, which cost a whole round trip.

Deliberate break → revert (all three reverted byte-identical, cmp-confirmed)

  1. One extra skipped file33 > 24 fails, naming skipped_max. With the ceiling removed, the identical break exits 0 and prints "VERIFIED". The ceiling is load-bearing, not decoration.
  2. Blinding run_selftests to the skip mark → claims "81 of 81 modules ran" while 7 ran nothing; verify.py --selftest catches it.
  3. Floor back to bare passed → CI goes red for being honest.

Two real bugs the reproduction surfaced

  • capability_activation_audit._fleet_label_index took the whole audit down on a missing gh binary, while a failed gh call was already handled as "unknown for this repo". Same information, opposite outcome.
  • ci.yml claimed the suite was state-independent, "verified by running it with ORCH_STATE_DIR pointed at an empty directory — 330 passed". That moved the wrong knob: the ledger and Brain live behind ORCH_LOCAL_RUNTIME. The experiment left the populated 40-row ledger in place and proved nothing about a fresh machine. Both are set now, and CLAUDE.md records the distinction.

Verification

tests selftests gates
locally 330 passed, 0 failed, 0 of 24 skipped 81 of 81 ran 5 of 5 green
runner reproduction 306 passed, 0 failed, 24 of 24 skipped 74 of 81 ran 3 of 5 green, exit 0

Also re-run from the mirror layout via orch-sync-mirror.shcmp-clean is not agreement — same verdict. The local run is not weakened: it still executes all 330 and skips nothing.

Note for whoever merges. The live exec mirror was deliberately not synced: the canonical tree is on codex/partitioned-review-reconciliation with another session's uncommitted edits to two files this PR also touches (different regions of capability_activation_audit.py and test_model_tier_resolution.py — they should merge cleanly). Per CLAUDE.md §1 that mirror gap is the manual circuit breaker between an agent's change and the dispatcher. Sync it from the canonical tree after merge. That other branch also appends a test, so the collection floor will need to go 330 → 331 when both land.

🤖 Generated with Claude Code

…unded

The first CI run of the public repo was red: 21 pytest failures, 8 module
selftests and 2 capability gates, against 330 green locally. None was a defect
in this tree. Every one needed something that exists only on the machine the
system runs on, and a GitHub runner has none of it.

Four prerequisite classes, established by reproducing the runner exactly
(committed files only via `git archive`, empty HOME, no agent CLIs, no sibling
checkouts, unauthenticated gh, fresh state) — 305 passed / 21 failed / 4 skipped
/ same 8 selftests / same 2 gates, byte-for-byte with CI:

  * the populated capability LEDGER (8 tests, 5 selftests, both gates). The
    ledger is machine-local state; a fresh bootstrap holds the 14 rows the code
    declares, this instance's holds 40. The other 26 are registration history and
    are not reconstructible from source.
  * `~/.codex/skills/.../audit_code_root.sh` (8 tests) — the skill compiler is
    deliberately exercised against a real installed skill, not a fixture.
  * the version-capable Codex binary for exact profiles (2 tests, 2 selftests) —
    default path is inside a macOS app bundle.
  * an installed agent CLI / credential file for a seat (2 tests).

Each of those now SKIPS with a reason NAMING the missing thing, via detectors in
the new `env_prereq.py`. Detection is of the prerequisite, never of `$CI`, so the
same code is right on a runner, here, and on a second instance. Assertions are
untouched: a check that runs asserts exactly what it asserted before. Selftests
gate the smallest SECTION that needs the missing thing rather than the whole
function, so CI keeps everything else.

One failure was NOT a prerequisite. `test_experiment_arm_identity` and
`exp_abcd`'s selftest replace `subprocess.Popen` wholesale, and model resolution
spawns a catalog probe when the advertised-model cache is cold — which lands in
the stub and dies on `stdout.write` with an int. Warm cache here, cold on a fresh
machine. Fixed by isolation with adapters' own `ORCH_MODEL_PROBE` kill-switch, so
CI now runs MORE, not less.

THE SKIP CEILING is the reason skipping was allowed at all. verify.py now:
  * bounds skipped tests / selftests / gates against `.verify-floor.json`
    (24/7/2 — exactly what a machine with none of the local prerequisites skips).
    One more than agreed is a RED naming the key to raise.
  * counts the floor as `passed + skipped`, so a check may move between passing
    and consciously-skipped but the two together may never shrink. `--update-floor`
    records `passed + skipped` and never re-measures the ceilings, so the floor
    cannot ratchet down behind a leak.
  * treats a selftest or gate that exits 0 having skipped as SKIPPED, not ok —
    the silent-zero-exit rule's twin, and the reason `ok` can be trusted again.
  * prints every skip and its reason, so green always states what did not run.
  * prints EVERY pytest failure, not the last 12 lines. The first CI run reported
    21 failures and named 7, which cost a whole round trip to diagnose.

Deliberate break -> revert, all three reverted byte-identical (`cmp`):
  * one extra skipped file: 33 > 24 fails, naming `skipped_max`. With the ceiling
    removed the IDENTICAL break exits 0 and reads "VERIFIED" — the ceiling is
    load-bearing, not decoration.
  * blinding `run_selftests` to the skip mark: claims "81 of 81 modules ran" while
    7 ran nothing; `verify.py --selftest` catches it.
  * floor back to bare `passed`: CI goes red for being honest.

Also fixed, both found by the reproduction:
  * `capability_activation_audit._fleet_label_index` crashed the whole audit on a
    missing `gh` binary, while a FAILED gh call was already handled as "unknown".
  * ci.yml claimed state-independence "verified with ORCH_STATE_DIR pointed at an
    empty directory". That moved the wrong knob: the ledger and Brain live behind
    ORCH_LOCAL_RUNTIME, so the experiment left the populated 40-row ledger in
    place and proved nothing. Both are set now.
  * `capability_admission`'s commitment-detector probe depended on the real audit
    ledger (gitignored) and on which records it happened to contain. It now runs
    against a synthetic empty record set — deterministic everywhere.

DEDUP FINDING (CLAUDE.md 0), recorded before writing code and kept in
env_prereq.py's docstring: grepped for the concept, not the name. `pytest.skip`
appears in exactly one file (test_model_tier_resolution.py, twice, both
`shutil.which`-gated — the idiom this generalises); no conftest.py, no pytest.ini,
no pyproject.toml, no shared applicability helper. Nearest relatives are
single-call-site degradations, not reusable machinery: capability_admission.py:335
("cannot judge without the ledger") and _fleet_label_index skipping a failed repo.
Nothing to wire, activate or un-gate. env_prereq.py is test-applicability
infrastructure, not an orchestrator capability — no dispatch path, no outcome, no
ledger row — so the admission gate does not bind on it.

Verified: 330 passed / 0 failed / 0 of 24 skipped / 81 of 81 selftests / 5 of 5
gates locally AND re-run from the mirror layout (cmp-clean is not agreement).
In the runner reproduction: 306 passed / 0 failed / 24 of 24 skipped / 74 of 81
selftests / 3 of 5 gates, exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in: 22 minutes

Limit details: You’ve used the included review currently available. Your 64 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7af3a077-e103-4145-baec-e07e8e2057b3

📥 Commits

Reviewing files that changed from the base of the PR and between 73eb96f and 285eb86.

📒 Files selected for processing (24)
  • .github/workflows/ci.yml
  • .verify-floor.json
  • CLAUDE.md
  • README.md
  • adapters.py
  • capability_activation_audit.py
  • capability_admission.py
  • capability_advisor.py
  • capability_outcome_bridge.py
  • capability_recurrence_check.py
  • env_prereq.py
  • exp_abcd.py
  • feedback.py
  • range_lane_rollout.py
  • test_capabilities.py
  • test_capability_admission.py
  • test_capability_lifecycle_e2e.py
  • test_capability_set_coverage.py
  • test_capacity_profiles.py
  • test_experiment_arm_identity.py
  • test_feedback_model_provenance.py
  • test_model_tier_resolution.py
  • test_skill_compiler.py
  • verify.py

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

@stranske
stranske merged commit 10b5ccf into main Aug 22, 2026
2 checks passed
@stranske
stranske deleted the claude/objective-dubinsky-3265d6 branch August 22, 2026 02:29
stranske added a commit that referenced this pull request Aug 24, 2026
…wanted, plus .json (#110)

Maint Coverage Guard failed the moment it was unheld, on both runs (#3 scheduled,
#4 manual), with:

  error: Coverage verification could not find required coverage artifacts on a
         successful Gate run.
  warn:  Unable to locate a recent successful Gate workflow run with required
         coverage artifacts: gate-coverage-trend, gate-coverage-trend-history,
         gate-coverage.

maint-coverage-guard.yml downloads exactly three artifact NAMES. The Gate
produced the first two under those names and the third as "gate-coverage.json",
so the guard probed ten successful runs, found two of three every time, and
reported no usable run at all.

Measured rather than inferred: Gate run #241 (main, success) uploaded
gate-coverage-summary.md, gate-coverage.json, gate-summary.md,
gate-coverage-3.13-1, gate-coverage-trend-history, gate-coverage-trend,
gate-coverage-summary, gate-coverage-3.12-1 -- trend and trend-history present,
"gate-coverage" absent.

An artifact NAME and the FILE inside it are different things, and this step
conflated them. Renaming the artifact is safe and was checked, not assumed:
"gate-coverage.json" as an artifact name appears nowhere else in this repo, and
the only download-artifact steps in any workflow here are the guard's three. The
file inside is still gate-coverage.json, so anything reading contents is
unaffected.

A paired literal in two files that disagreed -- this repo's signature defect --
and it stayed invisible for as long as the guard was held. That is the second
thing the holds were concealing, after PR #90's six F821s.

pr-00-gate.yml is create_only in the Workflows sync manifest with Orchestrator
absent from overwrite_repos, so this local fix persists. The same mismatch
likely affects every consumer whose Gate emits coverage, which is worth raising
upstream separately.

448 collected, ruff and black clean, test_ci_gate_config 12 passed.

Co-authored-by: Tim Stranske <tim@stranskemo.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant