Repository navigation
fix(coding): preserve branch identity across restarts - #2787
Conversation
![coreplane-switchboard[bot]](https://avatars.githubusercontent.com/in/4630317?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: Three major restoration defects remain: same-ID baseline replacement, skipped seeded/cold capture, and lost standalone PR base.
Warning
Changes requested · head ad28755 · 3 findings: 3 major
| Severity | Finding | Where |
|---|---|---|
| major | F1 Spec contradiction — agent-coding.md item 2: same-ID request restarts replace the original baseline | src/core/dispatch/runLoop.ts:508 |
| major | F2 Spec contradiction — agent-coding.md item 2: seeded/cold checkouts skip the durable baseline | src/core/dispatch/runLoop.ts:524 |
| major | F3 Restore the original PR base before validating a resumed baseline | src/core/dispatch/runLoop.ts:498 |
F1 invariant: The first acknowledged branch identity baseline remains the same immutable evidence for the original run across live ownership changes, terminal archival, and same-ID request-restart segments; no segment may recapture the advanced branch or replace retained evidence.
- Fresh run acknowledges its first valid known, boundary, or unknown receipt in either ledger. → Retain one exact bounded owner-bound receipt; subsequent omitted state patches preserve it and replacement writes refuse.
- Live run hands off and is reclaimed under another generation, including a finish-only resume. → Restore the original receipt without comparing the advanced branch; stale generations and foreign bindings remain fenced.
- Ordinary non-pilot reattachment refuses after the run pushed, and abandonLostWorkspace closes the predecessor before restartCarried dispatches the same run ID. → Restore the predecessor's archived receipt into the new segment before writable execution; never recapture the pushed commits as inherited start state.
- Ordinary live harness interruption closes the run and dispatches its request again with restartCarried under the same run ID. → Carry the original receipt across this restart entry path and audit all earlier run-created commits against it.
- InMemoryRunLedger finishes with receipt A, then reserves/promotes the same ID with omitted evidence and receives receipt B with a changed first head or fingerprint. → Restore A on claim and refuse B before writable execution; finish must retain A rather than selecting replacement live evidence over the finished record.
- RunHistoryDO archives receipt A in work_evidence_json, then claims the same ID and receives replacement live receipt B. → Preserve A across claim/setState and terminal upsert; liveWork must not supersede the archived first baseline.
- Same-ID continuation omits its baseline or encounters unknown, foreign, or malformed retained evidence. → Preserve exact valid evidence including unknown; refuse unavailable or conflicting original bindings rather than treating the segment as a new first attachment.
F2 invariant: Every preattached coding checkout with a trusted full first SHA captures and acknowledges identity evidence from that SHA before writable execution, regardless of the executor selection path.
- Fresh resident selection has binding.ref and a valid full binding.sha. → Read the frozen SHA, durably acknowledge its receipt, and use it for the matching branch's identity audit.
- Fresh seeded-sandbox fallback has selection.seeded.ref/sha but no selection.binding. → Use the trusted seeded checkout head and matching ref for capture and acknowledgment instead of returning unknown and opening writable execution without a receipt.
- Fresh unseeded Ship child has the verified selection.cold.ref/sha from prepareColdPublicationCheckout but no selection.binding. → Capture and acknowledge the verified cold head; a subsequent push to its owned precreated branch must not be refused solely because the resident binding is absent.
- Resident, seeded, or cold selection resumes without a receipt and without a recorded push of the attachment branch. → Use the appropriate trusted full attachment SHA for the permitted initial capture rather than limiting capture to resident bindings.
- Any selection restores a valid original receipt after its current attachment head advances. → Restore and acknowledge the unchanged original receipt without rereading the advanced branch.
- A legacy resume already pushed the attachment branch but has no original receipt. → Keep the explicit unknown refusal and do not manufacture a new baseline from any selection's advanced head.
- An unbound/model-cloned selection has no trusted frozen head, or dispatch resolves no repository/ref. → Preserve the existing explicit unknown/no-read behavior; never fabricate known evidence or fall back to a mutable branch read.
- Publication targets the recorded attachment branch versus another branch with a proven first-push boundary. → Use acknowledged original evidence for the recorded branch, retain the existing created/before boundary semantics for another branch, and refuse missing evidence on either path.
Full review
F1 (high confidence): The same-ID restartCarried path carries events, token and timing, but not the receipt. After an ordinary workspace-loss or harness restart, resume is undefined, so this code rereads the advanced branch. Both ledgers admit a new baseline, and terminal persistence prefers that live evidence over the archived original. The run’s earlier pushed commits can therefore become exempt “inherited” commits. Carry the original receipt across these restart paths and enforce immutability against archived evidence during claim and finish.
F2 (high confidence): binding exists only for resident selections. Seeded fallbacks and verified cold Ship checkouts instead expose their full heads through selection.seeded.sha and selection.cold.sha. They return unknown here, skip receipt acknowledgment, and still open writable execution. A subsequent push to their owned attachment branch then fails the PR identity audit despite having a trusted initial head. Capture the matching frozen head/ref from each supported selection and await its durable acknowledgment.
F3 (high confidence): A standalone coding run targeting a PR records a baseline with base main, but claimRun does not persist baseRef, and repoContextOf restores a base only from a coordinator tag. On restart, this expression substitutes the feature branch as the base. The receipt comparison then rejects the unchanged original run as invalid, aborting both model continuation and finish-only publication. Persist and restore the original PR base before matching the receipt; do not weaken the base-binding check.
ad28755 to
ca012f1
Compare
There was a problem hiding this comment.
Changes requested: F3 is fixed; F2 still permits legacy pushed-head recapture, and F1 still loses baseline restoration on an attaching-segment boot restart.
Warning
Changes requested · head ca012f1 · 2 findings: 1 major, 1 minor
| Severity | Finding | Where |
|---|---|---|
| major | review:5421288377:F2 Spec contradiction — agent-coding.md item 2: canonical prior pushes do not prevent baseline recapture | src/core/dispatch/runLoop.ts:533 |
| minor | review:5421288377:F1 Spec contradiction — agent-coding.md item 2: boot restarts of attaching same-ID segments omit the original baseline | src/core/dispatcher.ts:4296 |
review:5421288377:F2 invariant: Every preattached coding checkout with a trusted full first SHA captures and acknowledges identity evidence from that SHA before writable execution, regardless of the executor selection path; a legacy run that already published without original identity evidence must not recapture its advanced head.
- Fresh resident selection has binding.ref and a valid full binding.sha. → Read the frozen SHA, durably acknowledge its receipt, and use it for the matching branch's identity audit.
- Fresh seeded-sandbox fallback has selection.seeded.ref/sha but no selection.binding. → Use the trusted seeded checkout head and matching ref for capture and acknowledgment instead of returning unknown and opening writable execution without a receipt.
- Fresh unseeded Ship child has the verified selection.cold.ref/sha from prepareColdPublicationCheckout but no selection.binding. → Capture and acknowledge the verified cold head; a subsequent push to its owned precreated branch must not be refused solely because the resident binding is absent.
- Resident, seeded, or cold selection resumes without a receipt and without a recorded push of the attachment branch. → Use the appropriate trusted full attachment SHA for the permitted initial capture rather than limiting capture to resident bindings.
- Any selection restores a valid original receipt after its current attachment head advances. → Restore and acknowledge the unchanged original receipt without rereading the advanced branch.
- A legacy resume already pushed the attachment branch but has no original receipt. → Keep the explicit unknown refusal and do not manufacture a new baseline from any selection's advanced head.
- An unbound/model-cloned selection has no trusted frozen head, or dispatch resolves no repository/ref. → Preserve the existing explicit unknown/no-read behavior; never fabricate known evidence or fall back to a mutable branch read.
- Publication targets the recorded attachment branch versus another branch with a proven first-push boundary. → Use acknowledged original evidence for the recorded branch, retain the existing created/before boundary semantics for another branch, and refuse missing evidence on either path.
- A legacy seeded or cold resume retains an accepted branchPushReceipts entry for its attachment branch but no pushedBranch scalar or pushed_head display event; the process died after the durable receipt commit and before publication of those projections. → Treat the canonical receipt as evidence of a prior push, perform no advanced-head identity read, and keep publication unknown without the original baseline.
- A resident, seeded, or cold legacy resume has an earlier accepted owned-branch receipt, an auxiliary branch as the latest pushedBranch, and a trimmed owned-branch display event. → Check historical evidence by attachment ref rather than only the last pushed branch; do not exempt earlier owned-branch commits by recapturing the advanced head.
- A legacy resumed coordinator publication retains publicationReceipts or an accepted publicationSettlement for the attachment branch without an original identity baseline and without its push display/scalar projections. → Recognize those canonical accepted publication paths before initial capture; preserve the legacy identity refusal rather than treating the advanced checkout as the run's first head.
- Historical publication evidence is malformed or an earlier Door/checkpoint publication is unresolved, with no original identity baseline. → Uncertainty must not establish a new known baseline from the current head; retain refusal evidence and require original proof or reconciliation without laundering prior commits into inherited state.
review:5421288377:F1 invariant: The first acknowledged branch identity baseline remains the same immutable evidence for the original run across live ownership changes, terminal archival, and same-ID request-restart segments; no segment may recapture the advanced branch or replace retained evidence.
- Fresh run acknowledges its first valid known, boundary, or unknown receipt in either ledger. → Retain one exact bounded owner-bound receipt; subsequent omitted state patches preserve it and replacement writes refuse.
- Live run hands off and is reclaimed under another generation, including a finish-only resume. → Restore the original receipt without comparing the advanced branch; stale generations and foreign bindings remain fenced.
- Ordinary non-pilot reattachment refuses after the run pushed, and abandonLostWorkspace closes the predecessor before restartCarried dispatches the same run ID. → Restore the predecessor's archived receipt into the new segment before writable execution; never recapture the pushed commits as inherited start state.
- Ordinary live harness interruption closes the run and dispatches its request again with restartCarried under the same run ID. → Carry the original receipt across this restart entry path and audit all earlier run-created commits against it.
- InMemoryRunLedger finishes with receipt A, then reserves/promotes the same ID with omitted evidence and receives receipt B with a changed first head or fingerprint. → Restore A on claim and refuse B before writable execution; finish must retain A rather than selecting replacement live evidence over the finished record.
- RunHistoryDO archives receipt A in work_evidence_json, then claims the same ID and receives replacement live receipt B. → Preserve A across claim/setState and terminal upsert; liveWork must not supersede the archived first baseline.
- Same-ID continuation omits its baseline or encounters unknown, foreign, or malformed retained evidence. → Preserve exact valid evidence including unknown; refuse unavailable or conflicting original bindings rather than treating the segment as a new first attachment.
- A pushed run archives baseline A, its same-ID successor reserves an attaching row that restores A, and the bot restarts before that successor is promoted; launchResumes dispatches the reclaimed row through opts.restart rather than restartCarried. → Carry the reclaimed attaching row's raw original baseline and independently retained repository/ref/base into claimRun and runLoop; restore A without reading advanced head B or failing its immutable acknowledgment.
| Prior finding | Resolution | Evidence at this head |
|---|---|---|
| review:5421288377:F3 | fixed | Verified provision.ts registerRun/reserveRun and run.ts claimRun persist the resolved nonprivate baseRef independently of the checkout ref. resumeLaunch.ts repoContextOf restores meta.baseRef before coordinator fallback, launchResumes supplies that context to both model and finish plans, and resolve.ts resolveTarget passes resume.repoCtx through. runLoop.ts still matches the original baseline's base strictly and supplies it to the PR target. The non-main standalone-base and changed-base-refusal tests verify restoration and conflict refusal; no receipt-derived override weakens the binding. |
Full review
review:5421288377:F2 — high confidence: The legacy-push guard checks only pushedBranch and display events, ignoring canonical accepted receipts. A seeded/cold run can crash after branchPushReceipts is committed but before those projections are published (runLoop.ts:2409–2419). On restart without an original baseline, it rereads the advanced head and acknowledges the run’s earlier commits as inherited, bypassing their identity audit. The new seeded/cold test supplies this exact receipt-only state and expects a reread and PR opening. Check canonical publication history before capture, including per-ref receipts and accepted checkpoints; missing projections must not authorize recapture. Change that regression case to expect the legacy refusal.
review:5421288377:F1 — high confidence: Archived replacement is now fenced, but restoration still misses the boot restart of an attaching same-ID successor. After baseline A is archived and restored into the successor’s reservation, a bot restart launches that row through opts.restart; this wiring reads only opts.restartCarried. With the branch advanced to B, runLoop rereads B, then the store correctly rejects its replacement baseline. Evidence survives, but the run fails instead of continuing. Carry the reclaimed attaching row’s raw baseline and independently retained repository/ref/base through claimRun and runLoop, without rereading the advanced head.
|
Warning Polylane could not verify the production impact of this pull request. Checked the new Also considered · 2 refuted
Analysed against 7 cloud accounts and 1 repository
Polylane could not find the cloud resources this repository manages, so this review looked at the entire cloud account. Connect this repository to its resources and the next review will focus on exactly what this code deploys to. Polylane analysed Rate this review: 👍 helpful · 👎 not helpful · 😕 confusing |
ca012f1 to
d3e1de5
Compare
There was a problem hiding this comment.
Changes requested: F1 and the previously resolved F3 are fixed; F2 still permits advanced-head recapture across a same-ID request restart.
Warning
Changes requested · head d3e1de5 · 1 finding: 1 major
| Severity | Finding | Where |
|---|---|---|
| major | review:5421288377:F2 Spec contradiction — agent-coding.md item 2: request restarts drop canonical prior-write refusal | src/core/dispatcher.ts:3524 |
review:5421288377:F2 invariant: Every preattached coding checkout with a trusted full first SHA captures and acknowledges identity evidence from that SHA before writable execution, regardless of the executor selection path; a legacy run that already published without original identity evidence must not recapture its advanced head.
- Fresh resident selection has binding.ref and a valid full binding.sha. → Read the frozen SHA, durably acknowledge its receipt, and use it for the matching branch's identity audit.
- Fresh seeded-sandbox fallback has selection.seeded.ref/sha but no selection.binding. → Use the trusted seeded checkout head and matching ref for capture and acknowledgment instead of returning unknown and opening writable execution without a receipt.
- Fresh unseeded Ship child has the verified selection.cold.ref/sha from prepareColdPublicationCheckout but no selection.binding. → Capture and acknowledge the verified cold head; a subsequent push to its owned precreated branch must not be refused solely because the resident binding is absent.
- Resident, seeded, or cold selection resumes without a receipt and without a recorded push of the attachment branch. → Use the appropriate trusted full attachment SHA for the permitted initial capture rather than limiting capture to resident bindings.
- Any selection restores a valid original receipt after its current attachment head advances. → Restore and acknowledge the unchanged original receipt without rereading the advanced branch.
- A legacy resume already pushed the attachment branch but has no original receipt. → Keep the explicit unknown refusal and do not manufacture a new baseline from any selection's advanced head.
- An unbound/model-cloned selection has no trusted frozen head, or dispatch resolves no repository/ref. → Preserve the existing explicit unknown/no-read behavior; never fabricate known evidence or fall back to a mutable branch read.
- Publication targets the recorded attachment branch versus another branch with a proven first-push boundary. → Use acknowledged original evidence for the recorded branch, retain the existing created/before boundary semantics for another branch, and refuse missing evidence on either path.
- A legacy seeded or cold resume retains an accepted branchPushReceipts entry for its attachment branch but no pushedBranch scalar or pushed_head display event; the process died after the durable receipt commit and before publication of those projections. → Treat the canonical receipt as evidence of a prior push, perform no advanced-head identity read, and keep publication unknown without the original baseline.
- A resident, seeded, or cold legacy resume has an earlier accepted owned-branch receipt, an auxiliary branch as the latest pushedBranch, and a trimmed owned-branch display event. → Check historical evidence by attachment ref rather than only the last pushed branch; do not exempt earlier owned-branch commits by recapturing the advanced head.
- A legacy resumed coordinator publication retains publicationReceipts or an accepted publicationSettlement for the attachment branch without an original identity baseline and without its push display/scalar projections. → Recognize those canonical accepted publication paths before initial capture; preserve the legacy identity refusal rather than treating the advanced checkout as the run's first head.
- Historical publication evidence is malformed or an earlier Door/checkpoint publication is unresolved, with no original identity baseline. → Uncertainty must not establish a new known baseline from the current head; retain refusal evidence and require original proof or reconciliation without laundering prior commits into inherited state.
- An ordinary non-pilot legacy resume has a canonical owned-branch receipt or unresolved publication evidence, no original baseline and no owned push projection, and its workspace reattachment fails before runLoop; abandonLostWorkspace dispatches a same-ID restartCarried segment with contiguous events. → Compute and carry the canonical per-ref refusal before closing the predecessor; the successor must not capture its advanced resident, seeded or cold attachment head. An auxiliary last scalar or absent projection must not clear the refusal.
- A legacy resumed non-pilot run has no original baseline and unresolved Door or malformed producer history, then its harness interrupts and dispatches ran.restart under the same run ID; there is no accepted receipt for finally to reproject. → Carry producer uncertainty through the interrupted request-restart payload and retain unknown identity evidence; do not treat the replacement checkout as an unwritten first attachment.
- A live interrupted run has accepted canonical native or leased-PR receipts whose display events were omitted before entering the loop. → Preserve their per-ref refusal across request restart, including finally's restored accepted projections; a later auxiliary push must not erase the owned branch history.
- A boot-reclaimed attaching successor has canonical receipts, malformed/unresolved publication history, gapped events or same-ID restart metadata without an original baseline. → Normalize refusal-only history from the saved row into the loop; perform no advanced-head identity capture. A valid original baseline must still restore before the history guard.
| Prior finding | Resolution | Evidence at this head |
|---|---|---|
| review:5421288377:F1 | fixed | Verified all retained-baseline cases at this head: checkpointState.ts preserves exact known/boundary/unknown receipts and refuses replacements; InMemoryRunLedger and RunHistoryDO restore archived A during same-ID reservation, repeated claim and promotion, fence foreign/corrupt/stale evidence and prevent live B replacing A at state/terminal writes. dispatcher.ts carries raw baseline and independently retained repo/ref/base across workspace-loss and live interruptions. Its new restartEvidence normalization uses attachingRestartEvidence for opts.restart and the same claimRun/runLoop fields as restartCarried. launchResumes' attaching entry and the archived-A→attaching→boot→advanced-B dispatcher regression now restore A without an advanced-head read. Raw malformed evidence remains present for refusal; target conflicts and gapped/unknown history are not treated as new proof. The legacy-no-baseline canonical-history gap remains under original F2. Previously closed F3 was independently reverified and remains fixed: provision.ts and run.ts store baseRef, resumeLaunch.ts restores it before coordinator fallback, resolve.ts consumes resume.repoCtx, and runLoop.ts strictly matches the original base and supplies it to publication; standalone non-main-base restoration and changed-base-refusal tests retain independent expected values. |
Full review
review:5421288377:F2 — high confidence: The canonical-history guard still misses workspace-loss → restartCarried. A legacy run can retain an accepted owned-branch receipt but no baseline, scalar or pushed_head event after crashing between durable acceptance and display publication. If reattachment fails, dispatcher.ts:3524 carries only event-gap uncertainty, which is false for contiguous events. The successor receives no canonical history; runLoop.ts:538 checks {} and captures the advanced head, exempting the run’s earlier commits as inherited fingerprints. Archival restoration cannot prevent this because it restores only an existing baseline. The live-interruption payload likewise omits unresolved producer uncertainty when no accepted push exists to reproject. Preserve the canonical refusal across both request-restart entry paths before successor capture, while continuing to restore valid original baselines first.
d3e1de5 to
104388f
Compare
There was a problem hiding this comment.
LGTM: Scoped F2 is fixed: original identity evidence or refusal survives every restart entry path; no major findings remain.
Note
Approved · head 104388f · no findings
| Prior finding | Resolution | Evidence at this head |
|---|---|---|
| review:5421288377:F2 | fixed | Independently verified all 16 retained cases at this head. runLoop.ts selects the ref/full SHA from the same resident binding, repository-matched seed or verified cold checkout; fresh and unwritten resumes await readStartState and commitState OK before writable execution. Unbound selections or missing repository/ref remain unknown without a mutable read. Valid original known, boundary and unknown receipts restore before refusal hints; malformed/foreign originals refuse. branchIdentityHistory.ts checks canonical native receipts, leased-PR receipts, producer history and accepted settlements by attachment ref, including absent display/scalar and auxiliary-last; malformed/pending/unresolved Door history blocks recapture. codingPrPostStep.ts retains matching-branch original evidence and other-branch created/boundary/unknown semantics on both open and edit paths. dispatcher.ts now calls restartEvidenceOf before abandonLostWorkspace closes the predecessor, carrying canonical refusal, raw baseline and independent repo/ref/base through carriedRunIdentity, prepareRestartTurn and restartCarried into claimRun/runLoop. runLoop.ts's interrupted payload marks coding runs without an acknowledged original baseline identityUncertain, which dispatcher preserves regardless of finally's accepted projections or auxiliary last push. Boot opts.restart uses the same normalizer, with same-ID restart metadata, event gaps and malformed history refusing capture; valid originals still restore first. Inspected the real failed-reuse→advanced-head dispatcher regression, the three live-interruption canonical receipt/Door/malformed assertions, original-evidence-plus-refusal restoration, boot archived-A→attaching→advanced-B regression and resident/seed/cold read/ACK barrier proofs. No tests/builds were run during review. Prior F1/F3 closures remain untouched. |
Full review
review:5421288377:F2 is fixed: canonical refusal survives workspace loss, live interruption and boot recovery, while valid original evidence restores first.
All three test-guard rename checks are refactors, verification intact: the suite rename to restartEvidenceOf preserves both the malformed-baseline/independent-target test and the owned-push/uncertain-history test with their original assertions; the canonical-refusal cases add coverage rather than replace it.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Coding runs retain the branch identity evidence captured before their first write. After a restart, the same evidence still judges their commits before a pull request opens.
Why: The first identity read lived only in the bot process, so a pushed run lost it on restart and correctly refused publication. This makes the existing identity contract durable.
Where to look
Feedback wanted: Check original-head binding, failed acknowledgment behavior, immutable restoration and private terminal retention across both stores.
Risk: More than 400 changed lines, mostly independent proofs. Two commits isolate unchanged type extraction. Splitting persistence from its consumer would leave the contract incomplete. Coding execution now waits for the initial compare; failed persistence holds writes. Revert the pair to roll back.
Verified: 18 final entry-path proofs passed; prior integration, native-store and capture suites passed. Types and consistency passed; all 44 rendered images retained their bytes. Delivery pending.
Decisions (3)
Validation (8 criteria)
For agents
Review only this source head. Public review5421288377 at the prior head reported F1 same-ID archival loss, F2 seeded/cold capture skipped, F3 lost standalone base. Public5422029401 verified F1 and F3 fixed, leaving F2 request-restart carry. This head carries canonical refusal before workspace-loss predecessor close and holds interrupted coding restarts without an original ACKed snapshot. No new first-head recapture; validated snapshots restore first. No live model probes, coding runs, cleanup or retained-byte release were performed. Both memory and bot need the containing forward release; resident/sandbox remain protected. Initial native push/restart failures are historical and not rewritten by this receipt.
🤖 Generated with Codex