Skip to content

fix(session): lock header so concurrent stale persist cannot rewind - #154

Closed
cursor[bot] wants to merge 38 commits into
mainfrom
cursor/bc-283cb3b5-582d-4c3d-90c5-0563ba3dcd10-07f9
Closed

cursor[bot] wants to merge 38 commits into
mainfrom
cursor/bc-283cb3b5-582d-4c3d-90c5-0563ba3dcd10-07f9

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026 •

Copy link
Copy Markdown
Contributor

Superseded

This concurrent session-header locking slice is fully contained in #188. Do not merge this predecessor.

Fresh exact ancestry evidence immediately before closure:

#188 retains sequential stale-prefix rejection plus the SELECT … FOR UPDATE header lock and adds a publication-gated Created-session start boundary with exact replay, locale, participant, and release-binding protections. #188 remains Draft and subject to unchanged exact-head CI/security/review gates. Closing this PR does not promote successor behavior to protected-main truth.

seonghobae and others added 30 commits August 14, 2026 10:46
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.
Treat landed PostgreSQL readiness as Implemented and keep #61 as the
Active created-session persist slice.
Treat landed migration rollback coverage as Implemented and keep #61
as the Active persist slice.
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>
cursoragent and others added 5 commits August 16, 2026 15:38
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>
A stale Activate-only persist could count command rows, then overwrite
session_state after Activate+Pause committed, leaving the session
unloadable. Lock the assessment_session header row for the caller
transaction and prove two workers cannot rewind a paused projection.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@cursor
cursor Bot requested a review from seonghobae August 16, 2026 16:00
cursoragent and others added 2 commits August 16, 2026 16:00
Retarget the session persist/load landing vehicle from sequential
stale-prefix #129 to the header-row lock successor.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Commit created identity first so a second writer waits on FOR UPDATE
instead of missing the uncommitted row. Record physical session and
command uniqueness in the ERD constraint table.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment

No blocking lock, race, uniqueness, or fail-closed defect on 7042425. The header-row SELECT … FOR UPDATE sits before insert/count/session_state update, and the concurrent Activate-only vs Activate+Pause test keeps load at Paused with two commands. Sequential stale-prefix rejection from #129 is preserved. lock_timeout is test-only; a waiter maps the server timeout to Database instead of a silent rewind.

This is not merge approval. The PR is still Draft, exact-head checks are queued, and independent last-push review is still required. Prefer this head over #146, #129, #125, #121, #109, #106, and #61. Do not merge those persist-lock predecessors in parallel.

Residual (successor, not this PR)

persist_assessment_session still accepts a reconstituted Created aggregate from from_persisted_created, so a caller can skip AssessmentSession::new publication checks. That is the start-boundary gap (#153 / stored-release start), not a lock defect. HTTP POST /v1/sessions must not call persist directly.

Process

Reviewer seonghobae is already requested. Do not self-approve. Do not open another persist-lock PR. Next buyer-facing slice after this lock lands: publication-gated session start, then HTTP session-creation.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

cursor Bot pushed a commit that referenced this pull request Aug 16, 2026
Name the start-boundary successor of the #154 header lock in
traceability, ADR-0005, as-built schema, ERD, UML, quality attributes,
and the changelog.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@seonghobae seonghobae closed this Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants