Skip to content

fix(server): re-snoozing a woken thread to the same wake time hides it again - #14299

Open
vitalyiegorov wants to merge 1 commit into
pingdotgg:mainfrom
vitalyiegorov:fix/resnooze-fresh-stamp
Open

vitalyiegorov wants to merge 1 commit into
pingdotgg:mainfrom
vitalyiegorov:fix/resnooze-fresh-stamp

Conversation

@vitalyiegorov

@vitalyiegorov vitalyiegorov commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

What Changed

thread.snooze now always stamps snoozedAt with the current time. I removed the branch that kept the original snoozedAt when 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.

threadRaisedHandWhileSnoozed wakes a snoozed thread when a failure or a completion is newer than snoozedAt. 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 old snoozedAt. 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 commandId receipts, 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 run on decider.snoozed.test.ts, decider.active-order.test.ts, ProjectionPipeline.test.ts and client-runtime threadCommands.test.ts and threadSnoozed.test.ts: 100 passed. The new case fails without the fix.
  • Typecheck for apps/server and packages/client-runtime.
  • Lint and fmt on the touched files.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (no UI change)
  • I included a video for animation/interaction changes (none)

Implemented with Claude Opus 5.5 and Claude Sonnet 5.5 in T3 Code (Claude Code harness).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Behavior Updates
    • Re-snoozing a thread for the same wake time now refreshes its snooze timestamp and update time, including in the immediate interface update.

…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>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 29, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 254b54f

Macroscope's review found this PR approvable — This is a narrowly scoped snooze lifecycle bug fix that synchronizes server and optimistic client timestamps, with a regression test and no schema, default, infrastructure, or sensitive-area changes. Its runtime impact is limited to correctly hiding a thread again when the user re-snoozes it.

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

@coderabbitai

coderabbitai Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: bd0cd7b0-c450-40b4-b63b-c644d050f156

📥 Commits

Reviewing files that changed from the base of the PR and between 27bdf1a and 254b54f.

📒 Files selected for processing (3)
  • apps/server/src/orchestration/decider.snoozed.test.ts
  • apps/server/src/orchestration/decider.ts
  • packages/client-runtime/src/state/threadCommands.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.


📝 Walkthrough

Walkthrough

The 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.

Changes

Snooze timestamp refresh

Layer / File(s) Summary
Refresh timestamps on repeated snoozes
apps/server/src/orchestration/decider.ts, packages/client-runtime/src/state/threadCommands.ts, apps/server/src/orchestration/decider.snoozed.test.ts
The server and optimistic client updates assign fresh timestamps for every snooze. The server test verifies this behavior when the wake time is unchanged.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 254b5

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 Summary

Architecture risk: 🔵 Low · up to 254b5

The change affects 2 systems.

Changed systems: apps/server, packages/client-runtime

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 2 changed files map to changed impact.
  • observed — packages/client-runtime (library) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/orchestration/decider.snoozed.test.ts: The same-wake-time test changes from expecting an idempotent re-emission that preserves snoozedAt and keeps updatedAt at NOW to expecting a fresh snooze timestamp, with updatedAt matching it.
  • observed — Modified behavior in apps/server/src/orchestration/decider.ts: The same-wake-time duplicate special case and its existingSnoozedAt calculation were removed. The replacement comment states that every snooze stamps fresh timestamps, including re-snoozing a woken thread; retries remain deduplicated by commandId.
  • observed — Modified behavior in apps/server/src/orchestration/decider.ts: thread.snoozed now always sets snoozedAt and updatedAt to the current occurredAt, rather than retaining the prior snooze and update timestamps when the wake time is unchanged.
  • observed — Modified behavior in packages/client-runtime/src/state/threadCommands.ts: The optimistic snooze update now assigns snoozedAt to now unconditionally instead of preserving the existing timestamp when snoozedUntil is unchanged.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary fix: re-snoozing a woken thread to the same wake time hides it again.
Description check ✅ Passed The description includes the required What Changed, Why, and Checklist sections. It explains the bug, the implementation, scope boundaries, linked issue, and verification results. The UI checklist ite…
Linked Issues check ✅ Passed The changes satisfy the coding requirements in #14298. apps/server/src/orchestration/decider.ts now assigns fresh snoozedAt and updatedAt values for every explicit thread.snooze, including the…
Out of Scope Changes check ✅ Passed The changes remain within #14298. The server change implements the requested behavior, the client change keeps optimistic state consistent, and the test updates the affected contract. No unrelated fea…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

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:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: A thread that woke from snooze cannot be snoozed again to the same wake time

1 participant