Repository navigation
Fix codex session process-group teardown ordering - #10147
Merged
Merged
Conversation
added 4 commits
September 3, 2026 08:51
…on/neat-heron-138-stacked # Conflicts: # src/v1/stage0/src/process_group.rs
…on/neat-heron-138-stacked # Conflicts: # src/v1/stage0/src/process_group.rs
…on/neat-heron-138-stacked # Conflicts: # src/v1/stage0/src/process_group.rs
gunbai-bot Bot
added a commit
that referenced
this pull request
Sep 3, 2026
… proves it moves (#9981) * ROOT-2B: one cooked-literal escape, three declaration routes, and ownership by surface cardinality The evaluation-budget consequence projection needed a Rust string escape, and the rule was already authored twice byte-identically. It moves to extdeps.languages.rust.string_literal as escape_rust_cooked_string_literal_body: "cooked" because Rust also has raw literals this rule does not serve, and CR -> \r added because both predecessors escaped LF only, so an isolated CR passed through unescaped. Seven exact-output witnesses; removing the CR pass reds exactly the two CR witnesses and leaves the LF control green. The wet-actuator family now carries a declaration route per row -- PathIncluded, MacroUsePubModSpliced, PlainPubMod -- and each arm drives a different population rather than labelling rows: the plain-render basenames, the lib.rs root modules, and the filename exclusion. The retained population keeps its name and meaning, because stage0_executable_assembly consumes it as the retained-source classifier and a generated module has no retention row to find. That surfaced a module the assembly census could not name: a main-wet-generated module compiled by the host shell is neither partition-owned, nor an ordinary emitted mirror, nor hand-retained. WetActuatorLibModuleOwner is that arm, and it owns the macro-spliced dispatch module too -- which had been unowned all along. Ownership is now decided by counting ACTIVE SURFACES instead of walking an ordered chain of remembered pairs. The chain had a hole: an emitted-and-retained module resolved as retained because is_retained was tested first. That pair now refuses and stays enrolled as its own control. Partitioned-and-emitted is NOT two surfaces -- the partition row says which package compiles the mirror -- and the first run of the rewrite proved it by refusing both fixture partition rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Regenerate the stage0 mirror, enrol the escape witnesses, and seal the refusal behind a module THREE FIXES, ONE OF WHICH CI FOUND AND TWO OF WHICH REVIEW DID. CI's red was real and my commit caused it: adding a registration row changes gunbc.stage0_crate_layout_generated, and the SEED MIRROR of that module (gunbc_stage0_crate_layout_generated.rs) is what emit_lib_rs_from_paths reads to place the pub mod line. main_wet regenerates the .dag projection; it does not regenerate the Rust mirror, so the emitter kept answering from the old block and lib.rs drifted against it. Regenerated through the seed emitter and verified BOTH gates: first_generation_equal=true and fixed_point_equal=true. The cycle is worth recording because it bites once per generated module: the emitter that must place the declaration is BUILT FROM the mirror that must already contain it. Emitting before rebuilding produces a lib.rs missing the module, and that state is not silent -- the seed refuses to compile with E0433 on the constant's own path, which is the module-exposure wall doing its job. The seven escape checks were plain `fn`, so the discovery bridge never enrolled them: my runs were valid measurements of declarations no gate would ever execute. They are `test fn` now, individually rather than folded into one aggregate, so a regressed substitution still names its own cell. ServeBudgetRefusal was called sealed and was not. Rust privacy is module-scoped, so private fields plus one associated constructor left every line of cli_run -- and every descendant -- free to write the struct literal with four favourable values and no InterpError in sight, which is the fabricated refusal the carrier exists to forbid. It now lives in its own child module behind free functions, so the parent can render one and cannot assemble one. Free functions rather than methods for a second reason the closure host already recorded: an impl method has no DeclarationRef spelling, so it cannot be cited at all. The call site also classified twice -- once for the guard, once under expect. One match now produces one opaque consequence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * The registry guard is real and one step removed, and the product falsifier is named as steps TWO ROWS, BOTH WRITTEN FROM A MUTATION THAT WAS RUN. Deleting the evaluation-budget artifact from generated_artifact_registry and running the gate produced EXACTLY ONE finding, and it was .gitattributes drift: the merge-driver enrollment projection enumerates registry members, so dropping one changes its bytes. Nothing said the thing an author would want said -- that a committed file at a generated location is now adjudicated by nobody. So the guard holds by side effect, and an artifact excluded from that projection for any reason would leave the registry silently. The stall row records the rung that measurement supports, and its trigger names the direct capability rather than one more artifact. The product falsifier is recorded as STEPS, not as numbers. Every step is an existing entry point; the only new thing is the one-token perturbation of the authority, and that perturbation is what makes the other four discriminate at all -- unperturbed, they are green whether or not the seed reads the projection, because the value it would have chosen independently is the same value. Step 3 rebuilds before serving for the reason the merge driver already records about regen, and which this construction paid to learn twice: a binary that predates the change it must express reports a confident green for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Regenerate the rust-source-type-bindings mirror main landed without it Not my change and not my defect: #9949 corrected the String Proven row's prose in gunbc.rust_source_type_bindings and its seed mirror was never regenerated, so the drift arrived here with the merge. Reproduced locally after integrating main -- one file, prose only, no behavioural delta -- and regenerated through the seed emitter, verifying from the INSTALLED seed rather than the binary that emitted it: first_generation_equal=true and fixed_point_equal=true at this merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Close the rendering bypass, refuse duplicate wet registrations, and join the live populations THREE REVIEW FINDINGS, EACH A HOLE THE PREVIOUS FIX LEFT OPEN. Sealing the constructor stopped a caller assembling a refusal with a chosen code. It did not stop a caller holding a LEGITIMATE refusal from passing an encoder closure that ignores its argument -- the machine body took `json_string: impl Fn(&str) -> String` from the parent, so the code was still caller-selectable one level up. Construction and rendering are two boundaries and only one was closed. The child now calls the ancestor's private encoder directly, and the three functions narrow from pub(crate) to pub(super) so their spelling states the boundary they actually have. The wet route lookup returned a List, which is a decision to tolerate duplicates: surface counting read any nonempty result as one surface, and owner construction folded the routes to whichever came last. So the same basename registered twice was accepted silently, and two rows disagreeing about the route resolved by authoring order. WetRouteAbsent / WetRouteUnique / WetRouteDuplicate makes that a state the caller must answer, with both controls -- same route twice, and conflicting routes -- because they prove different things. No compile error stands behind either: a macro-spliced duplicate contributes no lib.rs line for rustc to reject. The fifth owner arm was fixture-executed and not live-consumed, and stage0_lib_declared_module_basenames was definition-only, which is the inert shape. One authority-to-authority join now requires every live wet registration to appear exactly once in the root-module population, resolve through the wet arm, and keep its exact route -- with the two specimens named so it cannot pass vacuously on a population that quietly lost a member. It does NOT claim the no-retention-row property: the live census binds the real roster, so that arm cannot be posed there, and it stays where it can be shown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Arm and invoke from one contract, and refuse a refusal that names another subject The host took the executed function and the budget limits as two independently supplied values, armed the evaluator from one and invoked the other. Today's call site passes the same `function` to both -- but that is a fact about one call, not about the interface, and the interface is what the next caller inherits. A process armed for one entry while evaluating another produces a refusal whose `entry` names the wrong function: a located diagnostic pointing somewhere true-looking and wrong, which is worse than no diagnostic. ServeArmedContract binds the subject and its limits once, before the listener serves. serve_contract_entry is the only way to name the evaluated function at this seam, and the startup announcement, the arming call and the invocation all read that one value; `function` and `serve_budget` are not consulted again. It is deliberately NOT the full ContractIdentity<Subject>. Surface and epoch have no host consumer at this seam, and minting fields nothing reads is the richer-type-name-as-safety move DESIGN section 4b names as cosmetic. The exceeded carrier is then checked against the armed contract before anything is rendered. If the interpreter reports a refusal for another entry, arming and evaluation had different subjects, so the path refuses instead of rendering a diagnostic about a function this process never armed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * The evaluation-budget consequence falsifier: a durable instrument for the whole transaction The five-arm receipt existed only as a procedure I ran by hand and a list of steps in a .dag row. A prose procedure is a second writable sequence beside the thing it describes, and the first attempt at it proved the hazard: I misread a `git fetch <sha>` line as evidence that the remote builder ignores the working tree, and killed a valid run over it. So the transaction is now one on-demand instrument that owns it end to end: bind a clean parent and a disposable detached worktree, build the candidate tool BEFORE perturbing anything, move exactly one authority literal (located in the source, not sed-and-hope), require exactly one attributed drift, regenerate through both generations, rebuild, supervise a real serve process in its own process group, drive a real thread-CPU breach over the committed fixture, judge status/code/entry/clock/limit and the moved-vs-former counts, terminate and wait, restore, and re-verify that the parent's HEAD, tree, status and worktree inventory are untouched. WHY RUST AND NOT A MODELED ACTUATOR, measured rather than preferred: the model has no vocabulary for a live child. extdeps.shell.exec runs one command to completion and std.process_termination describes a process that ENDED -- no handle, no readiness, no later termination. The two .dag-shaped options were to invent a managed-process substrate whose only consumer is this receipt, or to hide start/readiness/request/kill inside one opaque command where no fold can read the adjudication. The out-of-band-actuation tell is about bypassing a modeled operation that exists; here it does not. Nineteen typed refusal classes, because the remedies differ: a dirty parent is an operator problem, a wrong drift population is a defect in the bridge, and a failed teardown means no verdict may be reported at all. A cleanup failure never overwrites the experiment's own cause -- both are carried. TimedOut, Signaled, SpawnRefused and Completed stay distinct, which is the permanent form of a mistake this session made by hand: a loop that wrapped runs in `timeout` and read any nonzero exit as a false verdict reported two green witnesses as red. process_group.rs is an extraction, not a new capability: the Codex session driver already spawned into a group and signalled it. It is deliberately not reused as a generic supervisor -- its flow waits for the child before signalling, which is correct only because its protocol makes Codex exit, and would deadlock against a server that stays alive. Both files carry seed-growth justification and retention rows whose trigger names the capability that retires them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * The falsifier's first run refused, on a defect in the falsifier It compared `git worktree list` before and after and refused ParentCheckoutChanged. Correct by its own rule and wrong as a fact: this repository is shared, other sessions add and remove worktrees while the transaction runs, and none of that is something this instrument caused or can control. A check whose red is dominated by events outside its subject is not a wall -- it is a source of false refusals that trains a reader to ignore the real one. What the transaction owns is its own worktree, so that is what must be gone, and a survivor is now WorktreeCleanupFailed rather than a claim about the parent. The same run exposed a second defect it could not report: when cleanup refused, the match arm replaced the outcome wholesale, so a PASSING product transaction whose cleanup failed came back saying only that cleanup failed. The terminal now carries PassedWithCleanupFailure -- still not a pass, because an instrument that leaves residue has not finished, but the receipt survives beside the cause. Both are defects the instrument found in itself by running, which is the point of running it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Delete the prose procedure and the plan, now that both have executing homes The falsifier PASSED at c9ee149: moved value carried once, former value zero times, HTTP 500 from a real thread-CPU breach at 52176400ns against a 50ms bound, tree restored, worktree removed, parent untouched. So the two prose artifacts that stood in for it can go. evaluation_budget_consequence_falsifier_steps() hand-described the same transaction the instrument now performs and adjudicates. Two writable sequences for one procedure drift in either direction, and the one made of prose cannot fail. What replaces it is a pointer to the entry point. The plan document is deleted by the cut it planned, which is what it said would retire it. Its content has executing homes rather than a summary: the inverse quality order and the no-floor law are annotations on std.evaluation_budget's own arms; the terminal consequence law is the sealed host carrier plus the generated projection; the bridge lifecycle is the seed-growth justification and the two retention rows; the acceptance receipts are the instrument. Not mine, and checked rather than assumed: rust-unit-tests is red on this branch AND on main's own runs, on the same test (shell_service_unmodeled_output_key_refuses, 645 passed / 1 failed both places). The only difference my branch makes to that job is 142 ignored against main's 141 -- exactly the one #[ignore] test this work adds. #10017 names main's red as its subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Restore the prose procedure and the plan: the deletion was premature and unparseable Two independent things said the same thing about 3fd5892. The parser said it first, ten times. Deleting evaluation_budget_consequence_falsifier_steps() left the annotation block that EXPLAINED the deletion standing at end-of-file, and DESIGN section 4c attaches an annotation to the module item that FOLLOWS it. Prose about an absence has no subject, so required-witnesses-floor refused the parse phase — which also took namespace-wave-admission and floor down with it, since neither runs without an index. Reproduced locally with the same ten diagnostics before repairing, and the repaired tree reports `parse OK 4516 file(s) parse-clean`. The reviewing authority said it independently: the prose step list and the plan document delete only after the executable terminal has actually passed at the exact current head, and its five finding-5 seams are still open, so no such terminal exists yet. Both are restored verbatim from 3fd5892^ and will be deleted in the same commit that carries the qualifying receipt, not before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Close the five instrument seams: teardown, observation, readiness, port, provenance The review's finding 5 was not that the falsifier fails, but that several states it labels "completed", "ready", "settled" and "bound" do not yet mean what the labels claim. Each seam below is one such label. 1. TEARDOWN NOW ASKS ABOUT THE GROUP. terminate_process_group polls kill(-pgid, 0) to ESRCH after reaping the leader -- an unreaped zombie is still a member, so polling first would report presence for a process already dead -- and escalates to SIGKILL on the group whenever anything survives, INCLUDING when the leader exited cleanly. It returns ProcessGroupTermination { leader, residue, escalated_to_kill }; only GroupAbsent is success, and any errno that is not ESRCH is ResidueObservationFailed rather than absence, because an instrument that cannot see the group has not established that it is gone. The host's wall now asks group_is_gone() instead of reading the leader's status. PID reuse after the reap fails in the safe direction: a recycled pgid reads as presence, which refuses. 2. THE OBSERVATION ALGEBRA IS EXHAUSTIVE. spawn-refused / wait-failed-after-spawn / completed / signaled / timed-out-and-terminated / timed-out-with-termination- failure, with the teardown verdict carried on both timeout arms. WaitFailed is no longer mapped onto SpawnRefused -- that said a process which ran and could not be observed had never run, and the remedy is the reverse -- and it now tears the child down, since a failed wait established nothing about it. The stdout and stderr drain threads are joined on EVERY arm; they were dropped on three of four, discarding the child's own account of exactly the interesting cases. 3. READINESS IS A JOINT TYPED OBSERVATION, not a Boolean. ServeReadiness is Ready / ExitedBeforeReady / ReadinessTimedOut / ReadinessObservationFailed, and Ready requires all three of: the owned child still running, its exact announcement, and a connection to the port IT named. The old shape derived "timed out" versus "exited before ready" from the TERMINATION attempt rather than from the readiness interval, so a server that stayed alive and never listened was labelled as having exited. Teardown is now adjudicated separately from readiness, and neither can overwrite the other. 4. THE PORT IS THE OS'S CHOICE. 8300 + pid % 400 collides across pids and can be held by an unrelated local listener -- which, under a connect-only readiness wall, could SATISFY readiness and send every later disagreement to the subject's account. The child is now started with --port 0 and announces what it bound. That required a seed correction: gunbc serve announced the REQUESTED port, which is a fib whenever the OS chose, so it now announces listener.local_addr(). Every character of the announcement is pinned except the port -- entry, release revision, both budget arms -- so matching it binds the listener to the contract this run armed rather than establishing that something, somewhere, listens. 5. THE RECEIPT CARRIES THE PROVENANCE IT CLAIMED. The source has always said the pre-perturbation binary's digest identifies the producer that noticed the drift; it said so in prose while the receipt held no digest at all. Four are now recorded -- orchestrator, dry-gate gunbc, serving gunbc, generated artifact -- and the two gunbc digests MUST differ, since the binary embeds the generated consequence that just changed; equal digests refuse as SubjectBinaryUnchanged rather than letting the old producer answer for the new value. The dry-gate digest is taken while that binary is still the only one to have existed at that path, because the rebuild overwrites it in place. The HTTP half is typed rather than grepped: headers are retained and Content-Type must be application/json; charset=utf-8, the body is parsed by a flat-object decoder that REFUSES a shape it does not model instead of skipping it, and the subject's own stderr diagnostic must agree with the body on entry, clock, elapsed and limit. A substring search could not tell a field from the same text inside another string, nor a missing field from a malformed document. Checks clean and clippy --all-targets -D warnings clean. The qualifying exact-head run is next; the prose procedure and plan document delete in the commit that carries its receipt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Delete the prose falsifier procedure and the completed plan, with its annotation The second attempt at a deletion the reviewing authority had already accepted in substance. The first attempt (3fd5892) removed the function and left the annotation block that explained the removal standing at end-of-file, where DESIGN 4c has nothing for it to name; that is what refused the parse phase ten times and took namespace-wave-admission and the floor down with it. Prose about an absence has no subject, so it goes with the thing it described. evaluation_budget_consequence_falsifier_steps() hand-described the same perturb -> gate -> regenerate -> build -> serve -> judge -> restore transaction that v1_compiler.evaluation_budget_consequence_falsifier_host now performs and adjudicates. Two writable sequences for one procedure drift in either direction, and the one made of prose cannot fail. DESIGN section 6 says a measurement worth re-deriving is worth an entry point rather than a description of one; the entry point exists, so the description is deleted rather than demoted to documentation. The plan document is deleted by the cut it planned, which is what it said would retire it. Its laws have executing homes: the inverse quality order and the no-floor law are annotations on std.evaluation_budget's own arms, and the terminal consequence law is the sealed ServeBudgetRefusal carrier plus the generated projection. Nothing links to it. This lands FIRST rather than last, against the sequencing I was given, for a reason that sequencing did not anticipate: the receipt binds subject_commit and subject_tree, so a qualifying run can only ever describe the tree it ran on. Were the deletion to follow the pass, it would produce a different tree and invalidate the very receipt it was waiting for. So the deletion is made first and the qualifying run is taken ON this head. If that run does not pass, this commit is reverted rather than kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Reopened seams 1, 2 and 5: pinned-identity teardown, bounded drainage, whole-document JSON Three real defects, all mine, all found by review rather than by execution. SEAM 1 — I HAD WRITTEN AN ANNOTATION THAT WAS FALSE OF ITS OWN CODE. The previous terminate_process_group reaped the leader and THEN signalled the numerically matching group if anything appeared present. Once the leader is reaped its pid is free for reuse, so that signal could land on an unrelated group that merely inherited the number: wrong-subject actuation, not a safe over-approximation. The comment beside it said PID reuse "fails in the safe direction", which is true of REFUSING on presence and false of the signalling the code actually did. The annotation described the design I had in mind rather than the one I wrote, and being confident it was safe is what stopped me re-reading it. The rewrite makes the order the safety argument. An unreaped leader -- running or zombie -- keeps its pid allocated and therefore keeps the group identity pinned, so every signal now happens BEFORE the reap, and after the reap this function neither signals nor observes the group by number: a survivor is refused to the caller instead. Membership is read from /proc rather than inferred, because a signal cannot ask who is in a group, only act on everyone who is; the fields are located from the last ')' since comm may itself contain spaces and parentheses. The leader is excluded from the residue because it is adjudicated separately by its exit status, and residue answers the different question of whether the server's HELPERS outlived it -- the ones that would still hold the port next run. SEAM 2 — THE DRAIN COULD HANG EXACTLY WHERE IT MATTERED. observe joined read_to_string threads unconditionally. A descendant that survived teardown still holds the write end of the pipe, so EOF never arrives and the join blocks forever -- on precisely the arm where teardown had already failed and the child's output was the thing worth reading. Draining is now bounded and its result is typed: StreamOutcome is Closed or Unfinished, so a truncated read can never be judged as a complete one. WaitFailed becomes WaitFailedAfterSpawn and carries the typed ProcessGroupTermination rather than a formatted debug string; the timeout arms already preserved that algebra and this arm was throwing it away into prose. SEAM 5 — THE DECODER TOOK THE LAST ANSWER AND IGNORED THE REST OF THE DOCUMENT. A body with two `code` members silently let the later win, and bytes after the closing brace were never looked at, so a valid-looking prefix could carry anything at all behind it. Duplicate members now refuse by name -- including a duplicate that changes type, which would otherwise put one copy in each map and collide in neither -- and the whole document must end after the closing brace. Those two walls ship with their REDs, as ordinary --lib tests rather than inside the tens-of-minutes #[ignore] transaction: a duplicate that would have won, a cross-type duplicate, trailing content, an unmodelled nested shape, plus the positive control and a trailing-whitespace case so the wall cannot fire on well-formed responses. All six execute on the merge path. 6 passed. clippy --all-targets -D warnings clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * An external prune of the experiment's worktree is not the subject's build failure The falsifier refused at 17f3808 with CandidateProductBuildFailed, and the build had genuinely failed: the seed's build script hard-requires `git rev-parse HEAD` to stamp the source commit, and inside the experiment's worktree git had stopped working. But nothing about the subject caused it. A linked worktree is a directory whose `.git` FILE points at an admin directory under the common `.git/worktrees/`. Any process sharing the repository can delete that -- `git worktree prune` does, routinely -- and on a machine where many sessions share one clone it is not even unusual. The directory survives; only its registration dies. Measured after the refusal: the worktree directory was still there, its `.git` still read `gitdir: .../worktrees/ebc-falsifier-305713`, and that directory was gone, so `git -C <worktree> rev-parse HEAD` answered `fatal: not a git repository`. The paired WorktreeCleanupFailed ("is not a working tree") is the same event seen from the other end -- two symptoms, one external cause. So the instrument charged the subject for something the subject did not do, which is the exact failure this whole construction exists to prevent. The three build sites now ask whether the experiment's own environment was still intact BEFORE attributing the failure, and refuse as ExperimentWorktreeUnregistered when it was not. The ordering is the point: ask first, attribute second. The check resolves the `.git` pointer rather than asking git from inside the worktree, because a call from inside fails for this reason and for several unrelated ones and therefore cannot discriminate between them. This is a real hazard rather than a one-off: the shared clone here carries over 130 worktrees, and any session's prune reaches all of them. The durable fix is a plain clone, which no prune can unregister; that is a larger change than this PR should carry, and the typed refusal now names the condition when it recurs. clippy --all-targets -D warnings clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * One authority for the budget-refusal diagnostic text review 59017 found the template "serve: refused {} on {} clock: elapsed_ns={} limit_ms={}" authored twice: once by the emitter and once by the falsifier, which re-minted it to match serve's stderr. One fact in two places -- an edit to the emitter silently stopped being checked by the falsifier. The obvious consolidation was unavailable and it is worth saying why. The falsifier holds four scalars parsed from the response body, not a ServeBudgetRefusal, and that type deliberately has no constructor over loose values -- the whole point being that a boundary which merely CHOSE the same text must not be able to render a refusal. Handing the falsifier a from_parts would have removed the duplication by reopening the hole the type exists to close. So what is shared is the TEXT RENDERING over scalars, not the refusal. A caller holding four scalars can now describe what a refusal WOULD say and still cannot make the boundary emit one. One property is deliberately given up: the falsifier's private copy would have caught an unannounced change to the emitter's template. That was never its subject -- it checks that the body's fields agree with the printed line -- and a template checked against a copy of itself is a change detector rather than an oracle (§5). The §3 fork is the more expensive of the two. Verified: cargo clippy --all-targets -- -D warnings green. * Teardown: git forgetting the worktree is not the directory surviving The falsifier reached its verdict at 68f6667 and reported PassedWithCleanupFailure: `git worktree remove --force` exited 128 with "is not a working tree" while the directory was still present with its contents. The report was CORRECT -- cleanup really had not happened and the directory leaked -- so this is not a false alarm being silenced. The cause is that this is a SHARED checkout. A `git worktree prune` run by any other session deregisters this instrument's worktree mid-transaction, after which `worktree remove` refuses for a directory that still exists. That is an environmental fact about the registry, not a fact about the subject under test -- the same distinction `ExperimentWorktreeUnregistered` already draws on the build path, now drawn on the teardown path instead of being re-invented as a second notion of the same thing. The obligation this function owes is that OUR directory is gone and no registration of ours survives. Where git has already forgotten the path, finishing that obligation is ours, so the tree is removed directly. This does not widen the refusal. The detection is narrow -- nonzero exit AND git's specific "is not a working tree" text -- every other failure still refuses with the observation attached, and every post-condition below runs unchanged: a surviving directory, a surviving registration, or a mutated parent checkout each still refuse. The path removed is this run's own generated scratch directory, so no other worktree is addressable by it. Verified: cargo clippy --all-targets -- -D warnings green. * Three absorbing fallbacks in the falsifier host become typed refusals review 59127 found three places where this host answered with a default instead of refusing. All three are the §5 tell -- the failure arm widened rather than stopped the line -- and all three are in an instrument whose entire purpose is to be believed when it says the consequence moved. 1. ABSENT RESPONSE MEMBERS. `fields.string("code").unwrap_or_default()` and its four siblings collapsed "the body did not carry this member" into "" or 0, and fed that into the comparisons. The refusal that followed named the wrong fact: `code ` rather than "the body carried no code", and a defaulted limit_ms of 0 invents a number the server never sent. Now `ResponseMemberAbsent { member, body }` at the extraction site. 2. GIT OUTPUT THAT WAS NEVER COLLECTED. `stdout_of` answered `String::new()` for every non-Completed observation, so a timed-out or signalled `git rev-parse` produced an empty HEAD that then compared unequal and was reported as ParentCheckoutChanged -- a located, confident, WRONG culprit. Every site that DERIVES a verdict from git's stdout now goes through `git_stdout`, which refuses with `GitObservationFailed { what, observation }`. `stdout_of` remains only where `completed_zero` has already adjudicated the observation on the line above. 3. AN UNDECLARED CARGO. `env::var("CARGO").unwrap_or_else(|_| "cargo")` silently PATH-searched. This instrument's claim is same-identity evidence -- that the binary it judges came from the source it perturbed -- so building with a different toolchain than the one running the test is exactly the substitution it exists to detect. Cargo always sets CARGO for a test process; its absence means the run is not shaped the way the receipt assumes, and now refuses. Verified: cargo clippy --all-targets -- -D warnings green. * Dissolve stage0_wet_route_is_present into the coproduct it was standing in front of review 59152 found this Boolean predicate re-encoding which WetRouteResolution variants constitute presence, and it is right that the shape is wrong even though the two behaviours agreed. A predicate answering "is a wet route present" and a match answering WHICH resolution it is are two encodings of one fact (§2, "model a concept once"), and the predicate is the one that can drift: it could admit a route the match then calls absent, or exclude one the match would have owned. At the owner-resolution site the guard was followed immediately by a TOTAL match over the same value, so it decided nothing that the match did not decide again. The coproduct is now matched directly at both sites. At the owner resolution the ABSENT arm carries the rest of the chain -- extracted to `stage0_resolve_non_wet_module_owner` rather than inlined, because the ordinary retained/emitted decision has nothing to do with wet routes and burying it inside a wet-route arm would make a reader answer a question about actuators to find it. Guard and match can no longer disagree, because there is no guard. Where the active surfaces are counted, a DUPLICATE route still counts as a surface, so the cardinality answer stays a property of the module; the ambiguity is refused where the owner is decided rather than being dropped from the count. The rationale moved above the declarations: §4c admits annotations at module-item grain only, and my first attempt put them inside the bodies, which the compiler refused with 6 diagnostics. Verified: the module compiles with 0 blocking errors, and required-regen reports first_generation_equal=true. * Close the falsifier's observation and teardown integrity findings Six findings from the reviewing authority's ruling on this head. All six were verified against the tree before being accepted; all six were real, and two of them are defects in my own earlier repairs rather than in the original code. 1. THE POST-REAP SIGNAL PATH STILL EXISTED. `await_serve_ready` called `child.try_wait()`, which CONSUMES the exit status, and then called `terminate_process_group`, which signals the pid and the group by number. That is the exact ordering `terminate_process_group`'s own annotation forbids, at a call site that violated its precondition -- so the seam I described as closed was open at one entry. Readiness now observes the leader through /proc without reaping (`observe_leader_without_reaping`), so the identity stays pinned and the reap remains last. A pid that has left /proc unreaped refuses rather than being signalled. 2. `GroupAbsent` COULD BE MANUFACTURED FROM AN INCOMPLETE OBSERVATION. `process_group_members` skipped a numeric process on any stat read error, malformed syntax, missing field, or unparsable pgrp. A process we cannot inspect might be in the target group, so skipping it let an unreadable /proc produce an empty vector and then "the group is gone". Only NotFound -- the process genuinely vanished mid-scan -- is still skipped; everything else refuses. 3. THE TERMINAL'S SUCCESS ARM WAS A PREDICATE OVER ONE FIELD. `group_is_gone()` consulted only the residue, so a leader that had TIMED OUT could coexist with a successful teardown, and every caller had to remember the omission. `ProcessGroupTermination` is now `Settled | Unsettled`, where `Settled` is unconstructible without BOTH an adjudicated leader and a completely observed absence. 4. THE STREAM TERMINAL COULD LIE, AND THE SERVE PATH COULD HANG. The reader mapped EOF and read failure to the same bare `return`, so a broken read became "complete output"; `snapshot()` answered "" for a poisoned lock. The reader now records WHY it stopped, `finish_within` JOINS rather than merely observing the thread finished, and `StreamOutcome` distinguishes Closed / ReadFailed / DeadlineExceeded / ReaderPanicked. The serve path's unbounded `finish()` is gone: teardown is adjudicated BEFORE the drain, because an unsettled teardown is exactly when a descendant still holds the write end -- draining first blocked forever on the arm that was supposed to report the failure. 5. MY OWN `git_stdout` REPAIR WAS INCOMPLETE. It accepted any `Completed`, ignoring exit code and stream terminals, so a nonzero `rev-parse` with empty stdout still produced a successful empty read -- the very failure the function was added to remove. It now requires exit zero AND both streams complete, and `completed_zero` carries the same join. 6. THE JSON DECODER DID NOT REQUIRE MEMBER SEPARATORS. It consumed a comma wherever it saw one and accepted a fresh key with none before it, so `{,"a":1}`, `{"a":1,}`, `{"a":1,,"b":2}` and `{"a":1 "b":2}` all parsed. A position state machine now requires exactly one separator between members. The body is also decoded with `String::from_utf8` rather than `from_utf8_lossy`: the response declares charset=utf-8, and repairing invalid bytes into U+FFFD would let a broken body be compared as though it agreed. Also narrowed, not repaired: the receipt's digest fields claimed "which binaries actually answered". `file_digest` hashes files at paths and does not inspect the image a process loaded, so the heading claimed process-incarnation identity on evidence establishing on-disk artifact identity. The claim is now stated as what it is; binding the loaded image is named as the stronger thing not claimed. EXECUTING EVIDENCE, on the ordinary merge path rather than in the #[ignore] falsifier -- which exercises the happy path and so cannot establish that these arms refuse. Eight `--lib` tests: malformed separators (with the valid object as positive control), zero-exit-with-truncated-stream, nonzero git with empty stdout (with an ordinary git read as control), a failing reader, a descendant holding the pipe open, an unadjudicated leader, an unobserved residue, and a dead leader observed unreaped while its pid is still addressable. Verified discriminating rather than assumed: restoring the old `completed_zero` widen turns exactly one of these red and leaves the rest green. * The /proc enumeration error refuses too, and the refusal is now authorable review 59209, BLOCKING and correct. I repaired the `stat` read failure in the previous commit and left `Err(_) => continue` on the DIRECTORY ENTRY one line above it -- the same class, in the same function, under a comment I had just written saying every non-disappearance failure must refuse. An incomplete enumeration could still produce an empty vector and therefore `GroupAbsent`, certifying a teardown from a scan that never finished. It now refuses, naming the incompleteness. THE MORE USEFUL HALF OF THIS COMMIT. When the reviewing authority asked for a red proving that an unreadable or malformed /proc entry yields observation failure rather than absence, I reported that I could not author it: the real /proc cannot be made to return a malformed `stat`. That was a claim about this function's SIGNATURE, not about the class -- and DESIGN §4b says to ask whether a check's red is authorable BEFORE concluding it is not, because a missing harness is a next-rung trigger rather than a ceiling. So the scan takes a root. Production passes `/proc` and nothing else ever does; the parameter exists so a fixture can stand in, and it is not a policy knob. Three tests now execute on the merge path: a well-formed fixture scan that finds its members and reads a zombie's state (the positive control, without which every refusal below would pass on a scan that refused everything); three unparsable forms -- no comm terminator, too few fields, unparsable pgrp -- each refusing; and the one arm that may still be skipped, a process whose stat is NotFound because it genuinely vanished mid-scan. Verified discriminating rather than assumed: restoring the skip on a missing comm terminator turns the refusal test red, and its failure names the case. * Close the remaining observation-integrity findings: identity, sealing, drainage, JSON Six open items from the reviewing authority's ruling on the previous head. Each was verified against the tree first; each was real. 1. THE POST-RELEASE SIGNAL PATH SURVIVED MY OWN REPAIR. I had fixed readiness to observe without reaping, but its LeaderVanished and ObservationFailed arms both became ReadinessObservationFailed, whose caller then called terminate_process_group -- so the path still refused only AFTER signalling the released number. The generic observer's WaitFailed arm had the same shape, under a comment saying the failed wait had established nothing about the child. A caller convention could not fix this, because the convention is what kept failing. `PinnedProcessGroupIdentity` is now the only way to reach a signal: it can be produced solely by an observation that found the process present and unreaped, and `terminate_process_group` takes it instead of a `u32`. Losing the identity no longer compiles into a signal. `ServeGuard::drop` obeys the same algebra -- it reaps its own handle and sends nothing -- because a drop cannot report, so an unprovable identity there would be the quietest possible wrong-subject actuation. 2. `Settled` WAS STILL WRITABLE. The builder joined the facts correctly, but the enum was `pub(crate)`, so any module could construct the success arm directly -- and my own positive test did exactly that, which means the test established what `build` happens to do rather than that bypassing it is impossible. The type is now a struct with a private state, so `Settled` is reachable only through `build`. 3. COMPLETE-OUTPUT AUTHORITY AT EVERY CONSUMER, not just in the helper. `stdout_of` is DELETED rather than fixed, because leaving the unsafe spelling beside the safe one is how call sites keep finding it. `completed_drained` is the single extractor. The source-status check no longer reports a git timeout as SourceCheckoutNotClean with an empty status; the dry-gate verdict no longer counts drift lines out of a truncated prefix; the deregistered-worktree exception no longer decides on an incomplete stderr. 4. THE /proc SCAN'S ITERATOR ARM. Split into a production wrapper (`process_group_members_at`) and a testable core over a FALLIBLE entry source, with the stat parser separated and made strict -- it now rejects a record whose own pid prefix disagrees with the directory it came from, a multi-character state field, and a missing comm opener. 5. THE DECODER STILL ACCEPTED NON-JSON: raw control characters inside strings and leading-zero numbers. Both refuse. The diagnostic previews sliced by BYTE offset, so a malformed multibyte document could panic the instrument while it was building the refusal meant to describe it; previews now count characters. 6. THE SEED-GROWTH AUTHORITY WAS FALSE. It named `await_listening`, which does not exist -- the declaration is `await_serve_ready` -- and the roster had not been re-censused for anything added since. It is now generated from the two modules' actual top-level declarations: 51 rows, up from 17. Also corrected: the termination comment describing "three fields rather than one enum" above an enum, the stream heading describing an `Unfinished` variant that no longer exists, and two digest comments still claiming producer identity after the receipt narrowed to on-disk identity. EVIDENCE, all on the ordinary merge path: 9 arms in the falsifier host and 8 in process_group. New this commit -- a reader that actually PANICS when polled, so the ReaderPanicked arm executes `join().is_err()` rather than being constructed by hand; invalid UTF-8 response bytes; control characters and leading zeros; a multibyte malformed document that must refuse without panicking; a failed directory entry; a non-NotFound stat read failure; a non-numeric entry ignored; a stat declaring a different pid; and the positive control the ruling named -- a clean scan with no members reporting an empty group, without which a scanner that refused every empty result would pass every RED. Full suite: 715 passed, 0 failed. clippy --all-targets green. * Review 59301: the evaluation budget's nanosecond fold carries the nanosecond carrier evaluation_limit_nanos returned a bare Int and effective_nested_limit_nanos took and returned two of them, in the module that imports Nanosecond from std.measure and cites it as the canonical exact elapsed-time carrier. A wall remainder and a cpu remainder type-checked against each other in the one place that must never substitute them silently: the two clocks are separately armed and separately assessed, and the smaller of a wall remainder and a cpu limit bounds neither. Both now traffic in Nanosecond; the comparison unwraps at the comparison, which is the idiom std.measure's own comparator uses. Also finishes review 59257: two probe_route_is call sites survived at :640-641, which is what the floor lane refused on (function not found in scope). The wet lib-module population is three rows, not two, since gunbc_file_transport_generated carries PlainPubMod — asserted by identity beside the count, and confirmed by execution: required-regen reaches first_generation_equal=true only because the emitter renders that basename's ordinary pub mod line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * File the duplicate-match-arm compiler gap as its own failure-mode row The class is the DUAL of exhaustiveness, not an instance of it: exhaustiveness asks whether the union of the arms covers the domain, and gunbc does refuse when it does not — the same file refused loudly for a MISSING WetActuatorLibModuleOwner arm earlier in this PR. The unasked question is whether each arm contributes an inhabitant no earlier arm already covers, and a duplicate arm makes the union no larger, so an exhaustiveness check is green by construction on exactly the source that carries the defect. The measured grain is narrow and stated narrowly — two syntactically identical closed-variant constructor heads at one match — because the general subsumption question is a ratchet and claiming it here would let the narrow wall be cited as coverage for the broad class. Rung found at: outside the ladder. Ceiling 3. Trigger names the capability: a match-arm usefulness judgment that REFUSES the covered arm, with a discriminating red, a positive control, and a retention control over adding a variant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Enrol the generated-artifact registry-membership stall in all_guarantee_stalls The row was declared and never rostered, so neither every_live_stall_is_below_its_ceiling nor every_live_stall_names_a_next_rung_trigger ever ran over it — a stall carrier whose own consumers could not see it. Found by the four-tree merge census the authority asked for, not by reading the diff: main's split of guarantee_rung_drop.dag into guarantee_stall.dag merged as a RENAME, so git carried the row's DEFINITION across and the roster line, which lived in a list main had rewritten, did not come with it. The census classified that path ExactSideSelected_theirs, which is what sent me to look. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0176SNKT2W9dH3kRX5jp3Nga * Fix codex session process-group teardown ordering (#10147) * Fix codex session process-group teardown ordering * Refuse silent natural-exit overruns * Document process-group termination grace policy --------- Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com> --------- Co-authored-by: Brian Searls <briansearls1@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: gunbai-bot[bot] <289086189+gunbai-bot[bot]@users.noreply.github.com> Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.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
1 participant
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.
Stacked on PR #9981 and intended to merge after it.
Summary:
Verification: