Repository navigation
fix(server): keep a dated snooze on awake threads; keep a scheduled model until a default turn takes it - #97
Open
chrisohearn wants to merge 1 commit into
Open
chrisohearn wants to merge 1 commit into
chrisohearn wants to merge 1 commit into
Conversation
… model for default turns From the PR #94 delta review: - A default agent send to a thread that counted as awake only because of a pending request, guest message or error erased its dated snooze. It now delivers and keeps the stored snooze. So does a thread that has never run. The send's warning says it was delivered. - A scheduled model change was cleared once only explicit picks were ahead of it, so a later default message got the older pick. It now stays until a default-model turn starts with it, after everything queued before it, and a model named after the change replaces it. - Tests: a later completed run, such as a usage limit's resume, releases snooze-held turns only once the thread is awake. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
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.
Follow-up to #94 (ledger
r4/w4-tools), from its delta review. This PR is stacked on #94's branch, and GitHub retargets it tointegration/mcp-events-20261109when #94 merges. Its diff is one commit.In every acceptance check,
<deploy_ms>is the deploy time in epoch milliseconds.1. A default agent send no longer erases a dated snooze on a thread that counts as awake
Problem. A thread can count as awake while its snooze is still stored: a pending request, a waiting guest message, or a run that failed after the snooze.
isSnoozedthen returns false. #94 used that result for two decisions: whether to hold the message, and whether to end the snooze. So a default agent send (respectSnooze: true) to such a thread cleared the stored snooze for good. This is the same class as the incident: a dated hold ended by an agent that never meant to end it.Change. The two decisions are now separate in
Orchestrator.ts:keepsSnooze: the sender respects the snooze, the message is not an automatic wake, the stored date is in the future, and it isn't a usage-limit snooze. When true, the stored snooze is never cleared.respectsSnooze:keepsSnooze, the thread has run before, andisSnoozedis true. When true, the turn is held as before.A thread that counts as awake, or has never run, gets the message at once and keeps its dated hold.
t3_thread_sendreports this as adeliveredwarning: "It counts as awake for now (a pending request or an error), so the message was delivered; the snooze stays."Test.
runtimeLayer.test.ts, "delivers to a snoozed thread that counts as awake without ending its snooze": a background wake fails after the snooze, then a respect-snooze send starts at once and the storedsnoozedUntilstays 2099-01-01. On #94's head the snooze is cleared.2. A scheduled model change stays until a default-model turn takes it
Problem. #94 cleared the schedule once no queued default-model run from before the change remained. When only explicit picks were left ahead, the change was dropped before any default turn used it. The explicit pick then ran and moved the thread to its model, so later default messages got the older pick instead of the scheduled model.
Change.
threadAtRunStartnow clears the schedule only in two cases:modelSelection. That newer instruction replaces the change.An explicit pick queued before the change runs with its own model and leaves the change pending. Server continuations that repeat an earlier run's model don't count as naming one.
Test. The scheduled-change case in
runtimeLayer.test.tsnow runs to the end: two default runs and one explicit pick queued before the change, and one default message after it.On #94's head the change is gone after the second boundary.
3. Test: a later completed run releases snooze-held turns only once the thread is awake
runtimeLayer.test.ts, "a later completed run releases snooze-held messages only once the thread is awake":snoozeDeferred.This is the path a usage-limit resume takes:
handleRunUpdaterunsreleaseUnattendedHolds.Acceptance (read-only)
Pass:
thread.unsnoozedevent whose actor is an MCP send withrespectSnoozeleft at its default;Notes
Docs: decision record 0017,
docs/user/outside-agents.mdanddocs/orchestration-v2/orchestrator-mcp-server.mdare updated for both changes.Agent context: the new warning text is in a tool result, not the tool list, so the budget is unchanged. No migration.
Checks: under the resource rule, I ran only focused checks, under the shared lock with two workers:
apps/servertypecheck;runtimeLayercases (the two new tests, the scheduled change, the respect-snooze hold, and the post-only case);OrchestratorMcpService,SnoozeDeferredDelivery,Orchestrator.control-reads,coreandAgentContextBudget(158 tests).Hosted CI is the full gate.
🤖 Generated with Claude Code