sdk: concurrent f.llm calls beyond ~5 are dispatched after their 30s lease has expired - #564
agent-relay-code[bot] wants to merge 2 commits into
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
Shepherd decision (2026-09-24): #576 is the selected candidate for the duplicate concurrent-probe fix pending a focused async-probe check. Your head b84c841 is older and currently fails |
|
The stronger async probe behavior from this PR has been ported onto #576 and pushed as d69cd27b9f17a8a4a4f84958f222fdf3a2a5f9bf (guarded from #576 head 909a1d2). #564 remains open while exact-head CI/review on #576 runs; no overlapping push requested. If #576 proves the behavior, this PR will be superseded with attribution. |
|
Superseded by #576 after guarded verification. The technically correct implementation is now on #576 head |
|
Closing as superseded by #576 after exact-head CI and focused behavior verification; no merge performed. |
Fix authored LLM preflight starving worker leases
Implementation commit:
78f7012; evidence commit:b84c841.Concurrent authored calls ran synchronous CLI identification and model-readiness
probes before slot admission. Each later call blocked the same Node event loop
that receives dispatches and renews leases. WorkerSlots already bounded admission
correctly; the worker simply could not handle its dispatch before the deadline.
A cold first probe could also expire the durable root lease.
The reported five-versus-nine threshold is consistent with this mechanism:
the first child is exposed to approximately (N−1) × probe time. A 4–7 second
probe stays below 30 seconds at five calls and exceeds it at nine. The new slow
regression also fails on the unchanged base with
journal step "llm-1" completed with lease_expired(literal command and output).
Changes
single-flight handling of concurrent cold calls. Preserve CLI/source/model/
managed-mode keys and step-specific refusal diagnostics.
generator owns identification, managed-wrapper handling, model readiness,
auth fallback and redaction; synchronous
flows checkdrives the same logic.still refuse before any provider probe. This is broader than exporting only
resolveCliin the reviewed plan: that alone would bypass static refusalsduring cache warming. There is no deferred-probe pass treated as successful.
maximum execution overlap and every run journal. A durable-root test uses two
registered models with 45-second cold probes, so cache alone cannot pass it.
Add probe-driver parity and static-refusal regression tests.
The generator approach avoids introducing a separately packaged worker-thread
entry into the standalone CLI's embedded Node payload. Moving preflight inside
WorkerSlots would only bound child exposure by capacity and would leave the
root lease exposed.
Verification
All literal commands and captured output are in the
evidence ledger.
Final focused run: 177 tests passed.
1 and
4.
All 26 journals scanned.
complete revert/fail/byte-for-byte restore/pass transcripts.
With async probing retained, removing the cache fails probe counts, not leases.
downstream patch
produced the nine-row grid and reached both human gates, but its last resume
failed with
unawaited_step(captured command/output).
Its 29 journals contain no lease_expired completion.
This acceptance item remains incomplete.
Typechecking and the SDK build completed; outputs are in the ledger.
The complete
npm testrun is not green:67 failed, 2810 passed, 26 skipped, 2 errors.
Failures include unavailable bubblewrap, missing Surface/runtime fixtures,
Bun 1.3.6 versus the required 1.4.0, and flow-handle identity errors.
The 15 hosted-isolation failures reproduce on the unchanged base;
no claim is made that every full-suite failure was baseline-verified.
The standalone CLI builds, but its authored-import smoke test refuses on
both changed and base builds.
The provider passes above use the Node CLI.
Tradeoffs and scope
Cached credentials revoked mid-execution are discovered by execution
(
worker_error) rather than the next preflight auth refusal; a new executionprobes again. Failed probe facts are also scoped to that execution.
The first communication-environment check still uses its existing synchronous
10-second local broker probe; subsequent successful checks are reused.
This is not a claim that every SDK subprocess is asynchronous.
No kernel, lease duration, WorkerSlots, fatal-error handling (#560), gate, or
workflow changes. The prompt-lab branch had unrelated history, so verification
used its isolated worktree with this checkout's built CLI; no example branch
was merged or pushed.
Checks
The checks fail on the base commit too, so these failures were not introduced by this change: they come from the repository itself or from the environment the checks ran in. This pull request is a draft until someone looks.
What ran (.relayflow/check.sh)
Output on this branch (last 80 lines)
Output on the base commit (last 80 lines)
What the repair agent found
Repair notes —
.relayflow/check.shon this machineBranch
relayflow/flows-software-garden-860ae350, atb84c841.Three distinct causes were behind the failing run. Two were missing setup and
are now handled in
.relayflow/check.sh(uncommitted, as instructed). One is aproperty of this container that no step in the script can change; it is
recorded here and left failing.
Fixed in check.sh — 1. the script aborted before it reached any SDK test
The previous
.relayflow/check.logends inside "provisioning bubblewrap". Thescript runs under
set -e, andexits 1 here, so the whole check died at that line — the surface gate, the SDK
suite and the schema gate never ran at all. That sysctl is best-effort by
intent (its own comment says the sandbox tests "skip themselves" when the
facility is missing), so its failure now prints a note instead of aborting.
Fixed in check.sh — 2. bun 1.4.0
tests/authored-node-runtime.test.tsasserts the exact bun version inbeforeAll, because the standalone CLI it builds embeds that bun's runtime:All four workflows pin
bun-version: "1.4.0"throughoven-sh/setup-bun@v2;this machine shipped 1.3.6 next to node.
check.shnow installs the pinnedversion under
~/.bunwhen the one on PATH does not match, which is the localequivalent of that CI step. After it, the suite is green:
Fixed in check.sh — 3. extensionless ESM fixtures under a CommonJS ancestor
Seven
tests/live-kernel.test.tscases failed with a null step output:The eight node fixtures in
testdata/preflightare extensionless (they standin for real agent CLIs) and all eight are ESM. Node picks an extensionless
file's module system from the nearest
package.json, walking up past the reporoot. The repo has none, so in CI the walk finds nothing and module-syntax
detection loads them as ESM. This checkout sits under
$HOME, where/home/daytona/package.json— an unrelated project, outside the repo — sets"type": "commonjs". That disables detection, and node 25 then runs thefixture as CommonJS, exiting 0 with no output whatsoever:
This is exactly why the in-place cases failed while the cases that first copy a
fixture into a temp dir passed.
check.shnow writestestdata/preflight/package.json={"type":"module"}for the duration of therun and removes it on exit. The other eleven fixtures in that directory are
/bin/sh, so the declaration cannot mislabel one. After it:A one-line committed
testdata/preflight/package.jsonwould fix this for anycheckout location, not just this one. That is a repo change unrelated to the
issue under work, so it is left for a human to decide rather than folded in
here.
NOT fixed — bubblewrap cannot create a usable sandbox in this container
22 failures remain, all in the three hosted-extension sandbox suites:
Every one reduces to the same line:
Reproduced outside the test suite, with no repo code involved:
The process runs with
CapEff: 0000000000000000and a seccomp filter(
Seccomp: 2), andkernel.apparmor_restrict_unprivileged_usernsis 1 andcannot be written even as root:
CI relaxes that sysctl; this container will not allow it. Three other routes
were tried and all are refused by bubblewrap 0.12.0 as Debian builds it:
(
/usr/bin/bwrapwas restored to its packaged state afterwards: mode-rwxr-xr-x, no file capabilities.) Running it as root does work(
sudo -n bwrap ... --unshare-all /bin/trueexits 0), but the suite runsunprivileged and running vitest as root would rewrite ownership across the
checkout, so that was not done.
These failures are not from this branch. The same test fails identically at the
branch's base commit
f6ece41, in a clean worktree:check.shdeliberately installs bubblewrap when it is absent, becausebabysitter-native-extension.test.tsskips its sandbox case without/usr/bin/bwrapand would silently stop gating. Installing a bubblewrap thatcannot unshare does not restore that gate — it only converts the skip into a
failure — but leaving bubblewrap uninstalled would not have helped either:
hosted-extension-isolation.test.tsandhosted-extension-protocol.test.tscarry no skip guard and fail on a missing
/usr/bin/bwrapjust the same.Where the run stands now
sh .relayflow/check.shwith the two setup fixes above:Before:
5 failed | 179 passed,29 failed | 2856 passed. The 22 that remainare exactly the three bubblewrap suites (13 + 8 + 1).
The kernel suite and the surface-package gate are green. The SDK suite's
non-zero exit ends the script under
set -e, so its last stage never runs;executed by hand it passes:
The working tree is clean; the temporary
testdata/preflight/package.jsonwasremoved by the script's own trap.
Fixes #561
Summary by cubic
Fixes concurrent
f.llmcalls beyond ~5 failing withlease_expiredby making authored CLI preflight probes asynchronous and shared.Bug Fixes
Written for commit b84c841. Summary will update on new commits.