fix(claim): do not report accept when decision persist fails

[SebTardif]

What Problem This Solves

Fixes an issue where a reviewer pressing Accept on a clawtributor claim would be told the claim was accepted even when saving the decision to the database failed. Discord could add the clawtributors role, then the bot still DMed the claimant, posted the announcement, and replied "Claim accepted" while the stored claim stayed pending.

That false success is worse than a failed Accept: staff think the review is finished, the claimant is told they are a clawtributor, and a later /claim still looks pending.

Why This Change Was Made

Accept now waits for recordClaimDecision to finish. If the write fails, the review button replies with an error and does not DM, announce, or mark the review message accepted. The previous success copy is sent only after the decision is saved. Reject is unchanged.

User Impact

Reviewers get an accurate private reply when Accept cannot save the decision. A database failure no longer looks like a completed clawtributor grant. Claimants are not told they were accepted while the claim record is still pending.

Evidence

Live bun run of /tmp/proof-hermit-F004.ts against upstream src/server/claimServer.ts and this branch. The script calls the production Accept button (claimReviewComponents[0].run) after Discord's role PUT returns 204. One run makes recordClaimDecision throw database unavailable. The other run lets the write finish.

$ bash /tmp/proof-hermit-F004.sh
===== BEFORE (upstream accept swallows persist errors) =====
PERSIST REJECT
{
  "reply": "Claim accepted. <@user-1> has been given the role.",
  "dmCount": 1,
  "announcementCount": 1,
  "patchCount": 1
}
PERSIST OK
{
  "reply": "Claim accepted. <@user-1> has been given the role.",
  "dmCount": 1,
  "announcementCount": 1,
  "patchCount": 1
}
===== AFTER (patched accept surfaces persist errors) =====
PERSIST REJECT
{
  "reply": "Could not record claim. The clawtributors role was added, but the claim decision could not be saved. Ask a moderator to retry or update the claim record.",
  "dmCount": 0,
  "announcementCount": 0,
  "patchCount": 0
}
PERSIST OK
{
  "reply": "Claim accepted. <@user-1> has been given the role.",
  "dmCount": 1,
  "announcementCount": 1,
  "patchCount": 1
}

Before this patch a failed decision write still printed Claim accepted, sent the DM, announced the grant, and patched the review message. After the patch a failed write prints Could not record claim and skips those success side effects. A successful write still prints Claim accepted.

Related: the swallow landed in #10 (2b970350, 2026-05-12). Same false-success class as #26 (automod-bypass role changes) and #27 (inactivity-warn addMember).

Real behavior proof

• Behavior or issue addressed: Review-channel Accept reported Claim accepted, DMed the claimant, and announced the clawtributors grant even when saving the accepted decision failed, so the stored claim stayed pending.

• Real environment tested: macOS 26.6.1 Darwin 25.6.0 arm64, Node v26.7.0, bun 1.3.14, patched checkout /tmp/oc-pr-hermit-F004 on fix/f004-claim-persist-error (base 72b5de1).

• Exact steps or command run after this patch:

  bash /tmp/proof-hermit-F004.sh

• Evidence after fix: terminal output from the patched Accept handler:

  $ bash /tmp/proof-hermit-F004.sh
  AFTER persist reject
  {
    "reply": "Could not record claim. The clawtributors role was added, but the claim decision could not be saved. Ask a moderator to retry or update the claim record.",
    "dmCount": 0,
    "announcementCount": 0,
    "patchCount": 0
  }
  AFTER persist ok
  {
    "reply": "Claim accepted. <@user-1> has been given the role.",
    "dmCount": 1,
    "announcementCount": 1,
    "patchCount": 1
  }

• Observed result after fix: A failed decision write now replies Could not record claim and does not DM, announce, or patch the review message. A successful write still replies Claim accepted and still DMs, announces, and patches.

• What was not tested: A live Discord review-channel click against production D1. Discord role PUT was stubbed to 204 so the persist path is the only failure.

Summary

Accept no longer treats a swallowed recordClaimDecision error as a completed grant. Persist is awaited on the success path. Persist failure replies with an error.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 1, 2026, 8:03 AM ET / 12:03 UTC.

ClawSweeper review

What this changes

The PR makes the Discord claim-review Accept button return a recovery message and skip success notifications when saving the accepted decision fails.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 3 items remain

Keep open: current main still logs a failed claim-decision write and proceeds with success notifications, while this focused patch stops those false-success effects. The implementation is sound on review, but the supplied proof remains mock-only.

