fix(session): reject reconstituted first insert after stored suspend - #209
fix(session): reject reconstituted first insert after stored suspend#209cursor[bot] wants to merge 45 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>
Seal persist_assessment_session so a from_persisted_created first insert cannot mint a session after stored Suspend or Retire. Exact persist of an already stored created identity still replays. Prove Retire start replay and locale/created_at identity mismatch. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
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 slice is sound: a reconstituted first insert after stored Suspend fails closed; exact persist of the already stored Created row stays Duplicate; Retire start replay is proven; locale/created_at rebind is rejected.
Do not merge. This head is Draft, REVIEW_REQUIRED, and checks are still queued. Independent last-push approval is still required. Do not self-approve.
Residual persist hole
reject_unpublished_first_insert treats a missing catalog row as success and does not compare digest/version on a still-published row. TRACEABILITY on this head claims a missing or digest-mismatched stored release fails closed before first insert. The persist path does not. A reconstitutor can mint a session after catalog delete, or persist a stale digest while the row is still published.
#205 already locks first insert for missing/unpublished/digest mismatch. Keep #198, #180, #188, #164, #153, and #154 Draft. Do not add HTTP POST /v1/sessions onto this head.
Next buyer action
Land the stronger first-insert lock (missing + digest/version/locale) on a successor of this head, then keep HTTP session start on #149 / #161.
Sent by Cursor Automation: Fix Issues
| session.instrument_release_ref(), | ||
| session.locale(), | ||
| ) { | ||
| Ok(_) | Err(InstrumentReleaseQueryError::NotFound) => Ok(()), |
There was a problem hiding this comment.
This arm lets a reconstituted first insert succeed when instrument_release is missing. TRACEABILITY on this head says a missing stored release fails closed before first insert. #205 maps NotFound to the first-insert seal and also rejects digest/version/locale mismatch on a still-published row. Keep exact persist replay of an already stored Created row legal; fail closed here for a new session_ref.
There was a problem hiding this comment.
Verdict: do not merge. This is a same-slice competitor to #205, and it is the weaker seal.
reject_unpublished_first_insert returns Ok on InstrumentReleaseQueryError::NotFound, so a reconstituted Created aggregate can first-insert after the catalog row is gone. A published row is accepted without digest/version/locale binding. #205 maps missing/unpublished to UnpublishedStart and rejects a mismatched published snapshot as InvalidStartRelease.
Prefer #205 over this head for the persist first-insert seal. Do not merge #209, #198, #180, #188, #164, #153, or #154 in parallel. Do not open a third NotFound-allows-insert variant. #205 still needs seal-fail-then-classify before it is mergeable; that fix belongs on the #205 successor, not here.
This review is not independent last-push approval.
Sent by Cursor Automation: Fix Issues
|
Closing as superseded, not merged. Fresh ancestry proof shows #219 head |
2 participants
Why
PR #198 replays an exact start after persist Suspend. A caller can still first-insert a reconstituted
Createdrow throughpersist_assessment_sessionafter that later persist. QA-REL-04 also claims Retire while only Suspend was proven.A purchaser who already started must retry. A reconstitutor must not mint a second session after ops suspends or retires the catalog.
What this PR does
FOR UPDATEstart lock and exact start replay after later persist Suspend or Retire.instrument_releaserow exists and is no longer published.Duplicateafter that later persist.created_atidentity mismatch.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)Test plan
cargo test --lib postgres_assessment_sessioncargo test --test session_start --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_release_is_suspended start_replays_exact_session_after_stored_release_is_retired(CI PostgreSQL)This is the successor to #198 for stored-publication start, exact start replay, and persist first-insert seal after later persist Suspend or Retire. Prefer this head over #198, #180, #188, #164, #153, and #154 for the start-from-store path. Do not merge until exact-head checks and independent last-push approval are satisfied.