Skip to content

End a flows tool call before the shortest harness timeout - #1097

Merged
alexeyzimarev merged 4 commits into
mainfrom
alexeyzimarev/ai-3075-flows-survive-harness-tool-timeout
Sep 22, 2026
Merged

alexeyzimarev merged 4 commits into
mainfrom
alexeyzimarev/ai-3075-flows-survive-harness-tool-timeout

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Part of #1095 — AI-3075

What & why

Codex aborts an MCP tool call at 300 s. A flows call is bounded at 9 minutes, so on Codex a round over five minutes ends in the harness's own timeout error, and an aborted start_review_flow never hands the driver its flow_run_id. The bounds now fit under 300 s for every harness, so the call ends on the benign "Flow still running" reply, id included.

bound value
ToolCallBudget 4 min
PollCap, SettlementAbsoluteDeadline 3m30s
SettlementElapsedDeadline 3 min

Where to look

SettlementElapsedDeadline stays at 3 minutes: it is one cycle of the server's reconcile sweep, which is what lets a participant_unreachable retry succeed. The absolute settlement cap therefore sits 30 s above it, so a daemon lane that keeps making progress re-arms the window for 30 s before the caller gets the retryable busy error — on Claude Code too. #1095 stays open for the recoverable id, the Codex tool_timeout_sec and the timeout guidance.

Verification

  • The two new tests against the 9-minute bounds: Expected to be less than 00:05:00 but received 00:09:35 (start) and 00:08:35 (wait: true). Both pass with the change.
  • dotnet run --project test/Capacitor.Cli.Tests.Unit/Capacitor.Cli.Tests.Unit.csproj → total 4611, failed 1: session_start_omits_workspace_root_when_cwd_has_no_git_repo, which passes run alone.
  • dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release 2>&1 | grep -E 'IL[23][01][0-9]{2}' → no output.

🤖 Generated with Claude Code

Codex aborts an MCP tool call at 300 s, and an aborted start never hands the
driver its flow_run_id. The 3-minute settlement window is one cycle of the
server's reconcile sweep and cannot shrink, so the absolute settlement cap
keeps only 30 s above it. The bounds apply to every harness, not per client.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 21, 2026

Copy link
Copy Markdown

AI-3075

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

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-21T15:43:56.775303Z c969818 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

Copy link
Copy Markdown

PR Summary by Qodo

End flow tool calls before harness timeouts

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Bounds flow calls below Codex’s 300-second MCP timeout.
• Preserves retry guidance and flow IDs when rounds remain active.
• Adds deadline coverage and updates documented status-wait durations.
Diagram

sequenceDiagram
    actor H as Harness
    participant M as MCP Flows
    participant S as Flow Server
    participant R as Reviewer
    H->>M: Start flow
    M->>S: POST start
    S->>R: Launch round
    S-->>M: Running status
    loop Shared call budget
        M->>S: Poll status
        S-->>M: Still running
    end
    M-->>H: Flow ID and retry
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Per-harness timeout budgets
  • ➕ Allows harnesses with longer limits to wait longer per call.
  • ➕ Reduces repeated status calls for Claude Code and other permissive clients.
  • ➖ Duplicates deadline configurations and increases drift risk.
  • ➖ Requires reliable harness detection and client-specific testing.
2. Return flow IDs immediately
  • ➕ Decouples flow identity from long-running round completion.
  • ➕ Eliminates the risk of losing recovery information to tool-call timeouts.
  • ➖ Requires a larger protocol and caller-workflow change.
  • ➖ Makes every start asynchronous and may affect existing completion behavior.

Recommendation: Use the PR’s shortest-harness global budget for this fix: it is backward-compatible, deterministic, and protects all clients without client detection. Returning the flow ID immediately is the stronger long-term architecture, but it should be handled separately because it changes the start protocol and caller expectations.

Files changed (9) +209 / -132

Bug fix (1) +52 / -55
McpFlowsServer.csBound flow calls below the shortest harness timeout +52/-55

Bound flow calls below the shortest harness timeout

• Introduces the 300-second shortest-harness limit, reduces the shared tool-call budget to four minutes, and caps polling and absolute settlement at 3.5 minutes. The three-minute reconcile-aware settlement window remains unchanged so 'participant_unreachable' retries can still succeed.

src/Capacitor.Cli/Commands/McpFlowsServer.cs

Tests (4) +132 / -72
McpFlowsServerSettlementRetryTests.csAlign settlement retry tests with reduced caps +5/-5

Align settlement retry tests with reduced caps

