Repository navigation
Let a flow driver find its running flow without the id - #1105
Conversation
…1103) An entry kcap wrote earlier is healed to the new shape through the ownership lane; the fingerprint covers integer values, so a timeout the user set is kept. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…1103) The lookup route, GET /api/flows?requesting_session_id=...&state=all, is kcap-server#1982 and is not on the server yet; an older server's 404 is worded as pass-the-id. The blocking tools' descriptions, both flow skills and the README carry the harness-timeout guidance. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoRecover flow status by session when the run ID is unavailable
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59f42b9779
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| new("kcap-flows", ["mcp", "flows"], NeedsProjectCwd: true, | ||
| "Structured AI agent flows — launches a SEPARATE hosted participant agent; requires login + a running daemon."), | ||
| "Structured AI agent flows — launches a SEPARATE hosted participant agent; requires login + a running daemon.", | ||
| ToolTimeout: TimeSpan.FromMinutes(10)), |
There was a problem hiding this comment.
Codex users who enable kcap through codex plugin add load kcap/.codex-mcp.json, but that descriptor's kcap-flows entry still contains only command and args; ToolTimeout is emitted only by CodexConfigToml. I verified with the installed Codex 0.144 CLI that a tool_timeout_sec value in a plugin MCP descriptor is honored (codex mcp get reports the configured value). Consequently, native-plugin users retain Codex's default timeout and can still have flow calls aborted despite this change, so the descriptor should carry the 600-second value too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added in c525a37: kcap/.codex-mcp.json carries tool_timeout_sec: 600 on kcap-flows, pinned against the descriptor's ToolTimeout by McpCanonicalContractTests.Bundled_codex_mcp_json_carries_the_flows_tool_timeout.
| var sessionId = McpSessionId.ResolveWithin(arguments, requestingSessionId); | ||
| var url = $"{apiRoot}/api/flows?requesting_session_id={Uri.EscapeDataString(sessionId)}&state=all"; |
There was a problem hiding this comment.
When the flows MCP server runs under Cursor, Copilot, Gemini, Kiro, OpenCode, or Antigravity, HarnessRequesterContext.Resolve() has no per-process session signal: HarnessRequesterContext.cs documents that KCAP_SESSION_ID is shell-only and may be inherited from a parent, while only Claude and Codex expose their own session IDs. Thus requestingSessionId here is null or wrong, and starts were attributed using that same value, so omitting flow_run_id cannot recover the correct run in those supported drivers despite the new universal tool/skill guidance. This needs a reliable per-harness session association or a locally persisted run ID rather than depending solely on requestingSessionId.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct as a limitation, and not fixable inside this PR: those harnesses export no per-process session signal, so their starts carry no requesting_session_id and no session-keyed lookup can find them. In 075e212 the tool descriptions, both skills and the README name Claude Code and Codex as the harnesses where the id may be omitted. A local run ledger for the rest is #1123.
Code Review by Qodo
1. Id-free flow recovery always fails
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| var sessionId = McpSessionId.ResolveWithin(arguments, requestingSessionId); | ||
| var url = $"{apiRoot}/api/flows?requesting_session_id={Uri.EscapeDataString(sessionId)}&state=all"; | ||
| using var getCts = clock.CreateTimeoutSource(PerGetTimeout); | ||
| using var resp = await client.GetAsync(url, getCts.Token); |
There was a problem hiding this comment.
1. Id-free flow recovery always fails 🔗 Cross-repo conflict ≡ Correctness
ResolveStatusFlowRunIdAsync calls GET /api/flows?requesting_session_id=…&state=all, but the pinned kcap-server exposes no collection GET route under /api/flows. Any status call without flow_run_id therefore receives 404 and stops before reading the running flow, including the harness-timeout recovery path introduced by this PR.
Agent Prompt
## Issue description
The newly optional `flow_run_id` relies on a session-filtered flow-list endpoint that the pinned kcap-server does not implement, so ID-free status recovery always returns the unsupported-server error.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1250-1303]
- /cross_repos/kcap-server/src/Capacitor.Api.Public/Flows/FlowEndpoints.cs[38-98]
## Recommended Fix
Implement and release the authenticated `GET /api/flows` route in kcap-server with the exact query parameters, ordering, status semantics, and response fields consumed here, then coordinate deployment so it precedes or accompanies this CLI release. If that coordination is unavailable, keep `flow_run_id` required and do not advertise ID-free recovery until the server capability exists.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Known and stated in the description: the route is kurrent-io/kcap-server#1982, with the row contract posted there. Against today's server a bare call answers cannot look up flows by session … pass the flow_run_id, which is what the driver had to do before this change, so the schema change is additive and ships safely ahead of the route. Merge order is the maintainer's decision.
Codex reads a plugin descriptor entry into the same server config as config.toml, so the key is honoured there and a native-plugin install would otherwise keep the 300 s default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A bare status call resolves the session on Claude Code and Codex only: the JSON harnesses give the flows server no session, so the guidance names the two; #1123 covers the rest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ows-status-without-id # Conflicts: # docs/CHANGES.md
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Closes #1103 — AI-3104
What & why
A driver whose harness aborted
start_review_flownever receives theflow_run_id, and context compaction loses it later; either way the run keeps working with no way back to it. The two status tools take the id as optional and read the calling session's newest open flow instead: several open flows are listed to choose from, and none open falls back to the newest settled one so a failure the driver missed still reads as failed. The Codexkcap-flowsregistration and the bundled plugin descriptor carrytool_timeout_sec = 600, healed into entries kcap wrote earlier and never into edited or foreign ones. The four blocking tools, both flow skills and the README say up front what a harness abort means and which call recovers from it.Where to look
The lookup calls
GET /api/flows?requesting_session_id=…&state=all, which is kurrent-io/kcap-server#1982 and is not on the server yet; against today's server the bare call answers "cannot look up flows by session, pass the id". The row contract the CLI reads is posted on that issue. The bare call resolves a session on Claude Code and Codex only, since the JSON harnesses give the flows server no session identity; the guidance says so, and #1123 covers those harnesses.Verification
Capacitor.Cli.Core.Tests.Unit: 3604 passed, 0 failedCapacitor.Cli.Tests.Unit: 4621 passed, 0 failedCapacitor.Cli.Tests.Integration/McpFlowsServerTests: 36 passeddotnet publish -c Release: no IL2026/IL3050 warnings;dotnet build Capacitor.slnx: 0 warningsget_review_flow_status(wait: true)reaching a live run needs the server route.🤖 Generated with Claude Code