fix(web): gate public digest on maintainer approval; keep resolved community drafts resolved

[Hmbown]

No-Issue: verified bug-hunt findings

Four community-agent findings, each re-checked on current main before fixing. The lane covers web/lib/community-agent*.ts, web/app/api/admin/post/route.ts and web/app/[locale]/digest/page.tsx.

1. /digest showed unreviewed model output as "maintainer-approved"

Problem. runDigest wrote digest:weekly-<week> straight to KV, and /digest rendered every record under that prefix. The page copy ("Maintainer-approved summaries", "appears here only after the cron draft has been reviewed") and llms.txt ("reviewed by a maintainer") were therefore not true. The digest is built from public issue and PR titles.

Resulting behavior. The cron now stages the record with approved: false, and the model can't set it. The record fields are copied one by one, not spread from model output. /digest renders only records that are approved and well-formed (isPublishedDigest), which also hides legacy records that were never approved. When the maintainer posts the digest from /admin, the record is approved in the language they reviewed (approvedLang, published: true in the response); /digest shows only that language. If the maintainer edited the digest first, it still goes to GitHub, but the stored unedited model text is not published (published: false). Discard deletes the staged record. The page and llms.txt copy is now accurate, so I left it unchanged.

2. Discarded or already-posted drafts came back as pending

Problem. Discard deleted the draft, so the next triage tick drafted it again and paid for another model call. After a post, GitHub bumps updated_at past generatedAt, so the draft was overwritten with posted: false. runDupes had no existing-draft check.

Resulting behavior. Post and discard write a draft-resolved:<type>:<id> marker that lasts 90 days. For triage, PR-review and stale drafts, activity on the item more than 10 minutes after the decision reopens it (see follow-up below). saveDraft refuses to write when that marker exists or the stored draft is already posted. This covers every generator, including the content-watch linkcheck and semantic-drift drafts. hasFreshDraft checks the same state, so resolved items cost no model call. runDigest checks the marker before its model call.

3. The admin post action was not idempotent, and body parsing was unguarded

Problem. The route never checked draft.posted, so a second tab could post the same comment again. A KV write failing after GitHub accepted the post returned a 500, and retrying posted a duplicate. A bare req.json() turned malformed JSON into an unhandled 500. The 64 KiB limit was enforced only through Content-Length.

Resulting behavior.

• An already-posted draft now returns 409.
• Before calling GitHub, the route writes a short-lived "posting" claim (15 min), and any existing marker returns 409.
• If GitHub answers a 4xx other than 408/429, the claim is released so the maintainer can retry. A 5xx, 408, 429 or network error leaves the outcome unknown, so the claim stays until it expires.
• If bookkeeping fails after GitHub accepted the post, the route returns ok: true with a warning.
• The body is read with readBoundedBody, which counts real bytes. Invalid JSON gets a JSON 400.

4. The admin queue stopped at the first 100 KV keys

Problem. listDrafts made a single list({ limit: 100 }) call with no cursor, so newer drafts never appeared and the pending count was wrong.

Resulting behavior. It now follows cursor, reads at most 500 drafts in parallel batches of 50 (bounded KV operations per request), and keeps what it read if a later list page fails.

Test evidence

• New web/lib/community-agent-review-state.test.ts: 14 passed. Against the unfixed source, 13 fail: 11 on behavior (for example expected 200 to be 409, expected [ …(100) ] to have a length of 1500 but got 100, processed: 1 vs 0, SyntaxError on malformed JSON) and 2 because the new isPublishedDigest helper doesn't exist yet. The one that passes on both is the claim-release retry test, a guard against over-locking.
• web/lib/public-api-security.test.ts: I removed one source-text assertion that depended on the old inline 502 layout. The new file now covers that property (a digest post that GitHub rejects returns 502, with no ok and no publication).
• Full web vitest: 57 files, 515 tests passed. tsc --noEmit -p . exit 0. eslint on the touched files exit 0.

Review follow-up (commit 1d83478)

A review of the first commit raised nine points. Each was re-checked against the branch.

Fixed

