fix(chat): persist undrained pending context across a close - #6813
fix(chat): persist undrained pending context across a close#6813rnoack1 wants to merge 1 commit into
Conversation
GPT 5.6 Review (fork) — 🔴 changes requested (blocking)Reviewed 4 of 4 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands. BLOCKING -- src/kiro_crew/history.py:416 -- Overflow splitting reorders queued context BLOCKING -- src/kiro_crew/history.py:1385 -- Failed quarantine leaves retired context hydratable BLOCKING -- src/kiro_crew/history.py:3415 -- Failed restoration strands surviving context BLOCKING -- test/test_pending_context_survives_close.py:1171 -- Tests dismantle shared isolation mid-run (origin: validation) FINDING -- src/kiro_crew/dashboard/dashboard_persistence.py:117 -- local import [BLOCK-MERGE] 5bbc14b Adjudication (Opus 4.8) — is blocking on each finding proportionate?All four adjudicable findings are empty; the block between the input markers is empty. Four fenced findings (F1–F4) remain, ruled annotate-only. I opened F1 — F2 — F3 — F4 — None supports the complete FLAG record; when torn, UPHOLD-FENCED. |
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound root-cause fix, but a disjoint channel-routing trust gate and heavy tail-case machinery ship in one revert unit, and default callers stay lossy. Watch
[DESIGN-REVIEWED] 5bbc14b |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All verification is complete. The base-tree defect is confirmed real (in-memory queue, close pops the slot, drain clears before the provider call, FIFO eviction after a 200), the persistence mechanism is reused rather than invented, and the two description-vs-diff gaps are confirmed. Composing the review. First-Principles-Verdict: CONCERNS Declared rider 10 ( Not justified as shipped
What this change shipsInventory (10 items, capped — the change has more) — 6 justifiedIntent: stop the gateway silently destroying background context it acknowledged with 200 — a FIX (defect confirmed on base:
Capped at 10: also ships a new 409 Watch
[FIRST-PRINCIPLES-REVIEWED] 5bbc14b |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
b825b9a to
4cb86c6
Compare
4cb86c6 to
de97fff
Compare
de97fff to
69bf44a
Compare
69bf44a to
bfbb1f6
Compare
45020bc to
032d191
Compare
032d191 to
5960d32
Compare
UX Review (Fable 5, fork) — 🟡 CONCERNSUX-level review of The base tree confirms the screenshots are PR-added (not on disk), and the capture script only covers the artifact page. I have everything needed: the diff adds two dismissible failure notices (artifact page: two title variants; Papyrus: one), all localized across 13 catalogs, using the established UX-Verdict: CONCERNS Honest, well-layered failure notices — but none has been seen by a cold reader, and the PR body promises an auto-retraction the code deliberately doesn't ship. Watch
Evidence gaps
Suggestions
[UX-REVIEWED] 5bbc14b |
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
|
@rnoack1 A repo-wide audit flags five open PRs that overlap this one. Nothing on
Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong. |
Undrained /context and /note entries were in-memory only, so closing a tab discarded content the API had acknowledged; they now round-trip via session metadata.
Problem / Motivation
slot._pending_contextis in-memory only. Nothing serializes it, and the close path pops theslot from
state._slots, so undrained background context is discarded silently.A producer is told the write succeeded —
POST /api/chat/slots/{slot}/contextand.../noteboth return 200 — and then the content disappears with no trace on any surface: no transcript
row, no log line, nothing a user could go and find.
/noteat least leaves its visible halfbehind, so its content survives in the transcript.
/contextis context-only by design, so itscontent is lost outright.
Reachable by ordinary use: an app or a scheduled job queues context, the user closes the tab
before sending their next message, and the queued content is gone. Reopening from History does
not recover it, because resume constructs a fresh slot with an empty queue.
Why it matters
It is silent loss of data the system acknowledged. The 200 is a promise the close quietly
breaks, and nothing downstream can detect it — the queue reads empty afterwards,
indistinguishable from never having been written.
The asymmetry inside the existing
/noteendpoint is the clearest evidence this is unintendedrather than a deliberate trade-off: that endpoint takes real care to make its visible half
durable (its docstring calls the visible line "permanent"), while the context half it writes in
the same call has no durable representation at all.
The same gap loses context across a gateway restart, not just a user-initiated close.
Declared behaviour changes (riders)
Twelve behaviour changes ride along with the durability fix. Each is detailed under Declared
riders and accepted risks near the end of this body; they are ALSO listed here because that
section sits past the 8 KB point at which some review tooling truncates a PR body, so a reader
limited to the first 8 KB would see no declaration at all.
/contextcan now answer 429context_not_queued. The per-slot queue used to evict itsoldest entry, so the endpoint always answered 200; it now REFUSES instead, because eviction
destroyed content the API had already acknowledged. A caller that treated 200 as
unconditional must handle 429.
/notedoes NOT answer this 429: it still writes itsvisible line and reports
contextSkipped: true, so only the silent half is affected. (/notehas its own unrelated, pre-existing 429,
deferred_notes_full.)An entry posted with no
maxAgenow expires.DEFAULT_CONTEXT_TTL_SECSgives the queuea seven-day backstop where such an entry previously had no expiry of any kind. This is what
stops a refusing queue staying refused forever.
The artifact companion's context is now durable with a 1 h TTL, where it was previously
ephemeral(memory-only, withheld from the metadata line). This is the change under testrather than a side effect, but it IS a changed default for that caller and is declared as
one.
A persisted
linked_session_keyis trust-checked at hydration. It was previously adoptedverbatim at both hydration sites. An unprovable value now leaves the slot UNBOUND, so this
rider's blast radius is CHANNEL ROUTING, disjoint from context durability. Known limit,
measured rather than argued: a key carrying a literal
_exactly where the transcript stemhas one is refused, because it is byte-identical to the impostor shape the gate exists to
refuse. See
test_a_legitimate_literal_underscore_key_is_refused_and_that_is_measured.DEPRECATED: the app-kit "re-post your context on reconnect" recipe. A caller
following it now DOUBLE-INJECTS, because queued context survives the close it used to be
lost to, and nothing server-side detects the repost -- there is no idempotency key on
/context. MIGRATION: drop the reconnect repost and rely on the queue surviving, or keythe repost on the caller's own marker. The in-repo caller is adapted; an external one is
not reachable from inside the gateway.
docs/app-kit/api-reference.mdstates this./contextaccepts an optionalcontextKey, and honours it. A POST naming a key anUNEXPIRED entry from the same
sourcealready carries is a no-op returning the ordinary{ ok, pending }. It exists because the queue now survives a close, so a caller repostingafter a reload would otherwise deliver the same content twice with no recovery until the TTL.
Omit it and nothing is deduplicated. Declared in
docs/app-kit/api-reference.md; the onlyin-repo consumer is the artifact companion, which keys on the artifact version.
Papyrus's co-author context now carries
maxAge3600. It previously had no per-entryexpiry, so a co-author session left idle over an hour now loses its document context where
before it never expired. The agent re-reads the document on demand, so the recovery is a tool
call rather than a lost turn -- but it IS a changed default for that surface.
ephemeralkeeps itstruedefault, so an omitted flag stays MEMORY-ONLY. The pre-existing contract is unchanged: no caller that omits the flag begins writing to disk. Durability is opt-IN — a caller passesephemeral: falseto have an entry persisted to the session metadata line and re-seated after a close or a gateway restart, and only the literal booleanfalseopts in. Both in-repo callers pass it explicitly, so this rider changes no caller's behaviour. Declared indocs/app-kit/api-reference.mdbeside the options list._disk_meta_keyfixes a pre-existing "every save aborts after a rebind" defect. Neithercosmetic nor optional: without it the durable copy is not writable after a rebind, so the
fix this PR exists for would not hold. Declared because it changes behaviour on a path this
PR did not otherwise need to touch.
A new
user_messagescount on slot state and the slot projection. The artifact page'sstale-context notice keyed its retraction on the total message count, so the agent's OWN reply
retracted the notice before the user had said anything. The count of USER turns is a new field
rather than a client-side derivation because the client sees counts, not roles. Declared because
it adds a field to a payload other surfaces read. It exists ONLY to serve rider 12: without
the notice there is nothing whose retraction needs a user-turn count, so the two ride or fall
together.
Over-budget pending context now spills to a sidecar file beside the transcript, and a
session delete can REFUSE. Entries that do not fit the metadata line are written to
context-overflow/<stem>.jsonlrather than dropped, which is a new on-disk artifact with itsown lifecycle: it is folded back on the reads that opt in, pruned after each commit, and
removed with the transcript.
delete_sessionnow returnsFalsewhen that file can be neitherremoved nor quarantined, because deleting the transcript while it survives leaves the deleted
session's context hydratable by the next session at the same key. Declared because it adds a
file to the sessions directory and a refusal path to an operation that previously always
proceeded. The sidecar is NOT on the common path: a save with nothing over the budget and no
existing spill writes no file at all, so the per-slot ceiling (~484 KB against a 5 MB line
budget, needing 11+ same-transcript holders to spill) does not buy an unconditional write on
every save that merely holds queued context.
Two new user-visible failure notices, across all 13 catalogs. A
/contextpost thatfails now SAYS so on the surface that made it: the artifact page renders
chat_context_notice_title(first share failed) orchat_context_stale_notice_title(arefresh failed, so the agent holds an older version) with a dismiss control, and Papyrus
renders the co-author equivalent. Each carries its own body, and the stale one promises the
LATEST version rather than a first share. Required rather than optional: the repo's
errors-use-error-noticerule mandates that a user-initiated failure reach anErrorNoticerather than only a console line, and silent loss on this exact path is whatthis PR exists to end -- a durable queue that fails quietly would replace one silent loss
with another. Declared because it adds prose to every catalog and a surface a first-time
reader meets only on failure. The notice retracts on the next USER message, which is what
rider 10 is for.
What changed
Persist the live entries on the slot save; re-seat them on every restore path, all through one
restore_pending_contextso they cannot drift.All four hydration sites
A slot-owned key is hydrated in four places — the sites that hydrate
channel_folder_filed:chat_persistence.py_rehydrate_slot_from_history(gateway restart, cron delivery)chat_persistence.py_apply_recent_session(recent / foldered / pinned restore)chat_handlers.pyapi_chat_slot_resume(History reopen)channel_slots.pysurface_channel_session(channel reconcile)All four seat the key, because absence of a slot-owned key means "cleared". A site that
hydrates an empty queue makes the next forced save delete the stored copy — losing context
rather than merely failing to restore it. That reaches recent/foldered/pinned sessions and
reconciler-surfaced channel slots, which the Slack thread backfill shares this queue with.
This is also why the save writes the key on every save rather than only when
closed: anon-close save that omitted it would clear a copy an earlier close had written. Writing it
unconditionally also covers a crash between the enqueue and the next message.
Concurrent drain vs. flush
The save runs in an executor thread while the drain runs on the event loop, so a flush can
export the queue, the loop can drain it, and the write would then persist entries already
handed to the model — with a crash before the next save re-injecting them.
The slot carries a generation counter that the drain bumps, captured before the export and
re-checked immediately before the metadata line is serialized. A mismatch deletes the key,
which is also the correct end state: a drained queue should clear the stored copy. Appends
deliberately do not bump it — persisting a subset is safe and self-correcting, persisting a
consumed entry is not.
Retirement is carried by the flush, not by a per-turn forced save
The drain bumps the generation but deliberately does NOT mark the slot dirty -- arming the periodic flush there would race the very window the generation check exists to close -- and the flush persists the
emptied queue. An earlier revision instead forced an fsync'd durable save on every
context-consuming turn and awaited it before the drained text was prepended, with
shield/cancel/requeue handling around it.
That ordering was removed. Committing the emptied queue before delivery opens a window where a
crash between the commit and the prompt reaching the model leaves zero copies of content a
200 already acknowledged, which is the failure this change exists to prevent — the rebind
retirement's own comment calls a zero-copy outcome "strictly worse than a duplicate". Leaving
the retirement to the flush inverts the residual to at most one duplicate injection, which a
restart recovers from and a deletion does not. It also removes an fsync per context-consuming
turn, ~120 lines of cancellation repair, and a model-visible failure prepend.
TTL arithmetic hardened at its own site
context_entry_expiredcomputesinjectedAt + maxAge, which a metadata line carrying"maxAge": "60"turns intoint + str. Untreated, that 500s the resume, and on the restartpath raises into a broad handler that pops the slot — so the whole tab silently fails to
restore.
Fixed inside
context_entry_expiredrather than only at the restore, so every caller isprotected: the drain, the per-source cap count, and the deferred-note promotion. A
present-but-invalid value reports EXPIRED rather than "never expires", so unparseable data is
pruned instead of made immortal — which also retires a NaN
maxAgethat previously compared asnon-expiring forever. The restore additionally skips such entries so garbage never occupies a
seat.
Choosing that site over a restore-only guard is deliberate: it is where the unsafe arithmetic
actually lives, and the boundary validators (
_validate_max_age) only ever guarded the liveenqueue.
Cross-session authorization
A note stamps both halves, and the save is a late resolution point for the queued one. The
persist now applies the same
_note_authorized_elsewherefilter this function already appliesto the message window, with the matching count-gated
note_save_dropdenial. Without it, a slotrebound after the write copies one session's note content onto another's metadata line
silently — while the visible half of the same note is dropped and audited.
The restore drops foreign-authorized entries for the same reason. Unstamped entries are
untouched:
/contextand the Slack backfill share this queue and record no session.Size bound
The live ceilings permit
_MAX_PENDING_CONTEXT(50) entries of_MAX_CONTEXT_CONTENT(40 000)chars — two million characters, reachable through the documented API with no tampering — and it
would all land on the session metadata line.
_maybe_rotatecan only drop message lines. Once the metadata line alone exceeds_SESSION_MAX_BYTES, its budget loop floors at one kept message, and since_maybe_rotaterunsfrom
append, every subsequent message archives the rest of the transcript. The persistedpayload is now bounded well below that ceiling, and the bound is enforced at the DOOR rather than
at save time:
append_pending_contextREFUSES an entry that would not fit, and/contextanswers429
context_not_queued. Nothing is truncated after acknowledgement -- an earlier revision droppedoldest-first at export, which meant a caller could be told 200 and then lose its content with no
surface reporting it. A refusal is visible and retryable; silent truncation is neither.
Source label
sourceis validated on restore against the same rule the HTTP boundary applies: the draininterpolates it into
[Background context from "<source>"], so a crafted label could forge aframe boundary and make injected content read as a separate trusted block. An unusable label is
removed and the content kept, letting the drain's own default name it.
Expiry
Wall-clock across the close. The restore routes through
append_pending_context, somaxAgeand both per-slot ceilings are applied by the same code that governs a live enqueue. FIFO
eviction is GONE — this diff removes it, and a full queue now answers 429
context_not_queuedinstead of silently dropping the oldest already-acknowledged entry. Anentry with no
maxAgenever expires, which is what makes the wedge below possible.Deliberately not changed
A context prefix that has been drained still does not appear in the transcript. That
exclusion is intentional — see the raw-message capture for the Slack mirror in
_run_chat, andthe silent-consumption contract added in #4780 after an agent recited injected context verbatim
into a reply. Persisting the drained prefix would reintroduce exactly that.
There is a residual fidelity cost: a reopened session shows a user message whose reply was
shaped by text absent from the record. Addressing it collides with the silent-context promise,
so it is left out of scope rather than traded away here.
Not covered
slot._deferred_notes— a held note's context half awaiting end-of-turn promotion. It is aturn-in-flight structure on a path the close already cancels, and folding it in would widen this
change past the data loss it fixes.
Also noted and not chased:
_live_slot_resume_responsematches by slot name, else byeffective_session_key, and its own comment documents a channel-key mismatch producing two tabsfor one conversation. In that state both would seat the same entries. I could not construct the
mismatch, so I can assert only that the dedup this relies on is documented as having a hole.
Compatibility: callers that re-post on reconnect
The queue is now durable, so a caller following the old "re-post it if it must survive a
restart" recipe supplies a second copy alongside the restored one and the model sees the content
twice.
docs/app-kit/api-reference.mdstates the required change (drop the re-post for/contextand for the context half of/note).Measured blast radius in this repo — three call sites use
api.chatSlotContext:website/src/pages/ArtifactDetailPage.tsx(two sites)injectedVersionRef, which a page reload clears, so a reload re-posts alongside the restored copy.website/src/apps/papyrus/PapyrusPage.tsxWorth noting for callers: durability on
/contextand/noteis opt-IN —ephemeralkeeps itstruedefault, so an omitted flag stays memory-only and a caller passesephemeral: falseto have an entry persisted. Only the literal booleanfalseopts in. This is declared indocs/app-kit/api-reference.md. Worth noting for callers:ephemeral: truenow keeps an entry memory-only. It queues, drainsand expires like any other, but
export_pending_contextwithholds it, so it never reaches themetadata line and does not survive a close or a restart. It bounds the QUEUE's durability, not the
conversation's: once an ephemeral entry is actually injected, its content is in the transcript.
Tests
test/test_pending_context_survives_close.py— 107 tests over a realConversationLog, sothey exercise the actual metadata line rather than a mock of it.
the restart path, so the resume call site needs its own coverage.
check exists to catch. Verified to fail when that check is disabled, on exactly the
persisted-key assertion, with a guard asserting the drain really fired inside the save.
slot.
"60",[1],True, NaN, Inf, badinjectedAt) asserted to leavethe session resumable — a 200 from the endpoint and a restored tab — not merely to avoid
raising.
sourceasserted not to forge a second frame in the rendered drain output.unstamped entries survive.
over-budget entry is REFUSED at the door (and that a held note's context half is reserved, so
later
/contextcannot squeeze out content already acknowledged).source,maxAgeandinjectedAtasserted to round-trip, not justcontent.positive control on the same read so the absence cannot pass vacuously).
Regression: 5690 passed / 7 skipped across the channel, persistence, slot, restore, resume,
hydrate, metadata and rotation suites.
Gates: baselined
black,isort,flake8(zero new findings, verified by per-filebase-vs-HEAD counts), and
mypy src/kiro_crew/clean across 1167 files.One existing stub in
test_slack_mirror_context_leak.pygained the new counter field. That filealready documents the convention: the drain's requirements are satisfied at the stub rather than
guarded in the drain, because making the drain tolerate an incomplete slot would silently
disable the protection for every such caller.
Pattern harvest
Rule candidate: flag a request handler that answers 200 for caller-supplied content whose only storage is an in-process structure no persistence path writes.
That is precisely the shape of this defect. Two endpoints accepted background context, returned 200, and appended it to an in-memory queue; closing the tab dropped the slot and took the queue with it, so content the API had already acknowledged was gone with no error anywhere. Nothing connected "we told the caller 200" to "this survives an ordinary lifecycle event", which is why the loss was invisible at the call site and stayed invisible in tests.
A narrower and more mechanically checkable invariant fell out of the fix and is worth a rule of its own: for a metadata key whose absence is destructive — the slot-owned keys, where a save that omits the key deletes it — every save path must write that key unconditionally. A conditional write is indistinguishable from a delete, and the difference only shows up as data loss on some later read. That one is statically checkable: enumerate the slot-owned key set, then require each save site to assign every member of it.
Visual evidence
Opening the companion chat silently POSTs
/api/chat/slots/<slot>/contextso the agent knowswhich artifact it is looking at. This PR gives that POST's failure its own surface. Both frames
are the REAL built SPA driven by
website/scripts/capture-artifact-context-notice.mjs, whichasserts as well as photographs -- it exits non-zero unless the notice renders its own title and
carries no transport text.
The enqueue failed outright -- consequence and remedy, not the transport error:
The queue answered 429
context_not_queued-- distinct copy, so the capacity case is notreported as an outright failure:
The FIRST injection failed -- the generic title, scoped apart from the resume-freshness wording:
The generic title with the CAPACITY message renders pixel-identical to the frame above -- the notice copy does not vary by cause -- so the capture harness asserts that combination without committing a duplicate frame.
Aggregate persistence budget at the handover union. Per-slot admission (
pending_context_budget_room) bounds each queue against its own cap, but a rows-only save unions a DIFFERENT holder's queue onto the same metadata line, and neither side's admission saw the other._maybe_rotatecan only drop MESSAGE lines -- never the metadata one -- so a large enough union evicted real transcript rows.merge_pending_contextnow keeps entries in delivery order up to HALF the session byte budget, leaving the rest of the file for history.Disposition for entries that do not fit: they are SHED FROM THE TAIL AND RECORDED by
ctxIdin alogger.warningnaming the budget and the count. That is the difference from the size cap removed earlier in this PR, which skipped non-fitting entries silently so a caller told 200 could not learn its content was gone. The first entry is always kept, so a single oversized row cannot empty the queue. The bound is intrinsic to the union -- it takes no size parameter -- so all three call sites are covered and no caller can opt out.Before this change both cases wrote to the page's SAVE-error channel, so the banner was titled
Save failed -- telling a user with a dirty draft their work had not been written -- and the
429 was rewritten by
friendlyErrTextinto the tunnel rate-limit string, naming the wrong causeand promising an automatic retry nothing performs.
Reproduce:
cd website && node scripts/capture-artifact-context-notice.mjsRelease notes for the next changelog port
CHANGELOG.mdis not writable from this PR —scripts/check_changelog_history.pyrequires thefile at head to carry ONLY shipped sections (
draft_headingsrefuses an## [Unreleased]) andevery released section is frozen append-only history. So the two operator-visible changes below
are staged here for the release-porting commit rather than added to the changelog directly.
/contextcan now answer 429context_not_queued(/notedoes NOT -- it keeps writing itsvisible line and reports
contextSkipped: true). The per-slot queue used toevict its oldest entry to make room, so the endpoint always answered 200. It now REFUSES instead,
because eviction silently destroyed content the API had already acknowledged — the bug class this
change exists to close. Callers that treated 200 as unconditional must handle 429 and retry.
Known consequence, bounded: a slot holding entries with no
maxAgethat never takes another turnrefuses every later post until the queue-level backstop ages them out, where eviction self-healed
at once. A staleness horizon IS set here —
DEFAULT_CONTEXT_TTL_SECS, seven days.rider's regression surface is CHANNEL ROUTING, which is disjoint from the context-durability fix
it ships with, so it is called out separately.
_rehydrate_slot_from_historyand_apply_recent_sessionpreviously adopted a persistedlinked_session_keyunconditionally; theynow adopt only a value that names the transcript being hydrated, since that field is
agent-writable and decides where the slot ROUTES. If a legitimate spelling is ever refused the
slot stays UNBOUND and answers from its own dashboard session, so a channel thread stops seeing
replies with only a
logger.warningand a SELchat:adopt_persisted_bindingdeny event toexplain it. Two spellings were refused in error during development, both from drift between
ConversationLog._pathandtranscript_stems.DOUBLE-INJECTS, because queued context survives the close it used to be lost to. Nothing
server-side detects a re-post — there is no idempotency key on
/context— so this cannot befixed for external callers from inside the gateway. Only the in-repo caller
(
website/src/pages/ArtifactDetailPage.tsx) is adapted, by holding an in-memory injection claimaround the POST. External callers should key on their own idempotency marker instead of on the
endpoint, as
docs/app-kit/api-reference.mdnow states.Declared riders and accepted risks
Operator note — the persisted-binding gate narrows previously unconditional adoption.
_rehydrate_slot_from_historyand_apply_recent_sessionused to adopt a persistedlinked_session_keyon sight; they now adopt only a value that names the transcript beinghydrated, because that field is agent-writable and decides where a slot ROUTES. The
tightening is deliberate, but it has an operational failure mode worth knowing before it
is met in the field: if a legitimate spelling is ever refused, the slot comes back UNBOUND
and answers from its own dashboard-only session, so a channel thread stops seeing replies
with only a
logger.warningnaming the accepted spellings to explain it. Both outcomes arenow recorded in the Security Event Log (
chat:adopt_persisted_binding), so the decision isauditable rather than inferable from a rotated log line. Two spellings were refused in
error during development, both from drift between
ConversationLog._pathandtranscript_stems; that agreement is pinned bytest_transcript_naming_is_closed_over_transcript_stems.Recorded rationale — why the session metadata line, not a per-session sidecar. The
design review asked for this to be written down if the shape stays. A sidecar record with
its own lifecycle would indeed remove the "absence means cleared" coupling, the digest
compare-and-clear retirement, and the byte-budget interaction. It was not chosen because
the queue must be seated and cleared ATOMICALLY WITH the binding it belongs to: a slot's
linked_session_key, its transcript identity and its queue are read on the same hydrationand rewritten on the same save, and
update_metadata_ifalready gives that onecompare-and-set. A sidecar splits it into two writes with no shared commit point, so a
crash between them yields a queue seated against a binding that no longer holds — the same
class of loss this PR closes, reintroduced at a different seam and harder to detect
because each file is individually well-formed. The coupling is real and is the price of
one atomic write; it is pinned by a census test rather than left to convention.
Accepted risk — a queue of no-
maxAgeentries can wedge a dormant slot. Capacityis now enforced by REFUSAL rather than by evicting the oldest entry, and the queue is
only drained by a user turn. So a session that never takes another turn keeps its seats
occupied, and later
/contextand/noteposts to it are refused with 429indefinitely, where the previous FIFO eviction self-healed. This is a deliberate
trade: eviction silently destroyed content the API had already acknowledged with a 200,
which is the class of bug this PR exists to close, and a refusal is at least visible to
the caller. A staleness horizon IS set here:
DEFAULT_CONTEXT_TTL_SECS, seven days,applied inside
context_entry_expired— the single expiry chokepoint — and only to entriescarrying a real timestamp, so an unstamped entry keeps its never-expires behaviour.
Consequence —
pending_contextis slot-owned, so absence means CLEARED. Every savepath must therefore write the key unconditionally, and every hydration path must seat
it, or an ordinary save deletes a copy an earlier close committed. That makes each save
and hydration site load-bearing rather than incidental; the census test
test_the_set_of_metadata_write_sites_is_pinnedexists to fail when a new save path isadded without classifying it.