Conversation
…an run The nightly workflow's two guards that sit on the acceptance criteria have been blind, not merely red. In run 33247186128 the golden-run replay regression hit `ImportError: libEGL.so.1` during collection, reported `36 errors during collection`, and executed none of its nodes; the short mock-stack soak never brought the stack up, recording `still missing: roles, handshake, bridge, bridge_guard`, so it measured no memory and no lag. Five consecutive nightly runs across three commits, the oldest 2026-08-25, were red this way. Whether nightly ever passed was not measured and is not claimed here. main.yml installs libegl1, libgl1, libxkbcommon0 and libdbus-1-3 and stayed green throughout, so the defect was divergence between the two workflows. Its step is copied verbatim, comments included, into the two nightly jobs that import PySide6 through the launcher. The bounded apt flow and the prohibition on calling `exit` from that login shell are load-bearing and are why it is copied rather than rewritten. `bounded mock soak (B-phase-exit)` is deliberately left alone: it passed in the same run without Qt, so installing libraries there would prevent nothing. The new guard binds both halves. One assertion checks the step per JOB, because a file-level check passes as soon as any one job carries it, which is the half-fixed state a hand edit produces. The other pins the package list identical in both workflows, so a nightly copy cannot quietly lose a package. Verification: each guard was severed independently against these exact bytes. Removing the step from one job fails the per-job assertion while the list assertion still passes; narrowing the package list fails the list assertion while the per-job one still passes. Both mutants still parse as YAML and both reds are pytest exit 1, real assertion failures rather than collection errors; each restore was sha256-identical. 0 assertions removed, 0 skips, 0 xfails. tests/governance 308 passed; the CI contract siblings 74 passed, 1 skipped (that skip is pre-existing and in a sibling file); ruff check and format clean. This commit does not claim the nightly jobs now pass. It claims they can run.
The new guard adds one tracked Python path, so the moving count in docs/CLAIM_CORRECTIONS.md moves with it. Re-derived at the staged index as a set difference in both directions rather than by arithmetic: `tests/test_ci_qt_offscreen_contract.py` entered and nothing left, 734 to 735. The frozen PR7 correction at 672 is untouched, as that row requires. docs/current_candidate_metrics.md and docs/architecture-montana-important.svg are regenerated from the staged index by their generator, in this last commit, because the freshness guard reads them out of the index rather than the working tree.
|
@codex review Head: This restores a CI environment rather than changing product behaviour, so the What was wrong. Three things I would specifically like challenged.
Not claimed here: that the jobs now pass. Roughly 36 test files, the replay 0 assertions removed, 0 skips, 0 xfails; the workflow diff is 114 insertions and Written with AI assistance; every number above was measured in the session that wrote it. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4234aa356e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for relative in (".github/workflows/main.yml", ".github/workflows/nightly.yml"): | ||
| text = (ROOT / relative).read_text(encoding="utf-8") | ||
| assert QT_PACKAGE_LINE in text, ( |
There was a problem hiding this comment.
This file-wide containment assertion does not bind the package list to either named install step. For example, shortening only golden-replay's loop while leaving mock-stack-short-soak unchanged makes both tests pass: the first test sees the unchanged step name, and this assertion finds the required line in the other job. The replay lane can therefore become blind again while the guard remains green; locate each job's parsed install step and verify its condition and script against the canonical main-workflow step.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| return yaml.safe_load((ROOT / relative).read_text(encoding="utf-8")) | ||
|
|
||
|
|
||
| def test_qt_dependent_nightly_jobs_install_qt_libraries() -> None: |
There was a problem hiding this comment.
A repository-wide search finds neither this module nor either test node in governance/agent_preventions.yaml. Consequently these tests run only as ordinary remaining-suite tests rather than through the strict active-guard path, and the confirmed nightly false-green has no stable prevention ID or immutable red/green binding. Add the prevention record and both named nodes so the guard cannot later be skipped, deselected, or weakened without reopening the incident.
AGENTS.md reference: AGENTS.md:L387-L390
Useful? React with 👍 / 👎.
Dispatch result, and a separate finding this pull request does not fixDispatch 33290570326 on head
The soak's launcher log on this branch has no The new failure is real, and it is not mine to fix here
The So the first receipt cannot exist before ~566–601 s, while the assistant fault Dated, not guessed. The assistant-at-300 s event, the I am reporting this separately rather than folding a fix into this pull request, One side benefit worth namingEach completed short soak writes a 15-minute, per-role RSS and descriptor series It does not answer the memory question. This run's 62 samples span 300 s and Written with AI assistance; every number above was measured in the session that wrote it. |
Review found the first guard had a hole, and it was the same shape as the defect it was written for. Asserting the package line file-wide passes as soon as ONE job still carries it: shortening the loop in `golden-replay` alone left both assertions green, because the per-job check only looked for the step NAME and the file-wide check found the intact line in the other job. The replay lane could go blind again under a green guard. Each Qt-dependent nightly job's step is now compared to the canonical main.yml step field by field — run, if, timeout-minutes, env — and a separate assertion anchors the canonical step's package list, since the copies are compared against it and a silent narrowing there would otherwise propagate while everything stayed green. The prevention is registered as NIGHTLY-QT-LIBRARIES-BLIND-JOB-001 with both guard nodes bound to the `remaining` partition, so the guard cannot later be skipped, deselected or renamed without reopening it. The baseline is regenerated in the same edit and its floor moves by exactly one, 553 to 554. Verification, all three controls against these exact bytes: narrowing the loop in ONE job only — the exact scenario the previous guard survived — reddens the per-job node; deleting the whole step from one job reddens the same node; narrowing the canonical step in main.yml reddens both nodes. Every mutant still parses as YAML with three jobs, every red is pytest exit 1, and every restore was byte-identical. tests/governance 308 passed. 0 assertions removed, 0 skips.
The prevention record and its baseline changed the tree, so the generated metrics and SVG are re-derived from the staged index in this last commit, which is where the freshness guard reads them.
|
@codex review Head: Both P1 findings are taken. The first one was right in a way worth naming: the P1 — the package loop was not bound inside each job
Correct, and reproduced before fixing. My control had narrowed the list with an Each Qt-dependent nightly job's step is now located in the parsed workflow A second assertion anchors the canonical step's own package line. Without it the
Every mutant still parses as YAML with three jobs, so each red is behavioural P1 — the prevention was not registeredAlso correct. Registered as The dispatch evidence still binds at this head
Docs freshness 68 passed; the changed-Python count is unchanged at 735, since Written with AI assistance; every number above was measured in the session that wrote it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e55ac3417
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| every check would stay green while all three jobs went blind together. | ||
| """ | ||
| canonical = _qt_step(_workflow(MAIN_WORKFLOW), CANONICAL_JOB, MAIN_WORKFLOW) | ||
|
|
There was a problem hiding this comment.
The canonical anchor checks only that the loop header occurs as a substring, not that the packages can reach the install path. For example, replacing each dpkg -s ... || missing=... probe in the canonical step and its nightly copies with true || missing=... leaves this assertion and every field comparison green; on a runner missing libEGL, missing then remains empty, apt is skipped, and both nightly jobs return to the same import/collection blindness this prevention is meant to catch. Exercise or structurally validate the missing-package accumulation/install path rather than anchoring only the loop text.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
Review's counter-example: replace each `dpkg -s ... || missing=...` probe with `true || missing=...`. Every textual and field assertion stays green — the step name is unchanged, the loop header is unchanged, and the nightly copies still match the canonical step exactly — while `missing` never fills, apt is never called, and a runner without libEGL sends both jobs back to import-time blindness. Anchoring the loop text could not see this, because the defect is in the probe rather than in the package list. The canonical step's script is now run under bash against a stubbed `dpkg`, `sudo` and `apt-get`. One test drives dpkg reporting every package absent and asserts all four package names reach an apt-get invocation, which is the accumulation and install path itself. The other drives dpkg reporting everything present and asserts apt is never invoked, so a script that unconditionally installed could not satisfy the first test while reintroducing the bounded network stall the step's own comments record having cost two pull requests' evidence. Verification: with the probe neutered in BOTH workflows, so that every textual and field comparison still matches, only the executed-script node reddens — 1 failed, 3 passed — at pytest exit 1, with both workflows still parsing as YAML and both files restored byte-identical. The four nodes pass on Windows, the riskier of the two platforms this partition runs on, and bash is present on both runner images. Both new nodes are registered under NIGHTLY-QT-LIBRARIES-BLIND-JOB-001 and the baseline is regenerated in the same edit. tests/governance 308 passed.
The registry and baseline changed the tree again, so the generated metrics and SVG are re-derived from the staged index in this last commit.
|
@codex review Head: The P1 is taken, and it was the right escalation. The previous round bound the The script is now executed, not described
The canonical step's
The second one is not decoration. Without it a script that installed Control, with the probe neutered in BOTH workflows so that every textual and PlatformThe four nodes are bound to RegistryBoth new nodes are registered under Unchanged since the dispatch
Docs freshness 68 passed; changed-Python count unchanged at 735; derived pair Written with AI assistance; every number above was measured in the session that wrote it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85c176cc41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| canonical = _qt_step(_workflow(MAIN_WORKFLOW), CANONICAL_JOB, MAIN_WORKFLOW) | ||
| env, log = _stub_tree(tmp_path, dpkg_exit) | ||
| completed = subprocess.run( | ||
| [bash, "-c", canonical["run"]], |
There was a problem hiding this comment.
The helper changes the production invocation from bash -el {0} to bash -c. In the Ubuntu-runner scenario documented by the install step itself, adding exit 0 to all three matching scripts leaves these four tests green under -c, but the production login shell runs .bash_logout, where the failing clear_console changes the exit status and fails every affected job. Write the script to a temporary file and execute it through the same bash -el argument and profile boundary used by the workflows.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| def _qt_step(workflow: dict, job_id: str, relative: str) -> dict: | ||
| jobs = workflow["jobs"] | ||
| assert job_id in jobs, f"{relative} has no job {job_id!r}; jobs are {sorted(jobs)}" | ||
| steps = [step for step in jobs[job_id]["steps"] if str(step.get("name", "")) == QT_STEP_NAME] |
There was a problem hiding this comment.
This lookup verifies the step's name and contents but loses its position in the job. In both inspected nightly jobs, moving the unchanged install step to the end leaves all four guards passing; on a runner without libEGL, however, Golden replay lane or Run short mock-stack soak executes and fails before the libraries are installed. Assert that the install-step index precedes the first Qt-dependent invocation in each job.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| for package in ("libegl1", "libgl1", "libxkbcommon0", "libdbus-1-3"): | ||
| assert package in apt_log, ( |
There was a problem hiding this comment.
The missing-package test only checks whether each package name appears anywhere in the apt log, not whether apt actually installs it. Changing the first apt call in the canonical step and both copies to apt-get --simulate install -y $missing leaves all four tests passing because the stub logs the packages and returns success, so the fallback is skipped; a real runner would then finish the step without installing libEGL and the nightly lanes would remain blind. Parse the recorded argv and require a non-simulated install invocation containing the complete package set.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| executing the script catches that, so this drives the real production | ||
| script with `dpkg` reporting every package absent. | ||
| """ | ||
| completed, apt_log = _run_canonical_script(tmp_path, dpkg_exit=1) |
There was a problem hiding this comment.
The executed-script tests supply only uniform dpkg outcomes: either every package is absent or every package is present. A script that probes only libegl1 and, when it is absent, hardcodes all four package names into missing leaves all four guards passing; on a runner where libegl1 is present but libgl1 or another sibling is absent, it skips apt and the Qt-dependent job remains blind. Parameterize the probe so each package is independently reported as the sole missing package and require exactly that package to reach installation.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| stubs = { | ||
| "dpkg": f"#!/bin/sh\nexit {dpkg_exit}\n", | ||
| "sudo": '#!/bin/sh\nexec "$@"\n', | ||
| "apt-get": f'#!/bin/sh\necho "$@" >> "{posix_log}"\nexit 0\n', |
There was a problem hiding this comment.
The apt-get stub always returns success, so the refresh-and-retry branch is never executed. Replacing both fallback apt calls in the canonical script and its nightly copies with true leaves all four guards passing; when the first install cannot use the runner image's cached index—the exact production condition this branch documents—the step then reports success without installing the missing libraries. Make the stub fail the first install, then assert that the script performs an update followed by a real retry installation containing the missing packages.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
|
|
||
| #: Compared field by field rather than by the whole mapping, so that an | ||
| #: unrelated future key on one side names itself instead of failing opaquely. | ||
| BOUND_STEP_FIELDS = ("run", "if", "timeout-minutes", "env") |
There was a problem hiding this comment.
Only four selected step fields are compared, so adding continue-on-error: true to either nightly install step leaves all four guards passing. If apt then fails on an unavailable mirror or stale index, Actions continues into the Qt-dependent command, which again dies during import or collection rather than stopping at the named installation failure. Compare the complete behavior-bearing step mapping, or at minimum explicitly reject continue-on-error on each bound copy.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| #: The nightly jobs that import PySide6, transitively, through the launcher. | ||
| #: `mock-soak` is deliberately absent -- it was green throughout run 33247186128 | ||
| #: and needs no Qt libraries, so requiring them there would buy nothing. | ||
| QT_DEPENDENT_NIGHTLY_JOBS = ("golden-replay", "mock-stack-short-soak") |
There was a problem hiding this comment.
The guard's scope is fixed by this hand-maintained two-job tuple. Adding another nightly job that runs the same pytest tests/ -m golden command but omits the Qt-library step leaves all four tests passing because the new job is never inspected; on an Ubuntu image without libEGL, that new acceptance lane fails during collection without measuring its assertions. Derive the guarded inventory from the nightly jobs' Qt-consuming commands, or fail when a new job appears without an explicit non-Qt classification.
AGENTS.md reference: AGENTS.md:L391-L394
Useful? React with 👍 / 👎.
Review supplied seven concrete mutations that the previous version survived, and each is now a test with a control rather than a note: - the install step MOVED to the end of the job, so the lane that needs the libraries runs first: ordering is asserted against every step that runs python; - continue-on-error: true added to a copy, so a failed install no longer stops the job: the whole step mapping is compared, not a chosen list of fields; - a NEW nightly job added with no step at all: every job must be explicitly classified as needing Qt or not, so an unclassified job fails rather than going uninspected; - apt-get reached with --simulate, which logs the package names and installs nothing: the recorded argv is parsed and a real, non-simulated install is required; - a script that probes only libegl1 and hardcodes all four names: each package is now reported absent on its own and must reach installation on its own; - the documented refresh-and-retry branch never exercised: the stub now fails the first install and the script must update and really retry; - the script run under `bash -c` rather than the workflow's `bash -el` login shell: the script is written to a file and run exactly as the workflow runs it, and the shell itself is asserted so the reproduction cannot silently drift. The seventh control did NOT redden at first and that is recorded rather than smoothed over. Appending `exit 0` left all twelve executed-script nodes green on Windows, because the failing `clear_console` in ~/.bash_logout is a property of the Ubuntu runner image and this partition also runs on Windows. The rule the step's own comments open with is therefore asserted structurally as well — the script must never call `exit` — and the same mutation then reddens. Controls: seven mutations, each leaving both workflows parsable as YAML, each red a real AssertionError at pytest exit 1, every restore byte-identical. All ten guard nodes are registered under NIGHTLY-QT-LIBRARIES-BLIND-JOB-001 and the baseline is regenerated in the same edit. tests/governance 308 passed; 13 passed on Windows; 0 assertions removed, 0 skips, 0 xfails.
The registry and baseline changed the tree again, so the generated metrics and SVG are re-derived from the staged index in this last commit.
|
@codex review Head: All seven findings are taken. Each is now a test with its own reddening control,
Every mutant still parses as YAML, every red is pytest exit 1 with a real What changed
The one control that did not reproduceAppending Executing the script cannot carry that invariant on both platforms, so the rule I would rather state that limit than let a platform-dependent control stand in StateTen guard nodes registered under
Written with AI assistance; every number above was measured in the session that wrote it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3fa9fc61a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| installing nothing, which is indistinguishable from a real install if the | ||
| recorded argv is searched as flat text. | ||
| """ | ||
| return [argv for argv in invocations if "install" in argv and not (SIMULATION_FLAGS & set(argv))] |
There was a problem hiding this comment.
When the workflow uses apt-get -o APT::Get::Simulate=true ... install, this filter treats the invocation as a real install because it rejects only standalone aliases. I confirmed with local apt 2.8.3 that this form exits 0 and prints simulated Inst/Conf actions without installing, and changing all three workflow copies to it leaves all 13 guard nodes green. Fresh evidence after the prior simulation finding is therefore a configuration-form bypass: a runner missing libEGL can still reach the Qt command without the library. Parse -o/--option assignments (and combined short options) or make the stub record an actual installation effect rather than inferring it from argv.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| assert any("update" in argv for argv in invocations), ( | ||
| f"the first install failed and no index refresh followed: {invocations!r}" | ||
| ) | ||
| installs = _real_installs(invocations) | ||
| assert len(installs) >= 2, f"no retry install after the refresh: {invocations!r}" |
There was a problem hiding this comment.
When the cached package index cannot satisfy the first install, these assertions require an update somewhere and a second install somewhere, but never compare their order. Fresh evidence after the prior fallback finding: swapping the fallback to run the retry install before apt-get update in all three workflow copies leaves all 13 nodes green. The stub fails only the first install, so that unchanged second attempt succeeds artificially; on the production bash -e path, the same cached-index failure can abort at that retry before the refresh runs. Make installs continue failing until an update is observed and assert the exact failed-install → update → real-install sequence.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| ci_partition: core | ||
| red_evidence: measured_2026_08_30_on_python_3_14_6_restoring_the_entire_production_lakeshore_218s_py_to_exact_HEAD_sha256_782caf11415538fcfaec43af9668ac67e0ce52f97ae4e483bc500814cdf86def_still_parsed_and_reddened_the_mixed_0x42_exact_advisory_evidence_assertion_1_failed_while_the_existing_unknown_only_node_passed_then_restored_corrected_sha256_9c2ef716108b03dc65f24cb5f2b1a4f1f725d6f575cccdc8b5a03f87847f44f1_byte_identical; prior_unknown_only_control_measured_2026_08_30_on_python_3_14_6_changing_added_lakeshore_line_233_if_reasons_to_append_unknown_status_bits_0x02_within_that_same_parsing_line_reddened_the_exact_advisory_evidence_assertion_1_failed_then_restored_sha256_782caf11415538fcfaec43af9668ac67e0ce52f97ae4e483bc500814cdf86def_byte_identical_and_1_passed_no_immutable_receipt_is_bound_yet_pending_immutable_capture | ||
| green_evidence: measured_2026_08_30_on_python_3_14_6_after_byte_identical_restore_both_exact_core_nodes_2_passed_pending_immutable_capture | ||
| - id: NIGHTLY-QT-LIBRARIES-BLIND-JOB-001 |
There was a problem hiding this comment.
This adds only a runtime records entry, while its own red_evidence says seven accepted mutations previously left the guard suite green; the diff adds no linked entries under false_green_pairs. Those are confirmed coverage escapes, so folding them into the runtime ID omits the separate stable IDs, exact guard links, lifecycle, and independent evidence required to prevent the guard failures themselves from disappearing. Add the corresponding linked false-green prevention entries and regenerate the baseline.
AGENTS.md reference: AGENTS.md:L366-L369
Useful? React with 👍 / 👎.
Review found two more bypasses, and both came from the same root: deciding whether an install really happened by reading the recorded command line. `apt-get -o APT::Get::Simulate=true install` exits 0 and prints its Inst and Conf plan while changing nothing. The previous filter rejected only standalone aliases, so this configuration-option form was treated as a real install and all thirteen nodes stayed green. Separately, the fallback assertions required that an update and a second install both occurred but never that the update came FIRST; with a stub that failed only the first attempt, a retry placed before the refresh succeeded artificially. Rather than extend the argv parsing again, the stub now models apt. It is a small Python program executed through the same interpreter, it understands the option grammar including `-o KEY=VALUE`, `--option KEY=VALUE`, `-oKEY=VALUE` and clustered short options, and it records an actual installation effect. `dpkg` consults that same state. The tests then assert the installed SET, which a simulated install cannot produce in any spelling. For the fallback, every install now keeps failing until an update has been observed, so a script that retries before refreshing cannot recover at all, and the observed sequence is asserted explicitly as failed install, then update, then real install. A separate cheap assertion refuses to let the workflow ask for a simulation at all, in any spelling, which is the half a modelled stub cannot give. Three tests were RENAMED as they were strengthened and none was removed: the per-package and whole-set nodes now say "is really installed" rather than "reaches a real install", and the fallback node names the order it now checks. The registry guard links are updated in the same edit, so the rename cannot read as a removal. Each confirmed coverage escape is now its own linked false-green pair. The registry permits exactly one pair per guard node, so escapes that a single node closes are recorded together in that node's entry: eight pairs across eight nodes, covering all eleven confirmed escapes. The baseline moves by exactly eight, 554 to 562. Verification: nine controls, one per accepted finding, every one reddening at pytest exit 1 with both workflows still parsing as YAML and every restore byte-identical. tests/governance 308 passed; 14 passed on Windows; 0 assertions removed, 0 skips, 0 xfails.
The registry gained eight linked false-green pairs, so the generated metrics and SVG are re-derived from the staged index in this last commit.
|
@codex review Head: Three new findings taken. The fourth comment at Both new bypasses had one root: inferring the outcome from argvYou are right that The stub now models apt. It is a small Python program run through the same The fallback is now an order, not a set. Every install keeps failing until an A separate cheap assertion refuses to let the workflow ask for a simulation at Controls, one per accepted finding, against these exact bytes
Every mutant still parses as YAML, every red is pytest exit 1 with a real RegistryEach confirmed coverage escape is now its own linked false-green pair. The One thing to look at deliberatelyThree tests were renamed as they were strengthened, and none was removed: State
Written with AI assistance; every number above was measured in the session that wrote it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0170a4fd2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| with log.open("a", encoding="utf-8") as handle: | ||
| handle.write(" ".join(argv) + "\n") | ||
|
|
||
| ALIASES = {"--simulate", "-s", "--dry-run", "--just-print", "--no-act", "--recon"} |
There was a problem hiding this comment.
Fresh evidence beyond the prior simulation-form finding: adding -d to apt_options in all three matching workflow steps leaves all 14 guard nodes green because this model treats every non-simulated install as creating the installed markers. However, apt-get(8) defines -d/--download-only as retrieving packages without unpacking or installing them, so an Ubuntu runner missing libEGL would still reach the Qt-dependent command without the libraries. Model/reject this mode (including APT::Get::Download-Only=true) rather than recording an installation effect for it.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| # at all, so both paths are real and both have to be bounded. | ||
| missing="" | ||
| for package in libegl1 libgl1 libxkbcommon0 libdbus-1-3; do | ||
| dpkg -s "$package" >/dev/null 2>&1 || missing="$missing $package" |
There was a problem hiding this comment.
When a runner retains a package in deinstall ok config-files state after removal, dpkg -s still exits 0 because it reports the retained status record; I confirmed this with a temporary dpkg admindir containing exactly that state, consistent with dpkg-query --help describing -s as “Display package status details.” This loop therefore considers a config-only libegl1 present, leaves missing empty, and reaches the Qt-dependent command without libEGL.so.1. Query ${db:Status-Status} and require installed rather than relying on the status command's exit code.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
|
|
||
| for job_id in _qt_dependent_jobs(): | ||
| step = _qt_step(nightly, job_id, NIGHTLY_WORKFLOW) | ||
| assert step == canonical, ( |
There was a problem hiding this comment.
Changing the Qt step's if expression in the canonical step and both nightly copies to runner.os != 'Linux' leaves all 14 guard nodes green: this equality preserves the same wrong condition everywhere, while _run_canonical_script executes the body without evaluating the workflow condition. The actual Ubuntu jobs would therefore skip installation and again fail during Qt import without exercising their acceptance assertions. Assert the canonical condition's Linux semantics independently of copy equality.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| def test_canonical_qt_step_still_installs_the_evidenced_package_set() -> None: | ||
| """Anchor the content itself, since every copy is compared against it.""" | ||
| canonical = _canonical_step() | ||
| assert QT_PACKAGE_LINE in canonical["run"], ( |
There was a problem hiding this comment.
Changing timeout-minutes from 8 to 50 in the canonical step and both copies leaves all 14 nodes green; independently, deleting the HTTP/HTTPS timeout and retry options from all three scripts also leaves all 14 green. Under the stalled-mirror condition documented by the step itself, this lets the install consume the enlarged outer bound—or the entire existing bound in one apt call—and fail before either nightly acceptance lane runs. Pin the canonical step timeout and required per-call apt bounds rather than checking only the package-loop text and stubbed success paths.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
Correction: my earlier diagnosis of the soak failure was wrongEarlier in this thread I reported that the short soak profile "can never pass" I had not read the assistant's log when I wrote that. It says what actually The real defect: the reader and the writer of
|
| keys | |
|---|---|
| writer emits | active_experiment_id, app_mode, last_transition_receipt, last_transition_receipt_fingerprint, manager_incarnation, revision, schema_version, state_fingerprint, updated_at |
| reader accepts | active_experiment_id, app_mode, schema_version, updated_at |
| extra, and fatal | last_transition_receipt, last_transition_receipt_fingerprint, manager_incarnation, revision, state_fingerprint |
set(payload) == required is False, unconditionally. Nothing about timing,
load or ordering changes it. The assistant can never read the active experiment
state, so it never produces a periodic report, so no receipt ever exists — which
is why the soak found no pre-fault receipt, and why the evidence artifact
contains no periodic-receipts.jsonl at all.
Dated, and not a regression from the merged candidate
Both halves landed on 2026-07-25 in two different feature commits:
b5085fc3feat(core): safety lifecycle, typed command authority, channel descriptors — the writer's extra fields;f290fb18feat(agents): out-of-engine report runtime and periodic PNG delivery — the reader's exact-set check.
b5085fc3 is an ancestor of f290fb18, and the writer was already emitting
manager_incarnation at f290fb18. So the reader was introduced in a state where
it could never succeed against the file the writer was already producing.
Periodic reporting has never worked since it was added. The merged candidate
did not cause this; the libEGL blindness is why nobody saw it.
Consequence for the week-long run
On a week-long run the periodic report subsystem produces nothing at all, and it
fails quietly — an ERROR line every thirty seconds and no report. It is not
measurement data loss, and I am not claiming that: the raw acquisition path is
separate. It is a dead subsystem, and it is the reason the acceptance-criteria
soak cannot pass.
The fix belongs in its own pull request, not this one. This pull request's claim
is unchanged and still stands on its own evidence: the nightly jobs can run.
Written with AI assistance; every number above was measured in the session that wrote it.
CI went red on test (windows-latest, remaining) and it was this file's fault. Seven executed-script nodes failed there while passing on a local Windows machine, which is the shape that hides a real environment difference. The evidence bundle named it exactly: `sudo: command not found`, and all four packages reported missing on a run where only one was supposed to be. Both follow from one cause. The stub directory never reached the script's PATH, and the miss was SILENT because the script's own probe is `dpkg -s "$package" >/dev/null 2>&1` -- a missing `dpkg` sends its not-found error to /dev/null, so every package is marked absent and the run then dies at the first `sudo`. A LOGIN shell rebuilds PATH from its own profile, so passing it in the child environment is not enough. The script is now sourced from a wrapper that sets PATH AFTER the profile has run. The login shell, which is the property under test, is preserved: it is still `bash -el <file>`. The first attempt at that wrapper was still wrong, and the control caught it. A drive-letter path is not a PATH entry bash can search; the inherited environment is converted at shell startup, but a path prepended inside the shell is not, so the lookup still missed. The wrapper now converts through `cygpath` when it is present and uses the path unchanged elsewhere. The failure can no longer be silent: the wrapper checks that dpkg, apt-get and sudo all resolve, and the run fails with the tool that is missing rather than measuring nothing and reporting a package as absent. Verification: with the inherited PATH injection REMOVED entirely -- the CI condition, reproduced -- all 14 nodes pass, so the wrapper alone carries it. That same control failed before the cygpath conversion, which is how the second defect was found. All nine mutation controls still redden at pytest exit 1 with both workflows parsing and every restore byte-identical. 0 assertions removed, 0 skips.
The guard changed, so the generated metrics and SVG are re-derived from the staged index in this last commit.
|
@codex review Head: The previous head went red on Seven nodes failed on the hosted Windows runner while passing locallyThat is the shape that hides an environment difference rather than a defect in Two symptoms, one cause. The stub directory never reached the script's PATH, A login shell rebuilds PATH from its own profile, so passing PATH in the The first wrapper was still wrong, and the control caught itI did not trust the fix, because it passed locally before the fix as well. So I A drive-letter path is not a PATH entry bash can search. The inherited
So the wrapper alone now carries it, which is what the runner needs. And the failure can no longer be silent. The wrapper asserts that Everything else still holdsAll nine mutation controls still redden at pytest exit 1, both workflows still
Docs freshness 68 passed; derived pair regenerated to a fixed point as the Separately, and not part of this pull request: the correction posted above Written with AI assistance; every number above was measured in the session that wrote it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e690d52ced
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "for tool in dpkg apt-get sudo; do\n" | ||
| ' command -v "$tool" >/dev/null 2>&1 || { echo "STUB_NOT_ON_PATH:$tool" >&2; exit 3; }\n' | ||
| "done\n" | ||
| f'. "{script.as_posix()}"\n', |
There was a problem hiding this comment.
Fresh evidence after the prior login-shell finding: appending return 0 to all three identical workflow scripts leaves all 14 guard nodes green because . makes return valid, while the production bash -el <step-file> invocation rejects it with “can only return from a function or sourced script” and exits 2. Thus the PATH wrapper again changes the production process boundary it claims to reproduce; inject the stub PATH after login without sourcing the step, then execute the step file through the workflow's direct invocation semantics.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| assert completed.returncode != 3, ( | ||
| "the stubbed tools never reached the script's PATH, so this run measured nothing " | ||
| f"about the install path: {completed.stderr.strip()}" | ||
| ) |
There was a problem hiding this comment.
The new sentinel closes a separately confirmed coverage failure—these nodes passed locally while seven hosted Windows nodes measured system tools instead of the stubs—but a repository-wide search of governance/agent_preventions.yaml finds no prevention ID, false-green pair, or red evidence for the login-shell PATH loss or STUB_NOT_ON_PATH. If this shared helper later drops or weakens the bootstrap, the prevention-map validator therefore has nothing that reopens this incident; register this escape and its exact default-CI guard rather than leaving the check only inside the helper.
AGENTS.md reference: AGENTS.md:L361-L369
Useful? React with 👍 / 👎.
#72 and #121 are the same workflow change, and their guards are complementaryMeasured, not inferred: the I opened #121 without reading the open queue first; that was my mistake and #72 came first. But closing either one loses something real, so I am not proposing that yet. The two tests take
Notably, #72's approach is immune to the open P1 on #121 — that the test sources the step while Proposal: converge them into one pull request that keeps both guards — the parsed-workflow Until then neither should merge, because merging one leaves the other's guards unwritten. |
…he escape The guard reproduced the wrong process boundary. It injected the stub PATH by writing a wrapper that SOURCED the step, because a login shell rebuilds PATH and the stub directory has to be added after the profile runs. But sourcing changes the property under test: a top-level `return` is valid in a sourced script and fatal in an executed one. Appending `return 0` to all three byte-identical workflow scripts left all fourteen guard nodes green, while the production invocation `bash -el <step-file>` rejects it with "can only return from a function or sourced script" and exits 2. `BASH_ENV` is read after the login profile, so the bootstrap can restore and verify the stub PATH without sourcing anything. The target stays the file handed directly to `bash -el`, exactly as the workflow hands it over. Measured with that same mutation in place, all three scripts: the previous guard reported 14 passed and exit 0; this one reports 7 failed, 7 passed and exit 1, and every failing node is one that actually executes the script. Removing the post-login PATH instead yields 1 failed on `STUB_NOT_ON_PATH:dpkg`. Both workflow files were restored byte-identically afterwards, `main.yml` at `4adea30ebf869505c871f28252adf8b805ae9c1a` and `nightly.yml` at `53a74a12854ad3ae57d1856c668a3c3cbed8c21c`. The escape is now in the prevention registry as NIGHTLY-QT-LOGIN-PATH-AND-DIRECT-STEP-BOUNDARY-FALSE-GREEN-409, bound to the exact guard node and its partition, with both in-place controls as red evidence. It was previously enforced only inside the helper, so nothing would have reopened the incident if a later change dropped the bootstrap.
|
@codex review Head Finding 1, the process boundary. I ran your control myself, both halves, appending
Every one of the seven failures is a node that actually runs the script. Replacing the post-login Finding 2, the registry. Registered as Nothing was weakened: no test removed, no assertion removed, no skip or xfail, and the file has the One thing I would like your view on, separate from the findings. #72 makes the byte-identical |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1835450197
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| shell = workflow["jobs"][job_id].get("defaults", {}).get("run", {}).get("shell") | ||
| assert shell == WORKFLOW_SHELL, ( |
There was a problem hiding this comment.
Adding the same shell: /usr/bin/true {0} override to the canonical step and both nightly copies leaves all 14 contract nodes passing: whole-step equality still holds, this assertion reads only the job default, and _run_canonical_script independently hardcodes bash -el. GitHub Actions instead honors the step-level override, so the actual jobs would report a successful install step without executing its script and then fail at the Qt consumer. Validate that each install step has no overriding shell (or resolve and assert its effective shell) before treating the execution guard as production-equivalent.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| consumers = [ | ||
| (i, str(step.get("name", ""))) | ||
| for i, step in enumerate(steps) | ||
| if "python" in str(step.get("run", "")) or "pytest" in str(step.get("run", "")) |
There was a problem hiding this comment.
Adding if: ${{ false }} to both Golden replay lane and Run short mock-stack soak leaves all 14 contract nodes passing because this predicate recognizes a consumer solely from its run text and never validates the consumer step's activation metadata. The nightly jobs would then finish successfully after skipping both acceptance commands, producing no replay or soak evidence despite this guard considering their ordering valid. Bind each expected consumer by identity and reject conditions or overrides that can skip its production invocation.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
|
@codex review exact head $sha. This update closes the two current false-green paths without changing the production workflow. The guard now binds the exact nightly acceptance consumers, their automatic schedule, execution environment, effective shell, ordered setup-action prefix, and the absence of skip/continue/container/working-directory bypasses. It also hashes the complete parsed step prefix so an indirectly constructed GITHUB_ENV, GITHUB_PATH, PYTEST_ADDOPTS, or BASH_ENV channel cannot evade a token search. Coordinator evidence at this exact tree:
Please review exact head $sha, especially for any remaining way the two named acceptance consumers can be skipped or have their command replaced while this guard remains green. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf4f9cc5c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _assert_acceptance_execution_context(workflow: dict, job_id: str, step: dict) -> None: | ||
| """Bind environment and directory inputs that can replace the exact command.""" | ||
| assert "env" not in workflow, ( |
There was a problem hiding this comment.
Changing both acceptance jobs' runs-on values to self-hosted leaves all 69 contract nodes green because this execution-context check never validates the runner label. A self-hosted runner can inherit a BASH_ENV startup file containing exit 0; under the asserted bash -el {0} invocation, every run step then returns success before its script—including both acceptance consumers—while the workflow appears green. Require the reviewed ubuntu-latest environment for each acceptance job.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| - node: tests/test_ci_qt_offscreen_contract.py::test_the_step_skips_the_network_when_every_package_is_present | ||
| ci_partition: remaining | ||
| - node: tests/test_ci_qt_offscreen_contract.py::test_the_step_script_never_calls_exit | ||
| ci_partition: remaining |
There was a problem hiding this comment.
Fresh evidence beyond the earlier registration comment is that this exact head adds test_acceptance_jobs_are_unconditional_exact_commands and its new false-green controls, but the prevention record still ends with the older install-script guards; a repository-wide search finds no registered test_acceptance node or linked false-green pair. Consequently, deleting or deselecting the new guard would not reopen the prevention record even though the two newly confirmed escapes would return. Register the positive guard and the corresponding false-green obligations.
AGENTS.md reference: AGENTS.md:L387-L390
Useful? React with 👍 / 👎.
| assert "if" not in step, f"nightly acceptance step {expected['name']!r} has a step-level `if` and can be skipped." | ||
| assert "continue-on-error" not in step, ( | ||
| f"nightly acceptance step {expected['name']!r} has step-level `continue-on-error`; its failure " | ||
| "can no longer fail the job." | ||
| ) | ||
| assert step.get("run") == expected["run"], ( |
There was a problem hiding this comment.
Changing the existing mock-stack consumer's timeout-minutes from 30 to 1 and adding the same one-minute limit to the golden consumer leaves all 69 contract nodes green because this check compares only run and ignores the rest of each acceptance-step mapping. Both long-running measurements can then be terminated before producing their acceptance evidence, recreating a blind nightly lane despite the guard passing. Require the reviewed timeout value for each consumer, including the absence of an added timeout where appropriate.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| NIGHTLY_ACCEPTANCE_PRECEDING_USES = { | ||
| job_id: ( | ||
| "actions/checkout@v4", | ||
| "conda-incubator/setup-miniconda@8ee1f361103df19b6f8c8655fd3967a8ecb162d5", |
There was a problem hiding this comment.
The supposedly exact preceding-action prefix explicitly accepts actions/checkout@v4, which is a moving major-version tag, while only setup-miniconda is commit-pinned. If that tag advances to an action version that writes BASH_ENV, PYTEST_ADDOPTS, or a replacement executable through the Actions command files, the parsed YAML and prefix digest remain unchanged and all contract nodes stay green while both later consumers can execute different behavior; the main workflow already demonstrates the appropriate full-SHA checkout pin. Pin checkout to an immutable commit before treating the prefix as bound.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
1 participant
Two nightly guards have been blind, not red
The nightly workflow carries the two checks that sit directly on the readiness
criteria: golden-run replay regression (D4), which is how a misrepresented
dataset gets caught, and short mock-stack soak (evidence), which is how
memory growth and lag get caught.
In run 33247186128 neither of them failed an assertion:
ImportError: libEGL.so.1at import;36 errors during collection;1 skipped, 9296 deselected, 36 errors. No test executed.source stack did not reach the exact four-role startup cut; still missing: roles, handshake, bridge, bridge_guard. No memory or lag measured.The soak's own evidence artifact confirms the cause rather than inferring it: its
log-launcher.txtis a traceback ending in the samelibEGL.so.1, and both theengine and assistant logs read
never started. Its prerequisiteexact-sixgate had already passed, so nothing upstream was wrong.
Five consecutive nightly runs across three commits were red this way, the oldest
2026-08-25. Whether nightly ever passed was not measured and is not claimed.
The likely cause is runner-image drift, which
main.yml's own comments recordhaving seen before; no breaking repository commit is alleged.
The defect was divergence between two workflows
main.ymlinstallslibegl1 libgl1 libxkbcommon0 libdbus-1-3and stayed greenthroughout.
nightly.ymlhad no such step at all. That step is copiedverbatim, comments included, into the two nightly jobs that reach PySide6
through the launcher. It is copied rather than rewritten because two things in it
are load-bearing and were each paid for once already: the bounded apt flow, and
the prohibition on calling
exitfrom that login shell.bounded mock soak (B-phase-exit)is deliberately left alone — it passed inthe same run without Qt, so installing libraries there would prevent nothing.
main.ymlis not touched.The guard binds both halves
one job carries the step — precisely the half-fixed state a hand edit leaves.
quietly lose a package and reintroduce the same blindness under another name.
Verification
Each guard was severed independently, against the exact bytes on this branch:
Both mutants still parse as YAML, so each red is behavioural rather than a
collection error; both reds are pytest exit 1, real
AssertionErrors; eachrestore was sha256-identical.
0 assertions removed, 0 tests removed, 0 skips, 0 xfails. The diff to
nightly.ymlis 114 insertions and 0 deletions.tests/governance308 passed. The CI-contract siblings plus this guard74 passed, 1 skipped (that skip is pre-existing, in a sibling file).
ruff checkandruff format --checkclean. Docs freshness 68 passed withTRUSTED_BASE_SHAbound as CI binds it. The changed-Python count was re-derivedat the staged index as a set difference in both directions — one path entered,
none left, 734 to 735 — and the derived pair was regenerated to a fixed point as
the last commit.
What this pull request does and does not claim
It claims the two jobs can run. It does not claim they now pass.
With the libraries restored, roughly 36 test files, the replay lane and a real
soak will execute for the first time in at least five days, against a master
that has just absorbed a large integration candidate. If either job reddens on
the dispatch below, that is the guard seeing again — possibly a genuine
regression signal — and it will be reported as its own separate finding rather
than folded into this one.
Evidence dispatch on this branch: run 33290570326.
Written with AI assistance; every number above was measured in the session that wrote it.