| author |  Theo Browne <me@t3.gg> 2026-07-31 15:46:15-07:00 |
| committer |  GitHub <noreply@github.com> 2026-07-31 15:46:15-07:00 |
| log | 491219bf1f4db7144b747fe02af0cbc075668b12 |
| tree | eee343d3f4404c6fe1c597438cc8d0be748ca19b |
| parent | e5c75470678e33ca2bd1001d9d93493fe437972a |
| signature | Signed by PGP key B5690EEEBB952194 |
fix: threads with open PRs no longer auto-settle (#5151)
2 files changed, 31 insertions(+), 3 deletions(-)
packages
client-runtime
packages/client-runtime/src/state/threadSettled.test.ts+24-2
| ... | @@ -115,13 +115,15 @@ describe("effectiveSettled", () => { | ... | @@ -115,13 +115,15 @@ describe("effectiveSettled", () => { |
| 115 | // Settled iff nothing blocks (pending work / live session) AND | 115 | // Settled iff nothing blocks (pending work / live session) AND |
| 116 | // the override says settled, or (with no override) a merged PR | 116 | // the override says settled, or (with no override) a merged PR |
| 117 | // or staleness auto-settles. The "active" pin suppresses both | 117 | // or staleness auto-settles. The "active" pin suppresses both |
| 118 | // auto signals. | 118 | // auto signals, and an open PR suppresses the inactivity path: |
| 119 | // a thread with a PR out for review is never done, however quiet. | ||
| 119 | expected: | 120 | expected: |
| 120 | pending === undefined && | 121 | pending === undefined && |
| 121 | !running && | 122 | !running && |
| 122 | (settledOverride === "settled" || | 123 | (settledOverride === "settled" || |
| 123 | (settledOverride === null && | 124 | (settledOverride === null && |
| 124 | (changeRequestState === "merged" || inactivity === "stale"))), | 125 | (changeRequestState === "merged" || |
| 126 | (changeRequestState !== "open" && inactivity === "stale")))), | ||
| 125 | })), | 127 | })), |
| 126 | ), | 128 | ), |
| 127 | ), | 129 | ), |
| ... | @@ -176,6 +178,26 @@ describe("effectiveSettled", () => { | ... | @@ -176,6 +178,26 @@ describe("effectiveSettled", () => { |
| 176 | } | 178 | } |
| 177 | }); | 179 | }); |
| 178 | 180 | ||
| 181 | it("never auto-settles a stale thread with an open change request", () => { | ||
| 182 | const stale = makeShell({ activityAt: STALE }); | ||
| 183 | expect( | ||
| 184 | effectiveSettled(stale, { | ||
| 185 | now: NOW, | ||
| 186 | autoSettleAfterDays: 3, | ||
| 187 | changeRequestState: "open", | ||
| 188 | }), | ||
| 189 | ).toBe(false); | ||
| 190 | // An explicit user settle still wins: open PR only blocks the auto path. | ||
| 191 | const settled = makeShell({ settledOverride: "settled", activityAt: STALE }); | ||
| 192 | expect( | ||
| 193 | effectiveSettled(settled, { | ||
| 194 | now: NOW, | ||
| 195 | autoSettleAfterDays: 3, | ||
| 196 | changeRequestState: "open", | ||
| 197 | }), | ||
| 198 | ).toBe(true); | ||
| 199 | }); | ||
| 200 | |||
| 179 | it("keeps an explicitly un-settled merged-PR thread active", () => { | 201 | it("keeps an explicitly un-settled merged-PR thread active", () => { |
| 180 | const shell = makeShell({ | 202 | const shell = makeShell({ |
| 181 | settledOverride: "active", | 203 | settledOverride: "active", |
packages/client-runtime/src/state/threadSettled.ts+7-1
| ... | @@ -222,7 +222,8 @@ export function threadWokeAt( | ... | @@ -222,7 +222,8 @@ export function threadWokeAt( |
| 222 | * override. Past the blockers, the explicit user override (thread.settle / | 222 | * override. Past the blockers, the explicit user override (thread.settle / |
| 223 | * thread.unsettle commands, projected into settledOverride + settledAt) | 223 | * thread.unsettle commands, projected into settledOverride + settledAt) |
| 224 | * wins in both directions; without one, a thread auto-settles on a | 224 | * wins in both directions; without one, a thread auto-settles on a |
| 225 | * merged/closed PR immediately or on inactivity past the window. The server | 225 | * merged/closed PR immediately or on inactivity past the window — except |
| 226 | * that an open PR blocks the inactivity path entirely. The server | ||
| 226 | * un-settles on real activity (user message, session start, approval/ | 227 | * un-settles on real activity (user message, session start, approval/ |
| 227 | * user-input request), so an override never goes stale silently. | 228 | * user-input request), so an override never goes stale silently. |
| 228 | */ | 229 | */ |
| ... | @@ -260,6 +261,11 @@ export function effectiveSettled( | ... | @@ -260,6 +261,11 @@ export function effectiveSettled( |
| 260 | if (options.changeRequestState === "merged" || options.changeRequestState === "closed") { | 261 | if (options.changeRequestState === "merged" || options.changeRequestState === "closed") { |
| 261 | return true; | 262 | return true; |
| 262 | } | 263 | } |
| 264 | // An open PR is unfinished business regardless of how long the thread has | ||
| 265 | // been quiet: review can take days, and hiding the thread would bury the | ||
| 266 | // work waiting on it. Only merge/close (above) or an explicit user settle | ||
| 267 | // resolves it. | ||
| 268 | if (options.changeRequestState === "open") return false; | ||
| 263 | if (options.autoSettleAfterDays === null) return false; | 269 | if (options.autoSettleAfterDays === null) return false; |
| 264 | 270 | ||
| 265 | const lastActivityAt = threadLastActivityAt(shell); | 271 | const lastActivityAt = threadLastActivityAt(shell); |