Make issue close safe to retry with an idempotency key - #332
Make issue close safe to retry with an idempotency key#332naveenspark wants to merge 16 commits into
Conversation
…-safety # Conflicts: # internal/db/errors.go # internal/db/pgstore/stubgen/main_test.go
roborev: Combined Review (
|
roborev: Combined Review (
|
|
looking |
A keyed close retry only needs the committed issue.closed receipt, which the events table already stores under the idempotency key. Schema version 27 and the close_event_deliveries table existed to re-broadcast the event batch after a daemon crashed between commit and publish. That gap exists for every mutation today because the hook queue is in-memory, and SSE clients already recover missed events through Last-Event-ID, so a per-mutation delivery table and claim lease bought little for its cost. The table also carried two defects: a key reused after the seven-day lookup window collided with its permanent primary key and returned a 500, and the PostgreSQL table registry never listed it, so split-role privilege validation skipped it. The ambiguous-commit case still publishes exactly once. Both stores return the attempted event batch alongside a commit error, and the handler publishes that batch only when the receipt it finds is the same event this attempt wrote. Replayed receipts now also check the caller's host scope against the issue's current project, matching the comment replay path, so a moved issue cannot leak state through the old route. Generated with Claude Code Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
wesm
changed the title
roborev: Combined Review (
|
A comment idempotency key now identifies one request to one issue. Before this, a short-id request looked the key up by project while a ULID request looked it up by issue, so the same key sent to a second issue in the same project with the same body replayed the first issue's comment instead of posting one, and the two ref forms took different locks for the same retry. Every keyed comment now resolves to an issue UID first and locks, looks up, and fingerprints under that UID. A retry with a full ULID still survives a project move because it needs no route-scoped resolution. Replaying a close or comment receipt exposes the issue's current state, so the replay paths now apply the same visibility rules as a fresh request: the route must be the project the receipt was written in or the project the issue lives in now, that current project must not be archived, and it must be inside the caller's host scope. Without this, a receipt could be read through an unrelated project route or after its project was archived. Generated with Claude Code Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
roborev: Combined Review (
|
A keyed comment retry that names the issue by short ID resolves inside the route project. After the issue moves, that resolution fails and the retry returned 404, so the caller could not recover its receipt and might post the comment again in the new project. The receipt written in the route project still names the issue, so the handler now uses it to find the issue UID when the short ID is a suffix of that UID and any qualifier names the route project. The locked, issue-scoped lookup then replays as usual. A key alone cannot steer a retry to a different issue. A full-ULID retry skips ordinary resolution, so it replayed the comment and issue after the issue was soft-deleted. Replay now returns the same not-found response a fresh comment on a deleted issue gets. Generated with Claude Code Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
roborev: Combined Review (
|
2 participants
A close whose response is lost can now be retried without closing twice, closing the wrong issue, or losing the audit trail. Send
Idempotency-KeyonPOST .../actions/close,kata close --idempotency-key, or the MCPidempotency_keyfield. A retry with the same key returns the original receipt (reused: trueplusoriginal_event) instead of running a second close.If-Match(kata close --if-match, MCPrevision) rejects a close of a stale revision with 412.Before this, a client that timed out after the daemon had committed had no safe move. Retrying returned a no-op with no event, and the CLI's follow-up
--commentcould post twice.How a keyed retry is judged:
issue.closedevent that carries the key, found in the events table within the same seven-day window as create and comment keys. No new table or schema version.idempotency_mismatch.retry_protocol: "close-v1"in the body. Older daemons reject the unknown field, so they cannot ignore the headers and run an unguarded close. API schema version is now 0.15.0.Boundary: hook delivery is still an in-memory queue. A daemon crash between commit and publish drops hook jobs for a keyed close the same way it does for every other mutation today.
Also in this PR: a comment idempotency key now identifies one request to one issue rather than one per project, so the same key on two issues posts two comments. A comment retry survives a project move whether it names the issue by full ULID or by short ID through the original project route. A replay of a soft-deleted issue returns not found like a fresh comment would. PostgreSQL advisory-lock identities are hashed so keys with arbitrary bytes are safe.