Skip to content

fix(sdk): reuse authored CLI probes for each run so concurrent f.llm never outlives its lease (#561) - #576

Merged
khaliqgant merged 12 commits into
mainfrom
relayflow/flows-software-garden-9997fbb6
Sep 24, 2026
Merged

khaliqgant merged 12 commits into
mainfrom
relayflow/flows-software-garden-9997fbb6

Conversation

@agent-relay-code

@agent-relay-code agent-relay-code Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Closes #561.

The stale dispatch

authoredWorkerRunner runs the declarative preflight (checkAuthoredFlow)
per authored f.llm/f.agent call, and that preflight real-probes the CLI —
claude auth status and friends — synchronously. Nine concurrent calls
therefore spawned nine blocking probes on the one Node event loop.

WorkerSlots was never the problem: it bounds admission correctly, and
overflow calls do wait before run.start. The damage is done to the calls that
were admitted. While the remaining probes block the loop, the worker cannot
service its dispatch or send a heartbeat, so the kernel sees a lease started and
never renewed. At +30s it journals lease_expired, retries, and the worker's
withWorkerLease throws Agent lease is already expired on the stale dispatch
it finally picks up.

The fix

Give each authored run one cliProbeCache (and one readProjectConfig) shared
across its calls — exactly what declarative preflight already does for a whole
flow, where a single preflight() call caches probes across every step. The
cache is per run and holds refusals as well as passes, keyed on CLI, model and
resolution source (packages/sdk/tests/preflight-run-cache.test.ts pins that).
One probe is paid; the other eight calls resolve from cache and reach the worker
well inside the lease.

Three files change, 22 lines net:

  • packages/sdk/src/preflight.ts — accept an optional cliProbeCache; export
    CliProbeOutcome.
  • packages/sdk/src/cli/check.ts — thread it through checkAuthoredFlow.
  • packages/sdk/src/authored-worker-step.ts — one cache per runner.

Also in this branch: tests/artifact-gates.test.ts declared an absolute
mkdtempSync cwd, which #566 turned into a compile refusal one commit before
#517 added the fixture. main is red at that merge, on this branch and any
other cut from it. The fixture now makes its directory inside the run root and
declares the relative name; no assertion changed.

Acceptance

  • A test that runs 9+ concurrent f.llm calls against the real kernel
    with a fake CLI, no lease_expired in any child journal, at capacity 1 and
    the default
    — packages/sdk/tests/authored-probe-cache.test.ts. It reads
    every child journal through journalRead and asserts exactly one
    step.attempt.started per run, and that peak concurrent sessions never
    exceed capacity.
  • Mutation-verified, re-run at this branch's head. Removing only the
    fifth checkAuthoredFlow argument (evidence/561/mutation.patch) fails all
    five cases, and the journal shows the reported shape verbatim — attempt 1
    "completionReason":"lease_expired" with "wallclock_ms":30011, a
    retry_backoff sleep, attempt 2 "completionReason":"success". Restoring
    (git diff --exit-code clean, sha256
    b9cd463d9f1ee274175a0b659119262c0ba5b18f1afc11b866c3e0f7f34023b7) returns
    5 passed. Captures: evidence/561/mutation-red-at-head.txt,
    evidence/561/mutation-green-at-head.txt, walked through in
    evidence/561/README.md.
  • The repro passes locally — evidence/561/live-default.txt and
    evidence/561/live-one.txt, both exit 0, journals asserted by
    evidence/561/check-live-journals.py. Captured while this environment had
    authenticated Claude. It no longer has it, so this was not re-confirmed
    at head; evidence/561/live-one-recheck-unauthenticated.txt is the
    unauthenticated re-run, which still shows the mechanism from outside (one
    1.04s probe, then eight refusals in 0.01–0.12s). The kernel-backed regression
    covers the same behaviour with a fake CLI and needs no credentials.
  • examples/prompt-lab restores Promise.all — not done here.
    examples/prompt-lab does not exist in this repository; it lives on
    feat/examples-prompt-lab. It needs a follow-up there once this ships
    (evidence/561/prompt-lab.txt).

Check status

.relayflow/check.sh is green except for 22 cases in
tests/hosted-extension-{isolation,protocol}.test.ts, which need a real
bubblewrap sandbox. This container denies unprivileged user namespaces and the
AppArmor sysctl cannot be relaxed from inside it, so installing bubblewrap only
changes the error. Unrelated to this change and classified in full, with the
commands, in .relayflow/repair-notes.md.

🤖 Generated with Claude Code


Note

Medium Risk
Changes the preflight path on every authored LLM/agent worker step and timing-sensitive lease behavior; mitigated by kernel-backed regression tests and mutation verification, but still core execution infrastructure.

Overview
Fixes worker lease expiry when many concurrent f.llm / f.agent calls each ran synchronous CLI auth/model probes before dispatch. Nine probes on one Node loop blocked heartbeats; admitted steps hit lease_expired (~30s) and stale WorkerLeaseLostError.

Per authored run, a new authoredPreflight path shares one probe cache (and project config read), dedupes in-flight probes, and runs live checks via probeCliAsync so the event loop can serve worker I/O. Probe implementation moves to cli/cli-probe.ts; checkAuthoredFlow accepts an optional probe cache / options bag for the same facts declarative preflight already caches per flow.

Adds authored-probe-cache (and related) tests plus evidence/561/ transcripts (baseline, mutation patch, live repro) documenting the failure shape and verification.

Reviewed by Cursor Bugbot for commit 2dab6ae. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Fixes concurrent f.llm/f.agent runs each paying a synchronous CLI auth probe before worker admission. Slow probes blocked the worker's event loop, so issued 30-second leases were never heartbeated and expired, raising Agent lease is already expired on a stale dispatch.

Each authored run now probes once and shares results across its calls. A new authoredPreflight keeps a per-run probe cache (keyed on CLI, model, and resolution source; failures cached too) and reads project config once. Probe logic moved into cli/cli-probe.ts; live-flow probes run asynchronously via probeCliAsync so the loop yields, and asynchronous probes isolate stdin from concurrent calls. checkAuthoredFlow now takes an options bag, and authored option bags without a projectConfig key are still detected and never become config. The synchronous public API is unchanged.

Bug Fixes

  • artifact-gates declared an absolute temp cwd, which is now a compile refusal; the fixture's directory is created inside the run root with a relative name. The merge with main resolved the conflict with the revert of the summary.md working file.

Tests

  • authored-probe-cache runs 9 concurrent LLM calls against the real kernel with a fake CLI at capacities 1 and 4, asserting no lease_expired and one probe per run.
  • authored-parallel-llm, authored-preflight, cli-probe, and preflight-run-cache cover the new cache, sync/async probe drivers, and cache key shape.
  • Mutation-verified: dropping the cache argument reproduces lease_expired on attempt 1 across all five cases; evidence/561/ holds the transcripts.

Written for commit 2dab6ae. Summary will update on new commits.

Review in cubic

Relayflow and others added 6 commits September 24, 2026 06:12
The regression added by #517 declares `cwd` on a spec it hands to
`preflight`, and used an `os.tmpdir()` directory — an absolute path.
#566 landed one commit earlier and made an absolute `cwd` a compile
refusal (`agent-cwd.ts`: a declared `cwd` is run-root-relative, the
same rule `relayflowd_core::spec::is_run_root_relative_path` applies
at the kernel boundary). Each PR was green alone; together they are
not, and `main` at e30226c fails this test with `invalid_spec` where
it expects `gate_path_unscanned`.

The fixture now makes its directory inside the run root and declares
the relative name. Nothing else moves: the warning, `ok`, the real
`AgentWorker` dispatch, the empty scan snapshot, the journaled JSON
output and the lowered gate command are asserted exactly as before.

Reverting this file to its e30226c bytes fails the case and restoring
it passes; both captures are in evidence/561/artifact-gates-{red,green}.txt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…vidence

The repository check on this branch failed with 30 tests across five files.
None came from #561. Each is classified by reverting this branch's three
source files to origin/main (e30226c), re-running and restoring:

  * artifact-gates (1)     — main is red; #566 and #517 conflict semantically.
                             Fixed in the preceding commit.
  * live-kernel (7)        — this sandbox's HOME declares "type": "commonjs"
                             above the checkout, so testdata/preflight's
                             extensionless ESM fixture CLIs load as CommonJS
                             and emit nothing, silently. Local setup only.
  * authored-node-runtime  — Bun 1.3.6 where every workflow pins 1.4.0.
  * hosted-extension (22)  — unprivileged user namespaces denied to this
                             container; bwrap cannot run even once installed.

The mutation is re-run at this head. The mutated run reproduces the issue's
exact signature — lease_expired on a first attempt that never heartbeated,
retry, second attempt success — and the restore is byte-identical by SHA-256.

The live-Claude repro could not be re-run: this environment's claude is no
longer authenticated. The capture says so rather than the acceptance box
claiming a pass it cannot show.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The committed mutation transcripts were captured before the artifact-gate
fix (1ccaaed) landed. Re-run `evidence/561/mutation.patch` against the
current head so the transcript matches the bytes a reviewer checks out,
and record the restore with `git diff --exit-code` plus a sha256sum.

The failing capture carries the issue's exact journal shape at both
capacity 1 and the default: attempt 1 completes `lease_expired` with
`wallclock_ms: 30011`, a `retry_backoff` sleep follows, and attempt 2
succeeds. Restored, all five cases pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d5e726f6-0970-4b53-b274-aec3fd93daa1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@khaliqgant

Copy link
Copy Markdown
Member

Shepherd decision (2026-09-24): I compared this branch with duplicate #564. #576 is the safer starting point: its exact head 909a1d2 typechecks (npm run typecheck && npm run typecheck:tests exit 0) against the newer SDK surface, while #564 head b84c841 currently fails the same typecheck at authored-flow-executor.ts:438 and is based on older 2.0.29. #564 does contain stronger async-probe coverage (6s/45s cold probes), so before closure I will verify whether that behavior is required here and, if so, port only the minimal tested part into this branch. No merge or mark-ready; current head remains guarded.

Relayflow and others added 2 commits September 24, 2026 13:45
Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946
# Conflicts:
#	packages/sdk/tests/artifact-gates.test.ts

Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946

@cursor cursor 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.

Stale Bugbot comment from a previous run.

Comment thread packages/sdk/tests/communication-preflight.test.ts
Comment thread packages/sdk/src/cli/cli-probe.ts
Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946
@khaliqgant

Copy link
Copy Markdown
Member

Addressed Cursor exact-head findings from 06528e6: (1) sync probe spawnSync now uses stdio [ignore,pipe,pipe], preserving the previous non-blocking stdin contract; (2) checkAuthoredFlow accepts both the positional ProjectConfig API and the legacy {projectConfig, probeCache, communicationChecked} options shape. Local SDK typecheck/typecheck:tests and communication/authored-preflight/cli-probe/preflight-cache tests pass. Pushed 8ae646aa3a5c7f625331478a9eb6bbf35aa12b0d; awaiting fresh exact-head CI.

@cursor cursor 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.

Stale Bugbot comment from a previous run.

Comment thread packages/sdk/src/cli/check.ts
Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3656b13. Configure here.

Comment thread packages/sdk/src/cli/check.ts
Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946

@khaliqgant khaliqgant left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Independent exact-head review of 2dab6ae: focused SDK typecheck/typecheck:tests passed; communication-preflight (13), authored-preflight (3), cli-probe (6), preflight-run-cache (1) passed; required validate 36058480705, packed-consumer 36058480717, guard 36058478311, linux-x64-artifact 36058480822 all passed. Async probes preserve stdin isolation and legacy/options-bag compatibility. No substantive blocker found.

@khaliqgant
khaliqgant merged commit 9b5dcae into main Sep 24, 2026
8 checks passed
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.

sdk: concurrent f.llm calls beyond ~5 are dispatched after their 30s lease has expired

1 participant