Repository navigation
Conversation
trissim
commented
Oct 21, 2026
trissim
left a comment
Contributor
Author
There was a problem hiding this comment.
agent-comms/review: failure at c73e229.
I ran the touched test files at this head (PYTHONPATH=src timeout 300 pytest -n 0 -o faulthandler_timeout=60): 11 failed, 118 passed. Then I ran the same 11 at the merge base a8da1a76d: 4 fail there too, so they came from main, and 7 pass. The other 7 failures came from this PR. "4 passed" in the body is a cherry-picked proof, while the PR's own touched files go red underneath it.
src/agent_comms/todos.py:3395-3398checks unit competition against the assignment's acting assigner, not the order's commander. Core's watcher reissue now refuses its own commander's orders:worker is assigned by lead on open order order-P1 (P1). Five tests intests/test_holder_standing_rules.pyfail here and pass on the base: the fourtest_blocked_holder_*tests andtest_startup_heartbeat_enforces_and_publishes_both_new_rule_lines. That's the standing rule that reissues a blocked unit's orders, and it's dead. IDEN-1: one field answers both "who acted" and "whose order competes". Fix: compare the open Todo'screator/creator_createdwith the authority of the order being assigned (current.creator_identity, which is theassigner.commanding_identitythat you already declared).Assignment.parentstays the actor record and never becomes the competition key.src/agent_comms/todos.py:2216-2217:GoalRef.authorize_orderthrows away the actor and authorizes from the goal reference's incarnation. "Who may change this order" is now answered by whichever goal the operation carries.test_commander_run_rules.py::test_clean_transfers_keep_history_and_third_owner_requires_a_tracefails withTodoConflictattodos.py:2217, and it passes on the base. Fix: delete theGoalRefoverride and keep the single baseactor.require_may_change(current). Make the deserialized actor carry its authority, which means resolvingCommanderForkIdentitybeforeapply. Don't move authorization onto the goal reference.src/agent_comms/todos.py:2721:SupersededVerdict.preparecallsheld_for_commander(self.actor). With a fork,self.actoris aCommanderForkIdentitywhose name and birth are the fork's. The query is keyed oncreator=?, so it finds nothing, and everyjudge supersededa fork makes is refused with "The replacing problem is not held for this commander." No test covers it. Fix:held_for_commander(self.actor.commanding_identity). Then add the fork case totest_fork_and_parent_judge_show_same_unjudged_edits.src/agent_comms/todos.py:3416:resolve_ordercallsself.get(problem). That raisesUnknown todo: P2.where it should refuse withworker holds no order on P2.test_held_problem_orders.py::test_a_problem_not_held_by_the_unit_is_refusedfails, and it passes on the base. Fix: select through the newheld_inowner (held_by(commander, unit)filtered onid) so that one query owns "this unit holds this order".- The body claims "4 passed". Rerun every touched test file and report the full count, with each failure that comes from main named against the base run.
Main-origin failures, so not yours: test_send_and_check_accept_a_deletion_check_on_fresh_main, test_watcher_git_and_github_reads_leave_wire_free, and both test_unowned_checkout_work cases.
Contributor
Author
trissim
added a commit
that referenced
this pull request
Oct 21, 2026
P440: keep the assigner typed so reissues and fork verdicts act under their commander (supersedes #2540)
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.
P440 — parent reads and registered fork actors
Implements replacement order
b08c157fc51a. Supersedes the child-owned-order interpretation in #2490. Draft until the exact-head hosted checks are complete, then ready for the required merge queue. Installation and installed acceptance are not claimed.Owners and consumers
CommanderContext.commanderremains the commanding parent. Its Todo, unit, history and edit readers never change identity. One excluded typedactorfield containsCommanderForkCaller | NoThreadCaller;CommanderVerb.applyregisters then binds it once. Model copies preserve it._caller,commandingand the context'sretirecompatibility members are absent.ToolCaller; explicit fork retirement does not erase open Todos. Posting does not retire.ThreadCallernow resolves command authority and acting identity separately. A registered fork's authority is derived from its registry parent before Todo transactions; only the actor incarnation is serialized. Recorded-incarnation equality stays birth-based when a caller carries this authority capability.ProblemOrderGoalActionowns one shared binding implementation and the existing assigner field; both creating and reordering keep parent ownership while recording the authenticated actor. Native mutation arguments are attributed by their authenticated caller, not a supplied actor. Assignment authorization uses the already-bound request's commander; competition still compares the original assignment's actual assigner inside the Todo write transaction.Assignment.sql_fieldobtains wire keys through the declared dataclass/codec fields.TodoStore.held_in, usingTodo.unit_indexand_open, is the sole open-unit selection forholds_pending,for_goal,require_unit_in,held_by,correct, andfire. Transactional callers pass an existing DB; readers use their snapshot. No registry read occurs in the Todo write transaction.current_assignments(). It compares the stored assignment actor with each receiving registered fork. The old child-creator query is gone. This predicate change was explicitly authorized in bus message 15581; no new capacity query or machinery.ProblemNumberowns parsing and spelling. Allocation rows contain only issued, unsent numbers; created IDs stay onTodo.id, with a numeric index.ProblemAllocation.reserverejects malformed IDs with a namedTodoError. The existing carry preserves old issuance/sequence and removes only allocator copies of created IDs.Patterns removed: IDEN-1 (commander vs actor), IDEN-5 (competing assignment ownership), IMPL-5 (repeated held-unit selection), MEMB-5 (Assignment wire shape restatement), TIME-3 (deleted context aliases).
Behavioral qualification
Only the four requested tests are run locally, in the background, with the existing shared Python,
PYTHONPATH=src,timeout 300,-n0, and-o faulthandler_timeout=60:test_registered_fork_sends_parent_problem_with_fork_actor: real commander parser/Core path on a copied bus sends parent-created P40, keeps creator/request = lead, records assignment/message actor = fork-one, preserves copied-context reads, counts the live fork, refuses a competing fork, and preserves the Todo through post/retire.test_fork_and_parent_judge_show_same_unjudged_edits: retained external edit through the real judge command: 1 unjudged / 1 unjudged.test_open_unit_consumers_agree_on_one_assignment: pending/goal/competition agree, including reused name with another birth. SQLite selects the assignment index by owner, owner birth, and state.test_malformed_problem_ids_are_refused_without_writes: includes P0 and X1, names the refused ID, leaves no issued rows.Exact remotely verified head
c73e22976acf1734c1173c6f2d74bb8b50f57ed1: 4 passed in 2.70s. Net production delta against merged main: +142 (303 added, 161 deleted). Branch mechanical_callercount: 0; the live main checkout still has 5 because this repair is not merged/installed. No full local suite or ratchets.The first run exposed the stale capacity predicate, now fixed under 15581, and an unescaped regex in my refusal test, now a literal ID check. Hosted checks on the earlier head passed smoke/native-scripts but failed debt-src/debt-tools. My lint/binding/new-file formatting findings and exception-to-bool parser path have been corrected; the current head's hosted success is not yet confirmed. The old-table StringIdentity finding is a confirmed detector error, assigned by the commander in 15723 to a separate measure-owner PR. This PR deliberately keeps the historical SQLite name unchanged and introduces no old-row model. It remains draft until that owner fix lands, main is merged, and all five hosted implementation checks pass.
Source evidence and limits
Existing refactor-audit
Package/Repositorytooling parsed current roots: 460 Core modules, 23 tools, 463 tests, 283 Toad modules; zero parse omissions. The inventory enumerates declarations, bases, imports, writes, decisions and consumers. One constructor omission intest_bus_fileswas found and migrated. Dynamic binding and codec internals are outside this syntactic proof. NRA's newer parser could not importtree_sitterin the mandated existing Python; it is not counted as a successful run and no environment was created.A mistaken whole-file formatter pass was removed by a one-shot converter which reconstructed 49 successful retained edit calls over the Git index and asserted identical ASTs for all 30 files before restoring their pre-formatter layout. The converter was deleted after its single successful run. Only known-successful file edits were reconstructed in this isolated checkout. No failed/UNKNOWN tool or input was retried; the private session was read-only.
The configured resource-check executable is absent. Measured before the small serial qualification: 35 GiB free disk, 6.9 GiB available RAM; owned checkout 241 MiB and scratch well below the 3 GiB combined cap. No new venv, clone or
.artifactsoutput.Assumed