fix(web): bind digest approval to the reviewed draft; exclusive per-request claims

Independent review of 1d83478bd held two P1s.

Approval binding: approveDigestRecord took only weekId/lang and approved
whatever record sat at the key, so draft B + record A (B's record put
failed after its draft put) posted B to GitHub and published A. The route
also reloaded and posted current storage, so a draft regenerated after the
admin page loaded was posted unseen.
- The admin page sends reviewedSha256 (SHA-256 of the text shown in the
  selected language) with every post/discard; the route answers 409 before
  any claim or GitHub call when the stored draft no longer hashes to it.
- approveDigestRecord(kv, draft, lang) publishes only a well-formed record
  of the same weekId and generatedAt that re-renders (renderDigestBody, now
  the single renderer shared with runDigest) to exactly the reviewed text,
  copying fields explicitly in one put. Otherwise it stays unpublished and
  the maintainer gets a mismatch/missing warning.

Claim serialization: the token claim overwrote one shared marker key, so
two handlers that both passed the absence check could each write and read
back their own token and both post. Claims now live in per-request keys
(draft-claim:<type>:<id>:<uuid>); a request proceeds only if a list shows
its key alone and no decision marker exists, otherwise it deletes its key
and answers 409. Overlapping claimants can both refuse, never both proceed.
Discard takes the same claim, so a discard cannot overlap a post. The
claim is released on a definite GitHub rejection or once the posted marker
is written; on an unknown outcome it is kept until it expires. The code
comment states the limit: this relies on a list seeing finished puts, which
Workers KV guarantees within one location, not across locations.

Tests: community-agent-review-state, content-watch, community-agent-security,
public-api-security: 4 files, 51 passed. Against the previous source 6 of
the new tests fail (draft B/record A publication, explicit-field publish,
barrier race post/post, post/discard overlap, stale reviewed text, missing
hash). tsc --noEmit exit 0; eslint on touched files exit 0.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
This commit is contained in:
Hunter BandClaude Opus 5.5 committed 2026-09-29 11:26:21 -07:00
1 parent 1d83478bd3
commit 989081cb83
5 files changed
+411 -74

No files matched your search

