fix(local): preserve journal connections through downstream execution

[kjgbot]

A local agent followed by more than 30 seconds of deterministic work fails with journal client: closed by caller. step.complete drives downstream steps before replying, but used the 30-second request timeout; the launcher also queued status reads on that same connection and hid the original worker error when closing it.

Give the worker its own connection, apply run-lifecycle timeout semantics to step.complete, drain its acknowledgement before publishing success, and retain the worker error on failure. The fix is independent of #249; the same failure was reproduced before and after integrating that PR temporarily for testing.

Validation and full captured commands/output are in the evidence report. The real-daemon agent → 35-second verification → report reproducer exits 0 and exports a successful journal. Reverting either fix reproduces the corresponding timeout; restoring the exact bytes passes. Final standalone checks: SDK build and test typecheck, 18 SDK tests, and six launcher tests. Protocol rejection, disconnect, explicit close, journal pagination, and failed-command behavior are covered.

The exact checked-in workflows/drive-local.yaml still refuses at selection because its F8b change is already applied; its exit-1 transcript is included. This PR does not claim a completed live Claude drive tick. No kernel changes, gate changes, or #249 implementation changes are included.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 551b79bb-6ad9-4ddc-b4fa-f084685e50c8

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Review swarm: maintainability

No fresh transcript was produced for run a699f83f-2b73-469b-ae61-c3eb1ae8d7d1 (MISSING).

[github-actions]

Review swarm: history

No fresh transcript was produced for run a699f83f-2b73-469b-ae61-c3eb1ae8d7d1 (MISSING).

[github-actions]

Review swarm: structure

No fresh transcript was produced for run a699f83f-2b73-469b-ae61-c3eb1ae8d7d1 (MISSING).

[github-actions]

Review swarm: FAILED

• maintainability: MISSING
• history: MISSING
• structure: MISSING

Cloud run: a699f83f-2b73-469b-ae61-c3eb1ae8d7d1

[kjgbot]

maintainability lens — PASS

Now I've read the RFC-context prelude and the two changed source files. Let me finalize the review.

---

Maintainability review — PR #250 (journal-close-0909)

Blockers — none. The behavior change is small and the tests pin it.

Concerns

1. scripts/run-local-workflow.mjs:93 and scripts/run-local-workflow.mjs:100 — worker?.close() is now called both in the try (to drain the completion ack) and in finally. This bakes in an implicit contract that AgentWorker.close() is idempotent, but nothing in the diff or a comment states it. A future edit to AgentWorker that made close() throw on second call would silently break this launcher. Add a one-line comment on the second call, or guard with a closed flag.

2. packages/sdk/src/journal-client.ts:344-370 — stepComplete drops its bounded timeout by passing null to request(). The comment ("drives downstream steps before replying, so their execution bounds apply") is good but vague — "execution bounds" isn't a term defined elsewhere in this file. A stranger will read this as "trust me it terminates." Naming the actual bounds (step timeout_ms, socket disconnect, caller close()) would let a future reader judge whether a new caller of stepComplete is safe to use unbounded. The new tests do pin the three failure paths, which mitigates.

3. scripts/run-local-workflow.mjs:79 — worker.on('error', error => { workerError ??= error; client.close(); }) silently swaps assignment semantics from "keep last" to "keep first." Correct for this bug (client-close cascading errors shouldn't overwrite the true cause) but subtle. A one-line reason on the ??= would carry that intent forward.

4. scripts/run-local-workflow.mjs:113 — new catch rethrows workerError ?? error. This relies on the worker's 'error' handler running synchronously before the pending-request rejection surfaces in the try chain. That's true today because failAll calls reject synchronously, but it's an implicit ordering assumption worth a comment on catch.

5. docs/evidence/journal-close-0909/*.txt (all files) — every transcript contains /Users/khaliqgant/fl-jc/… hard paths and references a daemon at /Users/khaliqgant/.relayflows-toolchain/…. The evidence doc acknowledges this ("These archives contain this worktree's absolute paths"), but it means the evidence is inspectable only, not reproducible. Fine as a record of what happened; keep them in mind if anyone tries to re-run them in six months.

6. scripts/run-local-workflow.test.mjs:97 — the 32-second real sleep is the only way to exceed the 30 s protocol timeout, but that's a real 55 s test budget added to --test. Worth a one-line comment on the sleep constant that ties it to the protocol timeout, so a future reader who bumps the SDK timeout to 60 s knows to bump this too.

Notes

• Test names are long-sentence-form, matching the existing style in scripts/run-local-workflow.test.mjs. Good.
• journal-client-completion.test.ts:26-56 it.each branch cleverly reuses one server for three failure modes; readable, but the if/else if/else on expected and the branching send behavior in the server handler are close to the ceiling of what this pattern should hold — a fourth mode should split into named it() cases.

REVIEW_PASSED

[kjgbot]

history lens — PASS

Blockers: none. PR #250 at 4361ef6 passes the HISTORY lens.

Notes:

• The timeout change follows an established correction. DRIVE-LOG’s WP-11 entry records removing bounded request timers from run.start and run.resume because they execute workflow work before replying. packages/sdk/src/journal-client.ts:344–371 applies that same distinction to step.complete. It does not raise the kernel lease constant or remove heartbeat behavior—the separate mistake diagnosed on September 9.
• scripts/run-local-workflow.mjs:74–93,109–119 separates worker traffic from polling, awaits completion acknowledgement before reporting, and preserves the original worker error. These changes address the recorded journal-close symptom without adding a new contradiction with settled decisions regressions: red/green flows for the 2026-08-27 platform bugs (dormant until gates 2+6) #5, flow/drive f59e279 08271341 #7, or drive: WP-11: repair PR #9 under review before anything else #11: execution still crosses the journal protocol, and completion remains kernel-owned.
• Both commit subjects match their diffs: 908e565 changes connection handling and adds regression evidence; 4361ef6 adds standalone verification output. Neither commit body makes additional claims. docs/journal-close-0909-evidence.md:47–64,91–106 distinguishes temporary fix(sdk): renew agent leases while CLI steps execute #249 integration testing from standalone delivery and explicitly disclaims a completed live Claude drive tick. That avoids the history’s repeated mistake of presenting fixture success as full workload acceptance.

Concerns, non-blocking:

• Removing the request timer leaves a live connection waiting indefinitely if downstream execution never returns. docs/journal-close-0909-evidence.md:102–106 explicitly documents this limitation; it is not represented as solved shutdown or cancellation behavior.
• Historical reproducer inputs contain machine-specific paths, disclosed at docs/journal-close-0909-evidence.md:43–46. Portable regression equivalents appear in scripts/run-local-workflow.test.mjs:84–138.

This is a history and claims review; I did not independently rerun the test or mutation commands. The checked-in transcripts are reviewed evidence, not fresh execution results.

REVIEW_PASSED

[kjgbot]

structure lens — MISSING

[kjgbot]

🎯 review-swarm: FAILED (M:pass H:pass S:missing)

Lens transcripts posted as sibling comments above.