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 #121 can persist and load a Created session after a later suspend or retire, but it has no start boundary. persist_assessment_session accepts any Created aggregate, including one from from_persisted_created. A buyer who starts a session after the release is suspended would otherwise persist a reconstituted identity and bypass AssessmentSession::new.
Add created_session_for_start / start_created_assessment_session so a new session calls AssessmentSession::new from a currently published release and then persists.
Fail closed on unpublished/suspended/retired releases, locale mismatch, invalid refs, and persist-path database failures before an unpublished start can insert a row.
Retarget caller-facing persist errors so a negative stored timestamp and a later stored state tell the operator what to repair. Cover blank-key load InvalidReference.
Record the start boundary in ADR-0005, TRACEABILITY, as-built/ERD/UML, and CHANGELOG. This slice remains Active PR, not Implemented.
cargo test --test postgres_assessment_session_persistence (needs TEST_DATABASE_URL)
This is the successor to #121 for Created persist/load/start. Prefer this head over #121, #109, persist-only #106, and #61. Do not land draft #100 (0020). Do not merge #125. Later-state/command-history landing is #129; rebase #129 onto this head after it lands rather than opening another later-state PR. 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 reason will be displayed to describe this comment to others. Learn more.
Stale comment
Do not merge #138 in parallel with the command-history lineage.
created_session_for_start / start_created_assessment_session correctly force AssessmentSession::new so a suspended or retired release cannot begin a new session. That start boundary is the right product cut. This head is still the #121 created-only persist/load stack: no assessment_session_command, no stale-prefix reject, no header-row FOR UPDATE. A buyer who Activate/Pause on this head still loses later state on restart.
Landing vehicle for persist/load/command-history is #146 (successor of #129/#125). Rebase the start boundary onto #146 after that lock head is green; do not open a third persist lineage. HTTP POST /v1/sessions and RFC 9457 remain the next buyer-visible gap and still depend on #87. Do not land #100.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Created persist/load/start landing vehicle
Head bb18b06 is the correct successor of #121/#109/#106/#61 for Created-session persist, load, and start_created_assessment_session.
Verified:
Start is AssessmentSession::new then persist. Unpublished/suspended/retired fail before insert.
Load uses from_persisted_created and does not re-check publication.
Invalid stored identity, out-of-range timestamp, and later stored state fail closed.
HTTP / OpenAPI / POST /v1/sessions were not invented on this branch.
Activate-after-restart is out of this Created-only slice; later-state/command-history remains #129 (and any successor that rejects stale shorter history). Do not land #100 (0020).
Next buyer-visible gap after this lands is HTTP start, which still needs #87 and #98. Do not open another Created persist PR.
Independent last-push approval and exact-head required checks remain required. This review does not approve.
The reason will be displayed to describe this comment to others. Learn more.
Review of bb18b06 (Created persist/load/start)
No blocking correctness or security defect on this head. Unpublished, suspended, locale-mismatch, numeric/blank refs, and zero timestamps fail in AssessmentSession::new before persist_assessment_session runs. Load reconstitutes via from_persisted_created and is not treated as authorization. Exact replay is idempotent; rebinding and later stored states fail closed. Docs keep this slice as Active PR #138, not protected-main Implemented.
Do not merge from this comment. Independent last-push review is still required. Prefer this head over #121, #109, #106, and #61. Do not land draft #100. Later-state/command-history landing remains #129 (do not merge #125). HTTP POST /v1/sessions remains #87 + #98 + the HTTP landing vehicle (#149); do not add routes on this branch.
Residuals that belong on later slices, not a competing start PR
persist_assessment_session still accepts any Created aggregate, including reconstitution. ADR-0005 already says persist is not the start boundary. HTTP must call start_created_assessment_session / created_session_for_start.
Start trusts the caller-supplied InstrumentRelease. Compose with #98 published-release load in one transaction before HTTP start.
Retired coverage lives on session_release_binding; optional on a successor test only.
Next action
Keep #138 as the Created persist/load/start landing vehicle. Reviewers: approve only an unchanged check-clean head. Do not open another start-composition PR (#153 is a same-title duplicate — close or retarget it onto this head). After this lands, rebase #129 onto it.
Retarget as-built, ERD, and TRACEABILITY from in-memory-start #138 to
the stored-release composition landing vehicle.
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
Review of bb18b06 (Created persist/load/start)
No blocking correctness or security defect on this head. Unpublished, suspended, locale-mismatch, numeric/blank refs, and zero timestamps fail in AssessmentSession::new before persist. Load is not authorization. Docs keep this as Active PR, not Implemented.
Landing vehicle is now #164. That successor keeps this persist/load/in-memory-start contract and adds start_created_assessment_session_from_stored_release so start reads stored publication state in the same transaction. Prefer #164 over this PR, #121, #109, #106, #61, and #98. Do not merge this PR in parallel with #164.
Do not merge from this comment. Independent last-push review is still required on #164. Later-state/command-history landing remains #154 (do not merge #125). HTTP POST /v1/sessions remains #87 + #149 after #164; do not add routes here.
Pushed e307a2b1: seed claim_deadline_at on the processing recovery fixture so rust no longer fails integration_consumption_claim_deadline_shape after #81. Restore still has to keep the exact database-authoritative deadline.
This remains the created persist/load/start landing. Do not merge until exact-head checks and independent last-push approval succeed. Rebase #129 onto this head after it lands. Never self-approve.
Hourly loop: prefer this start slice over closed #125. Exact head e307a2b1 unit suites session_start, session_persisted_identity, and session_release_binding are GREEN locally. Repo Runtime CI was rerun on this unchanged SHA after org-queue cancellation. Keep #146 (header lock) as the successor after this lands. 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.
Hourly loop: preferred session-start slice. Exact head e0a41bab had only CodeQL after the main merge because Runtime CI/SAST/Security were action_required. Approved those first-time workflow runs; rust/coverage now queued. Keep #146 draft until this lands. Do not merge until exact-head checks and independent last-push approval succeed. Never self-approve.
Hourly loop: this remains the preferred session-start landing. Exact head 7c240849 is MERGEABLE and last observed check-clean; merge is still BLOCKED on independent last-push approval. Keep #146 (header lock) draft until this lands. Left #54 (already merged).
Closing as superseded by protected-main implementation. Current protected main 5544149ca5dc55d2bfc3402cc59c03c44830de5f already contains the stronger persisted-session boundary in src/postgres_assessment_session.rs, including start_created_assessment_session_from_stored_release, and the persist-backed POST /v1/sessions / GET /v1/sessions/{session_ref} transport in src/session_http.rs. This branch diverged from an old base (22dc8ed…) and should not be reconciled or merged; any remaining session-command work belongs to its current landing lanes.
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 #121 can persist and load a Created session after a later suspend or retire, but it has no start boundary.
persist_assessment_sessionaccepts any Created aggregate, including one fromfrom_persisted_created. A buyer who starts a session after the release is suspended would otherwise persist a reconstituted identity and bypassAssessmentSession::new.What this PR does
created_session_for_start/start_created_assessment_sessionso a new session callsAssessmentSession::newfrom a currently published release and then persists.InvalidReference.Out of scope
POST /v1/sessions/ OpenAPI / RFC 9457 (feat(api): add safe RFC 9457 problem-details primitive #87)load_assessment_sessionas authorizationTest plan
cargo test --test session_start --test session_persisted_identity --test session_release_binding --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 #121 for Created persist/load/start. Prefer this head over #121, #109, persist-only #106, and #61. Do not land draft #100 (
0020). Do not merge #125. Later-state/command-history landing is #129; rebase #129 onto this head after it lands rather than opening another later-state PR. Do not merge until exact-head checks and independent last-push approval are satisfied.