Repository navigation
Bound the orchestrator's process-lifetime collections - #534
Merged
Merged
Conversation
Proves eviction actually happens for postedApprovalIds and pendingDelegationThreads, and that eviction is safe: a redelivered gate-blocked event for a resolved approval still posts nothing once its guard entry expires, and an abandoned delegation's reply still posts — just unthreaded — once its entry expires.
Both grew one entry per approval/delegation for the life of the hub process, never pruned. Bound each with @corbits/collections' createExpiringMap rather than a plain Set/Map: - pendingDelegationThreads: already deleted on the event that matters (the specialist's first reply consumes it); now also TTL-bounded at AGENT_TURN_STALE_MS, the same threshold agent-turns.ts already uses to call a running turn no longer believable. A delegation nobody answers within that window is already considered abandoned elsewhere in the system, so evicting it can't drop a live reply — a very late reply still posts, just unthreaded instead of nested under the delegating message. - postedApprovalIds: every gate the reactor blocks on resolves out of "pending" on its own within one hour (DEFAULT_GATE_TIMEOUT_MS), so a guard entry older than that is guaranteed to belong to a non-pending approval. TTL is set well past that bound; eviction can never reopen the duplicate-card hole this guard exists to close, because postApproveBlock re-reads the approval's live status independently of the guard on every call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CL-7229:
postedApprovalIdsandpendingDelegationThreadsinpackages/chat/src/chat-orchestrator.tsgrew one entry per approval /delegation for the life of the hub process, never pruned. Bounds both
with
@corbits/collections'createExpiringMap(already consumed bycrypto-cache.tsandworkflow-routes.ts), per the ruling: event-drivendelete plus a TTL backstop, not TTL-only, not unbounded.
pendingDelegationThreadsAlready deleted on the event that matters —
threadDelegatedReplyconsumes and deletes the entry the moment the delegated specialist's
first reply lands. What was missing is a bound for the case where that
event never fires (the specialist never wakes, crashes, or was mentioned
by mistake). Now TTL-bounded at
AGENT_TURN_STALE_MS, the samethreshold
agent-turns.tsalready uses to call arunningturn nolonger believable. A delegation nobody answers within that window is
already considered abandoned elsewhere in the system, so evicting it
can't drop a reply a running turn still needs — a very late reply still
posts, it just lands unthreaded instead of nested under the delegating
message. Cosmetic degrade, never a lost message.
postedApprovalIdsEvery gate the reactor blocks on — approval gates included — resolves
out of
"pending"on its own withinDEFAULT_GATE_TIMEOUT_MS(onehour,
vendor/intx/inference/src/reactor.ts), either by a human'sdecision or by timing out to a terminal
"timeout"/"expired"status.So a guard entry older than that bound is guaranteed to belong to an
approval that's no longer pending. TTL is set to double the gate timeout
plus a margin. Eviction can never reopen the duplicate-card hole this
guard exists to close:
postApproveBlockre-reads the approval's livestatus on every call, independent of the in-memory guard, and bails out
on anything but
"pending"regardless of whether the guard remembersit — the guard is only ever an optimization on top of that authoritative
check, never the sole source of correctness.
Test plan
bun run typecheckHUB_DATA_DIR=$(mktemp -d) bun test packages/chat/— 757 pass, 0 failand safety at the boundary (an evicted, still-pending-looking entry
never causes a duplicate post or a lost delegation reply)
bunx prettier --check .bun run check:structural(ignoringcheck:report-error, whichfails on main independent of this change)
https://linear.app/abklabs/issue/CL-7229/bound-the-orchestrators-process-lifetime-collections