+10 -6
View File
@@ -1,7 +1,7 @@
"use client";
import { useState } from "react";
import { draftStorageKey, type AgentDraft } from "@/lib/community-agent";
import { draftStorageKey, reviewedBodyHash, type AgentDraft } from "@/lib/community-agent";
interface Props {
drafts: AgentDraft[];
@@ -17,13 +17,17 @@ export function AdminClient({ drafts, posted, isZh, typeLabels }: Props) {
const [editBody, setEditBody] = useState("");
const [loading, setLoading] = useState<string | null>(null);
const handleAction = async (draftKey: string, action: "post" | "discard", editedBody?: string) => {
const handleAction = async (draft: AgentDraft, action: "post" | "discard", editedBody?: string) => {
const draftKey = draftStorageKey(draft);
setLoading(draftKey);
try {
// Binds the action to the text shown here; the route refuses it if the
// stored draft was regenerated since this page loaded.
const reviewedSha256 = await reviewedBodyHash(isZh ? draft.bodyZh : draft.bodyEn);
const res = await fetch("/api/admin/post", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ action, draftKey, editedBody, lang: isZh ? "zh" : "en" }),
body: JSON.stringify({ action, draftKey, editedBody, lang: isZh ? "zh" : "en", reviewedSha256 }),
});
const data = await res.json();
if (data.ok) {
@@ -93,7 +97,7 @@ export function AdminClient({ drafts, posted, isZh, typeLabels }: Props) {
/>
<div className="flex gap-2">
<button
onClick={() => handleAction(key, "post", editBody)}
onClick={() => handleAction(draft, "post", editBody)}
disabled={loading === key}
className="btn btn-primary disabled:opacity-50"
>
@@ -114,7 +118,7 @@ export function AdminClient({ drafts, posted, isZh, typeLabels }: Props) {
</div>
<div className="flex gap-2 flex-wrap">
<button
onClick={() => handleAction(key, "post")}
onClick={() => handleAction(draft, "post")}
disabled={loading === key}
className="btn btn-primary disabled:opacity-50"
>
@@ -127,7 +131,7 @@ export function AdminClient({ drafts, posted, isZh, typeLabels }: Props) {
{isZh ? "编辑后发布" : "Edit & post"}
</button>
<button
onClick={() => handleAction(key, "discard")}
onClick={() => handleAction(draft, "discard")}
disabled={loading === key}
className="btn btn-danger"
>
+55 -9
View File
@@ -3,7 +3,7 @@ import { NextResponse } from "next/server";
import { BodyReadError, readBoundedBody } from "@/lib/bounded-body";
import {
approveDigestRecord,
claimDraftForPosting,
claimDraft,
clearDraftResolution,
deleteDigestRecord,
deleteDraft,
@@ -12,8 +12,11 @@ import {
getDraftResolution,
markDraftResolved,
parseDraftKey,
releaseDraftClaim,
reviewedBodyHash,
validateSession,
type CommunityAgentEnv,
type DraftClaim,
} from "@/lib/community-agent";
export const dynamic = "force-dynamic";
@@ -93,11 +96,12 @@ export async function POST(req: Request) {
if (!body || typeof body !== "object" || Array.isArray(body)) {
return NextResponse.json({ error: "invalid JSON body" }, { status: 400 });
}
const { action, draftKey, editedBody, lang } = body as {
const { action, draftKey, editedBody, lang, reviewedSha256 } = body as {
action?: unknown;
draftKey?: unknown;
editedBody?: unknown;
lang?: unknown;
reviewedSha256?: unknown;
};
if (typeof action !== "string" || !ALLOWED_ACTIONS.has(action)) {
@@ -119,11 +123,24 @@ export async function POST(req: Request) {
if (lang !== undefined && lang !== "en" && lang !== "zh") {
return NextResponse.json({ error: "invalid lang" }, { status: 400 });
}
if (typeof reviewedSha256 !== "string" || !/^[0-9a-f]{64}$/.test(reviewedSha256)) {
return NextResponse.json({ error: "missing or invalid reviewedSha256" }, { status: 400 });
}
const reviewLang = lang === "zh" ? "zh" : "en";
const draft = await getDraft(env.CURATED_KV, draftKey);
if (!draft) {
return NextResponse.json({ error: "draft not found" }, { status: 404 });
}
// Act only on the exact text the maintainer was shown. A draft regenerated
// after the admin page loaded must be reviewed again, not posted unseen.
const originalBody = reviewLang === "zh" ? draft.bodyZh : draft.bodyEn;
if ((await reviewedBodyHash(originalBody)) !== reviewedSha256) {
return NextResponse.json(
{ error: "draft changed since it was loaded; reload and review it again" },
{ status: 409 }
);
}
if (action === "discard") {
// A posted draft is already public on GitHub (and, for a digest, on
@@ -132,6 +149,17 @@ export async function POST(req: Request) {
if (draft.posted || resolution?.state === "posted" || resolution?.state === "posting") {
return NextResponse.json({ error: "draft already posted" }, { status: 409 });
}
// Hold the same claim a post takes, so a discard and a post of one draft
// cannot both run.
let claim: DraftClaim | null = null;
try {
claim = await claimDraft(env.CURATED_KV, parsedKey.type, parsedKey.id);
} catch (e) {
return NextResponse.json({ error: `could not claim draft: ${String(e)}` }, { status: 500 });
}
if (!claim) {
return NextResponse.json({ error: "draft is being posted or was already resolved" }, { status: 409 });
}
try {
// The marker stops the next cron run from regenerating this draft.
await markDraftResolved(env.CURATED_KV, parsedKey.type, parsedKey.id, "discarded");
@@ -141,6 +169,9 @@ export async function POST(req: Request) {
}
} catch (e) {
return NextResponse.json({ error: `discard failed: ${String(e)}` }, { status: 500 });
} finally {
// The marker (or, if it failed, the draft) now carries the decision.
await releaseDraftClaim(env.CURATED_KV, claim).catch(() => undefined);
}
if (draft.type === "digest") revalidateDigestPage();
return NextResponse.json({ ok: true, action: "discarded" });
@@ -164,15 +195,20 @@ export async function POST(req: Request) {
if (resolution) {
return NextResponse.json({ error: `draft already ${resolution.state}` }, { status: 409 });
}
let claim: DraftClaim | null = null;
try {
if (!(await claimDraftForPosting(env.CURATED_KV, parsedKey.type, parsedKey.id))) {
return NextResponse.json({ error: "draft already posting" }, { status: 409 });
claim = await claimDraft(env.CURATED_KV, parsedKey.type, parsedKey.id);
if (!claim) {
return NextResponse.json({ error: "draft already posting or resolved" }, { status: 409 });
}
// Keeps the cron from regenerating the draft while the post is in flight.
await markDraftResolved(env.CURATED_KV, parsedKey.type, parsedKey.id, "posting");
} catch (e) {
if (claim) await releaseDraftClaim(env.CURATED_KV, claim).catch(() => undefined);
return NextResponse.json({ error: `could not claim draft: ${String(e)}` }, { status: 500 });
}
const heldClaim = claim;
const originalBody = lang === "zh" ? draft.bodyZh : draft.bodyEn;
const commentBody = editedBody ?? originalBody;
// After GitHub accepted the post, bookkeeping failures must not turn into
@@ -180,6 +216,9 @@ export async function POST(req: Request) {
const recordPosted = async (): Promise<string | undefined> => {
try {
await markDraftResolved(env.CURATED_KV, parsedKey.type, parsedKey.id, "posted");
// The marker now carries the decision, so a later claim sees it; a
// reopened draft (new activity clears the marker) is postable again.
await releaseDraftClaim(env.CURATED_KV, heldClaim).catch(() => undefined);
await env.CURATED_KV?.put(draftKey, JSON.stringify(draft), { expirationTtl: 60 * 60 * 24 * 7 });
return undefined;
} catch (e) {
@@ -218,6 +257,7 @@ export async function POST(req: Request) {
}
try {
await clearDraftResolution(env.CURATED_KV, parsedKey.type, parsedKey.id);
await releaseDraftClaim(env.CURATED_KV, heldClaim);
} catch { /* the claim expires on its own */ }
return NextResponse.json({ error: `GitHub ${res.status}: ${text}` }, { status: 502 });
};
@@ -242,17 +282,23 @@ export async function POST(req: Request) {
let warning = await recordPosted();
// Publishing to /digest is the approval, for the language shown to the
// maintainer only. The structured record holds the unedited model
// text, so an edited digest is posted to GitHub but not published there.
// maintainer only, and only of the record that renders to exactly the
// text they reviewed. An edited digest is posted to GitHub but not
// published there, since the record holds the unedited text.
let published = false;
if (editedBody === undefined || editedBody === originalBody) {
try {
published = await approveDigestRecord(env.CURATED_KV, draft.id, lang === "zh" ? "zh" : "en");
const approval = await approveDigestRecord(env.CURATED_KV, draft, reviewLang);
published = approval === "published";
if (published) revalidateDigestPage();
else if (approval === "missing") {
warning ??= "Posted to GitHub, but the weekly record is gone, so /digest does not show it.";
} else {
warning ??= "Posted to GitHub, but the stored weekly record does not match the reviewed text, so /digest does not show it.";
}
} catch (e) {
warning ??= `Posted to GitHub, but publishing the digest page failed (${String(e)}).`;
}
if (!published) warning ??= "Posted to GitHub, but the weekly record is gone, so /digest does not show it.";
} else {
warning ??= "Posted to GitHub. The text was edited, so /digest does not show this week.";
}
+211 -21
View File
@@ -27,6 +27,7 @@ import {
isPublishedDigest,
listDrafts,
MAX_LISTED_DRAFTS,
reviewedBodyHash,
saveDraft,
type AgentDraft,
} from "./community-agent";
@@ -40,6 +41,8 @@ class FakeKv {
yieldEachOp = false;
pageSize = 1000;
failListAtCursor: string | null = null;
/** Awaited before a put lands, to hold a request at an exact step. */
beforePut: ((key: string, value: string) => Promise<void>) | null = null;
private async tick() {
if (this.yieldEachOp) await new Promise((resolve) => setTimeout(resolve, 0));
@@ -52,6 +55,7 @@ class FakeKv {
async put(key: string, value: string): Promise<void> {
await this.tick();
if (this.beforePut) await this.beforePut(key, value);
if (this.failPutsMatching?.test(key)) throw new Error("kv put failed");
this.values.set(key, value);
}
@@ -118,6 +122,22 @@ function postBody(value: Record<string, unknown>): string {
return JSON.stringify(value);
}
/**
* An admin action as the admin page sends it: bound to the stored draft text
* in the selected language, as shown to the maintainer.
*/
async function act(kv: FakeKv, fields: Record<string, unknown>): Promise<Response> {
let reviewedSha256 = fields.reviewedSha256;
if (reviewedSha256 === undefined) {
const raw = kv.values.get(String(fields.draftKey));
const stored = raw ? (JSON.parse(raw) as AgentDraft) : null;
reviewedSha256 = await reviewedBodyHash(
stored ? (fields.lang === "zh" ? stored.bodyZh : stored.bodyEn) : ""
);
}
return adminPost(adminRequest(postBody({ ...fields, reviewedSha256 })));
}
function useAdminEnv(kv: FakeKv) {
mocks.getAgentEnv.mockResolvedValue({
CURATED_KV: kv,
@@ -196,7 +216,7 @@ describe("weekly digest publication requires maintainer approval", () => {
useAdminEnv(kv);
const posts = stubGitHub();
const draftKey = onlyKey(kv, "draft:digest:");
const res = await adminPost(adminRequest(postBody({ action: "post", draftKey, lang: "en" })));
const res = await act(kv, { action: "post", draftKey, lang: "en" });
await expect(res.json()).resolves.toMatchObject({ ok: true, published: true, number: 900 });
expect(posts).toHaveLength(1);
const published = JSON.parse(kv.values.get(recordKey)!);
@@ -214,7 +234,7 @@ describe("weekly digest publication requires maintainer approval", () => {
useAdminEnv(kv);
stubGitHub();
const draftKey = onlyKey(kv, "draft:digest:");
const res = await adminPost(adminRequest(postBody({ action: "post", draftKey, lang: "zh" })));
const res = await act(kv, { action: "post", draftKey, lang: "zh" });
await expect(res.json()).resolves.toMatchObject({ ok: true, published: true });
expect(JSON.parse(kv.values.get(onlyKey(kv, "digest:weekly-"))!)).toMatchObject({ approvedLang: "zh" });
});
@@ -228,9 +248,9 @@ describe("weekly digest publication requires maintainer approval", () => {
useAdminEnv(kv);
stubGitHub();
const draftKey = onlyKey(kv, "draft:digest:");
expect((await adminPost(adminRequest(postBody({ action: "post", draftKey, lang: "en" })))).status).toBe(200);
expect((await act(kv, { action: "post", draftKey, lang: "en" })).status).toBe(200);
const discard = await adminPost(adminRequest(postBody({ action: "discard", draftKey })));
const discard = await act(kv, { action: "discard", draftKey });
expect(discard.status).toBe(409);
expect(isPublishedDigest(JSON.parse(kv.values.get(onlyKey(kv, "digest:weekly-"))!))).toBe(true);
});
@@ -244,7 +264,7 @@ describe("weekly digest publication requires maintainer approval", () => {
useAdminEnv(kv);
stubGitHub();
const draftKey = onlyKey(kv, "draft:digest:");
const res = await adminPost(adminRequest(postBody({ action: "post", draftKey, lang: "en", editedBody: "# Edited" })));
const res = await act(kv, { action: "post", draftKey, lang: "en", editedBody: "# Edited" });
// The maintainer is told why the digest is not on /digest.
await expect(res.json()).resolves.toMatchObject({
ok: true,
@@ -262,7 +282,7 @@ describe("weekly digest publication requires maintainer approval", () => {
useAdminEnv(kv);
const draftKey = onlyKey(kv, "draft:digest:");
const res = await adminPost(adminRequest(postBody({ action: "discard", draftKey })));
const res = await act(kv, { action: "discard", draftKey });
await expect(res.json()).resolves.toMatchObject({ ok: true, action: "discarded" });
expect([...kv.values.keys()].filter((k) => k.startsWith("digest:weekly-"))).toEqual([]);
@@ -281,7 +301,7 @@ describe("weekly digest publication requires maintainer approval", () => {
useAdminEnv(kv);
stubGitHub(422);
const draftKey = onlyKey(kv, "draft:digest:");
const res = await adminPost(adminRequest(postBody({ action: "post", draftKey, lang: "en" })));
const res = await act(kv, { action: "post", draftKey, lang: "en" });
expect(res.status).toBe(502);
const payload = await res.json();
expect(payload.ok).toBeUndefined();
@@ -289,6 +309,64 @@ describe("weekly digest publication requires maintainer approval", () => {
expect(isPublishedDigest(JSON.parse(kv.values.get(onlyKey(kv, "digest:weekly-"))!))).toBe(false);
});
it("never publishes an older record when a later run saved its draft but not its record", async () => {
const kv = new FakeKv();
stubDigestSources();
mocks.agentChat.mockResolvedValue({ content: JSON.stringify(DIGEST_MODEL_OUTPUT), usage: { input: 1, output: 1 } });
await runDigest({ CURATED_KV: kv, DEEPSEEK_API_KEY: "k" });
const recordKey = onlyKey(kv, "digest:weekly-");
const recordA = kv.values.get(recordKey)!;
// Run B saves its draft, then its record write fails: draft B + record A.
mocks.agentChat.mockResolvedValue({
content: JSON.stringify({ ...DIGEST_MODEL_OUTPUT, titleEn: "Digest B", summaryEn: "Run B." }),
usage: { input: 1, output: 1 },
});
kv.failPutsMatching = /^digest:weekly-/;
await expect(runDigest({ CURATED_KV: kv, DEEPSEEK_API_KEY: "k" })).resolves.toMatchObject({ ok: false });
kv.failPutsMatching = null;
const draftKey = onlyKey(kv, "draft:digest:");
expect(JSON.parse(kv.values.get(draftKey)!).bodyEn).toContain("Digest B");
expect(kv.values.get(recordKey)).toBe(recordA);
useAdminEnv(kv);
const bodies: string[] = [];
vi.stubGlobal("fetch", vi.fn(async (_input: string | URL | Request, init?: RequestInit) => {
bodies.push(String(init?.body));
return jsonResponse({ number: 900, html_url: "https://github.com/Hmbown/CodeWhale/issues/900" }, 201);
}));
const res = await act(kv, { action: "post", draftKey, lang: "en" });
await expect(res.json()).resolves.toMatchObject({
ok: true,
published: false,
warning: expect.stringContaining("does not match the reviewed text"),
});
// GitHub got the reviewed B; /digest shows nothing rather than A.
expect(bodies).toHaveLength(1);
expect(JSON.parse(bodies[0]).title).toBe("Digest B");
expect(isPublishedDigest(JSON.parse(kv.values.get(recordKey)!))).toBe(false);
});
it("publishes only the fields of a record that matches the reviewed text", async () => {
const kv = new FakeKv();
stubDigestSources();
mocks.agentChat.mockResolvedValue({ content: JSON.stringify(DIGEST_MODEL_OUTPUT), usage: { input: 1, output: 1 } });
await runDigest({ CURATED_KV: kv, DEEPSEEK_API_KEY: "k" });
const recordKey = onlyKey(kv, "digest:weekly-");
kv.values.set(recordKey, JSON.stringify({ ...JSON.parse(kv.values.get(recordKey)!), extra: "<script>" }));
useAdminEnv(kv);
stubGitHub();
const draftKey = onlyKey(kv, "draft:digest:");
await expect((await act(kv, { action: "post", draftKey, lang: "zh" })).json()).resolves.toMatchObject({
published: true,
});
const published = JSON.parse(kv.values.get(recordKey)!);
expect(isPublishedDigest(published)).toBe(true);
expect(published).not.toHaveProperty("extra");
expect(published).toMatchObject({ approvedLang: "zh", titleZh: DIGEST_MODEL_OUTPUT.titleZh });
});
it("hides legacy records that were never approved", () => {
const legacy = { ...DIGEST_MODEL_OUTPUT, weekId: "2026-W01", generatedAt: "2026-01-05T00:00:00.000Z" };
expect(isPublishedDigest(legacy)).toBe(false);
@@ -327,7 +405,7 @@ describe("resolved drafts are not resurrected by the cron", () => {
const kv = new FakeKv();
await saveDraft(kv, draft());
useAdminEnv(kv);
const res = await adminPost(adminRequest(postBody({ action: "discard", draftKey: "draft:triage:42" })));
const res = await act(kv, { action: "discard", draftKey: "draft:triage:42" });
expect(res.status).toBe(200);
stubIssues("2020-01-01T00:00:00.000Z");
@@ -341,7 +419,7 @@ describe("resolved drafts are not resurrected by the cron", () => {
await saveDraft(kv, draft());
useAdminEnv(kv);
stubGitHub();
const res = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42", lang: "en" })));
const res = await act(kv, { action: "post", draftKey: "draft:triage:42", lang: "en" });
expect(res.status).toBe(200);
// Our own comment bumps updated_at past the draft's generatedAt.
@@ -356,7 +434,7 @@ describe("resolved drafts are not resurrected by the cron", () => {
await saveDraft(kv, draft());
useAdminEnv(kv);
stubGitHub();
expect((await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })))).status).toBe(200);
expect((await act(kv, { action: "post", draftKey: "draft:triage:42" })).status).toBe(200);
// New commits or a reply a day later.
stubIssues(new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString());
@@ -365,7 +443,7 @@ describe("resolved drafts are not resurrected by the cron", () => {
// The fresh draft can be posted.
const posts = stubGitHub();
expect((await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })))).status).toBe(200);
expect((await act(kv, { action: "post", draftKey: "draft:triage:42" })).status).toBe(200);
expect(posts).toHaveLength(1);
});
@@ -392,7 +470,7 @@ describe("admin post action is idempotent and bounded", () => {
useAdminEnv(kv);
const posts = stubGitHub();
const res = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
const res = await act(kv, { action: "post", draftKey: "draft:triage:42" });
expect(res.status).toBe(409);
expect(posts).toEqual([]);
});
@@ -404,11 +482,11 @@ describe("admin post action is idempotent and bounded", () => {
const posts = stubGitHub();
kv.failPutsMatching = /^draft:triage:42$/;
const first = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
const first = await act(kv, { action: "post", draftKey: "draft:triage:42" });
expect(first.status).toBe(200);
await expect(first.json()).resolves.toMatchObject({ ok: true, action: "posted", warning: expect.any(String) });
const retry = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
const retry = await act(kv, { action: "post", draftKey: "draft:triage:42" });
expect(retry.status).toBe(409);
expect(posts).toHaveLength(1);
});
@@ -419,11 +497,11 @@ describe("admin post action is idempotent and bounded", () => {
useAdminEnv(kv);
stubGitHub(422);
const failed = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
const failed = await act(kv, { action: "post", draftKey: "draft:triage:42" });
expect(failed.status).toBe(502);
const posts = stubGitHub();
const retry = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
const retry = await act(kv, { action: "post", draftKey: "draft:triage:42" });
expect(retry.status).toBe(200);
expect(posts).toHaveLength(1);
});
@@ -436,18 +514,18 @@ describe("admin post action is idempotent and bounded", () => {
useAdminEnv(kv);
const posts = stubGitHub(status);
const failed = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
const failed = await act(kv, { action: "post", draftKey: "draft:triage:42" });
expect(failed.status).toBe(502);
await expect(failed.json()).resolves.toMatchObject({ error: expect.stringContaining("check GitHub before retrying") });
stubGitHub();
const retry = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
const retry = await act(kv, { action: "post", draftKey: "draft:triage:42" });
expect(retry.status).toBe(409);
expect(posts).toHaveLength(1);
}
);
it("posts once when two requests for the same draft race", async () => {
it("never posts twice when two requests for the same draft race", async () => {
const kv = new FakeKv();
await saveDraft(kv, draft());
kv.yieldEachOp = true;
@@ -455,14 +533,126 @@ describe("admin post action is idempotent and bounded", () => {
const posts = stubGitHub();
const results = await Promise.all([
adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" }))),
adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" }))),
act(kv, { action: "post", draftKey: "draft:triage:42" }),
act(kv, { action: "post", draftKey: "draft:triage:42" }),
]);
const statuses = results.map((r) => r.status);
// Overlapping claims may both refuse (fail closed), never both proceed.
expect(statuses.filter((s) => s === 200)).toHaveLength(posts.length);
expect(posts.length).toBeLessThanOrEqual(1);
expect(statuses.every((s) => s === 200 || s === 409)).toBe(true);
// Refused claims are released, so the maintainer's retry posts exactly once.
if (posts.length === 0) {
expect((await act(kv, { action: "post", draftKey: "draft:triage:42" })).status).toBe(200);
}
expect(posts).toHaveLength(1);
expect((await act(kv, { action: "post", draftKey: "draft:triage:42" })).status).toBe(409);
expect(posts).toHaveLength(1);
});
/**
* Hold two admin actions at the reviewer's interleaving: both pass the
* "no decision yet" check, the first takes its claim and reaches GitHub,
* and only then does the second's claim write land.
*/
function raceAfterAbsenceChecks(kv: FakeKv) {
let prechecks = 0;
let bothChecked!: () => void;
const bothCheckedP = new Promise<void>((resolve) => (bothChecked = resolve));
let firstAtGitHub!: () => void;
const firstAtGitHubP = new Promise<void>((resolve) => (firstAtGitHub = resolve));
const realGet = kv.get.bind(kv);
kv.get = async (key: string) => {
const value = await realGet(key);
if (key.startsWith("draft-resolved:") && ++prechecks === 2) bothChecked();
return value;
};
let claimPuts = 0;
kv.beforePut = async (key, value) => {
// A claim write: its own key, or (the earlier design) a token written
// over the shared marker.
const isClaim = key.startsWith("draft-claim:") || (key.startsWith("draft-resolved:") && value.includes('"claim"'));
if (!isClaim) return;
claimPuts += 1;
await (claimPuts === 1 ? bothCheckedP : firstAtGitHubP);
};
const posts: string[] = [];
vi.stubGlobal("fetch", vi.fn(async (input: string | URL | Request) => {
posts.push(inputUrl(input));
firstAtGitHub();
// Let the second request finish its claim while this post is in flight.
await new Promise((resolve) => setTimeout(resolve, 20));
return jsonResponse({ id: 1 }, 201);
}));
return posts;
}
it("refuses a second post whose claim lands while the first is at GitHub", async () => {
const kv = new FakeKv();
await saveDraft(kv, draft());
kv.yieldEachOp = true;
useAdminEnv(kv);
const posts = raceAfterAbsenceChecks(kv);
const results = await Promise.all([
act(kv, { action: "post", draftKey: "draft:triage:42" }),
act(kv, { action: "post", draftKey: "draft:triage:42" }),
]);
expect(results.map((r) => r.status).sort()).toEqual([200, 409]);
expect(posts).toHaveLength(1);
});
it("refuses a discard that overlaps an in-flight post of the same draft", async () => {
const kv = new FakeKv();
await saveDraft(kv, draft());
kv.yieldEachOp = true;
useAdminEnv(kv);
const posts = raceAfterAbsenceChecks(kv);
const [post, discard] = await Promise.all([
act(kv, { action: "post", draftKey: "draft:triage:42" }),
act(kv, { action: "discard", draftKey: "draft:triage:42" }),
]);
expect(post.status).toBe(200);
expect(discard.status).toBe(409);
expect(posts).toHaveLength(1);
expect(JSON.parse(kv.values.get("draft-resolved:triage:42")!)).toMatchObject({ state: "posted" });
expect(JSON.parse(kv.values.get("draft:triage:42")!)).toMatchObject({ posted: true });
});
it("refuses an action on a draft regenerated after the page loaded, before calling GitHub", async () => {
const kv = new FakeKv();
await saveDraft(kv, draft({ bodyEn: "reviewed A" }));
useAdminEnv(kv);
const shownA = await reviewedBodyHash("reviewed A");
// The cron regenerates the pending draft while the admin page shows A.
await saveDraft(kv, draft({ bodyEn: "unseen B", generatedAt: "2026-01-02T00:00:00.000Z" }));
const posts = stubGitHub();
for (const action of ["post", "discard"]) {
const res = await act(kv, { action, draftKey: "draft:triage:42", lang: "en", reviewedSha256: shownA });
expect(res.status).toBe(409);
await expect(res.json()).resolves.toMatchObject({ error: expect.stringContaining("changed") });
}
expect(posts).toEqual([]);
expect(JSON.parse(kv.values.get("draft:triage:42")!)).toMatchObject({ bodyEn: "unseen B", posted: false });
expect([...kv.values.keys()].filter((k) => k.startsWith("draft-resolved:") || k.startsWith("draft-claim:"))).toEqual([]);
});
it("requires the reviewed-text hash", async () => {
const kv = new FakeKv();
await saveDraft(kv, draft());
useAdminEnv(kv);
const posts = stubGitHub();
const res = await adminPost(adminRequest(postBody({ action: "post", draftKey: "draft:triage:42" })));
expect(res.status).toBe(400);
expect(posts).toEqual([]);
});
it("answers malformed JSON with a JSON 400", async () => {
const kv = new FakeKv();
useAdminEnv(kv);
+3 -2
View File
@@ -12,6 +12,7 @@ import {
hasFreshDraft,
getDraftResolution,
digestRecordKey,
renderDigestBody,
logUsage,
type WeeklyDigestRecord,
type AgentDraft,
@@ -430,8 +431,8 @@ export async function runDigest(env: AgentEnv): Promise<Record<string, unknown>>
const draft: AgentDraft = {
id: weekId,
type: "digest",
bodyEn: `# ${parsed.titleEn}\n\n${parsed.summaryEn}\n\n${parsed.sections.map((s) => `## ${s.heading}\n${s.items.map((i) => `- ${i}`).join("\n")}`).join("\n\n")}`,
bodyZh: `# ${parsed.titleZh}\n\n${parsed.summaryZh}\n\n${parsed.sections.map((s) => `## ${s.heading}\n${s.items.map((i) => `- ${i}`).join("\n")}`).join("\n\n")}`,
bodyEn: renderDigestBody(parsed, "en"),
bodyZh: renderDigestBody(parsed, "zh"),
generatedAt: new Date().toISOString(),
posted: false,
};
+132 -36
View File
@@ -317,8 +317,6 @@ export type DraftResolutionState = "posting" | "posted" | "discarded";
export interface DraftResolution {
state: DraftResolutionState;
at: string;
/** Random token of the request holding a "posting" claim. */
claim?: string;
}
const RESOLUTION_PREFIX = "draft-resolved:";
@@ -362,11 +360,7 @@ export async function getDraftResolution(
try {
const parsed = JSON.parse(raw) as Partial<DraftResolution>;
if (parsed.state === "posting" || parsed.state === "posted" || parsed.state === "discarded") {
return {
state: parsed.state,
at: typeof parsed.at === "string" ? parsed.at : "",
...(typeof parsed.claim === "string" ? { claim: parsed.claim } : {}),
};
return { state: parsed.state, at: typeof parsed.at === "string" ? parsed.at : "" };
}
} catch {
/* fall through: treat an unreadable marker as present */
@@ -388,26 +382,76 @@ export async function markDraftResolved(
}
/**
* Claim a draft before posting it to GitHub. Returns false when another
* request holds or wins the claim.
*
* KV has no compare-and-set, so this is best effort, not a lock: it writes a
* random token and reads it back, which stops concurrent posts that meet in
* one location (two tabs, a double submit) but cannot fully order writers in
* different edge locations, because KV is eventually consistent across them.
* An exclusive hold on one draft identity while a maintainer action (post or
* discard) runs. Each request writes its own claim key, so no request can
* overwrite another's claim, and proceeds only if its key is the only one
* under the draft's claim prefix.
*/
export async function claimDraftForPosting(
export interface DraftClaim {
key: string;
}
const CLAIM_PREFIX = "draft-claim:";
function claimPrefix(type: AgentDraftType, id: string): string {
return `${CLAIM_PREFIX}${draftKey(type, id).slice("draft:".length)}:`;
}
/**
* Claim a draft before acting on it. Returns null when another request holds
* a claim, or when a decision (posted, discarded, a post in flight) was
* recorded before this claim.
*
* Why this is exclusive: two overlapping claimants each write their key before
* they list. Whichever lists second sees both keys, so at most one sees only
* its own. If they overlap exactly, both see two keys and both refuse (the
* maintainer retries); two can never both proceed. A claimant that arrives
* after the winner still sees the winner's key, which is kept until it
* expires unless the winner's action definitely did not happen.
*
* This holds when a list reflects every put that finished before it. Workers
* KV guarantees that within one location, not across locations (its writes
* can take up to 60 seconds to reach other locations), so two maintainers
* posting the same draft from different regions in the same instant are not
* serialized. A real compare-and-set authority (a Durable Object) would be
* needed for that; this admin surface has one maintainer.
*/
export async function claimDraft(
kv: KVNamespace | undefined,
type: AgentDraftType,
id: string
): Promise<boolean> {
if (!kv) return true;
const key = resolutionKey(type, id);
const claim = crypto.randomUUID();
const value: DraftResolution = { state: "posting", at: new Date().toISOString(), claim };
await kv.put(key, JSON.stringify(value), { expirationTtl: POSTING_CLAIM_TTL_SEC });
const current = await getDraftResolution(kv, type, id);
return current?.state === "posting" && current.claim === claim;
): Promise<DraftClaim | null> {
if (!kv) return { key: "" };
const prefix = claimPrefix(type, id);
const key = prefix + crypto.randomUUID();
await kv.put(key, new Date().toISOString(), { expirationTtl: POSTING_CLAIM_TTL_SEC });
let won = false;
try {
const { keys } = await kv.list({ prefix });
won = keys.length === 1 && keys[0].name === key;
// A decision recorded after the caller's own check still wins.
if (won && (await getDraftResolution(kv, type, id))) won = false;
} finally {
if (!won) await kv.delete(key).catch(() => undefined);
}
return won ? { key } : null;
}
/** Give up a claim, for an action that definitely did not happen. */
export async function releaseDraftClaim(kv: KVNamespace | undefined, claim: DraftClaim): Promise<void> {
if (!kv || !claim.key) return;
await kv.delete(claim.key);
}
/**
* SHA-256 (hex) of the draft text a maintainer was shown. The admin page
* sends it with every action, and the route acts only if the stored draft
* still has exactly that text, so a draft regenerated after the page loaded
* is never posted, published or discarded unseen.
*/
export async function reviewedBodyHash(body: string): Promise<string> {
const bytes = new Uint8Array(await crypto.subtle.digest("SHA-256", new TextEncoder().encode(body)));
return Array.from(bytes, (b) => b.toString(16).padStart(2, "0")).join("");
}
export async function clearDraftResolution(
@@ -471,29 +515,81 @@ export function isPublishedDigest(value: unknown): value is WeeklyDigestRecord {
);
}
/** The structured fields of a digest that its markdown body is rendered from. */
export type DigestContent = Pick<
WeeklyDigestRecord,
"titleEn" | "titleZh" | "summaryEn" | "summaryZh" | "sections"
>;
/**
* Mark the stored digest for `weekId` approved in the language the maintainer
* reviewed. Returns false if it is gone.
* The one renderer from structured digest to the markdown a maintainer
* reviews and GitHub receives. Approval re-renders the stored record with it
* and publishes only on an exact match with the reviewed draft.
*/
export function renderDigestBody(digest: DigestContent, lang: "en" | "zh"): string {
const title = lang === "zh" ? digest.titleZh : digest.titleEn;
const summary = lang === "zh" ? digest.summaryZh : digest.summaryEn;
const sections = digest.sections
.map((s) => `## ${s.heading}\n${s.items.map((i) => `- ${i}`).join("\n")}`)
.join("\n\n");
return `# ${title}\n\n${summary}\n\n${sections}`;
}
export type DigestApproval = "published" | "missing" | "mismatch";
/**
* Publish the stored digest for `draft` in the language the maintainer
* reviewed. The record is published only if it is the same generation as the
* reviewed draft and renders to exactly the reviewed text; a missing,
* malformed or different record (for example, a later cron run whose record
* write failed after its draft write) stays unpublished. The publication is a
* single put of the approved record.
*/
export async function approveDigestRecord(
kv: KVNamespace | undefined,
weekId: string,
draft: AgentDraft,
lang: "en" | "zh"
): Promise<boolean> {
if (!kv) return false;
const key = digestRecordKey(weekId);
): Promise<DigestApproval> {
if (!kv || draft.type !== "digest") return "missing";
const key = digestRecordKey(draft.id);
const raw = await kv.get(key);
if (!raw) return false;
const record = JSON.parse(raw) as WeeklyDigestRecord;
const approved: WeeklyDigestRecord = {
...record,
if (!raw) return "missing";
let record: unknown;
try {
record = JSON.parse(raw);
} catch {
return "mismatch";
}
const approved = {
...(record && typeof record === "object" ? (record as Record<string, unknown>) : {}),
approved: true,
approvedAt: new Date().toISOString(),
approvedLang: lang,
};
if (!isPublishedDigest(approved)) return false;
await kv.put(key, JSON.stringify(approved), { expirationTtl: DIGEST_RECORD_TTL_SEC });
return true;
if (!isPublishedDigest(approved)) return "mismatch";
const reviewed = lang === "zh" ? draft.bodyZh : draft.bodyEn;
if (
approved.weekId !== draft.id ||
approved.generatedAt !== draft.generatedAt ||
renderDigestBody(approved, lang) !== reviewed
) {
return "mismatch";
}
// Copy the fields explicitly so nothing else stored under the key is published.
const published: WeeklyDigestRecord = {
weekId: approved.weekId,
titleEn: approved.titleEn,
titleZh: approved.titleZh,
summaryEn: approved.summaryEn,
summaryZh: approved.summaryZh,
sections: approved.sections,
generatedAt: approved.generatedAt,
approved: true,
approvedAt: approved.approvedAt,
approvedLang: lang,
};
await kv.put(key, JSON.stringify(published), { expirationTtl: DIGEST_RECORD_TTL_SEC });
return "published";
}
export async function deleteDigestRecord(kv: KVNamespace | undefined, weekId: string): Promise<void> {