Drop agent-run events the server refuses instead of retrying forever

[alexeyzimarev]

AI-3060 — no GitHub issue: this was filed in Linear directly, and opening one here would import a duplicate back into Linear.

What & why

The daemon's agent-run event queue retried any failed POST forever with one consumer, so a single event the server refuses (today: the policy snapshot/decision types an older server answers with 500) blocked every later AgentRunStopped/AgentRunHeartbeat on that daemon until restart. Delivery now mirrors HookSpool.IsRetryable: a non-retryable status is logged once and dropped, and a retryable answer (5xx, 408, 429) is retried with the same backoff for at most 15 answers (~5 min) before the event is dropped.

Where to look

A 401 and a transport failure (no response) retry without spending that bound — they say nothing against the event, fail everything behind it too, and clear on their own (login, reconnect). The bearer token is now set per request rather than on the shared client's default headers, which the drain and the shutdown post both wrote.

Verification

• AgentRunEventQueueTests: 7/7 pass (400/404/413 dropped, 503 recovered in order, 500 dropped after the bound, 401 and transport failures uncounted); each guard mutated in turn turns its test red.
• Full daemon unit suite: 3543 passed, 3 failed — installed codex newer than the vendored schema pin, KCAP_DAEMON_ID set in the local shell, and an orchestrator timing test that passes in isolation.

🤖 Generated with Claude Code

[linear-code]

AI-3060

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-25T13:22:19.721164Z	7349eb4	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[qodo-code-review]

PR Summary by Qodo

Prevent refused agent-run events from blocking the delivery queue

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Drops permanently refused events so later agent-run lifecycle events continue draining.
• Bounds retryable HTTP responses while preserving retries for transport failures.
• Extracts event delivery into a tested queue with request-scoped bearer authentication.

Diagram

graph TD
    A["ServerConnection"] --> B["Event Queue"] --> C["Serialize Event"] --> D["POST Event"] --> E{"HTTP Result"}
    E -->|Success| F["Deliver Next"]
    E -->|Non-retryable| G["Log and Drop"] --> F
    E -->|Retryable below limit| H["Backoff Retry"] --> D
    E -->|Retry limit reached| G
    D -->|Transport failure| H

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Shared HTTP resilience handler

• ➕ Centralizes status classification and exponential-backoff behavior
• ➕ Could standardize retry policies across daemon HTTP calls
• ➖ Generic handlers do not naturally model ordered queue advancement and event dropping
• ➖ Distinguishing transport failures from counted responses requires specialized policy state

2. Persistent outbox with dead-letter storage

• ➕ Survives daemon restarts without losing queued events
• ➕ Preserves rejected events for inspection or later replay
• ➖ Adds storage, migration, cleanup, and replay complexity
• ➖ Does not remove the need to classify poison events that block ordered delivery

Recommendation: Keep the dedicated in-memory queue and reuse HookSpool.IsRetryable as implemented. It directly fixes head-of-line blocking while preserving ordering and transport recovery; a persistent outbox is only warranted if restart durability or rejected-event recovery becomes a product requirement.

Files changed (3) +274 / -101

Bug fix (1) +139 / -0

AgentRunEventQueue.csAdd bounded, status-aware agent-run event delivery +139/-0

Add bounded, status-aware agent-run event delivery

• Introduces a dedicated ordered event queue that drops non-retryable responses and caps retryable responses at 15 attempts with exponential backoff. Transport failures remain unbounded, serialization failures are dropped, and bearer tokens are attached per request rather than through shared default headers.

src/Capacitor.Cli.Daemon/Services/AgentRunEventQueue.cs

Refactor (1) +8 / -101

ServerConnection.csDelegate agent-run event delivery to the dedicated queue +8/-101

Delegate agent-run event delivery to the dedicated queue

• Replaces the embedded channel and retry loop with AgentRunEventQueue for normal draining and direct shutdown posts. Queue completion, task draining, HTTP disposal, serialization, and event-delivery logging now belong to the extracted component.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Tests (1) +127 / -0

AgentRunEventQueueTests.csCover refusal, bounded retry, ordering, and transport recovery +127/-0

Cover refusal, bounded retry, ordering, and transport recovery

• Adds scripted HTTP tests proving that 400, 404, and 413 responses are dropped immediately; transient 503 responses recover in order; persistent 500 responses exhaust the bound; and transport failures do not consume response attempts.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentRunEventQueueTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Credential lapses lose run events ✓ Resolved 🔗 Cross-repo conflict ☼ Reliability

Description

DeliverAsync counts HTTP 401 responses toward MaxRetryableResponses and discards the pending
event after the fifteenth response, even though a later login can repair the credentials. When the
discarded event is AgentRunStopped, the authorized server handler is never reached and the server
retains the agent-run row used to mark the hosted session as live.

Code

src/Capacitor.Cli.Daemon/Services/AgentRunEventQueue.cs[R77-80]

+                if (++retryableResponses >= MaxRetryableResponses) {
+                    LogRetriesExhausted(eventType, agentId, status, retryableResponses);
+
+                    return true;

Relevance

●●● Strong

Recent reliability precedents accept preserving shutdown events and correcting token-refresh
behavior; 401 loss directly violates the retry contract.

PR-#734
PR-#257

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR drops any retryable response after fifteen attempts, while the shared retry contract
explicitly identifies 401 as a credential lapse whose payload must survive until login. The server
protects the endpoint with authorization and only removes live agent-run state after processing an
authorized stop event, so dropping that event leaves the server-side row present.

kcap-cli -> kcap-server
src/Capacitor.Cli.Daemon/Services/AgentRunEventQueue.cs[65-80]
src/Capacitor.Cli.Core/HookSpool.cs[30-40]
External repo: kurrent-io/kcap-server, src/Capacitor.Api.Public/Agents/PublicAgentRunEndpoints.cs [5-12]
External repo: kurrent-io/kcap-server, src/Capacitor.Server.Services/Agents/AgentRunProjector.cs [79-101]
External repo: kurrent-io/kcap-server, src/Capacitor.Server.Services/ReadModels/SessionReads.cs [28-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
HTTP 401 is a repairable credential lapse, but the new response bound permanently drops an agent-run event after fifteen unauthorized responses. A dropped stop event prevents the server from clearing its live agent-run state after the user logs in again.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentRunEventQueue.cs[69-80]
- src/Capacitor.Cli.Core/HookSpool.cs[30-40]
- /cross_repos/kcap-server/src/Capacitor.Api.Public/Agents/PublicAgentRunEndpoints.cs[5-12]
- /cross_repos/kcap-server/src/Capacitor.Server.Services/Agents/AgentRunProjector.cs[79-101]

## Recommended Fix
Do not spend the finite response budget on HTTP 401. Keep the event pending until token resolution produces working credentials or cancellation occurs, or persist it durably for replay after login; add a test where more than fifteen 401 responses are followed by successful authentication and verify that the event is delivered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. A test summary repeats its behavior ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

Transport_failures_do_not_count_toward_the_bound has a summary that repeats the method name and
the visible MaxRetryableResponses + 5 setup rather than adding a non-obvious constraint. When the
test mechanics change, this duplicate explanation becomes another description to update without
helping a reader understand behavior beyond the test body.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentRunEventQueueTests.cs[R52-53]

+    /// <summary>Pins that an unreachable server does not spend the bound: the head outlasts more
+    /// transport failures than the bound allows and is still delivered.</summary>

Relevance

●●● Strong

Recent test-review precedents accept removing summaries and comments that restate test names or
immediately visible behavior.

PR-#1031
PR-#616

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2762993 disallows comments that merely restate what the code clearly expresses. The summary
repeats both the test name and the immediately visible setup that exceeds the retry bound.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentRunEventQueueTests.cs[52-56]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The summary on `Transport_failures_do_not_count_toward_the_bound` repeats behavior already expressed by the test name and setup.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentRunEventQueueTests.cs[52-53]

## Recommended Fix
Remove the redundant XML summary. Keep the descriptive test name and existing setup as the documentation of the invariant.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

  Explored: repo: kurrent-io/kcap-deployments (sha: f6399149) — View relationship

  Explored: repo: kurrent-io/kcap-server (sha: 21e3a057) — View relationship

Review mode: ⚖️ Balanced: This is a substantive daemon delivery-logic change involving retries, event ordering, shutdown behavior, authentication headers, and resource ownership, so it warrants a complete single-pass review.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