Repository navigation
fix(server): re-snoozing a woken thread to the same wake time hides it again - #14299
vitalyiegorov wants to merge 1 commit into
Conversation
…t again A snooze to the same wake time kept the original snoozedAt, so a failure or completion that woke the thread stayed newer than the snooze and the thread never hid again. Every snooze now stamps fresh; retries are already deduplicated by commandId. Fixes pingdotgg#14298 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a small, tested bug fix that refreshes snooze timestamps so a previously woken thread can be hidden again when re-snoozed to the same time. Its runtime impact is confined to existing snooze visibility and timestamp handling, with no schema, infrastructure, or static-analysis changes. Notes:
You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe server now stamps fresh snooze and update times for every snooze command. The client’s optimistic update also stamps the current time. A test checks repeated snoozes to the same wake time. ChangesSnooze timestamp refresh
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to Repeated snoozes now refresh the baseline so earlier failures no longer immediately wake the thread. No concrete merge-blocking risk is established; the possible cross-instance retry effect remains unconfirmed. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 2 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
had this same issue and this PR solves it 🙌🏻 |
Dismissing prior approval to re-evaluate 254b54f
|
Thanks for working on this. We merged the orchestrator V2 rewrite in #2829, and we are closing this PR as part of that transition. This change touches decider.ts, which the V2 merge removed. V2 uses new commands, contracts, projections and orchestration services. Sorry for the extra work this creates. If the change is still needed on V2, please rebuild it on current main, verify it there, and open a new PR linking back here. We're closing the current implementation without assuming the underlying request is resolved. |
|
Rebuilt on V2 main as asked: #15881. Same change, now in |
What Changed
thread.snoozenow always stampssnoozedAtwith the current time. I removed the branch that kept the originalsnoozedAtwhen a thread was re-snoozed to the same wake time. It was in the server decider (apps/server/src/orchestration/decider.ts) and in its optimistic twin on the client (packages/client-runtime/src/state/threadCommands.ts). The diff removes more lines than it adds and changes no contract or schema.The existing decider test that asserted the old behavior now asserts the fix: a re-snooze to the same wake time gets a fresh
snoozedAt. Without the fix it fails.Why
Fixes #14298.
threadRaisedHandWhileSnoozedwakes a snoozed thread when a failure or a completion is newer thansnoozedAt. Once that happens, the user snoozes the thread again, usually with the same preset. Presets such as Tomorrow and Next week resolve to fixed clock times, so the new request carries the same wake time. The decider treated it as a duplicate and kept the oldsnoozedAt. The failure was therefore still "newer than the snooze", and the thread never hid again. The server accepted every retry and nothing changed.In the reported case, one Claude thread hit its usage limit after being snoozed and was re-snoozed six times in 12 minutes. Every attempt was accepted and none of them hid the thread.
A fresh stamp is the right meaning: re-snoozing a woken thread is the user saying "seen it, not now". The old branch protected against double-clicks and racing clients. Real retries are already deduplicated by
commandIdreceipts, and a double-click now only moves the stamp by milliseconds.This only covers re-snoozing. Whether completed work or a usage-limit failure should wake a snoozed thread at all (#6368) is a separate policy question, and this PR doesn't touch it.
Verification:
vp test runondecider.snoozed.test.ts,decider.active-order.test.ts,ProjectionPipeline.test.tsand client-runtimethreadCommands.test.tsandthreadSnoozed.test.ts: 100 passed. The new case fails without the fix.apps/serverandpackages/client-runtime.UI Changes
No UI code changes. The visible effect is that re-snoozing a woken thread hides it again. Before, from the report in #14298: two threads stay in the inbox as Failed after six accepted "Next week" re-snoozes.
Checklist
Implemented with Claude Opus 5.5 and Claude Sonnet 5.5 in T3 Code (Claude Code harness).
🤖 Generated with Claude Code