You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #129 rejects a stale shorter command history before rewriting session_state. That check still runs after the insert loop and without locking the assessment_session header row. Under READ COMMITTED, a concurrent Activate-only worker can count a prefix, wait for Activate+Pause to commit, then write session_state = active over paused. Load then fails closed and the buyer cannot continue.
cargo test --test postgres_assessment_session_persistence (needs TEST_DATABASE_URL)
This is the successor to #129. Prefer this head over #129, #125, #121, #109, persist-only #106, and #61. Do not land #100. Do not merge until exact-head checks and independent last-push approval are satisfied.
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 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.
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>
The header-row lock is in the right place. require_existing_created_identity takes SELECT … FOR UPDATE before insert, count, and session_state update, so a concurrent Activate-only persist cannot count a prefix under READ COMMITTED and then overwrite Pause/Resume. command_persist_locks_session_header_until_caller_commits proves the waiter hits lock_timeout while the first transaction still holds the row, and the sequential stale-prefix test still proves ConflictingReplay after commit.
Do not merge #129, #125, #121, #109, #106, or #61 in parallel. Prefer this lock head over those predecessors. Do not land #100. Do not merge #138 in parallel; it still lacks this lock.
Remaining buyer gap (not a lock defect):persist_assessment_session still accepts any SessionState::Created aggregate, including from_persisted_created. A start after suspend/retire can bypass AssessmentSession::new if a caller persists a reconstituted identity. HTTP POST /v1/sessions must not call persist directly. The start boundary belongs on a successor of this head (created_session_for_start / start_created_assessment_session).
Independent last-push review is still required. This comment is not approval.
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>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Review
This lock successor is the right repair class, but do not land it in parallel with #154.
command_persist_locks_session_header_until_caller_commits persists created identity in the same uncommitted transaction that later holds FOR UPDATE. A second writer under READ COMMITTED cannot see that row, so it fails MissingCreatedIdentity instead of waiting on the header lock. The postgres persist suite was unchecked on this head.
#154 keeps the same SELECT … FOR UPDATE, commits created identity first, then proves:
a second writer hits lock timeout on the existing header row;
two concurrent workers cannot rewind a paused projection (InvalidStoredIdentity was the RED failure before the lock).
Prefer #154 over this head, #129, #125, #121, #109, persist-only #106, and #61. Do not open a fourth persist-lock PR. HTTP session-creation remains #149; do not mix it onto this persist slice.
The reason will be displayed to describe this comment to others. Learn more.
Queue note
This head's sequential stale-prefix reject is real. The remaining concurrent rewind (SELECT without FOR UPDATE) is now on #161, which also maps POST /v1/sessions and the as-built OpenAPI 3.2.0 document.
Prefer #161 over this head, #129, #125, #121, #109, persist-only #106, and #61. Do not merge this persist-only lock in parallel.
Pushed a recovery-fixture repair on this successor to closed #129: seed claim_deadline_at on the processing restore row and prove COPY keeps the exact source deadline. Rust was failing integration_consumption_claim_deadline_shape after #81.
Do not merge until exact-head rust/coverage and independent last-push approval succeed. Rebase onto #138 after that landing. Never self-approve.
Uncommitted created-session inserts are invisible to a second READ
COMMITTED writer, so FOR UPDATE returned MissingCreatedIdentity instead
of waiting. Commit the header first, then assert SQLSTATE 55P03.
command_persist_locks_session_header_until_caller_commits was RED because the hold transaction inserted the created header and the waiter's SELECT … FOR UPDATE ran under READ COMMITTED against that uncommitted insert, so it returned MissingCreatedIdentity instead of waiting. The contract is "created identity already exists, then command persist locks the header."
The test now commits the created row first, holds FOR UPDATE on command persist, and asserts SQLSTATE 55P03 (LOCK_NOT_AVAILABLE). Local postgres_assessment_session_persistence is 12/12 green, including the lock test and the recovery fixture.
Do not merge until exact-head rust/coverage and independent last-push approval succeed. Rebase onto #138 after that landing. Never self-approve.
Hourly loop: keep this header-lock successor behind #138. Exact head 555d7b64 remains Draft. Prefer this lock over closed #125 after #138 lands; do not merge in parallel. Independent last-push review is still required; this comment is not approval.
… success
The library Display wrap test cannot use expect_err because postgres::Client
is not Debug, and the let-else success arm is unreachable coverage.
Production coverage missed isolated query ? arms after the header lock
when the command or session relation is gone. Each case uses a fresh
transaction so an aborted persist cannot hide the next probe.
Production coverage still missed isolated execute ? arms: empty-history
UPDATE of the session projection, and INSERT of the first command row.
Sinks raise inside BEFORE triggers so the rust ? mapping is executed.
Linux branch coverage missed the later && arms on created-identity
compare and command replay classify. Each stored field is rebound
alone so every conjunct executes false.
Closing as superseded by protected-main behavior rather than reconciling this 43-commit Draft. Current protected main 5544149ca5dc55d2bfc3402cc59c03c44830de5f already persists assessment-session command history through persist_assessment_session_commands, locks the session header before the concurrent command path, and carries the real PostgreSQL regression in tests/postgres_assessment_session_persistence.rs: a second writer uses lock_timeout = '200ms', fails while the first transaction holds the header row, and the failure is classified as database lock timeout rather than a successful state rewind. The stronger persist-backed HTTP session path is also already on main. This stale branch should not be merged or force-rebased.
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
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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Why
PR #129 rejects a stale shorter command history before rewriting
session_state. That check still runs after the insert loop and without locking theassessment_sessionheader row. UnderREAD COMMITTED, a concurrent Activate-only worker can count a prefix, wait for Activate+Pause to commit, then writesession_state = activeoverpaused. Load then fails closed and the buyer cannot continue.What this PR does
SELECT … FOR UPDATEbefore inserting or counting commands.lock_timeoutwhile the first transaction still holds the row, and that the paused session remains loadable after commit.Out of scope
Test plan
cargo test --lib stale_shorter_command_history -- --nocapturecargo test --test session_persisted_identity --test session_aggregate_lifecycle --test session_state --test session_release_binding --test session_command_replay_contract --test documentation_architecture_contract --test traceability_active_pr_contractcargo clippy --all-targets -- -D warningscargo test --test postgres_assessment_session_persistence(needsTEST_DATABASE_URL)This is the successor to #129. Prefer this head over #129, #125, #121, #109, persist-only #106, and #61. Do not land #100. Do not merge until exact-head checks and independent last-push approval are satisfied.