Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/server/src/orchestration-v2/Orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3400,6 +3400,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
passedChecks: existing.endedWatch.passedChecks,
conflicting: existing.endedWatch.conflicting,
}),
...(existing.endedWatch?.checksAwaitedSince !== undefined && {
checksAwaitedSince: existing.endedWatch.checksAwaitedSince,
}),
remarksThrough: startedAt,
remarkIds: [],
wakes: 0,
Expand Down
5 changes: 3 additions & 2 deletions apps/server/src/orchestration-v2/PullRequestWatchReactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc
left.remarksThrough === right.remarksThrough &&
left.remarkIds.join("\n") === right.remarkIds.join("\n") &&
left.conflicting === right.conflicting &&
left.wakes === right.wakes
left.wakes === right.wakes &&
left.checksAwaitedSince === right.checksAwaitedSince
);
}

Expand Down Expand Up @@ -539,7 +540,7 @@ export const make = Effect.gen(function* () {
remarks: fingerprint?.remarks ?? null,
});
yield* eachTarget(group, (target) => {
const report = evaluatePullRequestWatch(target.watch, detail, remarks);
const report = evaluatePullRequestWatch(target.watch, detail, remarks, now);
if (report.changes.length > 0) {
const wake = pullRequestWatchMessage({
number: target.link.number,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import type { ThreadPullRequestWatch } from "@t3tools/contracts";
import { SourceControlProviderKind, type ThreadPullRequestWatch } from "@t3tools/contracts";
import { assert, describe, it } from "@effect/vitest";
import * as Schema from "effect/Schema";
import {
ForgejoComment,
forgejoComment,
} from "@t3tools/source-control-forgejo/server/forgejoPullRequestJson";

import { evaluatePullRequestWatch } from "./pullRequestWatch.ts";
import { evaluatePullRequestWatch as evaluate } from "./pullRequestWatch.ts";

// These cases read the head once or at one moment, so the no-checks notice never comes due.
const evaluatePullRequestWatch = (
...args: Parameters<typeof evaluate> extends [...infer Head, number] ? Head : never
) => evaluate(...args, 0);

const decodeForgejoComment = Schema.decodeSync(ForgejoComment);

Expand All @@ -25,6 +30,7 @@ describe("Forgejo comment watch boundary", () => {
wakes: 2,
} satisfies ThreadPullRequestWatch;
const finalDetail = {
provider: SourceControlProviderKind.make("forgejo"),
headSha: seen.headSha,
checks: [],
mergeability: "mergeable",
Expand Down
46 changes: 43 additions & 3 deletions apps/server/src/orchestration-v2/pullRequestWatch.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import type {
PullRequestCheck,
import {
SourceControlProviderKind,
type PullRequestCheck,
PullRequestComment,
ThreadPullRequestWatch,
} from "@t3tools/contracts";
import { assert, describe, it } from "@effect/vitest";

import { PULL_REQUEST_WATCH_WAKE_LIMIT, evaluatePullRequestWatch } from "./pullRequestWatch.ts";
import {
PULL_REQUEST_WATCH_WAKE_LIMIT,
evaluatePullRequestWatch as evaluate,
} from "./pullRequestWatch.ts";

// These cases read the head once or at one moment, so the no-checks notice never comes due.
const evaluatePullRequestWatch = (
...args: Parameters<typeof evaluate> extends [...infer Head, number] ? Head : never
) => evaluate(...args, 0);

const STARTED = "2026-10-02T12:00:00.000Z";

Expand All @@ -32,6 +41,7 @@ const check = (name: string, status: PullRequestCheck["status"]): PullRequestChe
type Detail = Parameters<typeof evaluatePullRequestWatch>[1];

const detail = (overrides: Partial<Detail> = {}): Detail => ({
provider: SourceControlProviderKind.make("github"),
headSha: "aaaaaaaaaa",
checks: [check("lint", "success"), check("test", "pending")],
mergeability: "mergeable",
Expand Down Expand Up @@ -252,4 +262,34 @@ describe("evaluatePullRequestWatch", () => {
assert.isFalse(pushed.exhausted);
assert.equal(pushed.next.wakes, 1);
});

const anHourLater = (
first: ThreadPullRequestWatch,
read: Detail,
): ReadonlyArray<ReadonlyArray<string>> => {
const kinds: Array<ReadonlyArray<string>> = [];
let current = first;
for (let minute = 0; minute <= 60; minute += 10) {
const { changes, next } = evaluate(current, read, noRemarks, minute * 60_000);
kinds.push(changes.map((change) => change.kind));
current = next;
}
return kinds;
};

it("does not tell a watch saved before the no-checks clock that its seen head has none", () => {
// Such a watch may have seen this head's checks pending, which records nothing.
const legacy = watch({ headSha: "aaaaaaaaaa" });
assert.isTrue(anHourLater(legacy, detail({ checks: [] })).every((kinds) => kinds.length === 0));
const pushed = anHourLater(legacy, detail({ headSha: "bbbbbbbbbb", checks: [] }));
assert.deepEqual(pushed.flat(), ["no-checks"]);
});

it("never reports missing checks on a host whose checks it cannot read", () => {
const azure = detail({
provider: SourceControlProviderKind.make("azure-devops"),
checks: [],
});
assert.isTrue(anHourLater(watch(), azure).every((kinds) => kinds.length === 0));
});
});
33 changes: 32 additions & 1 deletion apps/server/src/orchestration-v2/pullRequestWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,23 @@ import type {
PullRequestDetail,
ThreadPullRequestWatch,
} from "@t3tools/contracts";
import * as DateTime from "effect/DateTime";

/**
* Wakes in a row that bring only comments. Check, conflict, or push news resets the count, so
* this only stops a chatty bot looping an agent that is replying to it.
*/
export const PULL_REQUEST_WATCH_WAKE_LIMIT = 10;
/** How long a GitHub head may go without any check before the agent is told none reported. */
const PULL_REQUEST_WATCH_NO_CHECKS_MS = 15 * 60_000;
const LISTED_ITEMS = 10;
const SNIPPET_LENGTH = 200;

export type PullRequestWatchChange =
| { readonly kind: "checks-failed"; readonly failed: ReadonlyArray<PullRequestCheck> }
| { readonly kind: "checks-passed"; readonly count: number; readonly required: boolean }
| { readonly kind: "remarks"; readonly remarks: ReadonlyArray<PullRequestComment> }
| { readonly kind: "no-checks"; readonly minutes: number }
| { readonly kind: "conflicting" };

export interface PullRequestWatchReport {
Expand All @@ -32,8 +36,12 @@ const isFailedCheck = (check: PullRequestCheck) =>

export function evaluatePullRequestWatch(
watch: ThreadPullRequestWatch,
detail: Pick<PullRequestDetail, "headSha" | "checks" | "mergeability" | "viewer" | "author">,
detail: Pick<
PullRequestDetail,
"provider" | "headSha" | "checks" | "mergeability" | "viewer" | "author"
>,
remarks: ReadonlyArray<PullRequestComment> | null,
now: number,
): PullRequestWatchReport {
const changes: Array<PullRequestWatchChange> = [];
const headSha = detail.headSha ?? null;
Expand Down Expand Up @@ -64,6 +72,23 @@ export function evaluatePullRequestWatch(
passedChecks = passedNow ? gateNames : [];
}

// Only a head never read with checks waits for them: an empty read after checks were seen is
// the failed read above. A watch saved before this field may have seen pending checks on its
// head, so its clock starts only at the next push. Other hosts can lack a checks API (Azure
// DevOps pipelines), where an empty list says nothing, so only GitHub waits.
let checksAwaitedSince = watch.checksAwaitedSince;
if (detail.provider === "github") {
const awaitedBefore = headMoved
? DateTime.formatIso(DateTime.makeUnsafe(now))
: (watch.checksAwaitedSince ?? null);
checksAwaitedSince = detail.checks.length > 0 ? null : awaitedBefore;
const awaitedMs = checksAwaitedSince === null ? 0 : now - Date.parse(checksAwaitedSince);
if (awaitedMs >= PULL_REQUEST_WATCH_NO_CHECKS_MS) {
changes.push({ kind: "no-checks", minutes: Math.round(awaitedMs / 60_000) });
checksAwaitedSince = null;
}
}

const own = (detail.viewer ?? detail.author?.login)?.toLowerCase();
const through = Date.parse(watch.remarksThrough);
const activeAt = (remark: PullRequestComment) => remark.editedAt ?? remark.createdAt;
Expand Down Expand Up @@ -106,6 +131,7 @@ export function evaluatePullRequestWatch(
remarkIds,
conflicting,
wakes,
checksAwaitedSince,
},
exhausted: commentsOnly && wakes >= PULL_REQUEST_WATCH_WAKE_LIMIT,
};
Expand Down Expand Up @@ -153,6 +179,10 @@ function changeLines(
return ` - ${remark.author?.login ?? "someone"}${where}: ${said}${remark.url ? ` ${remark.url}` : ""}`;
}),
];
case "no-checks":
return [
`- No checks have reported${context.commit} after ${change.minutes} minutes. None may be configured to run on this change.`,
];
case "conflicting":
return [`- The branch now conflicts with ${context.baseBranch}.`];
}
Expand All @@ -162,6 +192,7 @@ const SUMMARY: Record<PullRequestWatchChange["kind"], string> = {
"checks-failed": "checks failed",
"checks-passed": "checks passed",
remarks: "new comments",
"no-checks": "no checks reported",
conflicting: "merge conflict",
};

Expand Down
Loading
Loading