Skip to content

P440: Keep commander reads separate from registered fork actors - #2540

Closed
trissim wants to merge 7 commits into
mainfrom
fix/P440-fork-owner-repair
Closed

trissim wants to merge 7 commits into
mainfrom
fix/P440-fork-owner-repair

Conversation

@trissim

@trissim trissim commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

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.commander remains the commanding parent. Its Todo, unit, history and edit readers never change identity. One excluded typed actor field contains CommanderForkCaller | NoThreadCaller; CommanderVerb.apply registers then binds it once. Model copies preserve it. _caller, commanding and the context's retire compatibility members are absent.
  • Acting identity supplies invocation, send attribution, verdict/strike actors and fork gating. Refusing retirement belongs to ToolCaller; explicit fork retirement does not erase open Todos. Posting does not retire.
  • Core's order binding previously overwrote commander with caller and its mutations assumed actor = creator. ThreadCaller now 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.
  • ProblemOrderGoalAction owns 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_field obtains wire keys through the declared dataclass/codec fields. TodoStore.held_in, using Todo.unit_index and _open, is the sole open-unit selection for holds_pending, for_goal, require_unit_in, held_by, correct, and fire. Transactional callers pass an existing DB; readers use their snapshot. No registry read occurs in the Todo write transaction.
  • Capacity consumes the parent Todos already read and each holder's 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.
  • ProblemNumber owns parsing and spelling. Allocation rows contain only issued, unsent numbers; created IDs stay on Todo.id, with a numeric index. ProblemAllocation.reserve rejects malformed IDs with a named TodoError. The existing carry preserves old issuance/sequence and removes only allocator copies of created IDs.
  • All discovered production, proof and test constructors were migrated to the explicit assigner contract. Fixture-only malformed Todo IDs were replaced with canonical problem IDs. No compatibility aliases or alternate codecs.

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:

  1. 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.
  2. test_fork_and_parent_judge_show_same_unjudged_edits: retained external edit through the real judge command: 1 unjudged / 1 unjudged.
  3. 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.
  4. 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 _caller count: 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/Repository tooling 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 in test_bus_files was found and migrated. Dynamic binding and codec internals are outside this syntactic proof. NRA's newer parser could not import tree_sitter in 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 .artifacts output.

Assumed

  • The commander/merger performs fresh-environment installation from the pushed accepted head using the existing carry, preserving sessions, stores and UNKNOWN inputs.
  • Installed acceptance still must demonstrate parent-owned send with fork attribution and identical judge counts. Copied-store CLI proof is not that installation proof.
  • No broader local suite, performance claim, native-provider/UI acceptance or hosted-check success is inferred from these four tests.

@trissim trissim changed the title P440: share Todo queries and problem number ownership P440: Keep commander reads separate from registered fork actors Oct 21, 2026

@trissim trissim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. src/agent_comms/todos.py:3395-3398 checks 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 in tests/test_holder_standing_rules.py fail here and pass on the base: the four test_blocked_holder_* tests and test_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's creator/creator_created with the authority of the order being assigned (current.creator_identity, which is the assigner.commanding_identity that you already declared). Assignment.parent stays the actor record and never becomes the competition key.
  2. src/agent_comms/todos.py:2216-2217: GoalRef.authorize_order throws 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_trace fails with TodoConflict at todos.py:2217, and it passes on the base. Fix: delete the GoalRef override and keep the single base actor.require_may_change(current). Make the deserialized actor carry its authority, which means resolving CommanderForkIdentity before apply. Don't move authorization onto the goal reference.
  3. src/agent_comms/todos.py:2721: SupersededVerdict.prepare calls held_for_commander(self.actor). With a fork, self.actor is a CommanderForkIdentity whose name and birth are the fork's. The query is keyed on creator=?, so it finds nothing, and every judge superseded a 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 to test_fork_and_parent_judge_show_same_unjudged_edits.
  4. src/agent_comms/todos.py:3416: resolve_order calls self.get(problem). That raises Unknown todo: P2. where it should refuse with worker holds no order on P2. test_held_problem_orders.py::test_a_problem_not_held_by_the_unit_is_refused fails, and it passes on the base. Fix: select through the new held_in owner (held_by(commander, unit) filtered on id) so that one query owns "this unit holds this order".
  5. 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.

@trissim

trissim commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

This work continues in #2555. It carries every commit of this PR through c73e229, merged with main, and fixes the four review defects. unit-forks has two strikes on P440, so a commander fork took the task. Closing this PR in favour of #2555.

@trissim trissim closed this Oct 21, 2026
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)
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