fix(triggers): suppress redundant progress-comment DELETE after gadget cleanup#1222
Merged
zbigniewsobiecki merged 3 commits intodevfrom Apr 29, 2026
Merged
Conversation
Three production hardening fixes derived from the 2026-04-29 24h log/webhook audit. Discovered alongside PR #1220 (workItemId-on-respond-to-* fix). Failure modes: A) Linear PM-ack silently skipped on PM-focused agents (24/day) B) Pipeline-capacity gate fails open on every PM status-changed (32/day) C) Progress-comment double-delete race produces 404 log spam (72/day) Decomposed per spec strategic decision #8 into three independent plans (none blocks another): 1-pm-ack-coverage (consolidate dispatch via manifest registry) 2-capacity-gate-pm-scope (shared adapter helper, fail-closed semantics) 3-progress-comment-double-delete (consumed-flag, gated fallback, 404-DEBUG) This commit ships scaffolding only. Plan 3's implementation lands in this branch; plans 1 and 2 follow as their own PRs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t mid-run cleanup Plan 017/3 (progress-comment-double-delete). Closes failure mode C from spec 017's 24h log audit on 2026-04-29. Root cause: deleteProgressCommentOnSuccess (post-agent hook) read sessionState.initialCommentId, fell back to result.agentInput.ackCommentId when session state was empty, and issued a DELETE. But "session state cleared by an in-run gadget" was indistinguishable from "session state never populated", so the fallback fired and re-deleted comments that gadgets had already disposed of mid-run. GitHub returned 404 and produced 72 WARN entries per day on cascade-router (no functional impact — the comment WAS deleted — but the noise dominated WARN volume and obscured real failures). Three changes: 1. New `initialCommentIdConsumed: boolean` flag on SessionStateData (default false). Both `deleteInitialComment` (gadget-driven) and `clearInitialComment` (sidecar-driven) set it to true after disposing of the comment. Distinguishes "had a comment, now gone" from "never had one". 2. `deleteProgressCommentOnSuccess` reads the flag first; if true, skips the entire deletion path including the legacy `agentInput.ackCommentId` fallback. The legacy fallback continues to work for paths that never populated session state (consumed stays false). 3. `githubClient.deletePRComment` treats HTTP 404 as success-equivalent under RFC-7231 idempotency: returns without throwing, logs at DEBUG with the comment id. Other HTTP errors (5xx, 401, network) continue to throw. Defense in depth: even if a future regression of similar shape recurs, the WARN volume stays clean while a DEBUG breadcrumb persists for audit. Test coverage: +7 tests in sessionState (new `initialCommentIdConsumed flag` describe block), +3 in ack-comments (consumed-bypass + headline regression pin + legacy-fallback preservation), +4 in client (404 → DEBUG + 5xx/401/network rejection regression pins). Full suite: 470 files / 8649 tests passing (+14 from baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merged
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Failed to delete progress comment after agent success, all 404 from GitHub) caused by the post-agent cleanup hook redundantly deleting comments that in-run gadgets had already disposed of. No functional impact (comment IS deleted), but the noise dominated WARN volume and obscured real failures.initialCommentIdConsumed: booleanon session state, set bydeleteInitialCommentandclearInitialComment; (2) post-agent hook gates the deletion path (including the legacyagentInput.ackCommentIdfallback) on!consumed; (3)githubClient.deletePRCommenttreats HTTP 404 as success (RFC-7231 idempotency, defense in depth) and logs at DEBUG. Other HTTP errors continue to throw.Test plan
SessionStateData.initialCommentIdConsumed(default value, recordInitialComment doesn't flip, deleteInitialComment success/error semantics, clearInitialComment also flips, init resets).deleteProgressCommentOnSuccess(consumed-bypass, headline regression pin againstackCommentIdfallback, legacy fallback preservation).deletePRComment404 idempotency (404→DEBUG no-throw, 5xx/401/network throw regression pins).npm test(full unit suite): 470 files / 8649 tests passing (+14 from baseline).npm run typecheckclean.npm run lintclean.Failed to delete progress comment after agent successon cascade-router drops to <1/24h under normal operation.🤖 Generated with Claude Code