[Suggestion] R1-4: The archive server's new sealed-Managed-lease fallback is untested at the CLI layer; session-archive.test.ts (96 tests, untouched by this diff) contains zero references to 'managed'.

Failure scenario: acquireSealedManagedMaintenanceLease and its lease semantics are tested only via SessionService directly (managed-session-metadata.test.ts), so the one hop where the daemon actually wires them — this fallback, and the ordering 'sealed-managed first, legacy maintenance lease second' — has no witness. If the sealed-managed attempt is deleted (or mis-ordered so the legacy lease is tried first), every daemon archive/unarchive/delete of a sealed Managed session falls to acquireSessionMaintenanceLease, which cannot take the sealed schema-3 lock → runWithDaemonWriterLease returns mutationApplied: false → the route fails for exactly the sessions the design's acceptance row requires to succeed. All metadata-suite tests stay green under that mutation because they call SessionService methods directly.

grep sweep: session-archive.test.ts for [Mm]anaged → 0 matches; a legacy acquire's takeOverSealed refuses a sealed schema-3 lock, so deleting the sealed-managed arm fails the route with all metadata-suite tests green.

Suggested fix: Add a session-archive.test.ts case that seeds a sealed Managed session (as the metadata suite does) and drives the archive/unarchive/delete request path through runWithDaemonWriterLease, asserting success and that the lock file is byte-identical afterwards.

The fix must not violate: The legacy fallback must keep working for non-managed sessions — acquireSealedManagedMaintenanceLease deliberately returns undefined for legacy transcripts (sessionService.ts:916-919), so the test matrix must cover both branches, not just the Managed one.

Acceptance criterion: That new case: red if the acquireSealedManagedMaintenanceLease arm is removed (route starts failing on the sealed lock). Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-4 (still stands, re-verified 2026-10-03): The archive server's sealed-Managed-lease fallback remains untested at the CLI layer — the sealed-first fallback at :256-268 is verbatim present, and session-archive.test.ts (unchanged by this diff) still has zero 'managed' references across its 96 tests. Head has not moved since round 1 (1443597).

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-21: pump() clears the pending recovery timer on entry, but its memory-blocked exit (this.memoryBlocked = true; return; at :197-199) returns without rescheduling it — the only exit path that discards an armed wake without a replacement.

Failure scenario: Single-worker deployment, idle (zero active runs). The store holds activation X assigned to a crashed worker with lease expiring at T; a prior pump armed the recovery wake for T. At T−ε an operator submits activation Y → pump() clears the wake timer, sees candidates (Y), finds hasMemoryHeadroom() false, sets memoryBlocked, returns. No active run exists, so no execute() completion ever re-pumps; memory later frees but nothing notifies. X is never reclaimed at expiry and Y never starts, despite both being recoverable, until an unrelated submit or notifyCapacityChanged() arrives — potentially forever.

[probe] PR code: 'Timed out waiting for scheduler state.' after 60s of fake time past expiry, handled: [] (nothing fired); with the one-line re-arm added: handled: ['x','y'], both released completed. Deterministic flip in both directions.

Suggested fix: On the memory-blocked exit, reschedule the recovery wake (scheduleRecoveryWake()) — but guard against a zero-delay loop: an assigned head whose lease has already expired is itself runnable (listRunnable admits expired leases), so the re-armed delay must be clamped to a positive floor or only unexpired leases armed.

The fix must not violate: listRunnable() admits assigned session heads whose lease has expired (managed-activation-store.ts:490-507), so any re-arm on the memory-blocked path must not compute a zero delay from an already-expired lease.

Acceptance criterion: Extend embedded-harness-scheduler.test.ts with a fake-timer case: dead-worker assignment as in the existing recovery test, then submit with hasMemoryHeadroom: () => false so the pump exits memory-blocked; advance past the peer lease expiry and assert hasMemoryHeadroom is consulted again (the wake fired). Mutation check: remove the re-arm → the assertion goes red. Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-21 (still stands, re-verified 2026-10-03): pump() still clears the pending recovery timer on entry, and its memory-blocked exit (this.memoryBlocked = true; return; at :197-199) still returns without rescheduling it — the only exit path that discards an armed wake without a replacement. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-22: The new HTTP-store suite never drives a failing or inconsistent server response — the fake hard-wires consistent single-page responses and echo-accurate receipts, so every integrity branch of the client's journal read loop (empty page / non-contiguous revisions / byteLength-vs-recordDigest mismatch, nextRevision inconsistency and early termination before head, journal-vs-head mismatch), the multi-page pagination path (limit=100), appendTransaction's commit-receipt validation, and the reachable-but-never-triggered managed_session_resource_missing 409 branch have zero coverage anywhere in the package.

Failure scenario: A refactor that drops or weakens one of these client guards — e.g. removing the nextRevision !== afterRevision check — ships green: every test here and in the contract suite still passes because the fake always returns one consistent page and a valid receipt. Against the real Java store, a >100-transaction journal then fails to restore (hang, truncation, or spurious corrupt error) or a tampered page is accepted as valid history. Likewise a commit whose referenced resource is missing server-side (the exact 409 the fake implements) hits entirely untested client behavior, including whether staged resources are retained for retry (releaseCommitted only runs on success).

[probe] mutation check in the scratch tree, both suites: unmodified PR — Tests 10 passed (10); with the nextRevision !== afterRevision guard deleted — Tests 10 passed (10), ships green; with the head-mismatch guard also deleted — Tests 10 passed (10). Both integrity guards are uncoverable by the existing tests.

Suggested fix: Add tests with a misbehaving fake: (a) a journal spanning two pages pinning pagination and the hasMore/nextRevision/head invariants; (b) a tampered recordDigest or non-contiguous journalRevision page asserting the corrupt error; (c) a commit receipt with journalRevision !== grant.journalRevision + 1; (d) a commit referencing a resource absent from both inline bytes and the server map, asserting the 409 surfaces and staged resources survive for retry.

The fix must not violate: Any new fake response must satisfy the client's strict page/receipt contract (transaction.journalRevision === afterRevision + 1, nextRevision === afterRevision, receipt journalRevision === grant.journalRevision + 1 and committedSequence === descriptor.lastSequence), otherwise the new tests fail on a different guard than the one under test.

Acceptance criterion: The new tests themselves — mutation check: delete the nextRevision !== afterRevision guard (http-managed-session-store.ts:449-450) or the !hasMore && afterRevision !== head.journalRevision guard (:451-452); the new pagination test must go red. Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-22 (still stands, re-verified 2026-10-03): The HTTP-store suite still never drives a failing or inconsistent server response — the fake still hard-wires consistent single-page responses and echo-accurate receipts, so the client's integrity branches remain unexercised. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-24: collectRefs treats any object in a record carrying the five ref field names as a ManagedSessionDurableRef and runs assertManagedSessionDurableRef on it, but event payloads contain free-form JSON positions (e.g. cancel.requested.target, typed 'json' in EVENT_SCHEMAS, validated only by assertJsonValue) that legitimately admit such key names — a schema-valid event then fails the whole transaction, or worse, silently binds a phantom resource dependency.

Failure scenario: appendTransaction is called with a cancel.requested event whose payload.target is {resourceId:'x', kind:'y', schemaVersion:1, byteLength:2, digest:'nothex'} — the event passes parseManagedSessionEvent (target is unrestricted JSON), but collectRefs throws ManagedSessionRecordError('resource ref.digest must be a lowercase SHA-256 hex digest.') and the entire append is rejected. With a valid 64-hex digest instead, the object is accepted as a ref, flows into descriptor.refs → commitResources, and the commit request carries a CommitResource for a resource that was never published — turning a user's cancel into a store-side failure or a bogus dependency.

[probe] intact PR, schema-valid cancel.requested with the five ref keys + bad digest: appendTransaction rejects 'resource ref.digest must be a lowercase SHA-256 hex digest'; with a valid 64-hex digest: commit succeeds and the POST body's resources array carries the phantom ref with no bytesBase64; control (one key renamed): clean commit, resources: []. Flip: with the skip-on-invalid patch the rejection disappears (probe then fails on the patched tree); restore re-run 3/3 green.

Suggested fix: Discover refs positionally instead of duck-typing: walk only the ref-typed fields declared by EVENT_SCHEMAS/parseManagedSessionHeader (ref, refOrNull, refs), or skip objects whose assertManagedSessionDurableRef validation fails when they occur outside declared ref positions.

The fix must not violate: The contract fixture pins that a genesis commit's resources equal exactly the header's refs (asserted via expect.arrayContaining + toHaveLength at managed-session-store-contract.test.ts:180-186), so any narrowing of ref discovery must still surface the header's definitionRef/rootSnapshotRef/baseTranscriptProof and each checkpoint.committed stateRef — commitResources(descriptor.refs) is what verifies/stages those bytes.

Acceptance criterion: A test appending a transaction containing a cancel.requested event whose target carries the five ref keys with non-ref values, asserting the commit succeeds; removal of the fix must make it fail with the 'resource ref' error. Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-24 (still stands, re-verified 2026-10-03): collectRefs still adopts any object carrying the five ref field names as a ManagedSessionDurableRef and runs assert on it — the duck-typing walk over event payloads (a free-form JSON position such as a cancel.requested target can collide with the shape) is unchanged. A same-mechanism round-2 candidate (R2-5) was confirmed and dropped as a duplicate of this entry. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-26: Input snapshotting is asymmetric across the five mutators: only cancelQueued clones its caller-supplied object (structuredClone before the serial() queue), so enqueue/claim/renew/release read and validate the caller's live objects inside the deferred callback, and a caller mutation between the call and the queue drain is silently persisted.

Failure scenario: A caller that reuses a mutable descriptor across unawaited calls — const p = store.enqueue(input, limits); input.payloadRef = 'event:a2'; await p; — gets the journal line for activation a1 written with a2's payloadRef: the durable event, the returned snapshot, and every later replay carry the wrong value, and re-submitting the intended a1 values is then permanently rejected ('reused with different data'). Deterministic trigger, exotic in practice; all current in-repo callers build fresh objects per call.

[probe] unmodified PR, caller mutates payloadRef between call and drain: journal line, returned descriptor, and replayed descriptor all carry 'event:a2'; re-submitting the intended a1 values is rejected ('was reused with different data'). Flip with the structuredClone hoist: all three carry 'event:a1'; re-submit accepted. Existing 8/8 suite still passes with the fix.

Suggested fix: Hoist const captured = structuredClone(input) (and a clone of limits in enqueue) above this.serial(...) in enqueue, claim, renew, and release, mirroring cancelQueued, and validate the clone inside the callback.

The fix must not violate: All five mutators return this.serial(...) promises and surface invalid-argument errors as async rejections; the fix must keep validation inside the serial callback so a bad identity/lease still rejects the returned promise rather than throwing synchronously (EmbeddedHarnessScheduler.submit only handles rejections around its await).

Acceptance criterion: A test in managed-activation-store.test.ts calling store.enqueue(input, limits) without awaiting, synchronously mutating input.payloadRef, awaiting, reopening via FileManagedActivationStore.open, and asserting the stored descriptor's payloadRef equals the call-time value — red against current code. Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-26 (still stands, re-verified 2026-10-03): Input snapshotting remains asymmetric — only cancelQueued clones its caller-supplied object (structuredClone :370) while enqueue/claim/renew/release still read and validate the caller's live object inside the serial queue (:312/:385/:427/:455). Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-3: The entire Managed read path of SessionTranscriptReader — restore (readManagedRestoreProjection, buildManagedSessionRestoreProjection), live restore, navigation turns, and paging (managedPageIndex, managedNavigationTurns, managedReplayPage) — has no test anywhere in this PR.

