Skip to content

P451: state-owned check bases and scope-owned completion - #2582

Merged
trissim merged 6 commits into
mainfrom
fix/p451-order-completion
Oct 22, 2026
Merged

trissim merged 6 commits into
mainfrom
fix/p451-order-completion

Conversation

@trissim

@trissim trissim commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

P451 — state-owned check bases and scope-owned completion

P451 remains unfinished. GitHub merged this PR as 2a1fb1cd18f827f0912428ccde45a84cb174f4d3 at 2026-10-22 00:12:05 UTC, from head fabbb3d3285c445df4fa2ab66b4a20f5a49a22d0. It reported isDraft=true alongside state=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_main executed once and failed at order problem, in 9.82s. The fetch succeeds, but PendingProblemOrder.check_base returns WorkBranches.main, whose declaration is the bare branch name main, 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 use WorkBranches.main; Delivered delegates to its delivery's merge base with main; Merged uses its merge commit's first parent. OrderCheck.require_acceptance receives that base instead of selecting/fetching main itself.
  • OpenProblemOrder shares correction replacement and delivery identity across pending/claimed and awaiting-installation states. Correcting preserves assignment, order ID, review and strikes.
  • OrderScope.delivery owns construction: triage reads --seen into ReportDelivery; fix builds the existing commit/operation delivery. DoneVerb and Commander.done have no scope/state dispatch for constructing a delivery or choosing a check base.
  • CLI source members own --sha interpretation. Reserved keywords derive from their declarations; the reference parent owns free-reference input. DecodedValue carries the already-decoded object through FieldCodec without an encode/decode round trip. Core receives the nominal DeliverySource, not an op string.
  • The commit input owns source validation at the CLI boundary. The existing shared scanner remains there with its existing judge consumer; todos.py no longer imports the commander, locally or globally. This removes the circular dependency without relocating that scanner.
  • Deleted the old central done construction, dead commander revision helper, moved raw op switch, and new circular function-local import. Checked order fields are read through checked() rather than pretending every ProblemOrder declares them.
  • There was no stale check reader to delete: before this change, CheckedDelivery.verified already read todo.order.checked().check (original todos.py:213–214), and CommitDelivery.merge_state read 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.load parsed 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.log and p451-owner-before-after.log, under /home/ts/.cache/agent-scratch/unit-hygiene/.

The inventory finds one order.scope.delivery consumer and no commander import in todos.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:

Check already prints 0 before any work: `grep -rn 'input_uncertain' src/agent_comms --include='*.py' | grep -v plan_phases.py | wc -l` proves nothing; write a check that fails now

P450 acceptance:

Check already prints 0 before any work: `grep -c 'cls.normalize_field(hints\[f.name\]' src/agent_comms/pi_payloads.py` proves nothing; write a check that fails now

P442 meaningful done-argument refusal:

sha: Missing required fields for DoneVerb: sha; fix: supply this declared field

An earlier probe lacked the family kind and failed setup; it is not the meaningful reproduction.

P448 default closing evidence:

Only a delivered order has closing evidence.

Explicit --artifact already worked. The regression preserves it; no new fix is claimed for that explicit path.

Verification

  • At merged head fabbb3d32, the batch recorded 28 collected/executed, 23 passed, 5 failed, 28.37s. The superseding integration run recorded 28 passed, 0 failed, 41.35s at 984179d2f after 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.
  • In that earlier head, the five test_order_claims.py cases refused ordinary-holder deliver --fork worker at thread_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.
  • Focused command: 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.log owns 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.
  • PR-head hosted run 39696946529: smoke, native-scripts, install-rehearsal, review-verdict, debt-src and debt-tools all SUCCESS. That review job did not establish substantive approval. Queue run 39697740889 had review-verdict FAILURE beside five successes and this PR still merged; declaring the check required alone did not prevent that merge. No ratchet or full suite ran locally. New-test ruff and git diff --check passed.
  • The earlier ready head grew FunctionLocalImport/StringIdentity/FamilyFlattened and three type errors. It was sent back. This PR was returned to draft while fixing those causes, not the budgets. Current hosted counters are green.

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 ffa0a6c43db7 remains Claimed, not complete. Reviewer 17734 accepted the eight merged edits and independently ran 27 selected tests at 954534a8e, 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 main 954534a8e; p451-published-integration.log records the matching remote. The current ordered check is git 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.

@trissim
trissim marked this pull request as ready for review October 21, 2026 23:41
Copilot AI balanced review requested due to automatic review settings October 21, 2026 23:41
@trissim
trissim enabled auto-merge October 21, 2026 23:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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
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
trissim marked this pull request as ready for review October 22, 2026 00:06
@trissim
trissim added this pull request to the merge queue Oct 22, 2026
@trissim
trissim marked this pull request as draft October 22, 2026 00:10
Merged via the queue into main with commit 2a1fb1c Oct 22, 2026
6 checks passed
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.

2 participants