Deepen Spec PR identity ownership #515

What to build

Deepen the pull request module so a Spec run's Ticket accounting and Delivery use one ownership implementation. The module owns numbered observations, expected branches, draft transitions, checklist edits and the deliberate transition to Delivery's post-opening capture. Callers request outcomes instead of synchronizing two identity caches.

Today a Ticket can retarget the Spec PR and fail, and the factory still edits that PR's Tickets checklist. Resuming also observes one PR but drafts by branch, while opening discards the created PR's URL and discovers again by branch. The change must keep accounting on its identified PR and stop invalid observations from authorizing edits or the Spec review.

This is one complete refactor and regression fix, sized for one fresh implement session. Delivery's existing rule allowing a replacement during the opening Spec review remains intact.

Acceptance criteria

• One crate-private pull request module owns identity selection and validation for both Spec PR Ticket accounting and Delivery. Remove the Spec PR's duplicate PR record, the caller's identity-following shim, and the identity-bearing before-ready callback/result plumbing. Checklist formatting belongs behind the same interface; the Spec run supplies checklist text, not identity synchronization instructions.
• Resume uses an ordinary observation. An open Spec PR must belong to the expected repository, have the expected Spec branch and Base branch, and not come from a fork before it is drafted. Draft conversion selects the observed number. An absent or already closed PR still leaves accounting without an active PR, as today.
• First landing opens a draft when accounting has no active PR. The returned creation URL establishes the exact number in the expected repository; observe and validate that number rather than rediscovering by branch. Invalid, missing or unreadable creation information fails the landing; it cannot silently adopt another PR.
• Once Ticket accounting identifies a PR, every draft/checklist operation observes that number afresh and validates open state, expected branches and repository ownership. A closed, missing, retargeted, wrong-head or fork PR is not edited and is never replaced by another branch-selected PR during that phase.
• Intermediate checklist failures remain warnings and do not stop otherwise runnable Tickets. A landing validates the accounting PR before reporting its URL; an invalid or unreadable observation fails that landing, drains active children through the existing path, and starts no Spec review. After a valid landing observation, a checklist body-write failure remains warning-only, as today.
• The transition into Delivery ends Ticket accounting's capture scope once. Delivery still captures the first successful observation after its opening session, or during completion discovery after an opening failure. A replacement created during the Spec review is still permitted before that capture. After capture, all existing numbered validation and replacement rejection remain intact.
• The same owner remains available to Spec accounting after Delivery returns, including identity captured only by Failed run completion discovery. Checklist restoration uses Delivery's captured identity before readiness and after failure without callers copying or reconstructing that identity. Restoration failure before readiness still fails Delivery; final accounting failures remain warnings and preserve the primary cause.
• Ordinary resume and Delivery operations remain interruptible. Ticket accounting, first-landing publication and final checklist accounting use completion operations after resume, so interruption still drains and accounts for children. Completion does not clear the recorded Command interruption.
• Preserve checklist markers, unrelated body text, idempotence, Spec-derived titles, warning-only built-with annotation, the existing Repair loop and Self-merge rules, Failed run preservation, and child teardown order. Creating or editing a checklist grants no authority to mark ready or merge.
• Interface tests use coherent scripted observations and assert the affected PR and resulting outcome. Cover resume read/draft replacement, creation/discovery replacement, wrong target/head/fork, disappearance/closure, warning-only writes, terminal landing gates, interruption, and both successful and failed Delivery capture. Keep the existing replacement-during-review and late-replacement rejection regressions.
• Keep an end-to-end regression where the second Ticket retargets the already opened Spec PR, replaces its body with a sentinel, and fails: the sentinel stays unchanged, the target problem is reported, and the failed Ticket outcome is retained. Also cover successful Ticket completion followed by an invalid accounting PR: no Spec review starts and a replacement stays untouched.
• Update the glossary's Spec PR and Delivery entries to describe numbered Ticket accounting and its explicit transition to the existing post-opening Delivery capture. This documentation is this Ticket's work. No new domain term or ADR is needed.
• Formatting, linting with warnings denied, and the repository's required test checks pass.

Blocked by

None (can start immediately).

Decisions

The design tree is settled in four rounds: constraints → ownership and interface → observations and phase transitions → tests and documentation. The frontier is empty; no human decision remains.

Round 1: constraints and scope

