Skip to content

Never file a kernel's changed-identity report as a gate verdict (graph amendment 29) - #1140

Open
MaxGhenis wants to merge 1 commit into
mainfrom
fix/gate-identity-failure-not-persisted
Open

MaxGhenis wants to merge 1 commit into
mainfrom
fix/gate-identity-failure-not-persisted

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

A kernel that reports a changed identity no longer has that report filed as a gate verdict (graph amendment 29). This closes finding 1 of the independent review of #1125 (Subfleet job 20261007-081708).

The defect

run_graph derives every node key from kernel.implementation_hash() before any node runs. gates.battery@1 raises in run() once its binding registry, or a file in a binding's source closure, changed after the kernel was built. The executor turned every gate-kernel exception into a fail verdict (_failed_gate_result, amendment 7) and stored it under the already-derived key of the unedited registry. A later run with the correct registry on the same store was then served that fail as a cache hit.

Reproduced on origin/main (9f0a2b9): the binding was edited from the population observer after toy.calibrate, then the toy graph was run through toy.gates.terminal:

run1 gate outcome fail hit False key f5127ac27a48
run2 fresh correct registry: outcome fail hit True key f5127ac27a48
run3 cold store: outcome pass hit False key f5127ac27a48

The fix

  • New error type. microcosm.graph.KernelIdentityChangedError, a NodeRejectedError, is added to errors.py and exported. A kernel raises it when state its implementation_hash describes has moved since that hash was fixed.
  • Executor. except KernelIdentityChangedError: raise comes before the generic handler, for any role (executor.py:2903). For a gate this replaces amendment 7's verdict, and F4's "becomes fail", in that one case. The run refuses and nothing for the node is applied or stored. The existing _settle_failed_run keeps what the run had already completed, because sources are unchanged and those keys still name what computed them. The next run reuses that work and computes the gate afresh.
  • gates.battery@1. It raises the new type, which used to be ValueError, from implementation_hash() and from run(). run() now checks before it evaluates, before it returns, and when evaluation raises, chained to that exception. A closure file that can no longer be read or parsed counts as a change: before, it surfaced as a raw SyntaxError or FileNotFoundError and was filed as a fail. The message now names the source closure, which was Add shared transport-graph kernels: targets, ordered calibration, gates, diagnostics and export (NZ v0, G5b) #1125 review nit 4.
  • Docs and changelog. docs/graph-acceptance.md gains amendment 29, which is runtime-only: decl.py and kernel.py are untouched and the interface lock is unchanged. docs/transport-graph-kernels.md (Gates) is updated, and there is a changelog fragment.

Why the executor does not re-check hashes itself

The first draft of this branch also re-derived every gate's implementation_hash right after run() and refused on any change. The internal review executed a probe against it, and that version is not what ships.

Seven of the eight production gate kernels hash module files from disk on every call through source_hash: the UK gates, both export readbacks and geography.gate@1. A file rewritten under a running process does not change the code it already imported. That draft would therefore have aborted a correct UK build at its first gate after any save, checkout or pull that touched one of those files, at any point after key derivation.

So the check belongs to the kernel, which knows whether its identity is in-memory state. test_a_hash_that_moves_without_a_report_is_not_second_guessed pins that gates whose hash only re-reads files keep their current behaviour.

Invariants