Priority: P2
Reviewed head: 38a96ea9d456155ea809ad88330a85fe55a910bc

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is focused and has direct regression coverage, but real behavior proof is still required before merge.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The changed production owner is the claim-review Accept button; the supplied terminal trace and added test invoke it, but stub the Discord role PUT and replace the decision writer, so they prove control flow rather than recovery through real persistence. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The changed production owner is the claim-review Accept button; the supplied terminal trace and added test invoke it, but stub the Discord role PUT and replace the decision writer, so they prove control flow rather than recovery through real persistence. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	6 items	Current main still has the false-success path: The fetched default branch catches and logs a failed accepted-decision write, then continues to DM, announce, patch the review message, and reply with success; this PR’s central behavior is not already implemented. Introduced control-flow gate: The introduced try/catch awaits the decision write, sends a Carbon recovery response on rejection, and returns before the existing notification and review-message success effects. Decision writer propagates failures: The claim decision helper awaits the database insert/update without swallowing errors, so its rejected promise reaches the newly added handler branch.
Findings	None	None.
Security	None	None.

How this fits together

Hermit’s claim flow turns a Discord member’s request into a staff review message, then grants a role and records the staff decision. The Accept button updates Discord, persists the claim state, and only then notifies the claimant and review channel.

flowchart TD
A[Claim request] --> B[Review message]
B --> C[Moderator selects Accept]
C --> D[Discord role grant]
D --> E[Claim decision store]
E -->|saved| F[Claimant and staff notifications]
E -->|failed| G[Moderator recovery reply]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The changed production owner is the claim-review Accept button; the supplied terminal trace and added test invoke it, but stub the Discord role PUT and replace the decision writer, so they prove control flow rather than recovery through real persistence. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve merge risk (P1) - The supplied evidence does not exercise the actual decision writer or a real Discord role operation, so recovery from a real persistence failure remains unverified.
• Complete next step (P2) - No discrete repair finding remains; the external contributor needs real behavior proof before merge.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test delta	production +23/-7; tests +126/-0	The small handler change is accompanied by focused success and persistence-failure coverage.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Keep the post-persistence success gate and add redacted evidence from a real claim-review path or approved production-path harness using the actual decision writer.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep the post-persistence success gate and add redacted evidence from a real claim-review path or approved production-path harness using the actual decision writer.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: current main catches the decision-write rejection and then executes the success effects; the PR’s focused test exercises both rejection and success branches with controlled dependencies.

Is this the best way to solve the issue?

Yes. Awaiting persistence and returning before success-only effects is the narrowest maintainable repair for this false-success path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 38f929b69cba.

Labels

Label justifications:

• P2: This corrects a bounded staff-facing false-success path in the claim-review workflow.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed production owner is the claim-review Accept button; the supplied terminal trace and added test invoke it, but stub the Discord role PUT and replace the decision writer, so they prove control flow rather than recovery through real persistence. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Current main still has the false-success path: The fetched default branch catches and logs a failed accepted-decision write, then continues to DM, announce, patch the review message, and reply with success; this PR’s central behavior is not already implemented. (src/server/claimServer.ts:203, 38f929b69cba)
• Introduced control-flow gate: The introduced try/catch awaits the decision write, sends a Carbon recovery response on rejection, and returns before the existing notification and review-message success effects. (src/server/claimServer.ts:203, 38a96ea9d456)
• Decision writer propagates failures: The claim decision helper awaits the database insert/update without swallowing errors, so its rejected promise reaches the newly added handler branch. (src/data/claimRequests.ts:100, 38a96ea9d456)
• Focused regression coverage: The added test asserts that a rejected decision write yields one recovery reply and no DM, announcement, or review-message patch; it also retains the successful-write path. (tests/claimAcceptPersist.test.ts:70, 38a96ea9d456)
• Proof remains mocked: The PR body explicitly says the Discord role PUT was stubbed, and both the supplied trace and test replace the decision writer; this is useful control-flow evidence but not real persistence-path proof. (tests/claimAcceptPersist.test.ts:73, 38a96ea9d456)
• Claim-flow history: Feature history identifies the merged claim-deduplication work as the origin of decision persistence, with later claim-review maintenance on the same server file. (src/server/claimServer.ts:203, 2b9703501567)

Likely related people:

• fuller-stack-dev: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Hannes Rudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add redacted evidence from a real claim-review path or approved production-path harness using the actual decision writer; show both failed-write recovery and successful completion.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (12 earlier review cycles; latest 8 shown)

• reviewed 2026-08-30T13:07:01.434Z sha 38a96ea :: needs real behavior proof before merge. :: none
• reviewed 2026-08-30T17:04:14.694Z sha 38a96ea :: needs real behavior proof before merge. :: none
• reviewed 2026-08-30T21:30:15.027Z sha 38a96ea :: needs real behavior proof before merge. :: none
• reviewed 2026-08-31T04:00:28.320Z sha 38a96ea :: needs real behavior proof before merge. :: none
• reviewed 2026-08-31T08:08:12.015Z sha 38a96ea :: needs real behavior proof before merge. :: none
• reviewed 2026-08-31T12:08:22.395Z sha 38a96ea :: needs real behavior proof before merge. :: none
• reviewed 2026-09-01T04:57:58.641Z sha 38a96ea :: needs real behavior proof before merge. :: none
• reviewed 2026-09-01T09:00:54.960Z sha 38a96ea :: needs real behavior proof before merge. :: none