Skip to content
Closed
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
1 change: 1 addition & 0 deletions apps/server/src/environment/ServerEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export const make = Effect.gen(function* () {
threadVisitedTracking: true,
threadPullRequests: true,
threadPullRequestWatch: true,
pullRequestWatchQuietDeadline: true,
pullRequestStackActions: true,
threadPullRequestLinking: true,
serverResolvedCommandContext: true,
Expand Down
20 changes: 19 additions & 1 deletion apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as Effect from "effect/Effect";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
import * as Ref from "effect/Ref";
import * as Schema from "effect/Schema";
import * as Stream from "effect/Stream";
import type { Tool } from "effect/ai";

Expand All @@ -32,10 +33,15 @@ import * as McpToolAccess from "../../McpToolAccess.ts";
import * as McpToolAccessTestkit from "../../McpToolAccess.testkit.ts";
import { listThreadPullRequests } from "./handlers.ts";
import * as PullRequestsHandlers from "./handlers.ts";
import { CHILD_PULL_REQUEST_REPORT_NEXT, PullRequestsToolkit } from "./tools.ts";
import {
CHILD_PULL_REQUEST_REPORT_NEXT,
PullRequestsToolkit,
WatchPullRequestInput,
} from "./tools.ts";

const PROJECT_ID = ProjectId.make("project-1");
const THREAD_ID = ThreadId.make("thread-1");
const isWatchPullRequestInput = Schema.is(WatchPullRequestInput);

const testCrypto = Crypto.make({
randomBytes: (size) => new Uint8Array(size).fill(7),
Expand Down Expand Up @@ -225,6 +231,16 @@ const makeHarness = Effect.fn("makePullRequestsToolkitHarness")(function* (
});

describe("pull request toolkit handlers", () => {
it.each([
[0, false],
[1, true],
[10080, true],
[10081, false],
[2e11, false],
])("bounds quiet wait duration %s", (quietTimeoutMinutes, accepted) => {
expect(isWatchPullRequestInput({ quietTimeoutMinutes })).toBe(accepted);
});

it.effect("refuses a credential without the pull-requests capability", () =>
Effect.gen(function* () {
const harness = yield* makeHarness();
Expand Down Expand Up @@ -271,13 +287,15 @@ describe("pull request toolkit handlers", () => {
const harness = yield* makeHarness();
const result = yield* harness.call("watch_pull_request", {
url: "https://github.com/t3tools/t3code/pull/9",
quietTimeoutMinutes: 45,
});
expect(result).toMatchObject({ number: 9, watching: false, wasWatching: false });
expect(yield* Ref.get(harness.commands)).toMatchObject([
{
type: "thread.pull-request.watch",
number: 9,
watching: true,
quietTimeoutMinutes: 45,
link: { url: "https://github.com/t3tools/t3code/pull/9", source: "agent" },
},
]);
Expand Down
6 changes: 5 additions & 1 deletion apps/server/src/mcp/toolkits/pullRequests/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
PullRequestListFailedError,
PullRequestNotOpenError,
type PullRequestTargetInput,
type WatchPullRequestInput,
PullRequestWatchFailedError,
PullRequestWatchFromSubagentError,
PullRequestThreadNotFoundError,
Expand Down Expand Up @@ -261,7 +262,7 @@ const make = Effect.gen(function* () {
: Effect.fail(new Failure({ cause }));

const setWatching = Effect.fn("PullRequestsToolkit.setWatching")(function* (
input: PullRequestTargetInput,
input: WatchPullRequestInput,
watching: boolean,
) {
const thread = yield* requireThread(PullRequestWatchFailedError, input.threadId);
Expand All @@ -287,6 +288,9 @@ const make = Effect.gen(function* () {
repository: target.repository,
number: target.number,
watching,
...(watching && input.quietTimeoutMinutes !== undefined
? { quietTimeoutMinutes: input.quietTimeoutMinutes }
: {}),
...(watching ? { link: { url: target.url, source: "agent" as const } } : {}),
})
.pipe(Effect.catchCause(dispatchFailure(PullRequestWatchFailedError)));
Expand Down
16 changes: 14 additions & 2 deletions apps/server/src/mcp/toolkits/pullRequests/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
PositiveInt,
PullRequestReviewGuide,
PullRequestReviewGuideRun,
PullRequestWatchQuietTimeoutMinutes,
PullRequestState,
ThreadPullRequestLinkSource,
TrimmedNonEmptyString,
Expand Down Expand Up @@ -59,6 +60,17 @@ export const PullRequestTargetInput = Schema.Struct({
});
export type PullRequestTargetInput = typeof PullRequestTargetInput.Type;

export const WatchPullRequestInput = Schema.Struct({
...PullRequestTargetInput.fields,
quietTimeoutMinutes: Schema.optional(
Schema.NullOr(PullRequestWatchQuietTimeoutMinutes).annotate({
description:
"Wake you once if the PR has no new actionable check results, comments, reviews, conflicts, or pushes for this many minutes. New watch updates or a push restarts the timer. A number starts the timer now; null turns it off. Omit to keep this watch's setting (new watches have none); after the quiet wake, a call without it restarts the timer.",
}),
),
});
export type WatchPullRequestInput = typeof WatchPullRequestInput.Type;

export class PullRequestUrlInvalidError extends Schema.TaggedError<PullRequestUrlInvalidError>()(
"PullRequestUrlInvalidError",
{},
Expand Down Expand Up @@ -316,8 +328,8 @@ const ListThreadPullRequestsTool = Tool.make("list_thread_pull_requests", {

const WatchPullRequestTool = Tool.make("watch_pull_request", {
description:
"Watch an open PR for this thread, linking it first if needed. T3 wakes you on failed checks, passed required checks, new comments or reviews, or conflicts. Handle old comments first. Only later ones wake you. After this call, end your turn. The watch stays on after each wake, so do not call this again on a wake. A wake is not a merge decision. Check readiness yourself.",
parameters: PullRequestTargetInput,
"Watch an open PR for this thread, linking it first if needed. T3 wakes you on failed checks, passed required checks, new comments or reviews, or conflicts. Handle old comments first. Only later ones wake you. After this call, end your turn. The watch stays on after each wake, so do not call this again on a wake, except to restart a quiet timer after its notice. A wake is not a merge decision. Check readiness yourself.",
parameters: WatchPullRequestInput,
success: WatchPullRequestResult,
failure: PullRequestToolError,
dependencies,
Expand Down
79 changes: 53 additions & 26 deletions apps/server/src/orchestration-v2/Orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2688,7 +2688,14 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
thread.settledOverride === "settled" ||
thread.settledAt !== null ||
isProviderNativeSubagentThread(thread);
if (link?.watch?.startedAt !== command.startedAt || (command.wake && inactive)) {
if (
link?.watch?.startedAt !== command.startedAt ||
(command.expectedQuietDeadline !== undefined &&
(command.expectedQuietDeadline?.dueAt !== link.watch.quietDeadline?.dueAt ||
command.expectedQuietDeadline?.timeoutMinutes !==
link.watch.quietDeadline?.timeoutMinutes)) ||
(command.wake && inactive)
) {
return yield* new OrchestratorDispatchError({
commandId: command.commandId,
commandType: command.type,
Expand Down Expand Up @@ -3583,38 +3590,58 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
: linked;
const existing = links.find(visible);
if (existing === undefined) return thread;
const quietTimeoutMinutes =
command.type === "thread.pull-request.watch"
? command.quietTimeoutMinutes === undefined
? (existing.watch?.quietDeadline?.timeoutMinutes ?? null)
: command.quietTimeoutMinutes
: null;
const quietDeadline =
quietTimeoutMinutes === null
? undefined
: {
timeoutMinutes: quietTimeoutMinutes,
dueAt: DateTime.formatIso(DateTime.add(now, { minutes: quietTimeoutMinutes })),
};
const watch =
command.type === "thread.pull-request-watch.sync"
? existing.watch?.startedAt === command.startedAt
? (command.watch ?? undefined)
: existing.watch
: !command.watching
? undefined
: (existing.watch ?? {
startedAt,
headSha: null,
failedChecks: [],
passed: false,
passedChecks: [],
conflicting: false,
// Watching again must not repeat check or conflict news the last watch told.
...(existing.endedWatch && {
headSha: existing.endedWatch.headSha,
failedChecks: existing.endedWatch.failedChecks,
passed: existing.endedWatch.passed,
passedChecks: existing.endedWatch.passedChecks,
conflicting: existing.endedWatch.conflicting,
}),
...(existing.endedWatch?.owedChecks !== undefined && {
owedChecks: existing.endedWatch.owedChecks,
}),
...(existing.endedWatch?.checksAwaitedSince !== undefined && {
checksAwaitedSince: existing.endedWatch.checksAwaitedSince,
}),
remarksThrough: startedAt,
remarkIds: [],
wakes: 0,
});
: existing.watch !== undefined
? (command.quietTimeoutMinutes === undefined &&
existing.watch.quietDeadline?.dueAt !== null) ||
(existing.watch.quietDeadline === undefined && quietDeadline === undefined)
? existing.watch
: { ...existing.watch, quietDeadline }
: {
startedAt,
...(quietDeadline === undefined ? {} : { quietDeadline }),
headSha: null,
failedChecks: [],
passed: false,
passedChecks: [],
conflicting: false,
// Watching again must not repeat check or conflict news the last watch told.
...(existing.endedWatch && {
headSha: existing.endedWatch.headSha,
failedChecks: existing.endedWatch.failedChecks,
passed: existing.endedWatch.passed,
passedChecks: existing.endedWatch.passedChecks,
conflicting: existing.endedWatch.conflicting,
}),
...(existing.endedWatch?.owedChecks !== undefined && {
owedChecks: existing.endedWatch.owedChecks,
}),
...(existing.endedWatch?.checksAwaitedSince !== undefined && {
checksAwaitedSince: existing.endedWatch.checksAwaitedSince,
}),
remarksThrough: startedAt,
remarkIds: [],
wakes: 0,
};
if (watch === existing.watch && links === linked) return thread;
const final =
command.type === "thread.pull-request-watch.sync" && watch === undefined
Expand Down
11 changes: 11 additions & 0 deletions apps/server/src/orchestration-v2/PullRequestWatchReactor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ function planRead(
fingerprint: ProviderChangeRequestWatchFingerprint | null,
now: number,
): ReadPlan {
if (
group.targets.some(
({ watch }) =>
watch.quietDeadline?.dueAt != null && now >= Date.parse(watch.quietDeadline.dueAt),
)
) {
return { detail: true, activity: true };
}
if (last === undefined || group.targets.some((target) => !last.watches.has(watchKey(target)))) {
return { detail: true, activity: true };
}
Expand All @@ -157,6 +165,8 @@ function planRead(
function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatch): boolean {
return (
left.startedAt === right.startedAt &&
left.quietDeadline?.timeoutMinutes === right.quietDeadline?.timeoutMinutes &&
left.quietDeadline?.dueAt === right.quietDeadline?.dueAt &&
left.headSha === right.headSha &&
left.failedChecks.join("\n") === right.failedChecks.join("\n") &&
left.passed === right.passed &&
Expand Down Expand Up @@ -270,6 +280,7 @@ export const make = Effect.gen(function* () {
threadId: target.thread.id,
...identityOf(target.link),
startedAt: target.watch.startedAt,
expectedQuietDeadline: target.watch.quietDeadline ?? null,
watch: next,
...(endedWatch === undefined ? {} : { endedWatch }),
...(wake === undefined
Expand Down
41 changes: 41 additions & 0 deletions apps/server/src/orchestration-v2/pullRequestWatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,47 @@ const remark = (
const noRemarks: ReadonlyArray<PullRequestComment> = [];

describe("evaluatePullRequestWatch", () => {
it("reports an unchanged wait once, then rearms only on actionable news or a push", () => {
const start = Date.parse(STARTED);
const dueAt = "2026-10-02T12:45:00.000Z";
const armed = {
...watch({ headSha: "aaaaaaaaaa", wakes: 3 }),
quietDeadline: { timeoutMinutes: 45, dueAt },
};
const before = evaluate(armed, detail(), noRemarks, start + 44 * 60_000);
assert.deepEqual(before.changes, []);
assert.deepEqual(before.next.quietDeadline, armed.quietDeadline);

const quiet = evaluate(before.next, detail(), noRemarks, start + 45 * 60_000);
assert.deepEqual(quiet.changes, [{ kind: "quiet", minutes: 45 }]);
assert.deepEqual(quiet.next.quietDeadline, { timeoutMinutes: 45, dueAt: null });
assert.equal(quiet.next.wakes, 3);
assert.isFalse(quiet.exhausted);
assert.deepEqual(evaluate(quiet.next, detail(), noRemarks, start + 90 * 60_000).changes, []);

const pushed = evaluate(
quiet.next,
detail({ headSha: "bbbbbbbbbb" }),
noRemarks,
start + 91 * 60_000,
);
assert.deepEqual(pushed.next.quietDeadline, {
timeoutMinutes: 45,
dueAt: "2026-10-02T14:16:00.000Z",
});
const done = evaluate(
armed,
detail({ checks: [check("lint", "success")] }),
noRemarks,
start + 45 * 60_000,
);
assert.deepEqual(
done.changes.map((change) => change.kind),
["checks-passed"],
);
assert.equal(done.next.quietDeadline?.dueAt, "2026-10-02T13:30:00.000Z");
});

it("reports each failure at once, even while another check never finishes", () => {
const bot = check("CodeRabbit", "pending");
const first = detail({ checks: [check("lint", "failure"), check("test", "pending"), bot] });
Expand Down
21 changes: 20 additions & 1 deletion apps/server/src/orchestration-v2/pullRequestWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type PullRequestWatchChange =
| { 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: "quiet"; readonly minutes: number }
| { readonly kind: "conflicting" };

export interface PullRequestWatchReport {
Expand Down Expand Up @@ -136,10 +137,23 @@ export function evaluatePullRequestWatch(
const commentsOnly = changes.length > 0 && changes.every((change) => change.kind === "remarks");
const progress = headMoved || (changes.length > 0 && !commentsOnly);
const wakes = (progress ? 0 : watch.wakes) + (commentsOnly ? 1 : 0);
let quietDeadline = watch.quietDeadline;
if (quietDeadline !== undefined) {
if ((headMoved && watch.headSha !== null) || changes.length > 0) {
quietDeadline = {
...quietDeadline,
dueAt: DateTime.formatIso(DateTime.makeUnsafe(now + quietDeadline.timeoutMinutes * 60_000)),
};
} else if (quietDeadline.dueAt !== null && now >= Date.parse(quietDeadline.dueAt)) {
changes.push({ kind: "quiet", minutes: quietDeadline.timeoutMinutes });
quietDeadline = { ...quietDeadline, dueAt: null };
}
}
return {
changes,
next: {
startedAt: watch.startedAt,
...(quietDeadline === undefined ? {} : { quietDeadline }),
headSha,
failedChecks,
passed,
Expand Down Expand Up @@ -201,6 +215,10 @@ function changeLines(
return [
`- No checks have reported${context.commit} after ${change.minutes} minutes. None may be configured to run on this change.`,
];
case "quiet":
return [
`- No new actionable check results, comments, reviews, conflicts, or pushes for ${change.minutes} minutes. Check whether CI or review is stuck; this notice is not a review or merge signal. The watch stays on and sends this notice once; new watch updates, a push, or calling watch_pull_request again restarts the timer.`,
];
case "conflicting":
return [`- The branch now conflicts with ${context.baseBranch}.`];
}
Expand All @@ -211,6 +229,7 @@ const SUMMARY: Record<PullRequestWatchChange["kind"], string> = {
"checks-passed": "checks passed",
remarks: "new comments",
"no-checks": "no checks reported",
quiet: "still waiting",
conflicting: "merge conflict",
};

Expand All @@ -232,7 +251,7 @@ export function pullRequestWatchMessage(input: {
"",
exhausted
? `T3 Code stopped watching after ${PULL_REQUEST_WATCH_WAKE_LIMIT} comment-only updates in a row. Call watch_pull_request to watch it again.`
: "Look into each item and act on it as your task requires. This PR stays linked and watched, so do not link, watch, or list it again. End your turn when you are done, and T3 Code wakes you on the next change. When you hand the work back to the user, call unwatch_pull_request first so the thread returns to their inbox.",
: `Look into each item and act on it as your task requires. This PR stays linked and watched, so do not link, watch, or list it again${changes.some((change) => change.kind === "quiet") ? ", except to deliberately start another bounded quiet wait" : ""}. End your turn when you are done, and T3 Code wakes you on the next change. When you hand the work back to the user, call unwatch_pull_request first so the thread returns to their inbox.`,
].join("\n");
const failed = changes.some(
(change) => change.kind === "checks-failed" || change.kind === "conflicting",
Expand Down
Loading
Loading