Repository navigation
fix(memory): make session finalization durable and owner-aware - #1005
mergify[bot] merged 18 commits into
Conversation
ReviewIs the issue real?Yes. Issue #996 documents five concrete, source-cited defects against upstream
Is the fix correct?Largely yes, and the test suite is unusually rigorous for this class of problem — it doesn't just cover happy paths:
One thing worth flagging relative to memoria#310 (which I reviewed separately and found a P1 there: idempotent-store payload fingerprinting hashes No bugs found in Astra's own logic that would break the durability/safety guarantees this PR exists to provide. P3 — the cross-repo duplicated ID-derivation constant is a maintenance fragility, not a correctness bug today; worth a shared contract test or a documented protocol version string between the two repos before either side's hash formula changes independently. |
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Request changes at exact head 5c9a3cdf38f248dab1489fb860d038b843bc5a67. Reviewed against base and merge-base d2d4404056075a01284e28639436bed06a400ca6.
Two P2 recovery issues need correction:
-
Explicit close loses finalization when the journal row does not exist (
crates/services/src/session_memory_lifecycle.rs:502–504). The close transaction returns success without enqueuing work in this case. A first-turn handoff can fail or hit its one-second deadline before creating the row; that error is logged and turn finalization continues. Once canonical context commits, a later successful close still cannot recover it: both the due-work scan and canonical-input recovery start from an existing work row. The removed close-time task no longer supplies another path. Ensure the owned work row exists in the close transaction and then persist finalization intent. Add a close/restart regression with committed canonical context and no pre-existing journal, rather than seeding one throughpublish. -
The finalization snapshot silently stops at the first 500 working memories (
crates/runtime/src/session_memory/durable.rs:79–93). The new caller requests 4,096 entries, butHttpMemoriaPort::retrieve_scoped_typedclamps its one request to 500 and ignores the backend'snext_cursor(memoria_compact.rs:872–913). With more than 500 scoped working records, the 4,096-item completeness guard cannot detect truncation. If the final working receipt is on the first page, the frozen plan excludes the remainder and cleanup can be recorded complete after deleting only that page; if the receipt is later, reconciliation retries the same first page indefinitely. Read bounded cursor pages and establish completeness before freezing the plan, retaining working state if the inventory exceeds the supported bound. Add a multi-page regression, including a final receipt outside the first page.
Review coverage included all 34 changed files and the surrounding owners/call chains: CLI attestation and exit handoff; authenticated close/finalize/status and current authority resolution; extraction preparation and canonical recovery; shared session fences, owner/session predicates, lease expiry and cancellation; frozen write/plan receipts and exact-ID cleanup; optional-stage replay; schema/deletion integration, tests and documentation. The journal extends the existing lifecycle and memory algorithms coherently, but the two boundaries above prevent the advertised recovery/completeness guarantees. I also checked the matched Memoria #310 protocol's owner/branch/key receipt and paginated-list contracts; that dependency and the operator-owned v102 preservation/rollout plan remain necessary.
Verification: all 19 astra-memoria library tests, all 47 schema-inventory tests, and git diff --check passed locally. Two isolated source-extracted probes passed with results demonstrating the issues: the unchanged close function with a SQL boundary double returns Ok, zero writes and no pending work for an absent row (the existing-row control queues correctly); the unchanged HTTP retrieval method against a loopback items + next_cursor response requests limit=500, returns 500 for a 4,096 request and leaves the completeness guard false. These are focused probes, not an end-to-end database/runtime run. I did not run local live providers or shared databases.
Before submission I independently rechecked the exact open head, identity and substantive review deduplication, and paginated all check runs, commit statuses, workflows and latest-attempt jobs. Exact-head CI is complete: 42 check runs (41 success, one neutral), no commit statuses, eight successful workflows; Test Suite 38626968798 has all 28 jobs successful and Static Checks 38626968792 has all six successful. Live run 38626962257 is successful and its log records this exact head with 8/8 cases passing. Protected-branch settings were not available through the connector; the complete visible checks/jobs were verified.
## Problem and behavior Astra issue [matrixorigin/Astra#996](matrixorigin/Astra#996) needs partial-stage retries that survive process death and lost HTTP acknowledgments. Ordinary store uses a fresh UUID plus approximate/vector deduplication; concurrent requests can create duplicates and similar content can reuse another operation's session metadata. That is insufficient evidence for a durable archive-before-purge decision. This adds an optional, atomic single-memory idempotency protocol: - `POST /v1/memories/idempotent` requires an exact operation key and retains existing authentication, scoped memory-write permissions, branch/main guards, sensitivity checks and per-user SQL routing. - The authenticated owner, branch and key derive a deterministic receipt. The SQL primary key arbitrates concurrent inserts; matching replays return the actual persisted record, while conflicting payloads and inactive receipts fail. Only the insert winner reports a new memory and schedules entity extraction. - Ordinary single-store requests remain compatible and may opt in with a key. Batch store rejects keys before mutation. The receipt lasts while its record/tombstone is retained; physical erasure ends that lifetime. The dedicated route makes old backends fail before a write, so Astra must not fall back to ordinary store after a missing or malformed acknowledgment. Deploy this Memoria change before the matched Astra durable-finalization client in [matrixorigin/Astra#1005](matrixorigin/Astra#1005). Both PRs are drafts for review. ## Review follow-up Addresses [the review comment](#310 (comment)): - Ordinary vector dedup excludes receipt-bearing rows, protecting them from metadata refresh and superseding. The public HTTP regression uses deterministic embeddings and verifies that ordinary-row dedup still works and the original keyed receipt remains replayable. - Fingerprinting explicitly sorts nested JSON objects. The current dependency graph does not enable `serde_json/preserve_order`, so conversion through `json!` already sorts HashMap keys; reordered multi-key/nested metadata is now covered by regression tests and remains stable if that feature is enabled later. - API memory responses strip the reserved fingerprint while preserving caller metadata; single/batch writes and structured filters reject the reserved field. Tests cover response omission and these rejection paths. ## Validation Passed against newly created, isolated local MatrixOne databases and synthetic identities: - `cargo test -p memoria-service --lib idempotency_tests` (1 test) - `cargo test -p memoria-service --test idempotent_store_db_it -- --test-threads=1` (1 test; 8 concurrent replays, conflicting content/session, owner separation, soft-delete retry) - `cargo test -p memoria-api --test idempotent_store -- --test-threads=1` (1 public HTTP test; repeated receipts, missing/conflicting keys, authentication, owner separation and batch rejection) - `cargo clippy -p memoria-api -p memoria-service -p memoria-storage -p memoria-core --lib --test idempotent_store --test idempotent_store_db_it -- -D warnings` - `git diff --check` Broad all-target Clippy also exposes pre-existing warnings in `subject_id_filter.rs`, `governance_pool_budget_e2e.rs` and scheduler lib tests; these unrelated files are unchanged. No QA/production writes or live provider calls were used. No schema migration is needed. Host review used the open-code-review delegate skill: all 9 selected Rust files reviewed; 2 documentation files read separately; 0 skipped, 100% coverage of this change. Documentation describes the API, deployment dependency and deletion limits. ### Review follow-up: resolved branch receipts Fixed the blocking branch-scope finding from [the exact-head review](#310 (comment)). Keyed writes snapshot the effective checkout (or normalized explicit branch), resolve its table once, and retain the routed SQL store/table for receipt lookup, insertion and conflict replay across embedding. An omitted branch and the same explicit branch now share one receipt; invalid branches fail closed. Clients requiring a fixed retry scope must send it explicitly; Astra #1005 pins its durable reads, writes and cleanup to main. Validation: the public authenticated HTTP tests pass (2 tests), including checked-out/explicit aliases on both keyed endpoints, branch-local row counts, checkout during a semaphore-paused deterministic embedding, cross-branch key separation and missing-branch rejection. The existing eight-way concurrent replay/conflict/deletion DB regression and receipt identity unit test pass. Clippy passes for API/service/storage/core libraries and both idempotency integration targets with warnings denied. All database tests used an isolated local MatrixOne database and mock embeddings.
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Request changes at exact head 992a78a2592970c2de65f235e6296a2886a2fb52, reviewed against current base and merge-base 106f54cff4c0e4605ec6c9002c83e025dc09aa2d.
The two earlier findings are addressed: explicit close now creates the missing journal within the canonical close transaction, and finalization establishes bounded cursor completion before freezing its inventory. The native SQLx JSON decoding change also removes the truncating journal reads. The new regressions for those boundaries pass on this exact head.
One new P2 recovery issue remains:
Best-effort stale-snapshot cleanup can overwrite the confirmed final working receipt with an ID it deletes (crates/runtime/src/session_memory/durable.rs:327–343). The new typed-read adapter records the highest-turn row from every returned page as working_memory_id, including reads made by cleanup_prior_session_memory_entries after write already checkpointed the new snapshot's receipt. Memoria lists by memory ID, so the new hash-derived ID can be outside the first 16 cleanup rows. If that page's highest-turn stale row is deleted but its response is lost, cleanup exits with an error after persisting that stale ID. Cleanup is intentionally best effort, so extraction still succeeds and clears active_request/prepared_working. Finalization then repeatedly rejects the absent receipt even though the actual final snapshot exists; no pending extraction remains to repair it. Preserve the confirmed store receipt across cleanup reads (or keep observed snapshot identity separate), and cover successful extraction followed by a lost stale-delete response and close/restart recovery.
Verification: all 19 astra-memoria library tests, all 47 schema-inventory tests and git diff --check passed locally. A focused source-extracted Rust probe confirms the finding using unchanged typed-read, delete, plan-preparation, extraction-completion and cleanup functions with in-memory database/transport boundary doubles: the no-loss control finalizes; the lost-delete-response case retains the actual final snapshot, marks extraction complete, points the journal at an absent stale ID, and fails plan preparation on all three retries. This is a focused boundary probe, not an end-to-end database run. I did not run local live providers or shared databases.
Review coverage: all 36 changed files and surrounding owners/call chains, including CLI authority attestation and exit handoff; authenticated close/finalize/status; canonical context recovery and turn admission; owner/session isolation, shared fences, leases and cancellation; frozen writes, paginated inventory and exact-ID cleanup; optional-stage retries; schema/deletion integration, tests and documentation. The architecture extends the canonical session lifecycle coherently, but the receipt mutation above breaks durable recovery on an ordinary cleanup failure. The matched Memoria #310 protocol must be deployed first, and the operator-owned v102 database preservation/rollout remains required.
Immediately before submission I rechecked the open exact head, reviewer identity, substantive review bodies and original inline anchors, all paginated check runs/statuses/workflows and latest-attempt jobs. All 15 branch-listed required checks are green; all visible CI is complete. Static Checks 38690964591 passed 6/6 jobs and Test Suite 38690964665 passed 28/28. Live run 38690964328 records this exact head with 8/8 passing; its uploaded artifact digest matches artifact 11952931714. Full branch-protection administration details are unavailable through the connector; branch-listed requirements and all visible checks/jobs were verified.
c683db3 to
f35847a
Compare
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Request changes for one additional P2 recovery defect at reviewed head f35847a0577bc2b2d36bd227d5a57202f0e58de4.
When a resumable Session has already completed CLI-exit finalization, closing it during an admitted follow-up turn can leave the memory journal permanently complete for the previous turn. The close transaction compares only the committed canonical watermark with completed_turn, so it preserves the old completion flags before the in-flight turn commits. The late handoff is then rejected by the closed/consolidated guard, and the recovery scan excludes the journal. The canonical conversation remains available, but the final turn is omitted from automatic memory extraction/consolidation until another explicit finalization request intervenes. See the inline finding at session_memory_lifecycle.rs:499–503.
Evidence: I traced the authenticated close handler through DatabaseSessionService::update_session, the shared Session fence and execution-slot admission, terminal memory publication, canonical context commit, and the recovery scan/claim. An isolated Rust probe compiled the exact current SessionMemoryWork implementation and close/publication checkpoint bodies: closing before the new canonical commit produced pending=false / complete=true; publication of the finishing turn returned closed generation is already consolidated; the control closing after canonical advancement remained pending. This is a source-derived state-transition check, not a database integration reproduction. Please add the real database regression for this ordering.
The previous findings are fixed in this revision: close creates a missing journal; destructive inventory follows bounded cursor pages before freezing; native JSON reads preserve large journals; stale cleanup reads preserve the confirmed working receipt. I reconciled both substantive historical reviews using their body SHAs and all three original inline anchors; none reviews this head.
Review coverage included the 42-file PR, the c683-to-current harness delta, authenticated CLI/server entrypoints, owner/consent resolution, leases and cancellation, database fencing and deletion, retry/idempotency receipts, resource bounds, and failure/boundary tests. The canonical Server now owns remote finalization; CLI hands off to it, while the existing extraction/governance algorithms remain shared. The matched Memoria protocol source is compatible and PR #310 is merged; deployed backend/schema readiness was not exercised.
Validation:
- Local:
astra-memoria19/19 andastra-test-harness615/615 passed; schema inventory 47/47; formatting andgit diff --checkpassed. - Python harness progress: 9/11 passed. The two supervisor/cancellation failures also reproduce unchanged on base
b95f23418b73600039eac5b52059132dcffbf29a; they are not attributed to this PR. - CI: all 15 required contexts succeeded. Full paginated inventories contain 40 checks, 0 statuses, 6 workflow runs, 7 suites and 38 jobs; required checks match GitHub Actions app 15368 and their job/run/suite provenance. The database CI logs show all 8 durable-runtime and 5 journal integration cases passing, within 100/100 runtime and 475/475 service tests. These test jobs used the PR merge with b95.
- Optional live harness: run 38833651631 checked out this exact head and passed 8/8, including the missing-child-model case.
PR base and merge-base: b95f23418b73600039eac5b52059132dcffbf29a. Current main: c801986631b03a2a8d31e249845b68df45a994de (one commit ahead); I inspected that persistence-read delta and it does not repair this finding. Open state, fork/author, connected identity XuPeng-SH, required-check configuration, applicable rulesets and review history were revalidated. The admin protection-detail endpoint is unavailable to this integration; required contexts/app bindings are exposed by the branch response.
No product changes, remote CI reruns, shared-service/database access, deployment or paid provider calls were performed. Local real-DB validation and large-scale contention measurements were not run. Requested reviewer configuration: gpt-6-astra / xhigh; this execution API does not expose independently verifiable backend model/effort metadata.
f35847a to
32f1149
Compare
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Request changes for one remaining P2 settlement/recovery defect at 32f1149f925cb3dc75a4bad85fbaed747dff35e3.
The new execution-slot check fixes the originally tested close-while-running ordering. A later interval remains: runtime commits terminal status and releases the Session execution slot before it commits the canonical context head. If the bounded memory handoff was missed and explicit close falls in that interval after a prior successful CLI-exit finalization, both checks at session_memory_lifecycle.rs:511–514 see the old generation as complete. Close keeps complete=true; canonical turn 2 then commits, but recovery never selects the journal. The conversation is retained while automatic memory extraction/consolidation remains at turn 1.
Reachability is established by the actual caller chain: terminal settlement invokes sync_session_execution_slot_after_status_tx (runs.rs:12977–12988), whose terminal branch removes the slot (runs.rs:13733–13735); the transaction commits in persistence.rs:1348–1350. Only afterward does runtime call commit_canonical_turn (server/run/lifecycle/mod.rs:16560–16570). The one-second handoff explicitly permits failure without blocking terminal settlement (finalization.rs:1044–1054). The new regression commits the canonical turn before releasing the slot (session_memory_lifecycle_db_it.rs:529–547), so it does not exercise this production ordering.
Validation against a newly created, isolated local MatrixOne container:
- A scratch integration harness linked the unchanged production crates at this exact head and reused the current six journal DB regression bodies/fixtures. All six passed, including the new occupied-slot late-handoff and missed-handoff cases.
- An additional service-boundary regression used real Run admission, canonical reservation, terminal status/slot release, owned Session close, canonical commit, and a restarted journal store. It failed the required pending assertion with
pending=false settled_turn=1 canonical_turn=2 due=false claimable=false. - No remote memory backend or model was involved. The container was removed after validation; the product checkout remains unchanged.
- Workspace formatting and
git diff --checkpassed.
Please retain close/recovery work through the outstanding canonical settlement, rather than treating slot absence alone as completion. Add the terminal-release → close → canonical-commit ordering with a missed handoff to the database regression.
I reviewed the full PR ownership/call-chain map, database I/O and owner/session isolation, cancellation/leases, retries and stage receipts, bounded resource use, and error/boundary coverage. The new helper adds a bounded owner/session lookup under the existing fence; it introduces no separate lifecycle or authority owner. Unchanged memory, CLI, and harness implementations reuse the prior source review and validation evidence. Earlier missing-journal, paginated-inventory, native-JSON and confirmed-receipt fixes remain intact. All three historical review bodies and four original inline anchors were checked; none reviews this head.
CI was revalidated from complete paginated inventories: 41 checks, 0 commit statuses, 7 successful workflow runs, 8 suites and 39 jobs. All 15 required contexts succeed and match Actions app 15368 and their exact job/run/suite provenance. CI reports 477/477 service and 102/102 runtime database-lane tests passing, including all six journal and eight durable-runtime cases. The optional live harness run 38845744907 checked out this head and passed 8/8.
PR base/merge-base: c801986631b03a2a8d31e249845b68df45a994de. Current main: 40012430c17977ac6bbda24c24a1800615710a04; its tree is identical to that base. Open state, author/fork, identity XuPeng-SH, required contexts/app bindings, applicable rulesets and review history were revalidated. The denied classic-protection endpoint was not retried.
Limitations: the additional regression exercises the real service/database boundary with production ordering established by source tracing, not a full live HTTP/model turn. No large-scale contention measurement or deployed backend/schema rollout check was performed. Requested reviewer configuration is gpt-6-astra/xhigh; backend model/effort metadata is not exposed by this execution API.
32f1149 to
0adda4c
Compare
Merge main b091b23 into the Memoria finalization branch. Preserve upstream strict unexecuted admission checks and retain configured-name versus Offering ID negative controls across all shipped rejection cases.
Return closed reason codes from the canonical child custody verifier, preserving verdicts, failure classification and semantic judge admission. Publish the typed diagnostics through safe progress and the strict CI summary consumer without evidence payloads or identities. Cover successful proofs, contradictions, unavailable physical execution and result windows, and private-data exclusion. Rebase the PR onto main 45f5698 before this follow-up.
Reuse the fenced execution-slot idle proof when explicit close evaluates a previously finalized generation. Retain work for a turn admitted before close even when its memory handoff and canonical commit have not arrived yet. Cover prior exit completion, actual Run admission, close before publication, normal and missed handoffs, canonical settlement, restart recovery, and completed idle repeated closes with a MatrixOne integration regression.
0adda4c to
6ba9a29
Compare
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Reviewed the complete current PR (44 changed files) at head 40cbadd against integrated main/base 7375abe. No material changed-code defects found; approve.
The earlier findings are addressed in the current source:
- Explicit close creates durable work even when the bounded input handoff never created a journal. A completed prior exit generation is invalidated while an admitted follow-up still owns an execution slot or live canonical writer/reservation authority. The owner/session fence and database-time expiry guard remain intact (journal and settlement guard).
- Terminal Run settlement and slot release still precede canonical context commit. I traced the integrated runtime through its atomic commit-and-writer-release path and verified that close in the intervening gap remains discoverable after restart. The new journey wait observes the actual canonical watermark, retains its exact-turn assertion, and does not treat settlement_finished as context-commit proof (runtime commit, journey assertion).
- Server finalization publishes the bounded durable input before terminal completion while retaining post-loop extraction dispatch ownership. The new DB test asserts publication without premature dispatch (handoff).
- Finalization requires complete bounded inventory, preserves confirmed working receipts through stale cleanup, replays uncertain writes with frozen payloads, and checkpoints exact deletion receipts. Owner-bound authority is refreshed for operations; stale generations and incomplete evidence retain working memory (durable adapter).
I also reviewed public close/finalize/status routing, CLI handoff and strict-recall attestation, owner/session isolation, indexed DB access and lock ordering, cancellation-safe transactions, bounded worker/retry/HTTP costs, schema/deletion integration, and harness proof/diagnostic consumers. Harness diagnostics use the existing verifier and a closed public reason vocabulary; unavailable evidence is not upgraded to success.
Validation:
- Fresh private MatrixOne container: 9/9 local DB cases passed using unchanged production services—seven current journal regressions plus independent separate-release and atomic-release settlement probes. Both probes reported pending=true, settled_turn=1, canonical_turn=2, due=true, claimable=true. Container removed afterward.
- Local changed harness diagnostic tests: 2/2 passed; schema inventory tests: 47/47 passed; cargo fmt --all -- --check and git diff --check passed. Product checkout remains clean.
- Exact-head CI: all 15 required contexts succeeded with GitHub Actions app 15368 provenance verified through fully paginated checks, runs, jobs and suites. SDK/web scopes were true. Eight workflows are terminal; an earlier PR-title run was cancelled and superseded by successful title checks.
- Existing CI: services DB 481/481, runtime DB 104/104, and integration 295/295, including the changed canonical-context journey and pre-terminal handoff regression.
- Optional live harness 8/8 passed, including flash_missing_child_model_fail_closed.
The four historical review bodies and five original inline anchors were checked; none reviewed this head, and their resolved findings were revalidated rather than duplicated. Local tests use synthetic owners/data and fixture completion checkpoints, not a deployed Memoria service or live model. No remote CI reruns, paid API calls, product edits, or shared-service access were performed. Required checks were verified through accessible branch metadata and active rulesets; denied admin/protection endpoints were not retried. Requested reviewer configuration: gpt-6-astra/xhigh; independent runtime model/effort metadata was not exposed by the environment.
Merge Queue Status
This pull request spent 29 seconds in the queue, including 2 seconds running CI. Required conditions to merge
|
2 participants
Summary
CLI strict recall compares Astra session identity with Memoria record ownership even when UC or connected personal accounts use distinct namespaces. Explicit close also races detached extraction, and process-local governance cannot recover a partial archive/purge after restart or a lost response.
This completes the remaining owner/finalization work from #996 after #997:
Related issue
Refs #996. This is the remaining implementation after #997; the issue should remain open until the matched backend protocol is available and the changes have been reviewed together.
Backend dependency: matrixorigin/memoria#310. Deploy Memoria's dedicated idempotent-store endpoint before enabling this Astra image. The endpoint's owner/branch/key primary-key arbitration is needed to safely replay a request after its response is lost; vector similarity deduplication cannot provide that guarantee. Old backends fail before a write and never authorize cleanup. Neither PR should be merged automatically.
Change type
User and compatibility impact
Database rollout: this adds fresh schema contract
2026-10-14-v102. Astrarejects older completion markers and does not migrate databases automatically.
Existing deployments require an operator-owned database rollout/preservation
plan; this PR performs no reset. Upgrading only the image is insufficient.
Adds authenticated
POST /sessions/{session_id}/memory/finalizeandGET /sessions/{session_id}/memory/status. A queue acknowledgment is not a confirmed archive. CLI retains its five-second exit budget; extraction/finalization retain 20/60-second attempt budgets. Worker leases last 90 seconds, polling every two seconds with concurrency two and retry backoff from five seconds to 16 minutes.Strict session recall adds a
memory_scopeenvelope and requires a matched Server for the updated CLI. UC, connected personal and explicitly enabled self-hosted owners remain separate from Astra session authorization. Consent revocation/disconnect stops future remote operations without promising remote erasure. Astra Session deletion removes queued inputs; Memoria's retention and explicit deletion govern remote records.Architecture and complexity delta
Verification
Current head:
40cbadd5, based on main7375abe0. Targeted E2E Clippy, formatting and whitespace checks passed locally. The repaired handoff/resume regression passed real database CI and integration passed 295/295; the optional live harness also passed 8/8. All current test and static-check workflows have passed. At26271bdc, core-services passed 481/481 and core-runtime passed 104/104, including the memory regressions; earlier evidence below applies to its recorded commits/runs.Passed locally on rebased main (synthetic fixtures; designated isolated MatrixOne):
make format-checkcargo test -p astra-memoria --lib(19 tests)cargo test -p astra-services --test session_memory_lifecycle_db_it -- --ignored --test-threads=1(3 tests on fresh v102; rerun of the freshly built test binary passed)cargo test -p astra-services --test schema_assertions core_schema_catalog_matches_live_idempotent_bootstrap -- --ignored --test-threads=1(1 test; idempotent fresh bootstrap, generated catalog, rejection of unsupported old marker)cargo test -p astra-runtime --lib session_memory::durable::tests -- --ignored --test-threads=1(4 tests before the schema-version follow-up; local package debug=0)cargo test -p astra-cli --lib cli::session::session_startup::tests::cli_ -- --test-threads=1(7 tests; local runtime/CLI package debug=0)browser_identity_services_replace_generation_and_private_skill_cacheandescape_browser_login_restores_memory_and_external_discovery(2 tests)cargo clippy -p astra-memoria -p astra-services -p astra-runtime -p astra-cli -p astra-thin-client --all-targets -- -D warnings; CLI all-target recheck after fixture updatespython3 scripts/schema/test_schema_inventory.py(47 tests)git diff --checkThe first CI run exposed a missing schema inventory classification and stale
login-maintenance mocks; this follow-up classifies the journal and aligns those
fixtures with Server handoff. The positive owner fixture now contains a real
working-memory entry rather than arbitrary text. An unrelated macOS PTY timing
case also failed on that head; terminal-reader code is unchanged. Updated-head CI
is rerunning and must pass before the draft becomes ready.
A full repository
make checkwas not completed locally; the relevant all-targetRust checks are listed above. No online/live provider tests were enabled locally.
Public entrypoints and unhappy paths: authenticated exit/close/status; unauthenticated finalization; valid UC/personal/local owner namespaces and foreign identity/owner/session; unsuccessful/malformed queue acknowledgments without purge; lost episode/scene/delete receipts; optional-stage retries without re-archive; stale final-turn watermark; unsupported backend retaining working memory; lease takeover and old worker rejection; canonical-context recovery after missed handoff.
Database verification uses newly created isolated local MatrixOne databases and synthetic identities. No QA/production mutation or live model calls. CI's runtime online lane explicitly designates its isolated database for the shared test guard. Rebased on current upstream
mainat89dcde88.Host review used the open-code-review delegate skill: all 28 selected code/build files reviewed; all four documentation changes also read; 0 skipped, 100% change coverage.
Final checklist
Review follow-up: explicit Memoria branch scope
Memoria #310 now derives keyed receipt identity from the resolved storage branch and freezes its routed store/table across embedding. Astra's durable HTTP adapter explicitly selects main for archive/working writes, typed/strict reads and exact-ID cleanup, matching its deterministic main-scoped receipt verification. Normal direct memory tools retain their existing checkout behavior. The loopback/MatrixOne durable lifecycle regressions now enforce the main selector in write bodies and read/delete queries.
This branch also includes current main (
d2d44040, #1006), including the upstream live-harness model/cache configuration and child-answer verification changes. Format checks and live-harness workflow contract tests pass (6 passed, 2 optional skips). Runtime all-target Clippy passes with warnings denied against this combined source tree. The updated online durable lifecycle regressions are executing in CI; full CI is still running. Follow-up review covered all 5 changed Rust files across Astra and Memoria (100%, zero skips); both accompanying protocol/design document changes were also read.Live harness follow-up: missing configured model
The missing-model case required
invalid_requestwithout constraining how the model was selected. The canonical admission contract distinguishes an unknown configured name (model_selection_invalid/invalid_request) from an unknown Offering ID (model_offering_not_found/tool_unavailable). The probe now supplies an explicit fixed configured-name request and additionally verifies its policy mode, selector kind and requested name in canonical execution evidence. The failed result, exact error kind, no-child and no-fallback assertions remain required.The previous head's deterministic checks and four durable memory DB/loopback regressions passed. One rerun also passed the recovered-validation quality case, while the ambiguous missing-model assertion failed again. This follow-up changes only that fixture; it does not change runtime admission, quality thresholds or CI gates. YAML/JSON parsing, diff and format checks pass. All 47 strict case-loading tests pass. The live regression on
c653517passed 8/8 cases. The stricter fixture also required updating the synthetic baseline inshipped_rejection_cases_reject_extra_attempts_with_different_outcomes;5c9a3cdsupplies the complete fixed-name request and adds a negative Offering-ID-selector control, while retaining the extra-attempt rejection checks. The complete Harness library suite passes serially: 607/607, zero skipped. The initial default parallel local run hit six process/readiness timeout tests; the original timeouts were retained. All-target Harness Clippy passes with warnings denied. Full CI on5c9a3cdpasses (40 successful checks), including the 8/8 live regression, core + Server-hooks shard, online memory regressions and CLI inventory/aggregates. Follow-up review covered both changed files (2/2, 100%, zero skips).Review follow-up: missing journal and complete inventory
Commits
b56f26f7and94228684address both new recovery findings:DatabaseSessionService, reconstructs the store after restart, and recovers the final extraction and governance intent.mainbranch.The new 501-row database regression also exposed a journal read defect: MatrixOne's default
CAST(state_json AS CHAR)truncates at 65535 characters. All five journal read paths now decode the native JSON column via SQLx, with its JSON feature explicitly enabled. An additional database regression round-trips a large multibyte input through close, status, claim, fenced reload, and canonical recovery lookup. No schema change is required.Validation for this follow-up at
94228684:Review follow-up: preserve confirmed working receipts
Commit
a059246daddresses the stale-cleanup receipt review. Before this fix, a cleanup read could replace a confirmed new working receipt with a stale ID from the first 16 backend rows. A successful deletion with a lost response could then leave completed extraction permanently unable to finalize.The typed-read adapter now preserves receipt identity once the current generation has prepared its working write. Pre-write observations can still update the previous generation's receipt, while frozen-plan reads retain their existing behavior. The fix reuses the existing durable write checkpoint; it adds no journal fields or alternate lifecycle.
Two MatrixOne/loopback regressions exercise the canonical extraction writer and actual stale cleanup, acknowledged/lost delete responses, extraction settlement, owned explicit close, three restart-style claims, plan reconciliation, and final governance cleanup. The lost-response scenario places all 16 stale snapshots ahead of the confirmed hash-derived receipt in the backend list. The happy control uses one stale snapshot to stay within the existing one-second best-effort cleanup budget. Model inference uses the supported rule fallback, with no paid-provider dependency.
Validation at
a059246d:cargo check -p astra-runtime --lib --tests), formatting, and diff checks pass.glm-5.3preflight, with zero executed cases; Server readiness and the DeepSeek status check succeeded. This lane is not passing, and its published diagnostics do not identify the underlying GLM failure.Integration follow-up: latest main
a4bc6653merges mainb091b23binto this branch and resolves both harness conflicts without rewriting previous commits.cargo check -p astra-runtime --lib --tests, workspace Rust formatting, and diff whitespace checks passed.Diagnostic follow-up: child adoption failure reasons
Commit
c683db37follows a rebase onto latest main45f56984(including #1012). Existing Memoria changes are preserved.Rebase follow-up: main b95f234
b95f2341(perf(runtime): reduce status and control read pressure #1013), resolving harness conflicts while preserving upstream explicit ID/title fanout selectors, pre-execution rejection handling, receipt-identity checks, and bounded tool-outcome diagnostics. Adapted the existing typed reason codes and fixtures to those canonical checks; the safe producer and consumer share 47 reason codes.Review follow-up: close during a resumed admitted turn
Commit
32f1149faddresses the latest P2 finding.close_during_resumed_turn_recovers_after_prior_exit_finalization. That regression exercised both normal late publication and missed-handoff canonical recovery, followed by completed repeated-close controls. Remaining PR checks are still running.Review follow-up: canonical settlement after terminal slot release
Addresses the remaining settlement-window review.
Terminal Run persistence releases the execution slot before canonical context commit. The shared memory idle check now also checks the coordinator's existing live writer/reservation expiry columns, using database time and current reads under the existing Session fence. Explicit close resets prior exit completion while either execution or canonical settlement remains outstanding; destructive checkpoints use the same guard. No new lifecycle state or schema is introduced. Repeated close after settlement still preserves receipts.
The new
close_after_terminal_slot_release_waits_for_canonical_commitdatabase regression reproduces prior successful exit finalization followed by an admitted turn with a missed handoff, terminal status/slot release, close, then canonical commit. It asserts that the slot is empty and the old canonical watermark is still present before close, verifies pending work and rejection of early cleanup, then verifies restarted recovery through turn 2 and idempotent repeated close. The earlier occupied-slot cases now also commit terminal status before canonical context, matching production order. All cases additionally reject cleanup after commit while writer authority remains live.Validation at
0adda4c2:cargo clippy -p astra-services --lib --test session_memory_lifecycle_db_it -- -D warnings: passed (with the existing local debug-profile overrides).cargo fmt --all --checkandgit diff --check: passed.close_after_terminal_slot_release_waits_for_canonical_commitand all 7/7 journal database cases. This regression requires no live provider/model. Other CI checks are still running.Main rebase: preserve durable handoff and sole post-loop dispatch
Rebased onto main
7375abe0, retaining the merged runtime/storage latency improvements and sandbox fixes. The harness conflict resolutions preserve the existing typed child-proof diagnostics and fail-closed case without changing their content.Main now gives Server a sole detached post-loop memory-dispatch owner. The integration preserves that ownership while still publishing this PR's bounded durable input before terminal completion. Finalization always performs the bounded preparation, then defers dispatch when the host owns post-loop settlement. Skipping preparation together with dispatch would leave the post-loop worker without its durable input; dispatching in both places would undo main's ownership fix.
The new ignored MatrixOne regression
post_loop_owner_preserves_durable_handoff_before_dispatchcalls actual turn finalization with the post-loop-owner host and a durable service. It requires the owner/session journal input and fingerprint to be persisted before return, while extraction remains unrequested and no memory backend write has occurred. Existing direct-dispatch and deferred-owner tests remain in place.Validation:
cargo check -p astra-runtime --lib --tests,cargo clippy -p astra-runtime --lib --tests -p astra-services --test session_memory_lifecycle_db_it -- -D warnings,cargo fmt --all --checkandgit diff --checkpassed locally (using the existing local debug-profile overrides). The runtime database regressions share one test-only isolated-database guard module, preserving Clippy duplicate-module enforcement. Normal PR CI has restarted for26271bdc; the new database regression is not claimed as locally executed.CI follow-up: wait for canonical context after terminal settlement
At
26271bdc, ordinary unit-test shards and the three core online shards passed. Integration CI ran 295 tests with 294 passing;e2e_matrix_execution_handoff_resume_reaches_provider_httpobserved canonicalcompleted_turn=0while expecting 1. The failed online-core aggregate reflects that integration failure rather than additional failing test suites.Merged main #999 moved
run_settlement_finishedinto the atomic terminal transaction, which precedes canonical context commit. The E2E test was still treating that event as a canonical-commit barrier. It now polls the actual owner/session context watermark under a 15-second deadline before retaining the original exact-turn assertion. A missing commit still fails by timeout and an unexpected higher turn still fails the assertion. No product acceptance condition is weakened.Validation at
40cbadd5:cargo clippy -p astra-runtime --features e2e-hooks --test system_matrix_http_e2e -- -D warnings,cargo fmt --all --checkandgit diff --checkpassed (using the existing local debug-profile overrides). The normal PR integration rerun passed 295/295 tests, with an explicit PASS fore2e_matrix_execution_handoff_resume_reaches_provider_http.Baseline memory regression evidence at
26271bdc: core-services 481/481, including the terminal-slot-release regression, and core-runtime 104/104, including the pre-terminal durable-handoff/post-loop-dispatch regression.The separate optional live harness run completed 8 cases, 7 passing. Its failed fanout case passed root exit, successful fanout-tool count, two-child count and child-adoption proof; its hard semantic judge reported
assessment=unavailable/InfraVerificationUnavailable. Model preflight passed. The retained safe artifact does not distinguish input preparation, judge invocation or response-parsing errors, so it does not establish a model-quality failure. No model selection, grading threshold or fail-closed behavior is changed by this test synchronization fix.Current-head CI follow-up: optional live harness run 38874421050 completed 8/8 passed, with no failed, cancelled or unavailable cases. The prior unavailable semantic assessment did not recur. No grading or model change was required for this rerun. Integration passed 295/295, including the original failing recovery case; all test and static-check workflows have passed.