Skip to content

Find a sessionless driver's flow from a local run ledger - #1131

Merged
alexeyzimarev merged 4 commits into
mainfrom
flows-run-ledger
Sep 23, 2026
Merged

alexeyzimarev merged 4 commits into
mainfrom
flows-run-ledger

Conversation

@alexeyzimarev

@alexeyzimarev alexeyzimarev commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Closes #1123 — AI-3146

What & why

Cursor, Copilot, Gemini, Kiro, OpenCode and Antigravity give the flows MCP server no session, so a bare get_review_flow_status / get_flow_status there could not reach a run whose start the harness aborted. The flows server now records each run it starts without a session in flow-runs-v1.json under the config root, keyed on repo root (or cwd). A sessionless bare status call reads that workspace's newest entries and confirms each with GET /api/flows/{id}, applying the same open/settled/several-open rule as the session lookup. Claude Code and Codex record nothing and keep the server-side lookup.

Where to look

  • The record is written before the start's poll lane, which is what makes it survive an abort: the server ignores MCP cancellations, so the lane still holds the id.
  • Every retained entry is confirmed, not a fixed few — a probe cap lets newer settled runs hide an open one. A 404 inside the start poll's NotFoundGrace answers "retry" instead of skipping.
  • Replies say "this workspace", never the path. Chats sharing a workspace get a listed choice, not a wrong run.

Verification

  • StatusWithoutFlowRunIdTests 24/24, FlowRunLedgerTests 6/6, integration McpFlowsServerTests 36/36
  • Full CLI unit suite 4637/4660 under heavy load; the 2 failures (session_start_posts_to_session_start_route, Finds_the_PR_by_the_tracked_remote_name_when_the_local_name_misses) pass alone
  • dotnet publish -c Release: no IL warnings
  • Not verified live on Cursor

🤖 Generated with Claude Code

The ledger entry is written before the start's poll lane, so it survives a harness abort: the server ignores MCP cancellations and still holds the id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 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-23T14:19:32.215379Z 62cdc84 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

Recover sessionless flows from a local run ledger

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

Grey Divider

AI Description

• Records sessionless flow starts in a workspace-scoped local ledger before polling.
• Resolves bare status calls by validating recent ledger entries against the flows API.
• Preserves session-based lookup and documents timeout recovery across supported harnesses.
Diagram

sequenceDiagram
    actor H as Sessionless Harness
    participant M as Flows MCP
    participant L as Run Ledger
    participant A as Flows API
    H->>M: Start flow
    M->>A: POST start
    A-->>M: Flow run ID
    M->>L: Record by workspace
    M->>A: Poll round
    H->>M: Status without ID
    M->>L: Read recent runs
    loop Newest candidates
        M->>A: GET flow
        A-->>M: Flow metadata
    end
    M-->>H: Status or choices
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Server-side workspace lookup
  • ➕ Centralizes flow discovery without local state.
  • ➕ Avoids filesystem locking and retention concerns.
  • ➖ Requires a backend API and deployment change.
  • ➖ Workspace identity may be ambiguous across machines and users.
  • ➖ Introduces broader authorization and privacy considerations.
2. In-memory MCP run registry
  • ➕ Simpler implementation with no persistent file.
  • ➕ Avoids disk I/O and lock contention.
  • ➖ Loses recovery data when the MCP process restarts.
  • ➖ Cannot share candidates across MCP processes in the same workspace.

Recommendation: Keep the local ledger approach. It requires no server protocol change, survives harness cancellation and process restarts, scopes candidates by workspace, and validates every entry against the authoritative API before selection. The bounded retention, entry cap, atomic replacement, and best-effort failure model appropriately limit persistence risk.

Files changed (9) +440 / -55

Enhancement (1) +81 / -0
FlowRunLedger.csAdd a bounded workspace-scoped flow run ledger +81/-0

Add a bounded workspace-scoped flow run ledger

• Introduces best-effort atomic persistence for sessionless flow starts under the configuration root. Entries are deduplicated, capped at 50, retained for 14 days, protected by a lock, and filtered by workspace.

src/Capacitor.Cli/Commands/FlowRunLedger.cs

Bug fix (1) +106 / -37
McpFlowsServer.csResolve sessionless status calls from recorded workspace runs +106/-37

Resolve sessionless status calls from recorded workspace runs

• Records successful sessionless starts before entering the polling lane, including vendor-preference retries. Bare status calls read recent workspace candidates, confirm them individually through the flows API, and reuse the existing open/settled/multiple-open selection rules while retaining session-based lookup.

src/Capacitor.Cli/Commands/McpFlowsServer.cs

Refactor (1) +6 / -3
McpSessionId.csExpose non-throwing session resolution +6/-3

Expose non-throwing session resolution

• Adds 'TryResolveWithin' so callers can distinguish a missing session and apply another lookup strategy. Existing throwing behavior now delegates to the new helper.

src/Capacitor.Cli/Commands/McpSessionId.cs

Tests (2) +226 / -7
FlowRunLedgerTests.csCover run-ledger persistence and retention behavior +83/-0

Cover run-ledger persistence and retention behavior

• Tests workspace filtering, newest-first ordering, limits, deduplication, retention expiry, maximum size, and recovery from corrupt JSON.

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

StatusWithoutFlowRunIdTests.csCover sessionless status recovery and start recording +143/-7

Cover sessionless status recovery and start recording

• Adds coverage for workspace isolation, polling, multiple open runs, stale server entries, missing candidates, and sessionless start recording. Also verifies session-bearing starts continue to bypass the ledger.

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

Documentation (4) +21 / -8
README.mdDocument sessionless flow recovery through the local ledger +2/-2

Document sessionless flow recovery through the local ledger

• Explains that status calls without a flow ID can recover workspace-local runs for harnesses lacking session identity. Removes the previous requirement that those harnesses always retain and pass the ID.

README.md

CHANGES.mdRecord the sessionless run-ledger design +13/-0

Record the sessionless run-ledger design

• Adds a change note describing workspace keys, pre-poll recording, API confirmation, and handling of multiple open flows.

docs/CHANGES.md

SKILL.mdTeach generic flow agents to recover sessionless runs +3/-3

Teach generic flow agents to recover sessionless runs

• Updates timeout recovery and tool guidance so agents omit a lost flow ID on any harness. Sessionless harnesses now resolve candidates from the current workspace.

kcap/skills/agent-flows/SKILL.md

SKILL.mdTeach review agents to recover sessionless runs +3/-3

Teach review agents to recover sessionless runs

• Revises review-flow timeout and status instructions to cover workspace-ledger recovery instead of limiting ID-free recovery to Claude Code and Codex.

kcap/skills/review-flows/SKILL.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 62cdc84981

ℹ️ 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".

/// <summary>Confirms each recorded run against the server, newest first; a run the server no
/// longer knows is skipped rather than read.</summary>
async Task<FlowRunIdResolution> ResolveFromRunLedgerAsync(HttpClient client, string apiRoot, string workspace, FlowRetryClock clock) {
var recorded = new FlowRunLedger(config, time).Recent(workspace, RunLedgerCandidates);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Search all retained runs before choosing a settled flow

When a sessionless workspace has six or more retained runs, this limits resolution to the five newest even though the ledger retains 50. If those five are closed, failed, or no longer known to the server while the sixth is still running or waiting, ChooseFlow returns a newer settled run or an error instead of the documented newest open flow. Continue through the retained entries until every possible open candidate has been ruled out, or otherwise avoid allowing the fixed probe limit to change which run is selected.

Useful? React with 👍 / 👎.

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.

Fixed in 747b74c: the lookup now confirms every entry the workspace retains (≤50) before choosing, so an open run behind newer settled or unknown ones is found. Test: An_older_open_run_behind_many_newer_settled_ones_is_still_found.


if (resp.StatusCode == HttpStatusCode.Unauthorized)
return new(null, await AuthRejectionNotice.ForPersistentUnauthorizedAsync(store, profiles.Name, apiRoot, time));
if (resp.StatusCode == HttpStatusCode.NotFound) continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Retry a newly recorded run before discarding a 404

When a successful start has just been recorded but the flow read model has not exposed its GET route yet, this immediately discards the newest candidate on a transient 404 and can return an older flow's status or claim that no recorded flow is known. The existing round poll explicitly gives initial 404s a NotFoundGrace window for this propagation case; ledger recovery needs equivalent bounded retry behavior before skipping the recorded run.

Useful? React with 👍 / 👎.

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.

Fixed in 747b74c: a 404 on a run recorded within NotFoundGrace (the start poll's own window) now answers "started moments ago — retry" instead of skipping it. Outside the grace a 404 is still skipped. Test: A_just_recorded_run_the_server_cannot_read_yet_is_a_retry_not_a_skip.

@qodo-code-review

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

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Older open flow is skipped ✓ Resolved 🐞 Bug ≡ Correctness
Description
ResolveFromRunLedgerAsync passes the hard-coded five-entry RunLedgerCandidates limit to Recent
before ChooseFlow evaluates open versus settled state, despite the ledger retaining up to 50
entries. When an older open run sits behind at least five newer settled or failed runs—or all five
fetched entries are stale—the id-less status lookup can return a newer settled fallback or no run
without examining the older valid candidate.
Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[R1313-1314]

+    async Task<FlowRunIdResolution> ResolveFromRunLedgerAsync(HttpClient client, string apiRoot, string workspace, FlowRetryClock clock) {
+        var recorded = new FlowRunLedger(config, time).Recent(workspace, RunLedgerCandidates);
Relevance

●●● Strong

Deterministic truncation skips valid older open runs; analogous state-selection correctness fixes
were accepted.

PR-#656
PR-#193

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The ledger retains up to 50 entries, but the new sessionless status path fetches states for only the
five newest entries before applying ChooseFlow. Because ChooseFlow selects the first fetched
flow when that truncated set contains no open run, an older retained open flow—which established
selection behavior would prefer over a newer settled flow—is never considered.

src/Capacitor.Cli/Commands/McpFlowsServer.cs[1293-1338]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1354-1359]
src/Capacitor.Cli/Commands/FlowRunLedger.cs[27-34]
src/Capacitor.Cli/Commands/FlowRunLedger.cs[48-54]
test/Capacitor.Cli.Tests.Unit/Commands/StatusWithoutFlowRunIdTests.cs[182-202]
src/Capacitor.Cli/Commands/FlowRunLedger.cs[15-17]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1293-1315]

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 sessionless status path limits ledger entries before determining which retained runs are open or valid. An older active run can therefore be hidden by five newer settled runs, while five stale entries can prevent a valid older run from being resolved at all.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1293-1338]
- src/Capacitor.Cli/Commands/FlowRunLedger.cs[15-17]
- src/Capacitor.Cli/Commands/FlowRunLedger.cs[48-54]
- test/Capacitor.Cli.Tests.Unit/Commands/StatusWithoutFlowRunIdTests.cs[327-405]

## Recommended Fix
Apply the open-versus-settled selection rule across every retained workspace entry, up to `FlowRunLedger.MaxEntries`, rather than selecting from only the first five. Preserve a bounded or batched request strategy if needed, but do not select a settled flow until all older candidates that could still be open have been considered; stop early only once multiple open flows are known. Add sessionless tests where the sixth candidate is the only open run behind at least five newer settled runs, and where the first five candidates return 404, asserting that the older valid run is resolved.

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



Remediation recommended

2. Local paths leak to models ✓ Resolved 🐞 Bug ⛨ Security
Description
ResolveFromRunLedgerAsync interpolates the raw repoRoot ?? cwd workspace into no-record,
no-known-run, and multiple-open tool responses. Those responses are returned to the model, exposing
absolute local paths despite the server's existing invariant that repoRoot must never leak to the
model.
Code

src/Capacitor.Cli/Commands/McpFlowsServer.cs[R1315-1316]

+        if (recorded.Count == 0)
+            return new(null, $"Error: this harness gives kcap no session id, and no flow started from {workspace} is recorded on this machine. {PassTheFlowRunId}");
Relevance

●●● Strong

Direct absolute-path disclosure conflicts with established MCP privacy handling; closely matching
path-leak fixes were accepted.

PR-#643
PR-#226

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workspace key is explicitly the repository root or current working directory, and the new lookup
inserts it verbatim into multiple returned errors. Elsewhere in the same tool server, an explicit
invariant states that the local repository root must not leak to the model.

src/Capacitor.Cli/Commands/McpFlowsServer.cs[409-413]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1293-1299]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1313-1316]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1338-1338]
src/Capacitor.Cli/Commands/McpFlowsServer.cs[1385-1387]

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

## Issue description
Sessionless status responses expose the absolute repository or working-directory path to the calling model.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1295-1295]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1313-1338]
- src/Capacitor.Cli/Commands/McpFlowsServer.cs[1385-1387]

## Recommended Fix
Continue using the absolute path only as the internal ledger key, but replace it in model-visible messages with a generic label such as `this workspace` or a non-sensitive repository identity. Update status tests to assert that neither `repoRoot` nor `cwd` appears in tool responses.

ⓘ 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: 9cfe10b6)
Review mode: 🧠 Deep: This adds persistence, concurrency/atomic-file handling, sessionless flow resolution, HTTP confirmation and status-selection logic across multiple code paths, creating several independent, easy-to-miss behavioral risks.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/McpFlowsServer.cs Outdated
Comment thread src/Capacitor.Cli/Commands/McpFlowsServer.cs Outdated
alexeyzimarev and others added 3 commits September 23, 2026 17:47
A fixed probe count let newer settled runs hide an open one, and a 404 inside the start's not-found grace may only mean the server has not caught up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ows-run-ledger

# Conflicts:
#	docs/CHANGES.md
…ows-run-ledger

# Conflicts:
#	docs/CHANGES.md
@alexeyzimarev
alexeyzimarev merged commit ed67499 into main Sep 23, 2026
8 checks passed
@alexeyzimarev
alexeyzimarev deleted the flows-run-ledger branch September 23, 2026 16:42
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.

Flows: recover a running flow without its id on harnesses that give the flows server no session

1 participant