Sign up
View stargazersSign up

fix(threads): snooze no longer stays disabled after a steered follow-up#23787

open1
main
GitHub fork: fix/23778-snooze-steered-followup
opened19d agobyFfixfon
2 files changed
View on GitHubTwo-way sync enabledPR last imported 14h ago

Problem

Fixes #23778.

After a follow-up message is steered into a running turn, Snooze stays disabled for up to 2 minutes (QUEUED_TURN_START_GRACE_MS) and then turns on by itself. Sending the follow-up first queues a new run. When the message is steered into the running turn, that queued run is cancelled before it starts, and the message is stamped just after it. The shell's latestRun is then the cancelled run, and every timestamp on it is older than latestUserMessageAt. So hasQueuedTurnStart reports a pending start, and canSnooze returns false until the grace window ends. The same check feeds the settled and queued state.

Change

  • packages/client-runtime/src/state/threadSettled.ts: hasQueuedTurnStart returns false when the latest run is cancelled and has no startedAt. A run cancelled before it started can never adopt the message, so the message is not waiting on a start. This is one guard line plus a short comment.
  • I did not use the reporter's two suggestions as written. The shell carries no message runId, and latestRun holds only the newest unheld run, so the client cannot fall back to the steered-into run. "Cancelled before start" is visible on the run the shell already has.
  • Behavior change: if a cancelled-before-start run is the latest only because a newer queued run is held (queueHeld), a message waiting behind that held queue no longer counts as a pending start. A held queue is not about to start, so I think this is correct, but it is a change.
  • The server has the same logic in threadHasQueuedTurnStart (apps/server/src/orchestration-v2/ThreadSettlementService.ts), which feeds auto-settle; this PR leaves it alone, and it could be a follow-up.

Scope and approval

This is a small, focused fix for the bug in #23778, which includes the persisted run and message records and the cause. It touches only packages/client-runtime/src/state (2 files, +40/−0). It adds no setting and does not change behavior for queued runs that have not started or for cancelled runs that had started.

Verification

  • New tests in threadSnoozed.test.ts:
    • A latest run cancelled before it started, older than the latest user message: hasQueuedTurnStart is false and canSnooze is true.
    • Guard cases that still wait on a newer message: a queued run that has not started, and a cancelled run that had started.
    • Without the fix: 1 failed (the new issue test, expected true to be false), 22 passed. With the fix: 23 of 23 pass.
  • The other test files that import threadSettled (threadSnoozed, customSnooze, entities): 50 of 50 pass.
  • vp lint and vp fmt --check on the 2 changed files: no errors. packages/client-runtime typecheck: 0 errors.
  • vp run check:changed is inconclusive. Format, lint and tests pass. Knip and typecheck fail because my local node_modules is older than the current lockfile, like in #23721. Knip reports 3 unused apps/mobile dependencies. Typecheck fails in infra/relay, apps/mobile and apps/web on missing modules (@distilled.cloud/cloudflare, react-native-pdf-jsi, expo-application, remark-math, katex, pele/lazy). No error touches the changed files, and the change alters no signature or export. CI has to confirm the full run.
  • I did not run the app.

Claude Sonnet 5.5 (xhigh) via T3 Code.

Fixes pingdotgg/t3code#23778

Checks

22 successful · 9 skipped
All checks passed
5ffa7a8

Added 1 commit

  • fix(threads): snooze no longer stays disabled after a steered follow-up5ffa7a8
CcoderabbitaiBot19d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The queued-turn check now treats a selected run cancelled before starting as not pending. Tests cover the effect on snoozing and verify that queued runs and runs cancelled after starting still keep a newer message queued.

Changes: Queued turn detection

Layer / File(s)Summary
Cancelled run handling
packages/client-runtime/src/state/threadSettled.ts, packages/client-runtime/src/state/threadSnoozed.test.ts		hasQueuedTurnStart excludes a cancelled run without startedAt. Tests cover snoozing after pre-start cancellation, and confirm that queued runs or runs cancelled after starting still leave a newer message queued.

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: t3dotgg

Merge Risk: External image Minimal · up to 5ffa7

The client-side change addresses Snooze remaining disabled after a steered follow-up. No merge-blocking risk is established; normal checks and CI should still complete.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the primary fix: Snooze no longer remains disabled after a steered follow-up.
Linked Issues checkPassedDirect issue #23778 requires Snooze to become available after a steered follow-up is adopted and no work remains pending. The reported change excludes a latest run with status cancelled and no `star…
Out of Scope Changes checkPassedThe changes are limited to packages/client-runtime/src/state/threadSettled.ts and its focused test file. The guard implements the issue fix, and the tests verify the changed and preserved behavior. …
Description checkPassedThe description covers the problem, the change, scope and approval rationale, and focused verification results. It also states the full-check limitations and that CI must confirm them.
Full details: Linked Issues check

Explanation

Direct issue #23778 requires Snooze to become available after a steered follow-up is adopted and no work remains pending. The reported change excludes a latest run with status cancelled and no startedAt from hasQueuedTurnStart. The added tests cover this regression and preserve the queued-run and started-then-cancelled cases. The supplied verification reports 23 of 23 relevant tests passing. The unchanged server auto-settle check is outside the client Snooze behavior required by #23778.

Full details: Out of Scope Changes check

Explanation

The changes are limited to packages/client-runtime/src/state/threadSettled.ts and its focused test file. The guard implements the issue fix, and the tests verify the changed and preserved behavior. The disclosed effect for a held queue is part of the same pending-start decision and is not demonstrated to be unrelated scope.

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 044f07257c9684ae4543bd1e1ebf9c394e9e3830 and 5ffa7a815bc984d48e516d656e241e577d07c406.

📒 Files selected for processing: 2

  • packages/client-runtime/src/state/threadSettled.ts
  • packages/client-runtime/src/state/threadSnoozed.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.

MmacroscopeappBot19d ago
Synced from GitHub

Approvability

Verdict: Approved at 5ffa7a8

Macroscope's review found this PR approvable — This is a narrowly scoped client-state bug fix that stops a pre-start cancelled run from keeping snooze disabled for the grace period. The production change is one guarded condition, and the accompanying tests cover both the regression and unchanged neighboring cases.

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