• Updates settlement and polling expectations to use the 210-second cap and asserts elapsed retries remain below the shortest harness timeout.

test/Capacitor.Cli.Tests.Unit/Commands/McpFlowsServerSettlementRetryTests.cs

SettlementProgressWindowTests.csRebase progress-window scenarios onto the new absolute cap +50/-47

Rebase progress-window scenarios onto the new absolute cap

• Compresses scripted watermark progress into the 30-second margin above the three-minute rolling window. Renamed and revised assertions continue verifying re-arms, stalls, regressions, and absolute clipping.

test/Capacitor.Cli.Tests.Unit/Commands/SettlementProgressWindowTests.cs

StatusWaitArgumentTests.csVerify status waits finish before harness timeout +38/-7

Verify status waits finish before harness timeout

• Updates status-wait tests for the 210-second poll cap and adds coverage proving a perpetually running round returns benign guidance before 300 seconds, including worst-case request overshoot.

test/Capacitor.Cli.Tests.Unit/Commands/StatusWaitArgumentTests.cs

ToolCallBudgetTests.csVerify shared start budget remains harness-safe +39/-13

Verify shared start budget remains harness-safe

• Updates shared-budget arithmetic for the shorter poll cap and adds a worst-case start test combining settlement delay with a nonterminal round. The test verifies the response includes both still-running guidance and the recoverable flow ID before the harness timeout.

test/Capacitor.Cli.Tests.Unit/Commands/ToolCallBudgetTests.cs

Documentation (4) +25 / -5
README.mdDocument the shorter flow status wait +1/-1

Document the shorter flow status wait

• Changes the documented 'get_flow_status(wait: true)' duration from roughly eight minutes to roughly 3.5 minutes so callers know when to re-enter the wait.

README.md

CHANGES.mdExplain harness-safe flow deadline sizing +20/-0

Explain harness-safe flow deadline sizing

• Adds a design-history entry describing the lost-flow-ID failure mode, the 300-second Codex limit, and the rationale for the new settlement, polling, and tool-call bounds.

docs/CHANGES.md

SKILL.mdUpdate generic flow wait guidance +2/-2

Update generic flow wait guidance

• Updates agent instructions and the generic tool reference to describe a roughly 3.5-minute bounded status wait followed by another status call when needed.

kcap/skills/agent-flows/SKILL.md

SKILL.mdUpdate review flow wait guidance +2/-2

Update review flow wait guidance

• Revises review-flow instructions and tool documentation to reflect the shorter bounded wait while retaining the benign still-running retry workflow.

kcap/skills/review-flows/SKILL.md

@qodo-code-review

qodo-code-review Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Model starts still exceed harness timeout ✓ Resolved 🐞 Bug ☼ Reliability
Description
HandleToolCallAsync calls StartFlowAsync directly for model-bearing starts without passing a
cancellation token, while the shared HttpClient has an infinite timeout. When either the v4
request or its v3 fallback is held open, this path bypasses both the new four-minute budget and
settlement deadline, so the harness can abort before the caller receives the flow run id.
Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[R519-521]

+    /// end before <see cref="ShortestHarnessToolTimeout"/>. A harness abort mid-poll leaves a reviewer
+    /// launched and paid for with nobody waiting on its result.</para></summary>
+    internal static readonly TimeSpan ToolCallBudget = TimeSpan.FromMinutes(4);
Relevance

●●● Strong

Unbounded model-start network work conflicts with the PR’s stated harness-timeout goal;
timeout-boundary fixes are routinely accepted.

PR-#480
PR-#187

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The production client disables its global timeout, and the model-start branches invoke the
token-aware StartFlowAsync overload without supplying a token. In contrast, ordinary starts run
through SendWithSettlementRetryAsync, which creates a deadline and passes its token into the POST,
proving the new bounds do not cover the model path.

src/Capacitor.Cli/Commands/McpFlowsServer.cs[74-79]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[238-253]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[649-665]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1128-1157]

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

## Issue description
Model-bearing starts bypass the new tool-call deadline because their direct `StartFlowAsync` calls receive no cancellation token while the shared HTTP client has an infinite timeout.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[238-253]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[511-521]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1128-1157]

## Recommended Fix
Create a deadline scope for the model-start lane using the same `settlementStartedAt` and `ToolCallBudget`, pass its token through `StartFlowAsync` so both v4 and fallback requests share the deadline, and map deadline cancellation to a controlled retryable tool response without adding settlement re-POST behavior.

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



Remediation recommended

