Skip to content

[codex] fix(comments): make durable effects retryable - #1243

Closed
kkroo wants to merge 1 commit into
masterfrom
codex/supersede-pr-903-comment-effects
Closed

[codex] fix(comments): make durable effects retryable#1243
kkroo wants to merge 1 commit into
masterfrom
codex/supersede-pr-903-comment-effects

Conversation

@kkroo

@kkroo kkroo commented Aug 10, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the control plane for AI-agent companies, where comments can trigger durable operational work such as references, activity, and wakes.
  • Idempotent comments must be safe to replay after a request, worker, or replica fails partway through that work.
  • The predecessor implementation in fix(comments): durable effect ledger for idempotent comment pipeline (supersedes #836) #903 adds a durable per-comment effect ledger with transactionally enqueued intents and lease-based claims.
  • Its review found that an effect could become terminally failed, while the comment was still stamped processed even though that effect never ran.
  • This pull request independently succeeds fix(comments): durable effect ledger for idempotent comment pipeline (supersedes #836) #903 by preserving the ledger design while keeping failed effects replayable through database-backed backoff and reconciliation.
  • The benefit is that transient delivery failures cannot permanently lose a required comment side effect.

Linked Issues or Issue Description

Refs #903 (App-authored predecessor; this independent successor does not modify or close it). Related internal tracking: BLO-19116.

Problem: after ten failures, a comment effect was parked as failed, excluded from retries, and treated as settled. The parent comment then received idempotency_processed_at although a sink such as wake delivery had not completed.

Expected behavior: every non-processed effect remains outstanding. Repeated failures use durable backoff, including a longer cooldown after the threshold, and the reconciler can reclaim the effect until it completes.

What Changed

  • Added the durable issue_comment_effects ledger migration and schema, including next_attempt_at plus a due-work index.
  • Ported the transactional keyed-comment effect pipeline, lease claims, reconciliation, and wake idempotency receipts from fix(comments): durable effect ledger for idempotent comment pipeline (supersedes #836) #903 onto current master.
  • Changed failure handling so failed is a backoff state rather than terminal settlement; comments stamp processed only after every effect is processed.
  • Added regression coverage for exhausted-effect recovery through the actual reconciler path.

Verification

  • pnpm --filter @paperclipai/db check:migrations — passed.
  • pnpm --filter @paperclipai/server exec vitest run --no-file-parallelism --maxWorkers=1 --reporter=verbose src/services/issue-comment-effects.test.ts — 15 passed.
  • pnpm --filter @paperclipai/server exec vitest run --no-file-parallelism --maxWorkers=1 --reporter=verbose src/__tests__/issue-update-comment-wakeup-routes.test.ts src/services/wake-idempotency.test.ts — 25 passed.
  • pnpm --filter @paperclipai/db typecheck and pnpm --filter @paperclipai/server typecheck — passed.
  • pnpm --filter @paperclipai/db build and pnpm --filter @paperclipai/server build — passed.

Risks

  • The migration adds a new ledger table and indexes; it is additive and has no data backfill.
  • Failed effects now remain visible as unfinished until recovery, which is intentional and avoids falsely acknowledging lost side effects.
  • Backoff delays recovery during sustained downstream outages, but prevent hot-loop retries while retaining eventual replay.

For core feature work, check ROADMAP.md first and discuss it in #dev before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See CONTRIBUTING.md.

Model Used

OpenAI GPT-5 Codex — tool-assisted reasoning and code execution; the session does not expose a more specific serving-model version or context-window size.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have described the underlying bug in this PR and linked the predecessor
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots (not applicable: no UI change)
  • I have updated relevant documentation to reflect my changes (not applicable: no user-facing contract or command changed)
  • I have considered and documented any risks above
  • All Paperclip CI gates are green (awaiting GitHub Actions)
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups (awaiting review)
  • I will address all Greptile and reviewer comments before requesting merge

Keep failed comment side effects unsettled and recover them through durable backoff and reconciliation.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@allyblockcast

allyblockcast Bot commented Aug 10, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-19116

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 10, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-19116

@kkroo

kkroo commented Sep 1, 2026

Copy link
Copy Markdown
Author

Closing as stale — this draft can no longer be landed as a diff.

Measured against master ac91e966:

  • 808 commits behind
  • 6 conflicting file(s) (git merge-tree --write-tree)

At that distance the surrounding code has moved out from under the patch, so "resolve the conflicts" would mean rewriting it against a codebase it was never written for — not merging it. Reopening is cheap if the branch is still wanted; the commits are untouched.

Closing the PR does not close the underlying issue. Where this PR referenced a BLO issue, that issue remains the record of intent and the place to re-implement from, against current master.

No judgement here on whether the original problem still exists — I did not verify that per-PR, and it should be re-checked before any re-implementation, since some of these may already have been fixed independently.

@kkroo kkroo closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant