| author | |
| committer | |
| log | 758dc290e900e35958d37fec41fed4a357ad60fa |
| tree | dbcca5f31502044777fd6af2a85376c7cc899918 |
| parent | dcbaf05279984187b96c99edc2416abf91e77b96 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>12 files changed, 715 insertions(+), 44 deletions(-)
apps/server/src/git/GitManager.test.ts+1-1| ... | @@ -2661,7 +2661,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { | ... | @@ -2661,7 +2661,7 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { |
| 2661 | providerOperation: "listChangeRequests", | 2661 | providerOperation: "listChangeRequests", |
| 2662 | providerCommand: "gh", | 2662 | providerCommand: "gh", |
| 2663 | errorDetail: | 2663 | errorDetail: |
| 2664 | "GitHub API rate limit exceeded. Run `gh api rate_limit` to inspect the quota and reset time.", | 2664 | "GitHub API rate limit exceeded. For the GraphQL quota and reset time, run `gh api graphql -f query='{rateLimit{remaining resetAt}}'`; `gh api rate_limit` reports REST.", |
| 2665 | }); | 2665 | }); |
| 2666 | const loggedText = [ | 2666 | const loggedText = [ |
| 2667 | warning?.message ?? "", | 2667 | warning?.message ?? "", |
apps/server/src/orchestration-v2/PullRequestWatchReactor.ts+126-33| ... | @@ -26,7 +26,10 @@ import * as Schedule from "effect/Schedule"; | ... | @@ -26,7 +26,10 @@ import * as Schedule from "effect/Schedule"; |
| 26 | import * as Schema from "effect/Schema"; | 26 | import * as Schema from "effect/Schema"; |
| 27 | import type * as Scope from "effect/Scope"; | 27 | import type * as Scope from "effect/Scope"; |
| 28 | 28 | ||
| 29 | import { PullRequestProviderError } from "../pullRequest/PullRequestProvider.ts"; | 29 | import { |
| 30 | type ProviderChangeRequestWatchFingerprint, | ||
| 31 | PullRequestProviderError, | ||
| 32 | } from "../pullRequest/PullRequestProvider.ts"; | ||
| 30 | import * as PullRequestService from "../pullRequest/PullRequestService.ts"; | 33 | import * as PullRequestService from "../pullRequest/PullRequestService.ts"; |
| 31 | import { forkParked } from "../serverActivation.ts"; | 34 | import { forkParked } from "../serverActivation.ts"; |
| 32 | import * as Orchestrator from "./Orchestrator.ts"; | 35 | import * as Orchestrator from "./Orchestrator.ts"; |
| ... | @@ -41,10 +44,16 @@ const SWEEP_MINUTES = 2; | ... | @@ -41,10 +44,16 @@ const SWEEP_MINUTES = 2; |
| 41 | /** Reads in a row that failed for a reason other than a rate limit before the watch ends. */ | 44 | /** Reads in a row that failed for a reason other than a rate limit before the watch ends. */ |
| 42 | const READ_FAILURE_LIMIT = 8; | 45 | const READ_FAILURE_LIMIT = 8; |
| 43 | /** | 46 | /** |
| 44 | * A pull request with nothing in flight is read again only when its sync snapshot moves, or | 47 | * Without a host fingerprint, a pull request with nothing in flight is read again only when its |
| 45 | * after this long, for news the snapshot cannot show, such as a bot editing its review. | 48 | * sync snapshot moves, or after this long, for news the snapshot cannot show, such as a bot |
| 49 | * editing its review. | ||
| 46 | */ | 50 | */ |
| 47 | const QUIET_REREAD_MS = 10 * 60_000; | 51 | const QUIET_REREAD_MS = 10 * 60_000; |
| 52 | /** | ||
| 53 | * With a host fingerprint, how long until the activity is read again anyway: the fingerprint | ||
| 54 | * does not see edits to comments inside review threads. | ||
| 55 | */ | ||
| 56 | const FINGERPRINT_REREAD_MS = 30 * 60_000; | ||
| 48 | 57 | ||
| 49 | const isProviderError = Schema.is(PullRequestProviderError); | 58 | const isProviderError = Schema.is(PullRequestProviderError); |
| 50 | 59 | ||
| ... | @@ -81,12 +90,25 @@ interface WatchGroup { | ... | @@ -81,12 +90,25 @@ interface WatchGroup { |
| 81 | 90 | ||
| 82 | /** The last successful read of a pull request, kept in memory: a restart reads each once. */ | 91 | /** The last successful read of a pull request, kept in memory: a restart reads each once. */ |
| 83 | interface LastRead { | 92 | interface LastRead { |
| 93 | /** When the activity was last read. */ | ||
| 84 | readonly at: number; | 94 | readonly at: number; |
| 95 | /** The sync snapshot this read saw. */ | ||
| 85 | readonly fingerprint: string; | 96 | readonly fingerprint: string; |
| 86 | /** Nothing is in flight or unread, so only a moved snapshot or the reread brings news. */ | 97 | /** A check was still running or mergeability unknown, so the detail can move unannounced. */ |
| 87 | readonly quiet: boolean; | 98 | readonly inFlight: boolean; |
| 99 | /** The last activity read listed every remark; an incomplete one is read again. */ | ||
| 100 | readonly remarksComplete: boolean; | ||
| 88 | /** The watches this read evaluated; a watch started since takes its first look next pass. */ | 101 | /** The watches this read evaluated; a watch started since takes its first look next pass. */ |
| 89 | readonly watches: ReadonlySet<string>; | 102 | readonly watches: ReadonlySet<string>; |
| 103 | /** The host fingerprint parts these reads answer, when the host gives one. */ | ||
| 104 | readonly status: string | null; | ||
| 105 | readonly remarks: string | null; | ||
| 106 | } | ||
| 107 | |||
| 108 | /** Which reads a pass makes for one pull request. */ | ||
| 109 | interface ReadPlan { | ||
| 110 | readonly detail: boolean; | ||
| 111 | readonly activity: boolean; | ||
| 90 | } | 112 | } |
| 91 | 113 | ||
| 92 | const watchKey = ({ thread, watch }: WatchTarget) => `${thread.id} ${watch.startedAt}`; | 114 | const watchKey = ({ thread, watch }: WatchTarget) => `${thread.id} ${watch.startedAt}`; |
| ... | @@ -143,14 +165,35 @@ const snapshotFingerprint = ({ link }: WatchTarget) => { | ... | @@ -143,14 +165,35 @@ const snapshotFingerprint = ({ link }: WatchTarget) => { |
| 143 | ].join(" "); | 165 | ].join(" "); |
| 144 | }; | 166 | }; |
| 145 | 167 | ||
| 146 | function needsRead(group: WatchGroup, last: LastRead | undefined, now: number): boolean { | 168 | /** |
| 147 | return ( | 169 | * With a host fingerprint, the detail (1 point on GitHub) is read when anything moved or a check |
| 148 | last === undefined || | 170 | * is still running, since check counts by state cannot tell which check finished, and the |
| 149 | !last.quiet || | 171 | * activity (15 points) only when the remarks moved. Without one, both are read whenever the sync |
| 150 | last.fingerprint !== group.fingerprint || | 172 | * snapshot moved or something is in flight. |
| 151 | now - last.at >= QUIET_REREAD_MS || | 173 | */ |
| 152 | group.targets.some((target) => !last.watches.has(watchKey(target))) | 174 | function planRead( |
| 153 | ); | 175 | group: WatchGroup, |
| 176 | last: LastRead | undefined, | ||
| 177 | fingerprint: ProviderChangeRequestWatchFingerprint | null, | ||
| 178 | now: number, | ||
| 179 | ): ReadPlan { | ||
| 180 | // A watch takes its first look with everything. | ||
| 181 | if (last === undefined || group.targets.some((target) => !last.watches.has(watchKey(target)))) { | ||
| 182 | return { detail: true, activity: true }; | ||
| 183 | } | ||
| 184 | if (fingerprint === null) { | ||
| 185 | const read = | ||
| 186 | last.inFlight || | ||
| 187 | !last.remarksComplete || | ||
| 188 | last.fingerprint !== group.fingerprint || | ||
| 189 | now - last.at >= QUIET_REREAD_MS; | ||
| 190 | return { detail: read, activity: read }; | ||
| 191 | } | ||
| 192 | const activity = | ||
| 193 | last.remarks !== fingerprint.remarks || | ||
| 194 | !last.remarksComplete || | ||
| 195 | now - last.at >= FINGERPRINT_REREAD_MS; | ||
| 196 | return { detail: activity || last.status !== fingerprint.status || last.inFlight, activity }; | ||
| 154 | } | 197 | } |
| 155 | 198 | ||
| 156 | function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatch): boolean { | 199 | function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatch): boolean { |
| ... | @@ -170,9 +213,10 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc | ... | @@ -170,9 +213,10 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc |
| 170 | /** | 213 | /** |
| 171 | * Wakes a thread's agent when a pull request it watches (`watch_pull_request`) needs a look: | 214 | * Wakes a thread's agent when a pull request it watches (`watch_pull_request`) needs a look: |
| 172 | * checks finished on the head commit, someone else commented, or the branch started to | 215 | * checks finished on the head commit, someone else commented, or the branch started to |
| 173 | * conflict. A pass every two minutes reads each watched pull request once for all the threads | 216 | * conflict. A pass every two minutes looks at each watched pull request once for all the threads |
| 174 | * of a project that watch it, and skips one whose sync snapshot has not moved while nothing is | 217 | * of a project that watch it. Where the host has a fingerprint, one batched read of those tells |
| 175 | * in flight. | 218 | * the pass which pull requests moved, and only those are read; elsewhere a pull request is read |
| 219 | * unless its sync snapshot has not moved while nothing is in flight. | ||
| 176 | * Settling or archiving a thread ends its watches, and a merged or closed pull request ends | 220 | * Settling or archiving a thread ends its watches, and a merged or closed pull request ends |
| 177 | * its watch. Each ended watch is logged once with why it ended and how long it went without | 221 | * its watch. Each ended watch is logged once with why it ended and how long it went without |
| 178 | * a push, for debugging watches that live too long. | 222 | * a push, for debugging watches that live too long. |
| ... | @@ -396,16 +440,53 @@ export const make = Effect.gen(function* () { | ... | @@ -396,16 +440,53 @@ export const make = Effect.gen(function* () { |
| 396 | ), | 440 | ), |
| 397 | ).pipe(Effect.map((landed) => landed.every(Boolean))); | 441 | ).pipe(Effect.map((landed) => landed.every(Boolean))); |
| 398 | 442 | ||
| 399 | const readGroup = Effect.fn("PullRequestWatchReactor.readGroup")(function* (group: WatchGroup) { | 443 | const referenceOf = (group: WatchGroup) => { |
| 400 | const now = yield* Clock.currentTimeMillis; | ||
| 401 | if (!needsRead(group, lastReads.get(group.key), now)) return; | ||
| 402 | const first = group.targets[0]!; | 444 | const first = group.targets[0]!; |
| 403 | const reference = { projectId: first.thread.projectId, ...identityOf(first.link) }; | 445 | return { projectId: first.thread.projectId, ...identityOf(first.link) }; |
| 446 | }; | ||
| 447 | |||
| 448 | /** | ||
| 449 | * The host fingerprint of a watched pull request, read for every group at once so they share | ||
| 450 | * one batched request. Null, for a host without one or one that gave no answer, keeps the | ||
| 451 | * sync-snapshot gating; "paused" skips the pass while the host is rate limited, since every | ||
| 452 | * read would be refused. | ||
| 453 | */ | ||
| 454 | const fingerprintOf = (group: WatchGroup) => | ||
| 455 | pullRequests.watchFingerprint(referenceOf(group)).pipe( | ||
| 456 | Effect.catchCause( | ||
| 457 | (cause): Effect.Effect<ProviderChangeRequestWatchFingerprint | null | "paused"> => | ||
| 458 | Cause.hasInterruptsOnly(cause) | ||
| 459 | ? Effect.interrupt | ||
| 460 | : isRateLimited(cause) | ||
| 461 | ? Effect.succeed("paused") | ||
| 462 | : // The snapshot-gated reads report a host that cannot be read, and count toward | ||
| 463 | // giving up. | ||
| 464 | Effect.logDebug("pull request watch fingerprint failed", { | ||
| 465 | pullRequest: group.key, | ||
| 466 | cause, | ||
| 467 | }).pipe(Effect.as(null)), | ||
| 468 | ), | ||
| 469 | ); | ||
| 470 | |||
| 471 | const readGroup = Effect.fn("PullRequestWatchReactor.readGroup")(function* ( | ||
| 472 | group: WatchGroup, | ||
| 473 | fingerprint: ProviderChangeRequestWatchFingerprint | null, | ||
| 474 | ) { | ||
| 475 | const now = yield* Clock.currentTimeMillis; | ||
| 476 | const last = lastReads.get(group.key); | ||
| 477 | const plan = planRead(group, last, fingerprint, now); | ||
| 478 | if (!plan.detail) return; | ||
| 479 | const reference = referenceOf(group); | ||
| 480 | // The reads are recorded against this fingerprint, so a cached answer older than it must not | ||
| 481 | // stand in for them. Clients are not told: this only drops this pull request's cached reads. | ||
| 482 | if (fingerprint !== null) yield* pullRequests.invalidate({ reference }); | ||
| 404 | const read = yield* Effect.exit( | 483 | const read = yield* Effect.exit( |
| 405 | Effect.all( | 484 | Effect.all( |
| 406 | [ | 485 | [ |
| 407 | pullRequests.detail({ ...reference, allowStale: false }), | 486 | pullRequests.detail({ ...reference, allowStale: false }), |
| 408 | pullRequests.activity(reference), | 487 | plan.activity |
| 488 | ? pullRequests.activity(reference).pipe(Effect.map(Option.some)) | ||
| 489 | : Effect.succeed(Option.none<PullRequestActivity>()), | ||
| 409 | ], | 490 | ], |
| 410 | { concurrency: 2 }, | 491 | { concurrency: 2 }, |
| 411 | ), | 492 | ), |
| ... | @@ -450,16 +531,21 @@ export const make = Effect.gen(function* () { | ... | @@ -450,16 +531,21 @@ export const make = Effect.gen(function* () { |
| 450 | } | 531 | } |
| 451 | 532 | ||
| 452 | // Never advance the remark watermark past comments an incomplete read could have missed. | 533 | // Never advance the remark watermark past comments an incomplete read could have missed. |
| 453 | const remarks = yield* readRemarks(group.key, reference, activity); | 534 | // A pass that did not read the activity has no remarks to report. |
| 535 | const remarks = Option.isSome(activity) | ||
| 536 | ? yield* readRemarks(group.key, reference, activity.value) | ||
| 537 | : null; | ||
| 454 | lastReads.set(group.key, { | 538 | lastReads.set(group.key, { |
| 455 | at: now, | 539 | at: plan.activity || last === undefined ? now : last.at, |
| 456 | fingerprint: group.fingerprint, | 540 | fingerprint: group.fingerprint, |
| 541 | inFlight: | ||
| 542 | detail.mergeability === "unknown" || | ||
| 543 | detail.checks.some((check) => check.status === "pending"), | ||
| 457 | // Comments a partial read could not see are read again next pass. | 544 | // Comments a partial read could not see are read again next pass. |
| 458 | quiet: | 545 | remarksComplete: plan.activity ? remarks !== null : (last?.remarksComplete ?? false), |
| 459 | remarks !== null && | ||
| 460 | detail.mergeability !== "unknown" && | ||
| 461 | detail.checks.every((check) => check.status !== "pending"), | ||
| 462 | watches: new Set(group.targets.map(watchKey)), | 546 | watches: new Set(group.targets.map(watchKey)), |
| 547 | status: fingerprint?.status ?? null, | ||
| 548 | remarks: fingerprint?.remarks ?? null, | ||
| 463 | }); | 549 | }); |
| 464 | yield* eachTarget(group, (target) => { | 550 | yield* eachTarget(group, (target) => { |
| 465 | const report = evaluatePullRequestWatch(target.watch, detail, remarks); | 551 | const report = evaluatePullRequestWatch(target.watch, detail, remarks); |
| ... | @@ -533,14 +619,21 @@ export const make = Effect.gen(function* () { | ... | @@ -533,14 +619,21 @@ export const make = Effect.gen(function* () { |
| 533 | ), | 619 | ), |
| 534 | { discard: true }, | 620 | { discard: true }, |
| 535 | ); | 621 | ); |
| 622 | const fingerprints = yield* Effect.forEach(groups, fingerprintOf, { | ||
| 623 | concurrency: "unbounded", | ||
| 624 | }); | ||
| 536 | yield* Effect.forEach( | 625 | yield* Effect.forEach( |
| 537 | groups, | 626 | groups, |
| 538 | (group) => | 627 | (group, index) => { |
| 539 | readGroup(group).pipe( | 628 | const fingerprint = fingerprints[index]!; |
| 540 | Effect.catchCause( | 629 | return fingerprint === "paused" |
| 541 | logFailure("pull request watch check failed", { pullRequest: group.key }), | 630 | ? Effect.void |
| 542 | ), | 631 | : readGroup(group, fingerprint).pipe( |
| 543 | ), | 632 | Effect.catchCause( |
| 633 | logFailure("pull request watch check failed", { pullRequest: group.key }), | ||
| 634 | ), | ||
| 635 | ); | ||
| 636 | }, | ||
| 544 | { concurrency: 4, discard: true }, | 637 | { concurrency: 4, discard: true }, |
| 545 | ); | 638 | ); |
| 546 | }).pipe( | 639 | }).pipe( |
apps/server/src/orchestration-v2/runtimeLayer.test.ts+173| ... | @@ -2767,6 +2767,179 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { | ... | @@ -2767,6 +2767,179 @@ it.layer(TestLayer)("OrchestrationV2LayerLive lifecycle", (it) => { |
| 2767 | }), | 2767 | }), |
| 2768 | ); | 2768 | ); |
| 2769 | 2769 | ||
| 2770 | it.effect("with a host fingerprint, a watch reads only what moved", () => | ||
| 2771 | Effect.gen(function* () { | ||
| 2772 | const orchestrator = yield* Orchestrator.OrchestratorV2; | ||
| 2773 | const projectId = ProjectId.make("pr-watch-fingerprint-project"); | ||
| 2774 | const threadId = ThreadId.make("runtime-pull-request-watch-fingerprint"); | ||
| 2775 | yield* seedProject({ | ||
| 2776 | projectId, | ||
| 2777 | title: "Watch fingerprint", | ||
| 2778 | workspaceRoot: "/workspace/watch-fingerprint", | ||
| 2779 | defaultModelSelection: null, | ||
| 2780 | createdAt: "2026-10-01T00:00:00.000Z", | ||
| 2781 | }); | ||
| 2782 | yield* orchestrator.dispatch({ | ||
| 2783 | type: "thread.create", | ||
| 2784 | createdBy: "user", | ||
| 2785 | creationSource: "web", | ||
| 2786 | commandId: CommandId.make("pr-watch-fingerprint-create"), | ||
| 2787 | threadId, | ||
| 2788 | projectId, | ||
| 2789 | title: "Watch fingerprint", | ||
| 2790 | modelSelection, | ||
| 2791 | runtimeMode: "full-access", | ||
| 2792 | interactionMode: "default", | ||
| 2793 | branch: null, | ||
| 2794 | worktreePath: null, | ||
| 2795 | }); | ||
| 2796 | yield* orchestrator.dispatch({ | ||
| 2797 | type: "thread.pull-request.watch", | ||
| 2798 | commandId: CommandId.make("pr-watch-fingerprint-start"), | ||
| 2799 | threadId, | ||
| 2800 | host: "github.com", | ||
| 2801 | repository: "pingdotgg/t3code", | ||
| 2802 | number: 11, | ||
| 2803 | watching: true, | ||
| 2804 | link: { url: "https://github.com/pingdotgg/t3code/pull/11", source: "agent" }, | ||
| 2805 | }); | ||
| 2806 | |||
| 2807 | let fingerprint: { status: string; remarks: string } | "rate-limited" = { | ||
| 2808 | status: "OPEN pending", | ||
| 2809 | remarks: "0", | ||
| 2810 | }; | ||
| 2811 | type CheckStatus = "pending" | "success" | "failure"; | ||
| 2812 | // What the host says now, and what a read answers: a read right after a cache fill can | ||
| 2813 | // still answer from before a change until the cache is invalidated. | ||
| 2814 | let checks: { lint: CheckStatus; test: CheckStatus } = { lint: "pending", test: "pending" }; | ||
| 2815 | let cachedChecks: typeof checks | null = null; | ||
| 2816 | let comments: Array<PullRequestComment> = []; | ||
| 2817 | let detailReads = 0; | ||
| 2818 | let activityReads = 0; | ||
| 2819 | const reactor = yield* PullRequestWatchReactor.make.pipe( | ||
| 2820 | Effect.provide( | ||
| 2821 | Layer.mergeAll( | ||
| 2822 | NodeServices.layer, | ||
| 2823 | Layer.mock(PullRequestService.PullRequestService)({ | ||
| 2824 | watchFingerprint: () => | ||
| 2825 | Effect.suspend(() => | ||
| 2826 | fingerprint === "rate-limited" | ||
| 2827 | ? Effect.fail( | ||
| 2828 | new PullRequestOperationError({ | ||
| 2829 | operation: "watchFingerprint", | ||
| 2830 | detail: "paused", | ||
| 2831 | cause: new PullRequestProviderError({ | ||
| 2832 | provider: "github", | ||
| 2833 | operation: "getChangeRequestWatchFingerprint", | ||
| 2834 | reason: "rate-limited", | ||
| 2835 | detail: "paused", | ||
| 2836 | }), | ||
| 2837 | }), | ||
| 2838 | ) | ||
| 2839 | : Effect.succeed(fingerprint), | ||
| 2840 | ), | ||
| 2841 | invalidate: () => | ||
| 2842 | Effect.sync(() => { | ||
| 2843 | cachedChecks = null; | ||
| 2844 | }), | ||
| 2845 | detail: () => | ||
| 2846 | Effect.sync(() => { | ||
| 2847 | detailReads += 1; | ||
| 2848 | const answer = cachedChecks ?? checks; | ||
| 2849 | cachedChecks = null; | ||
| 2850 | return { | ||
| 2851 | ...watchedPullRequestDetail({ | ||
| 2852 | projectId, | ||
| 2853 | number: 11, | ||
| 2854 | at: "2026-10-02T12:00:00.000Z", | ||
| 2855 | }), | ||
| 2856 | checks: Object.entries(answer).map(([name, status]) => ({ | ||
| 2857 | name, | ||
| 2858 | status, | ||
| 2859 | description: null, | ||
| 2860 | url: null, | ||
| 2861 | })), | ||
| 2862 | }; | ||
| 2863 | }), | ||
| 2864 | activity: () => | ||
| 2865 | Effect.sync(() => { | ||
| 2866 | activityReads += 1; | ||
| 2867 | return { | ||
| 2868 | comments, | ||
| 2869 | commentCount: comments.length, | ||
| 2870 | commentsTruncated: false, | ||
| 2871 | reviewThreads: [], | ||
| 2872 | commits: [], | ||
| 2873 | }; | ||
| 2874 | }), | ||
| 2875 | }), | ||
| 2876 | ), | ||
| 2877 | ), | ||
| 2878 | ); | ||
| 2879 | const reads = () => ({ detail: detailReads, activity: activityReads }); | ||
| 2880 | const summaries = Effect.map( | ||
| 2881 | orchestrator.getThreadRecords(threadId, ["messages"]), | ||
| 2882 | ({ messages }) => messages.flatMap((message) => message.notification?.summary ?? []), | ||
| 2883 | ); | ||
| 2884 | |||
| 2885 | // The first look reads everything. | ||
| 2886 | yield* reactor.sweep; | ||
| 2887 | assert.deepEqual(reads(), { detail: 1, activity: 1 }); | ||
| 2888 | |||
| 2889 | // While checks run, the cheap detail is read every pass: check counts by state cannot | ||
| 2890 | // tell which check finished, so a failure can arrive with the fingerprint unchanged. | ||
| 2891 | checks = { lint: "failure", test: "pending" }; | ||
| 2892 | yield* reactor.sweep; | ||
| 2893 | assert.deepEqual(reads(), { detail: 2, activity: 1 }); | ||
| 2894 | assert.deepEqual(yield* summaries, ["#11: checks failed"]); | ||
| 2895 | |||
| 2896 | // When the fingerprint moves, a cached answer from before the move is not trusted. | ||
| 2897 | yield* TestClock.adjust("1 millis"); | ||
| 2898 | cachedChecks = checks; | ||
| 2899 | checks = { lint: "failure", test: "success" }; | ||
| 2900 | fingerprint = { status: "OPEN settled", remarks: "0" }; | ||
| 2901 | yield* reactor.sweep; | ||
| 2902 | assert.deepEqual(reads(), { detail: 3, activity: 1 }); | ||
| 2903 | |||
| 2904 | // Nothing in flight and nothing moved, so the passes read nothing. | ||
| 2905 | for (let pass = 0; pass < 5; pass += 1) yield* reactor.sweep; | ||
| 2906 | assert.deepEqual(reads(), { detail: 3, activity: 1 }); | ||
| 2907 | |||
| 2908 | // A new comment moves the remarks, which take the activity read too. | ||
| 2909 | yield* TestClock.adjust("1 millis"); | ||
| 2910 | comments = [ | ||
| 2911 | { | ||
| 2912 | id: "comment-1", | ||
| 2913 | kind: "issue-comment", | ||
| 2914 | author: { login: "reviewer", name: null, avatarUrl: null }, | ||
| 2915 | body: "Please rename this.", | ||
| 2916 | createdAt: "2999-01-01T00:00:00.000Z", | ||
| 2917 | url: null, | ||
| 2918 | path: null, | ||
| 2919 | reviewState: null, | ||
| 2920 | }, | ||
| 2921 | ]; | ||
| 2922 | fingerprint = { status: "OPEN settled", remarks: "1" }; | ||
| 2923 | yield* reactor.sweep; | ||
| 2924 | assert.deepEqual(reads(), { detail: 4, activity: 2 }); | ||
| 2925 | assert.deepEqual(yield* summaries, ["#11: checks failed", "#11: new comments"]); | ||
| 2926 | |||
| 2927 | // While the host is rate limited, a pass reads nothing and the watch stays on. | ||
| 2928 | fingerprint = "rate-limited"; | ||
| 2929 | yield* reactor.sweep; | ||
| 2930 | assert.deepEqual(reads(), { detail: 4, activity: 2 }); | ||
| 2931 | assert.isDefined((yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch); | ||
| 2932 | |||
| 2933 | // Edits inside review threads leave no trace in the fingerprint, so the activity is read | ||
| 2934 | // again after half an hour regardless. | ||
| 2935 | fingerprint = { status: "OPEN settled", remarks: "1" }; | ||
| 2936 | yield* TestClock.adjust("30 minutes"); | ||
| 2937 | yield* reactor.sweep; | ||
| 2938 | assert.deepEqual(reads(), { detail: 5, activity: 3 }); | ||
| 2939 | assert.deepEqual(yield* summaries, ["#11: checks failed", "#11: new comments"]); | ||
| 2940 | }), | ||
| 2941 | ); | ||
| 2942 | |||
| 2770 | it.effect.each([ | 2943 | it.effect.each([ |
| 2771 | "single page", | 2944 | "single page", |
| 2772 | "paginated", | 2945 | "paginated", |
apps/server/src/pullRequest/GitHubPullRequestCli.test.ts+47| ... | @@ -532,6 +532,53 @@ layer("GitHubPullRequestCli.layer", (it) => { | ... | @@ -532,6 +532,53 @@ layer("GitHubPullRequestCli.layer", (it) => { |
| 532 | }), | 532 | }), |
| 533 | ); | 533 | ); |
| 534 | 534 | ||
| 535 | it.effect("fingerprints watched pull requests on one host in one read", () => | ||
| 536 | Effect.gen(function* () { | ||
| 537 | const node = (comments: number) => ({ | ||
| 538 | state: "OPEN", | ||
| 539 | mergeable: "MERGEABLE", | ||
| 540 | headRefOid: "abc123", | ||
| 541 | comments: { totalCount: comments, nodes: [] }, | ||
| 542 | reviews: { totalCount: 0, nodes: [] }, | ||
| 543 | reviewThreads: { totalCount: 0 }, | ||
| 544 | commits: { nodes: [{ commit: { statusCheckRollup: null } }] }, | ||
| 545 | }); | ||
| 546 | mockedExecute.mockReturnValueOnce( | ||
| 547 | Effect.succeed( | ||
| 548 | output( | ||
| 549 | // @effect-diagnostics-next-line preferSchemaOverJson:off | ||
| 550 | JSON.stringify({ | ||
| 551 | data: { w0: { pullRequest: node(1) }, w1: { pullRequest: null } }, | ||
| 552 | }), | ||
| 553 | ), | ||
| 554 | ), | ||
| 555 | ); | ||
| 556 | const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli; | ||
| 557 | |||
| 558 | const reads = yield* Effect.forEach( | ||
| 559 | [7, 8], | ||
| 560 | (number) => | ||
| 561 | cli.getPullRequestWatchFingerprint({ | ||
| 562 | cwd: "/w", | ||
| 563 | repository: "acme/web", | ||
| 564 | host: "github.com", | ||
| 565 | number, | ||
| 566 | }), | ||
| 567 | { concurrency: "unbounded" }, | ||
| 568 | ).pipe(Effect.forkChild); | ||
| 569 | yield* TestClock.adjust("10 millis"); | ||
| 570 | const [seven, eight] = yield* Fiber.join(reads); | ||
| 571 | |||
| 572 | expect(seven?.remarks.startsWith("1 ")).toBe(true); | ||
| 573 | // GitHub had no answer for #8, so its watch reads it in full. | ||
| 574 | expect(eight).toBeNull(); | ||
| 575 | expect(mockedExecute).toHaveBeenCalledOnce(); | ||
| 576 | expect(callAt(0).args.at(-1) ?? "").toContain( | ||
| 577 | 'w1: repository(owner: "acme", name: "web") { pullRequest(number: 8)', | ||
| 578 | ); | ||
| 579 | }), | ||
| 580 | ); | ||
| 581 | |||
| 535 | it.effect("reads a pull request the batch said nothing about on its own", () => | 582 | it.effect("reads a pull request the batch said nothing about on its own", () => |
| 536 | Effect.gen(function* () { | 583 | Effect.gen(function* () { |
| 537 | mockedExecute | 584 | mockedExecute |
apps/server/src/pullRequest/GitHubPullRequestCli.ts+79| ... | @@ -56,6 +56,7 @@ import { | ... | @@ -56,6 +56,7 @@ import { |
| 56 | pullRequestCoreGraphQlQuery, | 56 | pullRequestCoreGraphQlQuery, |
| 57 | type GitHubPullRequestCore, | 57 | type GitHubPullRequestCore, |
| 58 | type GitHubPullRequestSummary, | 58 | type GitHubPullRequestSummary, |
| 59 | type GitHubPullRequestWatchFingerprint, | ||
| 59 | decodePullRequestPreviewJson, | 60 | decodePullRequestPreviewJson, |
| 60 | PULL_REQUEST_PREVIEW_GRAPHQL_QUERY, | 61 | PULL_REQUEST_PREVIEW_GRAPHQL_QUERY, |
| 61 | decodePullRequestFilesJson, | 62 | decodePullRequestFilesJson, |
| ... | @@ -67,6 +68,7 @@ import { | ... | @@ -67,6 +68,7 @@ import { |
| 67 | decodePullRequestStacksJson, | 68 | decodePullRequestStacksJson, |
| 68 | decodePullRequestStatsJson, | 69 | decodePullRequestStatsJson, |
| 69 | decodePullRequestSummariesJson, | 70 | decodePullRequestSummariesJson, |
| 71 | decodePullRequestWatchFingerprintsJson, | ||
| 70 | decodeReactionSubjectScopeJson, | 72 | decodeReactionSubjectScopeJson, |
| 71 | decodeReviewerCandidatesJson, | 73 | decodeReviewerCandidatesJson, |
| 72 | decodeLabelCandidatesJson, | 74 | decodeLabelCandidatesJson, |
| ... | @@ -77,6 +79,7 @@ import { | ... | @@ -77,6 +79,7 @@ import { |
| 77 | decodeReviewThreadsJson, | 79 | decodeReviewThreadsJson, |
| 78 | buildPullRequestStatsGraphQlQuery, | 80 | buildPullRequestStatsGraphQlQuery, |
| 79 | buildPullRequestSummariesGraphQlQuery, | 81 | buildPullRequestSummariesGraphQlQuery, |
| 82 | buildPullRequestWatchFingerprintsGraphQlQuery, | ||
| 80 | buildPullRequestStackMembershipsGraphQlQuery, | 83 | buildPullRequestStackMembershipsGraphQlQuery, |
| 81 | decodePullRequestStackMembershipsJson, | 84 | decodePullRequestStackMembershipsJson, |
| 82 | encodeGraphQlRequestJson, | 85 | encodeGraphQlRequestJson, |
| ... | @@ -447,6 +450,17 @@ class PullRequestSummaryRead extends Request.Class< | ... | @@ -447,6 +450,17 @@ class PullRequestSummaryRead extends Request.Class< |
| 447 | GitHubPullRequestCliError | 450 | GitHubPullRequestCliError |
| 448 | > {} | 451 | > {} |
| 449 | 452 | ||
| 453 | class PullRequestWatchFingerprintRead extends Request.Class< | ||
| 454 | { | ||
| 455 | readonly cwd: string; | ||
| 456 | readonly repository: string; | ||
| 457 | readonly host: string; | ||
| 458 | readonly number: number; | ||
| 459 | }, | ||
| 460 | GitHubPullRequestWatchFingerprint | null, | ||
| 461 | GitHubPullRequestCliError | ||
| 462 | > {} | ||
| 463 | |||
| 450 | export interface GitHubPullRequestSearchBatch { | 464 | export interface GitHubPullRequestSearchBatch { |
| 451 | /** Rows across every repository asked for, newest update first, each naming its own. */ | 465 | /** Rows across every repository asked for, newest update first, each naming its own. */ |
| 452 | readonly items: ReadonlyArray<GitHubPullRequestSearchItem>; | 466 | readonly items: ReadonlyArray<GitHubPullRequestSearchItem>; |
| ... | @@ -543,6 +557,16 @@ export class GitHubPullRequestCli extends Context.Service< | ... | @@ -543,6 +557,16 @@ export class GitHubPullRequestCli extends Context.Service< |
| 543 | readonly host: string; | 557 | readonly host: string; |
| 544 | readonly number: number; | 558 | readonly number: number; |
| 545 | }) => Effect.Effect<ProviderChangeRequestSummary, GitHubPullRequestCliError>; | 559 | }) => Effect.Effect<ProviderChangeRequestSummary, GitHubPullRequestCliError>; |
| 560 | /** | ||
| 561 | * What a watch compares between passes, batched like summaries. Null when GitHub gave no | ||
| 562 | * answer for this pull request, so the watch reads it in full instead. | ||
| 563 | */ | ||
| 564 | readonly getPullRequestWatchFingerprint: (input: { | ||
| 565 | readonly cwd: string; | ||
| 566 | readonly repository: string; | ||
| 567 | readonly host: string; | ||
| 568 | readonly number: number; | ||
| 569 | }) => Effect.Effect<GitHubPullRequestWatchFingerprint | null, GitHubPullRequestCliError>; | ||
| 546 | 570 | ||
| 547 | readonly revalidateChecks: Effect.Success<typeof makeChecksRevalidator>; | 571 | readonly revalidateChecks: Effect.Success<typeof makeChecksRevalidator>; |
| 548 | 572 | ||
| ... | @@ -1918,6 +1942,60 @@ export const make = Effect.gen(function* () { | ... | @@ -1918,6 +1942,60 @@ export const make = Effect.gen(function* () { |
| 1918 | const getPullRequestSummary: GitHubPullRequestCli["Service"]["getPullRequestSummary"] = (input) => | 1942 | const getPullRequestSummary: GitHubPullRequestCli["Service"]["getPullRequestSummary"] = (input) => |
| 1919 | Effect.request(new PullRequestSummaryRead(input), summaryResolver); | 1943 | Effect.request(new PullRequestSummaryRead(input), summaryResolver); |
| 1920 | 1944 | ||
| 1945 | // Every watched pull request on a host in one read per pass. A pull request the batch has no | ||
| 1946 | // answer for gets null, and its watch reads it in full; a failed batch fails every entry. | ||
| 1947 | const watchFingerprintResolver = RequestResolver.makeGrouped< | ||
| 1948 | PullRequestWatchFingerprintRead, | ||
| 1949 | string | ||
| 1950 | >({ | ||
| 1951 | key: ({ request, context }) => | ||
| 1952 | JSON.stringify([ | ||
| 1953 | request.host.toLowerCase(), | ||
| 1954 | Context.getOrElse(context, GitHubCli.PinnedGitHubCredential, () => null) | ||
| 1955 | ?.credentialFingerprint ?? null, | ||
| 1956 | Context.getOrElse(context, SourceControlRateLimit.CredentialScope, () => ""), | ||
| 1957 | ]), | ||
| 1958 | resolver: (entries) => { | ||
| 1959 | const [first] = entries; | ||
| 1960 | const batchable = entries.filter( | ||
| 1961 | (entry) => buildPullRequestWatchFingerprintsGraphQlQuery([entry.request]) !== null, | ||
| 1962 | ); | ||
| 1963 | const query = buildPullRequestWatchFingerprintsGraphQlQuery( | ||
| 1964 | batchable.map((entry) => entry.request), | ||
| 1965 | ); | ||
| 1966 | const read = | ||
| 1967 | query === null | ||
| 1968 | ? Effect.succeed(new Map<number, GitHubPullRequestWatchFingerprint>()) | ||
| 1969 | : graphqlRead({ | ||
| 1970 | cwd: first.request.cwd, | ||
| 1971 | host: first.request.host, | ||
| 1972 | operation: "getPullRequestWatchFingerprint", | ||
| 1973 | query, | ||
| 1974 | decode: decodePullRequestWatchFingerprintsJson, | ||
| 1975 | }); | ||
| 1976 | return read.pipe( | ||
| 1977 | Effect.map((fingerprints) => { | ||
| 1978 | for (const entry of entries) { | ||
| 1979 | const index = batchable.indexOf(entry); | ||
| 1980 | entry.completeUnsafe( | ||
| 1981 | Exit.succeed(index === -1 ? null : (fingerprints.get(index) ?? null)), | ||
| 1982 | ); | ||
| 1983 | } | ||
| 1984 | }), | ||
| 1985 | Effect.catchCause((cause) => | ||
| 1986 | Effect.sync(() => { | ||
| 1987 | for (const entry of entries) entry.completeUnsafe(Exit.failCause(cause)); | ||
| 1988 | }), | ||
| 1989 | ), | ||
| 1990 | ); | ||
| 1991 | }, | ||
| 1992 | }).pipe( | ||
| 1993 | RequestResolver.setDelay(SUMMARY_BATCH_WINDOW), | ||
| 1994 | RequestResolver.batchN(STAT_ALIASES_PER_REQUEST), | ||
| 1995 | ); | ||
| 1996 | const getPullRequestWatchFingerprint: GitHubPullRequestCli["Service"]["getPullRequestWatchFingerprint"] = | ||
| 1997 | (input) => Effect.request(new PullRequestWatchFingerprintRead(input), watchFingerprintResolver); | ||
| 1998 | |||
| 1921 | return GitHubPullRequestCli.of({ | 1999 | return GitHubPullRequestCli.of({ |
| 1922 | withVerifiedCredential, | 2000 | withVerifiedCredential, |
| 1923 | revalidateChecks, | 2001 | revalidateChecks, |
| ... | @@ -2125,6 +2203,7 @@ export const make = Effect.gen(function* () { | ... | @@ -2125,6 +2203,7 @@ export const make = Effect.gen(function* () { |
| 2125 | }, | 2203 | }, |
| 2126 | 2204 | ||
| 2127 | getPullRequestSummary, | 2205 | getPullRequestSummary, |
| 2206 | getPullRequestWatchFingerprint, | ||
| 2128 | 2207 | ||
| 2129 | getPullRequestDetail, | 2208 | getPullRequestDetail, |
| 2130 | getPullRequestPreview: (input) => { | 2209 | getPullRequestPreview: (input) => { |
apps/server/src/pullRequest/GitHubPullRequestProvider.ts+5| ... | @@ -360,6 +360,11 @@ export const make = Effect.gen(function* () { | ... | @@ -360,6 +360,11 @@ export const make = Effect.gen(function* () { |
| 360 | getChangeRequestStack: (input) => | 360 | getChangeRequestStack: (input) => |
| 361 | cli.getPullRequestStack(input).pipe(Effect.mapError(fail("getChangeRequestStack"))), | 361 | cli.getPullRequestStack(input).pipe(Effect.mapError(fail("getChangeRequestStack"))), |
| 362 | 362 | ||
| 363 | getChangeRequestWatchFingerprint: (input) => | ||
| 364 | cli | ||
| 365 | .getPullRequestWatchFingerprint(input) | ||
| 366 | .pipe(Effect.mapError(fail("getChangeRequestWatchFingerprint"))), | ||
| 367 | |||
| 363 | getChangeRequestPreview: (input) => | 368 | getChangeRequestPreview: (input) => |
| 364 | cli.getPullRequestPreview(input).pipe(Effect.mapError(fail("getChangeRequestPreview"))), | 369 | cli.getPullRequestPreview(input).pipe(Effect.mapError(fail("getChangeRequestPreview"))), |
| 365 | 370 |
apps/server/src/pullRequest/PullRequestProvider.ts+18| ... | @@ -69,6 +69,15 @@ export class PullRequestProviderError extends Schema.TaggedError<PullRequestProv | ... | @@ -69,6 +69,15 @@ export class PullRequestProviderError extends Schema.TaggedError<PullRequestProv |
| 69 | } | 69 | } |
| 70 | } | 70 | } |
| 71 | 71 | ||
| 72 | /** | ||
| 73 | * `status` moves with the state, mergeability, head, or checks, which the detail read reports; | ||
| 74 | * `remarks` moves with comments and reviews, which only the activity read reports. | ||
| 75 | */ | ||
| 76 | export interface ProviderChangeRequestWatchFingerprint { | ||
| 77 | readonly status: string; | ||
| 78 | readonly remarks: string; | ||
| 79 | } | ||
| 80 | |||
| 72 | export interface PullRequestProviderFailure { | 81 | export interface PullRequestProviderFailure { |
| 73 | readonly reason: PullRequestProviderError["reason"]; | 82 | readonly reason: PullRequestProviderError["reason"]; |
| 74 | readonly retryAt?: number | undefined; | 83 | readonly retryAt?: number | undefined; |
| ... | @@ -436,6 +445,15 @@ export interface PullRequestProviderApi { | ... | @@ -436,6 +445,15 @@ export interface PullRequestProviderApi { |
| 436 | input: ProviderRepositoryRef & { readonly number: number }, | 445 | input: ProviderRepositoryRef & { readonly number: number }, |
| 437 | ) => Effect.Effect<ProviderChangeRequestSummary, PullRequestProviderError>; | 446 | ) => Effect.Effect<ProviderChangeRequestSummary, PullRequestProviderError>; |
| 438 | 447 | ||
| 448 | /** | ||
| 449 | * A cheap fingerprint of what a pull request watch reports, so it reads the change request in | ||
| 450 | * full only when this moves. Null when the host gave no answer for it. Optional: a host | ||
| 451 | * without one has its watched change requests read in full on every pass. | ||
| 452 | */ | ||
| 453 | readonly getChangeRequestWatchFingerprint?: ( | ||
| 454 | input: ProviderRepositoryRef & { readonly number: number }, | ||
| 455 | ) => Effect.Effect<ProviderChangeRequestWatchFingerprint | null, PullRequestProviderError>; | ||
| 456 | |||
| 439 | /** | 457 | /** |
| 440 | * The host-native stack a change request belongs to, or null when it is not stacked. Optional | 458 | * The host-native stack a change request belongs to, or null when it is not stacked. Optional |
| 441 | * because most hosts have no such object; the service derives chains from base branches there. | 459 | * because most hosts have no such object; the service derives chains from base branches there. |
apps/server/src/pullRequest/PullRequestService.ts+43| ... | @@ -85,6 +85,7 @@ import * as SourceControlRateLimit from "../sourceControl/SourceControlRateLimit | ... | @@ -85,6 +85,7 @@ import * as SourceControlRateLimit from "../sourceControl/SourceControlRateLimit |
| 85 | import { | 85 | import { |
| 86 | type ProviderChangeRequest, | 86 | type ProviderChangeRequest, |
| 87 | type ProviderListCursor, | 87 | type ProviderListCursor, |
| 88 | type ProviderChangeRequestWatchFingerprint, | ||
| 88 | type PullRequestProviderApi, | 89 | type PullRequestProviderApi, |
| 89 | PullRequestProviderError, | 90 | PullRequestProviderError, |
| 90 | } from "./PullRequestProvider.ts"; | 91 | } from "./PullRequestProvider.ts"; |
| ... | @@ -218,6 +219,13 @@ export class PullRequestService extends Context.Service< | ... | @@ -218,6 +219,13 @@ export class PullRequestService extends Context.Service< |
| 218 | readonly checks: ( | 219 | readonly checks: ( |
| 219 | input: PullRequestRef, | 220 | input: PullRequestRef, |
| 220 | ) => Effect.Effect<PullRequestChecks | null, PullRequestError>; | 221 | ) => Effect.Effect<PullRequestChecks | null, PullRequestError>; |
| 222 | /** | ||
| 223 | * What a pull request watch compares between passes, so it reads detail and activity only | ||
| 224 | * when something moved. Null when the host has no fingerprint or gave none for this one. | ||
| 225 | */ | ||
| 226 | readonly watchFingerprint: ( | ||
| 227 | input: PullRequestRef, | ||
| 228 | ) => Effect.Effect<ProviderChangeRequestWatchFingerprint | null, PullRequestError>; | ||
| 221 | readonly activity: ( | 229 | readonly activity: ( |
| 222 | input: PullRequestRef, | 230 | input: PullRequestRef, |
| 223 | ) => Effect.Effect<PullRequestActivity, PullRequestError>; | 231 | ) => Effect.Effect<PullRequestActivity, PullRequestError>; |
| ... | @@ -581,6 +589,14 @@ function withRateLimitBackoff( | ... | @@ -581,6 +589,14 @@ function withRateLimitBackoff( |
| 581 | ...(api.getChangeRequestStack === undefined | 589 | ...(api.getChangeRequestStack === undefined |
| 582 | ? {} | 590 | ? {} |
| 583 | : { getChangeRequestStack: wrap("getChangeRequestStack", api.getChangeRequestStack) }), | 591 | : { getChangeRequestStack: wrap("getChangeRequestStack", api.getChangeRequestStack) }), |
| 592 | ...(api.getChangeRequestWatchFingerprint === undefined | ||
| 593 | ? {} | ||
| 594 | : { | ||
| 595 | getChangeRequestWatchFingerprint: wrap( | ||
| 596 | "getChangeRequestWatchFingerprint", | ||
| 597 | api.getChangeRequestWatchFingerprint, | ||
| 598 | ), | ||
| 599 | }), | ||
| 584 | getChangeRequestActivity: wrap("getChangeRequestActivity", api.getChangeRequestActivity), | 600 | getChangeRequestActivity: wrap("getChangeRequestActivity", api.getChangeRequestActivity), |
| 585 | ...(api.getReviewThreadComments === undefined | 601 | ...(api.getReviewThreadComments === undefined |
| 586 | ? {} | 602 | ? {} |
| ... | @@ -2949,6 +2965,30 @@ export const make = Effect.gen(function* () { | ... | @@ -2949,6 +2965,30 @@ export const make = Effect.gen(function* () { |
| 2949 | }, | 2965 | }, |
| 2950 | ); | 2966 | ); |
| 2951 | 2967 | ||
| 2968 | const watchFingerprintCache = yield* Cache.makeWith( | ||
| 2969 | (key: string) => { | ||
| 2970 | const input = refOfCacheKey(key); | ||
| 2971 | return requireProject(input).pipe( | ||
| 2972 | Effect.flatMap((project) => | ||
| 2973 | project.api.getChangeRequestWatchFingerprint === undefined | ||
| 2974 | ? Effect.succeed(null) | ||
| 2975 | : project.api | ||
| 2976 | .getChangeRequestWatchFingerprint({ | ||
| 2977 | cwd: project.project.workspaceRoot, | ||
| 2978 | repository: project.repository, | ||
| 2979 | host: project.host, | ||
| 2980 | number: input.number, | ||
| 2981 | }) | ||
| 2982 | .pipe(Effect.mapError(toPullRequestError("watchFingerprint"))), | ||
| 2983 | ), | ||
| 2984 | ); | ||
| 2985 | }, | ||
| 2986 | { | ||
| 2987 | capacity: DETAIL_CACHE_CAPACITY, | ||
| 2988 | timeToLive: (exit) => (Exit.isSuccess(exit) ? DETAIL_CACHE_TTL : Duration.zero), | ||
| 2989 | }, | ||
| 2990 | ); | ||
| 2991 | |||
| 2952 | const detailCache = yield* Cache.makeWith( | 2992 | const detailCache = yield* Cache.makeWith( |
| 2953 | (key: string) => { | 2993 | (key: string) => { |
| 2954 | const statsKey = statsCacheKey(key); | 2994 | const statsKey = statsCacheKey(key); |
| ... | @@ -3321,6 +3361,9 @@ export const make = Effect.gen(function* () { | ... | @@ -3321,6 +3361,9 @@ export const make = Effect.gen(function* () { |
| 3321 | refreshAfterTurn, | 3361 | refreshAfterTurn, |
| 3322 | detail: credentialCached(detail), | 3362 | detail: credentialCached(detail), |
| 3323 | checks: credentialCached((input) => Cache.get(checksCache, refCacheKey(input))), | 3363 | checks: credentialCached((input) => Cache.get(checksCache, refCacheKey(input))), |
| 3364 | watchFingerprint: credentialCached((input) => | ||
| 3365 | Cache.get(watchFingerprintCache, refCacheKey(input)), | ||
| 3366 | ), | ||
| 3324 | activity: credentialCached(activity), | 3367 | activity: credentialCached(activity), |
| 3325 | preview: credentialCached(preview), | 3368 | preview: credentialCached(preview), |
| 3326 | threadComments, | 3369 | threadComments, |
apps/server/src/pullRequest/gitHubPullRequestJson.test.ts+80| ... | @@ -3,7 +3,9 @@ import { describe, expect, it } from "vite-plus/test"; | ... | @@ -3,7 +3,9 @@ import { describe, expect, it } from "vite-plus/test"; |
| 3 | 3 | ||
| 4 | import { | 4 | import { |
| 5 | buildPullRequestSummariesGraphQlQuery, | 5 | buildPullRequestSummariesGraphQlQuery, |
| 6 | buildPullRequestWatchFingerprintsGraphQlQuery, | ||
| 6 | decodePullRequestSummariesJson, | 7 | decodePullRequestSummariesJson, |
| 8 | decodePullRequestWatchFingerprintsJson, | ||
| 7 | buildReviewSubmissionJson, | 9 | buildReviewSubmissionJson, |
| 8 | buildPullRequestStackMembershipsGraphQlQuery, | 10 | buildPullRequestStackMembershipsGraphQlQuery, |
| 9 | decodePullRequestStackMembershipsJson, | 11 | decodePullRequestStackMembershipsJson, |
| ... | @@ -2026,3 +2028,81 @@ describe("batched pull request summaries", () => { | ... | @@ -2026,3 +2028,81 @@ describe("batched pull request summaries", () => { |
| 2026 | expect(decoded.get(1)?.stack).toEqual({ number: 3, size: 2, base: "main", position: 2 }); | 2028 | expect(decoded.get(1)?.stack).toEqual({ number: 3, size: 2, base: "main", position: 2 }); |
| 2027 | }); | 2029 | }); |
| 2028 | }); | 2030 | }); |
| 2031 | |||
| 2032 | describe("pull request watch fingerprints", () => { | ||
| 2033 | it("asks for every pull request in one aliased read, and refuses an unsafe selector", () => { | ||
| 2034 | const query = buildPullRequestWatchFingerprintsGraphQlQuery([ | ||
| 2035 | { repository: "pingdotgg/t3code", number: 7 }, | ||
| 2036 | { repository: "pingdotgg/lakebed", number: 8 }, | ||
| 2037 | ]); | ||
| 2038 | expect(query).toContain( | ||
| 2039 | 'w0: repository(owner: "pingdotgg", name: "t3code") { pullRequest(number: 7)', | ||
| 2040 | ); | ||
| 2041 | expect(query).toContain( | ||
| 2042 | 'w1: repository(owner: "pingdotgg", name: "lakebed") { pullRequest(number: 8)', | ||
| 2043 | ); | ||
| 2044 | expect( | ||
| 2045 | buildPullRequestWatchFingerprintsGraphQlQuery([{ repository: 'evil") { x', number: 1 }]), | ||
| 2046 | ).toBeNull(); | ||
| 2047 | }); | ||
| 2048 | |||
| 2049 | it("moves status with checks and remarks with comments, each without the other", () => { | ||
| 2050 | const pullRequest = (input: { | ||
| 2051 | readonly running: number; | ||
| 2052 | readonly failed: number; | ||
| 2053 | readonly commentEditedAt: string | null; | ||
| 2054 | readonly reviews: number; | ||
| 2055 | }) => ({ | ||
| 2056 | state: "OPEN", | ||
| 2057 | mergeable: "MERGEABLE", | ||
| 2058 | headRefOid: "abc123", | ||
| 2059 | comments: { totalCount: 1, nodes: [{ lastEditedAt: input.commentEditedAt }] }, | ||
| 2060 | reviews: { totalCount: input.reviews, nodes: [] }, | ||
| 2061 | reviewThreads: { totalCount: 0 }, | ||
| 2062 | commits: { | ||
| 2063 | nodes: [ | ||
| 2064 | { | ||
| 2065 | commit: { | ||
| 2066 | statusCheckRollup: { | ||
| 2067 | contexts: { | ||
| 2068 | checkRunCountsByState: [ | ||
| 2069 | { state: "IN_PROGRESS", count: input.running }, | ||
| 2070 | { state: "FAILURE", count: input.failed }, | ||
| 2071 | { state: "SUCCESS", count: 0 }, | ||
| 2072 | ], | ||
| 2073 | statusContextCountsByState: null, | ||
| 2074 | }, | ||
| 2075 | }, | ||
| 2076 | }, | ||
| 2077 | }, | ||
| 2078 | ], | ||
| 2079 | }, | ||
| 2080 | }); | ||
| 2081 | const decode = (...pullRequests: ReadonlyArray<object | null>) => { | ||
| 2082 | const decoded = decodePullRequestWatchFingerprintsJson( | ||
| 2083 | JSON.stringify({ | ||
| 2084 | data: Object.fromEntries( | ||
| 2085 | pullRequests.map((value, index) => [`w${index}`, { pullRequest: value }]), | ||
| 2086 | ), | ||
| 2087 | }), | ||
| 2088 | ); | ||
| 2089 | if (!Result.isSuccess(decoded)) throw new Error("fingerprints did not decode"); | ||
| 2090 | return decoded.success; | ||
| 2091 | }; | ||
| 2092 | const base = { running: 2, failed: 0, commentEditedAt: null, reviews: 1 }; | ||
| 2093 | const fingerprint = (input: Parameters<typeof pullRequest>[0]) => | ||
| 2094 | decode(pullRequest(input)).get(0)!; | ||
| 2095 | const before = fingerprint(base); | ||
| 2096 | const checkFinished = fingerprint({ ...base, running: 1, failed: 1 }); | ||
| 2097 | const summaryEdited = fingerprint({ ...base, commentEditedAt: "2026-10-05T23:47:43Z" }); | ||
| 2098 | const replied = fingerprint({ ...base, reviews: 2 }); | ||
| 2099 | |||
| 2100 | expect(checkFinished.status).not.toBe(before.status); | ||
| 2101 | expect(checkFinished.remarks).toBe(before.remarks); | ||
| 2102 | expect(summaryEdited.remarks).not.toBe(before.remarks); | ||
| 2103 | expect(summaryEdited.status).toBe(before.status); | ||
| 2104 | expect(replied.remarks).not.toBe(before.remarks); | ||
| 2105 | // A pull request GitHub had no answer for is left for a full read. | ||
| 2106 | expect(decode(null, pullRequest(base)).has(0)).toBe(false); | ||
| 2107 | }); | ||
| 2108 | }); |
apps/server/src/pullRequest/gitHubPullRequestJson.ts+133| ... | @@ -1993,6 +1993,139 @@ export function decodePullRequestSummariesJson( | ... | @@ -1993,6 +1993,139 @@ export function decodePullRequestSummariesJson( |
| 1993 | return Result.succeed(summaries); | 1993 | return Result.succeed(summaries); |
| 1994 | } | 1994 | } |
| 1995 | 1995 | ||
| 1996 | const WATCH_FINGERPRINT_EDITS = "totalCount nodes { lastEditedAt }"; | ||
| 1997 | const WATCH_FINGERPRINT_CHECK_COUNTS = | ||
| 1998 | "checkRunCountsByState { state count } statusContextCountsByState { state count }"; | ||
| 1999 | |||
| 2000 | /** | ||
| 2001 | * What a pull request watch needs to notice, priced by GitHub at one point for twenty-five pull | ||
| 2002 | * requests, where the detail and activity reads it gates cost sixteen for one. Counts and the | ||
| 2003 | * newest edit cover new comments, reviews (a reply in a review thread is a review), threads, and | ||
| 2004 | * a bot rewriting its summary; check counts by state move whenever a check starts or finishes. | ||
| 2005 | * Comments come most recently updated first, since an edit moves `updatedAt`, so an edit to an | ||
| 2006 | * old comment on a long pull request is still in the page. Reviews have no such order: an edit | ||
| 2007 | * to a review older than the last hundred waits for the watch's periodic full read, as do edits | ||
| 2008 | * to comments inside review threads, which cost a point per pull request to ask for. | ||
| 2009 | */ | ||
| 2010 | const PULL_REQUEST_WATCH_FINGERPRINT_SELECTION = | ||
| 2011 | "state mergeable headRefOid " + | ||
| 2012 | `comments(first: 100, orderBy: { field: UPDATED_AT, direction: DESC }) { ${WATCH_FINGERPRINT_EDITS} } ` + | ||
| 2013 | `reviews(last: 100) { ${WATCH_FINGERPRINT_EDITS} } ` + | ||
| 2014 | "reviewThreads { totalCount } " + | ||
| 2015 | `commits(last: 1) { nodes { commit { statusCheckRollup { contexts { ${WATCH_FINGERPRINT_CHECK_COUNTS} } } } } }`; | ||
| 2016 | |||
| 2017 | /** Watch fingerprints for pull requests on one host, one aliased lookup each; null when unsafe. */ | ||
| 2018 | export function buildPullRequestWatchFingerprintsGraphQlQuery( | ||
| 2019 | changeRequests: ReadonlyArray<{ readonly repository: string; readonly number: number }>, | ||
| 2020 | ): string | null { | ||
| 2021 | if (changeRequests.length === 0) return null; | ||
| 2022 | const selections: string[] = []; | ||
| 2023 | for (const [index, changeRequest] of changeRequests.entries()) { | ||
| 2024 | const [owner, name, ...rest] = changeRequest.repository.trim().split("/"); | ||
| 2025 | if (rest.length > 0 || owner === undefined || name === undefined) return null; | ||
| 2026 | if (!REPOSITORY_PART.test(owner) || !REPOSITORY_PART.test(name)) return null; | ||
| 2027 | if (!Number.isSafeInteger(changeRequest.number) || changeRequest.number <= 0) return null; | ||
| 2028 | selections.push( | ||
| 2029 | ` w${index}: repository(owner: "${owner}", name: "${name}") { pullRequest(number: ${changeRequest.number}) { ${PULL_REQUEST_WATCH_FINGERPRINT_SELECTION} } }`, | ||
| 2030 | ); | ||
| 2031 | } | ||
| 2032 | return `query PullRequestWatchFingerprints {\n${selections.join("\n")}\n}`; | ||
| 2033 | } | ||
| 2034 | |||
| 2035 | /** | ||
| 2036 | * Two parts, so a watch reads only what moved: `status` (state, mergeability, head, and checks) | ||
| 2037 | * needs the detail read, and `remarks` also needs the far costlier activity read. | ||
| 2038 | */ | ||
| 2039 | export interface GitHubPullRequestWatchFingerprint { | ||
| 2040 | readonly status: string; | ||
| 2041 | readonly remarks: string; | ||
| 2042 | } | ||
| 2043 | |||
| 2044 | const RawEditedSchema = Schema.Struct({ | ||
| 2045 | totalCount: Schema.Int, | ||
| 2046 | nodes: Schema.Array(Schema.NullOr(Schema.Struct({ lastEditedAt: Schema.NullOr(Schema.String) }))), | ||
| 2047 | }); | ||
| 2048 | const RawStateCountsSchema = Schema.NullOr( | ||
| 2049 | Schema.Array(Schema.Struct({ state: Schema.String, count: Schema.Int })), | ||
| 2050 | ); | ||
| 2051 | const decodeWatchFingerprintEntry = Schema.decodeUnknownExit( | ||
| 2052 | Schema.Struct({ | ||
| 2053 | state: Schema.String, | ||
| 2054 | mergeable: Schema.NullOr(Schema.String), | ||
| 2055 | headRefOid: Schema.String, | ||
| 2056 | comments: RawEditedSchema, | ||
| 2057 | reviews: RawEditedSchema, | ||
| 2058 | reviewThreads: Schema.Struct({ totalCount: Schema.Int }), | ||
| 2059 | commits: Schema.Struct({ | ||
| 2060 | nodes: Schema.Array( | ||
| 2061 | Schema.NullOr( | ||
| 2062 | Schema.Struct({ | ||
| 2063 | commit: Schema.Struct({ | ||
| 2064 | statusCheckRollup: Schema.NullOr( | ||
| 2065 | Schema.Struct({ | ||
| 2066 | contexts: Schema.Struct({ | ||
| 2067 | checkRunCountsByState: RawStateCountsSchema, | ||
| 2068 | statusContextCountsByState: RawStateCountsSchema, | ||
| 2069 | }), | ||
| 2070 | }), | ||
| 2071 | ), | ||
| 2072 | }), | ||
| 2073 | }), | ||
| 2074 | ), | ||
| 2075 | ), | ||
| 2076 | }), | ||
| 2077 | }), | ||
| 2078 | ); | ||
| 2079 | |||
| 2080 | // An edit only ever moves `lastEditedAt` forward, so the newest one stands for them all. | ||
| 2081 | const newestEdit = (connection: typeof RawEditedSchema.Type) => | ||
| 2082 | connection.nodes.reduce( | ||
| 2083 | (newest, node) => | ||
| 2084 | node?.lastEditedAt != null && node.lastEditedAt > newest ? node.lastEditedAt : newest, | ||
| 2085 | "", | ||
| 2086 | ); | ||
| 2087 | |||
| 2088 | const stateCounts = (counts: typeof RawStateCountsSchema.Type) => | ||
| 2089 | (counts ?? []) | ||
| 2090 | .filter(({ count }) => count > 0) | ||
| 2091 | .map(({ state, count }) => `${state}:${count}`) | ||
| 2092 | .toSorted() | ||
| 2093 | .join(","); | ||
| 2094 | |||
| 2095 | /** Fingerprints by alias index; a pull request GitHub returned nothing for is left out. */ | ||
| 2096 | export function decodePullRequestWatchFingerprintsJson( | ||
| 2097 | raw: string, | ||
| 2098 | ): Result.Result<ReadonlyMap<number, GitHubPullRequestWatchFingerprint>, DecodeFailure> { | ||
| 2099 | const decoded = decodeSummaries(raw); | ||
| 2100 | if (!Result.isSuccess(decoded)) return Result.fail(decoded.failure); | ||
| 2101 | const fingerprints = new Map<number, GitHubPullRequestWatchFingerprint>(); | ||
| 2102 | for (const [alias, value] of Object.entries(decoded.success.data ?? {})) { | ||
| 2103 | const index = /^w(\d+)$/.exec(alias)?.[1]; | ||
| 2104 | if (index === undefined || value?.pullRequest == null) continue; | ||
| 2105 | const entry = decodeWatchFingerprintEntry(value.pullRequest); | ||
| 2106 | if (!Exit.isSuccess(entry)) continue; | ||
| 2107 | const pr = entry.value; | ||
| 2108 | const contexts = pr.commits.nodes[0]?.commit.statusCheckRollup?.contexts; | ||
| 2109 | fingerprints.set(Number(index), { | ||
| 2110 | status: [ | ||
| 2111 | pr.state, | ||
| 2112 | pr.mergeable ?? "", | ||
| 2113 | pr.headRefOid, | ||
| 2114 | stateCounts(contexts?.checkRunCountsByState ?? null), | ||
| 2115 | stateCounts(contexts?.statusContextCountsByState ?? null), | ||
| 2116 | ].join(" "), | ||
| 2117 | remarks: [ | ||
| 2118 | pr.comments.totalCount, | ||
| 2119 | newestEdit(pr.comments), | ||
| 2120 | pr.reviews.totalCount, | ||
| 2121 | newestEdit(pr.reviews), | ||
| 2122 | pr.reviewThreads.totalCount, | ||
| 2123 | ].join(" "), | ||
| 2124 | }); | ||
| 2125 | } | ||
| 2126 | return Result.succeed(fingerprints); | ||
| 2127 | } | ||
| 2128 | |||
| 1996 | export interface GitHubPullRequestCore extends GitHubPullRequestDetail { | 2129 | export interface GitHubPullRequestCore extends GitHubPullRequestDetail { |
| 1997 | readonly viewerAccess: GitHubViewerAccess & GitHubRepositoryAccess; | 2130 | readonly viewerAccess: GitHubViewerAccess & GitHubRepositoryAccess; |
| 1998 | readonly comparison: GitHubBaseComparison | null; | 2131 | readonly comparison: GitHubBaseComparison | null; |
apps/server/src/sourceControl/GitHubCli.ts+1-1| ... | @@ -115,7 +115,7 @@ export class GitHubCliRateLimitError extends Schema.TaggedError<GitHubCliRateLim | ... | @@ -115,7 +115,7 @@ export class GitHubCliRateLimitError extends Schema.TaggedError<GitHubCliRateLim |
| 115 | { ...gitHubCliFailureFields, retryAt: Schema.optionalKey(Schema.Finite) }, | 115 | { ...gitHubCliFailureFields, retryAt: Schema.optionalKey(Schema.Finite) }, |
| 116 | ) { | 116 | ) { |
| 117 | get detail(): string { | 117 | get detail(): string { |
| 118 | return "GitHub API rate limit exceeded. Run `gh api rate_limit` to inspect the quota and reset time."; | 118 | return "GitHub API rate limit exceeded. For the GraphQL quota and reset time, run `gh api graphql -f query='{rateLimit{remaining resetAt}}'`; `gh api rate_limit` reports REST."; |
| 119 | } | 119 | } |
| 120 | 120 | ||
| 121 | override get message(): string { | 121 | override get message(): string { |
docs/user/source-control.md+9-9| ... | @@ -194,15 +194,15 @@ review is terminal. An open or unsynced link keeps it active. | ... | @@ -194,15 +194,15 @@ review is terminal. An open or unsynced link keeps it active. |
| 194 | Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While | 194 | Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While |
| 195 | the thread is active, the server checks the pull request every two minutes and wakes the agent when a | 195 | the thread is active, the server checks the pull request every two minutes and wakes the agent when a |
| 196 | check fails, the required checks pass, someone else comments or reviews, or the branch starts to | 196 | check fails, the required checks pass, someone else comments or reviews, or the branch starts to |
| 197 | conflict. Threads in a project that watch the same pull request share one check, and a pull request | 197 | conflict. Threads in a project that watch the same pull request share one check. On GitHub, a check |
| 198 | with nothing in progress is checked again when something changes or every 10 minutes, which keeps | 198 | first asks whether anything changed and reads the pull request only when it did, which keeps |
| 199 | watching inside GitHub's rate limit. Comments from your own account do not wake it. Watching ends when the pull request merges or closes, | 199 | watching inside GitHub's rate limit. Comments from your own account do not wake it. Watching ends |
| 200 | after 10 wakes in a row that bring only comments, after 8 failed reads in a row, or when you press | 200 | when the pull request merges or closes, after 10 wakes in a row that bring only comments, after 8 |
| 201 | Stop on the thread. A rate limit only pauses watching. Settling or archiving a thread also ends all | 201 | failed reads in a row, or when you press Stop on the thread. A rate limit only pauses watching. |
| 202 | its watches. Unsettle the thread before starting a new watch. Subagents cannot watch pull requests; | 202 | Settling or archiving a thread also ends all its watches. Unsettle the thread before starting a new |
| 203 | the thread that delegated to them does. To start or stop it yourself, use the row menu in the | 203 | watch. Subagents cannot watch pull requests; the thread that delegated to them does. To start or stop |
| 204 | **Linked pull requests** panel. In the thread details card, a watched pull request shows an eye; | 204 | it yourself, use the row menu in the **Linked pull requests** panel. In the thread details card, a |
| 205 | click it to stop watching. | 205 | watched pull request shows an eye; click it to stop watching. |
| 206 | 206 | ||
| 207 | A watched thread counts as working between wakes, so it stays in the **Working** section and does | 207 | A watched thread counts as working between wakes, so it stays in the **Working** section and does |
| 208 | not auto-settle. Agents stop watching when they hand the work back to you, and the thread then | 208 | not auto-settle. Agents stop watching when they hand the work back to you, and the thread then |