From cfa2d3254491cf621f7d3b639d21f0487c508e98 Mon Sep 17 00:00:00 2001 From: Courier Date: Sat, 3 Oct 2026 05:02:59 +0000 Subject: [PATCH 1/6] feat(tasks/report): accept worker startedAt so AgentRun durations are real (#1120) Accept an optional ISO 8601 startedAt in the report body. Store it as the run's start time when it carries a timezone, parses, is not in the future, and is within 24h of the report; missing/malformed/future/too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload hash so an identical-body retry keeps duplicate-replay semantics across the 24h acceptance boundary. --- AGENTS.md | 2 + .../[agentName]/tasks/report/route.test.ts | 264 ++++++++++++++++++ .../agents/[agentName]/tasks/report/route.ts | 44 ++- 3 files changed, 307 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fce65a86..a193db16 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -317,6 +317,8 @@ The `tasks/report` endpoint accepts these outcomes: Queue-backed `followup-pr` reports must echo the `prFixItem.{id, generation}` token issued by `next-task` on the task. Reports without it still record the AgentRun, but never settle PR-fix queue state — the item stays queued and the report is a no-op against the queue (#1074). +The report body also accepts an optional `startedAt` — a full ISO 8601 timestamp with timezone (for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`) marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. + #### Idempotent reporting (optional) `tasks/report` accepts an optional `idempotencyKey` (opaque string, trimmed, at most 200 characters). When present, `(agentName, idempotencyKey)` uniquely identifies the logical report: a retried request with the same key and the same payload returns the original `agentRunId` (with `duplicate: true`) without creating another `AgentRun` or re-running PR-fix resolution — the stored resolution is replayed when it has been persisted, and while the original report is still completing (or if its persistence failed) the retry receives an explicit `action: "skipped"` resolution instead; side effects are never repeated either way. Reusing the same key with a different payload is rejected with `409`, as is a claim that exists without a recorded result (only possible if the referenced run was deleted). An unexpected persistence failure returns a structured `500` — a worker that retries after it lands in the duplicate branch above. If a referenced `AgentRun` is later deleted, its claim stays behind and further retries with that key return `409`; the recovery is for the worker to proceed under a fresh key, or for an operator to delete the stale `AgentReportDedupe` row. Reports that omit the key keep the current at-least-once behavior. Workers that may crash between reporting and recording the result locally should derive the key from a durable per-run identity (for example `":"`); Dispatch treats the value as fully opaque. Keys are persisted in Dispatch's database — do not embed secrets or personal data in them. diff --git a/src/app/api/agents/[agentName]/tasks/report/route.test.ts b/src/app/api/agents/[agentName]/tasks/report/route.test.ts index 15fb09f6..bc7285b5 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.test.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.test.ts @@ -730,6 +730,270 @@ describe("POST /api/agents/[agentName]/tasks/report — AgentRun persistence", ( }); }); +describe("POST /api/agents/[agentName]/tasks/report — startedAt (#1120)", () => { + beforeEach(() => { + delete process.env.DISPATCH_AUTH_MODE; + resetAuthCaches(); + vi.clearAllMocks(); + }); + + it("stores a valid worker-reported startedAt as the run start (duration > 0)", async () => { + const startInstant = new Date(Date.now() - 60 * 60 * 1000); + const startedAtIso = startInstant.toISOString(); + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: startedAtIso, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt).toBeInstanceOf(Date); + expect(call.startedAt.getTime()).toBe(startInstant.getTime()); + expect(call.finishedAt).toBeInstanceOf(Date); + expect(call.finishedAt.getTime()).toBeGreaterThanOrEqual(call.startedAt.getTime()); + expect(call.finishedAt.getTime() - call.startedAt.getTime()).toBeGreaterThan(0); + }); + + it("falls back to the report time when startedAt is missing (startedAt ≈ finishedAt)", async () => { + const before = Date.now(); + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + }); + const after = Date.now(); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt).toBeInstanceOf(Date); + expect(call.finishedAt).toBeInstanceOf(Date); + // Same instant: both collapse to the report time. + expect(call.startedAt.getTime()).toBe(call.finishedAt.getTime()); + expect(call.finishedAt.getTime()).toBeGreaterThanOrEqual(before); + expect(call.finishedAt.getTime()).toBeLessThanOrEqual(after); + }); + + it("falls back to the report time when startedAt is a malformed string", async () => { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: "not-a-date", + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt.getTime()).toBe(call.finishedAt.getTime()); + }); + + it("falls back to the report time when startedAt is a number (no 400)", async () => { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: 123, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt.getTime()).toBe(call.finishedAt.getTime()); + }); + + it("falls back to the report time when startedAt is an object (no 400)", async () => { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: {}, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt.getTime()).toBe(call.finishedAt.getTime()); + }); + + it("falls back to the report time when startedAt is an empty string", async () => { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: "", + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt.getTime()).toBe(call.finishedAt.getTime()); + }); + + it("falls back to the report time when startedAt is in the future", async () => { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt.getTime()).toBe(call.finishedAt.getTime()); + }); + + it("falls back to the report time when startedAt is older than 24h", async () => { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: new Date(Date.now() - 25 * 60 * 60 * 1000).toISOString(), + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt.getTime()).toBe(call.finishedAt.getTime()); + }); + + it("accepts a startedAt ~23h old (24h bound is not over-eager)", async () => { + const startInstant = new Date(Date.now() - 23 * 60 * 60 * 1000); + const startedAtIso = startInstant.toISOString(); + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: startedAtIso, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt.getTime()).toBe(startInstant.getTime()); + expect(call.finishedAt.getTime() - call.startedAt.getTime()).toBeGreaterThan(0); + }); + + it("echoes the normalized startedAt in the response for a valid value", async () => { + const startInstant = new Date(Date.now() - 60 * 60 * 1000); + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: startInstant.toISOString(), + }); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.report.startedAt).toBe(startInstant.toISOString()); + }); + + it("omits startedAt from the response when the value is invalid", async () => { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: "not-a-date", + }); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.report.startedAt).toBeUndefined(); + }); + + it("reports differing only by startedAt hash to distinct idempotency payload hashes", async () => { + const base = { + taskType: "implement", + outcome: "pr_opened", + idempotencyKey: "worker-run-1:report", + }; + const first = await postRequest({ + ...base, + startedAt: new Date(Date.now() - 60 * 60 * 1000).toISOString(), + }); + const second = await postRequest({ + ...base, + startedAt: new Date(Date.now() - 120 * 60 * 60 * 1000).toISOString(), + }); + + expect(first.status).toBe(200); + expect(second.status).toBe(200); + expect(mockDedupe.create).toHaveBeenCalledTimes(2); + const hashA = mockDedupe.create.mock.calls[0][0].data.payloadHash; + const hashB = mockDedupe.create.mock.calls[1][0].data.payloadHash; + expect(hashA).not.toBe(hashB); + }); + + it("falls back to the report time for malformed formats (bare-year, date-only, locale string)", async () => { + const malformedFormats = ["2026", "2026-10-02", "10/02/2026 12:00"]; + + for (const startedAt of malformedFormats) { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls.at(-1)![0].data; + expect(call.startedAt).toBeInstanceOf(Date); + expect(call.finishedAt).toBeInstanceOf(Date); + // Malformed → collapsed to the report time: within ~2s of finishedAt. + expect(Math.abs(call.startedAt.getTime() - call.finishedAt.getTime())).toBeLessThan(2000); + // And the response must not echo the malformed value. + const body = await res.json(); + expect(body.report.startedAt).toBeUndefined(); + } + }); + + it("accepts a python-isoformat offset (+00:00) as a valid startedAt", async () => { + const startInstant = new Date(Date.now() - 2 * 60 * 60 * 1000); + const pythonIso = startInstant.toISOString().replace("Z", "+00:00"); + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: pythonIso, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt).toBeInstanceOf(Date); + expect(call.startedAt.getTime()).toBe(startInstant.getTime()); + }); + + it("an identical-body retry keeps the same payload hash across the 24h boundary (duplicate, not 409)", async () => { + const T0 = new Date("2026-10-03T12:00:00Z").getTime(); + const startedAtIso = new Date(T0 - 23 * 60 * 60 * 1000).toISOString(); + const body = { + taskType: "implement", + outcome: "issue_updated", + idempotencyKey: "key-stability", + startedAt: startedAtIso, + }; + + vi.useFakeTimers({ now: T0 }); + try { + const first = await postRequest(body); + expect(first.status).toBe(200); + const payloadHash = mockDedupe.create.mock.calls[0][0].data.payloadHash; + + // 25h later: the same startedAt string is now 48h old and no longer + // accepted, so the validated report changes — but the RAW-body hash + // must stay the same, so the retry dedupes instead of 409ing. + vi.setSystemTime(T0 + 25 * 60 * 60 * 1000); + + mockDedupe.create.mockRejectedValueOnce( + new Prisma.PrismaClientKnownRequestError("Unique constraint failed", { + code: "P2002", + clientVersion: "test", + }), + ); + mockDedupe.findUnique.mockResolvedValueOnce({ + id: "claim-1", + agentName: "test-agent", + idempotencyKey: "key-stability", + payloadHash, + agentRunId: "run-1", + prFixResolution: null, + }); + + const retry = await postRequest(body); + + expect(retry.status).toBe(200); + expect(retry.status).not.toBe(409); + const retryBody = await retry.json(); + expect(retryBody.duplicate).toBe(true); + expect(retryBody.agentRunId).toBe("run-1"); + } finally { + vi.useRealTimers(); + } + }); +}); + describe("POST /api/agents/[agentName]/tasks/report — idempotencyKey", () => { beforeEach(() => { delete process.env.DISPATCH_AUTH_MODE; diff --git a/src/app/api/agents/[agentName]/tasks/report/route.ts b/src/app/api/agents/[agentName]/tasks/report/route.ts index b14f8226..9c734b31 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.ts @@ -32,6 +32,11 @@ export interface TaskReportBody { error?: string; // #1121: evidence string (commit SHAs / paths) carried by an already_addressed report; recorded in settlement history. evidence?: string; + // #1120: worker-reported task start time (ISO 8601). Accepted only when + // parseable, not in the future, and within 24h of the report; otherwise + // omitted and the report time is used. It participates in the idempotency + // payload hash like the other fields. + startedAt?: string; // The (id, generation) attempt token next-task issued on this followup-pr // task, echoed back by the worker (#1074). Required for PR-fix queue // settlement; part of the report's payload identity (idempotency hash). @@ -44,6 +49,30 @@ function deriveStatus(outcome: ValidOutcome): string { return "completed"; } +// #1120: optional worker-reported start time. Accepted only when it is a +// string that parses to a real date, is not in the future, and is no more +// than MAX_STARTED_AT_AGE_MS before the report; anything else silently falls +// back to the report time so a clock-skewed or bogus value can never produce +// a negative or absurd duration. +const MAX_STARTED_AT_AGE_MS = 24 * 60 * 60 * 1000; + +// Require a full ISO 8601 timestamp with timezone (Z or ±HH:MM), e.g. +// 2026-10-03T04:20:58Z or 2026-10-03T04:20:58.123456+00:00 (Python isoformat). +// Date-only, bare-year, and locale-ambiguous strings are malformed → fallback. +const ISO_8601_TIMESTAMP_PATTERN = + /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2}(\.\d+)?)?(Z|[+-]\d{2}:\d{2})$/; + +function resolveReportedStartedAt(rawStartedAt: unknown, now: Date): string | undefined { + if (typeof rawStartedAt !== "string") return undefined; + if (!ISO_8601_TIMESTAMP_PATTERN.test(rawStartedAt)) return undefined; + const parsed = new Date(rawStartedAt); + const time = parsed.getTime(); + if (Number.isNaN(time)) return undefined; + if (time > now.getTime()) return undefined; + if (now.getTime() - time > MAX_STARTED_AT_AGE_MS) return undefined; + return parsed.toISOString(); +} + async function resolveIssueId( repoFullName: string | undefined, issueNumber: number | undefined, @@ -207,6 +236,10 @@ export async function POST( } : undefined; + // #1120: computed once before the report so the same instant is used for + // startedAt validation and finishedAt. + const now = new Date(); + const report: TaskReportBody = { taskType: taskType as ValidTaskType, outcome: outcome as ValidOutcome, @@ -217,6 +250,8 @@ export async function POST( summary: raw.summary as string | undefined, error: raw.error as string | undefined, evidence, + // #1120: normalized worker start time; undefined → report time (now). + startedAt: resolveReportedStartedAt(raw.startedAt, now), prFixItem, }; @@ -228,12 +263,12 @@ export async function POST( const touchedIssueUrls = buildTouchedUrls(report); // Persist AgentRun - const now = new Date(); const runData = { agentName, runType: report.taskType, status: deriveStatus(report.outcome), - startedAt: now, + // #1120: worker-reported start time when valid, else the report time. + startedAt: report.startedAt ? new Date(report.startedAt) : now, finishedAt: now, summary: report.summary, errorMessage: report.error, @@ -251,7 +286,10 @@ export async function POST( // concurrent or retried report with the same (agentName, idempotencyKey) // loses the unique-index race (P2002) and replays the stored result // instead of re-running the report or its side effects. - const payloadHash = reportPayloadHash(report); + // #1120/#1044: hash the RAW body value, not the time-validated one, so an + // identical-body retry keeps the same payload identity even when + // startedAt's acceptance flips across the 24h boundary between attempts. + const payloadHash = reportPayloadHash({ ...report, startedAt: raw.startedAt as string | undefined }); try { run = await prisma.$transaction(async (tx) => { const claim = await tx.agentReportDedupe.create({ From 068ec8b0b965558669d4084d3063e5be02cdcae9 Mon Sep 17 00:00:00 2001 From: Courier Date: Sat, 3 Oct 2026 06:36:47 +0000 Subject: [PATCH 2/6] test(tasks/report): pin +05:30 conversion and inclusive 24h bound; drop hash cast (#1120) Independent review of #1168 found three minors: no non-zero-offset test (TZ conversion exercised only via +00:00), the 24h bound's inclusive > semantics unpinned at exactly 24h, and a lying `as string | undefined` cast on the raw hash input. Add the two tests (the boundary one under fake timers so it is not race-prone), widen reportPayloadHash to accept startedAt?: unknown so no cast is needed (canonicalJson already takes unknown; hash output byte-identical), and note UTC normalization of the stored/echoed startedAt in AGENTS.md. --- AGENTS.md | 2 +- .../[agentName]/tasks/report/route.test.ts | 44 +++++++++++++++++++ .../agents/[agentName]/tasks/report/route.ts | 8 ++-- 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a193db16..c1bcc394 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -317,7 +317,7 @@ The `tasks/report` endpoint accepts these outcomes: Queue-backed `followup-pr` reports must echo the `prFixItem.{id, generation}` token issued by `next-task` on the task. Reports without it still record the AgentRun, but never settle PR-fix queue state — the item stays queued and the report is a no-op against the queue (#1074). -The report body also accepts an optional `startedAt` — a full ISO 8601 timestamp with timezone (for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`) marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. +The report body also accepts an optional `startedAt` — a full ISO 8601 timestamp with timezone (for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`) marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. An accepted `startedAt` is normalized to UTC (`Z`) form when stored and echoed back in the response. #### Idempotent reporting (optional) diff --git a/src/app/api/agents/[agentName]/tasks/report/route.test.ts b/src/app/api/agents/[agentName]/tasks/report/route.test.ts index bc7285b5..1b43f21b 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.test.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.test.ts @@ -945,6 +945,50 @@ describe("POST /api/agents/[agentName]/tasks/report — startedAt (#1120)", () = expect(call.startedAt.getTime()).toBe(startInstant.getTime()); }); + it("accepts a non-zero UTC offset (+05:30) and stores the normalized UTC instant", async () => { + const startInstant = new Date(Date.now() - 2 * 60 * 60 * 1000); + // Same instant rendered in a +05:30 zone: shift the UTC clock by the + // offset and tag the reading with it. + const shifted = new Date(startInstant.getTime() + 5.5 * 60 * 60 * 1000); + const startedAt = shifted.toISOString().replace("Z", "+05:30"); + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt).toBeInstanceOf(Date); + // Proves TZ conversion, not just acceptance: the stored instant equals + // the UTC reading, not the +05:30 clock reading. + expect(call.startedAt.toISOString()).toBe(startInstant.toISOString()); + }); + + it("accepts a startedAt exactly 24h old (24h bound is inclusive)", async () => { + const T0 = new Date("2026-10-03T12:00:00Z").getTime(); + const startedAtIso = new Date(T0 - 24 * 60 * 60 * 1000).toISOString(); + + vi.useFakeTimers({ now: T0 }); + try { + const res = await postRequest({ + taskType: "implement", + outcome: "pr_opened", + startedAt: startedAtIso, + }); + + expect(res.status).toBe(200); + const call = mockAgentRun.create.mock.calls[0][0].data; + expect(call.startedAt).toBeInstanceOf(Date); + expect(call.startedAt.getTime()).toBe(T0 - 24 * 60 * 60 * 1000); + // Accepted: the run kept the worker-reported start, not the report + // time (a fallback would collapse startedAt onto finishedAt). + expect(call.startedAt.getTime()).not.toBe(call.finishedAt.getTime()); + } finally { + vi.useRealTimers(); + } + }); + it("an identical-body retry keeps the same payload hash across the 24h boundary (duplicate, not 409)", async () => { const T0 = new Date("2026-10-03T12:00:00Z").getTime(); const startedAtIso = new Date(T0 - 23 * 60 * 60 * 1000).toISOString(); diff --git a/src/app/api/agents/[agentName]/tasks/report/route.ts b/src/app/api/agents/[agentName]/tasks/report/route.ts index 9c734b31..b37a6035 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.ts @@ -126,8 +126,10 @@ function canonicalJson(value: unknown): string { return `{${entries.map(([k, v]) => `${JSON.stringify(k)}:${canonicalJson(v)}`).join(",")}}`; } -function reportPayloadHash(report: TaskReportBody): string { - return createHash("sha256").update(canonicalJson(report)).digest("hex"); +function reportPayloadHash( + payload: Omit & { startedAt?: unknown }, +): string { + return createHash("sha256").update(canonicalJson(payload)).digest("hex"); } function isUniqueViolation(error: unknown): boolean { @@ -289,7 +291,7 @@ export async function POST( // #1120/#1044: hash the RAW body value, not the time-validated one, so an // identical-body retry keeps the same payload identity even when // startedAt's acceptance flips across the 24h boundary between attempts. - const payloadHash = reportPayloadHash({ ...report, startedAt: raw.startedAt as string | undefined }); + const payloadHash = reportPayloadHash({ ...report, startedAt: raw.startedAt }); try { run = await prisma.$transaction(async (tx) => { const claim = await tx.agentReportDedupe.create({ From c506e1e94850859a4f207895a14dc0d2b99c9725 Mon Sep 17 00:00:00 2001 From: Courier Date: Sat, 3 Oct 2026 05:44:20 +0000 Subject: [PATCH 3/6] fix(ci): pass --include=optional in audit script so --omit=dev survives .npmrc include=dev (#1162) npm gives include=dev precedence over --omit=dev even on the command line, so npm run audit evaluated the dev tree and failed on the dev-only, unfixable braces * advisory GHSA-vfj7-8cjw-p6xm via eslint-config-next. The CLI --include=optional replaces the project-level include list, restoring a prod+optional-only audit. --- .npmrc | 1 + SECURITY-ACCEPTED-RISKS.md | 7 ++++--- package.json | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.npmrc b/.npmrc index 7a4a3138..092987f9 100644 --- a/.npmrc +++ b/.npmrc @@ -1,4 +1,5 @@ # Ensure reproducible local validation: always install devDependencies. # Overrides global `omit=dev` so `npm ci` installs everything needed # for `npm run typecheck` and `npm run test` to work from a clean checkout. +# CAUTION: include=dev overrides --omit=dev (npm precedence), which is why scripts.audit passes --include=optional. See SECURITY-ACCEPTED-RISKS.md and issue #1162. include=dev diff --git a/SECURITY-ACCEPTED-RISKS.md b/SECURITY-ACCEPTED-RISKS.md index d198710c..cd7184a1 100644 --- a/SECURITY-ACCEPTED-RISKS.md +++ b/SECURITY-ACCEPTED-RISKS.md @@ -1,10 +1,10 @@ # Accepted Security Risks -**Last updated: 2026-08-15** +**Last updated: 2026-10-03** There are currently no accepted npm runtime advisories. -`npm audit --omit=dev` reports **0 vulnerabilities** across 17 production dependencies. +`npm run audit` (`npm audit --omit=dev --include=optional`, plus fetch-retry flags; see Previous Resolution History for why `--include=optional` is required) reports **0 vulnerabilities** across 17 production dependencies. ## Non-NPM Risks @@ -29,7 +29,7 @@ The following risks are tracked beyond npm advisories: - The project uses 17 production dependencies with transitive chains managed by npm. - Key deep-chain dependencies: `next` (framework), `@modelcontextprotocol/sdk` (MCP protocol), `prisma` / `@prisma/client` (ORM). -- **Mitigation:** Renovate keeps dependencies updated; `npm audit --omit=dev --audit-level=high` runs on every push to `main` and every pull request via `.github/workflows/security-audit.yaml` (separate from the main CI workflow) and fails the build on high/critical vulnerabilities. +- **Mitigation:** Renovate keeps dependencies updated; `npm run audit` (`npm audit --omit=dev --include=optional --audit-level=high`) runs on every push to `main` and every pull request via `.github/workflows/security-audit.yaml` (separate from the main CI workflow) and fails the build on high/critical vulnerabilities. ### Groomer Autonomous Issue Rewrites (accepted risk) @@ -60,3 +60,4 @@ The following previously accepted risks have been retired: |---|---|---| | Trivy action pinned to SHA | ✅ Resolved | `aquasecurity/trivy-action@ed142fd` (v0.36.0). The SHA pin is intentional: trivy is the release gate, so a floating tag must not reach a release build. Renovate's `github-tags` datasource cannot resolve a bare SHA pin (it only produced a `no-result` lookup failure on the dashboard), so the action is excluded from Renovate in `renovate.json` (`matchPackageNames: ["aquasecurity/trivy-action"]`, `enabled: false`) and is bumped manually, with the version comment, after reviewing an upstream release. | | `.npmrc` invalid omit config | ✅ Resolved | Fixed `omit=` → `omit=dev` | +| `.npmrc` `include=dev` neutralized `npm audit --omit=dev` (CI Security Audit red, #1162) | ✅ Resolved | `scripts.audit` now passes `--include=optional`, replacing the project-level include list on the CLI so the audit covers prod + optional deps only. The `braces *` advisory (GHSA-vfj7-8cjw-p6xm) is dev-only (`eslint-config-next` chain), has no patched version, and is intentionally out of scope for the production audit. Do NOT revert `.npmrc` to `omit=`: npm 11 flags the empty value with an invalid-config warning on every command. | diff --git a/package.json b/package.json index 248b0ef4..e309878d 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "dev": "next dev", "build": "NODE_ENV=production next build", "start": "next start", - "audit": "npm audit --omit=dev --audit-level=high --fetch-retries=5 --fetch-timeout=120000 --fetch-retry-mintimeout=20000 --fetch-retry-maxtimeout=120000", + "audit": "npm audit --omit=dev --include=optional --audit-level=high --fetch-retries=5 --fetch-timeout=120000 --fetch-retry-mintimeout=20000 --fetch-retry-maxtimeout=120000", "lint": "NODE_ENV=development eslint .", "test": "NODE_ENV=development vitest run", "test:watch": "NODE_ENV=development vitest", @@ -63,7 +63,8 @@ "vitest": "^5.0.0" }, "//": { - "overrides": "These pins exist to remediate npm advisories (originally added in #350). DO NOT remove without verifying the originating transitive deps have shipped patched versions: postcss ^8.5.10 (XSS-class advisory), sharp ^0.35.0 (libvips CVE-2026-33327/33328/35590/35591, #675 — already satisfied transitively by next@16.3.0's optional dep `sharp: ^0.35.3`, but the override survives any future `next` downgrade), deepmerge-ts ^8.0.0 (GHSA-ggr8-5vv4-36mx stack-exhaustion — pulled in by @prisma/config <=6.13.0-dev.1 and the transitive prisma <=7.10.0-integration-fix-prisma-publish-token.1, #761), mysql2 ^3.22.0 (GHSA-3f6p-5ww8-9rcr plaintext-credential leak via mysql_clear_password auth-plugin downgrade — pulled in by the transitive prisma >=6.20.0-dev.1 / <=7.10.0, #897). npm validates every entry of `overrides` strictly, so this rationale lives at the top level rather than inside the `overrides` block." + "overrides": "These pins exist to remediate npm advisories (originally added in #350). DO NOT remove without verifying the originating transitive deps have shipped patched versions: postcss ^8.5.10 (XSS-class advisory), sharp ^0.35.0 (libvips CVE-2026-33327/33328/35590/35591, #675 — already satisfied transitively by next@16.3.0's optional dep `sharp: ^0.35.3`, but the override survives any future `next` downgrade), deepmerge-ts ^8.0.0 (GHSA-ggr8-5vv4-36mx stack-exhaustion — pulled in by @prisma/config <=6.13.0-dev.1 and the transitive prisma <=7.10.0-integration-fix-prisma-publish-token.1, #761), mysql2 ^3.22.0 (GHSA-3f6p-5ww8-9rcr plaintext-credential leak via mysql_clear_password auth-plugin downgrade — pulled in by the transitive prisma >=6.20.0-dev.1 / <=7.10.0, #897). npm validates every entry of `overrides` strictly, so this rationale lives at the top level rather than inside the `overrides` block.", + "audit": "The --include=optional flag exists because .npmrc sets include=dev (so npm ci installs devDependencies even when a developer's global npm config omits them), and npm gives include=dev precedence over --omit=dev even on the command line. Without this flag, npm run audit evaluated the dev tree and failed on the dev-only, unfixable braces * advisory GHSA-vfj7-8cjw-p6xm (eslint-config-next -> @next/eslint-plugin-next -> fast-glob -> micromatch -> braces, issue #1162). Passing --include=optional on the CLI replaces the project-level include list, restoring a prod+optional-only audit with no invalid-config warnings." }, "overrides": { "postcss": "^8.5.10", From 23f976f634f126f6bc365f0aa64b16d5142f9f22 Mon Sep 17 00:00:00 2001 From: Courier Date: Sun, 4 Oct 2026 08:21:18 +0000 Subject: [PATCH 4/6] docs(agents): clarify startedAt echo on idempotent duplicate replay (#1120) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index c1bcc394..78ddb71c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -317,7 +317,7 @@ The `tasks/report` endpoint accepts these outcomes: Queue-backed `followup-pr` reports must echo the `prFixItem.{id, generation}` token issued by `next-task` on the task. Reports without it still record the AgentRun, but never settle PR-fix queue state — the item stays queued and the report is a no-op against the queue (#1074). -The report body also accepts an optional `startedAt` — a full ISO 8601 timestamp with timezone (for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`) marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. An accepted `startedAt` is normalized to UTC (`Z`) form when stored and echoed back in the response. +The report body also accepts an optional `startedAt` — a full ISO 8601 timestamp with timezone (for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`) marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. An accepted `startedAt` is normalized to UTC (`Z`) form when stored and echoed back in the response. The echo reflects re-validation of the current request, so on an idempotent duplicate replay the echoed `startedAt` may be absent even though the original accepted run stored it — for example when an identical-body retry lands more than 24 hours after the reported start (the stored AgentRun is unaffected). #### Idempotent reporting (optional) From f43bcd7fe615da0c25cc09abb2ee42c4ec8a6d74 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 5 Oct 2026 07:40:33 +0000 Subject: [PATCH 5/6] docs(agents): match startedAt contract wording to accepted extended-format grammar (#1120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Human review nit (joryirving) on PR #1168: 'a full ISO 8601 timestamp with timezone' is broader than the accepted grammar. ISO_8601_TIMESTAMP_PATTERN accepts extended format only with Z or ±HH:MM offsets; basic-form offsets (+0000) fall back silently. Reword to 'an extended ISO 8601 timestamp with timezone (Z or ±HH:MM)'. Docs-only; no behavior change. --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 78ddb71c..f5026838 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -317,7 +317,7 @@ The `tasks/report` endpoint accepts these outcomes: Queue-backed `followup-pr` reports must echo the `prFixItem.{id, generation}` token issued by `next-task` on the task. Reports without it still record the AgentRun, but never settle PR-fix queue state — the item stays queued and the report is a no-op against the queue (#1074). -The report body also accepts an optional `startedAt` — a full ISO 8601 timestamp with timezone (for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`) marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. An accepted `startedAt` is normalized to UTC (`Z`) form when stored and echoed back in the response. The echo reflects re-validation of the current request, so on an idempotent duplicate replay the echoed `startedAt` may be absent even though the original accepted run stored it — for example when an identical-body retry lands more than 24 hours after the reported start (the stored AgentRun is unaffected). +The report body also accepts an optional `startedAt` — an extended ISO 8601 timestamp with timezone (`Z` or `±HH:MM`), for example `2026-10-03T04:20:58Z` or `2026-10-03T04:20:58.123456+00:00`, marking when the worker began the task. When it parses, is not in the future, and is no more than 24 hours before the report, it is stored as the run's start time so the run records a real duration; missing, malformed, future, or too-old values silently fall back to the report time (never a 400). The raw body value participates in the idempotency payload identity, so an identical-body retry always replays as a duplicate. An accepted `startedAt` is normalized to UTC (`Z`) form when stored and echoed back in the response. The echo reflects re-validation of the current request, so on an idempotent duplicate replay the echoed `startedAt` may be absent even though the original accepted run stored it — for example when an identical-body retry lands more than 24 hours after the reported start (the stored AgentRun is unaffected). #### Idempotent reporting (optional) From a0ce64748a7f99f24b83c3a2e7eca267167c8773 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 5 Oct 2026 08:48:22 +0000 Subject: [PATCH 6/6] docs(report): use 'extended ISO 8601' in code comment to match accepted grammar (#1120) --- src/app/api/agents/[agentName]/tasks/report/route.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/api/agents/[agentName]/tasks/report/route.ts b/src/app/api/agents/[agentName]/tasks/report/route.ts index b37a6035..a47a7326 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.ts @@ -56,7 +56,7 @@ function deriveStatus(outcome: ValidOutcome): string { // a negative or absurd duration. const MAX_STARTED_AT_AGE_MS = 24 * 60 * 60 * 1000; -// Require a full ISO 8601 timestamp with timezone (Z or ±HH:MM), e.g. +// Require an extended ISO 8601 timestamp with timezone (Z or ±HH:MM), e.g. // 2026-10-03T04:20:58Z or 2026-10-03T04:20:58.123456+00:00 (Python isoformat). // Date-only, bare-year, and locale-ambiguous strings are malformed → fallback. const ISO_8601_TIMESTAMP_PATTERN =