Each is scoped to what the tests execute.

  1. No stale gate outcome. After any first run, a run with the unchanged kernel on the same store files the same gate receipt, and the same manifest key, as a run on a cold store. This holds for a gate whose identity is in-memory state that it checks (the battery's contract) and for a gate whose hash only re-reads files, under resume="auto" and "forbid", for every behaviour (pass, fail, raise), every move (none, before the gate, inside its run) and any identity value, including one equal to the original. The Hypothesis property with a same-store vs cold-store differential is test_graph_executor_gate_identity.py:523.
  2. Refusal is exactly a reported move. The first run raises KernelIdentityChangedError if and only if the kernel reports a change. Otherwise its gate outcome is the unperturbed kernel's, and amendment 7 is untouched for every other exception (:426, and the property).
  3. A refusal writes nothing for the refused node and keeps earlier records. No cache record exists under the gate's key or any later node's key, the CREATE record stays, and resume="require" then raises StoreMiss (:349, under auto and forbid).
  4. The report keeps its type for any role and is final even when the hash holds (:396).
  5. The battery check covers each window. A binding edit or a closure file changed (rewritten, unparseable, deleted) before the gate runs is refused, and so is a change made inside the gate's evaluation, whether it then returns or raises. A change made before the gate and undone during evaluation is caught by the check before evaluation (test_transport_graph.py:257, :355, :415). After each refusal, a fresh registry derives the original key and computes pass as a miss, every other node hits, and a cold store agrees.

Not covered. A move undone before the kernel's next check is not seen. Only gates.battery@1 checks its own identity. Seven other kernels' hashes read attributes set at construction:

  • uk_runtime/graph_kernels.py reads transforms and stages;
  • _engine in us_runtime/graph_kernels.py (two kernels) and frame/kernels.py;
  • _adapter_classes in frame/rules_kernels.py.

None of them is a gate, and none is re-checked, as before. Amendment 29 asks any kernel whose identity can move in process to check it and raise the new error.

Key movement

Executed by deriving node keys at 9f0a2b9 with and without this change, without running the graph, before rebasing onto #888. The rebase left this PR's hunks unchanged apart from one merged import line in executor.py.

  • Moved implementation hashes: gates.battery@1, whose own source changed, and export.prepare@1 and transport.package@1, which hash the battery's source. On the toy transport graph the gate nodes and toy.export.prepare move; every other node keeps its key.
  • No other kernel affected. No other implementation_hash in packages/*/src reads an edited file; all 41 definitions were checked.
  • Old records. A fail stored by the old code sits under a key this version never derives, so it is never served.

Red, green and mutants

  • Red/green. With executor.py from 9f0a2b9 (everything else at head), 12 of the new or changed tests in these files fail, including every closure-change case. All pass at head.
  • Mutants. Seven targeted mutants, each restored byte-for-byte afterwards. All seven are killed:
    • the base executor;
    • drop the executor re-raise;
    • drop each of the battery's four checks: before evaluation, before return, on exception, and the wrap of an undescribable closure;
    • drop the exception chain.
  • Review. An internal adversarial review ran four reviewers, each covering one area: executor paths, claims vs code, tests and mutants, and blast radius. Each finding went to an independent verifier. It produced 19 confirmed findings. All are addressed here, mainly through the redesign above, plus doc corrections: key movement, the dropped resume="forbid" repair claim, and the F4 and amendment 7 carve-out.

Tests run locally

Targeted files only, one at a time, with -p no:cacheprovider --basetemp=.hub-scratch/pytest -o tmp_path_retention_policy=failed. The environment is the shared .venv with this worktree's sources first on the path (hypothesis 6.167.1 and jsonschema 4.26.0 added from the lock into scratch). Whole suites go to CI.

File Passed
test_graph_executor_gate_identity.py (new) 7
test_graph_executor.py 92
test_acceptance_f_gates.py 6
test_graph_explain.py 20
test_graph_interface_lock.py 2
test_graph_parity_pins.py 11
test_graph_executor_source_identity.py 28
test_graph_acceptance_burndown.py 61
test_graph_manifest.py 43
test_transport_graph.py 24
test_transport_gate_kernels.py 31
test_transport_terminal_kernels.py 17
test_transport_binding_identity.py 32
test_transport_target_kernels.py 12
test_ordered_kernels.py 9
test_nz_spec_package.py 70

465 passed, 0 failed. Not run locally: the full suites, the US and UK engine jobs, and a wheel build.

Also run: ruff check and ruff format --check on the changed Python files (ruff 0.15.16, clean); tools/ci_test_plan.py verify (ok); tools/graph_acceptance_burndown.py --verify (ok). The acceptance suite files (test_acceptance_*.py) are not edited.

Owners

This PR adds no executor line above line 2903, so nothing it cites moves. Separately, packages/microcosm-build/src/microcosm/build/nz/benefit_unit_rule.json cites executor.py lines 636-643 and 1550-1573. #888 has already moved those functions to 638 and 1531 on main; I've raised that as a separate task.

axiom: n/a: graph executor runtime contract, no policy

🤖 Generated with Claude Code

…dment 29)

run_graph derives node keys before any node runs. gates.battery@1 raised
when its binding registry or a binding's source closure changed after the
kernel was built, and the executor filed that exception as a `fail` verdict
under the unedited registry's key, so later runs of the correct registry on
the same store were served the stale `fail` (#1125 review, finding 1).

- errors.py: KernelIdentityChangedError, a NodeRejectedError, exported from
  microcosm.graph.
- executor.py: re-raise it for any role instead of filing it; the run
  refuses, nothing for the node is stored, earlier work is kept.
- gates.battery@1: raise it from implementation_hash and from run, which
  now checks before evaluating, before returning and when evaluation
  raises; an unreadable or unparseable closure file is a change too.
- The executor deliberately does not re-derive hashes itself: most gate
  hashes re-read module files, and a rewrite under code already imported
  would refuse correct builds.
- Amendment 28, transport-graph-kernels Gates section, changelog.
- Tests: graph-level contract tests and a Hypothesis same-store vs
  cold-store differential; the review's three-run probe and closure-file
  changes through the real battery.

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant