mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-03 01:46:55 +02:00
test(eng-batching): read the report target as a field, not a spelling
The next targeted rerun (Claude Code 2.1.284) again asked eleven separate native questions and again counted zero: its briefs named no plan and its report declared '- **Review target (fixed):** `/abs/PLAN.md`' under '# Eng Review — PLAN.md: <plan>'. An unsourced brief now inherits the one current target field that names a PLAN.md file, whatever its list or emphasis markup; its ledger record still supplies the cited finding and must reproduce the brief exactly. A brief that names its plan must still match the report title. Replays of all three captures count 9, 9 and 3; controls reject a foreign, duplicate or missing target and an archived title.
This commit is contained in:
garrytan committed
1 parent
e758fbe96d
commit
475667ff5d
3 files changed
+405
-11
No files matched your search
+359
@@ -0,0 +1,359 @@
|
||||
{
|
||||
"source": "local targeted rerun smoke-2.1.284-1790711269 (Claude Code 2.1.284) of plan-eng-multi-finding-batching: observation.json transcript.calls and the saved report replayed from its Write/Edit inputs",
|
||||
"recordedOutcome": {
|
||||
"outcome": "collection_complete",
|
||||
"step0Count": 11,
|
||||
"reviewCount": 0
|
||||
},
|
||||
"calls": [
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01XnWRrh4F44QdznytAGmriy",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Reuse the job library's retry hooks or roll a custom scheduler?\nProject/branch/task: main branch, retry-framework plan; adding retries to 5 background workers.\nELI10: The job library you already use has retry hooks built in, and your plan says your custom version would be \"the same shape.\" Building your own copy inside each worker means five hand-written schedulers to keep correct, versus configuring one curve the library already knows how to run. The plan's reason for going custom is \"full control over the curve,\" and most retry hook APIs give you that through a backoff callback.\nStakes if we pick wrong: five bespoke schedulers drift apart, each grows its own bugs (no jitter, no cap, retry storms), and nobody at 3am knows which curve a given worker actually runs.\nRecommendation: A because the plan admits the library version has the same shape, and a custom curve is usually a config callback, not a new scheduler.\nCompleteness: A=9/10, B=5/10, C=n/a (investigation, decides nothing)\nPros / cons:\nA) Library hooks + custom curve (recommended)\n \u2705 One scheduler the library already tests; the curve becomes a per-worker config function (human: ~1 day / CC: ~20 min)\n \u2705 Attempt-count persistence, max attempts and dead-letter usually come along for free with the hooks\n \u274c If the hook API cannot accept an arbitrary curve function, that is a new fact and R1 reopens\nB) Custom inline scheduler (as planned)\n \u2705 Total control of delay math and logging, independent of the library's release cycle\n \u2705 No dependency on hook semantics nobody has read yet (human: ~3 days / CC: ~1 hr)\n \u274c Five hand-rolled schedulers to keep in sync, plus reimplementing attempt persistence and terminal handling\nC) Investigate first: bounded read of the hook API\n \u2705 Settles \"can the hooks take a custom curve\" with evidence before committing either way\n \u2705 Cheap: ~30 min human / ~3 min CC reading the hook signature and docs\n \u274c Decides nothing; R1 stays pending and the review pauses on this question\nNet: You are trading a library dependency you already carry for five copies of scheduler code you would own forever.",
|
||||
"header": "Retry mechanism",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Library hooks + custom curve (recommended)",
|
||||
"description": "Use the job library's built-in retry hooks; supply each worker's backoff curve as a callback/config. One scheduler the library already tests. Attempt persistence, max attempts and dead-letter usually included. Human ~1 day / CC ~20 min. Risk: if the hook API cannot take an arbitrary curve, R1 reopens. Completeness 9/10."
|
||||
},
|
||||
{
|
||||
"label": "Custom inline scheduler (as planned)",
|
||||
"description": "Roll the exponential-backoff scheduler inline in each of the 5 workers as PLAN.md:7-9 proposes. Total control of delay math and logging. Human ~3 days / CC ~1 hr. Cost: five schedulers to keep in sync, plus attempt persistence and terminal handling rebuilt by hand. Completeness 5/10."
|
||||
},
|
||||
{
|
||||
"label": "Investigate hook API first",
|
||||
"description": "Bounded ~30 min human / ~3 min CC read of the library's retry hook signature and docs, then return to this question. Approves nothing; R1 stays pending; R2\u2013R6 unchanged."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D1 \u2014 Reuse the job library's retry hooks or roll a custom scheduler?\nProject/branch/task: main branch, retry-framework plan; adding retries to 5 background workers.\nELI10: The job library you already use has retry hooks built in, and your plan says your custom version would be \"the same shape.\" Building your own copy inside each worker means five hand-written schedulers to keep correct, versus configuring one curve the library already knows how to run. The plan's reason for going custom is \"full control over the curve,\" and most retry hook APIs give you that through a backoff callback.\nStakes if we pick wrong: five bespoke schedulers drift apart, each grows its own bugs (no jitter, no cap, retry storms), and nobody at 3am knows which curve a given worker actually runs.\nRecommendation: A because the plan admits the library version has the same shape, and a custom curve is usually a config callback, not a new scheduler.\nCompleteness: A=9/10, B=5/10, C=n/a (investigation, decides nothing)\nPros / cons:\nA) Library hooks + custom curve (recommended)\n \u2705 One scheduler the library already tests; the curve becomes a per-worker config function (human: ~1 day / CC: ~20 min)\n \u2705 Attempt-count persistence, max attempts and dead-letter usually come along for free with the hooks\n \u274c If the hook API cannot accept an arbitrary curve function, that is a new fact and R1 reopens\nB) Custom inline scheduler (as planned)\n \u2705 Total control of delay math and logging, independent of the library's release cycle\n \u2705 No dependency on hook semantics nobody has read yet (human: ~3 days / CC: ~1 hr)\n \u274c Five hand-rolled schedulers to keep in sync, plus reimplementing attempt persistence and terminal handling\nC) Investigate first: bounded read of the hook API\n \u2705 Settles \"can the hooks take a custom curve\" with evidence before committing either way\n \u2705 Cheap: ~30 min human / ~3 min CC reading the hook signature and docs\n \u274c Decides nothing; R1 stays pending and the review pauses on this question\nNet: You are trading a library dependency you already carry for five copies of scheduler code you would own forever.": "Library hooks + custom curve (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T19:51:49.159Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_015Ri9YxuhvdexzxG5KqBTTc",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D2 \u2014 What delivery guarantee does processWebhookJob() keep once it can retry?\nProject/branch/task: main branch, retry-framework plan; the webhook worker is one of the 5 workers gaining retries through library hooks (D1).\nELI10: Today the webhook worker sends each event at most once: if the send fails or times out, the event is dropped, never duplicated. A retry cannot tell \"the request never arrived\" apart from \"it arrived but the response got lost,\" so any retry after a timeout can deliver the same event twice. Adding retries silently flips the guarantee from at-most-once to at-least-once. That is a contract change your webhook receivers depend on, and the plan does not name it.\nStakes if we pick wrong: receivers that are not idempotent process duplicate events (double emails, double charges, double state transitions); or, if we keep dropping on ambiguity, the retry framework never fixes the webhook worker's lost events.\nRecommendation: B because losing events is usually worse than duplicates, and a stable idempotency key makes duplicates safe for receivers; this is still a contract call you know better than the review does.\nNote: options differ in kind, not coverage \u2014 no completeness score.\nPros / cons:\nA) Keep at-most-once: retry only provably-unsent failures\n \u2705 No duplicate deliveries ever; existing receivers keep working with no change on their side\n \u2705 Still recovers the clear cases: connection refused, DNS failure, local enqueue error (human: ~1 day / CC: ~30 min)\n \u274c Timeouts and 5xx-after-send still drop events, so the biggest source of loss stays; needs per-attempt failure classification\nB) Move to at-least-once with a stable idempotency key (recommended)\n \u2705 Every event eventually reaches the receiver; retries after timeouts are safe because the event id stays constant across attempts\n \u2705 Matches Stripe, Shopify and Svix practice; receivers dedupe on the key (human: ~1.5 days / CC: ~30 min incl. docs)\n \u274c Contract change: receivers must dedupe; needs a documented header, changelog entry and migration note for existing receivers\nC) Exclude processWebhookJob() from retries\n \u2705 Zero semantic change for receivers; the other 4 workers still get retries\n \u2705 Smallest diff and no receiver communication (human: ~1 hr / CC: ~5 min)\n \u274c The webhook worker keeps losing events on every transient failure, which is likely why the plan touched it\nNet: Never-duplicate-but-lossy, never-lossy-but-receivers-must-dedupe, or leave the webhook worker exactly as it is.",
|
||||
"header": "Webhook delivery",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Keep at-most-once (retry only pre-send failures)",
|
||||
"description": "Webhook worker retries only failures provably raised before the request was written (connection refused, DNS, local error). Timeouts and 5xx-after-send still drop the event. No duplicates; receivers unchanged. Needs per-attempt failure classification. Human ~1 day / CC ~30 min."
|
||||
},
|
||||
{
|
||||
"label": "At-least-once + idempotency key (recommended)",
|
||||
"description": "Webhook worker retries all transient failures; every delivery carries a stable event id / idempotency key header constant across attempts. Documented contract change with changelog and receiver migration note. Receivers dedupe on the key. Human ~1.5 days / CC ~30 min."
|
||||
},
|
||||
{
|
||||
"label": "Exclude webhook worker from retries",
|
||||
"description": "`processWebhookJob()` keeps today's at-most-once, no-retry behavior; the other 4 workers retry via library hooks per D1. Smallest diff, no receiver impact, webhook events still lost on transient failure. Human ~1 hr / CC ~5 min."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D2 \u2014 What delivery guarantee does processWebhookJob() keep once it can retry?\nProject/branch/task: main branch, retry-framework plan; the webhook worker is one of the 5 workers gaining retries through library hooks (D1).\nELI10: Today the webhook worker sends each event at most once: if the send fails or times out, the event is dropped, never duplicated. A retry cannot tell \"the request never arrived\" apart from \"it arrived but the response got lost,\" so any retry after a timeout can deliver the same event twice. Adding retries silently flips the guarantee from at-most-once to at-least-once. That is a contract change your webhook receivers depend on, and the plan does not name it.\nStakes if we pick wrong: receivers that are not idempotent process duplicate events (double emails, double charges, double state transitions); or, if we keep dropping on ambiguity, the retry framework never fixes the webhook worker's lost events.\nRecommendation: B because losing events is usually worse than duplicates, and a stable idempotency key makes duplicates safe for receivers; this is still a contract call you know better than the review does.\nNote: options differ in kind, not coverage \u2014 no completeness score.\nPros / cons:\nA) Keep at-most-once: retry only provably-unsent failures\n \u2705 No duplicate deliveries ever; existing receivers keep working with no change on their side\n \u2705 Still recovers the clear cases: connection refused, DNS failure, local enqueue error (human: ~1 day / CC: ~30 min)\n \u274c Timeouts and 5xx-after-send still drop events, so the biggest source of loss stays; needs per-attempt failure classification\nB) Move to at-least-once with a stable idempotency key (recommended)\n \u2705 Every event eventually reaches the receiver; retries after timeouts are safe because the event id stays constant across attempts\n \u2705 Matches Stripe, Shopify and Svix practice; receivers dedupe on the key (human: ~1.5 days / CC: ~30 min incl. docs)\n \u274c Contract change: receivers must dedupe; needs a documented header, changelog entry and migration note for existing receivers\nC) Exclude processWebhookJob() from retries\n \u2705 Zero semantic change for receivers; the other 4 workers still get retries\n \u2705 Smallest diff and no receiver communication (human: ~1 hr / CC: ~5 min)\n \u274c The webhook worker keeps losing events on every transient failure, which is likely why the plan touched it\nNet: Never-duplicate-but-lossy, never-lossy-but-receivers-must-dedupe, or leave the webhook worker exactly as it is.": "Keep at-most-once (retry only pre-send failures)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T19:54:01.993Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_0132i9qkrzXUVyovu4GZxADn",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D3 \u2014 When a job runs out of retries, where does it go?\nProject/branch/task: main branch, retry-framework plan; retry bounds for all 5 workers running through library hooks (D1).\nELI10: Right now the plan describes the curve between retries but never says how many retries there are or what happens to a job that keeps failing. Without a limit, a poisoned job retries forever and eats worker capacity. With a limit but no landing spot, the job disappears with one log line nobody reads. A dead-letter store keeps the failed job, its payload reference and its last error so someone can inspect and replay it.\nStakes if we pick wrong: either an infinite-retry job starves the queue, or real work silently vanishes after the last attempt and the first sign is a customer asking where their data went.\nRecommendation: A because a dead-letter store plus an alert is a few dozen lines with library hooks, and it turns \"job vanished\" into \"job parked, here is why.\"\nCompleteness: A=10/10, B=5/10, C=3/10\nPros / cons:\nA) Bounded attempts + dead-letter store + alert (recommended)\n \u2705 Exhausted or fatal jobs are kept with last error and attempt history; operators can inspect and replay (human: ~1 day / CC: ~20 min)\n \u2705 Metric and alert on dead-letter growth turns a silent failure into a page at the right time\n \u274c One more table or queue to own, plus a small replay path to build and test\nB) Bounded attempts, log and drop\n \u2705 Simplest bound: `maxAttempts` default 5 per worker, one error log on exhaustion (human: ~2 hr / CC: ~5 min)\n \u2705 No new storage; nothing to operate\n \u274c Exhausted jobs are gone; recovery means replaying from upstream sources by hand, if that is even possible\nC) Leave to library defaults\n \u2705 Zero plan text and zero decision now\n \u2705 Whatever the library does is at least consistent across the 5 workers\n \u274c Nobody knows the limit or the terminal behavior until an incident teaches them; 3am failure mode\nNet: You are trading one small dead-letter store for never having to ask \"where did that job go.\"",
|
||||
"header": "Retry exhaustion",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Bounded + dead-letter + alert (recommended)",
|
||||
"description": "`maxAttempts` default 5 with per-worker override. On exhaustion or fatal error the job lands in a dead-letter store (table or queue) with last error, attempt history and payload reference. Metric and alert on dead-letter growth. Manual replay path. Human ~1 day / CC ~20 min. Completeness 10/10."
|
||||
},
|
||||
{
|
||||
"label": "Bounded, log and drop",
|
||||
"description": "`maxAttempts` default 5 with per-worker override. On exhaustion, log at error level with the last error and drop the job. No new storage, no replay. Human ~2 hr / CC ~5 min. Completeness 5/10."
|
||||
},
|
||||
{
|
||||
"label": "Library defaults, unspecified",
|
||||
"description": "Do not write attempt limits or terminal behavior into the plan; accept whatever the library does by default. Completeness 3/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D3 \u2014 When a job runs out of retries, where does it go?\nProject/branch/task: main branch, retry-framework plan; retry bounds for all 5 workers running through library hooks (D1).\nELI10: Right now the plan describes the curve between retries but never says how many retries there are or what happens to a job that keeps failing. Without a limit, a poisoned job retries forever and eats worker capacity. With a limit but no landing spot, the job disappears with one log line nobody reads. A dead-letter store keeps the failed job, its payload reference and its last error so someone can inspect and replay it.\nStakes if we pick wrong: either an infinite-retry job starves the queue, or real work silently vanishes after the last attempt and the first sign is a customer asking where their data went.\nRecommendation: A because a dead-letter store plus an alert is a few dozen lines with library hooks, and it turns \"job vanished\" into \"job parked, here is why.\"\nCompleteness: A=10/10, B=5/10, C=3/10\nPros / cons:\nA) Bounded attempts + dead-letter store + alert (recommended)\n \u2705 Exhausted or fatal jobs are kept with last error and attempt history; operators can inspect and replay (human: ~1 day / CC: ~20 min)\n \u2705 Metric and alert on dead-letter growth turns a silent failure into a page at the right time\n \u274c One more table or queue to own, plus a small replay path to build and test\nB) Bounded attempts, log and drop\n \u2705 Simplest bound: `maxAttempts` default 5 per worker, one error log on exhaustion (human: ~2 hr / CC: ~5 min)\n \u2705 No new storage; nothing to operate\n \u274c Exhausted jobs are gone; recovery means replaying from upstream sources by hand, if that is even possible\nC) Leave to library defaults\n \u2705 Zero plan text and zero decision now\n \u2705 Whatever the library does is at least consistent across the 5 workers\n \u274c Nobody knows the limit or the terminal behavior until an incident teaches them; 3am failure mode\nNet: You are trading one small dead-letter store for never having to ask \"where did that job go.\"": "Bounded + dead-letter + alert (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T19:55:16.802Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01GMRg1vxVPnVCefhfuCSKEw",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D4 \u2014 Should retry delays be randomized (jitter)?\nProject/branch/task: main branch, retry-framework plan; the backoff curve each worker supplies to the library hooks (D1).\nELI10: When many jobs fail at the same moment because a shared dependency went down, a pure exponential curve makes them all retry at the same moments too, so the recovering dependency gets hit by a wave on every step. Jitter randomizes each job's delay so the retries spread out. It is one line inside the curve callback each worker already supplies.\nStakes if we pick wrong: synchronized retry waves knock a recovering dependency back over (the classic thundering herd); or, with jitter, per-job retry timing becomes slightly less predictable and tests need a seeded random source.\nRecommendation: A because full jitter gives the least contention in AWS's published analysis and costs one line in a callback you are already writing.\nNote: options differ in kind, not coverage \u2014 no completeness score.\nPros / cons:\nA) Full jitter: random(0, exponentialDelay) (recommended)\n \u2705 Best spread of retries and lowest total contention after a shared outage (AWS Builders' Library)\n \u2705 One line inside the D1 curve callback; RNG injected so tests stay deterministic (human: ~1 hr / CC: ~5 min)\n \u274c An individual retry can fire almost immediately; minimum wait is not guaranteed\nB) Equal jitter: half fixed, half random\n \u2705 Guarantees a minimum wait of half the exponential delay while still spreading retries\n \u2705 Same one-line cost and same injectable RNG as full jitter (human: ~1 hr / CC: ~5 min)\n \u274c Slightly more contention than full jitter in the same analysis, and one more parameter to explain\nC) No jitter: deterministic curve\n \u2705 Fully deterministic; trivial to reason about and to assert exact delays in tests\n \u2705 Zero extra code beyond the exponential curve\n \u274c Every job that failed together retries together; retry storms on recovery are the expected outcome\nNet: One random() call now versus a synchronized retry wave the first time a dependency has a bad hour.",
|
||||
"header": "Jitter",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Full jitter (recommended)",
|
||||
"description": "delay = random(0, exponentialDelay) inside each worker's curve callback. Best spread, lowest contention. RNG injectable so tests are deterministic. Human ~1 hr / CC ~5 min."
|
||||
},
|
||||
{
|
||||
"label": "Equal jitter",
|
||||
"description": "delay = exponentialDelay/2 + random(0, exponentialDelay/2). Guarantees a minimum wait; slightly more contention than full jitter. RNG injectable. Human ~1 hr / CC ~5 min."
|
||||
},
|
||||
{
|
||||
"label": "No jitter",
|
||||
"description": "Deterministic exponential curve, no randomization. Simplest to test; retries synchronize after a shared outage."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D4 \u2014 Should retry delays be randomized (jitter)?\nProject/branch/task: main branch, retry-framework plan; the backoff curve each worker supplies to the library hooks (D1).\nELI10: When many jobs fail at the same moment because a shared dependency went down, a pure exponential curve makes them all retry at the same moments too, so the recovering dependency gets hit by a wave on every step. Jitter randomizes each job's delay so the retries spread out. It is one line inside the curve callback each worker already supplies.\nStakes if we pick wrong: synchronized retry waves knock a recovering dependency back over (the classic thundering herd); or, with jitter, per-job retry timing becomes slightly less predictable and tests need a seeded random source.\nRecommendation: A because full jitter gives the least contention in AWS's published analysis and costs one line in a callback you are already writing.\nNote: options differ in kind, not coverage \u2014 no completeness score.\nPros / cons:\nA) Full jitter: random(0, exponentialDelay) (recommended)\n \u2705 Best spread of retries and lowest total contention after a shared outage (AWS Builders' Library)\n \u2705 One line inside the D1 curve callback; RNG injected so tests stay deterministic (human: ~1 hr / CC: ~5 min)\n \u274c An individual retry can fire almost immediately; minimum wait is not guaranteed\nB) Equal jitter: half fixed, half random\n \u2705 Guarantees a minimum wait of half the exponential delay while still spreading retries\n \u2705 Same one-line cost and same injectable RNG as full jitter (human: ~1 hr / CC: ~5 min)\n \u274c Slightly more contention than full jitter in the same analysis, and one more parameter to explain\nC) No jitter: deterministic curve\n \u2705 Fully deterministic; trivial to reason about and to assert exact delays in tests\n \u2705 Zero extra code beyond the exponential curve\n \u274c Every job that failed together retries together; retry storms on recovery are the expected outcome\nNet: One random() call now versus a synchronized retry wave the first time a dependency has a bad hour.": "Full jitter (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T19:56:30.842Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01XLVRABVWXpSZRBNni2esSC",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D5 \u2014 Should the backoff delay have a ceiling?\nProject/branch/task: main branch, retry-framework plan; the curve parameters each worker passes to the library hooks (D1, jittered per D4).\nELI10: Exponential backoff doubles the wait after each failure. That is fine for 5 attempts, but D3 lets each worker raise its attempt count, and a worker set to 15 attempts from a 1 second base would wait about 4.5 hours before its last try; at 20 attempts it would wait 6 days. A cap says \"never wait longer than X between attempts,\" so the curve grows and then flattens. It is one min() call in the callback.\nStakes if we pick wrong: without a cap, a worker with a higher attempt count silently turns into a multi-day wait that looks like a stuck job; with a cap, one more number to document per worker.\nRecommendation: A because the cap is one min() and it makes \"how long can this job be delayed\" a question with an answer.\nCompleteness: A=9/10, B=4/10\nPros / cons:\nA) Cap each delay: default 10 min, per-worker override (recommended)\n \u2705 Worst-case wait between attempts is bounded and documented for every worker (human: ~1 hr / CC: ~5 min)\n \u2705 Also pins the curve defaults (base 1 s, multiplier 2) so all 5 workers start from the same documented numbers\n \u274c One more config value per worker to document and keep sane alongside maxAttempts\nB) No cap\n \u2705 Zero code; the curve is exactly the exponential the plan describes\n \u2705 Fewer knobs to explain\n \u274c Any worker that raises maxAttempts past ~12 gets hour-to-day waits nobody intended\nNet: One min() now versus a job that looks stuck for six days the first time someone bumps an attempt count.",
|
||||
"header": "Delay cap",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Cap each delay (recommended)",
|
||||
"description": "delay = min(jitteredExponential, maxDelay). `maxDelay` default 10 minutes with per-worker override. Curve defaults documented: base 1 s, multiplier 2, per-worker override. Human ~1 hr / CC ~5 min. Completeness 9/10."
|
||||
},
|
||||
{
|
||||
"label": "No cap",
|
||||
"description": "Raw exponential curve with no ceiling. Zero code, fewer knobs; high attempt counts produce hour-to-day waits. Completeness 4/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D5 \u2014 Should the backoff delay have a ceiling?\nProject/branch/task: main branch, retry-framework plan; the curve parameters each worker passes to the library hooks (D1, jittered per D4).\nELI10: Exponential backoff doubles the wait after each failure. That is fine for 5 attempts, but D3 lets each worker raise its attempt count, and a worker set to 15 attempts from a 1 second base would wait about 4.5 hours before its last try; at 20 attempts it would wait 6 days. A cap says \"never wait longer than X between attempts,\" so the curve grows and then flattens. It is one min() call in the callback.\nStakes if we pick wrong: without a cap, a worker with a higher attempt count silently turns into a multi-day wait that looks like a stuck job; with a cap, one more number to document per worker.\nRecommendation: A because the cap is one min() and it makes \"how long can this job be delayed\" a question with an answer.\nCompleteness: A=9/10, B=4/10\nPros / cons:\nA) Cap each delay: default 10 min, per-worker override (recommended)\n \u2705 Worst-case wait between attempts is bounded and documented for every worker (human: ~1 hr / CC: ~5 min)\n \u2705 Also pins the curve defaults (base 1 s, multiplier 2) so all 5 workers start from the same documented numbers\n \u274c One more config value per worker to document and keep sane alongside maxAttempts\nB) No cap\n \u2705 Zero code; the curve is exactly the exponential the plan describes\n \u2705 Fewer knobs to explain\n \u274c Any worker that raises maxAttempts past ~12 gets hour-to-day waits nobody intended\nNet: One min() now versus a job that looks stuck for six days the first time someone bumps an attempt count.": "Cap each delay (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T19:57:33.114Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01SrWycxMxofjLp1hxkciPj9",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D6 \u2014 Which errors should the four non-webhook workers retry, and which go straight to dead-letter?\nProject/branch/task: main branch, retry-framework plan; error handling inside the 4 non-webhook workers' library retry hooks (D1). The webhook worker's rule is already fixed by D2.\nELI10: Not every failure is worth retrying. A timeout or a \"service busy\" reply will likely pass on the next try. A validation error, a missing record or a bug that throws will fail the same way five times in a row, burning worker time and delaying the dead-letter record (D3) by the whole backoff curve. Classifying errors sends the hopeless ones to dead-letter immediately and spends retries only on the ones that can recover. The open question is what to do with an error nobody has classified yet.\nStakes if we pick wrong: either a bug retries five times per job across a whole queue before anyone sees it, or a transient error that nobody thought to list dead-letters real work on its first failure.\nRecommendation: A because classification is a short list per worker, and defaulting unknown errors to retryable never loses work: the worst case is five wasted attempts, not a dropped job.\nCompleteness: A=10/10, B=5/10, C=8/10\nPros / cons:\nA) Classify; unknown errors retry (recommended)\n \u2705 Hopeless errors (validation, 4xx, missing record, TypeError) land in dead-letter on attempt 1 with the real cause visible (human: ~half day / CC: ~15 min)\n \u2705 Unlisted errors still retry, so a forgotten transient class costs attempts, never data\n \u274c Each worker maintains a small error-class list, and a new fatal class retries needlessly until someone adds it\nB) Retry everything until maxAttempts\n \u2705 No lists to maintain; identical behavior in all 4 workers (human: ~0 / CC: ~0)\n \u2705 Impossible to misclassify a transient error as fatal\n \u274c A deploy with a bug retries every affected job 5 times over the full curve before dead-lettering; queue capacity burns and diagnosis is delayed\nC) Classify; unknown errors are fatal\n \u2705 Zero wasted attempts on anything not explicitly known to be transient (human: ~half day / CC: ~15 min)\n \u2705 Dead-letter fills fast, so new error classes surface quickly\n \u274c Any transient error missing from the list dead-letters real work on its first failure, which is the exact loss the retry framework exists to prevent\nNet: A short list per worker plus a safe default, versus either wasted retries on bugs or lost work on unlisted transients.",
|
||||
"header": "Error classes",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Classify; unknown \u2192 retryable (recommended)",
|
||||
"description": "Each of the 4 workers declares retryable classes (timeout, connection error, 429/503, deadlock/serialization failure) and fatal classes (validation error, 4xx other than 429, missing record, programming error). Fatal \u2192 dead-letter immediately without consuming attempts. Unclassified errors retry. Human ~half day / CC ~15 min. Completeness 10/10."
|
||||
},
|
||||
{
|
||||
"label": "Retry everything to maxAttempts",
|
||||
"description": "No classification. Every error consumes an attempt until `maxAttempts`, then dead-letter per D3. Zero code; bugs retry 5 times per job. Completeness 5/10."
|
||||
},
|
||||
{
|
||||
"label": "Classify; unknown \u2192 fatal",
|
||||
"description": "Same declared classes as A, but unclassified errors go to dead-letter immediately. No wasted attempts; unlisted transient errors lose work on first failure. Human ~half day / CC ~15 min. Completeness 8/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D6 \u2014 Which errors should the four non-webhook workers retry, and which go straight to dead-letter?\nProject/branch/task: main branch, retry-framework plan; error handling inside the 4 non-webhook workers' library retry hooks (D1). The webhook worker's rule is already fixed by D2.\nELI10: Not every failure is worth retrying. A timeout or a \"service busy\" reply will likely pass on the next try. A validation error, a missing record or a bug that throws will fail the same way five times in a row, burning worker time and delaying the dead-letter record (D3) by the whole backoff curve. Classifying errors sends the hopeless ones to dead-letter immediately and spends retries only on the ones that can recover. The open question is what to do with an error nobody has classified yet.\nStakes if we pick wrong: either a bug retries five times per job across a whole queue before anyone sees it, or a transient error that nobody thought to list dead-letters real work on its first failure.\nRecommendation: A because classification is a short list per worker, and defaulting unknown errors to retryable never loses work: the worst case is five wasted attempts, not a dropped job.\nCompleteness: A=10/10, B=5/10, C=8/10\nPros / cons:\nA) Classify; unknown errors retry (recommended)\n \u2705 Hopeless errors (validation, 4xx, missing record, TypeError) land in dead-letter on attempt 1 with the real cause visible (human: ~half day / CC: ~15 min)\n \u2705 Unlisted errors still retry, so a forgotten transient class costs attempts, never data\n \u274c Each worker maintains a small error-class list, and a new fatal class retries needlessly until someone adds it\nB) Retry everything until maxAttempts\n \u2705 No lists to maintain; identical behavior in all 4 workers (human: ~0 / CC: ~0)\n \u2705 Impossible to misclassify a transient error as fatal\n \u274c A deploy with a bug retries every affected job 5 times over the full curve before dead-lettering; queue capacity burns and diagnosis is delayed\nC) Classify; unknown errors are fatal\n \u2705 Zero wasted attempts on anything not explicitly known to be transient (human: ~half day / CC: ~15 min)\n \u2705 Dead-letter fills fast, so new error classes surface quickly\n \u274c Any transient error missing from the list dead-letters real work on its first failure, which is the exact loss the retry framework exists to prevent\nNet: A short list per worker plus a safe default, versus either wasted retries on bugs or lost work on unlisted transients.": "Classify; unknown \u2192 retryable (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T19:58:40.484Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_013bXU7e1WUvhP4r6Nh2cigC",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D7 \u2014 One shared retry-policy module, or five copies and \"refactor later\"?\nProject/branch/task: main branch, retry-framework plan; how the 5 workers carry the behavior approved in D3\u2013D6.\nELI10: After D1 the library does the scheduling, but every worker still has to hand it the same four things: a jittered, capped curve, an error classifier, a dead-letter handoff and an attempt log line. The plan copies that block into five files and promises to clean up later. \"Later\" for copy-pasted retry code usually means the fifth copy drifts (no cap, wrong jitter) and nobody notices until an incident. The alternative is one small module that each worker configures with its own numbers and error lists.\nStakes if we pick wrong: five curves that silently disagree, five places to fix the next retry bug, and five test suites that each cover a slightly different subset; or, with a shared module, one bug that hits all five workers at once (mitigated by the module's own tests).\nRecommendation: A because the behavior is identical by construction (D3\u2013D6 fixed it), the module is under 100 lines, and it removes more lines than it adds while making the retry rules testable once.\nCompleteness: A=10/10, B=4/10, C=7/10\nPros / cons:\nA) One shared retry-policy module (recommended)\n \u2705 Curve, classifier, dead-letter handoff, attempt log and config validation are tested once and behave the same in all 5 workers (human: ~1 day / CC: ~20 min)\n \u2705 Estimated 15\u201390 implementation lines saved; the helper's test suite replaces five near-duplicate suites\n \u274c A bug in the module reaches all 5 workers; the module's own tests are the guard\nB) Five inline copies, refactor later (as planned)\n \u2705 No shared dependency between workers; each can be changed in isolation (human: ~1.5 days / CC: ~30 min)\n \u2705 Matches the plan text exactly; nothing new to name or place\n \u274c Five copies to keep in sync and five test suites to write; \"later\" rarely arrives for retry glue\nC) Extract the curve builder only\n \u2705 The math most likely to drift (jitter + cap) lives in one place (human: ~1 day / CC: ~15 min)\n \u2705 Smaller shared surface than A\n \u274c Classifier, dead-letter handoff and log line are still copied five times, so most of the duplication and its tests remain\nNet: One under-100-line module now, or five copies plus a promise.",
|
||||
"header": "Shared module",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "One shared retry-policy module (recommended)",
|
||||
"description": "Small module: buildBackoff (base/multiplier/maxDelay/rng), classify (per-worker retryable/fatal lists), toDeadLetter (persist + metric), logAttempt, and startup config validation. All 5 workers use it with their own values. Human ~1 day / CC ~20 min. Completeness 10/10."
|
||||
},
|
||||
{
|
||||
"label": "Five inline copies (as planned)",
|
||||
"description": "Each worker carries its own curve, classifier, dead-letter handoff and log line; refactor deferred. Human ~1.5 days / CC ~30 min. Completeness 4/10."
|
||||
},
|
||||
{
|
||||
"label": "Extract curve builder only",
|
||||
"description": "Shared buildBackoff (jitter + cap) only; classifier, dead-letter handoff and log line stay inline in each of the 5 workers. Human ~1 day / CC ~15 min. Completeness 7/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D7 \u2014 One shared retry-policy module, or five copies and \"refactor later\"?\nProject/branch/task: main branch, retry-framework plan; how the 5 workers carry the behavior approved in D3\u2013D6.\nELI10: After D1 the library does the scheduling, but every worker still has to hand it the same four things: a jittered, capped curve, an error classifier, a dead-letter handoff and an attempt log line. The plan copies that block into five files and promises to clean up later. \"Later\" for copy-pasted retry code usually means the fifth copy drifts (no cap, wrong jitter) and nobody notices until an incident. The alternative is one small module that each worker configures with its own numbers and error lists.\nStakes if we pick wrong: five curves that silently disagree, five places to fix the next retry bug, and five test suites that each cover a slightly different subset; or, with a shared module, one bug that hits all five workers at once (mitigated by the module's own tests).\nRecommendation: A because the behavior is identical by construction (D3\u2013D6 fixed it), the module is under 100 lines, and it removes more lines than it adds while making the retry rules testable once.\nCompleteness: A=10/10, B=4/10, C=7/10\nPros / cons:\nA) One shared retry-policy module (recommended)\n \u2705 Curve, classifier, dead-letter handoff, attempt log and config validation are tested once and behave the same in all 5 workers (human: ~1 day / CC: ~20 min)\n \u2705 Estimated 15\u201390 implementation lines saved; the helper's test suite replaces five near-duplicate suites\n \u274c A bug in the module reaches all 5 workers; the module's own tests are the guard\nB) Five inline copies, refactor later (as planned)\n \u2705 No shared dependency between workers; each can be changed in isolation (human: ~1.5 days / CC: ~30 min)\n \u2705 Matches the plan text exactly; nothing new to name or place\n \u274c Five copies to keep in sync and five test suites to write; \"later\" rarely arrives for retry glue\nC) Extract the curve builder only\n \u2705 The math most likely to drift (jitter + cap) lives in one place (human: ~1 day / CC: ~15 min)\n \u2705 Smaller shared surface than A\n \u274c Classifier, dead-letter handoff and log line are still copied five times, so most of the duplication and its tests remain\nNet: One under-100-line module now, or five copies plus a promise.": "One shared retry-policy module (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T20:00:17.086Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01Ly1JT6sRQHG4ZVpQ7SUETD",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D8 \u2014 How do we prove processWebhookJob() still sends each event at most once?\nProject/branch/task: main branch, retry-framework plan; regression coverage for the rewritten webhook worker (D2 fixed the behavior to keep).\nELI10: The webhook worker is being rewritten and it carries a promise to receivers: an event is never sent twice. D2 kept that promise while adding retries for failures that happen before anything is sent. A rewrite with no test for the promise means the first duplicate email or double charge is found by a customer. The test is straightforward: a fake receiver counts sends, and we assert the count is exactly one across every failure pattern. The question is how deep to go: assertions against the worker alone, a run through the real library hooks, or both.\nStakes if we pick wrong: a retry path nobody tested sends duplicates to non-idempotent receivers, or a hook wiring mistake means pre-send failures never actually retry and the framework quietly does nothing for webhooks.\nRecommendation: A because the unit layer pins each failure class cheaply and the integration layer is the only thing that catches hook wiring and attempt persistence, which is where retry bugs actually live.\nCompleteness: A=10/10, B=7/10, C=7/10\nPros / cons:\nA) Unit + integration through the library hooks (recommended)\n \u2705 Every failure class (pre-send, timeout, 5xx, reset, success) asserted in isolation with a recording fake transport (human: ~1 day / CC: ~20 min)\n \u2705 One end-to-end run through the real hooks with a fake receiver catches wiring and attempt-persistence bugs the unit layer cannot see\n \u274c Two test layers to maintain; the integration test needs the library's test harness or an in-process queue\nB) Unit tests only\n \u2705 Fast, deterministic, no queue infrastructure in the test run (human: ~half day / CC: ~10 min)\n \u2705 Pins the classifier and the send-count contract per failure class\n \u274c Never exercises the real hook registration, so a miswired hook passes tests and never retries in production\nC) Integration test only\n \u2705 Exercises the real path receivers depend on (human: ~half day / CC: ~10 min)\n \u2705 Fewer tests to write\n \u274c Slower, and a failure tells you \"something duplicated\" without pointing at which failure class; edge classes get skipped for time\nNet: Cheap isolated assertions plus one real-path run, versus trusting either layer alone to protect a promise made to external receivers.",
|
||||
"header": "Webhook regression",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Unit + integration (recommended)",
|
||||
"description": "Unit: fake transport records every send; assert exactly 1 send after pre-send retries, 0 further sends after timeout/5xx/reset with a dead-letter entry, 1 send on success. Integration: real library hooks + fake receiver, same assertions, plus attempt count survives a simulated worker restart. Human ~1 day / CC ~20 min. Completeness 10/10."
|
||||
},
|
||||
{
|
||||
"label": "Unit tests only",
|
||||
"description": "The unit assertions from A against the worker with a fake transport; no run through the real library hooks. Human ~half day / CC ~10 min. Completeness 7/10."
|
||||
},
|
||||
{
|
||||
"label": "Integration test only",
|
||||
"description": "The integration run from A only; no isolated per-failure-class assertions. Human ~half day / CC ~10 min. Completeness 7/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D8 \u2014 How do we prove processWebhookJob() still sends each event at most once?\nProject/branch/task: main branch, retry-framework plan; regression coverage for the rewritten webhook worker (D2 fixed the behavior to keep).\nELI10: The webhook worker is being rewritten and it carries a promise to receivers: an event is never sent twice. D2 kept that promise while adding retries for failures that happen before anything is sent. A rewrite with no test for the promise means the first duplicate email or double charge is found by a customer. The test is straightforward: a fake receiver counts sends, and we assert the count is exactly one across every failure pattern. The question is how deep to go: assertions against the worker alone, a run through the real library hooks, or both.\nStakes if we pick wrong: a retry path nobody tested sends duplicates to non-idempotent receivers, or a hook wiring mistake means pre-send failures never actually retry and the framework quietly does nothing for webhooks.\nRecommendation: A because the unit layer pins each failure class cheaply and the integration layer is the only thing that catches hook wiring and attempt persistence, which is where retry bugs actually live.\nCompleteness: A=10/10, B=7/10, C=7/10\nPros / cons:\nA) Unit + integration through the library hooks (recommended)\n \u2705 Every failure class (pre-send, timeout, 5xx, reset, success) asserted in isolation with a recording fake transport (human: ~1 day / CC: ~20 min)\n \u2705 One end-to-end run through the real hooks with a fake receiver catches wiring and attempt-persistence bugs the unit layer cannot see\n \u274c Two test layers to maintain; the integration test needs the library's test harness or an in-process queue\nB) Unit tests only\n \u2705 Fast, deterministic, no queue infrastructure in the test run (human: ~half day / CC: ~10 min)\n \u2705 Pins the classifier and the send-count contract per failure class\n \u274c Never exercises the real hook registration, so a miswired hook passes tests and never retries in production\nC) Integration test only\n \u2705 Exercises the real path receivers depend on (human: ~half day / CC: ~10 min)\n \u2705 Fewer tests to write\n \u274c Slower, and a failure tells you \"something duplicated\" without pointing at which failure class; edge classes get skipped for time\nNet: Cheap isolated assertions plus one real-path run, versus trusting either layer alone to protect a promise made to external receivers.": "Unit + integration (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T20:02:01.530Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01WZ2b5HaUXKgTwZxqfKn3pd",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D9 \u2014 Cache the dependency graph across retries now, or measure first?\nProject/branch/task: main branch, retry-framework plan; per-attempt cost inside the 5 workers running through library hooks (D1).\nELI10: The plan worries that each retry reloads the job and rebuilds its dependency graph from scratch. After D1 the reload is just the library handing the job to the worker, which happens anyway. The rebuild is real extra CPU, but only on retries, and D3 caps those at 5 per failing job. Storing the graph on the first attempt would add a write to every job, including the large majority that succeed first time, to save work on the few that fail. Nobody has measured how long the rebuild takes.\nStakes if we pick wrong: either we add a write and a staleness risk to every job to fix a cost nobody measured, or a genuinely slow rebuild keeps burning worker time on retries and we only find out under load.\nRecommendation: C because an unmeasured optimization that taxes the happy path is the wrong trade; two timing metrics make the real decision cheap and data-driven.\nNote: options differ in kind (persisted cache vs in-process memo vs measure first) \u2014 no completeness score.\nPros / cons:\nA) Persist the graph with the job on attempt 1\n \u2705 Retries never recompute; cost is paid once per job regardless of which worker instance retries (human: ~1 day / CC: ~20 min)\n \u2705 Simple to reason about once the invalidation rule (payload version) is in place\n \u274c Adds a write and stored blob to every job, including the ones that never retry; stale-graph bugs if the payload changes between attempts\nB) In-process memo (bounded LRU)\n \u2705 No persistence, no schema change; a few lines around the graph builder (human: ~2 hr / CC: ~10 min)\n \u2705 Zero cost on the happy path beyond a map insert\n \u274c Retries after a 10-minute delay usually land on a different worker instance, so the hit rate is low and unpredictable\nC) Measure first: timing metrics + p95 budget (recommended)\n \u2705 Two metrics (graph compute ms, payload bytes) per attempt tell you whether this is 2 ms or 2 s before anyone writes cache code (human: ~1 hr / CC: ~5 min)\n \u2705 No happy-path cost, no staleness risk, and the retry-policy module already logs per attempt (D7) so the hook point exists\n \u274c If the rebuild is genuinely slow, retries stay expensive until the follow-up lands\nNet: Add a write to every job to save CPU on the few that retry, or spend an hour on metrics and decide with numbers.",
|
||||
"header": "Graph cache",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Persist the graph with the job",
|
||||
"description": "Compute once on attempt 1, store the graph beside the job row, reuse on retries, invalidate when the payload version changes. Adds a write to every job. Human ~1 day / CC ~20 min."
|
||||
},
|
||||
{
|
||||
"label": "In-process memo (bounded LRU)",
|
||||
"description": "Memoize the graph per worker instance keyed by job id + payload hash, bounded LRU. No persistence; low hit rate when retries land on another instance. Human ~2 hr / CC ~10 min."
|
||||
},
|
||||
{
|
||||
"label": "Measure first (recommended)",
|
||||
"description": "No cache. Add per-attempt timing metrics (job load ms, graph compute ms, payload bytes) via the retry-policy module's attempt log, set a p95 budget, and revisit caching with data. Human ~1 hr / CC ~5 min."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D9 \u2014 Cache the dependency graph across retries now, or measure first?\nProject/branch/task: main branch, retry-framework plan; per-attempt cost inside the 5 workers running through library hooks (D1).\nELI10: The plan worries that each retry reloads the job and rebuilds its dependency graph from scratch. After D1 the reload is just the library handing the job to the worker, which happens anyway. The rebuild is real extra CPU, but only on retries, and D3 caps those at 5 per failing job. Storing the graph on the first attempt would add a write to every job, including the large majority that succeed first time, to save work on the few that fail. Nobody has measured how long the rebuild takes.\nStakes if we pick wrong: either we add a write and a staleness risk to every job to fix a cost nobody measured, or a genuinely slow rebuild keeps burning worker time on retries and we only find out under load.\nRecommendation: C because an unmeasured optimization that taxes the happy path is the wrong trade; two timing metrics make the real decision cheap and data-driven.\nNote: options differ in kind (persisted cache vs in-process memo vs measure first) \u2014 no completeness score.\nPros / cons:\nA) Persist the graph with the job on attempt 1\n \u2705 Retries never recompute; cost is paid once per job regardless of which worker instance retries (human: ~1 day / CC: ~20 min)\n \u2705 Simple to reason about once the invalidation rule (payload version) is in place\n \u274c Adds a write and stored blob to every job, including the ones that never retry; stale-graph bugs if the payload changes between attempts\nB) In-process memo (bounded LRU)\n \u2705 No persistence, no schema change; a few lines around the graph builder (human: ~2 hr / CC: ~10 min)\n \u2705 Zero cost on the happy path beyond a map insert\n \u274c Retries after a 10-minute delay usually land on a different worker instance, so the hit rate is low and unpredictable\nC) Measure first: timing metrics + p95 budget (recommended)\n \u2705 Two metrics (graph compute ms, payload bytes) per attempt tell you whether this is 2 ms or 2 s before anyone writes cache code (human: ~1 hr / CC: ~5 min)\n \u2705 No happy-path cost, no staleness risk, and the retry-policy module already logs per attempt (D7) so the hook point exists\n \u274c If the rebuild is genuinely slow, retries stay expensive until the follow-up lands\nNet: Add a write to every job to save CPU on the few that retry, or spend an hour on metrics and decide with numbers.": "Persist the graph with the job"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T20:04:03.405Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01WjGqEAsM8pPCNSzxpWyPSV",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D10 \u2014 Track dead-letter retention as a TODO, skip it, or build it now?\nProject/branch/task: main branch, retry-framework plan; follow-up to the dead-letter store approved in D3.\nELI10: The dead-letter store keeps every job that ran out of retries or hit a fatal error. Nothing ever removes them. That is fine for months, then the table is large, the growth alert query slows, and nobody remembers why. A purge job with a retention period fixes it, but the retention period is a judgment call about how long failed-job evidence must stay around.\nStakes if we pick wrong: build it now with the wrong retention and you delete evidence of lost work; skip it and the store becomes an unbounded table someone discovers during an incident.\nRecommendation: A because the store is new, growth is slow, and the retention period deserves an owner's answer rather than a default picked inside a retry PR; the TODO carries a concrete trigger.\nCompleteness: A=6/10, B=2/10, C=10/10\nPros / cons:\nA) Add to TODOS.md with a trigger (recommended)\n \u2705 Keeps this PR right-sized: the retry framework ships without a retention debate attached\n \u2705 Trigger (10k rows or 3 months) means the TODO fires before growth matters (human: ~5 min / CC: ~1 min now)\n \u274c Unbounded growth until someone acts on the TODO; the ceiling is a slow query, not data loss\nB) Skip\n \u2705 Nothing to track or build\n \u2705 Zero effort now\n \u274c The store grows forever with no record that anyone considered it\nC) Build now in this PR\n \u2705 Store ships bounded from day one: purge job, 90-day default, keep flag, tests (human: ~2 hr / CC: ~10 min)\n \u2705 No follow-up to forget\n \u274c Expands this PR with a scheduled job and a retention default nobody has agreed to; deletes evidence if the default is wrong\nNet: A tracked follow-up with a trigger, versus a bigger PR that guesses how long failed-job evidence should live.",
|
||||
"header": "DLQ retention",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add to TODOS.md (recommended)",
|
||||
"description": "Record the TODO (what/why/pros/cons/context/depends-on) with trigger: build when the dead-letter store passes 10k rows or at 3 months, whichever first. Human ~5 min / CC ~1 min. Completeness 6/10."
|
||||
},
|
||||
{
|
||||
"label": "Skip \u2014 not valuable enough",
|
||||
"description": "Do not track retention. Completeness 2/10."
|
||||
},
|
||||
{
|
||||
"label": "Build it now in this PR",
|
||||
"description": "Scheduled purge job, retention config default 90 days, keep flag, tests, shipped with the dead-letter store. Human ~2 hr / CC ~10 min. Completeness 10/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D10 \u2014 Track dead-letter retention as a TODO, skip it, or build it now?\nProject/branch/task: main branch, retry-framework plan; follow-up to the dead-letter store approved in D3.\nELI10: The dead-letter store keeps every job that ran out of retries or hit a fatal error. Nothing ever removes them. That is fine for months, then the table is large, the growth alert query slows, and nobody remembers why. A purge job with a retention period fixes it, but the retention period is a judgment call about how long failed-job evidence must stay around.\nStakes if we pick wrong: build it now with the wrong retention and you delete evidence of lost work; skip it and the store becomes an unbounded table someone discovers during an incident.\nRecommendation: A because the store is new, growth is slow, and the retention period deserves an owner's answer rather than a default picked inside a retry PR; the TODO carries a concrete trigger.\nCompleteness: A=6/10, B=2/10, C=10/10\nPros / cons:\nA) Add to TODOS.md with a trigger (recommended)\n \u2705 Keeps this PR right-sized: the retry framework ships without a retention debate attached\n \u2705 Trigger (10k rows or 3 months) means the TODO fires before growth matters (human: ~5 min / CC: ~1 min now)\n \u274c Unbounded growth until someone acts on the TODO; the ceiling is a slow query, not data loss\nB) Skip\n \u2705 Nothing to track or build\n \u2705 Zero effort now\n \u274c The store grows forever with no record that anyone considered it\nC) Build now in this PR\n \u2705 Store ships bounded from day one: purge job, 90-day default, keep flag, tests (human: ~2 hr / CC: ~10 min)\n \u2705 No follow-up to forget\n \u274c Expands this PR with a scheduled job and a retention default nobody has agreed to; deletes evidence if the default is wrong\nNet: A tracked follow-up with a trigger, versus a bigger PR that guesses how long failed-job evidence should live.": "Add to TODOS.md (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T20:05:54.778Z"
|
||||
},
|
||||
{
|
||||
"sessionId": "0117efe3-a002-42c1-aa59-d45032643e13",
|
||||
"toolUseId": "toolu_01QrMt8ZhcL4wV3cKEjo3MsA",
|
||||
"questions": [
|
||||
{
|
||||
"question": "D11 \u2014 Track the webhook event-id header and at-least-once opt-in as a TODO, skip it, or ship the header now?\nProject/branch/task: main branch, retry-framework plan; follow-up to D2 (webhook worker stays at-most-once).\nELI10: D2 kept the promise that a webhook is never sent twice, which means a send that times out is still lost. The usual fix is to stamp every event with a stable id so receivers can ignore duplicates, and then retry freely. That is a contract change, so it was declined for this PR. The question is whether to track it, drop it, or at least ship the harmless id header now so receivers can start deduping before the semantics ever change.\nStakes if we pick wrong: lost webhook events keep landing in dead-letter with no plan to stop the loss; or a header change rides along in a retry PR without receiver communication.\nRecommendation: A because this is a receiver-facing contract change that deserves its own PR and docs, and the dead-letter store (D3) will produce the loss numbers that justify it; the trigger is concrete.\nCompleteness: A=6/10, B=2/10, C=8/10\nPros / cons:\nA) Add to TODOS.md with a trigger (recommended)\n \u2705 Keeps the retry PR free of webhook contract changes; the TODO fires on measured loss (human: ~5 min / CC: ~1 min now)\n \u2705 Dead-letter counts of post-send failures give the case for it with real numbers\n \u274c Webhook events lost to timeouts stay lost until the TODO is acted on\nB) Skip\n \u2705 Nothing to track\n \u2705 Zero effort now\n \u274c No record that at-most-once was a deliberate trade with a known cost\nC) Ship the stable event-id header now, at-least-once later\n \u2705 Receivers can start deduping today; the header is harmless under at-most-once (human: ~2 hr / CC: ~10 min)\n \u2705 Makes the eventual semantics change a config flip instead of a payload change\n \u274c Adds a webhook payload change and receiver docs to a retry PR; still needs the TODO for the semantics\nNet: Track it with a loss-based trigger, or ship a small header change now inside a PR about retries.",
|
||||
"header": "Webhook TODO",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Add to TODOS.md (recommended)",
|
||||
"description": "Record the TODO with trigger: post-send dead-letter entries exceed 1% of webhook sends in any week, or a receiver requests redelivery. Human ~5 min / CC ~1 min. Completeness 6/10."
|
||||
},
|
||||
{
|
||||
"label": "Skip \u2014 not valuable enough",
|
||||
"description": "Do not track. Completeness 2/10."
|
||||
},
|
||||
{
|
||||
"label": "Ship the header now",
|
||||
"description": "Add a stable per-event id header to every webhook delivery in this PR; D2 semantics unchanged; at-least-once opt-in remains a TODO. Human ~2 hr / CC ~10 min. Completeness 8/10."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"answered": true,
|
||||
"failed": false,
|
||||
"answers": {
|
||||
"D11 \u2014 Track the webhook event-id header and at-least-once opt-in as a TODO, skip it, or ship the header now?\nProject/branch/task: main branch, retry-framework plan; follow-up to D2 (webhook worker stays at-most-once).\nELI10: D2 kept the promise that a webhook is never sent twice, which means a send that times out is still lost. The usual fix is to stamp every event with a stable id so receivers can ignore duplicates, and then retry freely. That is a contract change, so it was declined for this PR. The question is whether to track it, drop it, or at least ship the harmless id header now so receivers can start deduping before the semantics ever change.\nStakes if we pick wrong: lost webhook events keep landing in dead-letter with no plan to stop the loss; or a header change rides along in a retry PR without receiver communication.\nRecommendation: A because this is a receiver-facing contract change that deserves its own PR and docs, and the dead-letter store (D3) will produce the loss numbers that justify it; the trigger is concrete.\nCompleteness: A=6/10, B=2/10, C=8/10\nPros / cons:\nA) Add to TODOS.md with a trigger (recommended)\n \u2705 Keeps the retry PR free of webhook contract changes; the TODO fires on measured loss (human: ~5 min / CC: ~1 min now)\n \u2705 Dead-letter counts of post-send failures give the case for it with real numbers\n \u274c Webhook events lost to timeouts stay lost until the TODO is acted on\nB) Skip\n \u2705 Nothing to track\n \u2705 Zero effort now\n \u274c No record that at-most-once was a deliberate trade with a known cost\nC) Ship the stable event-id header now, at-least-once later\n \u2705 Receivers can start deduping today; the header is harmless under at-most-once (human: ~2 hr / CC: ~10 min)\n \u2705 Makes the eventual semantics change a config flip instead of a payload change\n \u274c Adds a webhook payload change and receiver docs to a retry PR; still needs the TODO for the semantics\nNet: Track it with a loss-based trigger, or ship a small header change now inside a PR about retries.": "Add to TODOS.md (recommended)"
|
||||
},
|
||||
"unansweredQuestionIndices": [],
|
||||
"answeredAt": "2026-09-29T20:07:04.232Z"
|
||||
}
|
||||
],
|
||||
"plan": "# Eng Review \u2014 PLAN.md: Add background job retry framework\n\n- **Review target (fixed):** `/tmp/gstack-plan-count-vrYrwf/PLAN.md` \u2014 \"Plan: Add background job retry framework\"\n- **Report file:** `/tmp/gstack-e2e-plan-eng-batching-PLhMrg/gstack-test-plan-eng-batching.md` (destination explicitly requested by the user)\n- **Skill:** `/plan-eng-review` \u00b7 session `256191-1790711293-1222b505` \u00b7 2026-09-29 \u00b7 branch `main` @ `82eaa12`\n- **Evidence available:** the repository contains only `PLAN.md` and `CLAUDE.md`. No worker files, job library, or `processWebhookJob()` source exist in this checkout. Findings below quote the plan text (file:line) and are calibrated as plan-level, not code-verified.\n\n## Original plan (unchanged copy of PLAN.md lines 4-24)\n\n```markdown\n# Plan: Add background job retry framework\n\n## Architecture\nWe'll roll a custom exponential-backoff scheduler inline in each worker\nrather than use the existing job library's built-in retry hooks. Same\nshape as the library version, but we want full control over the curve.\n\n## Code quality\nThe retry envelope (compute delay, log attempt, dispatch) is duplicated\nacross 5 worker files with copy-pasted bodies. We will leave the\nduplication for now and refactor \"later.\"\n\n## Tests\nThe existing `processWebhookJob()` flow gets rewritten as part of this\nchange. No regression test for the prior at-most-once delivery guarantee\nis planned.\n\n## Performance\nOn every retry we re-fetch the full job payload from the database, then\niterate the payload to recompute the dependency graph. Could cache the\ngraph on the first attempt; not planned.\n```\n\n## Scope Challenge\n\n### A. Assessment\n- **Already solves it:** the job library's built-in retry hooks (PLAN.md:8 admits \"same shape as the library version\"). Library source not in checkout; hook API unverified.\n- **Complexity count (estimate from plan text):** 5 worker files (PLAN.md:13) + `processWebhookJob()` (PLAN.md:17, likely one of the five) = 5\u20136 changed files; 0 new classes/services (scheduler is inline). Under thresholds \u2192 complexity gate B skipped.\n- **Search check:** [Layer 1] library retry hooks + backoff callback; jitter, delay cap, dead-letter, idempotency key are standard practice (AWS Builders' Library; Hookdeck/Svix idempotency guides).\n- **TODOS.md:** none. **Distribution:** no new artifacts.\n\n### C. Findings (plan-level; no code in checkout)\n1. `[P1] (confidence: 8/10) PLAN.md:7-9` \u2014 rebuilding a retry scheduler the job library already provides. \u2192 R1 / D1\n2. `[P1] (confidence: 7/10) PLAN.md:17-19` \u2014 retrying `processWebhookJob()` changes at-most-once to at-least-once delivery; semantics change, not just a missing test. \u2192 Section 1\n3. `[P2] (confidence: 7/10) PLAN.md:7-9` \u2014 retry policy bounds unspecified (max attempts, delay cap, jitter, dead-letter, retryable vs fatal errors). \u2192 Section 1\n4. `[P2] (confidence: 7/10) PLAN.md:12-14` \u2014 five copy-pasted retry envelopes. \u2192 Section 2\n5. `[P1] (confidence: 8/10) PLAN.md:18-19` \u2014 no regression test for a rewritten flow with a stated guarantee (Regression Rule). \u2192 Section 3\n6. `[P2] (confidence: 6/10) PLAN.md:22-24` \u2014 full payload refetch + graph recompute on every retry. \u2192 Section 4\n\nScope Challenge result: **scope accepted as-is** (D1 changed the mechanism to library retry hooks; no feature was cut, so this is not a scope reduction). Dispositions: finding 1 accepted via D1 (R1 approved); findings 2\u20136 pending in their sections.\n\n## Section 1 \u2014 Architecture review\n\nWorking plan after D1: all 5 workers retry through the job library's hooks; each worker supplies its own backoff curve.\n\n```\nRETRY STATE MACHINE (per job, owned by the library after D1)\n\n enqueue \u2500\u2500\u25b6 [attempt n] \u2500\u2500success\u2500\u2500\u25b6 DONE\n \u2502\n \u251c\u2500 fatal error (R3d: non-retryable class) \u2500\u2500\u25b6 FAILED \u2500\u2500\u25b6 dead-letter (R3a)\n \u2502\n \u2514\u2500 transient error / timeout\n \u2502\n \u251c\u2500 n >= maxAttempts (R3a) \u2500\u2500\u25b6 FAILED \u2500\u2500\u25b6 dead-letter (R3a)\n \u2502\n \u2514\u2500 delay = min(base\u00b72^n (+ jitter R3b), cap R3c) \u2500\u2500\u25b6 [attempt n+1]\n\n Webhook worker only: timeout after the request was written is AMBIGUOUS \u2014\n the receiver may already have the event. A retry here = possible duplicate (R2).\n```\n\nFindings:\n- `[P1] (confidence: 7/10) PLAN.md:17-19` \u2014 \"The existing `processWebhookJob()` flow gets rewritten ... prior at-most-once delivery guarantee.\" Adding retries flips the webhook worker from at-most-once to at-least-once: a retry after an ambiguous timeout can deliver the same event twice. The plan treats this as a missing test; it is a deliLine truncated
|
||||
}
|
||||
Reference in new issue
Block a user