Repository navigation
P451: state-owned check bases and scope-owned completion - #2582
Merged
Merged
Conversation
trissim
marked this pull request as ready for review
October 21, 2026 23:41
trissim
enabled auto-merge
October 21, 2026 23:41
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
trissim
marked this pull request as draft
October 21, 2026 23:51
auto-merge was automatically disabled
October 21, 2026 23:51
Pull request was converted to draft
trissim
marked this pull request as ready for review
October 22, 2026 00:06
trissim
marked this pull request as draft
October 22, 2026 00:10
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.
P451 — state-owned check bases and scope-owned completion
P451 remains unfinished. GitHub merged this PR as
2a1fb1cd18f827f0912428ccde45a84cb174f4d3at 2026-10-22 00:12:05 UTC, from headfabbb3d3285c445df4fa2ab66b4a20f5a49a22d0. It reportedisDraft=truealongsidestate=MERGED: drafting and disabling auto did not dequeue it before merging. Explicit dequeue was too late. Receipts:p451-queue-inspect.log,p451-main-consumed-1.log.Known remaining defect (17561): the old order's Claimed/main and Delivered/merge-base rules still measure already-merged work after its fix. P425's actual Claimed state must be supported; changing its stored state is not the repair. A corrected order for the shared
WorkBranches.base_before(repo, commit)calculation was staged by the reviewer but remains blocked by the ended assigning fork. Do not close P451 from this PR's merge or green checks.Additional P451 regression confirmed from 17853: at
68b414250c4165977701717e9280a9e666736937,test_holder_standing_rules.py::test_send_and_check_accept_a_deletion_check_on_fresh_mainexecuted once and failed atorder problem, in 9.82s. The fetch succeeds, but PendingProblemOrder.check_base returns WorkBranches.main, whose declaration is the bare branch namemain, not the fetched remote revision. The retained real test repository proves local main has 0 obsolete declarations while fetched origin/main has 4; the acceptance check incorrectly reads the local 0. ClaimedProblemOrder and OperationDelivery repeat that same answer. This is my regression, not explained away by also failing on main. The corrected order must repair the shared base reader and fetch ownership for this family as well as the already-merged-work case. Existing MainRevision owns remote source-reference behavior; no caller-specific extra fetch or new literal in OrderCheck is the repair. Evidence:p451-17853-fresh-main-1.log(RESULT:1),p451-17853-refs.log. No source repair or installed acceptance is claimed.The independent #2584 deletion is now consumed. At integration head
984179d2f3c47e49e3926b0a41db61c553898bc3, the same focused batch executed 28 tests: 28 passed, 0 failed, 41.35s (p451-main-consumed-1.log). The previous five claim failures are repaired by #2584; no new repair order is needed. One scope CLI subprocess still inherits real HOME and is not proof of isolation or installed/native readiness; the shared child-environment repair belongs to its existing owner. The old incorrect-base assertion remains pending the corrected order, so these passes do not prove P425's requested workflow.Ownership and deletions
ProblemOrder.check_base(repo)owns the acceptance base: Pending/Claimed useWorkBranches.main; Delivered delegates to its delivery's merge base with main; Merged uses its merge commit's first parent.OrderCheck.require_acceptancereceives that base instead of selecting/fetching main itself.OpenProblemOrdershares correction replacement and delivery identity across pending/claimed and awaiting-installation states. Correcting preserves assignment, order ID, review and strikes.OrderScope.deliveryowns construction: triage reads--seenintoReportDelivery; fix builds the existing commit/operation delivery.DoneVerbandCommander.donehave no scope/state dispatch for constructing a delivery or choosing a check base.--shainterpretation. Reserved keywords derive from their declarations; the reference parent owns free-reference input.DecodedValuecarries the already-decoded object through FieldCodec without an encode/decode round trip. Core receives the nominalDeliverySource, not anopstring.todos.pyno longer imports the commander, locally or globally. This removes the circular dependency without relocating that scanner.revisionhelper, moved rawopswitch, and new circular function-local import. Checked order fields are read throughchecked()rather than pretending everyProblemOrderdeclares them.CheckedDelivery.verifiedalready readtodo.order.checked().check(originaltodos.py:213–214), andCommitDelivery.merge_stateread the current check (original line 317). The regression protects that existing behavior; this PR does not invent a stale-reader fix.Source evidence
Existing audit
Package.loadparsed 440 Core production modules, no omissions at the current head. Initial dependency inventory parsed 283 Toad modules, no omissions. Full selected declarations, inheritance, imports, reads and consumers:p451-current-source-audit.log; before evidence:p451-ast-before.logandp451-owner-before-after.log, under/home/ts/.cache/agent-scratch/unit-hygiene/.The inventory finds one
order.scope.deliveryconsumer and no commander import intodos.py. Dynamic external implementations of the source contract are outside this syntactic resolution; AST evidence is not behavioral proof.Against integrated main: production +182/-65 (net +117); entire PR +298/-66 (net +232). These are gross additions/deletions, not claims that moved lines were entirely removed behavior.
Copied-state refusals before editing
A consistent SQLite backup was taken through a read-only live connection; original stores were not edited. The snapshot contained P425 Claimed, P450 Withdrawn, P442 Withdrawn. These reproduce refusal stages, not fabricated historical open-state command journeys.
P425 acceptance:
P450 acceptance:
P442 meaningful done-argument refusal:
An earlier probe lacked the family kind and failed setup; it is not the meaningful reproduction.
P448 default closing evidence:
Explicit
--artifactalready worked. The regression preserves it; no new fix is claimed for that explicit path.Verification
fabbb3d32, the batch recorded 28 collected/executed, 23 passed, 5 failed, 28.37s. The superseding integration run recorded 28 passed, 0 failed, 41.35s at984179d2fafter consuming P440: delete dead delivery verb and use the unit tool #2584 (p451-main-consumed-1.log). All six completion cases, eight acceptance-write cases, eight scope cases and existing pending-order completion passed.test_order_claims.pycases refused ordinary-holderdeliver --fork workeratthread_execution.py:89:RelationViolationError: This thread is not a declared commander fork.They fail before delivery. No guard/assertion was weakened. Unit-mi-1b traced this separately; commander authorization for its follow-up repair was requested in message 17665. This is not a green 28-case run or a proven baseline attribution.timeout 300 env PYTHONPATH=src /home/ts/code/projects/agent-comms/.venv/bin/python -X faulthandler -m pytest tests/test_order_completion.py tests/test_order_check_writes.py tests/test_order_scopes.py tests/test_order_claims.py tests/test_held_problem_orders.py::test_done_problem_delivers_the_units_held_order -o addopts='' -o faulthandler_timeout=60 -p no:cacheprovider --basetemp=/home/ts/.cache/agent-scratch/unit-hygiene/p451-current-final-1.p451-current-final-1.logowns those results. Earlier failures, including adapter setup failures, are retained, not passed off as successful executions. The separate 23-case source batch passed 23/23 before the final main integration.git diff --checkpassed.Remaining acceptance
Not installed or live-qualified. The base calculation must be repaired for P425's actual Claimed state before the requested live correction/completion can succeed. My earlier suggestion to establish a different record state was wrong. Withdrawn historical orders are not silently reopened. Installed triage report completion and the configured live-agent/message journey remain required. Hosted installation rehearsal is not that live acceptance.
Order
ffa0a6c43db7remains Claimed, not complete. Reviewer 17734 accepted the eight merged edits and independently ran 27 selected tests at954534a8e, all passing. Its new findings—duplicate Pending/Claimed base answers and duplicated fetch ownership—need the commander's next order. The checkout is now clean and published at integrated main954534a8e;p451-published-integration.logrecords the matching remote. The current ordered check isgit grep -l 'def check_base' -- src | wc -l, expected 1. No bait import was added.Assumed: the new base rule and ownership cleanup require the corrected order; the current old rule is not represented as sufficient. Historical snapshot labels are not assumed to be unavailable historical states. Git merge is recorded above; no installed or live-working claim is made.