feat(session): load created sessions without rechecking publication by cursor[bot] · Pull Request #109 · ContextualWisdomLab/psychometrics-commons

cursor · 2026-08-16T15:19:06Z

Superseded by repaired durable-session line

Close this predecessor without merge. Its Created-session persist/load contract and the later coverage/error-path fixes have been carried through the repaired command-history/start lineage into #232; its one branch-only recovery delta is isolated on #177.

Fresh evidence immediately before closure:

feat(session): load created sessions without rechecking publication #109 exact head: d6b284cad9596179fda6f00c0fa855f6a0982a5c
current durable-session HTTP feat(api): start and reload sessions over persist-backed HTTP #232 exact head: c7b45145c9b693951fae498e0779d58cbe170f86
dedicated recovery test(recovery): preserve inbox claim deadline evidence #177 exact head: 8a0db4611a684285185f9edadf086a1808444ab9
protected main: a7637351be8f0f90c12651d3bcafd959bc52ac81

Current #109 review says the core load design is sound but this exact head remains non-mergeable because required load error/reference branches and the as-built pointer are incomplete; later review explicitly directs Created persist/load/start to a successor rather than updating #109. The session lineage subsequently passed through #125 and the stale-shorter-history repair #129; fresh ancestry proves current #232 is directly ahead of #129 and therefore carries that corrected persist/load/command-history foundation plus stored-release start locking and public start/reload HTTP.

A fresh #109↔#232 comparison shows the only #109-side commit delta absent from #232 is tests/postgres_recovery_invariants.rs (27 additions / 3 deletions), the claim_deadline_at COPY/restore concern already isolated on #177.

Do not reinterpret #109's review findings as fixed on this head; they remain evidence for closure. #232 and #177 remain Active PR work and must satisfy their own unchanged exact-head CI/security/review gates before merge.

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>

cursor · 2026-08-16T15:20:26Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

cursor

Stale comment

Review (ff82de8)

Created-session persist/load is correct for this slice. AssessmentSession::from_persisted_created never takes an InstrumentRelease and never calls accepts_new_sessions(). load_assessment_session reconstitutes through that constructor. Persist stays Created-only, exact replay, fail-closed rebinding. Docs stay Active PR, not Implemented.

No blocking resume-correctness defect on this head. A stored Created row can be loaded after the live release is suspended or retired. Missing rows return None. Later stored states and numeric-like lookup refs fail closed.

Do not self-approve or merge until exact-head checks and independent last-push approval land. Prefer this head over persist-only #106 and #61. Do not land draft #100 over 0014.

Non-blocking follow-ups on this slice:

AS_BUILT_SCHEMA.md table still cites predecessor #61 while the body/TRACEABILITY/ERD cite #109.

Headline invariant is not yet proven in one test: persist while published → suspend/retire → AssessmentSession::new is InstrumentReleaseUnavailable → load/from_persisted_created still returns the original Created identity and Activate still works.

from_persisted_created InvalidReference is only asserted for session_ref; participant/release/version numeric-like cases are missing.

Load query_opt database failure is untested (persist has the analogous case).