• Concurrent posts. Two requests could both read "no marker" and both post. The claim now writes a random token and reads it back before the GitHub call; the loser gets 409. Test: two handlers via Promise.all against a KV that yields on every call → statuses [200, 409], 1 GitHub post (2 before). KV has no compare-and-set and is eventually consistent across locations, so this is best effort, not a lock; the code comment says so. The earlier "safe to repeat" wording is withdrawn. (Superseded: see "Exclusive claims" below.)
• 5xx released the claim. GitHub can return 5xx/429 after creating the comment. Now only 4xx other than 408/429 release it. Tests: 500, 502, 504, 429 → 502 then retry 409, 1 post total; the release test now uses 422.
• Admin UI ignored warning. admin-client.tsx now shows the route's warning. The route also warns when an edited digest is posted but not shown on /digest, or when the weekly record is gone.
• 90-day suppression regardless of activity. Triage, PR-review and stale items updated more than 10 minutes after the maintainer's decision are drafted again (resolutionCovers); our own comment's updated_at bump stays covered. Test: post, then an update a day later → processed: 1, a new pending draft, which posts once.
• Unreviewed Chinese text published. Approval records approvedLang; isPublishedDigest requires it and /digest renders each digest only in that language. Tests for en and zh approval.
• Discarding a posted digest unpublished it. Discarding a posted (or in-flight) draft now returns 409. Test: post, then discard → 409, digest still published.
• /digest could lag an hour. Approval and digest discard call revalidatePath("/[locale]/digest", "page"), as the feed retry route already does.
• listDrafts could exhaust KV operations and lose partial results. See section 4. Tests: 250 keys over 100-key pages → 250; 1500 keys → 500 drafts and 500 reads; list failure on the third page → 200 kept.
• Drift and generator counters. runSemanticDrift and the triage / PR-review / stale runs now count a draft only when saveDraft wrote it. Test: a discarded drift finding → drafted: 0 (was 1).

Kept as intended, with reasons

• Legacy /digest records are hidden, with no migration. Before this PR every cron record went public without review, so no stored record can be shown to have been reviewed. Hiding them is the fix. Posting the next weekly digest publishes it again.
• Digests, dupes and content-watch findings stay suppressed for 90 days after a decision. They have no item updated_at, and a changed drift or link finding already gets a new key. Discard means the maintainer chose "not this", so regenerating the same identity is the old behavior this PR removes.
• The drift model call still runs every day. Drift draft keys come from the model's own findings, so no check can run before the call. The earlier "a resolved item costs no model call" applies to triage, PR-review, stale and digest only.
• A posted draft whose 7-day copy expired before markers existed can be drafted once more after deploy. This is one-time and happens only for pre-deploy items.

Follow-up test evidence

• community-agent-review-state, content-watch, community-agent-security, public-api-security: 4 files, 45 passed. On the previous source, 14 of the new or changed tests fail: race, 5xx×4, lang×2, discard-after-post, edited-digest warning, reopen-on-activity, legacy approvedLang, listDrafts cap and partial, and drift count.
• Full web vitest: 57 files, 526 passed. tsc --noEmit exit 0. eslint on the touched files exit 0. npm --prefix web run check ran through next build.

Second review follow-up (commit 989081c)

An independent review of 1d83478 held two P1s. Both fixed; this supersedes the "Concurrent posts" bullet above.

