| author |  Julius Marminge <julius0216@outlook.com> 2026-09-22 20:50:49-07:00 |
| committer |  GitHub <noreply@github.com> 2026-09-22 20:50:49-07:00 |
| log | 5975ec78b7eaff654e6183b99f8de7a05591e99d |
| tree | ce0437ed5ffe8ae0ce1a14d89c16ba0ccb3c0728 |
| parent | 0c91f687de606f62e157209f34e6aa74744b555a |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): background PR checks spend less GitHub quota (#13189)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>4 files changed, 144 insertions(+), 8 deletions(-)
apps
server
apps/server/src/git/GitManager.test.ts+56
| ... | @@ -15,6 +15,7 @@ import * as PlatformError from "effect/PlatformError"; | ... | @@ -15,6 +15,7 @@ import * as PlatformError from "effect/PlatformError"; |
| 15 | import * as References from "effect/References"; | 15 | import * as References from "effect/References"; |
| 16 | import * as Schema from "effect/Schema"; | 16 | import * as Schema from "effect/Schema"; |
| 17 | import * as Scope from "effect/Scope"; | 17 | import * as Scope from "effect/Scope"; |
| 18 | import { TestClock } from "effect/testing"; | ||
| 18 | import { ChildProcessSpawner } from "effect/unstable/process"; | 19 | import { ChildProcessSpawner } from "effect/unstable/process"; |
| 19 | import { expect } from "vite-plus/test"; | 20 | import { expect } from "vite-plus/test"; |
| 20 | import type { | 21 | import type { |
| ... | @@ -1490,6 +1491,61 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { | ... | @@ -1490,6 +1491,61 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { |
| 1490 | }), | 1491 | }), |
| 1491 | ); | 1492 | ); |
| 1492 | 1493 | ||
| 1494 | it.effect("branch PR lookup rechecks open PRs every minute and settled answers less often", () => | ||
| 1495 | Effect.gen(function* () { | ||
| 1496 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | ||
| 1497 | yield* initRepo(repoDir); | ||
| 1498 | const remoteDir = yield* createBareRemote(); | ||
| 1499 | yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); | ||
| 1500 | for (const branch of ["feature/open-pr", "feature/merged-pr", "feature/no-pr"]) { | ||
| 1501 | yield* runGit(repoDir, ["checkout", "-b", branch, "main"]); | ||
| 1502 | yield* runGit(repoDir, ["push", "-u", "origin", branch]); | ||
| 1503 | } | ||
| 1504 | const pullRequest = (number: number, headRefName: string, state: string) => ({ | ||
| 1505 | number, | ||
| 1506 | title: headRefName, | ||
| 1507 | url: `https://github.com/pingdotgg/codething-mvp/pull/${number}`, | ||
| 1508 | baseRefName: "main", | ||
| 1509 | headRefName, | ||
| 1510 | state, | ||
| 1511 | updatedAt: "2026-04-07T15:00:00Z", | ||
| 1512 | }); | ||
| 1513 | const { manager, ghCalls } = yield* makeManager({ | ||
| 1514 | ghScenario: { | ||
| 1515 | prListByHeadSelector: { | ||
| 1516 | "feature/open-pr": encodeCliJson([pullRequest(301, "feature/open-pr", "OPEN")]), | ||
| 1517 | "feature/merged-pr": encodeCliJson([pullRequest(302, "feature/merged-pr", "MERGED")]), | ||
| 1518 | }, | ||
| 1519 | }, | ||
| 1520 | }); | ||
| 1521 | const lookupAll = Effect.forEach( | ||
| 1522 | ["feature/open-pr", "feature/merged-pr", "feature/no-pr"], | ||
| 1523 | (branch) => manager.branchPullRequest({ cwd: repoDir, branch }), | ||
| 1524 | ); | ||
| 1525 | const prListCalls = () => ghCalls.filter((call) => call.startsWith("pr list ")); | ||
| 1526 | |||
| 1527 | yield* lookupAll; | ||
| 1528 | expect(prListCalls()).toHaveLength(3); | ||
| 1529 | |||
| 1530 | yield* TestClock.adjust("61 seconds"); | ||
| 1531 | yield* lookupAll; | ||
| 1532 | expect(prListCalls()).toHaveLength(4); | ||
| 1533 | expect(prListCalls().at(-1)).toContain("--head feature/open-pr"); | ||
| 1534 | |||
| 1535 | // Just inside the 5-minute window only the open PR is asked again. | ||
| 1536 | yield* TestClock.adjust("238 seconds"); | ||
| 1537 | yield* lookupAll; | ||
| 1538 | expect(prListCalls()).toHaveLength(5); | ||
| 1539 | expect(prListCalls().at(-1)).toContain("--head feature/open-pr"); | ||
| 1540 | |||
| 1541 | // Just past it the settled answers expire too. | ||
| 1542 | yield* TestClock.adjust("2 seconds"); | ||
| 1543 | yield* lookupAll; | ||
| 1544 | expect(prListCalls()).toHaveLength(7); | ||
| 1545 | expect(prListCalls().slice(-2).join("\n")).not.toContain("--head feature/open-pr"); | ||
| 1546 | }), | ||
| 1547 | ); | ||
| 1548 | |||
| 1493 | it.effect("branch PR lookup propagates provider failures", () => | 1549 | it.effect("branch PR lookup propagates provider failures", () => |
| 1494 | Effect.gen(function* () { | 1550 | Effect.gen(function* () { |
| 1495 | const repoDir = yield* makeTempDir("t3code-git-manager-"); | 1551 | const repoDir = yield* makeTempDir("t3code-git-manager-"); |
apps/server/src/git/GitManager.ts+9-1
| ... | @@ -144,6 +144,12 @@ const STATUS_RESULT_CACHE_CAPACITY = 2_048; | ... | @@ -144,6 +144,12 @@ const STATUS_RESULT_CACHE_CAPACITY = 2_048; |
| 144 | // exponentially via prLookupFailureTtl, so throttling pressure still drops | 144 | // exponentially via prLookupFailureTtl, so throttling pressure still drops |
| 145 | // under 429s instead of amplifying it. | 145 | // under 429s instead of amplifying it. |
| 146 | const PR_LOOKUP_CACHE_TTL = Duration.seconds(60); | 146 | const PR_LOOKUP_CACHE_TTL = Duration.seconds(60); |
| 147 | // Answers without an open PR ("no PR yet", merged, closed) only change when | ||
| 148 | // someone opens a PR, and the paths that do that in-app (turn end, push, | ||
| 149 | // create PR, user refresh) bypass this cache. Re-asking every minute for each | ||
| 150 | // idle branch was the bulk of background GitHub quota use, so these wait out | ||
| 151 | // a longer TTL and a PR opened outside the app shows up within minutes. | ||
| 152 | const PR_LOOKUP_NO_OPEN_PR_CACHE_TTL = Duration.minutes(5); | ||
| 147 | const PR_LOOKUP_FAILURE_BASE_TTL = Duration.seconds(20); | 153 | const PR_LOOKUP_FAILURE_BASE_TTL = Duration.seconds(20); |
| 148 | const PR_LOOKUP_FAILURE_MAX_TTL = Duration.minutes(15); | 154 | const PR_LOOKUP_FAILURE_MAX_TTL = Duration.minutes(15); |
| 149 | const PR_LOOKUP_CACHE_CAPACITY = 2_048; | 155 | const PR_LOOKUP_CACHE_CAPACITY = 2_048; |
| ... | @@ -1105,7 +1111,9 @@ export const make = Effect.gen(function* () { | ... | @@ -1105,7 +1111,9 @@ export const make = Effect.gen(function* () { |
| 1105 | timeToLive: (exit, key) => { | 1111 | timeToLive: (exit, key) => { |
| 1106 | if (Exit.isSuccess(exit)) { | 1112 | if (Exit.isSuccess(exit)) { |
| 1107 | prLookupFailureStreakByKey.delete(key); | 1113 | prLookupFailureStreakByKey.delete(key); |
| 1108 | return PR_LOOKUP_CACHE_TTL; | 1114 | return exit.value.latest?.state === "open" |
| 1115 | ? PR_LOOKUP_CACHE_TTL | ||
| 1116 | : PR_LOOKUP_NO_OPEN_PR_CACHE_TTL; | ||
| 1109 | } | 1117 | } |
| 1110 | return nextPrLookupFailureTtl(key); | 1118 | return nextPrLookupFailureTtl(key); |
| 1111 | }, | 1119 | }, |
apps/server/src/orchestration/ThreadSettlementReactor.test.ts+43
| ... | @@ -475,6 +475,49 @@ describe("ThreadSettlementReactor", () => { | ... | @@ -475,6 +475,49 @@ describe("ThreadSettlementReactor", () => { |
| 475 | }), | 475 | }), |
| 476 | ), | 476 | ), |
| 477 | ); | 477 | ); |
| 478 | it.effect("skips the branch recheck when a terminal link would settle nothing", () => | ||
| 479 | Effect.scoped( | ||
| 480 | Effect.gen(function* () { | ||
| 481 | yield* TestClock.setTime(Date.parse(NOW)); | ||
| 482 | const previous = { | ||
| 483 | projectId: PROJECT_ID, | ||
| 484 | repository: "owner/repository", | ||
| 485 | number: 1, | ||
| 486 | url: "https://example.test/owner/repository/pull/1", | ||
| 487 | }; | ||
| 488 | const fixture = yield* makeHarness({ | ||
| 489 | snapshot: makeSnapshot( | ||
| 490 | [ | ||
| 491 | makeThread("resumed-manual", { | ||
| 492 | branch: "main", | ||
| 493 | linkedPullRequest: previous, | ||
| 494 | latestUserMessageAt: "2026-08-28T00:00:00.000Z", | ||
| 495 | }), | ||
| 496 | ], | ||
| 497 | [makeProject()], | ||
| 498 | ), | ||
| 499 | settings: { | ||
| 500 | ...DEFAULT_SERVER_SETTINGS, | ||
| 501 | sidebarAutoSettleAfterDays: null, | ||
| 502 | sidebarAutoSettleOnMerge: true, | ||
| 503 | }, | ||
| 504 | branchPullRequest: () => Effect.succeed(makeBranchPullRequest("open")), | ||
| 505 | pullRequestSummary: (input) => | ||
| 506 | Effect.succeed({ | ||
| 507 | ...makePullRequestSummary({ ...input, state: "merged" }), | ||
| 508 | mergedAt: "2026-08-27T00:00:00.000Z", | ||
| 509 | }), | ||
| 510 | }); | ||
| 511 | yield* Effect.gen(function* () { | ||
| 512 | const reactor = yield* ThreadSettlementReactor.ThreadSettlementReactor; | ||
| 513 | yield* startHarness(reactor, fixture.activation, fixture.snapshotReads); | ||
| 514 | assert.deepStrictEqual(yield* Ref.get(fixture.commands), []); | ||
| 515 | assert.strictEqual((yield* Ref.get(fixture.summaryCalls)).length, 1); | ||
| 516 | assert.deepStrictEqual(yield* Ref.get(fixture.branchCalls), []); | ||
| 517 | }).pipe(Effect.provide(fixture.layer)); | ||
| 518 | }), | ||
| 519 | ), | ||
| 520 | ); | ||
| 478 | it.effect("uses saved PRs without settling resumed threads or branches with newer PRs", () => | 521 | it.effect("uses saved PRs without settling resumed threads or branches with newer PRs", () => |
| 479 | Effect.scoped( | 522 | Effect.scoped( |
| 480 | Effect.gen(function* () { | 523 | Effect.gen(function* () { |
apps/server/src/orchestration/ThreadSettlementReactor.ts+36-7
| ... | @@ -209,9 +209,30 @@ export const make = Effect.gen(function* () { | ... | @@ -209,9 +209,30 @@ export const make = Effect.gen(function* () { |
| 209 | }; | 209 | }; |
| 210 | const groups = Map.groupBy(lookupCandidates, lookupKey); | 210 | const groups = Map.groupBy(lookupCandidates, lookupKey); |
| 211 | 211 | ||
| 212 | const wouldSettle = Effect.fn("ThreadSettlementReactor.wouldSettle")(function* ( | ||
| 213 | group: ReadonlyArray<(typeof candidates)[number]>, | ||
| 214 | pullRequest: SettlementPullRequest, | ||
| 215 | ) { | ||
| 216 | const currentSettings = yield* settingsService.getSettings; | ||
| 217 | const decisionNow = DateTime.formatIso(yield* DateTime.now); | ||
| 218 | return group.some((thread) => { | ||
| 219 | const { settings } = resolveProjectSettings(currentSettings, thread.projectId); | ||
| 220 | return ( | ||
| 221 | resolveAutoSettlementAt({ | ||
| 222 | thread, | ||
| 223 | pullRequest, | ||
| 224 | now: decisionNow, | ||
| 225 | autoSettleAfterDays: settings.sidebarAutoSettleAfterDays, | ||
| 226 | autoSettleOnMerge: settings.sidebarAutoSettleOnMerge, | ||
| 227 | }) !== null | ||
| 228 | ); | ||
| 229 | }); | ||
| 230 | }); | ||
| 231 | |||
| 212 | const pullRequestFor = Effect.fn("ThreadSettlementReactor.pullRequestFor")(function* ( | 232 | const pullRequestFor = Effect.fn("ThreadSettlementReactor.pullRequestFor")(function* ( |
| 213 | thread: (typeof candidates)[number], | 233 | group: ReadonlyArray<(typeof candidates)[number]>, |
| 214 | ) { | 234 | ) { |
| 235 | const thread = group[0]!; | ||
| 215 | const reference = thread.linkedPullRequest ?? thread.branchPullRequest; | 236 | const reference = thread.linkedPullRequest ?? thread.branchPullRequest; |
| 216 | if (reference != null) { | 237 | if (reference != null) { |
| 217 | const matchesMerge = | 238 | const matchesMerge = |
| ... | @@ -236,10 +257,21 @@ export const make = Effect.gen(function* () { | ... | @@ -236,10 +257,21 @@ export const make = Effect.gen(function* () { |
| 236 | }, | 257 | }, |
| 237 | { recoverTransientFailure: false }, | 258 | { recoverTransientFailure: false }, |
| 238 | ); | 259 | ); |
| 260 | const terminal = { | ||
| 261 | state: summary.state, | ||
| 262 | closedAt: summary.closedAt ?? null, | ||
| 263 | mergedAt: summary.mergedAt ?? null, | ||
| 264 | } satisfies SettlementPullRequest; | ||
| 239 | const cwd = lookupCwdByThreadId.get(thread.id); | 265 | const cwd = lookupCwdByThreadId.get(thread.id); |
| 240 | if (summary.state !== "open" && thread.branch !== null && cwd !== undefined) { | 266 | if (summary.state !== "open" && thread.branch !== null && cwd !== undefined) { |
| 241 | // A reused branch can already have a new open PR while discovery | 267 | // A reused branch can already have a new open PR while discovery |
| 242 | // is replacing its old link. Do not let settlement win that race. | 268 | // is replacing its old link. Do not let settlement win that race. |
| 269 | // Only pay for the uncached lookup when this sweep would otherwise | ||
| 270 | // settle: a terminal link that settles nothing (resumed thread, | ||
| 271 | // settle-on-merge off) would re-query the host every minute. A | ||
| 272 | // group that becomes eligible after this check waits for the next | ||
| 273 | // sweep rather than settling on the unverified link. | ||
| 274 | if (!(yield* wouldSettle(group, terminal))) return undefined; | ||
| 243 | const current = yield* git.branchPullRequest( | 275 | const current = yield* git.branchPullRequest( |
| 244 | { cwd, branch: thread.branch }, | 276 | { cwd, branch: thread.branch }, |
| 245 | { refresh: true }, | 277 | { refresh: true }, |
| ... | @@ -253,11 +285,7 @@ export const make = Effect.gen(function* () { | ... | @@ -253,11 +285,7 @@ export const make = Effect.gen(function* () { |
| 253 | return current; | 285 | return current; |
| 254 | } | 286 | } |
| 255 | } | 287 | } |
| 256 | return { | 288 | return terminal; |
| 257 | state: summary.state, | ||
| 258 | closedAt: summary.closedAt ?? null, | ||
| 259 | mergedAt: summary.mergedAt ?? null, | ||
| 260 | } satisfies SettlementPullRequest; | ||
| 261 | } | 289 | } |
| 262 | if (thread.branch === null) return null; | 290 | if (thread.branch === null) return null; |
| 263 | const cwd = lookupCwdByThreadId.get(thread.id); | 291 | const cwd = lookupCwdByThreadId.get(thread.id); |
| ... | @@ -271,7 +299,8 @@ export const make = Effect.gen(function* () { | ... | @@ -271,7 +299,8 @@ export const make = Effect.gen(function* () { |
| 271 | groups.values(), | 299 | groups.values(), |
| 272 | (group) => | 300 | (group) => |
| 273 | Effect.gen(function* () { | 301 | Effect.gen(function* () { |
| 274 | const pullRequest = yield* pullRequestFor(group[0]!); | 302 | const pullRequest = yield* pullRequestFor(group); |
| 303 | if (pullRequest === undefined) return; | ||
| 275 | yield* Effect.forEach(group, (thread) => settleThread(thread, pullRequest), { | 304 | yield* Effect.forEach(group, (thread) => settleThread(thread, pullRequest), { |
| 276 | discard: true, | 305 | discard: true, |
| 277 | }); | 306 | }); |