Skip to content

P895: qualify edit ownership by source workplace, not latest session order - #3571

Merged
trissim merged 7 commits into
mainfrom
work/toad-render--P895
Oct 28, 2026
Merged

trissim merged 7 commits into
mainfrom
work/toad-render--P895

Conversation

@trissim

@trissim trissim commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

P895: 7 commits, 10 files, +532/-64. Delete the recency fallback and derive edit membership from qualified source workplaces. Seven new real-record control functions (nine declared native/external/finding cases) are written. Root recorded SourceOK on exact 83e23ccf73d0a4c49a3204a69729927e84ecb5f4; actual split execution is recorded below. Executor ran no tests. Root message35478 removes the earlier P883/P872 integration hold: P883 failed its focused controls and is excluded from this landing; no dependency requires P895 to wait. Root reports all required hosted checks successful on exact83e and forge MERGEABLE/CLEAN. Authorized to mark ready and request normal gh pr merge 3571 --auto --merge, without bypassing required checks or changing source/locks. Not installed; P895 not closed.

Actual root qualification — split heads, not one rerun

Root message35417 records SourceOK at exact83e and the following actual results:

Tested head Actual result Scope
83e23ccf73d0a4c49a3204a69729927e84ecb5f4 1 passed, 15.04s Only changed test_noncanonical_worktree_needs_qualified_link_not_matching_origin, including alias/cache identity, actual independent-clone membership and same-clone wrong-branch refusal
6704236f6df1e5fe984131bc6618d68f6ddc2d40 21 passed, 1 failed, 60.73s The21 other controls passed; the clone case failed on the fixture-family mistake subsequently corrected at83e. Production src/tools are byte-identical between670 and83e. Both hosted debt checks succeeded at670; root's focused source/path type diagnostic was absent.
53a816d5bf1d86663392ac48a7a40c2702e88a08 1 passed (within the original21-failed/1-passed16.44s run) Original unchanged verdict-refusal control only; its passing evidence remains at53a.

These are separate executions at their actual revisions, not a claimed all-controls rerun or combined runtime at83e. SourceOK is not an installation result. No author tests/checkers or source changes followed this qualification. Root's earlier draft/unarmed batch hold is lifted by message35478; P895 proceeds independently through the normal required-check merge path. Root owns installation and P895 application closure.

edit NRA
newest Core order gets the bill
source ownership is not a loyalty programme

Before / after: ownership

Before: EditWorkplaces.latest selected max(order time); StorageRule.checkout_problem returned that arbitrary fallback for every unrelated path. Unit/session identity plus recency manufactured Core source provenance. IDEN-6; fact: which source order owns an edit; at baseline turn_digests.py:272 / storage_rules.py:245; derive from actual allocation/repository and explicit or linked source boundaries; by EditWorkplaces through CheckoutAllocation, Repo, checked order declarations and PullRequests.

After: existing EditWorkplaces captures ephemeral qualified root-to-problem sets. Canonical allocations override generic roots, including released, unknown and missing allocations. Actual origin must match the problem's primary/declared companion repository and registered project. Invalid allocations reserve their boundary without supplying an owner. Explicit pre-PR source roots remain valid. Noncanonical worktrees require an existing qualified PR's repository and exact branch; matching origin alone supplies nothing. Multiple candidates select nobody, never latest. New/deleted filenames beneath a proved source root need no filename roster. Lexical and resolved membership reject aliases/escapes.

Deleted: the latest field, of/max(order time) selection, order_of, the 8-line StorageRule.checkout_problem fallback, and per-order reconstruction of an alleged latest workplace. No compatibility path, persistent attribution registry, synthetic verdict or source-event rewrite replaces them. A provisional EditWorkplace(Checkout) wrapper from the first checkpoint is also gone; the existing owner carries matching directly.

Test-only correction: construct the real lexical alias through its repository owner

Request35395: root's actual failed-only selection on670 reported 21 passed, 1 failed in60.73s. Both hosted debt checks succeeded at670; root's focused type check no longer reported the source/path assignment error. The remaining clone control stopped at my dataclasses.replace(independent, checkout=...): SourceRepository inherits Pydantic Repo, not a dataclass. Those21 passes and hosted source results belong to670; they are not an execution result for this new head.

Only tests/test_commander_forks.py:541–542 changes: use SourceRepository.of(separate / ".." / separate.name) to acquire the real origin while retaining the lexical checkout spelling, and assert alias.checkout != independent.checkout. The existing pulls.worktrees(alias) is independent_trees, independent-clone membership and SAME-clone wrong-branch refusals remain unchanged. No production src/tools byte changes, model_copy dict, fake cache, wrapper or weakened assertion. Delta 1 test file, +2/-1.

