Repository navigation
cli/assertions: retain a trace for inconsistent outcomes - #342
Merged
Merged
Conversation
Normally returned assertion advice could lose execution metadata when its result marker was missing or inconsistent with the findings. Record a fixed reporting failure after preserving the existing finding output, so consumers can distinguish that condition from an unrun phase. Revalidate typed markers without turning callback, output, or fatal failures into terminal results, and leave primary verdicts and receipts unchanged. Signed-off-by: Codex <codex@openai.com>
HouMinXi
marked this pull request as ready for review
October 31, 2026 15:58
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
HouMinXi
added a commit
that referenced
this pull request
Nov 1, 2026
* style: ruff format src/code_forge/machine.py (format only) No behavior change. machine.py is not format-clean on main, and the Lint changed Python files gate runs ruff format --check on every touched file, so the formatting lands as its own commit ahead of the #262 change. * fix(machine): skipped-source L0 findings no longer hold off the stall (#262) The identical-round stall counted every unpromoted CONFIRMED L0 finding as fix budget left. The LOCAL autofix loop skips MUTANT, E2E_CHECK, FIXVAL and INFRA findings, so they never spend budget or get promoted, and a map frozen only by them ran to max_total_rounds. The predicate now treats a fingerprint as unable to move the map when the current findings carry it and every finding with that fingerprint has a skipped source. Both sites share one _AUTOFIX_SKIPPED_SOURCES set. * fix(isolate): keep the child startup diagnostic on a failed start (#31) A forked sandbox child that failed before exec wrote b"error" and exited 127, and on a placement failure the parent never read even that. The recorded #31 qualification stop therefore said only "payload process was not placed in the run cgroup", with no child errno. The child now records which step it was in (setup, place, gate, stdio, exec), the exception type and errno, bounded to 200 ASCII bytes. A placement failure reports the child exit code, or that it was still running at the deadline, plus that diagnostic, read without blocking. An exec failure carries the same diagnostic. A failed start also closes the pipe and cgroup.procs descriptors the parent still held. Refs #31 * fix(machine): state the #262 stall rule's real scope, pin the allow-list Production L0 runners emit source "L0" only, so the skipped-source rule is a defensive alignment with the autofix skip set for injected or future L0 producers. Say so in the docstring and test module, and name the set instead of repeating its members. Add two cases: an unknown source still holds off the stall (the skip set is an allow-list), and one fingerprint carrying two skipped sources stalls. Refs #262 * Document the same-tick ctime limit in mutation workspace checks (#197) #197 notes that (dev, ino, ctime) stamps cannot see a change undone within one kernel clock tick, and that this also applies to the #103 snapshot races. Say so at the three stamp comparisons in _mutation_workspace.py: the cache file snapshot, the cache directory snapshot and the configuration read. Comments only. Refs #197 * fix(machine): record autofix-skipped L0 fingerprints per round snapshot (#262) The stall predicate read source sets from the merged state, which keeps one finding per fingerprint. A raw [fp-x/L0, fp-x/MUTANT] pair collapsed to the MUTANT finding, so the map stalled while fp-x still had fix budget left. Each round snapshot now carries l0_autofix_skipped: the L0 fingerprints whose every raw finding has a source the autofix loop skips. The predicate reads only the latest snapshot's list. A missing or malformed field proves no skip, so every unpromoted L0 fingerprint holds the stall off, as on main. Tests build rounds through the real merge and snapshot code, use a NO_CHANGE autofixer so budget assertions can fail, and add an end-to-end case for the duplicate pair. * fix(isolate): harden startup diagnostics and report cgroup cleanup failures Refs #31. - Child: if formatting the diagnostic itself raises, still write "error stage=unknown" before _exit(127). - Diagnostics keep only bytes 0x20-0x7e (others become '?') on both the child encode and the parent decode; exits below zero read "signal N". - The exec status read asks for LIMIT - len(status) bytes instead of reading the full limit and slicing. - _remove_cgroup raises CgroupCleanupError (and sets cleanup_error) when rmdir still fails with EBUSY, ENOTEMPTY or EACCES at the 10 s deadline. An absent cgroup still counts as removed (#320). A failed start keeps its own error and carries the cleanup failure as a note. - Tests patch isolate.os through a scoped proxy instead of the global os.open, cover every child stage, the fallback, fd ownership on each failure path, byte bounds, and the cleanup deadline with a fake clock. * fix(machine): skipped L0 fingerprints never pend promotion (#262) The pending-promotion guard counted any L0 fingerprint with a spent fix budget, including one the autofix loop now skips. A fingerprint that spent its budget as L0 and then arrived only from a skipped source is never promoted, so it held the stall off until max_total_rounds. The skip proof is now read before the guard and the guard ignores skipped fingerprints. Tests pin the remaining review findings: promotion at exactly the limit (>= vs >), attempts above the limit after a resume with a lower max_fix_attempts (>= vs ==), a sorted skip field over 14 fingerprints, malformed str/tuple/set/dict values failing closed, the skip proof coming from the projected row rather than a trailing pending or dispositionless host row, and a resume from a state.json written before the field. Refs #262 * Keep the in-flight error and the run evidence when cgroup cleanup fails Refs #31 python_mutmut _run_sandboxed: a CgroupCleanupError from teardown no longer replaces the error in flight (including KeyboardInterrupt, SystemExit and CancelledError); it is attached as a guarded note and the original is re-raised. The supervisor thread is joined in a finally on both paths. When the command finished but its cgroup was left behind, the run fails closed: CgroupCleanupError is raised with the receipt, exit code and timeout flag attached, and run() turns it into an INCOMPLETE TargetResult whose Cleanup record has owned_group_empty=False and the cleanup message in errors. isolate: - attach_note() adds a note and drops any failure while doing so. - The CgroupCleanupError message is printable and capped at 400 characters, and chains the last rmdir error as __cause__. - _remove_cgroup resets cleanup_error at the start, so a stale failure does not survive a later successful removal. - teardown forces removal, makes one attempt (timeout 0) when an earlier removal already ran out its deadline, and marks itself done only after the cgroup is gone, so it can be retried. - verify_isolation_support reports a probe cgroup it could not remove (reason probe_cleanup_failed), or notes it on an error in flight. Tests cover each of these, plus a read error that is not EAGAIN, a close failure during start cleanup, the real _remove_cgroup on the start path, the child fallback under KeyboardInterrupt, and the exec read asking for 200 then 50 bytes without a sleep. * Tighten the ctime tick-limit comments in mutation workspace checks (#197) Review follow-up, comments only (the module AST is unchanged): - The file snapshot check misses a same-size write; size changes are seen. - The directory check misses a create+remove only in the same tick as the directory's previous change. - _read_config misses a same-tick rewrite only when expected is None; with expected set, the sha256 comparison catches changed bytes. - All three say Linux 6.13+ multigrain timestamps close the window, in the wording conventions.py uses. - The tick is given as 1-4 ms everywhere in the file. * Report cgroup cleanup failures in every sandboxing adapter - verify_isolation_support: a probe cgroup that is already gone is not a cleanup failure (FileNotFoundError is ignored, as in _remove_cgroup). - Mutmut probe: a probe cgroup left behind is reported as a retryable cleanup error (probe-cleanup-failed), not as missing isolation. - Move the cleanup-failure result into base.cleanup_failed_result and use it from the gremlins, Stryker, cargo-mutants and patch-corpus run(), and from the gremlins version probe, so a run cgroup left behind gives an INCOMPLETE result instead of an uncaught CgroupCleanupError. - _run_sandboxed reads memory.peak inside the guarded block, so an interrupt there still tears down the supervisor and joins its thread. - Tests assert the retryable flag, the cleanup-failure.json contents and that the artifact is listed in native_artifacts. Refs #31 * Give the ctime tick as 1-10 ms depending on CONFIG_HZ (#197) The tick is 1 ms at HZ=1000, 4 ms at HZ=250 and 10 ms at HZ=100, so the four comments in _mutation_workspace.py now say 1-10 ms depending on CONFIG_HZ. Comments only; the module AST is unchanged. * fix(machine): require every frozen-window row to prove an L0 skip The skip proof was read from the latest projected row only, so an L0 fingerprint the autofix loop handled earlier in the frozen window counted as skipped. L0/MUTANT alternation stalled at round 3 with 2 of 3 attempts spent, and MUTANT/L0 at round 2 with 1 of 3, where main promotes at rounds 6-7. A fingerprint now counts as skipped only when all three window rows list it in l0_autofix_skipped. A missing or non-list field in any of them proves no skip. Tests: alternating sources in both orders promote; each window row must prove the skip; rows before the window do not matter; L0x3 then MUTANT stalls at round 5; MUTANTx3 then L0 still stalls at round 2 (known limit). The resume test now asserts round 5, which a fail-open reading of the missing field would turn into round 3. Refs #262 * Keep run evidence and report missing isolation in every sandboxing adapter - The gremlins, Stryker and cargo-mutants adapters now collect receipts, artifacts and the baseline in a RunEvidence object owned by run(). A sandbox failure on a later command keeps every earlier receipt and the passed baseline instead of only the failing receipt. - IsolationUnavailable raised while starting a sandboxed command becomes an INCOMPLETE result in the mutmut, gremlins, Stryker, cargo-mutants and patch-corpus run() paths, and UNAVAILABLE_ISOLATION in the gremlins probe. If the run cgroup was also left behind, that is reported as a cleanup error. Both cases go through base.sandbox_failure_result(), which replaces cleanup_failed_result(). - The gremlins probe uses a TemporaryDirectory, so it no longer leaves a forge-gremlins-probe-* directory behind on each call. Refs #31 * Label run-time isolation failures with the run phase, not probe Supervisor.start checks isolation again before each command, so an IsolationUnavailable reaching sandbox_failure_result happened on a run command, not in the adapter's probe. Report it in phase "baseline" when no baseline was recorded yet and "mutation" after it. A probe cgroup left behind stays a "cleanup" error, and the probes still report "probe". Refs #31 * test: wrap a docstring over the 105-column limit Refs #262 * Say which expected-None config reads lack a byte check (#197) _write_private_config and _retain_config_backup read with expected None but compare sha256 against the bytes they just wrote, so a same-tick rewrite there is caught. Only the first capture in acquire has no reference bytes. Comment only; the AST is unchanged. * Add each isolation note once and test sandbox failure results in full When Supervisor.start fails and its cgroup cannot be removed, the start error carries the cleanup note, and the mutmut adapter's teardown then added the same note again. attach_note now skips a note that is already on the exception. The adapter cleanup tests now check every field sandbox_failure_result fills: target id, adapter and tool versions, extractor version, owned_mounts_removed, the target id and message of each error, and the digest and byte count of the failure evidence against the published file. Two new patch-corpus tests cover a sandbox failure on the baseline command and a failure on a later corpus entry, where the earlier entry's receipt and event artifact must be kept. Refs #31 * Test that attach_note compares whole notes across the list A start failure and its teardown can fail on the same cgroup with different errno values, so their notes share a long prefix and must both be kept. A note added before the last one must not be added again. Refs #31 * Test that attach_note adds a note that shares part of another note Only an equal note is skipped. A note that contains another, is contained in one, or shares a suffix with one is still added, in either order. Refs #31 * Test that attach_note skips only an equal note Notes that differ in one character after a long shared prefix or before a long shared suffix, in case, or in trailing space are all kept, and a repeated note keeps its first place. Refs #31 * Test that attach_note compares notes by value, not identity The second note has the same text as the first but is a separate string built at runtime, and it is skipped. Refs #31 * feat(332): Phase6 credit preserve for schema_fail/incomplete-coverage Record clean_credit_action=preserved for Q2=(iii) receipt rows only via credit-side skip of _reset_clean_credit. Prior earned window survives; no new earn; still non-PASS. _fixpoint_reached product RESET rules untouched. Closed-set + earned_history treat preserved like interrupted/evidence_rejected. * fix(#332 phase6): address PR #319 joint review (H1/H2/M1-M3/L1-L6; M4 held) - H1: failed-attempt path preserves only when every round phase returned; predicate also requires all phases returned. - H2: _host_receipt_active() is the first predicate guard; non-receipt local runs keep the M6 reset exactly (regression test). - Gate path: preserve only when every gate error is the attempted-evidence rejection; otherwise wipe + reset (closes literal-damage + coverage softening found while probing). - M1: validator requires all phases returned, no protocol failures, and (iii)-exact dispositions (CONFIRMED schema/coverage cause present in l1_fingerprints; others limited to mutation infra / UNCERTAIN receipt audit). Negative tests carry match=. - M2: per-guard StateMachine tests, recurring CYCLE_RESTART survival, schema + coverage near-miss (incl. file), coverage on gate path. - M3: match= everywhere, seeded windows, factory-built fixture check. - L1: mutation-survivor ladder routed through the predicate. L2: CYCLE_RESTART log no longer says "restarting" when credit is preserved. L5/L6 docstrings. - M4: held fail-closed (recurring P3 + new schema resets); pinned by test, pending user decision. verify.py unchanged. * fix(#332 phase6): PR #319 fix2 (L-A, L-B, M4, L-C) - M4 (user decision, plan F Q7 / P section 2.2): preserve iff the unchanged fixpoint, re-evaluated without the (iii) schema/coverage findings, is CLEAN. Recurring sub-threshold P3 + schema/coverage now preserves; P0-P2, density, reversions, new CONFIRMED and semantic UNCERTAIN keep the reset. _fixpoint_reached is not modified (_fixpoint_without swaps findings and restores them). - M4 loader: a preserved row may carry an open product CONFIRMED only when it recurs as the fixpoint sees it (seen before, not FIXED in the previous projected row, not first-confirmed after a protocol observation; synthetic acquisition markers projected out). Severity is not persisted, so P0-P2 refusal stays in the machine counterfactual. H1/H2/M1 guards unchanged. - L-B: gate errors are read before the CYCLE_RESTART log; a non-attempted gate error clears the preserve decision first, so the log always names the ledger action. - L-A: pin audit-unavailable (+ receipt-write I/O) beside attempted evidence wipes credit, as on M6 (deliberate narrowing; T6 preserved). - L-C: run-level coverage of the survivor-ladder preserved arm, plus the default (P2) survivor reset. verify.py unchanged. * fix3-part1: tests for L2-infra INCOMPLETE arm (L-1), non-vacuous earned-window wipe (L-3), M4 docstring (N-2) Tests only; no src change vs ee7a258. All greens not independently reproduced. * fix3-c3: C3 narrow schema-fail companion preserve + preserved-row companion strip Per plan-trio c3-impl-spec-joint-v1.md (sha256 bc577a5a...cdc90). Predicate: _schema_coverage_credit_preserve removes UNTRUSTED companions of a single parsed schema-fail response (rules 1-5, D2) from the counterfactual. Writer: _finish_host_attempt omits those fps from a preserved row's dispositions (mechanism C). No loader change; state.py untouched. Narrow scope (single-response schema failure) is user-question-pending. Tests: spec section 4 unit + run-level; M4 :707 test renamed to coverage. * fix4-c3: C3 rule 2a per-pass reviewer response count (C3 v3, D1 count-all) Count every received reviewer response per pass at the producer, before validation, and require exactly one for companion eligibility. - factories: _L1Call.reviewer_responses (None per __call__) and record_reviewer_response; _provider counts parsed responses and InvalidJSONResponseError; the grouped composite reports None (2g). - outlet_c: _run_chunk counts once per received response when it has an owner, so chunks sum over the round. - machine: validated in-memory snapshot _responses_last_round, cleared before L1 and captured beside the attempted list; rule 2a before the attempts filter. Nothing is persisted. - tests: spec v3 section 4 T1-T16, T18; v1 fixture records its count through a real _L1Call; T16 made non-vacuous. Closes the H-1 chunked decoy (TD-P6-C3e): the round now resets. * fix5-lint: ruff-clean the 10 allowlisted files; L-6 non-vacuous bystander; M-1 fixture comment; J-L2n Outlet C single-response preserve test * Fix review findings on the child startup diagnostic The child now reads the placement gate once. EOF means the parent closed the gate without opening it, so the child reports stage=gate with EOFError and exits 127 instead of spinning on empty reads. The parent records the child right after fork, before closing its fds, so a failure in those closes still kills and reaps the child. Cleanup messages put the error type and errno ahead of the path, so a long path cannot push them past the length cap. sandbox_failure_result drops its infra_errors parameter. Its only caller that passed it now leaves the errors to the caller. Tests: - A per-test SIGALRM limit, so a blocking read or a busy loop fails the test instead of hanging the run. - A cleanup helper that kills a stuck child. - New cases for gate EOF, a failure right after fork, a long cleanup path, the pipe switching to non-blocking, and an errno that is not an int. - The KeyboardInterrupt case now uses its own BaseException and asserts that the call returns. Signed-off-by: Minxi Hou <houminxi@gmail.com> * fix6: failed host attempt never preserves (CR2G-M1, option A) _finish_failed_host_attempt resets clean credit on RESET/CYCLE_RESTART exactly as main 869873f does (never preserved, never left unavailable); the original exception still propagates. P6-1 (IO error resets, no exception), PB guard 3, spec v3 TD-P6-C3a, CR1 residual C4. The attempt's receipt-write errors are logged into infra_errors first, before any early return (CoS 14:37, CR1 C3 / CR2 caveat 6), using the existing extend(_last_receipt_write_errors) pattern. Regression tests (CR1 + CR2 probes): receipt-write OSError then RuntimeError / Ctrl-C / state-save OSError / workspace-blocker TimeoutBreaker / failure after the main-path gate, gate raising, literal damage then crash, crash before receipts are published, clean gate then crash; and no terminal PASS with window [1, 2, 4] after one healthy follow-up round. Nit1: deviation-7 comments at the legacy _c3_machine hand-sets. * fix6: split preserved-row validation into state_preserved.py (D-v11-1) Move _validate_preserved_dispositions, _recurring_confirmed, _project_product_row, _PRESERVED_III_FP and their private helpers _RECEIPT_AUDIT_FP and _MUTATION_INFRA_FP verbatim (ast.dump identical) into the one new module src/code_forge/state_preserved.py. state.py imports and re-exports them (958 lines, under 1000). state_preserved.py does not import state at module level; its single state dependency, is_host_round, is a late-bound wrapper that resolves state.is_host_round at call time. No test monkeypatches the moved names. Behavior unchanged. * fix(phase6): a failed host attempt never keeps a settled preserve; log receipt-write errors once CR3R-L1: when the main path already finished the round as `preserved` (receipt gate or active path) and a later step raises, the failure handler now turns the row back into the M6 reset before any early return: credit 0, earned window wiped, stripped C3 companion fingerprints restored to the row's dispositions. The original exception still propagates. CR2R2-L1: the failure handler logs only receipt-write errors that the TimeoutBreaker branch or the receipt gate finding has not already logged, so infra_errors and the CLI show one line. Tests: two new fix6 cases (state save OSError after preserve, gate record error after preserve) in the reset and healthy-round tests, the C3 companion restore, the failure-record save error never replacing the primary (CR3R I-4), and one-entry/one-line checks for both logging paths. * test(phase6): loader accepts a recurring UNCERTAIN fingerprint confirmed without protocol (CR2R2-L3) * fix(phase6): crash at any post-settle save resets; late-bind state._project_product_row (J-L1, J-N1, I-9) - test: an OSError on each state save after a `preserved` settle (receipt gate, 3-survivor stop, active fixpoint; b33b72c :2289/:2314/:2356) ends as reset with credit 0, and one healthy round then earns credit 1, window [4]. - test: the gate's `receipt: <error>` entry plus a failed state save at the gate logs the receipt-write error exactly once (J-N1 b). - docstrings of the fix6 failed-attempt tests match the fix7 behavior. - state_preserved rebinds `_project_product_row` to a late-bound shim over `state._project_product_row` (state re-exports the moved definition), so a patch of state reaches the split-out validator (joint I-9). * fix(phase6): a demoted CYCLE_RESTART preserve logs the restart, not "credit preserved" (L-B) A failure after the active path settled `preserved` on CYCLE_RESTART left this attempt's "clean credit preserved" line beside the reset row. The demotion now rewrites that line to the restart line, so the log names the action the ledger records. The two log strings are module constants. Test: the post-settle crash test gains the active CYCLE_RESTART save site. * fix(phase6): demote only the preserve this attempt settled (CR3R2-L1) fix7 demoted any preserved row at state.round on failure. A Ctrl-C between rounds, or a CorruptedStateError in _begin_host_attempt, turned the previous round's saved preserve into reset/0/[]. Record _settled_preserve_round when _finish_host_attempt settles a preserve, clear it at every loop start and at _begin_host_attempt entry, and demote only that round. Tests: interrupt/CorruptedStateError between rounds keeps the prior preserve on disk and in memory; an early crash after resume (begin, receipt diff; CR3 I-6) changes neither state.json nor memory. * refactor(phase6): one _project_product_row definition, in state.py (I-3) The real definition returns to state.py with its 1d5c6c0 body; state_preserved keeps only the call-time shim (like is_host_round). No duplicate def, no F811. Patching state._project_product_row still reaches the preserved validator (I-9), and the function pickles again. * fix(phase6): pin the demotion scope; demotions always reach disk; per-attempt bookings - A demotion now always takes the failure path's save (CR2G P12): the not-observed early return no longer applies once a row was demoted. - _booked_receipt_write_errors resets with _last_receipt_write_errors when each attempt's L1 starts (CR2G I4), so a second run() logs its own error. - Tests: same-attempt resets after the saved preserve (real #114 breaker, Ctrl-C in the hold check, stall and MAX_TOTAL_ROUNDS ESCALATED saves); earned row kept after a post-settle failure (CR2F7-L2 P11); pre-attempt preflight Ctrl-C after resume changes nothing; demotion reaches disk; a second run's identical receipt error is recorded once. * test(phase6): the P12 disk test observes nothing (phase status not_run) With the finished round's phase status left as returned, the failed path saved at 311077f too, so the test was green there. Now red at 311077f (row stays preserved on disk) and green at fix8. * fix(phase6): run() entry drops the settled-preserve guard (CR3R3-L1) A second run() on the same instance that failed before its first attempt (preflight, receipt diff, continuation index) still held the guard from the previous run(), so it demoted the row that run had settled preserved and saved the reset. run() now clears the guard before it loads prior state. Tests: the same-instance second run() interrupted at each of the three points keeps round 2's row, credit and window on disk and in memory (red on 072e3bb); a direct `_begin_host_attempt` call with a forced stale guard leaves the preserve alone (pins the begin-entry clear). * test(phase6): rerun and other-instance cases; P12 test fails inside its attempt Joint review R1 on 072e3bb (J-L1, J-N1, J-N2): - the same-instance second run() case gains a preflight TimeoutBreaker; - E2: run 1's demotion save fails (disk keeps preserved, credit 2); the next run() on the same instance, interrupted before its attempt, leaves disk preserved with credit 2 and window [1, 2]; - E3: A settles round 2 preserved, the ledger is rolled back and B settles round 2 earned (credit 3) or preserved (credit 2); A's second run(), interrupted before its attempt, leaves B's row unchanged; - test_fix8_demotion_always_reaches_disk now fails inside the attempt that settled the preserve, after the final save, with its observations dropped; it no longer relies on a guard left over from a finished run(). * test(phase6): two runs on one instance, the second failing at preflight (CR2F8-L1) R2 P13a/P13b on 072e3bb: run 1 returns ESCALATED (coverage) or FAIL (schema gate) with round 2 preserved; run 2 on the same instance fails in the startup preflight (Ctrl-C or RuntimeError). Round 2 stays preserved with credit 2 and window [1, 2], reloaded from disk and in memory. * fix(machine): reset all per-run state at run() entry A second run() on the same instance that failed before its first attempt still saw the previous run's phase findings, phase status and receipt-write errors. Its failed path then logged the earlier run's receipt-write error a second time and saved a FAIL verdict. Move every per-run, per-attempt and per-round attribute into one helper, _reset_run_state(), called from __post_init__ and as the first step of run(). It replaces the scattered run() entry resets, including the clear of the settled-preserve guard. Advisories, their buffer and the mutation review source stay instance-scoped, as before. Comment and docstring now say the demotion applies within the same run(), before the next attempt begins. * test(machine): a second run() failing early adds nothing to the state Add the secondary-interrupt rerun case, a P21/P13 test that each receipt-write error is logged once and the ledger and verdict reload unchanged, and a generic test that run 2 on the same instance saves the same state.json as a new instance doing the same run 2. * fix(machine): keep receipt cycle windows instance-scoped The run-entry reset also cleared the written and clean-window receipt cycles, which a reused instance relies on to attest and to detect a stale window across run() calls (tests/test_provider_capacity_reporting.py). Keep both for the life of the instance, as on main. * test: pin per-run reset through repeated run() on one instance (fix11) Differential tests: a second run() on a reused instance must save what a new instance saves, after run 1 stops in each of 14 LOCAL ways and 5 CI ways, and a gate edited between the runs must be pinned afresh. Kills the keep-reset mutants for the four round accumulators and the untracked-gate pinned flag. No production change. * test/docs: pin trusted-config refusal and host-attempt-round resets; correct reset docstring (fix12) - J-N1: run()-level CI test that kills the _trusted_config_refusal reset. - J-N6: double-fault run 1, then a same-instance run 2 stopped at _receipt_diff; kills the _host_attempt_round reset (red before fix10). - J-N2: correct the fix11 CI differential docstring (run 2 FAILs on the untracked gate) and add a CI case with an outside trusted gate root whose runs reach the measurement (run 1 PASSes). - J-N3 / N-1: _reset_run_state docstring limited to per-run, per-attempt, per-round and L2-phase state; names what is kept across runs. No production logic change. * test: reuse the existing _interrupt_receipt_diff helper in the fix12 test (fix12) * fix(state): omit unobserved unavailable fixpoints (#334) (#338) Co-authored-by: Codex <codex@openai.com> * fix(verify): validate authority for every open finding (#339) Align Check 8 with the existing closed-disposition allowlist. Preserve absent-basis compatibility while treating malformed or unknown authority as unresolved. Keep original receipt snapshots and evidence-only behavior unchanged. Co-authored-by: Codex <codex@openai.com> * mcp/assertions: expose validated advisory outcomes in results (#340) Project optional SARIF assertion metadata through the existing outcome validator and accepted report boundary for immediate and background results. Preserve findings, receipt audit, raw output and tuple compatibility. Partially addresses #330 section 1; remaining reporting and outlet work stays open. * cli/local: publish assertion execution metadata separately (#341) Local subprocess runs did not retain the optional assertion execution result in a dedicated artifact. Publish validated metadata beside state without changing the primary verdict or output contract. Invalidate stale metadata only after acquiring the lock and replace it atomically after advice, keeping interruptions and file errors separate from primary state. Signed-off-by: Codex <codex@openai.com> * cli/assertions: retain a trace for inconsistent outcomes (#342) Normally returned assertion advice could lose execution metadata when its result marker was missing or inconsistent with the findings. Record a fixed reporting failure after preserving existing finding output, while leaving callback and fatal errors, primary verdicts, and receipts unchanged. Signed-off-by: Codex <codex@openai.com> * Reject quote repairs that invent a reply key (#264) (#333) * Reject quote repairs that invent a reply key (#264) _loads_model_json repairs unescaped inner quotes. An excerpt that quotes a dict literal such as cfg = {"a": "b", "c": "d"} repairs into a finding with a new key "c" and an excerpt cut at "b. The parse succeeds and the evidence is silently wrong. Callers now pass the keys their reply may carry, per level (ReplyKeys). When a parse needed repair and the result carries a key the caller did not list, the repair is discarded and the original JSONDecodeError is raised. An api backend then spends its existing JSON-correction retry; the cli inner-result parse falls back to the raw string, which the caller's validation rejects. A parse that needed no repair is never checked. Wired: L1 review passes (factories._run_pass), subagent spawn and the test-assertion review (cli), the runtime axis and the real falsifier. Refs #264 * Format files touched by the #264 change with ruff 0.17.3 Format only; the AST of every file is unchanged. One message string in cli.py is split into two literals to stay within 105 columns. Needed because the lint gate runs ruff format --check on each touched file. Refs #264 * Update call-shape tests for the reply_keys keyword (#264) * Format two test files touched by the #264 change with ruff 0.17.3 * fix(llm_invoke): check reply keys on bare CLI stdout (#264) When a CLI or proxy prints the model's JSON with no type:result envelope, _invoke_cli kept the parse it made without reply_keys, so a quote repair that invented a key went through even when the caller passed keys. Parse that stdout again under the caller's keys and keep the raw text on refusal, as the inner-result path already does. Tests: bare stdout with and without keys; one test per site that threads reply_keys (continuation partial and combined parse, wider retry, _invoke_api into both, llm_invoke into excerpt repair, excerpt repair into _invoke_api); review item lists reject an extra key; a repaired falsify reply may carry receipt, a runtime reply surfaces, a finding excerpt. * fix(llm_invoke): close three reply-key gaps (#264) - A refused repair now raises ReplyKeyRejected, a JSONDecodeError at the original error. The API parse, the continuation parses and the wider retry no longer fall back to _extract_json_from_text for it. That scan could return an object quoted inside an excerpt, such as an empty review, and drop the real findings with no retry. - A CLI envelope (or the last stream event) whose result is an object is checked when the envelope parse itself needed quote repair. An unlisted key keeps the raw stdout, as a string result does. - _unknown_reply_key checks each object in a top-level list, since runtime._parse_llm_response unwraps a one-object list. A bare CLI stdout list is parsed again under the caller's keys. * fix(llm_invoke): stop scanning a failed quote repair for an object (#264) A quote repair that fails, or that the #216 quoted-key guard refuses, raised a plain JSONDecodeError, so _invoke_api scanned the text for any embedded object. An excerpt quoting x = {"findings": [], ...} then came back as a clean review and the real finding was lost after one call, with or without reply keys. Every failure after a repair was tried now raises RepairRefused, a JSONDecodeError subclass that ReplyKeyRejected derives from. After it, _fallback_reply tries only the reply's own leading object, so a cut falsify reply is still salvaged; after a key refusal it tries nothing. A failure with no repair tried keeps the full scan. Adds the CLI tests the cross-check asked for: a repaired envelope with only known keys keeps its result, and a raw newline is not a repair. * refactor(llm_invoke): drop a leading_only check the scan guard already covers (#264) * fix(llm_invoke): skip an object a later brace shows was enclosed (#264) The embedded-object scan now skips an object followed by a '}' that nothing after it opens. That covers a prose-led reply, an earlier syntax error and a leading object closed early by a quoted '"}]'. The falsify salvage refuses a repeated key and a verdict that is not the first key. * test(llm_invoke): pin the brace count behind the enclosed-object skip (#264) * test(reply_keys): check each caller's keys by calling it (#264) The wiring tests searched each caller's source for "reply_keys=...". A call that passed reply_keys=None with the old text left in a comment still matched. Each test now patches llm_invoke, drives the caller, and asserts every call received that caller's key table (by identity): the L1 provider, the subagent spawn, the test-assertion review, the runtime runner and the real falsifier. * fix(llm_invoke): keep a stream result event out of the bare-stdout re-parse (#264) When a CLI stream array's last result event has no "result" member, the event falls through to the bare-stdout branch. With reply_keys set, that branch parsed the whole stdout again and returned the stream array instead of the event, which main returns. Re-parse only when the parsed value is stdout itself, not an event taken from it. Test with and without keys. * fix(llm_invoke): end the scan at a '{' that fails inside a string (#264) Replaces the unmatched-'}' count, which code opening a brace after a quoted object could balance, and which refused a valid reply followed by prose quoting a '}' (a regression against main). - A '{' whose decode fails after reaching a '"' ends the scan, after the falsify salvage is tried; later '{' may sit inside its strings. - A decoded object followed by '}' or '"' is skipped. - An empty unknown key now counts as unknown (is not None). - ReplyKeys.items is read-only and left out of the hash. * fix(llm_invoke): refuse a reply inside enclosing JSON, duplicate keys and 3.12 trailing commas (#264) - _json_continues_after: a key-matching object is not the reply when the text after it closes a string or object it sits in, including a stray quote later followed by '}' or ']' (open braces in the quoted code). - _error_is_at_quote: ',}' and ',]' are structure on Python 3.12 too. - _loads_model_json: with reply_keys, refuse duplicate keys; drop the unreachable final raise. * style(tests): wrap a long test string (#264) * test(llm_invoke): pin trailing commas, the quote-first tail and ',]' (#264) * test(llm_invoke): pin the CLI empty-key check and ',]' after a string (#264) * style(tests): ruff format test_llm_invoke.py (#264) Format only. The next commit touches this file, and the lint gate runs ruff format --check on whole touched files. * refactor(daemon_state): build the static-rules list with extend (#264) Clears ruff PERF401 on this file, which the next commit touches; the lint gate runs ruff check on whole touched files. Same list, same order. * fix(llm_invoke): refuse cut excerpts-first replies, key canary and daemon (#264) - _text_after_refuses replaces _json_continues_after. After the object that opens a non-list reply, ',', ']', ')', ';', '=', '\', '//' and curly quotes count as continued code, not prose. A later whole object with the reply's keys also refuses the first: either could be the reply. - A '{' that opens the reply and fails to decode ends the scan, so a Python dict or JS object reply does not yield the JSON review it quotes. - _continue_truncated counts a partial as complete only when it parses whole. The fallback scan read a cut excerpts-first reply as an empty review and spent no continuation. - The inline canary review passes REVIEW_REPLY_KEYS, and the daemon-state questions pass their own key tables. A cut tail that starts with a word or a digit still reads as prose after a stop; the docstring and tests pin that limit. Two contract tests change: two matching objects now return None, and a reply that opens with '{a' returns None (F2 keeps its intent behind a prose prefix). * test(llm_invoke): each leading code mark and the daemon item keys (#264) Adds the right curly quote, ')' and '=' tails to the cut excerpts-first cases, and a repaired daemon conflict item with an invented key. * test(daemon_state): Q1 refuses a repaired reply with an unknown key (#264) * docs(llm_invoke): name the stop-cut tails rule 2 reads as prose (#264) The limit is any first character outside the refused set, not only a word or a digit. Pins '(', a list item and a single quote as well. * fix(llm_invoke): continue a length-cut reply even when the partial parses (#264) A partial cut by the output cap can parse whole when a quoted `"}], "findings": []}` closed the reply early, with the real findings after the cut. Always spend one continuation; an empty continuation returns the partial. Refs #264 Signed-off-by: Minxi Hou <houminxi@gmail.com> * fix(llm_invoke): measured stop-cut rule; scan refuses duplicate keys and literals (#264) - The text after a scanned reply object must start like prose or markdown (str.isspace whitespace), instead of refusing a fixed set of code marks. Chosen on a corpus of 57 cut tails and 32 valid trailing texts in three places: P1 lost 30/171 (was 156/171), valid replies refused 12/104 (was 14/104). The rule also applies after a list's object and a prose-led object, skips whole objects that follow, and refuses text opening more brackets than it closes or holding a string closing into JSON. - The scan decoder refuses a duplicate key, as _loads_model_json does. - After prose, a failed Python dict or JS object literal ends the scan at the first object with the reply's keys inside its braces. - A reply holding a '{' gets the JSON-correction retry even after prose, so a refusal costs one call rather than a failed review. - The canary review gets its own top-level keys; Q2Q3 checks the top level only, as on main, and the cost is documented. - Tests pin each _fallback_reply site against a full scan. Refs #264 Signed-off-by: Minxi Hou <houminxi@gmail.com> * fix(llm_invoke): offset the prose-led punctuation check; tests from mutation (#264) Mutation found that after a prose-led object the check of the text after its punctuation started at an offset into the slice, not the text, so `Here: {...}; = 1` was kept. Measure from the object's end. The corpus now loses 29/171 cut tails (was 30) and still refuses 12/104 valid replies. The leading-quote check is dropped: no quote starts prose, so the other rules already refuse it. New tests pin a duplicate key followed by a reply, literals whose quoted review is followed by prose, a spaced sentence mark, list punctuation, and the continuation site against a full scan. Refs #264 Signed-off-by: Minxi Hou <houminxi@gmail.com> * test(per_pass_usage): continue the length cut before checking its usage (#264) A length-cut reply is now always continued, even when the partial parses, so the usage test hands the continuation the rest of the reply instead of a complete partial. The transport fixture takes the reply text as an option. Refs #264 Signed-off-by: Minxi Hou <houminxi@gmail.com> * style(canary_gen): mark the dispatch fallback's broad catch as intended (#264) The file is now in this change, so lint_changed lints all of it. The catch is pre-existing and deliberate: any dispatch failure falls back to DELEGATED. Refs #264 Signed-off-by: Minxi Hou <houminxi@gmail.com> * style(tests): ruff format three test files this change touches (#264) Format only, no behaviour change. They were unformatted on main and lint_changed checks whole changed files. Refs #264 Signed-off-by: Minxi Hou <houminxi@gmail.com> * fix(llm_invoke): refuse duplicate reply keys on every path; bound the scan (#264) - _invoke_cli raises LLMInvokeError on a refused reply (duplicate key, invented key), in envelope, stream, object, string and bare forms, and parses stdout once. - loads_reply_json refuses duplicate keys and deep nesting; the reviewer JSON validator, canary review, factories, receipt and the canary provider in cli.py re-parse reply text through it. - A duplicate key gets its own correction message and spends the API correction retry; a repeat fails closed. - With reply keys only a leading or list object is taken. - Non-ASCII prose is a whitelist; a dict or object literal skips its strings when counting braces. - 256 KB and repair-count caps; RecursionError fails closed. - Echoed or restated continuations keep the partial; '$' no longer ends a sentence; '. Then' and '.keys()' are refused. - An object that opens a list must be followed by ']' or another item. Signed-off-by: Minxi Hou <houminxi@gmail.com> * test: reply refusals on the CLI, API and re-parse paths, and a CLI e2e (#264) test_reply_refusal_e2e.py reproduces the escaped-newline CLI attack that returned PASS at 071a9ea; it now fails closed. Signed-off-by: Minxi Hou <houminxi@gmail.com> * test: commit the stop-cut reply tail corpus and its measurement script (#264) Signed-off-by: Minxi Hou <houminxi@gmail.com> * test: kill the three surviving round 9 mutants (#264) The canary provider's re-parse, the brace skip inside strings in a failed literal, and a '.' ending a prose word only before whitespace or the end each survived the mutation run. Each now has a test that fails under it. Signed-off-by: Minxi Hou <houminxi@gmail.com> * fix: drop the json imports that loads_reply_json left unused (#264) canary_gen and reviewer_json now parse replies with loads_reply_json, so their json imports were unused (ruff F401), which the lint gate flags. Signed-off-by: Minxi Hou <houminxi@gmail.com> * style: ruff format receipt.py, format only (#264) receipt.py was not formatted on main. This branch now touches it, and the lint gate runs ruff format --check on whole touched files. Signed-off-by: Minxi Hou <houminxi@gmail.com> * test: keep the embedded raw-retention case on a reply prose may follow (#264) Since round 9 a caller with reply keys refuses prose before the reply and sends it to the correction retry (F2). The embedded style here led with prose and ran with one attempt, so all six cases failed. Prose now only follows the reply, which still keeps raw text that is not the parsed JSON; the prose-led refusal is covered in test_reply_refusals.py. Signed-off-by: Minxi Hou <houminxi@gmail.com> * style: ruff format test_writer_schema_raw_retention.py, format only (#264) The file was not formatted on main. This branch now touches it, and the lint gate runs ruff format --check on whole touched files. Signed-off-by: Minxi Hou <houminxi@gmail.com> --------- Signed-off-by: Minxi Hou <houminxi@gmail.com> Co-authored-by: Codex <codex@openai.com> * cli: report subagent assertion outcomes without changing verdicts (#343) Co-authored-by: Codex <codex@openai.com> --------- Signed-off-by: Minxi Hou <houminxi@gmail.com> Signed-off-by: Codex <codex@openai.com> Co-authored-by: Codex <codex@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Partially addresses #330: normally returned assertion-review phases now report a fixed host-owned failure when their typed outcome is missing, malformed, duplicated, or inconsistent with returned findings, rather than silently omitting execution metadata.
failed/reporting/inconsistent_outcomepair with unknown finding count.Scope and review
Two production files only. Defensive tests cover malformed markers, the validator cross-product, unchanged canonical outcomes, fixed metadata, output ordering, unbound validation, and exception propagation. No provider/exception text enters fallback metadata.
Three cycles across five local skills were independently reviewed by two reviewers. Composed onto merged #341 main
610b19410dc70239de8826b70cf178497741d629; exact reviewed composition bytes and unrelated AST preservation verified. Both independent final composed reviews passed.Validation
This is not a full closure of #330 and does not redesign interrupted/subagent paths, outlets, invocation reply keys, receipts, verdicts or credit accounting. Published as a draft for CI; not authorized for merge.