Failure scenario: buildManagedSessionRestoreProjection has zero references in any *.test.ts; session-transcript-reader.test.ts (the reader's own suite, untouched by this diff) contains no mention of 'managed'; the only tests driving a real Managed transcript (useResumeCommand.managed.test.ts, managed-session-metadata.test.ts) never reach SessionTranscriptReader. Concretely: if the Managed gate misroutes — e.g. indexHasManagedHeader misses a header, or managedReplayPage returns the wrong suffix — a daemon restore/navigation/page request for a Managed session hands back the authority's wrapper envelopes (or an empty/wrong history) as if they were the conversation, and no test in this PR fails. The data source (readManagedSessionRecords) is well tested; the reader's use of it is not.

grep sweep: buildManagedSessionRestoreProjection → 2 matches, both source (declaration + call); the seven other named restore/paging symbols → reader source only; session-transcript-reader.test.ts for 'managed' → 0 matches.

Suggested fix: Add a core test that opens a real Managed session via openManagedSession, writes user/assistant/title records through the sink, closes, then drives SessionTranscriptReader (getSelectiveRestoreProjection, readLiveRestoreProjection, navigation/page reads) against it and asserts the projected records — not wrapper envelopes — come back, including one assertion per accumulator path buildManagedSessionRestoreProjection wires (api history, last-assistant model, custom title injection, replay page suffix).

The fix must not violate: The test must not let a projected index enter the index cache — offerFreshIndexToCache skips indexes with projectedRecords — and the first-record authorization gate must stay outside the managedProjections WeakMap cache per its comment; the cache-exclusion branch is itself currently untested and should be pinned by the same test.

Acceptance criterion: That new test itself: it must go red if indexHasManagedHeader is made to return false (legacy index path selected) — today nothing in the repo fails under that mutation. Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-3 (still stands, re-verified 2026-10-03): The entire Managed read path of SessionTranscriptReader (restore, live restore, navigation turns, paging) still has zero direct test coverage — session-transcript-reader.test.ts has no reference to readManagedRestoreProjection / indexHasManagedHeader (grep-verified round 2). A same-claim round-2 candidate (R2-26) was confirmed and dropped as a duplicate of this entry. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-18: readManagedRestoreProjection's invariant comment is factually wrong for three of the five shapes it names: the sink admits and routes goal_state (managed-session-record-sink.ts:87 → commitGoalState), file_history_snapshot (:86 → commitFileHistory), and session_source (:88 → commitSessionSource) into enabled domains today, and the projection feeds their domain.committed bodies back into buildManagedSessionRestoreProjection (RECORD_CARRYING_DOMAINS).

Failure scenario: A maintainer trusting 'absent by construction' skips exactly the defensive handling those shapes need — the probe for R1-10 demonstrated a Managed log containing a file-history record flowing into this projection, so the comment documents an invariant the code does not have.

[probe] the R1-17 probe's cold read returned goal_state and file_history_snapshot records from a sink-written Managed log — shapes the comment claims 'cannot appear in a Managed log at all'; canCarry admits goal_state (:85), file_history_snapshot (:86), session_source (:87).

Suggested fix: Rewrite the comment to name only the shapes actually unadmitted (artifacts, session model), and point at the enabled-domain registry as the source of truth for the rest.

Acceptance criterion: N/A (comment correction; the enabling fact is pinned by the sink's own carry tests). Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-18 (still stands, re-verified 2026-10-03): readManagedRestoreProjection's invariant comment (:3510) remains factually wrong for three of the five shapes it names — the sink admits and routes goal_state and file_history_snapshot, so the comment's serialization claim is stale. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-27: lockSchema.formatVersion is never validated at the acquire boundary: acquireInternal copies it verbatim into the active record (format_version: lockSchema.formatVersion) with no isManagedFormatVersion check, so a caller-supplied out-of-contract value (0, negative, non-integer — all type-legal) produces a record every subsequent reader classifies as malformed, and every future acquire on that session fails SessionWriterUnavailableError('Existing session writer lock is malformed') until the lock file is manually deleted.

Failure scenario: Fail-closed, and no in-repo caller can trigger it today (both call sites pass MANAGED_SESSION_FORMAT_VERSION = 1); the same one-comparison hardening as R1-25's fix would close it.

[probe] intact PR, first acquire with formatVersion 0: throws SessionWriterUnavailableError and leaves the bad lock file behind (format_version: 0, state: active); every re-acquire — same schema, baseline, certified takeover — then throws 'Existing session writer lock is malformed' (session wedged in one call). Flip with the acquire-boundary isManagedFormatVersion check: attributable error 'Managed lock schema formatVersion 0 is not a supported log format', lock file ENOENT, baseline re-acquire resolves.

Suggested fix: Validate lockSchema.formatVersion with isManagedFormatVersion at the acquire boundary before constructing the lock record.

Acceptance criterion: N/A (defense-in-depth hardening on an unreachable-today path; the failure mode is itself fail-closed). Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-27 (still stands, re-verified 2026-10-03): lockSchema.formatVersion is still copied verbatim into the active record at acquire (format_version: lockSchema.formatVersion) with no isManagedFormatVersion check — a malicious or buggy caller can wedge a session with one call, all subsequent acquires reading 'lock malformed'. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-11: The managed-sealed branch of isLockRecord re-implements, inline, exactly the validation isValidCommitProof (session-writer-lease.ts:574-581) performs on the same two persisted fields — the same safe-integer/>= 0 check on last_commit_sequence and the same /^[0-9a-f]{64}$/ check on committed_prefix_hash.

Failure scenario: These two validators sit at opposite ends of the seal→takeover round trip: isValidCommitProof gates what sealForHandoff accepts from the authority, the inline copy gates what a later acquire will parse from the sealed lock. They can drift silently — e.g. widening the hash format in one copy but not the other makes every newly sealed schema-3 lock parse as malformed on the next acquire, permanently fencing the session behind a SessionWriterUnavailableError. Today they are textually identical, so nothing is broken; the cost is the drift seed on a barrier mechanism whose whole purpose is exactness.

[probe] applied exactly the suggested substitution (isValidCommitProof(record)) in the scratch tree; schema-3 suite: Tests 7 passed | 109 skipped — every 'managed lock schema 3' case passes unchanged; restored byte-identical.

Suggested fix: In the MANAGED_LOCK_SCHEMA_VERSION sealed branch, replace the four inline proof checks with isValidCommitProof(record) (function declarations hoist; record is Record<string, unknown>, which the unknown parameter accepts).

The fix must not violate: The substituted predicate must keep requiring a safe integer >= 0 for last_commit_sequence and /^[0-9a-f]{64}$/ for committed_prefix_hash — that is exactly the body of isValidCommitProof at session-writer-lease.ts:574-581, so the replacement is semantics-preserving only while that function is unchanged.

Acceptance criterion: N/A (behavior-preserving dedup — the existing 'managed lock schema 3' tests already pin the accepted record shapes both ways). Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-11 (still stands, re-verified 2026-10-03): The managed-sealed branch of isLockRecord still re-implements inline exactly the validation isValidCommitProof (:574-581) performs on the same two persisted fields — duplicated logic stays prone to asymmetric drift. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R1-25: The managed write barrier is one-directional: SessionWriterLease.takeOverSealed rejects a sealed managed lock only when options.lockSchema === undefined, and isManagedFormatVersion accepts any safe integer >= 1 — no site ever compares the acquirer's lockSchema.formatVersion against the sealed record's format_version, so an older binary can take over a NEWER managed seal and downgrade the lock record to its own older format.

Failure scenario: After a future release bumps MANAGED_SESSION_FORMAT_VERSION (mixed-version fleet — the exact downgrade scenario the barrier documents): the old binary's open() takes over the format-N+1 sealed lock (assertSealedProofMatches checks only the transcript byte proof, not format), installs an active lock with format_version: N; the authority scan then fails on the unreadable log and the assembly's error path calls journal.abort() → lease.release(), deleting the lock while takeOverSealed has already removed the retired .sealed. record — the log is left with NO barrier, so a plain legacy SessionWriterLease.acquire can append legacy records to a Managed transcript. Mechanism verified end-to-end at this commit; the trigger requires a future counterpart format version (only one exists today).

[probe] unmodified PR: new binary seals at format 2, old binary takes over — 'PROBE takeover ACCEPTED'; after journal.abort the lock file is absent (ENOENT); a legacy acquire+append SUCCEEDED (transcript now holds sealed-by-new-binary, managed-v2, legacy-appended). Flip with the suggested format comparison: 'PROBE takeover rejected: SessionWriterUnavailableError', sealed lock intact. (Refinement: in-repo legacy writers still refuse on content, so the demonstrated harm is the at-rest seal barrier deleted, strictly worse than fail-closed.)

Suggested fix: In takeOverSealed, additionally reject when options.lockSchema !== undefined && options.lockSchema.formatVersion < observed.record.format_version (use >= semantics so a newer binary can still continue an older log — plain equality would break forward takeover).

The fix must not violate: isManagedFormatVersion (session-writer-lease.ts:563-565) accepts any safe integer >= 1, so the fix must compare values, not gate on presence; and the acquire options type pins readonly schemaVersion: 3, which both callers hardcode — do not touch the schema gate, only add the format comparison.

Acceptance criterion: session-writer-lease.test.ts, next to the managed-seal takeover cases at L2962-2990: seal a lock with format_version: 2 and assert an acquirer with lockSchema: { schemaVersion: 3, formatVersion: 1 } is rejected — goes red if the new guard is removed. Please remove the fix and confirm that test goes red before landing.

— glm-5.3-flash via Qwen Code /review (v0.24.6)

[Suggestion] R1-25 (still stands, re-verified 2026-10-03): The managed write barrier remains one-directional — takeOverSealed rejects a sealed managed lock only when options.lockSchema === undefined, and isManagedFormatVersion accepts any safe integer >= 1 with no comparison against the expected schema/floor — presence-only gating intact. Head unchanged since round 1.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Critical] R2-7: The cold projection dereferences a possibly-null turn.settled resultRef: the event schema declares resultRef: 'refOrNull', but readerFacingBody (:300-318) forwards event.payload['resultRef'] into the resource read chain with no null guard, so a schema-legal null crashes the restore with a raw TypeError instead of a ManagedSessionRecordError naming the event.

Failure scenario: A Managed log whose turn.settled persists resultRef: null hard-fails cold restore outside the typed error taxonomy — fail-loud-late. Latent today: no in-tree producer emits null.

Acceptance criterion: with the guard in place, a synthetic log persisting resultRef: null restores as a typed ManagedSessionRecordError (or is skipped per the warn path) instead of a raw TypeError. Please remove the fix and confirm that test goes red before landing.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Critical] R2-8: managedNavigationTurns (:2706-2724) omits promptId entirely and assigns turns with overwrite = — the physical index builder fills promptId from daemonPromptId and uses first-wins ??=. The function's own comment (:2700-2705) claims the rules mirror the index builder's; they do not, on both content and precedence.

