| author |  Theo Browne <me@t3.gg> 2026-10-07 00:46:53-07:00 |
| committer |  GitHub <noreply@github.com> 2026-10-07 00:46:53-07:00 |
| log | 453e596da7e98d973e3853a5c2728681dedf363b |
| tree | a202fdc0ed4065fc4c294afe20dc232ed77c4174 |
| parent | 655dfbe8ba87f0f9ef1d6fad83c20634634653aa |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): settled threads stop polling their pull requests (#16762)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>3 files changed, 38 insertions(+), 20 deletions(-)
apps
server
src
orchestration-v2
docs
apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts+27-13
| ... | @@ -802,7 +802,7 @@ describe("PullRequestSyncReactor", () => { | ... | @@ -802,7 +802,7 @@ describe("PullRequestSyncReactor", () => { |
| 802 | ), | 802 | ), |
| 803 | ); | 803 | ); |
| 804 | 804 | ||
| 805 | it.effect("polls open pull requests on settled threads every fifteen minutes", () => | 805 | it.effect("leaves a settled thread's links unread until the thread is unsettled", () => |
| 806 | Effect.scoped( | 806 | Effect.scoped( |
| 807 | Effect.gen(function* () { | 807 | Effect.gen(function* () { |
| 808 | yield* TestClock.setTime(Date.parse(NOW)); | 808 | yield* TestClock.setTime(Date.parse(NOW)); |
| ... | @@ -811,29 +811,35 @@ describe("PullRequestSyncReactor", () => { | ... | @@ -811,29 +811,35 @@ describe("PullRequestSyncReactor", () => { |
| 811 | makeThread("settled", { | 811 | makeThread("settled", { |
| 812 | settledOverride: "settled", | 812 | settledOverride: "settled", |
| 813 | settledAt: "2026-08-21T00:00:00.000Z", | 813 | settledAt: "2026-08-21T00:00:00.000Z", |
| 814 | pullRequests: [makeLink(5, { state: "open" })], | 814 | pullRequests: [makeLink(5, { state: "open" }), makeLink(6, { state: "closed" })], |
| 815 | }), | 815 | }), |
| 816 | ]), | 816 | ]), |
| 817 | }); | 817 | }); |
| 818 | 818 | ||
| 819 | yield* Effect.gen(function* () { | 819 | yield* Effect.gen(function* () { |
| 820 | const reactor = yield* startAndSweep(fixture); | 820 | const reactor = yield* startAndSweep(fixture); |
| 821 | assert.strictEqual((yield* Ref.get(fixture.summaryCalls)).length, 1); | 821 | for (let index = 0; index < 15; index += 1) yield* sweepAgain(fixture, reactor); |
| 822 | 822 | assert.deepStrictEqual(yield* Ref.get(fixture.summaryCalls), []); | |
| 823 | yield* sweepAgain(fixture, reactor); | ||
| 824 | assert.strictEqual((yield* Ref.get(fixture.summaryCalls)).length, 1); | ||
| 825 | |||
| 826 | for (let index = 0; index < 13; index += 1) yield* sweepAgain(fixture, reactor); | ||
| 827 | assert.strictEqual((yield* Ref.get(fixture.summaryCalls)).length, 1); | ||
| 828 | 823 | ||
| 824 | yield* Ref.update(fixture.snapshots, (snapshot) => ({ | ||
| 825 | ...snapshot, | ||
| 826 | threads: snapshot.threads.map((thread) => ({ | ||
| 827 | ...thread, | ||
| 828 | settledOverride: null, | ||
| 829 | settledAt: null, | ||
| 830 | })), | ||
| 831 | })); | ||
| 829 | yield* sweepAgain(fixture, reactor); | 832 | yield* sweepAgain(fixture, reactor); |
| 830 | assert.strictEqual((yield* Ref.get(fixture.summaryCalls)).length, 2); | 833 | assert.deepStrictEqual( |
| 834 | (yield* Ref.get(fixture.summaryCalls)).map((call) => call.number).toSorted(), | ||
| 835 | [5, 6], | ||
| 836 | ); | ||
| 831 | }).pipe(Effect.provide(fixture.layer)); | 837 | }).pipe(Effect.provide(fixture.layer)); |
| 832 | }), | 838 | }), |
| 833 | ), | 839 | ), |
| 834 | ); | 840 | ); |
| 835 | 841 | ||
| 836 | it.effect("auto-links missing native stack layers and leaves dismissed ones alone", () => | 842 | it.effect("auto-links missing native stack layers on active threads only", () => |
| 837 | Effect.scoped( | 843 | Effect.scoped( |
| 838 | Effect.gen(function* () { | 844 | Effect.gen(function* () { |
| 839 | yield* TestClock.setTime(Date.parse(NOW)); | 845 | yield* TestClock.setTime(Date.parse(NOW)); |
| ... | @@ -856,6 +862,11 @@ describe("PullRequestSyncReactor", () => { | ... | @@ -856,6 +862,11 @@ describe("PullRequestSyncReactor", () => { |
| 856 | makeLink(41, { state: "merged" }, { source: "stack-dismissed" }), | 862 | makeLink(41, { state: "merged" }, { source: "stack-dismissed" }), |
| 857 | ], | 863 | ], |
| 858 | }), | 864 | }), |
| 865 | makeThread("settled", { | ||
| 866 | settledOverride: "settled", | ||
| 867 | settledAt: "2026-08-21T00:00:00.000Z", | ||
| 868 | pullRequests: [makeLink(42)], | ||
| 869 | }), | ||
| 859 | ]), | 870 | ]), |
| 860 | stack: () => Effect.succeed(stack), | 871 | stack: () => Effect.succeed(stack), |
| 861 | }); | 872 | }); |
| ... | @@ -865,8 +876,11 @@ describe("PullRequestSyncReactor", () => { | ... | @@ -865,8 +876,11 @@ describe("PullRequestSyncReactor", () => { |
| 865 | 876 | ||
| 866 | const syncCommands = yield* Ref.get(fixture.syncCommands); | 877 | const syncCommands = yield* Ref.get(fixture.syncCommands); |
| 867 | assert.deepStrictEqual( | 878 | assert.deepStrictEqual( |
| 868 | syncCommands.map((command) => [command.number, command.stack] as const), | 879 | syncCommands.map((command) => [command.threadId, command.stack] as const), |
| 869 | [[42, { kind: "native", ...stack }]], | 880 | [ |
| 881 | [ThreadId.make("one"), { kind: "native", ...stack }], | ||
| 882 | [ThreadId.make("settled"), { kind: "native", ...stack }], | ||
| 883 | ], | ||
| 870 | ); | 884 | ); |
| 871 | assert.deepStrictEqual( | 885 | assert.deepStrictEqual( |
| 872 | (yield* Ref.get(fixture.linkCommands)).map(({ commandId: _, ...rest }) => rest), | 886 | (yield* Ref.get(fixture.linkCommands)).map(({ commandId: _, ...rest }) => rest), |
apps/server/src/orchestration-v2/PullRequestSyncReactor.ts+10-6
| ... | @@ -172,10 +172,12 @@ export const make = Effect.gen(function* () { | ... | @@ -172,10 +172,12 @@ export const make = Effect.gen(function* () { |
| 172 | const isDue = (key: string, entries: ReadonlyArray<LinkEntry>, nowMs: number): boolean => { | 172 | const isDue = (key: string, entries: ReadonlyArray<LinkEntry>, nowMs: number): boolean => { |
| 173 | if (requested.has(key) || retryStacks.has(key)) return true; | 173 | if (requested.has(key) || retryStacks.has(key)) return true; |
| 174 | if (entries.some((entry) => entry.link.snapshot === null)) return true; | 174 | if (entries.some((entry) => entry.link.snapshot === null)) return true; |
| 175 | if (entries.every((entry) => entry.link.snapshot?.state === "merged")) return false; | 175 | // Settled threads stop watching their pull requests. Unsettling one makes its links due on |
| 176 | if (entries.some((entry) => entry.link.snapshot?.state === "open" && isUnsettled(entry.thread))) | 176 | // the next sweep, since the cadence clock below kept running while it was settled. |
| 177 | return true; | 177 | const active = entries.filter((entry) => isUnsettled(entry.thread)); |
| 178 | // Closed requests can reopen on the host, including after the thread settles. | 178 | if (active.every((entry) => entry.link.snapshot?.state === "merged")) return false; |
| 179 | if (active.some((entry) => entry.link.snapshot?.state === "open")) return true; | ||
| 180 | // Closed requests can reopen on the host. | ||
| 179 | const last = lastSyncedAt.get(key); | 181 | const last = lastSyncedAt.get(key); |
| 180 | return last === undefined || nowMs - last >= SLOW_SYNC_INTERVAL_MS; | 182 | return last === undefined || nowMs - last >= SLOW_SYNC_INTERVAL_MS; |
| 181 | }; | 183 | }; |
| ... | @@ -222,8 +224,10 @@ export const make = Effect.gen(function* () { | ... | @@ -222,8 +224,10 @@ export const make = Effect.gen(function* () { |
| 222 | link.snapshot === null || | 224 | link.snapshot === null || |
| 223 | !snapshotFieldsEqual(link.snapshot, fields) || | 225 | !snapshotFieldsEqual(link.snapshot, fields) || |
| 224 | !stacksEqual(link.stack, nextStack); | 226 | !stacksEqual(link.stack, nextStack); |
| 225 | // Persist discovered siblings before a terminal snapshot can trigger settlement. | 227 | // Persist discovered siblings before a terminal snapshot can trigger settlement. A settled |
| 226 | for (const layer of fetchedStack?.stack?.layers ?? []) { | 228 | // thread that shares this pull request with an active one takes the fresh snapshot, but |
| 229 | // gains no links. | ||
| 230 | for (const layer of isUnsettled(thread) ? (fetchedStack?.stack?.layers ?? []) : []) { | ||
| 227 | const layerKey = { | 231 | const layerKey = { |
| 228 | host: normalizeThreadPullRequestKey(link).host, | 232 | host: normalizeThreadPullRequestKey(link).host, |
| 229 | repository: link.repository, | 233 | repository: link.repository, |
docs/user/source-control.md+1-1
| ... | @@ -195,7 +195,7 @@ Linking and unlinking are available in the web and desktop clients. | ... | @@ -195,7 +195,7 @@ Linking and unlinking are available in the web and desktop clients. |
| 195 | The **Linked pull requests** panel lists every review and groups stacks. Unlink a review from its | 195 | The **Linked pull requests** panel lists every review and groups stacks. Unlink a review from its |
| 196 | row menu. An unlinked stack layer stays out of later syncs. Open linked reviews refresh on the server; | 196 | row menu. An unlinked stack layer stays out of later syncs. Open linked reviews refresh on the server; |
| 197 | closed reviews refresh periodically so reopening one on the host is detected. Merged reviews refresh | 197 | closed reviews refresh periodically so reopening one on the host is detected. Merged reviews refresh |
| 198 | when requested. With **Auto-settle merged threads** enabled, a thread can settle after every linked | 198 | when requested. A settled thread's reviews stop refreshing until you unsettle it. With **Auto-settle merged threads** enabled, a thread can settle after every linked |
| 199 | review is terminal. An open or unsynced link keeps it active. | 199 | review is terminal. An open or unsynced link keeps it active. |
| 200 | 200 | ||
| 201 | Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While | 201 | Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While |