HTTP session start must keep using AssessmentSession::new (publication + locale gate). Reconstitution is load-only. Next buyer-facing gap after this lands is HTTP session-creation (needs #87 RFC 9457) or later-state/command-history persist so Activate/Resume survives restart.

_{Sent by Cursor Automation: Fix Issues}

cursor

Stale comment

Review outcome

COMMENT on head ff82de8d6d7763a4a6be54a36251a22ce05f9584 (this automation cannot REQUEST_CHANGES on its own PR).

The load design is sound. AssessmentSession::from_persisted_created does not take a live InstrumentRelease and never calls accepts_new_sessions(). Persist stays Created-only, exact-replay, fail-closed rebinding. Missing load returns none; later stored states and numeric-like lookups fail closed. Docs stay Active PR, not Implemented.

Do not merge this head. Production error arms this slice added are untested, and the 100% owned-production line/branch gate will fail.

Must-fix before land

Load query_opt Database arm is uncovered. Persist already covers missing-table failure on insert. Drop the relation or abort the transaction, call load_assessment_session, and assert Database(_).

from_persisted_created InvalidReference is only tested for session_ref. Add three cases with a numeric-like value in the participant, release, and version slots.

Load InvalidStoredIdentity is untested. Persist a created row, weaken one identity CHECK, corrupt that column, and assert InvalidStoredIdentity.

Load ValueOutOfRange is untested. Drop the timestamp CHECK, store -1, and assert ValueOutOfRange.

docs/architecture/AS_BUILT_SCHEMA.md still names Active PR #61. Point that table cell at #109.

#109 is a strict superset of persist-only #106. Prefer this over #106 and #61 after those holes. Do not land draft #100 (0020) over this 0014 slice. Independent last-push approval is still required.

View PR

_{Sent by Cursor Automation: Fix Issues}

cursor

Stale comment

Review (PR #109 @ ff82de8)

No security fail-open, tenant leak, IDOR, or publication-bypass in the load path. from_persisted_created does not call accepts_new_sessions(). Missing rows return none; later stored states and numeric-like lookup refs fail closed. Persist stays Created-only with exact replay. TRACEABILITY says Active PR, not Implemented.

This head is not merge-ready. Required 100% owned line/branch coverage does not take the load query_opt error arm, the later from_persisted_created reference ? tails, or load-path InvalidStoredIdentity / ValueOutOfRange. The as-built physical-schema table still names persist-only #61.

Prefer the successor branch that covers those arms and retargets the as-built table to #109. Do not land #106, #61, or draft #100 (0020) over this slice.

HTTP start must keep using AssessmentSession::new (live published release). HTTP resume must not treat load_assessment_session(session_ref) as authorization. Independent last-push approval and exact-head checks are still required after the coverage and as-built pointer land.

_{Sent by Cursor Automation: Fix Issues}

Point TRACEABILITY, ERD, and the as-built physical-schema table at the coverage successor so queue decisions do not land uncovered #109 or persist-only #61. 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>

Processing consumption rows now require claim_deadline_at after #81. Seed the wall-clock deadline and prove binary restore preserves it.

seonghobae · 2026-08-16T16:09:36Z

Exact-head RCA: Runtime CI would fail at postgres_recovery_invariants on integration_consumption_claim_deadline_shape from #81/0019. Head now seeds claim_deadline_at and asserts exact restore equality. Please perform a bounded independent current-head review of this exact SHA against protected main a7637351. Do not update the branch or merge.

cursor

Review (PR #109 @ `d6b284c` vs `a763735`)

d6b284c is the right recovery RCA. After #81/0019, a processing consumption row must carry claim_deadline_at. The seed now writes a wall-clock deadline and the restore assertion compares source and restored timestamps exactly. That is the buyer-visible backup contract: a claimed inbox item must come back with the same database-authoritative deadline, not a rewritten clock.

The session load path is still sound and still not merge-ready.

Sound

from_persisted_created never takes a live InstrumentRelease and never calls accepts_new_sessions(). A later suspend or retire cannot rewrite stored provenance.
Persist stays Created-only, exact replay, fail-closed rebinding.
Missing row → none. Later stored state → UnsupportedStoredState. Numeric-like lookup → InvalidReference.
TRACEABILITY, CHANGELOG, ERD, and ADR-0005 stay Active PR, not Implemented.

Still blocking this head
The four ff82de8 findings are unchanged. Required 100% owned line/branch coverage does not take the load query_opt error arm, load-path InvalidStoredIdentity / ValueOutOfRange, or the later from_persisted_created reference ? tails. The as-built table still names persist-only #61.

ValueOutOfRange in session_persist_requires_read_committed_and_surfaces_database_failure is persist-only (u64::MAX insert). Display-only construction of the load variants does not cover those production lines.

What to do next
Do not land #109, persist-only #106, #61, coverage-only #121, or draft #100. Prefer #164 for Created persist/load/start-from-stored-release. Later-state lock remains #154. HTTP start remains #87 + #149 after #164. Do not add routes here.

HTTP start must keep using AssessmentSession::new (live published release). HTTP resume must not treat load_assessment_session(session_ref) as authorization. Independent last-push approval and exact-head checks are still required after coverage and the as-built pointer land. Do not update this branch for those fixes; they already exist on the successor.

_{Sent by Cursor Automation: Fix Issues}

cursor · 2026-08-16T16:20:45Z

+        &[&session_ref],
+    ) {
+        Ok(row) => row,
+        Err(error) => return Err(AssessmentSessionPersistenceError::from(error)),


This query_opt Err arm is still production code that this head never takes. Persist missing-table is covered; load after DROP TABLE is not. Required line-coverage CI will fail this line. Add a realistic load-path database failure and assert AssessmentSessionPersistenceError::Database plus the safe display/source contract. Covered on successor #164 / #121. Do not patch it on this branch.

cursor · 2026-08-16T16:20:45Z

+        created_at_unix_ms,
+    )
+    .map(Some)
+    .map_err(|_| AssessmentSessionPersistenceError::InvalidStoredIdentity)


Buyer resume/start HTTP will hit this reconstitution mapping. Tests still never produce InvalidStoredIdentity from load_assessment_session, and never produce load ValueOutOfRange. The persist-path u64::MAX case does not cover these lines. CHECK (created_at_unix_ms > 0) makes the negative-timestamp arm unreachable unless a test drops the constraint. Cover a CHECK-passing or constraint-dropped corrupt row, or delete the unreachable arm so 100% branch coverage is honest. Covered on the successor. Do not patch it here.

cursor · 2026-08-16T16:20:45Z

+    ) -> Result<Self, SessionReconstitutionError> {
+        let session_ref = normalized_reference(session_ref)
+            .ok_or(SessionReconstitutionError::InvalidReference)?;
+        let participant_ref = normalized_reference(participant_ref)


from_persisted_created is the HTTP resume reconstitution gate. tests/session_persisted_identity.rs on this head still only fails session_ref ("12345"). These later normalized_reference(...).ok_or(InvalidReference)? arms never return None. This repository has already failed Linux llvm-cov on isolated ? tails. Add independent invalid participant_ref, instrument_release_ref, and instrument_version_ref cases (numeric-like and blank). Covered on the successor. Do not patch it here.

cursor · 2026-08-16T16:20:45Z

 | `scoring_job_state` | scoring | Implemented subset |
 | `instrument_release` | instrument publication | Implemented subset |
 | `integration_consumption` | integration | **Active PR** #58 (not protected-main truth) |
+| `assessment_session` | session | **Active PR** #61 (not protected-main truth) |


This table still says Active PR #61. The section immediately below, TRACEABILITY, and ERD name Active PR #109. #61 is the persist-only predecessor and must not be the as-built pointer for this persist-and-load head. Change the table cell to #109. Do not mark Implemented. Fixed on successor #164. Do not patch it here.

cursor · 2026-08-16T16:20:45Z

+const VERSION_REF: &str = "instrument_version_big_five_ko_v1";
+
+#[test]
+fn persisted_created_identity_restores_without_a_live_release() {


This reconstitutes without a live release, so it never shows that AssessmentSession::new would now fail. Add the headline buyer case: create while published → suspend or retire → new returns InstrumentReleaseUnavailable → from_persisted_created still restores the original Created identity and Activate still works. Prefer that proof on #164 rather than this branch.

cursor · 2026-08-16T16:20:46Z

+        )
+        .expect("restored claim-deadline evidence should remain comparable")
+        .get(0);
+    assert!(


This is the correct #81/0019 contract. A processing row must keep the same claim_deadline_at after binary COPY restore, not a regenerated clock_timestamp(). Keep this exact-equality assertion; do not weaken it back to IS NOT NULL only.

seonghobae and others added 29 commits August 14, 2026 10:46

feat(session): persist created sessions bound to published releases

aa58a45

Store participant and published-release identity for SessionState::Created with exact replay and fail-closed rebinding. Command-replay persistence stays outside this first slice.

test(session): serialize shared PostgreSQL schema setup

d3ab60c

docs(schema): treat inbox consumption as protected-main after #58

303ace1

merge(main): reconcile persist PR #61 after #64

47fbc5b

fix(session): drop redundant poison-error closure

b1e82c0

test(session): cover database display and classify select failure

193c938

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.

merge(main): reconcile persist PR #61 after #62

16b1cb6

Treat landed PostgreSQL readiness as Implemented and keep #61 as the Active created-session persist slice.

merge(main): reconcile persist PR #61 after #63

ec4b27c

Treat landed migration rollback coverage as Implemented and keep #61 as the Active persist slice.

fix(session): classify replay select errors without isolated ?

1b65042

Linux llvm-cov leaves the isolated query_one ? tail uncovered unless the Err arm is an explicit match. Keep the search_path redirect test.

test(session): cover classify field conflicts and overflow

ce73e48

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.

Merge protected main into session-persistence slice

da20974

style(session): restore formatted module file

22681f8

fix(session): simplify replay database error path

3305f09

Merge branch 'main' into feat/session-persistence-20260814

d66ee3f

test(session): clean conflict classification sink

7c63cfd

test(session): schema-qualify fault cleanup

e4bf25e

docs(session): document persistence helper contracts

658f5ab

docs(session): complete immutable replay contract

a151779

fix(session): remove unreachable persistence reference validation

92eed0d

style(session): terminate persistence module with newline

5922c4d

fix(session): restore coverage-visible replay error branch

9a33e57

test(session): assert classify-select database error identity

60d4be9

The replay SELECT failure constructed Database evidence without checking its safe display text or source, leaving those two production lines uncovered on Linux.

test(session): cover isolation-probe database failure

077386c

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.

test(session): instantiate Database error wrap in the library

9ec26ad

Merge branch 'main' into feat/session-persistence-20260814

c7f854f

Merge branch 'main' into feat/session-persistence-20260814

e95f484

style(session): use let-else when wrapping a failed database connect

7f7ea0f

Satisfy clippy::manual_let_else in the library test that instantiates AssessmentSessionPersistenceError::Database.

cursor Bot requested a review from seonghobae August 16, 2026 15:19

docs(session): record created-session load as Active PR #109

ff82de8

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>

cursor Bot mentioned this pull request Aug 16, 2026

fix(session): complete created-session persist evidence #106

Closed

seonghobae marked this pull request as ready for review August 16, 2026 15:20

cursor Bot commented Aug 16, 2026

View reviewed changes

Comment thread docs/architecture/AS_BUILT_SCHEMA.md

Comment thread tests/session_persisted_identity.rs

cursor Bot mentioned this pull request Aug 16, 2026

fix(health): keep serving probes after a dropped connection #117

Closed

3 tasks

cursor Bot commented Aug 16, 2026

View reviewed changes

Comment thread src/postgres_assessment_session.rs

Comment thread src/postgres_assessment_session.rs

Comment thread src/session.rs

Comment thread docs/architecture/AS_BUILT_SCHEMA.md

This was referenced Aug 16, 2026

test(session): cover created-session load failure arms #121

Closed

test(health): cover accept ConnectionReset and postgres drop #122

Closed

cursor Bot mentioned this pull request Aug 16, 2026

feat(session): persist command history so Activate survives restart #125

Closed

test(recovery): preserve inbox claim deadline evidence

d6b284c

Processing consumption rows now require claim_deadline_at after #81. Seed the wall-clock deadline and prove binary restore preserves it.

cursor Bot mentioned this pull request Aug 16, 2026

feat(session): start sessions from locked stored publication state #180

Closed

cursor Bot commented Aug 16, 2026

View reviewed changes

cursor Bot mentioned this pull request Aug 16, 2026

test(recovery): preserve inbox claim deadline evidence #177

Closed

2 tasks

seonghobae closed this Aug 16, 2026

Uh oh!

Conversation

cursor Bot commented Aug 16, 2026 • edited by seonghobae Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Superseded by repaired durable-session line

Uh oh!

cursor Bot commented Aug 16, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review (ff82de8)

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review outcome

Must-fix before land

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review (PR #109 @ ff82de8)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seonghobae commented Aug 16, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Review (PR #109 @ d6b284c vs a763735)

Uh oh!

cursor Bot Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cursor Bot commented Aug 16, 2026 •

edited by seonghobae

Loading

cursor Bot left a comment •

edited

Loading

Review (`ff82de8`)

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

Review (PR #109 @ `ff82de8`)

Review (PR #109 @ `d6b284c` vs `a763735`)