1. Deepen ownership rather than add a second validation implementation. Ticket accounting and Delivery already know the same repository, expected branches and PR identity, but their callers reconcile separate records. Concentrating those facts in the existing pull request implementation passes the deletion test; independently adding checks to the Spec PR would preserve the shallow coordination interface.
2. Keep the two capture scopes explicit. Ticket accounting owns the PR it resumed or opened; Delivery owns the PR first observed after its opening session. The current glossary and regressions deliberately permit replacement during the Spec review. Preserve that behavior rather than extending one immutable number across the whole Spec run.
3. Publish one standalone Ticket. The owner, its two callers, adapters and regression tests form one cohesive change with no independent intermediate behavior to ship. No open issue covers this deepening; all open titles and bodies were read, then fetched again unchanged before publication. Closed Deepen Delivery's pull request lifecycle #507 established Delivery's later capture and is prior art, not coverage of this earlier phase.
4. Follow the existing ADRs. Ticket Runs still merge into the Spec branch under ADR 0006; Delivery's guarded Self-merge still follows ADR 0004. Worktree ownership, CI policy, Repair budgets, Claims and child process ownership keep their existing responsibilities because this friction concerns PR identity only.

Round 2: ownership and interface

5. Promote Delivery's pull request owner to a shared crate-private module. Reuse its coherent observations, expected branch checks, numbered selection and completion behavior. A Spec run constructs an owner before its Ticket loop and retains it through Delivery and final accounting; an ordinary Run constructs its own owner for Delivery. No global shared state or additional locking is needed.
6. Store identity and phase, not a cached mutable PR record. Keep the immutable expected repository, branch and Base branch beside the active number and URL. Ticket accounting and Delivery are explicit capture scopes inside the owner. State, target, draft status and fork information come from each fresh observation, so an old record cannot authorize a new operation.
7. Keep callers' interface at meaningful operations. Resume/open a Spec PR draft, show or land a Tickets checklist, deliver the PR, and report its identity. Number selection, validation, body reads and writes, capture transitions and failure observations stay behind the seam. Immutable number/URL information remains useful for reporting and confirmed Self-merge finishing, without becoming state callers must copy back.
8. Borrow the retained owner during Spec PR Delivery. Delivery changes the owner's capture scope internally once, before its opening work, and leaves that owner's final state available afterward. Its first capture still occurs after the opening session, or on failure completion. Keep the owner outside the Sessions scope so Failed run finishing can borrow it after sessions end. Return the existing outcome rather than a second identity that the Spec run must follow.
9. Pass checklist data through Delivery instead of an identity callback. The before-ready callback has one production purpose: restoring the Spec PR checklist. Give the Spec delivery path its checklist text, and restore it through the owner after annotation and before readiness. An ordinary Run supplies no checklist. This removes the cross-module borrow and synchronization protocol while preserving the ordering that matters.
10. Absorb checklist editing into the ownership implementation. Keep marker replacement and append behavior private behind checklist operations, including preservation of unrelated text and skipping unchanged writes. Delete the old stateful Spec PR shim once callers migrate; do not retain an adapter that merely forwards its old interface.
11. Extend the existing adapter seam. The production adapter is GitHub; a scripted adapter supplies complete snapshots, creation outcomes, bodies, transition failures and interruption in tests. Add only the Spec title/draft creation effects needed by the owner. Two adapters make this a real seam; there is no need for another general transport abstraction.

Round 3: observations and phase transitions

12. Resume observes by branch once, then drafts by number. Treat absence or an already closed PR as no active accounting PR, preserving restart behavior. For an open PR, capture and validate its repository-owned head and expected target, then make the draft request on that number. A fresh numbered gate before draft conversion prevents a changed observation from authorizing a transition.
13. Creation identifies what the factory just opened. Keep the returned, trimmed creation URL, validate that it names a positive PR number in the expected GitHub repository, and observe that number. Compare repository names as GitHub does, without case sensitivity. Validate the expected branches, open state and draft state before accepting it. A transport/parse/observation failure is terminal for the landing; never fall back to another branch discovery.
14. Validate accounting operations on coherent numbered snapshots. Reuse the owner's repository/head/fork/number checks and open/expected-base validation before draft conversion or a checklist body read/write. A later branch-selected replacement cannot affect selection. These checks prevent changes observed before the operation; they do not claim an atomic guarantee against a remote change between the last observation and a body edit.
15. Separate warning-only accounting from the landing gate. Showing intermediate/final progress warns on an invalid observation or write failure and leaves the PR untouched when invalid. Landing with an existing PR first performs a terminal identity/state/target observation; only a valid PR URL is returned. A body-write failure after that valid gate still warns, preserving today's distinction between delivery authority and optional progress text. If no PR is active, title lookup, creation and exact-number validation must succeed.
16. End accounting's capture once when Delivery begins. Reset its active identity within the owner before Delivery's opening work, without immediately rediscovering. Successful annotation normally makes the first post-session capture; an absent or failed observation leaves it uncaptured, and an opening failure may bind it during completion discovery. Never reset during Repairs or after capture. Existing successful and failed replacement-during-review behavior therefore survives.
17. Restore and finish on Delivery's active identity. The owner performs checklist restoration before readiness, and remains borrowed by ordinary gates, Self-merge and Failed run finishing. After failure, its final checklist operation uses the identity established by those paths. Do not rediscover after a captured PR disappears, and do not reintroduce follow through a differently named setter.
18. Preserve the ordinary/completion distinction per operation. Resume uses ordinary GitHub operations; subsequent Ticket accounting, including first landing, uses completion operations. Delivery uses ordinary operations except its existing merge reconciliation and Failed run completion paths. Final checklist accounting uses completion operations. The shared owner must not inherit a permanently completion-only GitHub view from the Ticket phase, because that would weaken Delivery interruption behavior.
19. Keep the rest of Delivery's policy unchanged. Annotation remains warning-only and readiness remains decisive. Preserve exact-head merge requests, same-number completion confirmation, Policy refusal handling, mergeability polling, work preservation and post-merge finishing. The change concentrates identity ownership; it does not add a new merge policy or let checklist success bypass a gate.

