Skip to content

fix(ship): recover original units after post-approval changes - #2414

Open
coreplane-switchboard[bot] wants to merge 1 commit into
mainfrom
plan/fix-p0-issue-2412-from-c-85c007/u1
Open

coreplane-switchboard[bot] wants to merge 1 commit into
mainfrom
plan/fix-p0-issue-2412-from-c-85c007/u1

Conversation

@coreplane-switchboard

@coreplane-switchboard coreplane-switchboard Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

An original Ship unit that reached merge-ready can resume after a later authorized changes-requested review at its unchanged PR head. Recovery runs a full review first and carries the original identity, lease, rounds, grant, and cumulative spend forward.

Why: Issue #2412 blocks correcting #2411 after its first approval. A fresh unit would lose the original writer and budget limits. This repair creates a guarded continuation of that unit before any code fix or retry is admitted.

Where to look

  1. Later review evidence Require one authorized changes-requested review at the exact original head and recheck its claim before later children.
  2. Cumulative child accounting Rebuild priced spend from complete persisted original children and reject missing, duplicate, foreign, or overlapping evidence.
  3. Recovery admission Accept the original merge-ready boundary while preserving its head, remaining lease, cost cap, and sole writer.
  4. Original grant at dispatch Carry the saved grant and remaining clock into the recovery Workflow without opening a new segment.
  5. Round and cost enforcement Refuse another child when original round or dollar capacity is spent or unknown.
  6. Behavior and proof row The spec names the post-approval recovery contract and focused test groups.

Feedback wanted: Check reviewer authorization, exact-head and original-unit binding, complete priced child history, lease and round enforcement, and the #2411 timing regression.

Risk: A false admission could start a duplicate or over-budget writer. Missing or conflicting identity, review, head, child, lease, or cost evidence must fail closed. No live #2411 recovery is claimed here.

Verified: Initial coding run reported 704 focused tests; the findings round passed focused F1/F2 tests. Current-head CI and CodeQL are green; re-review and live #2411 recovery remain open.

Decisions (3)
  • Review before findings. A later human review is evidence to investigate, so recovery starts a full original-unit review before typed findings can drive coding.
  • Preserve original authority. The claim retains the requester, thread, unit, branch, PR, grant, lease, and lifetime spend; the new Workflow is only a transport checkpoint.
  • Fail closed on incomplete accounting. Missing priced children or an exhausted cost cap cannot be treated as zero spend or a new budget.
Validation (4 criteria)
Criterion Proof
One later authorized changes-requested review at the unchanged original head can initiate recovery; stale, foreign, or ambiguous reviews cannot src/channels/adminCoordinator.test.ts::POST /admin/coordinator/recover-unit — unchanged-head original-unit recovery::post-approval recovery*
Original paid child history, grant, lease, and rounds constrain the continuation src/core/ship/coordinator.test.ts::original-unit recovery accounting::*; src/channels/adminCoordinator.test.ts::POST /admin/coordinator/recover-unit — unchanged-head original-unit recovery::post-approval recovery*
The recovery Workflow begins with a full read-only review on the original unit src/core/coordinator/driver.test.ts::runOriginalUnitRecovery::starts a full original-unit review*; src/core/coordinator/briefs.test.ts::composeChild — the child a brief names::briefs the post-approval review*
Held #2411 resumes safely under its original identity after release Live procedure pending reviewed source merge, deployment, durable cap/spend/lease readback, and an exact-head original-unit attempt; no live receipt yet
For agents

Original Ship thread and unit plan-fix-p0-issue-2412-from-c-85c007:U1 own this PR. Keep #2411 held until this source is reviewed, person-merged, released, and proved live; do not wake its original unit or nominal U4 from this PR. Legacy publication/budget recovery for website #405 and #2407 is a separate serialized #2341 gate. This body was repaired at the exact PR head without changing code or branch.

@coreplane-switchboard coreplane-switchboard Bot 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.

Changes requested: Recovery can proceed on superseded or dismissed external review evidence.

Warning

Changes requested · head 387d032 · 2 findings: 1 major, 1 minor

Severity Finding Where
major F1 Competing later approval by another reviewer does not invalidate the trigger src/channels/adminCoordinator.ts:2275
minor F2 Dismissal after the full-review spawn does not fence later coding src/channels/adminCoordinator.ts:909
Full review

F1 (major, high confidence): laterRecoveryReview rejects a later decision from the same reviewer, but not an approval from a different reviewer. If reviewer A requests changes and reviewer B later approves the unchanged head, recovery still accepts A’s request as its trigger. Treat a competing later decision as stale evidence, and add that case to the refusal tests.

F2 (minor, medium confidence): The external review is revalidated before the first read-only child, but not before the subsequent findings spawn. If GitHub dismisses it while that child runs, coding can still start from the claimed recovery. Recheck the claimed review before admitting later children, or explicitly establish why the independent typed review remains sufficient authority after dismissal.

Co-Authored-By: coreplane-switchboard[bot] <318072483+coreplane-switchboard[bot]@users.noreply.github.com>

@coreplane-switchboard coreplane-switchboard Bot 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.

Changes requested: Later cross-reviewer approvals can leave a stale trigger authorized, and cost-cap settlement can strand a pushed findings head.

Warning

Changes requested · head e8db530 · 2 findings: 2 major

Severity Finding Where
major F1 Recheck competing approvals across reviewers before coding src/channels/adminCoordinator.ts:2299
major F2 Reconcile a completed findings push before cost-cap termination src/core/ship/coordinator.ts:1998
Full review

F1 (major, high confidence): The initial selection rejects an approval from another reviewer after the changes request, but claimedRecoveryReviewValid checks only the claimed review. If another reviewer approves after the full read-only child runs, the findings child can still be admitted on a superseded trigger. Recheck later competing decisions across reviewers before admitting each subsequent child. The dismissal fence is present; this is a separate supersession gap.

F2 (major, high confidence): A findings child can complete and push a new head that takes cumulative spend over the cost cap. settleCoding then aborts before pr-check reconciles that push, leaving the durable publication binding at the old head and consuming the recovery on settlement. Reconcile the completed push before ending the unit without dispatching another child.

This branch has not been deployed

No deployments
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