fix(dispatcher): improve fanout delivery timeouts and retry budget#3270
Conversation
AI Session Analysis
|
| Status | Attribution | File | Lines |
|---|---|---|---|
| modified | ai | app/controlplane/internal/dispatcher/dispatch_timeout_test.go |
+183 / -97 |
| modified | ai | app/controlplane/internal/dispatcher/dispatcher.go |
+22 / -11 |
| modified | ai | app/controlplane/plugins/core/webhook/v1/webhook.go |
+11 / -6 |
| modified | ai | app/controlplane/plugins/sdk/v1/fanout.go |
+7 / -7 |
| modified | ai | AI_POLICY.md |
+5 / -0 |
| modified | ai | CLAUDE.md |
+3 / -0 |
| modified | human | devel/integrations.md |
+1 / -1 |
Policies (4)
| Status | Policy | Material | Messages |
|---|---|---|---|
| ✅ Passed | ai-config-ai-agents-allowed |
ai-coding-session-ses-0c |
- |
| ✅ Passed | ai-config-no-dangerous-commands |
ai-coding-session-ses-0c |
- |
| ✅ Passed | ai-config-no-secrets |
ai-coding-session-ses-0c |
- |
| ✅ Passed | ai-config-mcp-servers-allowed |
ai-coding-session-ses-0c |
- |
Powered by Chainloop and Chainloop Trace
00441fb to
e16b9b2
Compare
waveywaves
left a comment
There was a problem hiding this comment.
A few inline questions focused on keeping the fix tight and matching existing timeout patterns.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Bump MaxElapsedTime from 10s to 5m (~12-15 retry attempts) - Add per-attempt context.WithTimeout (10s) so a single hung HTTP request cannot consume the entire retry budget - Set http.Client.Timeout on the webhook plugin to match the per-attempt deadline (shared via sdk.PerAttemptTimeout) - Wrap backoff with backoff.WithContext so the retry loop stops promptly when the parent context is cancelled - Replace deprecated io/ioutil with io in the webhook plugin Refs chainloop-dev#3269 Closes chainloop-dev#39 Assisted-by: OpenCode Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: ses_0c4100537ffeM74j5m1U4O10fe
Reflects the new per-attempt HTTP timeout behavior introduced in the previous commit. Assisted-by: OpenCode Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: ses_0c4100537ffeM74j5m1U4O10fe
Add OpenCode to the examples and state explicitly that the value must match the tool that produced the work, not the underlying model, and must not be copied blindly from an example. Assisted-by: OpenCode Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: ses_0c4100537ffeM74j5m1U4O10fe
… SDK Move the per-attempt HTTP timeout out of the plugin SDK and into the webhook plugin as a local constant, addressing review feedback that the control-plane retry policy should not be part of the SDK API. - Remove sdk.PerAttemptTimeout from fanout.go - Remove dispatcher per-attempt context.WithTimeout and backoff.WithContext (the production caller uses context.TODO(), making cancellation inert) - Add local perAttemptTimeout constant in the webhook plugin - Remove tests for the removed dispatcher-level timeout behavior Assisted-by: OpenCode Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: ses_0c4100537ffeM74j5m1U4O10fe
799b8d2 to
1f30858
Compare
Changes
MaxElapsedTimefrom 10s to 5m: ~12-15 retry attempts with the existing exponential backoff, enough to ride out transient blips without leaking goroutines in the fire-and-forget dispatch model.Executecall now runs undercontext.WithTimeoutso a single hung HTTP request cannot consume the entire retry budget.http.Client.Timeouton the webhook plugin: matches the dispatcher's per-attempt deadline via a sharedsdk.PerAttemptTimeoutconstant.backoff.WithContextensures the retry loop stops promptly on parent context cancellation.io/ioutilwithioin the webhook plugin.Closes #3269
Refs #39
Assisted-by: OpenCode
🤖 Posted by Maximus bot (OpenCode) on behalf of @migmartri