Whole-tree AST:1,188 files, no omissions, latest0. Changed test file py_compile passed without importing product/test code; disposable bytecode removed. No executor tests/checkers/runtime probes. At author delivery this case was unrun; root subsequently ran the one affected case on83e:1 passed in15.04s, and recorded SourceOK. Before/after source evidence: p895-alias-source-before.txt, p895-alias-source-after.txt.

feed a Pydantic model into dataclasses.replace
copier refuses before the ownership assertions
use the actual repository owner, not the wrong photocopier

Additive capture correction: acquire registered source proof once

Request35363 continues the completed finding-family and local-cache corrections rather than redoing them. Before: actual included any canonical Git root, then allocation and generic-source consumers each rechecked registered identity. The linked-source loop also reused path for a raw str after Path acquisitions; root's actual focused mypy on370 reported that assignment at turn_digests.py:380. No executor checker was run.

After: EditWorkplaces.capture derives the registered repository identities from its captured project before acquiring actual: dict[Path, Repo]. Each canonical absolute present root is read with Repo.of once; only registered actual origins enter this one positive-proof map. Allocation, explicit pre-PR and linked-PR consumers share it. No consumer repeats registered membership, no helper/flag/wrapper/new selection cache is introduced, and canonical local-Path caching in PullRequests.worktrees is unchanged.

Negative reservations remain separate: unfiltered present = Checkout.git_roots(*paths) feeds captured (Thread, CheckoutAllocation) tuples for every recognized allocation. Every reserved path starts with an empty frozenset in the existing allocated result. Only then do qualified actual roots prove candidates through existing Todo membership, allocation-declared primary/companion repositories and owns_declared. Foreign/unregistered/wrong-origin present allocations cannot fall through to an ancestor generic source. Missing/unknown/released lexical boundaries, ambiguity and symlink handling remain unchanged. The temporary allocation tuple map is captured declaration input, not a second derived membership result.

The external worktree spelling is now worktree_path: str, converted once to root: Path; no cast, ignore or Any. Scope/finding producer/consumer inventory still derives qualified identities via FileLine.repository.named_repositories. Both existing located/unlocated companion variants, native/external defaults, independent clone and same-clone wrong-branch counterexamples are unchanged.

Delta relative370: 1 file, +28/-22. Whole-tree AST: 1,188 files, zero omissions, latest0, stale finding.located0; py_compile of the changed production file passed without product imports, disposable bytecode removed. Existing source evidence: p895-capture-source-before.txt, p895-capture-source-after.txt. Static source evidence is not mypy or behavioral acceptance. At the670 author-delivery checkpoint these controls were unrun. Root subsequently ran the actual failed-only selection on670: 21 passed, 1 failed in60.73s, including the added companion parameter case; the clone fixture failure was corrected at83e and its one affected case passed separately. The original passing refusal remains at its actual tested53a head. All original raw records/verdicts untouched; draft/unarmed.

foreign checkout fails positive proof
still reserves its negative boundary
rejection is not a backdoor permission slip

Review correction: finding capability belonged on the finding family

Root reports the actual 22-case run on 53a816d5bf1d86663392ac48a7a40c2702e88a08: 21 failed, 1 passed, 16.44 seconds. The 21 failures all stopped at my nonexistent ProblemFinding.located() call, before source membership. The one passing verdict-refusal belongs only to that tested head; it is not current-head acceptance. Executor did not run or repeat this selection.

IMPL-4; fact: whether a finding contributes source locations; at todos.py:2514; derive by ProblemFinding.source_locations, default (), and LocatedProblemFinding override (self.where,), through the existing family. CheckedProblemOrder.source_repositories now consumes this method beside owner and scope locations. FileLine.repository.named_repositories still derives qualified repository identity. No SourceReference import/inheritance, wrapper, type/attribute probe or text parsing.

The existing meaningful companion membership/released-source control is parameterized in place: scope-unlocated-finding supplies the companion through scope and keeps its finding unlocated; located-finding supplies that same qualified location through the actual finding only. The undeclared-companion negative removes both scope and finding evidence. Native/external unlocated fixtures, cache correction, independent clone/wrong-branch and all other controls remain unchanged. This adds one declared parameter case, not a mirror test. Root chooses the failed-only rerun with the updated parameter IDs.