Round 4: tests, documentation and verification

20. The owner interface is the primary test surface. Move existing Spec PR policy tests onto that interface and reuse the Delivery scripted-adapter prior art. Script changes between observations and transitions, rather than sleeping or asserting private helpers. Verify number-selected side effects and safe outcomes; retain outer Spec run and Delivery tests for orchestration, child draining and readiness ordering.
21. Test real wiring with the existing CLI scenarios. Keep replacement during both successful and failed opening reviews, rejection of replacement after Delivery capture, checklist restoration, first landing after interruption and drained-child accounting. Add the reproduced retargeted-failed-Ticket case and a terminal landing case. Verify created-PR number selection through the transport tests; keep the fake's numeric selector support rather than adding a broad new test hook.
22. Implement glossary changes with this Ticket. Spec PR already names the domain concept, so no new term is introduced. Sharpen its numbered accounting behavior and explain the capture-scope transition in Delivery. This reversible module refactor preserves the existing trade-offs and needs no ADR change.
23. The Strong call has executable evidence. At reviewed main head a29ebb6, a throwaway CLI regression let Ticket Numbered Issue branches after a merged or closed PR #21 open the Spec PR, then made Ticket CI watching, CI-fix Repair and the Repair cap #22 retarget it to develop, replace its body with untouched, and exit 1. The Run failed as expected and started no Spec review, but the safety assertion failed: accounting appended the Tickets checklist to the retargeted body. All 14 existing Spec PR tests and all 62 existing Spec run CLI scenarios passed, including successful/failed replacement during review, late replacement rejection and interrupted accounting. Both report diagrams passed the Mermaid parser. The throwaway regression was removed after the check; no production implementation or domain document was edited. The implement session must keep the regression and run the required checks after the refactor.

Architecture review

Strength: Strong · Dependencies: mock

Files: src/spec_run/spec_pr.rs, src/spec_run.rs, src/delivery.rs, src/delivery/pull_request.rs (promoted to src/pull_request.rs), src/main.rs, src/run.rs, src/github.rs, src/github/execution_tests.rs, tests/spec_run.rs, CONTEXT.md

Before

flowchart LR
  S["Spec run"] --> A["Spec PR checklist"]
  S --> D["Delivery"]
  A -. "branch draft and cached writes" .-> G["GitHub adapter"]
  D --> P
  subgraph P["Delivery pull request module"]
    direction TB
    I["PR identity"] --> T["PR transitions"]
  end
  P --> G
  D -. "captured identity" .-> S
  S -. "follow identity" .-> A
  classDef leak stroke:#dc2626,stroke-width:2px;
  class S,A leak

After

flowchart LR
  S["Spec run"] --> P
  S --> D["Delivery"]
  D --> P
  subgraph P["Pull request module"]
    direction TB
    A["Spec PR checklist"] --> I["PR identity"]
    I --> T["PR transitions"]
  end
  P --> G["GitHub adapter"]
  classDef deep stroke-width:3px;
  class P deep

Problem: the shallow identity coordination interface leaks selection and capture rules across the Spec PR and Delivery seam, allowing Ticket accounting to edit a retargeted PR.

Solution: deepen the shared pull request module so its implementation owns numbered operations and capture transitions behind one interface.

Wins

• Depth: one ownership interface
• Locality: identity checks concentrate
• Leverage: Ticket accounting and Delivery
• Tests cross one real seam
• Adapter supplies coherent observations

Deletion test: deleting the Spec PR's duplicate cache and identity-following shim concentrates their obligations in the deep module; deleting that module would spread numbered selection, capture transitions and validation back across callers.