fix(session): replay exact persist after first-insert seal miss - #218
fix(session): replay exact persist after first-insert seal miss#218seonghobae wants to merge 47 commits into
Conversation
Store participant and published-release identity for SessionState::Created with exact replay and fail-closed rebinding. Command-replay persistence stays outside this first slice.
Assert the Database error message and source, and fail the replay SELECT after ON CONFLICT by redirecting search_path so classify runs instead of the insert.
Linux llvm-cov leaves the isolated query_one ? tail uncovered unless the Err arm is an explicit match. Keep the search_path redirect test.
Linux branch coverage missed the later AND operands of exact-replay classification. Rebind each stored field independently, and prove a domain-legal u64::MAX creation time fails closed as ValueOutOfRange.
The replay SELECT failure constructed Database evidence without checking its safe display text or source, leaving those two production lines uncovered on Linux.
SHOW transaction_isolation can fail after the caller transaction is already aborted. Persist must surface that as a typed database error instead of leaving the probe Result uncovered.
Satisfy clippy::manual_let_else in the library test that instantiates AssessmentSessionPersistenceError::Database.
Name instrument_version_ref in the public persist contract and assert the committed version column. Keep TRACEABILITY, changelog, and as-built schema at Active PR #61 rather than promoting the slice to protected-main truth. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Restore persisted created-session identity from PostgreSQL without asking whether the original release still accepts new sessions, so later suspend or retire cannot rewrite provenance. Missing rows return none; later stored states and malformed lookup references fail closed. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Name the opened successor so TRACEABILITY, as-built schema, and ERD point at the persist-and-load head instead of the persist-only #106 slice. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Add the headline reconstitution case: create while published, then suspend or retire so AssessmentSession::new fails, then restore the original Created identity and Activate. Cover numeric-like participant, release, and version references. Point AS_BUILT_SCHEMA at Active PR #109 instead of predecessor #61. Fail closed on every later stored state and on load against a missing table. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Store accepted session commands in assessment_session_command and project the current lifecycle state. Load reconstitutes created identity without re-checking publication eligibility, then replays commands so Pause/Resume still work after process restart. Exact command replay is idempotent; sequence reuse and evidence rebinding fail closed. Later stored states without command history still fail closed. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Point TRACEABILITY, as-built schema, and ERD at the successor that stores assessment_session_command and replays Activate after restart. Keep #109 named as the persist-and-load predecessor. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
A worker that only remembers Activate must not rewind a later Pause/Resume projection. Count stored commands after exact replay and fail closed when the in-memory history is shorter, so load still reconstitutes the paused session after the rejected persist. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Point traceability, as-built schema, and ERD at the successor that rejects a shorter command history instead of rewinding Pause/Resume. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Rejecting a shorter command history is not enough under READ COMMITTED. Lock the created-session row with SELECT … FOR UPDATE before inserting or counting commands so a concurrent Activate-only persist cannot count a prefix and then rewind a later Pause/Resume projection. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Point TRACEABILITY, as-built schema, and ERD at the successor that locks assessment_session before command insert or count. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
New sessions must call AssessmentSession::new through created_session_for_start / start_created_assessment_session so a draft, suspended, or retired release cannot insert a row. Keep the #146 header-row lock and command-history persist. Reconstitution remains load, not start. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Name the start-boundary landing vehicle so TRACEABILITY, ERD, UML, and as-built schema point at this head instead of the #146 lock predecessor. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
In-memory AssessmentSession::new is not enough: a stale Published object could insert after another transaction persisted Suspend or Retire. Lock instrument_release with SELECT FOR UPDATE in the same start transaction, add start_created_assessment_session_from_stored_release, and fail closed on missing, unpublished, locale-mismatched, or digest-mismatched stored evidence. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Point TRACEABILITY, as-built schema, ERD, and UML at the successor that locks instrument_release before a new session insert. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep the #180 stored-publication FOR UPDATE lock, then return the original created session when a buyer retries the exact start after persist Suspend or Retire. A new session_ref or rebound participant still fails closed. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
…ution First insert now locks stored publication state. Exact replay of an already stored Created row still succeeds after later suspend. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Classify an already stored Created row when the publication lock finds a missing or unpublished release, so a concurrent retry after the first insert commits cannot turn later Suspend or Retire into a false UnpublishedStart. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Name the first-insert seal successor after the pull request number is assigned. Keep #205 as the unlocked-peek predecessor. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: unique persist-seal-replay slice is SOUND. persist_assessment_session always locks stored instrument_release. Missing, unpublished, or digest/version/locale-mismatched catalog evidence classifies an exact stored Created row as Duplicate before fail-closed. Reconstituted first insert with no row still fails closed. Prefer this head over #219, #209, #205, #198, and #180 for stored-publication start plus first-insert seal plus seal-fail persist replay.
Do not merge: Draft, required checks are not a terminal-success exact-head set, and this run is not an independent last-push approval. Do not add HTTP onto this branch. Do not open another persist-seal successor unless this head regresses exact Created replay after Suspend/Retire or missing/mismatched catalog.
Sent by Cursor Automation: Fix Issues
| let participant_ref = session.participant_ref(); | ||
| let created_at_unix_ms = postgres_bigint(session.created_at_unix_ms())?; | ||
| let session_state = session.state().persist_name(); | ||
| match require_published_release_for_first_insert(transaction, session) { |
There was a problem hiding this comment.
This is the landing persist path. Always locking instrument_release, then SELECT … FOR UPDATE on an exact Created row before returning UnpublishedStart / InvalidStartRelease, closes the #219 unlocked-peek hole. Keep HTTP off this branch; POST /v1/sessions stays #149 / #161 / #204 on this persist contract.
There was a problem hiding this comment.
Review — persist seal-fail exact replay
Unique slice on 64747b6 / docs deb38ae is SOUND. Prefer this head over #205, #209, #219, #198, #180, #188, #164, #153, #154, #146, and #138.
What this head does
persist_assessment_sessionalways locks storedinstrument_releasebefore insert. There is no unlockedSELECT 1peek.UnpublishedStartandInvalidStartReleaseclassify an exact stored Created row (SELECT … FOR UPDATEthenclassify_existing_session) before returning the seal error.- A reconstituted first insert with no row still fails closed (
persist_rejects_reconstituted_first_insert_after_stored_suspend,persist_rejects_first_insert_when_stored_release_is_missing). - Exact persist after later Suspend and Retire stays
Duplicate. - The helper is exhaustive:
Databaseand non-publication errors are not rewritten as replay.
That closes the #205 / #219 hole: under READ COMMITTED, a concurrent exact retry can miss the first insert, take the release lock after that insert commits and after ops Suspends or Retires the catalog, and must not turn a buyer who already has the session into UnpublishedStart. QA-REL-05 requires that retry to stay legal.
#219 c7b5eda still peeks assessment_session without a lock and seals only when that peek misses. Do not merge #219, #205, or #209 in parallel with this head.
Residual, not merge-blocking
persist_replays_exact_created_row_after_stored_suspendis sequential (insert, commit, suspend/retire, persist). A two-connection lock-wait — holder inserts and holds the release lock; waiter blocks; holder commits; catalog retires; waiter must returnDuplicate— is not in this head. The classify-after-seal path is the same; the wait itself is not proven.first_insert_seal_replays_only_publication_boundary_errorsdoes not instantiateDatabase. The match is exhaustive, so this is evidence completeness only.
Operator
Do not add HTTP routes on this branch. POST /v1/sessions remains #149 / #161 / #204 and must call start_created_assessment_session_from_stored_release on this persist path after this head is integrated.
Scoring-worker landing remains #217. Item-delivery reload remains #216. Response continue-from-reload remains #221.
Local cargo test --lib first_insert_seal: 2 passed.
Qualifying approval stays with an independent reviewer. Merge only unchanged deb38ae after then-live required checks are terminal-success. Do not self-approve. Do not open another persist-seal successor unless this head regresses seal-fail exact replay.
Sent by Cursor Automation: Fix Issues
| let participant_ref = session.participant_ref(); | ||
| let created_at_unix_ms = postgres_bigint(session.created_at_unix_ms())?; | ||
| let session_state = session.state().persist_name(); | ||
| match require_published_release_for_first_insert(transaction, session) { |
There was a problem hiding this comment.
This always-lock-then-classify-on-seal-fail path is the unique fix. #219 / #205 still peek SELECT 1 FROM assessment_session without a lock and only then seal, so a concurrent retry that peeked no row can take the release lock after the first insert commits and after a later Suspend/Retire and return UnpublishedStart for a session the buyer already has.
Keep this order. Do not restore the unlocked peek to “avoid” the release lock on exact replay.
|
|
||
| let mut transaction = client.transaction().unwrap(); | ||
| assert_eq!( | ||
| persist_assessment_session(&mut transaction, &session).unwrap(), |
There was a problem hiding this comment.
Residual only: this Retire arm is sequential after commit. It does not prove a waiter that blocked on the release lock while the first insert was uncommitted, then saw Retire after that insert committed, still classifies Duplicate.
Not merge-blocking — replay_existing_created_session_after_seal is the same path. A later two-connection test can lock it the way command_persist_locks_session_header_until_caller_commits already does for command persist.
|
Closing as superseded, not merged. Fresh ancestry proof shows #232 head |
2 participants
Why
PR #205 locks stored
instrument_releaseon the firstpersist_assessment_sessioninsert. It peeksassessment_sessionwithout a lock and returnsUnpublishedStartwithout classifying an exact stored Created row.Under
READ COMMITTED, a concurrent exact retry can peek no row while the first insert still holds the release lock, then take that lock after the insert commits and after ops suspends or retires the catalog, and fail closed even though the buyer already has the session.start_*already replays that miss. Persist did not. QA-REL-05 claims the retry stays legal.What this PR does
FOR UPDATEstart-lock, first-insert seal, and exact start-replay contract.instrument_releaseon persist.Duplicatebefore failing closed.UnpublishedStartwhen no row exists.Out of scope
POST /v1/sessions/ OpenAPI / RFC 9457 (feat(api): create and reload in-process sessions over HTTP #149, feat(session): map POST /v1/sessions and lock command persist #161, feat(api): apply participant session commands over HTTP #204)assessment_sessionTest plan
cargo test --lib postgres_assessment_sessioncargo test --test session_start --test session_release_binding --test session_persisted_identity --test documentation_architecture_contract --test traceability_active_pr_contractcargo clippy --all-targets -- -D warningscargo test --test postgres_assessment_session_persistence persist_rejects_reconstituted_first_insert_after_stored_suspend persist_replays_exact_created_row_after_stored_suspend persist_rejects_first_insert_when_stored_release_is_missing(needsTEST_DATABASE_URL)This is the successor to #205 for stored-publication start plus persist first-insert seal plus seal-fail exact persist replay. Prefer this head over #205, #209, #198, #180, #188, #164, #153, #154, #146, and #138 for the start-from-store path. Do not merge until exact-head checks and independent last-push approval are satisfied.