Correction delta: 2 files, +21/-6. Before/after whole-tree AST producer/consumer census and changed-file compile are in p895-finding-source.txt: 1,188 files, no parse omissions, zero stale finding.located() calls, declared latest count 0. Dynamic bindings remain outside AST proof. At that historical370 delivery checkpoint there was no behavioral or installed acceptance; subsequent split root qualification is recorded above. Installation remains unproved.

findings were calling another family's method
twenty-one controls never got past reception
the finding family now answers its own telephone

Review correction: local Git facts were keyed by the wrong identity

Root rejected head 21fadab0cd6c3dab111c18ab9eea79226859abbc before running tests: my added explicit-clone caller still reached _worktrees[RepositoryName]. Two distinct clones of the same forge repository therefore received the first clone's local listing. The separate-clone control was a real counterexample, not a passing result.

IDEN-6; fact: one local checkout's branch/worktree listing; at todos.py:990,1088; derive the canonical checkout Path through Checkout.resolved, by the existing PullRequests cache. _worktrees now uses Path -> Mapping[str, tuple[str, ...]]; worktrees(repository) computes the canonical checkout once, reads WorkBranches(checkout) on a miss and retains that same mapping for repeat calls. _read, _open and _todos retain their forge-owned keys. working_on and EditWorkplaces.capture still share this same method. No second cache, refresh, common-dir framework or per-edit discovery.

The existing independent-clone/wrong-branch test is preserved and extended: one actual PullRequests instance reads two real sources with identical origins but different local paths, retains each mapping on repeats, and normalizes a .. alias to the same local key. No mocks or preloaded answers. Those controls were UNRUN at author delivery; root's subsequent 53a run is recorded above. This correction adds only 2 files, +22/-7 relative to the rejected head; no earlier source/tests were removed.

two clones, one forge name
cache forwards both letters to the first house
address book repaired; delivery still awaits root's run

Before / after: shared owners and consumers

  • CheckoutAllocation remains the allocation grammar owner; base defaults and primary/companion overrides declare repository eligibility. No consumer copies its P-number/suffix decoder.
  • CitedRepository / NamedRepository and OrderScope / FixScope expose their existing qualified declarations. CheckedProblemOrder.source_repositories derives companion identities from those declarations and PR links.
  • PullRequests.worktrees exposes its existing frame cache with the corrected canonical local-Path key; working_on uses the same read. EditWorkplaces.capture batches forge reads by registered repository and qualifies only enumerated/configured roots. It unions branch facts across registered checkouts of one repository instead of overwriting them. An explicit separate clone contributes only its own already-declared root after the same cached worktree reader proves its branch; arbitrary sibling clones are not discovered. No recursive filesystem scan or per-edit Git/network discovery.
  • HeartbeatFrame passes the same registry snapshot and ProgressRead into ownership capture. HUD and overload consume that frame's mapping. Standalone judge/reminders acquire their mapping once at their read boundary.
  • OrderEditRead.owned remains the one native/external list/count gate. Only a singleton equal to the candidate Todo finalizes EditCall.problem. Original IDs, paths, hunks, clocks, records, cutoff/verdict semantics and merged-order exclusion remain with their existing owners.

Production consumer census: order_tools.py:231 list/judge; commander/standing.py:793 reminders; commander_hud.py:294 HUD; commander/vibe_check.py:143 load. Two production capture boundaries: turn_digests.py:239, heartbeat_frame.py:63. Parent/fork tooling continues through the same list owner.

Real-record controls — actual split root results above

tests/test_commander_forks.py adds actual Pi JSONL and ExternalTranscript tool starts alongside genuine Git origins, registered roots, retained Todo records and the existing fake-gh subprocess boundary:

  1. One held source plus foreign source and scratch report/plugin/probe: only proved source edits join; both original streams and zero verdict state remain. Native/external parameterization; list/count parity; new and deleted filenames.
  2. Primary and declared companion allocations; released allocation wins even over an explicit generic source root; undeclared companion cannot qualify.
  3. Misleading primary allocation with foreign origin refuses; overlapping pre-PR roots remain multiple candidates.
  4. Existing explicit pre-PR source remains legitimate; noncanonical PR worktree inside unit scratch requires exact repository/branch; same-origin unlinked worktree remains unowned. An explicitly declared separate clone of the source qualifies only on the linked branch, not a different branch.
  5. Two held orders with shared source: judge drops ambiguous membership but retains the canonical edit; no verdict is fabricated.
  6. Missing unknown allocation inside an enclosing genuine source remains unowned.
  7. Symlink aliases/foreign escapes cannot mint membership.

