Repository navigation
Refuse an evicted-read decision replay whose anchor was recovered by digest - #784
Merged
Merged
Conversation
…digest Without the Planner's retained read, a replay of an `open_decisions` key was accepted whenever the card stood right after a block carrying the replayed digest. When one edit rewrote the paragraph the card was opened on and left an identical copy of its old text between the rewritten paragraph and the card, the card's anchor was recovered by digest onto that copy, which is also the block before the card's widget, so the replay was reported as captured. With the read retained the same replay was refused. Every edit rebases the cards' anchors, and a rebase re-expresses an anchor it recovered by digest as a fresh position on the block it found. Asking afterwards whether the anchor still resolves by position therefore cannot tell a copy from the paragraph. The rebase of a card's relationship anchors now marks an anchor it found again by digest (`byDigest`, an optional field of the anchor in the card's stored record), keeps the mark through later rebases and a restart, and drops it only when the Planner anchors the card again. Comment, prose and follow-up anchors are rebased as before. Without the read, a replay is refused for a card whose anchor carries the mark, or whose stored anchor no longer resolves, with the cause that the card's paragraph can only be found again by its text. A card beside a paragraph that kept its place through insertions, deletions, edits and typing elsewhere is still accepted, with the read retained, evicted or lost to a restart. An edit that takes a card's position for any other reason (replacing the whole document, moving another block across the paragraph, a rebuilt epoch) marks it too, so such a replay is refused until the Planner reads again; a record stored before the mark existed has none. The decisions guide says so. Fixes #776 Assistant-model: Claude Sonnet 5.5 Assistant-workflow: goal (run 9570775f-38b7-4817-859f-1fd90b98cd5f) Assistant-duration: 40m converged, estimated 1h Assistant-verification: red before fix passed: with the source change stashed, the new replay tests failed (a rewrite plus copy through applyBlockEdit, through a browser plan:update, and after two restarts were all accepted as captured; one more lacked the new cause), and all 44 capture-decisions tests pass with it Assistant-verification: mutation check passed: dropping the mark's carry in the positional branch of the rebase failed 4 tests, not marking in the question rebase failed 4, and not marking at replay time failed 5 Assistant-verification: bun test apps/server passed: 4140 pass, 0 fail, 6 skipped without a database Assistant-verification: bun test packages apps/web apps/connector scripts passed: 2813 pass, 0 fail Assistant-verification: bun run fix passed: no changes, 72 existing lint warnings Assistant-verification: bun run types passed: whole workspace and e2e Assistant-verification: bun run ci passed: dprint, oxlint, tokens and design checks Assistant-verification: bun run e2e planner-decisions harness-decisions-deck mcp-update passed: 11 tests Assistant-verification: bun test apps/server/src/storage/postgres passed: 124 pass on a disposable PostgreSQL 17 at port 55432, because the 5432 that bun run test:postgres uses was taken; the marker's restart is covered with the memory adapter only Co-authored-by: Alex Lavaee <lavaman131@github.com>
lavaman131
force-pushed
the
fix-776-stale-replay-twin
branch
from
November 4, 2026 19:52
8808f7f to
e7c8a30
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Without the Planner's retained read, a stale
open_decisionsreplay is now refused, not reported as captured, when its card's anchor was recovered by digest onto an identical copy of a rewritten paragraph, including when the copy is the block right before the card's widget. Replays that were correctly accepted stay accepted: a card beside a paragraph that kept its place, with the read retained, evicted or lost to a restart.Root cause
Without the read,
repeats()inapps/server/src/questions/service-publish.tsaccepted a replay for a card standing beside the single passage it was opened on (position === 0 && standing === index). When one edit rewrote the paragraph and left an identical copy of its old text between it and the card, the card's anchor was recovered by digest onto the copy, which is also the block before the card's widget. Both checks agreed, so the key was reported as captured. With the read retained the same replay was already refused.A first fix asked only whether the stored anchor still resolved by position. That was not enough, and the test that accompanied it hid why. Every production edit path (the Planner's
edit_plan, a browserplan:update, a restart inPlan.open) runsQuestions.rebaseright after the edit, androom.rebasere-expresses a digest-recovered anchor as a fresh relative position on the copy and stores it. By the time of the replay the anchor resolved by position again, so the replay was still accepted. The test edited withPlan.publish, which skips that rebase, so it exercised a state no production edit leaves behind.Fix
The fact that an anchor was recovered by digest has to survive the rebase, so it is recorded:
Plan.Anchorhas an optionalbyDigestflag (packages/protocol/plan.d.ts). It is stored in the card's anchors in the sidecar record, which restores without validating them, so old data has no flag and nothing else changes.room.rebasetakes{ marking: true }. With it, an anchor found again by digest is marked, and an anchor that already carries the mark keeps it when it resolves by position. Only the question relationship rebase (Relationships.carry) andrepeats()ask for it; comment, prose and follow-up anchors are rebased exactly as before.repeats()refuses a card whose anchor carries the mark. It also marks on the fly, so a stored anchor that no longer resolves when no rebase has run yet is refused too.anchor_plan), which replaces the anchor.docs/decisions.mdstates the rule.Tests
In
apps/server/src/questions/capture-decisions.test.ts, through production paths:applyBlockEdit(the Planner'sedit_plan, including its rebase and commit), a real browserplan:updatethroughPlan.submitandPlan.drain, andPlan.close/Plan.open:/tmp/flakes/chopin-776-red2.txt): with the source change stashed, a rewrite plus a copy just before the card followed by nine later outlines was accepted as captured throughapplyBlockEditand through a browser update; the same after a restart following a Planner edit, and after a restart following a browser edit, later edits and another restart. A fifth test (the stored anchor not yet rebased) failed for lacking the new cause. All 44 tests pass with the fix.room.rebasefails 4 tests; not marking in the question rebase fails 4; not marking at replay time fails 5.bun test apps/server(4140 pass),bun test packages apps/web apps/connector scripts(2813 pass),bun run fix,bun run types,bun run ci, andbun run e2e planner-decisions harness-decisions-deck mcp-update(11 pass) are green.bun test apps/server/src/storage/postgrespassed (124) on a disposable PostgreSQL 17 at port 55432 because the portbun run test:postgresuses was taken; the marker's restart is covered with the memory adapter only.Limits
Fixes #776
Assistant-model: Claude Sonnet 5.5
Assistant-workflow: goal (run 9570775f-38b7-4817-859f-1fd90b98cd5f)
Assistant-duration: 40m converged, estimated 1h
Assistant-verification: red before fix passed: with the source change stashed, the new replay tests failed (a rewrite plus copy through applyBlockEdit, through a browser plan:update, and after two restarts were all accepted as captured; one more lacked the new cause), and all 44 capture-decisions tests pass with it
Assistant-verification: mutation check passed: dropping the mark's carry in the positional branch of the rebase failed 4 tests, not marking in the question rebase failed 4, and not marking at replay time failed 5
Assistant-verification: bun test apps/server passed: 4140 pass, 0 fail, 6 skipped without a database
Assistant-verification: bun test packages apps/web apps/connector scripts passed: 2813 pass, 0 fail
Assistant-verification: bun run fix passed: no changes, 72 existing lint warnings
Assistant-verification: bun run types passed: whole workspace and e2e
Assistant-verification: bun run ci passed: dprint, oxlint, tokens and design checks
Assistant-verification: bun run e2e planner-decisions harness-decisions-deck mcp-update passed: 11 tests
Assistant-verification: bun test apps/server/src/storage/postgres passed: 124 pass on a disposable PostgreSQL 17 at port 55432, because the 5432 that bun run test:postgres uses was taken; the marker's restart is covered with the memory adapter only
Co-authored-by: Alex Lavaee lavaman131@github.com