From d4705ac01bb7ed44329da3c0ec9d968295973d06 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 22 Jul 2026 14:10:43 -0700 Subject: [PATCH 1/3] fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR Sending a message in a settled thread un-settles it server-side (the decider prepends thread.unsettled on turn.start), but the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears, so the row snapped back into the settled tail the moment the new turn completed. Gate the change-request signal on thread idleness: a merged/closed PR only settles a thread quiet for at least an hour. Fresh activity keeps the follow-up conversation in the active list; once the burst goes stale the merge signal settles it again, matching the decider's activity-clears- overrides model. Web and mobile both partition through the shared effectiveSettled, so this covers both clients. Co-Authored-By: Claude Fable 5 --- .../src/state/threadSettled.test.ts | 25 ++++++++++++++++ .../client-runtime/src/state/threadSettled.ts | 29 ++++++++++++++++--- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/packages/client-runtime/src/state/threadSettled.test.ts b/packages/client-runtime/src/state/threadSettled.test.ts index 50f3c2d3912..f870eb373a1 100644 --- a/packages/client-runtime/src/state/threadSettled.test.ts +++ b/packages/client-runtime/src/state/threadSettled.test.ts @@ -163,6 +163,31 @@ describe("effectiveSettled", () => { ).toBe(true); }); + it("does not re-settle a warm thread on the merge signal: a message sent in a settled thread keeps it active until idle", () => { + // The merge signal never clears, so without the idle guard a follow-up + // message would un-settle the row only until its turn completed, then + // snap straight back into the settled tail. + const justActive = makeShell({ activityAt: "2026-04-09T23:30:00.000Z" }); + const idle = makeShell({ activityAt: "2026-04-09T22:59:59.999Z" }); + + for (const changeRequestState of ["merged", "closed"] as const) { + expect( + effectiveSettled(justActive, { + now: NOW, + autoSettleAfterDays: null, + changeRequestState, + }), + ).toBe(false); + expect( + effectiveSettled(idle, { + now: NOW, + autoSettleAfterDays: null, + changeRequestState, + }), + ).toBe(true); + } + }); + it("never settles a starting session, even with a settled override", () => { const shell = makeShell({ settledOverride: "settled", diff --git a/packages/client-runtime/src/state/threadSettled.ts b/packages/client-runtime/src/state/threadSettled.ts index 3cb4f7e65f7..41d3cfecd9c 100644 --- a/packages/client-runtime/src/state/threadSettled.ts +++ b/packages/client-runtime/src/state/threadSettled.ts @@ -90,13 +90,25 @@ export function canSettle( return true; } +/** + * A merged/closed change request settles its thread only once the thread has + * been idle this long. Without the idle guard the merge signal is permanent: + * sending a message to a merged-PR thread would un-settle the row only until + * its turn completed, then the still-merged PR would snap it straight back + * into the settled tail. An hour keeps the follow-up conversation visible + * while it is warm; once the burst goes stale the merge signal settles it + * again. + */ +export const CHANGE_REQUEST_SETTLE_IDLE_MS = 60 * 60 * 1_000; + /** * Settled resolution over the server-backed settled lifecycle. The explicit * user override (thread.settle / thread.unsettle commands, projected into * settledOverride + settledAt) wins in both directions; without one, a - * thread auto-settles on a merged/closed PR or inactivity past the window. - * The server un-settles on real activity (user message, session start, - * approval/user-input request), so an override never goes stale silently. + * thread auto-settles on a merged/closed PR (once idle) or inactivity past + * the window. The server un-settles on real activity (user message, session + * start, approval/user-input request), so an override never goes stale + * silently. */ export function effectiveSettled( shell: OrchestrationThreadShell, @@ -130,7 +142,16 @@ export function effectiveSettled( // until real activity clears it server-side. if (shell.settledOverride === "active") return false; if (options.changeRequestState === "merged" || options.changeRequestState === "closed") { - return true; + // Only an idle thread settles on the merge signal: the signal itself + // never clears, so without this guard fresh activity (a message sent in + // a settled thread) would re-settle the moment its turn completed. + const lastActivityAt = threadLastActivityAt(shell); + if ( + lastActivityAt === null || + Date.parse(lastActivityAt) < Date.parse(options.now) - CHANGE_REQUEST_SETTLE_IDLE_MS + ) { + return true; + } } if (options.autoSettleAfterDays === null) return false; From ca5b817bdd27b9f111b777e93ae29b68ce109da7 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 22 Jul 2026 14:14:45 -0700 Subject: [PATCH 2/3] test(client-runtime): cover idle-gate boundary and re-settle-after-idle; note clock-skew exposure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review follow-ups: assert the strict one-hour boundary (activity exactly an hour old stays warm), assert the same shell re-settles once the follow-up burst goes idle, and document that cross-device clock skew shifts the idle window by its size — the same accepted exposure as the inactivity auto-settle. Co-Authored-By: Claude Fable 5 --- .../src/state/threadSettled.test.ts | 19 +++++++++++++++++++ .../client-runtime/src/state/threadSettled.ts | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/client-runtime/src/state/threadSettled.test.ts b/packages/client-runtime/src/state/threadSettled.test.ts index f870eb373a1..fa1a8820ef4 100644 --- a/packages/client-runtime/src/state/threadSettled.test.ts +++ b/packages/client-runtime/src/state/threadSettled.test.ts @@ -168,6 +168,8 @@ describe("effectiveSettled", () => { // message would un-settle the row only until its turn completed, then // snap straight back into the settled tail. const justActive = makeShell({ activityAt: "2026-04-09T23:30:00.000Z" }); + // The idle gate is strict: activity exactly one hour old is still warm. + const boundary = makeShell({ activityAt: "2026-04-09T23:00:00.000Z" }); const idle = makeShell({ activityAt: "2026-04-09T22:59:59.999Z" }); for (const changeRequestState of ["merged", "closed"] as const) { @@ -178,6 +180,13 @@ describe("effectiveSettled", () => { changeRequestState, }), ).toBe(false); + expect( + effectiveSettled(boundary, { + now: NOW, + autoSettleAfterDays: null, + changeRequestState, + }), + ).toBe(false); expect( effectiveSettled(idle, { now: NOW, @@ -188,6 +197,16 @@ describe("effectiveSettled", () => { } }); + it("re-settles a merged-PR thread once the follow-up burst goes idle", () => { + // Same shell, advancing clock: active while warm, settled again after + // the idle window passes — the burst cools and the merge signal wins. + const shell = makeShell({ activityAt: "2026-04-09T23:30:00.000Z" }); + const options = { autoSettleAfterDays: null, changeRequestState: "merged" as const }; + + expect(effectiveSettled(shell, { ...options, now: NOW })).toBe(false); + expect(effectiveSettled(shell, { ...options, now: "2026-04-10T00:30:00.001Z" })).toBe(true); + }); + it("never settles a starting session, even with a settled override", () => { const shell = makeShell({ settledOverride: "settled", diff --git a/packages/client-runtime/src/state/threadSettled.ts b/packages/client-runtime/src/state/threadSettled.ts index 41d3cfecd9c..1b451b30506 100644 --- a/packages/client-runtime/src/state/threadSettled.ts +++ b/packages/client-runtime/src/state/threadSettled.ts @@ -97,7 +97,10 @@ export function canSettle( * its turn completed, then the still-merged PR would snap it straight back * into the settled tail. An hour keeps the follow-up conversation visible * while it is warm; once the burst goes stale the merge signal settles it - * again. + * again. Activity timestamps can originate on another device while `now` is + * this caller's clock: skew shortens or stretches the window by its size, + * the same exposure the inactivity auto-settle already accepts — worst case + * is a row changing lists early or late, never lost work. */ export const CHANGE_REQUEST_SETTLE_IDLE_MS = 60 * 60 * 1_000; From 3cea3f93648344c4a02cc022b357a3f8297321ac Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Thu, 23 Jul 2026 13:05:55 -0700 Subject: [PATCH 3/3] docs(client-runtime): state that activity blockers precede the settle override CodeRabbit review: the effectiveSettled doc claimed the override "wins in both directions", but pending approvals/user-input, live sessions, and unadjudicated queued turns are evaluated first and hold a thread active regardless of any override. Say so explicitly. Co-Authored-By: Claude Fable 5 --- .../client-runtime/src/state/threadSettled.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/client-runtime/src/state/threadSettled.ts b/packages/client-runtime/src/state/threadSettled.ts index 1b451b30506..85d1649c267 100644 --- a/packages/client-runtime/src/state/threadSettled.ts +++ b/packages/client-runtime/src/state/threadSettled.ts @@ -105,13 +105,15 @@ export function canSettle( export const CHANGE_REQUEST_SETTLE_IDLE_MS = 60 * 60 * 1_000; /** - * Settled resolution over the server-backed settled lifecycle. The explicit - * user override (thread.settle / thread.unsettle commands, projected into - * settledOverride + settledAt) wins in both directions; without one, a - * thread auto-settles on a merged/closed PR (once idle) or inactivity past - * the window. The server un-settles on real activity (user message, session - * start, approval/user-input request), so an override never goes stale - * silently. + * Settled resolution over the server-backed settled lifecycle. Activity + * blockers (pending approval/user-input, a live session, an unadjudicated + * queued turn) are checked first and hold a thread active regardless of any + * override. Past the blockers, the explicit user override (thread.settle / + * thread.unsettle commands, projected into settledOverride + settledAt) + * wins in both directions; without one, a thread auto-settles on a + * merged/closed PR (once idle) or inactivity past the window. The server + * un-settles on real activity (user message, session start, approval/ + * user-input request), so an override never goes stale silently. */ export function effectiveSettled( shell: OrchestrationThreadShell,