| author |  Theo Browne <me@t3.gg> 2026-07-28 02:15:14-07:00 |
| committer |  GitHub <noreply@github.com> 2026-07-28 11:15:14+02:00 |
| log | 9cf9fc9c5c0f48d42f407f86cf83d0f04fbdf5a8 |
| tree | a0257fedc6f1f26ff17385f95c492ed051774d2b |
| parent | e00781a6619125e30f3e30ec6e17bc4c61d30502 |
| signature | Signed by PGP key B5690EEEBB952194 |
Settle merged PR threads immediately (#4704)
2 files changed, 17 insertions(+), 57 deletions(-)
packages
client-runtime
packages/client-runtime/src/state/threadSettled.test.ts+15-32
| ... | @@ -163,32 +163,11 @@ describe("effectiveSettled", () => { | ... | @@ -163,32 +163,11 @@ describe("effectiveSettled", () => { |
| 163 | ).toBe(true); | 163 | ).toBe(true); |
| 164 | }); | 164 | }); |
| 165 | 165 | ||
| 166 | it("does not re-settle a warm thread on the merge signal: a message sent in a settled thread keeps it active until idle", () => { | 166 | it("settles immediately when a change request merges or closes", () => { |
| 167 | // The merge signal never clears, so without the idle guard a follow-up | 167 | const recentlyActive = makeShell({ activityAt: "2026-04-09T23:59:59.999Z" }); |
| 168 | // message would un-settle the row only until its turn completed, then | ||
| 169 | // snap straight back into the settled tail. | ||
| 170 | const justActive = makeShell({ activityAt: "2026-04-09T23:30:00.000Z" }); | ||
| 171 | // The idle gate is strict: activity exactly one hour old is still warm. | ||
| 172 | const boundary = makeShell({ activityAt: "2026-04-09T23:00:00.000Z" }); | ||
| 173 | const idle = makeShell({ activityAt: "2026-04-09T22:59:59.999Z" }); | ||
| 174 | |||
| 175 | for (const changeRequestState of ["merged", "closed"] as const) { | 168 | for (const changeRequestState of ["merged", "closed"] as const) { |
| 176 | expect( | 169 | expect( |
| 177 | effectiveSettled(justActive, { | 170 | effectiveSettled(recentlyActive, { |
| 178 | now: NOW, | ||
| 179 | autoSettleAfterDays: null, | ||
| 180 | changeRequestState, | ||
| 181 | }), | ||
| 182 | ).toBe(false); | ||
| 183 | expect( | ||
| 184 | effectiveSettled(boundary, { | ||
| 185 | now: NOW, | ||
| 186 | autoSettleAfterDays: null, | ||
| 187 | changeRequestState, | ||
| 188 | }), | ||
| 189 | ).toBe(false); | ||
| 190 | expect( | ||
| 191 | effectiveSettled(idle, { | ||
| 192 | now: NOW, | 171 | now: NOW, |
| 193 | autoSettleAfterDays: null, | 172 | autoSettleAfterDays: null, |
| 194 | changeRequestState, | 173 | changeRequestState, |
| ... | @@ -197,14 +176,18 @@ describe("effectiveSettled", () => { | ... | @@ -197,14 +176,18 @@ describe("effectiveSettled", () => { |
| 197 | } | 176 | } |
| 198 | }); | 177 | }); |
| 199 | 178 | ||
| 200 | it("re-settles a merged-PR thread once the follow-up burst goes idle", () => { | 179 | it("keeps an explicitly un-settled merged-PR thread active", () => { |
| 201 | // Same shell, advancing clock: active while warm, settled again after | 180 | const shell = makeShell({ |
| 202 | // the idle window passes — the burst cools and the merge signal wins. | 181 | settledOverride: "active", |
| 203 | const shell = makeShell({ activityAt: "2026-04-09T23:30:00.000Z" }); | 182 | activityAt: "2026-04-09T23:59:59.999Z", |
| 204 | const options = { autoSettleAfterDays: null, changeRequestState: "merged" as const }; | 183 | }); |
| 205 | 184 | expect( | |
| 206 | expect(effectiveSettled(shell, { ...options, now: NOW })).toBe(false); | 185 | effectiveSettled(shell, { |
| 207 | expect(effectiveSettled(shell, { ...options, now: "2026-04-10T00:30:00.001Z" })).toBe(true); | 186 | now: NOW, |
| 187 | autoSettleAfterDays: null, | ||
| 188 | changeRequestState: "merged", | ||
| 189 | }), | ||
| 190 | ).toBe(false); | ||
| 208 | }); | 191 | }); |
| 209 | 192 | ||
| 210 | it("never settles a starting session, even with a settled override", () => { | 193 | it("never settles a starting session, even with a settled override", () => { |
packages/client-runtime/src/state/threadSettled.ts+2-25
| ... | @@ -214,20 +214,6 @@ export function threadWokeAt( | ... | @@ -214,20 +214,6 @@ export function threadWokeAt( |
| 214 | return wakeAtMs <= Date.parse(options.now) ? shell.snoozedUntil : null; | 214 | return wakeAtMs <= Date.parse(options.now) ? shell.snoozedUntil : null; |
| 215 | } | 215 | } |
| 216 | 216 | ||
| 217 | /** | ||
| 218 | * A merged/closed change request settles its thread only once the thread has | ||
| 219 | * been idle this long. Without the idle guard the merge signal is permanent: | ||
| 220 | * sending a message to a merged-PR thread would un-settle the row only until | ||
| 221 | * its turn completed, then the still-merged PR would snap it straight back | ||
| 222 | * into the settled tail. An hour keeps the follow-up conversation visible | ||
| 223 | * while it is warm; once the burst goes stale the merge signal settles it | ||
| 224 | * again. Activity timestamps can originate on another device while `now` is | ||
| 225 | * this caller's clock: skew shortens or stretches the window by its size, | ||
| 226 | * the same exposure the inactivity auto-settle already accepts — worst case | ||
| 227 | * is a row changing lists early or late, never lost work. | ||
| 228 | */ | ||
| 229 | export const CHANGE_REQUEST_SETTLE_IDLE_MS = 60 * 60 * 1_000; | ||
| 230 | |||
| 231 | /** | 217 | /** |
| 232 | * Settled resolution over the server-backed settled lifecycle. Activity | 218 | * Settled resolution over the server-backed settled lifecycle. Activity |
| 233 | * blockers (pending approval/user-input, a live session, an unadjudicated | 219 | * blockers (pending approval/user-input, a live session, an unadjudicated |
| ... | @@ -235,7 +221,7 @@ export const CHANGE_REQUEST_SETTLE_IDLE_MS = 60 * 60 * 1_000; | ... | @@ -235,7 +221,7 @@ export const CHANGE_REQUEST_SETTLE_IDLE_MS = 60 * 60 * 1_000; |
| 235 | * override. Past the blockers, the explicit user override (thread.settle / | 221 | * override. Past the blockers, the explicit user override (thread.settle / |
| 236 | * thread.unsettle commands, projected into settledOverride + settledAt) | 222 | * thread.unsettle commands, projected into settledOverride + settledAt) |
| 237 | * wins in both directions; without one, a thread auto-settles on a | 223 | * wins in both directions; without one, a thread auto-settles on a |
| 238 | * merged/closed PR (once idle) or inactivity past the window. The server | 224 | * merged/closed PR immediately or on inactivity past the window. The server |
| 239 | * un-settles on real activity (user message, session start, approval/ | 225 | * un-settles on real activity (user message, session start, approval/ |
| 240 | * user-input request), so an override never goes stale silently. | 226 | * user-input request), so an override never goes stale silently. |
| 241 | */ | 227 | */ |
| ... | @@ -271,16 +257,7 @@ export function effectiveSettled( | ... | @@ -271,16 +257,7 @@ export function effectiveSettled( |
| 271 | // until real activity clears it server-side. | 257 | // until real activity clears it server-side. |
| 272 | if (shell.settledOverride === "active") return false; | 258 | if (shell.settledOverride === "active") return false; |
| 273 | if (options.changeRequestState === "merged" || options.changeRequestState === "closed") { | 259 | if (options.changeRequestState === "merged" || options.changeRequestState === "closed") { |
| 274 | // Only an idle thread settles on the merge signal: the signal itself | 260 | return true; |
| 275 | // never clears, so without this guard fresh activity (a message sent in | ||
| 276 | // a settled thread) would re-settle the moment its turn completed. | ||
| 277 | const lastActivityAt = threadLastActivityAt(shell); | ||
| 278 | if ( | ||
| 279 | lastActivityAt === null || | ||
| 280 | Date.parse(lastActivityAt) < Date.parse(options.now) - CHANGE_REQUEST_SETTLE_IDLE_MS | ||
| 281 | ) { | ||
| 282 | return true; | ||
| 283 | } | ||
| 284 | } | 261 | } |
| 285 | if (options.autoSettleAfterDays === null) return false; | 262 | if (options.autoSettleAfterDays === null) return false; |
| 286 | 263 |