Failure scenario: Managed restore consumers reading TranscriptNavigationTurnHint[] cannot associate turns with prompt ids, and duplicate-turn resolution collapses differently for Managed sessions than for legacy ones — the same session navigates/paginates differently depending on which engine built its index.

Acceptance criterion: a Managed restore's navigation hints match the physical builder's (promptId present, first-wins on duplicates) for the same replayed records. Please remove the fix and confirm that test goes red before landing.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R2-10: readSessionTranscriptSnapshot is exported with zero production callers (its only caller repo-wide is managed-session-authority.test.ts:1383 — grep-verified); buildManagedSessionRestoreProjection is likewise exported beyond its single internal call at :3530. New public surface over the exact restore invariant whose doc contract round 1 showed was wrong for three of five shapes (R1-18).

Failure scenario: None today — the export widens audited surface the project has not committed to; any future caller builds on a contract that is documented incorrectly today.

Suggested fix: Drop the exports or mark @internal unless a consumer is about to land.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R2-21: The turn_result rejection guard (sink.ts:286-293) has four disjuncts, but its only rejection test (:476-487) exercises just the missing-promptId shape — turnId === '', outcome === '', and non-string outcome/turnId are unexercised anywhere (grep-verified: zero shapes across the file). The test's name ('refuses a turn result with no prompt id or state') overstates its coverage.

Failure scenario: A regression narrowing the guard to promptId-only goes green silently.

Suggested fix: Table-drive the four guard disjuncts into explicit cases.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R2-22: session_source and file_history_snapshot success write paths, plus carried system subtypes' write/projection round-trips, have no sink-level tests: zero session_source hits in the suite (grep-verified), file_history_snapshot appears only in the unmapped-rejection case, and carried subtypes are mostly exercised as canCarry booleans only.

Failure scenario: A success-path commit of a malformed session_source / file_history_snapshot record (or a carried subtype the projection cannot reproduce) slips through until cold restore breaks on an integration machine.

Suggested fix: Add sink-level success write/projection round trips for session_source, file_history_snapshot, and each carried system subtype.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R2-23: commitContextCompacted captures fromSequence/toSequence (:392-393) before the resources.publish suspension (:397-400 — outside runSerial), then commits sequence: toSequence + 1 (:413). Sibling paths either derive sequence at commit time or publish inside runSerial; a concurrent commit landing in the publish window leaves the compaction's claimed range stale.

Failure scenario: A concurrent sink write during the publish window makes commit() fail closed with ManagedSessionConflictError on the stale sequence (impact is rejection, not corruption); whether sink.write is concurrently re-entered today is unverified — line-level mechanics verified.

Suggested fix: Derive the sequence at commit time like appendExecutionEvent does, or move the publish inside runSerial.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)

[Suggestion] R2-28: The managed-resume rejection test is not environment-hermetic: nothing in its setup clears QWEN_RUNTIME_DIR, while Storage.getRuntimeBaseDir() prioritizes that ambient variable over setRuntimeBaseDir (priority pinned by storage.test.ts:125) and the hook constructs SessionService(cwd) inheriting it. Run with an ambient QWEN_RUNTIME_DIR, the 'belongs to managed' assertion is the sole red one — causation verified statically (not re-executed: the review worktree was damaged externally).

Failure scenario: Any developer machine or lane leaking QWEN_RUNTIME_DIR goes red on an otherwise-green tree, discrediting the rejection the PR tests for. Note: reddening of the PR's own CI lanes is NOT attributed — CI environment unknown.

Suggested fix: Clear or sandbox QWEN_RUNTIME_DIR in the test setup, mirroring the sibling core tests.

— kimi-k3@fda99a87 via Qwen Code /review (v0.24.0)