Skip to content

fix(server): waking or pinning a usage-limit-snoozed thread no longer re-snoozes it on Resume at reset - #29172

Open
saphid wants to merge 2 commits into
pingdotgg:mainfrom
saphid:fix/limit-recovery-snooze-after-wake
Open

saphid wants to merge 2 commits into
pingdotgg:mainfrom
saphid:fix/limit-recovery-snooze-after-wake

Conversation

@saphid

@saphid saphid commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Problem

A thread stops on a usage limit and you press Snooze until reset. Later you wake it from the sidebar, the thread menu, or the mobile thread list, or you Pin it. Then you press Resume at reset on the banner. The thread disappears back into Snoozed, though all you asked for was auto-resume. After a Pin, the thread ends up pinned and snoozed at the same time.

The cause: thread.unsnooze and thread.pin clear snoozedUntil but leave limitRecovery.snooze set to true. "Resume at reset" sends { runId, resetAt, autoResume: true } with no snooze field. The thread.metadata.update decider fills the missing field from the stale flag, so it snoozes the thread to resetAt again.

Change

thread.unsnooze and thread.pin now clear limitRecovery.snooze, the same way thread.snooze already does (limitRecovery: thread.limitRecovery ? { ...thread.limitRecovery, snooze: false } : null). Once something other than limit recovery wakes the thread, recovery no longer owns a snooze. This matches the recoveryOwnsSnooze check the web and mobile clients already use. thread.settle is left alone, because #28360 changes that case.

Scope and approval

This is a very small, focused fix for an obvious bug, so there is no prior issue. You ask for auto-resume and the server hides the thread instead, and Pin produces a pinned-and-snoozed state that the pin decider otherwise rules out. The fix is two lines, copied from the sibling thread.snooze case, plus two rows in an existing test table.

Overlap search: #28360 (open) touches only thread.settle. #22775 (open) covers empty wake runs and projection queries. #28359 (open) covers auto-settle in ThreadSettlementService and ProjectionStore. None of them change the thread.unsnooze or thread.pin deciders. GitHub searches for "unsnooze limitRecovery" and "snooze limit recovery wake pin" found no other matches (only the original feature, #12687).

Verification

  • Added wake-then-resume and pin-then-resume rows to runtimeLayer.test.ts › "usage-limit recovery" › "guards a scheduled usage-limit continuation against %s". Each row snoozes until reset, wakes or pins, presses Resume at reset, and asserts the thread stays awake.
  • Red on main: both rows fail with AssertionError: expected DateTime.Utc(1970-01-01T00:01:00.000Z) to equal null, meaning snoozedUntil comes back as resetAt.
  • Green: vp test run on runtimeLayer.test.ts, UsageLimitRecoveryWorker.test.ts, ThreadSettlementService.test.ts, and ProjectionStore.test.ts passed 193 of 193 tests in 4 files. This includes the existing independent-patches row, which shows that snooze is still kept while recovery really owns the snooze.
  • vp run check:changed: format, lint, knip, graphql, typecheck (3 packages), and tests all pass.
  • Not checked: a manual run in a live client. This is a server decider change with no visual change.

Reviewed with sol-loop: 1 round with GPT-6.1 Sol on high. Sol verdict: approved.

Created with Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

… re-snoozes it on Resume at reset

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted Author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 2e12baa

Macroscope's review found this PR approvable — Waking or pinning a thread now clears its stale recovery snooze flag, preventing a later Resume at reset action from hiding it again. The change is limited to explicit thread actions and preserves existing auto-resume settings.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Unsnooze and pin actions now clear the snooze flag in existing limit-recovery settings. Tests cover both actions before auto-resume is enabled.

Changes: Limit-recovery snooze handling

Layer / File(s) Summary
Clear recovery snooze on wake and pin
apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/orchestration-v2/runtimeLayer.test.ts
Unsnooze and pin clear the snooze flag in existing limit-recovery settings. Tests verify that enabling auto-resume without a snooze value leaves the thread unsnoozed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: Minimal · up to 2a6de

Waking or pinning a usage-limit-snoozed thread now keeps it from being re-snoozed when Resume at reset runs. The change is narrow and covered by new tests, so merge risk is minimal.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: waking or pinning a usage-limit-snoozed thread no longer re-snoozes it on Resume at reset. The raw summary shows thread.unsnooze and thread.pin now clear limitRecovery…
Description check Passed The description follows the template. It has Problem, Change, Scope, and Verification sections. The Problem section gives the reproduction steps and the root cause. The Change section explains the fix…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: waking or pinning a usage-limit-snoozed thread no longer re-snoozes it on Resume at reset. The raw summary shows thread.unsnooze and thread.pin now clear limitRecovery.snooze, which matches the title. The title uses the conventional commit form and is specific. It is a little long at 102 characters, but it is clear.

Full details: Description check

Explanation

The description follows the template. It has Problem, Change, Scope, and Verification sections. The Problem section gives the reproduction steps and the root cause. The Change section explains the fix and why it matches thread.snooze and recoveryOwnsSnooze. The Scope section explains why no issue is linked and gives an overlap search. The Verification section lists focused tests, red-then-green results, the check:changed run, and what was not checked. The template asks for a conventional commit title in the header comment, and the title meets that. The description also names the model and harness that did the work, as the template requests. The description does not link a Scope issue, but it gives a reason for that, which the template allows for small obvious fixes.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 2e12baa and 2a6de34.

Files selected for processing: 2

  • apps/server/src/orchestration-v2/Orchestrator.ts
  • apps/server/src/orchestration-v2/runtimeLayer.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@saphid

saphid commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Review requested

Date (UTC) Reviewer Where
2026-11-13 Julius Discord DM

Logged so this PR shows when a maintainer was asked to review it.

@juliusmarminge juliusmarminge added the takeover:in-progress A takeover lead is working on this PR label Nov 14, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

The automated pipeline stopped on this pull request and will not merge it:

the fix is written and reviewed, but proving it needs something no worker has:

  • Permanent blocker: the reproduction requires first-party Claude usage-limit behavior and an account capable of producing the reported 429/reset state; the brief lists first-party Claude OAuth usage, limit, and renewal flows as unavailable on every VM. The scenario documentation also explicitly excludes state such as a run stopped by a usage limit. Both Wake and Pin variants remain unverified on base and head.

It stays open for a person to finish or close.

@juliusmarminge juliusmarminge added takeover:blocked Takeover blocked; reason in the PR body and removed takeover:in-progress A takeover lead is working on this PR labels Nov 14, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). takeover:blocked Takeover blocked; reason in the PR body vouch:trusted Author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants