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
4 changes: 4 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,10 @@ DISPATCH_AGENT_TOKEN="your_agent_token_here"
# again (#1211). An operator requeue resets the count. Default 3.
# PR_FIX_MAX_RECLAIMS=3

# Cooldown (ms) before an external-prerequisite wait is redispatched for a fresh
# worker-side readiness check. Bounded to 60 000–604 800 000; default 21 600 000 (6h).
# PR_FIX_EXTERNAL_WAIT_COOLDOWN_MS=21600000

# --- Legacy aliases (do not set) ---

# MISSION_CONTROL_URL and MISSION_CONTROL_AGENT_TOKEN were renamed to
Expand Down
3 changes: 3 additions & 0 deletions docs/pr-fix-outcome-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@
| `no_progress` | Bounded retry or no-progress exhaustion | `PARKED` | `NORMAL` | Diagnostic `<!-- dispatch-pr-fix-parked -->` comment; no `needs-human` label |
| `infra_failure` | Infrastructure or authentication failure | `PARKED` | `NORMAL` | Diagnostic parked comment; fail closed, not blindly retried |
| `security_failure` | Security failure | `PARKED` | `NORMAL` | Diagnostic parked comment; fail closed, not blindly retried |
| external wait | Worker is waiting on a prerequisite outside Dispatch | `WAITING_EXTERNAL` | Existing lane | No human escalation or attempt charge; retry after cooldown |

The structured outcome is stored on `PrFixQueueItem` and `PrFixHistory`; human decisions may also include `decisionDetail`. A `tasks/report` request may provide `blockReason` and `decision` (`question`, `options`, `evidence`). A bare `outcome: "blocked"` without `blockReason` means `no_progress`, not `human_decision`. An `outcome` supplied on a brand-new `NORMAL`-lane enqueue is advisory and only takes effect if/when the item parks (a fresh item starts `QUEUED`).

## Budgets and recovery

`fixAttempts` counts dispatchable worker executions. Hand-out reclamation (`handoutReclaims`) and `PR_FIX_HANDOUT_TIMEOUT_MS` wall time are separate budgets: a stale hand-out that never reached a worker does not consume an execution attempt. Stale or already-known feedback also consumes no execution budget. No new numeric thresholds are introduced.

An `outcome: "waiting-external"` report (or `WAITING_EXTERNAL` mark with the issued generation) parks work temporarily without implying prerequisite readiness. Queue listings exclude it by default; `include_blocked=true` includes pending external waits as read-only entries with the latest wait note and retry deadline, and does not make them claimable or signal readiness. The bounded `PR_FIX_EXTERNAL_WAIT_COOLDOWN_MS` setting defaults to six hours (accepted range: one minute to seven days). When due, queue discovery conditionally reissues it as a new generation; the next worker rechecks the prerequisite. This does not increment `fixAttempts` or enforce its cap. Repeated report/mark for the same waiting generation is idempotent and does not extend the cooldown. Late marks cannot change the generation.

Automatic enqueue/evidence does not release `BLOCKED` or `PARKED`. An operator explicitly requeues either status, or a maintainer marks the item back to `QUEUED`; requeue releases the parked/blocked state. `FIXED` retains its existing #940 and new-evidence reopen paths.

## Safety and reporting
Expand Down
6 changes: 3 additions & 3 deletions docs/pr-review-fix-queue.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ Each item is deduped by `(repo, pr)` and stores:
- `repo`, `pr`, `branch`, `url`, `title`
- optional source `issue` (extracted from PR title/body)
- `lane`: `NORMAL`, `ESCALATED`, or `NEEDS_HUMAN`
- `status`: `QUEUED`, `FIXED`, `BLOCKED`, `PARKED`, `STALE`, or `IGNORED`
- `outcome`: `human_decision`, `no_progress`, `infra_failure`, or `security_failure`; human decisions may also store `decisionDetail`
- `status`: `QUEUED`, `FIXED`, `BLOCKED`, `PARKED`, `STALE`, `IGNORED`, or `WAITING_EXTERNAL`
- `outcome`: `human_decision`, `no_progress`, `infra_failure`, or `security_failure`; human decisions may also store `decisionDetail`. External waits use report outcome `waiting-external` and status `WAITING_EXTERNAL`, with a bounded cooldown before a fresh generation checks readiness.
- `reason`, `feedback[]`, `evidenceKeys[]`
- `headSha` and `author` metadata
- `generation` — Dispatch-owned work generation (see below)
Expand Down Expand Up @@ -93,7 +93,7 @@ Items in the `NEEDS_HUMAN` lane receive `BLOCKED` status and are excluded from t

### Mark item status

`POST /api/pr-fix-queue/mark` — Marks an item `FIXED`, `BLOCKED`, `PARKED`, `STALE`, or `IGNORED` and records a history event. Requires auth. The body may include an `outcome` (`human_decision`, `no_progress`, `infra_failure`, or `security_failure`) and a `decision` object (`question`, `options`, and `evidence`) when recording a human decision.
`POST /api/pr-fix-queue/mark` — Marks an item `FIXED`, `BLOCKED`, `PARKED`, `STALE`, `IGNORED`, or `WAITING_EXTERNAL` and records a history event. Requires auth. The body may include an `outcome` (`human_decision`, `no_progress`, `infra_failure`, or `security_failure`) and a `decision` object (`question`, `options`, and `evidence`) when recording a human decision.

**Request body:**
```json
Expand Down
4 changes: 2 additions & 2 deletions docs/worker-execution-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Dispatch bearer tokens have two tiers. A **worker** token may call exactly:
- `POST /api/issues/claim` (without `force`) and `POST /api/issues/unclaim` (for the caller's own agent)
- `GET /api/issues/state`, `POST /api/issues/status` (status only for issues assigned to the caller's own agent)
- `GET /api/issues`, `GET /api/pr-fix-queue/queued`, `GET /api/pr-fix-queue/history`
- `POST /api/pr-fix-queue/mark` with `FIXED`, `BLOCKED`, or `STALE` (generation required, as today)
- `POST /api/pr-fix-queue/mark` with `FIXED`, `BLOCKED`, `PARKED`, `STALE`, or `WAITING_EXTERNAL` (generation required)
- `POST /api/pr-fix-queue/ack` (the durable hand-out acknowledgement, see "Hand-out acknowledgement" below)

The **maintainer** token (`DISPATCH_AGENT_TOKEN`, or the `DISPATCH_MAINTAINER_TOKEN` alias) keeps full rights and acts on any agent's behalf. A worker token calling a maintainer-only route gets an HTTP 403 naming the required tier: force claims, `QUEUED`/`IGNORED` marks, `POST /api/pr-fix-queue/requeue`, and the `POST /api/pr-fix-queue/sweep` reclaim job are maintainer-only. Unclaim is not tier-gated — the target agent is bound to the caller and is only bounded by the assignment check; the legacy `DISPATCH_WORKER_TOKEN` falls back to the request body, but `DISPATCH_WORKER_TOKENS` overrides that with a token-derived identity. A `DISPATCH_WORKER_TOKEN` value that duplicates a maintainer token resolves to the lower worker tier (fail-closed) and logs a one-time boot warning — set it to a distinct value.
Expand Down Expand Up @@ -224,7 +224,7 @@ curl -s -X POST "DISPATCH_URL/api/pr-fix-queue/mark" \
-d '{"repo":"org/repo","pr":42,"status":"fixed"}'
```

Valid statuses: `FIXED`, `BLOCKED`, `STALE`, `IGNORED`.
Valid statuses: `FIXED`, `BLOCKED`, `PARKED`, `STALE`, `IGNORED`, `WAITING_EXTERNAL`. `WAITING_EXTERNAL` defers the consumed generation for the configured cooldown; it neither charges an attempt nor signals prerequisite readiness.

### Normal Lane

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TYPE "PrFixStatus" ADD VALUE IF NOT EXISTS 'WAITING_EXTERNAL';
ALTER TABLE "PrFixQueueItem" ADD COLUMN "retryAt" TIMESTAMP(3);
3 changes: 3 additions & 0 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ enum PrFixStatus {
PARKED
STALE
IGNORED
WAITING_EXTERNAL
}

enum PrFixType {
Expand Down Expand Up @@ -219,6 +220,8 @@ model PrFixQueueItem {
// `needs-human` label is never applied. Release requires an explicit
// operator requeue or a maintainer mark to QUEUED.
handoutReclaims Int @default(0)
// Cooldown deadline for external dependency waits; due items are reissued lazily.
retryAt DateTime?
author String?
history PrFixHistory[]

Expand Down
66 changes: 66 additions & 0 deletions src/app/api/agents/[agentName]/next-task/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ vi.mock("@/lib/prisma", () => ({
prFixQueueItem: {
create: mocks.prFixCreate,
findUnique: mocks.prFixFindUniqueByRepo,
updateMany: mocks.prFixUpdateMany,
},
prFixHistory: { create: mocks.prFixHistoryCreate },
}),
Expand Down Expand Up @@ -645,6 +646,71 @@ describe("GET /api/agents/[agentName]/next-task", () => {
expect(secondBody.prFixItem.id).toBe(firstBody.prFixItem.id);
});

it("withholds future external waits and lazily hands out due retries at a new generation (#1231)", async () => {
const client = makeQueueStore();
mocks.prFixFindMany.mockImplementation(async ({ where }: any = {}) =>
client.prFixQueueItem.findMany({ where }),
);
mocks.prFixFindUnique.mockImplementation(async ({ where }: any) =>
client.items.find((row: any) => row.id === where.id) ?? null,
);
mocks.prFixUpdateMany.mockImplementation(async ({ where, data }: any) => {
const row = client.items.find((candidate: any) =>
candidate.id === where.id && candidate.generation === where.generation &&
(where.status === undefined || candidate.status === where.status) &&
(where.retryAt === undefined || candidate.retryAt?.getTime() === where.retryAt.getTime()) &&
(!where.OR || where.OR.some((condition: any) =>
condition.dispatchedGeneration === null
? candidate.dispatchedGeneration == null
: candidate.dispatchedGeneration !== condition.dispatchedGeneration.not,
)),
);
if (!row) return { count: 0 };
const patch: Record<string, any> = { ...data };
for (const [key, value] of Object.entries(patch)) {
if (value && typeof value === "object" && Array.isArray((value as any).push)) {
patch[key] = [...(row[key] ?? []), ...(value as any).push];
} else if (value && typeof value === "object" && typeof (value as any).increment === "number") {
patch[key] = (row[key] ?? 0) + (value as any).increment;
}
}
Object.assign(row, patch);
return { count: 1 };
});
const item = await enqueuePrFixItem(client, {
repo: "org/repo", pr: 123, lane: "NORMAL", reason: "review changes requested",
feedback: "please update tests", evidenceKey: "review:123",
});
await markPrFixItem(client, {
repo: "org/repo", pr: 123, status: "WAITING_EXTERNAL",
expectedGeneration: item.generation, note: "waiting on prerequisite",
});
const waiting = client.items[0];
const issuedGeneration = waiting.generation;
expect(waiting.retryAt.getTime()).toBeGreaterThan(Date.now());

const poll = async () => GET(
request("/api/agents/example-agent/next-task"),
{ params: Promise.resolve({ agentName: "example-agent" }) },
);
const early = await poll();
expect((await early.json()).type).toBe("idle");
expect(waiting.status).toBe("WAITING_EXTERNAL");

waiting.retryAt = new Date(Date.now() - 1_000);
const due = await poll();
const body = await due.json();
expect(body.type).toBe("followup-pr");
expect(body.prFixItem).toEqual({ id: waiting.id, generation: issuedGeneration + 1 });
expect(waiting.status).toBe("QUEUED");
expect(waiting.generation).toBeGreaterThan(issuedGeneration);
expect(waiting.fixAttempts).toBeUndefined();
expect(waiting.dispatchedGeneration).toBe(waiting.generation);

const repeat = await poll();
expect((await repeat.json()).type).toBe("idle");
});

it("serves the bumped generation through the production requeue path", async () => {
// Drive the real queue state machine (enqueue → BLOCKED → requeue) against
// an in-memory store, and verify next-task surfaces the persisted identity.
Expand Down
9 changes: 9 additions & 0 deletions src/app/api/agents/[agentName]/tasks/report/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ describe("POST /api/agents/[agentName]/tasks/report — validation", () => {
"no_changes_needed",
"already_addressed",
"handed-off",
"waiting-external",
];

for (const outcome of validOutcomes) {
Expand Down Expand Up @@ -1616,6 +1617,14 @@ describe("POST /api/agents/[agentName]/tasks/report — prFixItem attempt token
expect(body.report.prFixItem).toEqual({ id: "cktz-abc", generation: 2 });
});

it.each(["implement", "followup-pr"] as const)("accepts waiting-external for %s and records a neutral run", async (taskType) => {
const body = { ...validReport, taskType, outcome: "waiting-external", error: "missing prerequisite: X", ...(taskType === "followup-pr" ? { prFixItem: { id: "cktz-abc", generation: 2 } } : {}) };
const res = await postRequest(body);
expect(res.status).toBe(200);
expect(prFixResolveMock).toHaveBeenCalledWith(expect.objectContaining({ outcome: "waiting-external", error: "missing prerequisite: X", attempt: taskType === "followup-pr" ? { itemId: "cktz-abc", generation: 2 } : null }));
expect(mockAgentRun.create).toHaveBeenCalledWith(expect.objectContaining({ data: expect.objectContaining({ status: "waiting-external" }) }));
});

it("returns 400 when prFixItem is not an object", async () => {
const res = await postRequest({ ...validReport, prFixItem: "cktz-abc" });
expect(res.status).toBe(400);
Expand Down
3 changes: 3 additions & 0 deletions src/app/api/agents/[agentName]/tasks/report/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const VALID_OUTCOMES = [
"already_addressed",
"already_done",
"handed-off",
"waiting-external",
] as const;
type ValidOutcome = (typeof VALID_OUTCOMES)[number];

Expand Down Expand Up @@ -73,6 +74,7 @@ function deriveStatus(outcome: ValidOutcome): string {
if (outcome === "handed-off") return "handed-off";
if (outcome === "failed") return "failed";
if (outcome === "blocked") return "blocked";
if (outcome === "waiting-external") return "waiting-external";
return "completed";
}

Expand Down Expand Up @@ -590,6 +592,7 @@ export async function POST(
pullRequestUrl: report.pullRequestUrl,
outcome: report.outcome,
summary: report.summary,
error: report.error,
evidence: report.evidence,
// #1218: a `blocked` report's structured reason / decision payload;
// the resolver ignores both for any other outcome.
Expand Down
16 changes: 16 additions & 0 deletions src/app/api/pr-fix-queue/mark/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,22 @@ describe("POST /api/pr-fix-queue/mark — worker tier (#1111)", () => {
);
}

it("allows a worker to mark an item WAITING_EXTERNAL with the exact generation", async () => {
mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "WAITING_EXTERNAL", expectedGeneration: 2 });
mocks.markPrFixItem.mockResolvedValue({ mutated: true, item: { id: "fix-1", status: "WAITING_EXTERNAL" } });
const res = await workerPost({ repo: "org/repo", pr: 42, status: "WAITING_EXTERNAL", generation: 2 });
expect(res.status).toBe(200);
expect(mocks.markPrFixItem).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ status: "WAITING_EXTERNAL", expectedGeneration: 2 }));
});

it("rejects a worker external wait without a generation", async () => {
mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "WAITING_EXTERNAL" });
const res = await workerPost({ repo: "org/repo", pr: 42, status: "WAITING_EXTERNAL" });
expect(res.status).toBe(400);
expect((await res.json()).error).toContain("generation is required for WAITING_EXTERNAL");
expect(mocks.markPrFixItem).not.toHaveBeenCalled();
});

it("allows a worker to mark an item FIXED with a generation", async () => {
mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "FIXED", expectedGeneration: 2 });
mocks.markPrFixItem.mockResolvedValue({ mutated: true, item: { id: "fix-1", status: "FIXED" } });
Expand Down
11 changes: 9 additions & 2 deletions src/app/api/pr-fix-queue/mark/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ export async function POST(request: Request) {
const input = parseMarkPrFixInput(body);
if ("error" in input) return errorResponse(input.error, 400);

// Worker tokens may only settle attempts (FIXED/BLOCKED/PARKED/STALE):
// Worker tokens may settle attempts (FIXED/BLOCKED/PARKED/STALE) or defer a
// consumed generation while waiting on an external prerequisite (WAITING_EXTERNAL):
// PARKED lets a worker report bounded exhaustion or an operational
// failure (#1218). Moving an item back to QUEUED or IGNORED changes
// routing and requires a maintainer token (#1111).
const WORKER_SETTLE_STATUSES = ["FIXED", "BLOCKED", "PARKED", "STALE"];
const WORKER_SETTLE_STATUSES = ["FIXED", "BLOCKED", "PARKED", "STALE", "WAITING_EXTERNAL"];
if (
auth.type === "bearer" &&
auth.tier === "worker" &&
Expand Down Expand Up @@ -74,6 +75,12 @@ export async function POST(request: Request) {
? (body as Record<string, unknown>).allowStickyReclassify === true
: false;

// Every external wait consumes a handed-out generation, including operator
// marks, so it must always be fenced. Other operator marks retain legacy behavior.
if (input.status === "WAITING_EXTERNAL" && (input.expectedGeneration === undefined || input.expectedGeneration === null)) {
return errorResponse("generation is required for WAITING_EXTERNAL", 400);
}

// #1074: bearer (agent/bridge) marks must settle a specific attempt, so
// the generation token is required. Operator paths (oidc session, basic,
// disabled) keep the optional behavior for compatibility.
Expand Down
15 changes: 15 additions & 0 deletions src/app/automation/pr-fix-queue/page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,21 @@ describe("PR Fix Queue page", () => {
expect(screen.getByText(/ci failure/)).toBeInTheDocument();
});

it("shows external waits as read-only with their note and retry deadline", async () => {
const retryAt = "2026-01-03T12:00:00Z";
mockAuthedFetch.mockResolvedValueOnce({
ok: true,
json: async () => [
{ id: "wait-1", repo: "org/repo", pr: 44, status: "WAITING_EXTERNAL", lane: "NORMAL", reason: "CI prerequisite", latestNote: "missing prerequisite: X", retryAt, title: "Wait for CI", url: null, queuedAt: "2026-01-02T00:00:00Z" },
],
});
render(<PrFixQueuePage />);
await waitFor(() => expect(screen.getByText(/Waiting on external prerequisite/i)).toBeInTheDocument());
expect(screen.getByText("missing prerequisite: X")).toBeInTheDocument();
expect(screen.getByText(new Date(retryAt).toLocaleString())).toBeInTheDocument();
expect(screen.queryByRole("button", { name: /requeue/i })).not.toBeInTheDocument();
});

it("shows empty state when no items", async () => {
mockAuthedFetch.mockResolvedValueOnce({
ok: true,
Expand Down
24 changes: 24 additions & 0 deletions src/app/automation/pr-fix-queue/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ interface PrFixItem {
title: string | null;
url: string | null;
queuedAt: string;
retryAt?: string | null;
latestNote?: string | null;
outcome?: string | null;
fixAttempts?: number | null;
}
Expand Down Expand Up @@ -116,6 +118,7 @@ export default function PrFixQueuePage() {
const blocked = items.filter((i) => i.status === "BLOCKED");
const parked = items.filter((i) => i.status === "PARKED");
const queued = items.filter((i) => i.status === "QUEUED");
const waiting = items.filter((i) => i.status === "WAITING_EXTERNAL");

if (loading) {
return <div className="flex items-center justify-center h-64">Loading...</div>;
Expand Down Expand Up @@ -291,6 +294,27 @@ export default function PrFixQueuePage() {
</CardContent>
</Card>

<Card className="border-blue-200">
<CardHeader>
<CardTitle className="flex items-center gap-2">
Waiting on external prerequisite
{waiting.length > 0 && <Badge className="ml-2">{waiting.length}</Badge>}
</CardTitle>
</CardHeader>
<CardContent>
{waiting.length === 0 ? <p className="text-center py-8 text-muted-foreground">No external waits.</p> : (
<Table>
<TableHeader><TableRow><TableHead>Repo / PR</TableHead><TableHead>Last-known explanation</TableHead><TableHead>Retry At</TableHead></TableRow></TableHeader>
<TableBody>{waiting.map((item) => <TableRow key={item.id}>
<TableCell><a href={prLink(item)} target="_blank" rel="noopener noreferrer" className="font-medium hover:underline">{prTitle(item)}</a><div className="text-xs text-muted-foreground">{item.repo}</div></TableCell>
<TableCell className="text-sm">{item.latestNote || item.reason}</TableCell>
<TableCell className="text-sm text-muted-foreground">{item.retryAt ? formatTime(item.retryAt) : "Not scheduled"}</TableCell>
</TableRow>)}</TableBody>
</Table>
)}
</CardContent>
</Card>

<Card>
<CardHeader>
<CardTitle className="flex items-center gap-2">
Expand Down
Loading
Loading