2. Users get conflicting wait guidance ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
README.md line 575 and both flow-status tool descriptions say wait: true lasts roughly 3.5
minutes but still call the same 210-second PollCap an 8-minute cap. When polling reaches that cap
and returns a benign still-running result, drivers and users of either alias receive contradictory
retry guidance that can make the normal response appear premature.
Code

README.md[575]

+- **`get_flow_status`** — get the current status (running, waiting, completed, failed) and last result of a flow run. Optional `wait` (boolean, default `false`): when `true`, blocks — via repeated bounded internal checks, never a raw long-poll — until the round is terminal or roughly 3.5 minutes pass, instead of returning the current snapshot immediately. A long-running round is expected, not an error; on the 8-minute cap this returns the same benign "still running" text an unset/false `wait` already returns on the round-submission path, so simply call it again with `wait: true`.
Relevance

●●● Strong

Direct documentation contradiction after the timeout change; correcting stale cap wording is
deterministic and aligns with repository documentation practice.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2270057 requires the README CLI documentation to match changed defaults and behavior, and
PollCap is defined as 210 seconds with the polling loop returning a benign still-running result
when it expires. The cited README line and both generated MCP tool descriptions state the updated
roughly 3.5-minute duration while retaining the stale eight-minute-cap wording in the same
sentences.

Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
README.md[575-575]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[2129-2135]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[2197-2203]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1219-1224]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1599-1604]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[2126-2135]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[2194-2203]

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 updated flow-status documentation says the wait cap is roughly 3.5 minutes but still refers to it as an 8-minute cap in the same README sentence and both MCP tool descriptions, contradicting the 210-second implementation.

## Fix Focus Areas
- README.md[575-575]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[2129-2129]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[2197-2197]

## Recommended Fix
Replace each stale `on the 8-minute cap` phrase with wording consistent with the new 3.5-minute limit, such as `on the 3.5-minute cap` or the `PollCap`-neutral `on that cap`, while preserving the retry guidance.

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


3. Three comments embed harness metadata ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
ShortestHarnessToolTimeout is documented with the external claim that Codex aborts MCP calls at
300 seconds, and the same claim is copied into two test comments. A harness timeout change would
leave production and test commentary stale even though the code-level invariant only needs the
budgets to remain below the supported minimum.
Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[R478-481]

+    /// <summary>The shortest tool-call timeout among the harnesses that drive flows: Codex aborts an
+    /// MCP call at 300 s. A call the harness aborts never delivers its reply — for a start, the only
+    /// place the driver is handed its flow_run_id — so every bound below is sized to end the call
+    /// first.</summary>
Relevance

●●● Strong

Recent precedent accepts removing drift-prone external process and time metadata from comments.

PR-#972
PR-#529

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2897915 prohibits comments containing external process references or time-sensitive metadata.
The production comment and two test comments explicitly identify Codex and its current 300-second
timeout rather than expressing only the durable budgeting invariant.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
src/Capacitor.Cli/Commands/McpFlowsServer.cs[478-482]
test/Capacitor.Cli.Tests.Unit/Commands/StatusWaitArgumentTests.cs[222-224]
test/Capacitor.Cli.Tests.Unit/Commands/ToolCallBudgetTests.cs[147-150]

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

## Issue description
Three added comments encode a named harness's current 300-second timeout, creating process-dependent commentary that can become stale when harness behavior changes.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[478-482]
- test/Capacitor.Cli.Tests.Unit/Commands/StatusWaitArgumentTests.cs[222-224]
- test/Capacitor.Cli.Tests.Unit/Commands/ToolCallBudgetTests.cs[147-150]

## Recommended Fix
Rewrite the comments to state the durable invariant: flow calls must finish before the supported minimum tool-call timeout so their replies and flow identifiers are delivered. Avoid naming Codex or narrating its current timeout in comments.

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


4. Server recovery outlasts retry window 🔗 Cross-repo conflict ☼ Reliability
Description
SettlementAbsoluteDeadline assumes the server’s three-minute sweep interval completes recovery
within 210 seconds, but that interval precedes several sequential passes and the heal-convergence
pass alone has a further 30-second budget. When participant_unreachable requires durable-proof
convergence, a retry arriving after the relevant pass can hit the new ceiling before the next pass
repairs the role, so the CLI returns a settlement-busy result instead of completing the retry.
Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[509]

+    internal static readonly TimeSpan SettlementAbsoluteDeadline = TimeSpan.FromSeconds(210);
Relevance

●● Moderate

Potential cross-repository timing mismatch is plausible, but no closely matching accepted precedent
confirms changing this client ceiling.