Preserve meaningful existing parent/fork equality, reminder and merged-order controls. test_judge_edit_ownership.py now constructs real allocated Git worktrees and stops expecting scratch to join P2. test_judge_dropped.py constructs genuine primary/declared-companion allocations and asserts foreign companion membership before settlement, avoiding a vacuous rewritten check.

Source feasibility, not behavioral qualification

Before and after AST inventories each parse 1,188 Python files across src, tools, tests, 0 parse omissions. The separate-clone delta and subsequent local-cache identity correction were reparsed/recompiled with targeted owner reproof. The declared whole-tree AST check was repeated after the cache correction: 1,188 files, count 0, no omissions. Changed files compile in memory with no product imports. The explicitly permitted whole-tree AST check reports EditWorkplaces.latest count 0, expected 0. Static declaration/inheritance/consumer map and semantically read sites are retained:

  • /home/ts/.cache/agent-scratch/toad-render/p895-source-before.txt
  • /home/ts/.cache/agent-scratch/toad-render/p895-source-after.txt
  • /home/ts/.cache/agent-scratch/toad-render/p895-cache-key-source.txt (rejected-head key and corrected owner/diff)
  • /home/ts/.cache/agent-scratch/toad-render/p895-finding-source.txt (finding declarations, producers, consumers and correction)
  • /home/ts/.cache/agent-scratch/toad-render/p895-capture-source-before.txt, p895-capture-source-after.txt (shared registered Git qualification, separate all-present reservation capture, preserved finding producer/consumer closure and py_compile)

AST does not resolve arbitrary dynamic bindings or prove runtime reachability. Executor ran no tests, checker suite, package build, benchmark, service operation, installation, domain-store investigation/replay or provider traffic. Native PR/delivery metadata operations are the explicit ordering exceptions. No performance/installed-success claim.

Original four false memberships — classified, not approved

Captured ID Existing evidence P872/Core membership
toolu_01VRsKtt4SJ6ywsAgj3uKHpa NRA amortization, actual foreign origin and independently linked NRA PR132 none; not an NRA approval either
toolu_0169cBtPCmiKcgCt9deunbZF ohcs-trace/comment.md, non-Git scratch report none
toolu_01P9Lt8TKf2jYx24E98jiH16 plugin/introspection_trace.py, non-Git scratch experiment none
toolu_01XRfq9FtrLGBCmhDfGKZAho typemap/probe.py, non-Git scratch; argv is not ownership none; subsequent task/execution unproved

Classification comes from root's retained correction artifact, not a new live replay. No original event or verdict was changed. P872's accepted Core757/Toad957 remains untouched.

Overlaps / assumptions

Base/fetched main is 2bcce132379084ea6f689dde916277819f8c2868, preserving merged #3570. commander_hud.py overlaps #3538; todos.py overlaps #3566 and merged #3570. Necessary shared-owner changes additionally touch heartbeat_frame.py and change_report.py. Root integrates overlaps; this branch absorbs no sibling implementation. No P883 matched-builder/checker, P872, installer, UI, notification or claim-delivery repair is included.

Assumed: existing registered repository origins, qualified order declarations/links and enumerated Git worktrees are the source witnesses at capture time. No durable per-edit historical provenance exists; this change does not invent it. Original events remain authoritative evidence. Behavioral execution remains root-owned: actual split results are recorded above; the executor ran none. Installed qualification remains root-owned and outstanding.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

debt-src at 99363d7, every finding:

grew BooleanChainTerms:src/agent_comms/turn_digests.py +9
::warning file=src/agent_comms/turn_digests.py::LintFindings +1 (warns, never blocks)
grew LongBooleanChain +2
grew TypeErrors:src/agent_comms/turn_digests.py +1
found BooleanChainTerms:src/agent_comms/turn_digests.py at src/agent_comms/turn_digests.py:373 src/agent_comms/turn_digests.py:373 src/agent_comms/turn_digests.py:373 src/agent_comms/turn_digests.py:373 src/agent_comms/turn_digests.py:354 src/agent_comms/turn_digests.py:354 src/agent_comms/turn_digests.py:354 src/agent_comms/turn_digests.py:354 src/agent_comms/turn_digests.py:354
found LongBooleanChain at src/agent_comms/todos.py:4070 src/agent_comms/turn_digests.py:373 src/agent_comms/turn_digests.py:354

@trissim
trissim marked this pull request as ready for review October 28, 2026 22:54
Copilot AI balanced review requested due to automatic review settings October 28, 2026 22:54
@trissim
trissim merged commit f864f34 into main Oct 28, 2026
9 checks passed

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 insufficient remaining quota.

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.

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