• Approval bound to the reviewed bytes. The admin page now sends reviewedSha256 (SHA-256 of the text it showed, in the selected language) with every post and discard. If the stored draft no longer hashes to it, the route answers 409 before any claim or GitHub call. approveDigestRecord now takes the draft. It publishes only a record with the same weekId and generatedAt whose re-render (renderDigestBody, now the one renderer shared with runDigest) equals the reviewed text, and it copies fields explicitly in one put. The failure case the reviewer named (draft B saved, record B's put failed, record A still stored) now posts B to GitHub and leaves /digest unpublished, with a mismatch warning.
• Claims (revised in 7cf21ca). The claim is one key, draft-claim:<type>:<id>, holding the request's random token. A request refuses if a claim is already visible, writes its token, and proceeds only if reading the key back returns its token and no decision marker exists. It no longer uses KV list, whose results can lag writes by up to 60 s. A read-back that misses the request's own write returns 503 (retryable) instead of a false 409. Release deletes the key only while it still holds the request's token. On KV alone this is best effort: KV has no compare-and-set, so two requests whose writes land at the same moment, or that run in different locations, can both proceed. (Superseded by the Durable Object in ccc0009; KV is now only the local/dev fallback.) What it does stop is the sequential cases this page produces: a second tab, a retry, or a discard during a post. A repeated discard is now a 200 no-op. Discard takes the same claim.

Test evidence: community-agent-review-state, content-watch, community-agent-security, public-api-security: 4 files, 51 passed. Six of the new tests fail against 1d83478: record mismatch, explicit-field publish, the barrier race (both absence checks pass, then the second claim lands while the first is at GitHub), post/discard overlap, stale reviewed text, and missing hash. tsc --noEmit exit 0. eslint on the touched files exit 0.

Still open (nonblocking): listDrafts still stops at 500 reads and doesn't tell the admin page the list is partial.

Exclusive claims (commit ccc0009)

Once deployed, claims are exclusive through a Durable Object. KV is now only the fallback for next dev and tests.

• DraftClaimLock (web/lib/draft-claim-lock.ts; the class lives in web/worker.ts) is SQLite-backed, with one instance per draft identity via idFromName("draft-claim:<type>:<id>"). A Durable Object handles one event at a time, and its input gate holds other events while a storage call is pending. That makes grant, refuse and release atomic, so exactly one request wins.
• Lease. A claim lasts 15 minutes. After a 5xx, 408, 429 or network error the lease stays, so nobody can post again until the outcome is known. When it expires the draft is free again, so a crashed post can't block it forever. A definite GitHub 4xx releases the lease immediately.
• Hold after a recorded decision. After a post or discard is recorded, the object keeps refusing claims for 2 more minutes. KV can take 60 s or more to show the decision marker in other locations, and a retry from one of them still reads the draft as pending. Without the hold it could post a second time.
• Repeated discard (reviewer note). A second discard returns 200 whether the first one has finished (the marker is visible) or is still holding the draft. A discard that collides with a post is still refused with 409.
• Fails closed. If the binding is present but the object can't be reached, the route returns 500. It does not fall back to KV and does not call GitHub.
• Wiring. OpenNext's documented way to add Durable Objects is a named export from the custom worker entry (https://opennext.js.org/cloudflare/howtos/custom-worker). So worker.ts exports DraftClaimLock, and wrangler.jsonc adds the DRAFT_CLAIM_LOCK binding and a v1 migration with new_sqlite_classes: ["DraftClaimLock"]. CommunityAgentEnv has the binding type. getAgentEnv drops the binding under next dev, because wrangler's platform proxy can't run a class defined in the Worker's own entry.

Test evidence: draft-claim-lock.test.ts has 6 tests: 8 concurrent claims give exactly 1 grant, a control shows the same logic without the gate double-grants, one lock per draft, lease expiry, release only by the owner, and the post-decision hold. The route suite community-agent-review-state has 7 new tests using a serializing fake DO:

• 4 concurrent posts give 200/409/409/409 with one GitHub post and no KV claim keys.
• A stale-KV retry after a recorded post gets 409, and is allowed after the hold.
• After a 502 the lease is kept even with no KV marker, and expires after 15 minutes.
• A 422 frees the draft immediately.
• A concurrent double discard gives 200/200, and a third, stale click also gets 200.
• Post and discard exclude each other.
• An unreachable lock gives 500 with no GitHub call.

6 of the 7 route tests fail against 7cf21ca. The 422 test passes on both; it guards against locking too much. vitest on community-agent*, content-watch, public-api-security, draft-claim-lock and deploy-preflight: 7 files, 89 passed. tsc --noEmit exit 0. eslint on the touched files exit 0. esbuild bundles worker.ts with the exported class. Not checked: a real wrangler dev or deployed Durable Object. The fake models the input gate; it isn't workerd.

Deploy steps (founder; nothing here was deployed)

1. Merge this PR. The migration exists only in source until then.
2. From web/, run npm run deploy (opennextjs-cloudflare build && opennextjs-cloudflare deploy). The deploy applies migration v1, which creates the DraftClaimLock SQLite namespace and binds it as DRAFT_CLAIM_LOCK. No wrangler resource-create command or secret is needed; the account must allow SQLite-backed Durable Objects, which the Workers Free plan does.
3. Check it: in the Cloudflare dashboard, open Workers & Pages → codewhale-web → Settings → Bindings and confirm DRAFT_CLAIM_LOCK → DraftClaimLock is listed. Or post and discard a test draft from /admin and see the Durable Objects tab show one object per draft touched.
4. Rollback note: the migration is one-way. Later Worker versions must keep exporting DraftClaimLock while the binding exists; removing the class takes a deleted_classes migration (which deletes its stored leases), not just deleting the export. I did not test a wrangler rollback across this migration.

🤖 Generated with Claude Code

https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks

Evidence for 7cf21ca: community-agent-review-state: 34 passed, including 4 new tests: no list for claims, own-write-not-visible → 503 then a successful retry, a visible foreign claim is refused and left alone, and a repeated discard is a no-op. All 4 fail against 989081c. community-agent* + content-watch + public-api-security: 5 files, 64 passed. tsc --noEmit exit 0. eslint on the touched files exit 0. The in-memory KV fake is strongly consistent, so these tests can't show cross-location behavior.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.