PR-#475
PR-#1051

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR establishes a hard 210-second client ceiling. The server waits three minutes before each
periodic sweep, runs multiple passes before heal convergence, gives that convergence pass its own
30-second budget, and tests a real crash state that only this pass repairs; therefore the server
does not guarantee repair within the client's new window.

src/Capacitor.Cli/Commands/McpFlowsServer.cs[505-521]
External repo: kurrent-io/kcap-server, src/Capacitor.Server/Flows/FlowReconcilerService.cs [316-369]
External repo: kurrent-io/kcap-server, src/Capacitor.Server/Flows/FlowReconcilerService.cs [1447-1451]
External repo: kurrent-io/kcap-server, test/Capacitor.Server.Tests.Flows/HealConvergencePassTests.cs [163-177]

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 CLI now gives server-side heal convergence only 210 seconds, but the server waits three minutes before beginning a sequential reconciliation sweep and does not guarantee that the required pass finishes within the remaining 30 seconds.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[505-521]
- /cross_repos/kcap-server/src/Capacitor.Server/Flows/FlowReconcilerService.cs[316-369]
- /cross_repos/kcap-server/src/Capacitor.Server/Flows/FlowReconcilerService.cs[1447-1451]

## Recommended Fix
Coordinate a kcap-server change that triggers or prioritizes heal convergence when durable absence evidence arrives, with an end-to-end test proving `participant_unreachable` becomes retryable within 210 seconds. Keep the CLI ceiling only after that server timing guarantee exists; otherwise use a protocol that returns a recoverable flow identifier without waiting for convergence.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (sha: b56e8a37)
Review mode: ⚖️ Balanced: This changes timeout, retry, polling, and flow-ID delivery behavior across production code and multiple test paths, creating real harness and liveness risk, but it is not broad or defect-dense enough to require redundant extended review.

Grey Divider

Tip of the day
💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread README.md Outdated
Comment thread src/Capacitor.Cli/Commands/McpFlowsServer.cs
Comment thread src/Capacitor.Cli/Commands/McpFlowsServer.cs
/// daemon-lane progress can re-arm <see cref="SettlementElapsedDeadline"/> indefinitely, so this
/// is what bounds the lane. Stays under <see cref="ToolCallBudget"/> so that a start admitted at
/// the last moment still has time to return its flow_run_id.</summary>
internal static readonly TimeSpan SettlementAbsoluteDeadline = TimeSpan.FromSeconds(210);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

4. Server recovery outlasts retry window 🔗 Cross-repo conflict ☼ Reliability

SettlementAbsoluteDeadline assumes the server’s three-minute sweep interval completes recovery
within 210 seconds, but that interval precedes several sequential passes and the heal-convergence
pass alone has a further 30-second budget. When participant_unreachable requires durable-proof
convergence, a retry arriving after the relevant pass can hit the new ceiling before the next pass
repairs the role, so the CLI returns a settlement-busy result instead of completing the retry.
Agent Prompt
## Issue description
The CLI now gives server-side heal convergence only 210 seconds, but the server waits three minutes before beginning a sequential reconciliation sweep and does not guarantee that the required pass finishes within the remaining 30 seconds.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[505-521]
- /cross_repos/kcap-server/src/Capacitor.Server/Flows/FlowReconcilerService.cs[316-369]
- /cross_repos/kcap-server/src/Capacitor.Server/Flows/FlowReconcilerService.cs[1447-1451]

## Recommended Fix
Coordinate a kcap-server change that triggers or prioritizes heal convergence when durable absence evidence arrives, with an end-to-end test proving `participant_unreachable` becomes retryable within 210 seconds. Keep the CLI ceiling only after that server timing guarantee exists; otherwise use a protocol that returns a recoverable flow identifier without waiting for convergence.

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changed by this PR. participant_unreachable carries no last_processed_seq, so its window never re-arms and it exhausts at the flat SettlementElapsedDeadline — 3 minutes before this change and after it. The 210 s SettlementAbsoluteDeadline only ever clips the seq-re-armed path. The underlying observation is fair: the server sleeps 3 minutes between sweeps, so a retry that starts just after one can miss the next by the length of a sweep. It then gets the retryable error and succeeds on the next attempt. Widening that window is a separate decision; the doc comment no longer claims the window covers a full sweep cycle.

alexeyzimarev and others added 3 commits September 21, 2026 18:52
It is one POST outside the settlement-retry lane, so that lane's deadline
never reaches it, and the shared client has no timeout of its own. It cannot
be re-sent: a second POST would mint and launch a second run.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…exeyzimarev/ai-3075-flows-survive-harness-tool-timeout

# Conflicts:
#	docs/CHANGES.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant