Skip to content

fix: follow lost worker leases and retried attempts instead of abandoning the run (customer issue 1) - #593

Merged
khaliqgant merged 5 commits into
mainfrom
fix/worker-lease-retry-accounting
Oct 1, 2026
Merged

khaliqgant merged 5 commits into
mainfrom
fix/worker-lease-retry-accounting

Conversation

@miyaontherelay

@miyaontherelay miyaontherelay commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Customer rw_3a0fcb71, issue 1. Rebased directly onto main at c011e623 (v2.0.37); the published head is 00a21473.

What the customer saw

An f.agent attempt lost its worker lease. The kernel journaled lease_expired and retried it, and the CLI exited with worker wait canceled / protocol_error and no run id, while attempt 2 went on to succeed. Its output was discarded, and the failure report said attempt=2/1.

Root causes

  1. Lease timing depended on the two wall clocks agreeing. relayflowd issues lease_deadline_ms on its own clock, but withWorkerLease compared it with the worker's Date.now() and renewed at remaining/3. If the worker ran 45s ahead, every dispatch was refused as already_expired. If it ran 75s behind, the first renewal landed after the daemon had swept the lease. A paused process (laptop sleep or VM pause) loses the lease regardless.
  2. A lost root lease was treated as a failed flow. The body's wait rejected with a generic worker wait canceled. driveRoot then terminalized the root with worker_error on every retry (all 8 attempts burned), so the run became unrecoverable.
  3. The re-admitted child was misclassified. run.start on an existing admission key returns the child running (mid-retry) or in backoff. classifyOutcome only followed parked, so it reported "status running without a classifiable completion".
  4. Attempt accounting mixed two counts. A dead-worker attempt (crashed/lease_expired) is not charged against max_iterations (recovery.rs), so the raw attempt number could exceed the budget: attempt=2/1.

Fix

  • kernel: lease_ttl_ms on step.dispatch and step.heartbeat (additive). The SDK times the lease on its monotonic clock from it; older daemons keep the old comparison.
  • The root driver follows the kernel's re-dispatch after a lost root lease and re-drives the body; completed steps replay from receipts and the in-flight child is re-adopted. It never terminalizes a lease loss. If no retry arrives within 45s, or after 3 re-drives, it reports root_lease_lost with status running, the root run id and the exact flows resume command.
  • classifyOutcome follows running outcomes and backoff steps. A worker wait that looks expired only by the local clock keeps following the step; it fails only when the deadline stops moving for a whole lease.
  • Cancellation errors carry the abort reason. withWorkerLease surfaces the lease loss as the cause.
  • unchargedAttempts makes the report say attempt=2 (iteration 1/1; 1 earlier attempt lost to a dead worker, not charged).

Evidence: failing on the base, passing with this PR, against a real relayflowd and the real flows run --local-agent CLI

Harness: an authored flow with one f.agent whose fake claude takes 40s (longer than one 30s lease), run through the built CLI against a real daemon.

scenario base (#590 head 52137878) v2.0.35 release this PR
CLI clock 75s behind daemon exit 1 protocol_error: lease_conflict, no run id; agent-1 attempts 1–3 lease_expired/crashed n/a exit 0 success, agent-1 1 attempt
CLI clock 45s ahead exit 1 Agent lease is already expired; root burned 8× worker_error → run durably failed n/a exit 0 success
CLI SIGSTOPped 45s mid-agent (the customer's shape) exit 1 protocol_error, no run id, while agent-1 attempt 2 = success same exit 0 success: root lease_expired→retry, 2 success; agent-1 lease_expired→retry, 2 success (attempt 2's output used)

Journal of the passing pause run:

root    authored-root|1|lease_expired|retry | authored-root|2|success|step_done
child   agent-1|1|lease_expired|retry | agent-1|2|success|step_done

Resuming from the journal after the CLI exits (pause scenario, CLI already exited):

  • v2.0.35 flows resume --local-agent <root>: fails. It reads agent-1's first completion (journal step "agent-1" completed with lease_expired), then burns the root to worker_error ×8, so the run is durably failed and no longer recoverable.
  • fix(sdk): read an authored step's terminal completion, not a retried attempt #590 or this PR, flows resume --local-agent <root>: exit 0 success. It reuses agent-1's attempt-2 result without re-running it (the root gets a 3rd attempt).
  • A run stranded by a v2.0.35 CLI is rescued by resuming with this PR's CLI, as long as nobody has already run a v2.0.35 resume on it.

Tests

  • tests/worker-lease-drift-live.test.ts (real daemon): skew ±, and attempt=2/1 accounting. Fails on the base.
  • classify-outcome.test.ts: backoff longer than the 2s bound and a running re-admitted child. The backoff test fails on the base.
  • worker-lease-sweep.test.ts: skewed CLI follows a renewing lease (fails on the base); a never-swept lease still fails at 35s.
  • cli.test.ts: the old "bounds a worker wait" test asserted exit 1 over a mock daemon that then completed success. That is this bug, so it now asserts the step is followed to success.
  • kernel: dispatch_carries_the_lease_as_a_duration; cargo test -p relayflowd all green.
  • Pre-existing and unrelated: authored-root > completes the admitted root flakes on its 30ms fake lease (base 0/5, this branch 3/5 on this machine). About 100 PTY/subprocess/hosted-isolation tests fail identically on the base in this sandbox.

🤖 Generated with Claude Code


Note

Medium Risk
Changes lease timing, heartbeat validation, and run-outcome classification across kernel protocol and CLI—security-critical execution path, but behavior is narrowly scoped to skew/retry scenarios with broad test coverage.

Overview
Fixes customer rw_3a0fcb71: worker lease loss and kernel retries no longer cause the CLI to exit with protocol_error / worker wait canceled while the run continues successfully in the journal.

Kernel (additive): step.dispatch and step.heartbeat now include lease_ttl_ms (remaining duration on the daemon clock). Heartbeats at or after the daemon-side deadline are rejected so a late renewal cannot revive an expired assignment.

Workers: withWorkerLease times leases on performance.now() using lease_ttl_ms when present, instead of comparing the daemon’s lease_deadline_ms to the worker’s wall clock. Lease-loss errors win over generic wait-cancel errors from aborted bodies.

Authored roots: After a lost root lease, the driver waits for kernel re-dispatch (up to 3 redrives / 45s), replays completed steps, and does not terminalize the root on lease loss. If no retry arrives, it reports root_lease_lost with status: running and a flows resume hint—not protocol_error.

CLI classification: classifyOutcome now follows running outcomes and steps in backoff, and worker waits keep polling when the local clock says “expired” but the daemon is still renewing or sweeping. Failure reports add unchargedAttempts so attempt=2/1 is explained when the first attempt died uncharged.

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


Agent Relay sessions

  • claude session b714e785 · contributor · ran gh pr commands · last active 2026-09-30

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: aa72aa38-96a5-452a-b098-223e8fa70fc4

📥 Commits

Reviewing files that changed from the base of the PR and between ac87f25 and 25f4764.

📒 Files selected for processing (12)
  • kernel/relayflowd/src/server.rs
  • kernel/relayflowd/src/server/session/assignments.rs
  • kernel/relayflowd/src/server/tests.rs
  • packages/sdk/src/authored-root.ts
  • packages/sdk/src/cli/run.ts
  • packages/sdk/src/protocol.ts
  • packages/sdk/src/worker-lease.ts
  • packages/sdk/tests/authored-root.test.ts
  • packages/sdk/tests/classify-outcome.test.ts
  • packages/sdk/tests/worker-lease-lost.test.ts
  • packages/sdk/tests/worker-lease-sweep.test.ts
  • packages/sdk/tests/worker-lease.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/sdk/src/protocol.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The daemon now sends lease durations with dispatches and heartbeat responses. The SDK uses monotonic time to track worker leases, follows running and backoff states, retries authored roots after lease loss, and includes uncharged attempts in step failure details.

Changes

Lease handling and recovery

Layer / File(s) Summary
Lease duration protocol
kernel/DESIGN.md, kernel/relayflowd/src/server.rs, kernel/relayflowd/src/server/session/assignments.rs, kernel/relayflowd/src/server/tests.rs, packages/sdk/src/protocol.ts
Dispatch and heartbeat data include lease_ttl_ms alongside the absolute deadline. Heartbeats for expired assignments are rejected. Tests check dispatch and heartbeat durations and expired-heartbeat rejection.
Monotonic lease tracking and run polling
packages/sdk/src/worker-lease.ts, packages/sdk/src/cli/run.ts, packages/sdk/tests/classify-outcome.test.ts, packages/sdk/tests/cli.test.ts, packages/sdk/tests/worker-lease*.test.ts, packages/sdk/tests/worker-lease-drift-live.test.ts
Worker lease checks use monotonic time and convert daemon deadlines to local deadlines. CLI polling continues through running and backoff states and waits for the daemon to settle expired leases. Tests cover clock skew, delayed responses, and polling behavior.
Authored-root retry and resume reporting
packages/sdk/src/authored-flow-error.ts, packages/sdk/src/authored-root.ts, packages/sdk/src/cli/direct-run.ts, packages/sdk/src/cli/run.ts, packages/sdk/tests/authored-root.test.ts
Authored-root execution waits for redispatch and retries after lease loss. Exhausted retries or a timeout produce a resumable root_lease_lost error, which the CLI reports with the root run ID when available.
Uncharged attempt accounting
packages/sdk/src/failure-kinds.ts, packages/sdk/src/authored-node-runner.ts, packages/sdk/src/authored-worker-step.ts, packages/sdk/src/cli/step-failure.ts, packages/sdk/tests/worker-lease-drift-live.test.ts
Failure details carry counts of kernel-reported crashed or expired attempts. Rendered evidence distinguishes the attempt number from the charged iteration.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant AuthoredRoot
  participant Kernel
  AuthoredRoot->>Kernel: execute authored root
  Kernel-->>AuthoredRoot: report lease loss
  AuthoredRoot->>Kernel: wait for same-run redispatch
  Kernel-->>AuthoredRoot: dispatch root again
  AuthoredRoot->>Kernel: re-drive root
  AuthoredRoot-->>Caller: return result or root_lease_lost
Loading

Suggested reviewers: khaliqgant, kjgbot

Merge Risk: ⚪ Minimal · up to 25f47

No concrete merge-blocking issue is identified. The lease recovery and attempt-reporting changes appear consistent with their stated contracts; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 25f47

The change improves recovery without an observed expansion of access or authority. Existing ownership controls remain in place, but repeated interruptions and mixed-version recovery are not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The directly affected authority is worker ownership and completion of assigned attempts within daemon-managed runs. Placement retains worker pin and surface checks. The examined evidence does not establish broader tenant, credential, or deployment isolation.

Trust Boundaries and Controls

  • observed — Worker-supplied run, step, attempt, and lease identifiers cross into daemon enforcement with a server-owned connection identity. Completion is serialized under the run lock, requires assignment ownership, rejects canceled or completed runs, and must match the active attempt and idempotency key. These completion controls predate the PR.

Resilience and Maintainability Implications

  • observed — Kernel abandonment acts only on the matching running attempt and preserves manual-recovery parking. RootJournal redispatch entrypoints are test fixtures, not additional production exposure. Live-worker test source exercises real-daemon clock skew, but the focused root fixtures do not establish concurrent or repeated-loss recovery end to end.

Hardening Proposals

  • proposed — Add real-daemon recovery scenarios combining repeated root lease loss, delayed stale completions, cancellation, retry exhaustion, and persistence failures. Assert that ownership cannot be revived, only the active attempt can commit, cleanup converges, and reported state agrees with the durable run.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.91% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 20 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: recovering from lost worker leases and following retries instead of abandoning the run.
Description check ✅ Passed The description directly explains the customer issue, root causes, implementation changes, evidence, and tests for lease recovery and retry handling.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

A rabbit checks the ticking lease,
Then hops by monotonic time.
The daemon sends a fresh duration,
A root returns to try again.
Lost steps are counted, not concealed,
And resume paths keep moving.

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

@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/authored-root.ts
miyaontherelay added a commit that referenced this pull request Sep 30, 2026
Bugbot on #593: after a lost root lease the driver waited up to 45s for the
kernel's re-dispatch without observing the lifecycle signal, so a Ctrl-C in
that window blocked until the timer fired. The wait now rejects at once on
abort, and the driver rethrows it as the caller's cancel rather than as
root_lease_lost.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@khaliqgant
khaliqgant changed the base branch from fix/authored-step-terminal-completion to main September 30, 2026 13:37
miyaontherelay and others added 2 commits October 1, 2026 00:23
…ning the run

Customer rw_3a0fcb71 issue 1: an f.agent attempt lost its lease, the kernel
retried it, and the CLI exited `worker wait canceled` / protocol_error with no
run id while attempt 2 went on to succeed -- its output was discarded, and the
failure reported `attempt=2/1`.

Lease timing (kernel + SDK):
- relayflowd sends `lease_ttl_ms` on step.dispatch and step.heartbeat (additive).
- withWorkerLease times the lease on the worker's monotonic clock from that
  duration instead of comparing the daemon's absolute deadline with its own
  wall clock. A worker 45s ahead of the daemon refused every dispatch as
  already expired; 75s behind, it renewed after the daemon swept the lease.
  Old daemons keep the old comparison.

Following retries:
- A lost authored ROOT lease is not a failed flow: the root driver waits for
  the kernel's re-dispatch and re-drives the body (completed steps replay from
  receipts; the in-flight child is re-adopted). It no longer terminalizes the
  root as worker_error, which burned every root attempt and made the run
  unrecoverable. If no retry arrives it reports `root_lease_lost` (status
  running, root run id, exact `flows resume` command).
- classifyOutcome follows a child returned `running` (re-admitted mid-attempt)
  and a step in retry `backoff`, instead of "status running without a
  classifiable completion" after 2s.
- A worker wait whose lease looks expired only by this process's clock keeps
  following the step; it fails only when the deadline stops moving for a
  whole lease (never renewed, never swept).
- Cancellation errors carry the abort reason ("worker wait canceled: Agent
  lease expired before renewal ...") and withWorkerLease reports a lost lease
  as the cause, not the body's generic wait error.

Attempt accounting:
- stepFailureDetails counts attempts the kernel declared dead
  (crashed/lease_expired, completed_by kernel) as `unchargedAttempts`, and
  the report prints `attempt=2 (iteration 1/1; 1 earlier attempt lost to a
  dead worker, not charged)` instead of `attempt=2/1`.

Builds on #590 (terminal-completion read).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bugbot on #593: after a lost root lease the driver waited up to 45s for the
kernel's re-dispatch without observing the lifecycle signal, so a Ctrl-C in
that window blocked until the timer fired. The wait now rejects at once on
abort, and the driver rethrows it as the caller's cancel rather than as
root_lease_lost.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@khaliqgant
khaliqgant force-pushed the fix/worker-lease-retry-accounting branch from 5acbdbc to ac87f25 Compare October 1, 2026 02:08

@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/authored-root.ts
Comment thread packages/sdk/src/cli/run.ts

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 20 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/sdk/src/worker-lease.ts Outdated
Comment thread kernel/relayflowd/src/server/session/assignments.rs
Comment thread packages/sdk/src/authored-root.ts
Comment thread packages/sdk/src/protocol.ts Outdated
Comment thread kernel/relayflowd/src/server.rs Outdated
Comment thread packages/sdk/src/cli/run.ts Outdated
Comment thread packages/sdk/src/cli/run.ts
Comment thread kernel/relayflowd/src/server/tests.rs Outdated
Comment thread packages/sdk/tests/worker-lease.test.ts Outdated

@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 high 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 25f4764. Configure here.

Comment thread packages/sdk/src/cli/run.ts Outdated
@khaliqgant

Copy link
Copy Markdown
Member

Human / Relayflow Lead merge gate is ready for exact head 00a214735dd56fd590564aa898a987fdafa806d6 on exact base c011e6230a2b3929d27ae1b0af3c5e49163700d5.

  • GitHub reports CLEAN + MERGEABLE; all required checks are terminal green.
  • Zero unresolved review threads; every valid Cursor/Cubic finding has a pushed regression and thread reply.
  • Fresh independent exact-head review: GO, no substantive findings. Independent verification passed 132 changed SDK tests, worker-lease drift-live 3/3, SDK build/typecheck:tests, and full cargo test -p relayflowd.
  • Local focused lease/classifier matrix: 39/39; SDK build, source/test typechecks, diff and secrets checks pass; real SIGSTOP recovery passes.

Per AGENTS.md, I am not self-merging. A human or the Relayflow Lead should re-check the unchanged head/base and merge. Do not release from this comment: after merge, the coordinated kernel+SDK change implies the next release after v2.0.37 (v2.0.38) plus the Cloud runtime pin.

@khaliqgant
khaliqgant merged commit ae49491 into main Oct 1, 2026
9 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.

2 participants