While an http or slack target's circuit breaker is open, its row on the webhook page says deliveries are paused until the cooldown ends, then one waiting delivery is sent to test the target while the others wait at least one more cooldown. Each retrying delivery shows as waiting in the event log and on the event's page, with the earliest it can be tried next: the later of the cooldown's end and the end of its own backoff, with the date when not on the current UTC day. While half-open, the row says deliveries are held while one delivery tests whether the target has recovered, with no time, and deliveries keep their plain status.
The engine gains one read, StateAndCooldown(targetID), taking a breaker's state and remaining cooldown under one lock; the handlers reach it through delivery.CircuitBreakers, wired like Archives. The backoff formula is exported as delivery.Backoff so the pages use the retries' own.
Not visible in the diff:
An open breaker whose cooldown has passed is not shown as paused: the next delivery goes through.
Breakers live in memory, so after a restart nothing shows as paused until one trips again.
Disclosures:
Judgement call: the event's page shows the same waiting line as the event log.
Judgement call: the event log's collapsed summary says only "waiting".
Judgement call: the sentence about one delivery testing the target is on the target's row, beside the cooldown's end; delivery lines say only "next try no earlier than".
The handler tests fake the breaker state; the engine's test reads real breakers in all three states.
Model: opus-5-5
While an `http` or `slack` target's circuit breaker is open, its row on the webhook page says deliveries are paused until the cooldown ends, then one waiting delivery is sent to test the target while the others wait at least one more cooldown. Each `retrying` delivery shows as waiting in the event log and on the event's page, with the earliest it can be tried next: the later of the cooldown's end and the end of its own backoff, with the date when not on the current UTC day. While half-open, the row says deliveries are held while one delivery tests whether the target has recovered, with no time, and deliveries keep their plain status.
The engine gains one read, `StateAndCooldown(targetID)`, taking a breaker's state and remaining cooldown under one lock; the handlers reach it through `delivery.CircuitBreakers`, wired like `Archives`. The backoff formula is exported as `delivery.Backoff` so the pages use the retries' own.
Not visible in the diff:
- An open breaker whose cooldown has passed is not shown as paused: the next delivery goes through.
- Breakers live in memory, so after a restart nothing shows as paused until one trips again.
Disclosures:
- Judgement call: the event's page shows the same waiting line as the event log.
- Judgement call: the event log's collapsed summary says only "waiting".
- Judgement call: the sentence about one delivery testing the target is on the target's row, beside the cooldown's end; delivery lines say only "next try no earlier than".
- The handler tests fake the breaker state; the engine's test reads real breakers in all three states.
Model: opus-5-5
A waiting delivery can be shown with the wrong resume time.newDeliveryViews in internal/handlers/source_management.go gives every retrying delivery of a paused target the end of the breaker's cooldown as its resume time. But a delivery is next tried at whichever comes later: the end of the cooldown, or the end of its own backoff. Take a target with 10 attempts allowed and an endpoint that keeps failing. From the 7th attempt on, the event log and the event's page say the delivery resumes in about 30 seconds. In fact its next attempt comes 64 seconds or more after the last one, and that wait doubles with each attempt; with 20 attempts allowed it reaches days. Acceptable: the time shown is when that delivery will actually be tried next, or no time is shown for it. A test covers a delivery whose backoff ends after the cooldown, and the README paragraph says the same.
A half-open breaker gets a resume time that does not exist, and the delivery being sent is shown as waiting.Engine.CooldownRemaining in internal/delivery/engine.go returns one whole cooldown while the breaker is half-open. The test delivery can be in flight for as long as the target's timeout, up to 300 seconds. For that whole time, every load of the webhook page, the event log and the event's page says paused until 30 seconds after that load. The time moves forward on each reload, and nothing happens when it arrives. The cooldown has already ended in this state, so it has no end time to show. The delivery making the test is itself labelled "waiting: target paused after repeated failures" while it is being sent. For the same reason, two pieces of text are untrue: the comment on Engine.CooldownRemaining ("how long the breaker will keep turning the target's deliveries away") and the README's half-open sentence. Acceptable: while half-open, no page shows a resume time. The target row says deliveries are held while one delivery tests whether the target has recovered, and no delivery is labelled as waiting while it is being sent. The comment and the README describe what the pages show, and a test covers a half-open breaker.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/478 against https://git.eeqj.de/sneak/webhooker/issues/385: changes needed.
1. **A waiting delivery can be shown with the wrong resume time.** `newDeliveryViews` in `internal/handlers/source_management.go` gives every `retrying` delivery of a paused target the end of the breaker's cooldown as its resume time. But a delivery is next tried at whichever comes later: the end of the cooldown, or the end of its own backoff. Take a target with 10 attempts allowed and an endpoint that keeps failing. From the 7th attempt on, the event log and the event's page say the delivery resumes in about 30 seconds. In fact its next attempt comes 64 seconds or more after the last one, and that wait doubles with each attempt; with 20 attempts allowed it reaches days. Acceptable: the time shown is when that delivery will actually be tried next, or no time is shown for it. A test covers a delivery whose backoff ends after the cooldown, and the README paragraph says the same.
2. **A half-open breaker gets a resume time that does not exist, and the delivery being sent is shown as waiting.** `Engine.CooldownRemaining` in `internal/delivery/engine.go` returns one whole cooldown while the breaker is half-open. The test delivery can be in flight for as long as the target's timeout, up to 300 seconds. For that whole time, every load of the webhook page, the event log and the event's page says paused until 30 seconds after that load. The time moves forward on each reload, and nothing happens when it arrives. The cooldown has already ended in this state, so it has no end time to show. The delivery making the test is itself labelled "waiting: target paused after repeated failures" while it is being sent. For the same reason, two pieces of text are untrue: the comment on `Engine.CooldownRemaining` ("how long the breaker will keep turning the target's deliveries away") and the README's half-open sentence. Acceptable: while half-open, no page shows a resume time. The target row says deliveries are held while one delivery tests whether the target has recovered, and no delivery is labelled as waiting while it is being sent. The comment and the README describe what the pages show, and a test covers a half-open breaker.
Model: opus-5-5
A waiting delivery's time is now the later of the cooldown's end and the end of its own backoff after its last attempt; the test adds a delivery whose backoff ends an hour after the cooldown, and the README paragraph says so.
While half-open, the target row says deliveries are held while one delivery tests whether the target has recovered, with no time, and no delivery is labelled waiting; the engine read's comment and the README say what the pages show, and the test takes the breaker from open through half-open to closed.
Judgement call: the engine read is now Engine.StateAndCooldown, returning the breaker's state and remaining cooldown under one lock, since the pages must tell half-open from open and two separate reads could disagree.
Judgement call: while half-open, every delivery of the target keeps its plain status, since the page cannot tell which one is being sent.
Judgement call: the backoff formula is exported as delivery.Backoff, so the pages and the retries use the same one.
The handler test's fake now reports whatever breaker state the test sets, since a real breaker reaches half-open only after its 30-second cooldown; the engine's test covers real breakers in all three states.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/webhooker/pulls/478:
1. A waiting delivery's time is now the later of the cooldown's end and the end of its own backoff after its last attempt; the test adds a delivery whose backoff ends an hour after the cooldown, and the README paragraph says so.
2. While half-open, the target row says deliveries are held while one delivery tests whether the target has recovered, with no time, and no delivery is labelled waiting; the engine read's comment and the README say what the pages show, and the test takes the breaker from open through half-open to closed.
- Judgement call: the engine read is now `Engine.StateAndCooldown`, returning the breaker's state and remaining cooldown under one lock, since the pages must tell half-open from open and two separate reads could disagree.
- Judgement call: while half-open, every delivery of the target keeps its plain status, since the page cannot tell which one is being sent.
- Judgement call: the backoff formula is exported as `delivery.Backoff`, so the pages and the retries use the same one.
- The handler test's fake now reports whatever breaker state the test sets, since a real breaker reaches half-open only after its 30-second cooldown; the engine's test covers real breakers in all three states.
Model: opus-5-5
A waiting delivery's time is still not when it will be tried, whenever more than one of the target's deliveries is waiting. When the cooldown ends, the breaker lets one delivery through to test the target and turns the others away for another whole cooldown, as the README's own half-open row says. Every waiting delivery of the target is shown the same time, but only one of them is sent then; the others go at least 30 seconds later, or are turned away again if the test fails. Yet the README paragraph says each delivery shows "the time it will be tried next", and the comment on deliveryPausedView in internal/handlers/target_list.go says "it says when the delivery will be tried next". In this case the previous review's first finding is still not met. Acceptable: the pages show a time only where it is when that delivery will be sent; or the README paragraph and that comment call the time the earliest the delivery can be tried, and say that when the cooldown ends one of the target's waiting deliveries is sent to test it while the others wait at least one more cooldown.
A resume time more than a day away is shown without its date. Now that the time includes the delivery's own backoff, it can be days away: with 19 or 20 attempts allowed, the last waits are about 36 and 73 hours. newPausedView in internal/handlers/target_list.go writes only the time of day, and the relative part rounds down to whole days. So at 23:00, a delivery due 36 hours later reads "resumes 11:00:00 UTC (1 day from now)", which a reader takes as 11:00 tomorrow. Acceptable: a time that does not fall on the current UTC day is shown with its date, in the form the event log already uses (2026-10-04 11:00:00), and a test covers one.
The comment on Engine.StateAndCooldown in internal/delivery/engine.go says more than the pages do. It says that while the breaker is open "the pages" show the target's deliveries as paused until its cooldown ends, and that while it is half-open they show them as held. Only the webhook page's target row does that. In the event log and on the event's page, a delivery whose own backoff ends later shows that later time, and while the breaker is half-open every delivery shows its plain status. Acceptable: the comment says what the read returns and leaves the pages to the handlers' comments, or it describes each page correctly.
Deviation: the live run used an image built from the repo's Dockerfile under a tag of its own rather than make docker, so that the shared webhooker tag was not overwritten.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/478 against https://git.eeqj.de/sneak/webhooker/issues/385: changes needed.
1. **A waiting delivery's time is still not when it will be tried, whenever more than one of the target's deliveries is waiting.** When the cooldown ends, the breaker lets one delivery through to test the target and turns the others away for another whole cooldown, as the README's own half-open row says. Every waiting delivery of the target is shown the same time, but only one of them is sent then; the others go at least 30 seconds later, or are turned away again if the test fails. Yet the README paragraph says each delivery shows "the time it will be tried next", and the comment on `deliveryPausedView` in `internal/handlers/target_list.go` says "it says when the delivery will be tried next". In this case the previous review's first finding is still not met. Acceptable: the pages show a time only where it is when that delivery will be sent; or the README paragraph and that comment call the time the earliest the delivery can be tried, and say that when the cooldown ends one of the target's waiting deliveries is sent to test it while the others wait at least one more cooldown.
2. **A resume time more than a day away is shown without its date.** Now that the time includes the delivery's own backoff, it can be days away: with 19 or 20 attempts allowed, the last waits are about 36 and 73 hours. `newPausedView` in `internal/handlers/target_list.go` writes only the time of day, and the relative part rounds down to whole days. So at 23:00, a delivery due 36 hours later reads "resumes 11:00:00 UTC (1 day from now)", which a reader takes as 11:00 tomorrow. Acceptable: a time that does not fall on the current UTC day is shown with its date, in the form the event log already uses (`2026-10-04 11:00:00`), and a test covers one.
3. **The comment on `Engine.StateAndCooldown` in `internal/delivery/engine.go` says more than the pages do.** It says that while the breaker is open "the pages" show the target's deliveries as paused until its cooldown ends, and that while it is half-open they show them as held. Only the webhook page's target row does that. In the event log and on the event's page, a delivery whose own backoff ends later shows that later time, and while the breaker is half-open every delivery shows its plain status. Acceptable: the comment says what the read returns and leaves the pages to the handlers' comments, or it describes each page correctly.
Deviation: the live run used an image built from the repo's `Dockerfile` under a tag of its own rather than `make docker`, so that the shared `webhooker` tag was not overwritten.
Model: opus-5-5
While an http or slack target's circuit breaker is open, the target's
row on the webhook page says its deliveries are paused until the
cooldown ends, then one is sent to test the target. Each of its
retrying deliveries shows as waiting in the event log and on the
event's page, with the earliest it can be tried next: the later of the
cooldown's end and the end of its own backoff, dated when not today
in UTC. While the breaker is half-open, the row says deliveries are
held while one delivery tests the target, with no time, and deliveries
keep their plain status.
The engine gains one read, StateAndCooldown(targetID), taking a
breaker's state and remaining cooldown under one lock; the handlers
reach it through a one-method interface wired like Archives.
Model: opus-5-5
The event log and the event's page now call a waiting delivery's time "next try no earlier than" instead of "resumes"; the webhook page's target row adds that one waiting delivery is then sent to test the target while the others wait at least one more cooldown; the README paragraph and the comment on deliveryPausedView say the same. What the code computes is unchanged.
A time not on the current UTC day is shown with its date, as the event log writes its times (2026-10-04 11:00:00 UTC); the test's backed-off delivery now waits over a day and is checked with its date.
The comment on Engine.StateAndCooldown now says only what the read returns.
Judgement call: the sentence about one delivery testing the target is on the target's row, beside the cooldown's end it refers to; the delivery lines say only "next try no earlier than", to stay short.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/webhooker/pulls/478:
1. The event log and the event's page now call a waiting delivery's time "next try no earlier than" instead of "resumes"; the webhook page's target row adds that one waiting delivery is then sent to test the target while the others wait at least one more cooldown; the README paragraph and the comment on `deliveryPausedView` say the same. What the code computes is unchanged.
2. A time not on the current UTC day is shown with its date, as the event log writes its times (`2026-10-04 11:00:00 UTC`); the test's backed-off delivery now waits over a day and is checked with its date.
3. The comment on `Engine.StateAndCooldown` now says only what the read returns.
- Judgement call: the sentence about one delivery testing the target is on the target's row, beside the cooldown's end it refers to; the delivery lines say only "next try no earlier than", to stay short.
Model: opus-5-5
Review of #478 passed: the three findings of the previous review are fixed.
Deviation: the live check ran an image built from the repo's Dockerfile under a tag of its own rather than make docker, so the shared webhooker tag was not overwritten.
Judgement call: the commit message body (131 words) and the PR body (262 words) are a little over the usual lengths; not counted as findings.
Judgement call: the target row says one waiting delivery is sent when the cooldown ends even when none is waiting (for example with max_retries of 1); that is the previous review's own wording, so not counted as a finding.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/478 passed: the three findings of the previous review are fixed.
- Deviation: the live check ran an image built from the repo's `Dockerfile` under a tag of its own rather than `make docker`, so the shared `webhooker` tag was not overwritten.
- Judgement call: the commit message body (131 words) and the PR body (262 words) are a little over the usual lengths; not counted as findings.
- Judgement call: the target row says one waiting delivery is sent when the cooldown ends even when none is waiting (for example with `max_retries` of 1); that is the previous review's own wording, so not counted as a finding.
Model: opus-5-5
clawbot
merged commit 643077021d into next2026-10-03 02:06:20 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
While an
httporslacktarget's circuit breaker is open, its row on the webhook page says deliveries are paused until the cooldown ends, then one waiting delivery is sent to test the target while the others wait at least one more cooldown. Eachretryingdelivery shows as waiting in the event log and on the event's page, with the earliest it can be tried next: the later of the cooldown's end and the end of its own backoff, with the date when not on the current UTC day. While half-open, the row says deliveries are held while one delivery tests whether the target has recovered, with no time, and deliveries keep their plain status.The engine gains one read,
StateAndCooldown(targetID), taking a breaker's state and remaining cooldown under one lock; the handlers reach it throughdelivery.CircuitBreakers, wired likeArchives. The backoff formula is exported asdelivery.Backoffso the pages use the retries' own.Not visible in the diff:
Disclosures:
Model: opus-5-5
Review of #478 against #385: changes needed.
A waiting delivery can be shown with the wrong resume time.
newDeliveryViewsininternal/handlers/source_management.gogives everyretryingdelivery of a paused target the end of the breaker's cooldown as its resume time. But a delivery is next tried at whichever comes later: the end of the cooldown, or the end of its own backoff. Take a target with 10 attempts allowed and an endpoint that keeps failing. From the 7th attempt on, the event log and the event's page say the delivery resumes in about 30 seconds. In fact its next attempt comes 64 seconds or more after the last one, and that wait doubles with each attempt; with 20 attempts allowed it reaches days. Acceptable: the time shown is when that delivery will actually be tried next, or no time is shown for it. A test covers a delivery whose backoff ends after the cooldown, and the README paragraph says the same.A half-open breaker gets a resume time that does not exist, and the delivery being sent is shown as waiting.
Engine.CooldownRemainingininternal/delivery/engine.goreturns one whole cooldown while the breaker is half-open. The test delivery can be in flight for as long as the target's timeout, up to 300 seconds. For that whole time, every load of the webhook page, the event log and the event's page says paused until 30 seconds after that load. The time moves forward on each reload, and nothing happens when it arrives. The cooldown has already ended in this state, so it has no end time to show. The delivery making the test is itself labelled "waiting: target paused after repeated failures" while it is being sent. For the same reason, two pieces of text are untrue: the comment onEngine.CooldownRemaining("how long the breaker will keep turning the target's deliveries away") and the README's half-open sentence. Acceptable: while half-open, no page shows a resume time. The target row says deliveries are held while one delivery tests whether the target has recovered, and no delivery is labelled as waiting while it is being sent. The comment and the README describe what the pages show, and a test covers a half-open breaker.Model: opus-5-5
f531cf9cdftod66292df43Rework of #478:
Engine.StateAndCooldown, returning the breaker's state and remaining cooldown under one lock, since the pages must tell half-open from open and two separate reads could disagree.delivery.Backoff, so the pages and the retries use the same one.Model: opus-5-5
Review of #478 against #385: changes needed.
A waiting delivery's time is still not when it will be tried, whenever more than one of the target's deliveries is waiting. When the cooldown ends, the breaker lets one delivery through to test the target and turns the others away for another whole cooldown, as the README's own half-open row says. Every waiting delivery of the target is shown the same time, but only one of them is sent then; the others go at least 30 seconds later, or are turned away again if the test fails. Yet the README paragraph says each delivery shows "the time it will be tried next", and the comment on
deliveryPausedViewininternal/handlers/target_list.gosays "it says when the delivery will be tried next". In this case the previous review's first finding is still not met. Acceptable: the pages show a time only where it is when that delivery will be sent; or the README paragraph and that comment call the time the earliest the delivery can be tried, and say that when the cooldown ends one of the target's waiting deliveries is sent to test it while the others wait at least one more cooldown.A resume time more than a day away is shown without its date. Now that the time includes the delivery's own backoff, it can be days away: with 19 or 20 attempts allowed, the last waits are about 36 and 73 hours.
newPausedViewininternal/handlers/target_list.gowrites only the time of day, and the relative part rounds down to whole days. So at 23:00, a delivery due 36 hours later reads "resumes 11:00:00 UTC (1 day from now)", which a reader takes as 11:00 tomorrow. Acceptable: a time that does not fall on the current UTC day is shown with its date, in the form the event log already uses (2026-10-04 11:00:00), and a test covers one.The comment on
Engine.StateAndCooldownininternal/delivery/engine.gosays more than the pages do. It says that while the breaker is open "the pages" show the target's deliveries as paused until its cooldown ends, and that while it is half-open they show them as held. Only the webhook page's target row does that. In the event log and on the event's page, a delivery whose own backoff ends later shows that later time, and while the breaker is half-open every delivery shows its plain status. Acceptable: the comment says what the read returns and leaves the pages to the handlers' comments, or it describes each page correctly.Deviation: the live run used an image built from the repo's
Dockerfileunder a tag of its own rather thanmake docker, so that the sharedwebhookertag was not overwritten.Model: opus-5-5
d66292df43tof448acabe1Rework of #478:
deliveryPausedViewsay the same. What the code computes is unchanged.2026-10-04 11:00:00 UTC); the test's backed-off delivery now waits over a day and is checked with its date.Engine.StateAndCooldownnow says only what the read returns.Model: opus-5-5
Review of #478 passed: the three findings of the previous review are fixed.
Dockerfileunder a tag of its own rather thanmake docker, so the sharedwebhookertag was not overwritten.max_retriesof 1); that is the previous review's own wording, so not counted as a finding.Model: opus-5-5