Wrap bulk retry's filters in the query object the API expects - #15
Merged
Conversation
`POST /bulk/events/retry` takes its filters inside a `query` object. We sent
them at the top level, so bulk retry has never worked — not from
`hookdeck_bulk_retry`, and not from `hermes hookdeck retry --failed`, which
goes through the same client method.
Found by asking the agent to triage real failed events. It reasoned correctly,
called the tool, and reported back that Hookdeck was down:
Retry Attempt: ❌ Hookdeck API is currently down. The retry API endpoint
(POST /bulk/events/retry) is returning HTTP 500 FATAL_ERROR.
It was not down. The API answers a malformed body with 500 FATAL_ERROR rather
than 400, so a wrong shape is indistinguishable from an outage — which is how
this survived, and why the wrapper now lives in the client rather than in each
caller.
Every existing test fakes the client and asserts the dict handed to
`bulk_retry_events`, so none of them could see the wire. The new one drives a
real `httpx.MockTransport` and pins the encoded body.
Verified against the live API: the flat shape 500s, the wrapped shape returns a
batch (`estimated_count: 3`), and the agent now completes the same request it
previously reported as an outage.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of the query-shape fix, plus what it exposed once the call started
working.
**Scope.** `{"status": "FAILED"}` matches every failed event in the project,
and a project usually holds connections belonging to something else. The tool
takes no required arguments, so an agent can call it bare and redeliver
somebody else's traffic. It now resolves the configured routes to connection
ids and filters on them — the same definition of "ours" the dashboard uses to
decide what it may pause — and refuses rather than widening when it owns
nothing. An explicit `connection_id` still overrides.
**Empty matches.** The API answers a batch that would match nothing with 422,
which surfaced as an API error. On a healthy gateway that is the common case,
and "nothing to retry" is an answer, not a fault.
**The `since` path was never exercised.** The fix was verified with
`{status, webhook_id}` only. Checked against the live API: the nested
`created_at: {gte: …}` shape parses (it 422s on an empty match rather than
500ing on a bad body), so it was correct — but it was correct untested.
**Burst headroom.** A deferred event only gets back in on a retry, so a burst
drains at `max_concurrent` per round against the rule's `count` rounds. Their
product is the burst that survives, and past it the tail exhausts its retries
while waiting — 6 events against `max_concurrent: 1` and a `count: 5`
connection lost 2. `doctor` now reports the figure per connection from the rule
really on it, and reliability.md explains the sizing. The default stays 2: it
pairs with the provisioned count of 10 for ~20, and raising it is a spending
decision as much as a throughput one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
POST /bulk/events/retrytakes its filters inside aqueryobject. We sent them at the top level, so bulk retry has never worked — not fromhookdeck_bulk_retry, and not fromhermes hookdeck retry --failed, which goes through the same client method.Found by asking the agent to triage real failed events. It reasoned correctly, called the tool, and reported back:
It was not down. Verified against the live API:
The API answers a malformed body with 500 FATAL_ERROR rather than 400, so a wrong shape is indistinguishable from an outage. That is how this survived, and why the wrapper now lives in the client rather than being left to each caller.
Why no test caught it
Every existing test fakes the client and asserts the dict handed to
bulk_retry_events— the shape before it becomes a request. The wire was never exercised. The new test drives a realhttpx.MockTransportand pins the encoded body; reverting the fix fails it.Verified
Ships in the wheel. This one is worth the release on its own: an agent-callable write tool that has never functioned, and whose failure mode is a confident report that someone else's service is broken.