Harden agent transport recovery and diagnostics

README.md

@@ -13,6 +13,10 @@ export default flow('hello', async (f) => {
   console.log(greeting.trim());
   const answer = await f.agent('greeter', {
     task: 'Reply with one short hello sentence. Do not use tools or modify files.',
+    // Semantic failures do not retry; one classified transport loss may.
+    maxIterations: 1,
+    transportRetries: 1,
+    recoveryMode: 'reset',
   });
   console.log(answer.summary);
   f.done('success');

docs/SURFACE.md

@@ -358,6 +358,37 @@ flow-wide `FlowHeader.workspace` / `tools.fs` scopes. The chief harness above
 remains an aspirational example; this option does not make that entire harness
 executable today.
 
+### Agent retry and recovery controls
+
+`f.agent` separates semantic iteration from infrastructure recovery:
+
+```ts
+await f.agent("reviewer", {
+  task: "Review the current change and write the verdict file.",
+  maxIterations: 2,       // verification may reject one semantic result
+  transportRetries: 1,    // one additional classified infrastructure attempt
+  recoveryMode: "inspect", // reset | inspect | manual
+});
+```
+
+The compatibility defaults are `maxIterations: 1`, `transportRetries: 1`, and
+`recoveryMode: "reset"`. The bounded one-retry default preserves the existing
+single-crash resume contract while removing the old unbounded infrastructure
+loop; set `transportRetries: 0` to disable transport recovery. The SDK omits an
+undeclared budget so legacy canonical specs and hashes stay unchanged, while
+an explicit zero is retained.
+
+Transport retry is deliberately narrow. A signal close, a close without a
+status, a bounded set of transient spawn errors, or the exact historical Codex
+stdin-lifecycle failure is reported as `crashed` and may consume the transport
+budget. An ordinary nonzero CLI exit remains `worker_error` and is terminal;
+`maxIterations` never turns every nonzero exit into a retry. Every replacement
+attempt reuses the step idempotency key. `reset` restores declared start pins,
+`inspect` carries the dirty pins plus the prior transport trajectory, and
+`manual` parks rather than redispatching. Journaled direct-transport evidence
+includes bounded/redacted phase, cause, exit code, signal, OS error code, and
+stderr tail.
+
 ### Supported TypeScript LLM calls
 
 The local authored executor supports these signatures:
@@ -780,10 +811,12 @@ kernel dispatch, including for authored `f.agent` calls.
 A subscriber sends `HELLO view\n`, `HELLO drive\n`, or
 `HELLO passthrough\n`, then receives live stdout/stderr bytes. View and
 passthrough are passive. Only drive forwards subsequent bytes to child stdin.
-In this pipe-based slice, a drive greeting must arrive within 100ms of child
-startup. Without one, the worker closes stdin so unattended and passive-view
-agents receive EOF. Later drive greetings are rejected without marking human
-intervention; a closed stdin pipe cannot be reopened. Supporting drive attachment
+In this pipe-based slice, a drive greeting must arrive during a 100ms bounded
+enrollment window **before** child startup. Only then is the child spawned with
+a writable stdin pipe. Without one, the worker spawns with stdin ignored, so
+unattended Codex never enters its "additional input from stdin" lifecycle and
+passive viewers cannot change the input contract. Later drive greetings are
+rejected without marking human intervention. Supporting drive attachment
 at arbitrary times requires a future terminal/session transport. Drive readers
 pause while child stdin writes flush, preserving input under backpressure.
 There is no backlog, terminal resize, or framing after the greeting. Socket
@@ -868,8 +901,9 @@ journal to be excerpted.
 Each clause is present only when the journal holds the fact behind it; nothing
 is defaulted. The same fields appear as named keys on the `--json` diagnostic
 (`stepId`, `stepType`, `completionReason`, `attempt`, `maxIterations`,
-`exitCode`, `stdoutTail`, `stderrTail`, `detail`, `transcriptPath`, `hint`,
-`journalPath`), so the rendered line and the machine-readable record carry the
+`transportRetries`, `exitCode`, `transportPhase`, `transportCause`, `signal`,
+`errorCode`, `retryableTransport`, `stdoutTail`, `stderrTail`, `detail`,
+`transcriptPath`, `hint`, `journalPath`), so the rendered line and the machine-readable record carry the
 same facts rather than the message being the only copy.
 
 `attempt=<n>/<budget>` is read from the journal, not from the spec: `n` is the

kernel/DESIGN.md

@@ -58,6 +58,7 @@ One per attempt. Payload:
 | `pins.workspace` | agent steps: `[{surface, revision_id}]` — relayfile revision id per declared mount surface, or `{worktree_base_commit}` |
 | `pins.streams` | `[{stream, read_offset}]` — consumer offsets at attempt start |
 | `max_iterations` | from spec, echoed for legibility |
+| `max_transport_retries` | from spec, echoed for legibility like `max_iterations`; additional attempts allowed after classified infrastructure loss (`crashed` / `lease_expired`). Always present — an explicit `0` is the value that explains why a lost process was not retried, so the journal never omits it (the spec's canonical form omits its default of one; the journal does not) |
 
 Deterministic/llm steps journal `pins.streams` only if they consume streams;
 `pins.workspace` is empty (no workspace).
@@ -74,7 +75,7 @@ Payload:
 | `verification` | `{gate, verdict: pass\|fail, detail}` or null |
 | `end_pins` | agent steps: `{workspace: [{surface, revision_id}], streams: [{stream, read_offset}]}` — Appendix A rule 6: the next step's starting state **is** this |
 | `effects` | list of `{surface_path, idempotency_key}` dedupe keys recorded this attempt |
-| `trajectory_tail` | agent failure only: worker-supplied tail injected into an `inspect` retry; `step.complete` rejects one over 16 KiB of canonical JSON |
+| `trajectory_tail` | agent failure only: worker-supplied tail injected into an `inspect` retry; direct workers add bounded/redacted `transport{phase,cause,exit_code,signal,error_code?,retryable,stderr_tail}` evidence; `step.complete` rejects one over 16 KiB of canonical JSON |
 | `budget` | `{tokens_in, tokens_out, dollars, dollars_unmetered?}` — exact; zero for memoized replay by construction (no entry is written on replay). `dollars_unmetered: true` (omitted when false) marks tokens of unknown dollar cost: `dollars` is then metered cost only, dollar ceilings ignore the unknown part, token ceilings count it |
 | `completed_by` | `kernel` \| worker id — out-of-band completion uses the same entry, same discipline |
 | `next_attempt_at_ms` | when `disposition=retry`: computed backoff+jitter wake time |
@@ -290,9 +291,27 @@ attempt's `budget` field.
 under the attempt's idempotency key; every writeback is a journaled
 `effect.recorded` deduped by `(step_id, idempotency_key, surface_path)`;
 `step.completed` pins end state, which defines the next step's start. Dead
-attempt ⇒ recovery mode: `reset` restores pinned revisions and retries;
-`inspect` retries inside the dirty workspace with the failed attempt's tail
-injected; `manual` parks as `wait.human` with `diff_ref`.
+attempt ⇒ recovery mode: `reset` restores pinned revisions before a permitted
+retry; `inspect` resumes inside the dirty workspace with the failed attempt's
+tail injected; `manual` parks as `wait.human` with `diff_ref`. Infrastructure retry
+is a separate, explicit budget (`retry.max_transport_retries`, default one so a
+single process loss remains resumable; set zero to disable).
+Only `crashed` and `lease_expired` consume it. `worker_error`, timeout, budget,
+cancellation, and an ordinary nonzero CLI exit are terminal regardless of the
+budget; semantic verification retry remains bounded only by `max_iterations`.
+
+The kernel learns of a dead attempt two ways, and the recovery mode applies to
+both: the kernel notices an abandoned lease (`abandonment_actions`), or the
+worker reports its own loss through `step.complete` with `crashed` /
+`lease_expired` (`completion_actions`). Under `manual` either path journals
+`step.completed` with `disposition: park` and then the `wait.human` a human
+answers, each its own append. A process death between the two leaves the step
+folded to the placeholder wait id `park-<step>-<attempt>` with nothing to
+answer; resume recognises that placeholder and journals the missing
+`wait.human` from the same journaled facts (last completion reason, start
+pins), once. The SDK decides *which* transport losses are `crashed`
+(`worker-cli.ts` classifies, `cli-transport-evidence.ts` maps to the reason);
+the kernel decides what a `crashed` completion *does* (budget, recovery mode).
 
 ### Memoized resume
 

kernel/evidence/501/README.md

@@ -0,0 +1,93 @@
+# PR #501 review follow-up — `manual` recovery for worker-reported transport loss
+
+Review-swarm history lens and Cursor Bugbot both found the same defect in
+`f3bd47fe`: `completion_actions` retried every budget-eligible `crashed` /
+`lease_expired` completion without reading the agent step's `recovery_mode`.
+Only the kernel-noticed death (`abandonment_actions`) honoured `manual`, so
+the same dead attempt parked or redispatched depending on who noticed it
+first — contradicting RFC-0001 Appendix A rule 4.
+
+## What changed
+
+1. **`completion_actions` parks a `manual` agent step on a worker-reported
+   `crashed` / `lease_expired`** (`Disposition::Park` + `wait.human`), for any
+   transport budget including zero. The `wait.human` construction is one
+   function, `recovery::manual_park_wait`, shared with `abandonment_actions`
+   so the two producers cannot drift. `completion_actions` gained a
+   `start_pins` parameter so the `diff_ref` is anchored on the journaled
+   start pin, never the worker's `end_pins` claim.
+2. **Torn parks are repaired on resume.** A park is two appends, each its own
+   transaction (raised by the independent reviewer, confirmed by external
+   probe). A step folded to the placeholder `park-<step>-<attempt>`
+   (`state::park_placeholder_wait_id`) with no `wait.human` after it now has
+   the wait journaled by `recovery_actions_filtered` — once, from the same
+   journaled facts. This closes the same latent gap on the pre-existing
+   abandonment path.
+3. **`all_backing_off_steps_return_timers`** regained a retryable failure
+   precondition (`Crashed`; `worker_error` is terminal since the budget
+   split) so it exercises failure backoff again.
+4. **`step.attempt.started.max_transport_retries` is always journaled**
+   (`entry.rs` dropped `skip_serializing_if = is_zero_u32`). `kernel/DESIGN.md`
+   said "omitted at default (1)"; the code omitted zero — the one value that
+   explains why a lost process was not retried. Now it matches its sibling
+   `max_iterations`: always present. DESIGN.md updated, plus a paragraph tying
+   the SDK classifier, the completion-reason alphabet and the kernel
+   disposition together.
+
+## Evidence (literal commands + full output)
+
+| file | what |
+|---|---|
+| `mutation-transcript.txt` | one literal transcript, every command echoed: pre-mutation `sha256sum` of `machine.rs` + `recovery.rs`; mutation A+B applied (`manual_park = false && …`, repair guard `false && …`, shown as `git diff -U0`); RED — 5 regression tests fail with the original symptom (`disposition: retry`, second dispatch); restore via `cp` and `sha256sum -c` → `OK` for both files; mutation B alone (repair off); RED — the two torn-park tests fail (0 `wait.human` where 1 expected); restore + `sha256sum -c` → `OK`; GREEN — same commands pass |
+| `green-kernel.txt` | `cargo test --workspace`, exit 0 |
+| `clippy.txt` | `cargo clippy --workspace --all-targets -- -D warnings`: exits 101 on pre-existing findings only (`schema.rs:125`, `spec.rs:77`, `memoization.rs:102` as in the PR body, plus pre-existing test-target findings); `clippy-all-targets-warn.txt` lists every warning location — none on lines this change added |
+| `sdk-typecheck-build.txt` | surface built + packed + installed `--no-save` into sdk (documented flow), then `npm run typecheck && npm run typecheck:tests && npm run build`, exit 0 |
+| `green-sdk.txt` | `RELAYFLOWD_BIN=<built relayflowd> npx vitest run`: 154 files / 2410 tests pass; 2 environmental failures explained below |
+| `green-sdk-bundle-pristine.txt` | `tests/bundle.test.ts` re-run from a pristine `npm ci`: 23/23 pass, exit 0 |
+| `green-sdk-authored-node-runtime.txt` | the standalone suite under an isolated `mise install bun@1.4.0` (global config untouched) + Node 22.23.2 via `mise exec`, `FLOWS_BUILD_BUN` / `FLOWS_AUTHORED_NODE` absolute: 14/14 pass, exit 0 |
+| `codex-live-probe.txt` | one bounded live run of the installed `codex-cli 0.154.0` through the direct unattended transport (`flows check` + `flows run --local-agent`, output-only instruction, disposable cwd verified unchanged): `success`, exit 0, verified output, journal facts incl. the transport evidence. First attempt refused by the kernel on `cwd` — pre-existing preflight/run mismatch, see below |
+
+
+Pre-existing defect observed while probing (not fixed here, out of scope):
+`flows check` accepts a step-level `cwd:` (compiled into the kernel spec since
+#358) but `relayflowd` rejects the spec at `run.start` with
+`invalid_spec: unknown field "cwd"` — a Covenant 2 preflight/run mismatch.
+
+The two failures in `green-sdk.txt` are environmental, not from this change:
+
+- `tests/bundle.test.ts` — `REFUSED [bundle_invalid] package-lock.json:
+  node_modules/@agent-relay/cli-surface does not match its pinned version`.
+  The documented `--no-save` surface override resolved `cli-surface` to
+  12.4.0 over the lockfile's 12.2.4; the bundle builder refuses a non-pristine
+  tree by design. Green from a pristine `npm ci` (file above).
+- `tests/authored-node-runtime.test.ts` — `beforeAll` pins `bun --version`
+  to exactly `1.4.0`; this machine has 1.4.2. The PR body excluded this
+  standalone suite for a different toolchain reason (node flag).
+
+Regression tests added:
+
+- `relayflowd-core/src/machine/recovery_tests.rs` (focused module; owner asked
+  for it split out of the general `tests.rs`):
+  `manual_recovery_parks_a_worker_reported_transport_loss_instead_of_redispatching`
+  (crashed + lease_expired), `reset_recovery_still_retries_a_worker_reported_transport_loss`,
+  `recovery_journals_the_wait_human_a_torn_manual_park_never_wrote` (both producers)
+- `relayflowd-core/src/machine/tests.rs`: `all_backing_off_steps_return_timers`
+  precondition restored (stays in the general file)
+- `relayflowd-core/src/entry.rs`: `max_transport_retries_is_always_journaled`,
+  `a_pre_field_attempt_started_still_reads`
+- `relayflowd/tests/manual_recovery.rs` (in-process engine, mock worker):
+  park survives reopen + resume with no redispatch and answers to a human on
+  the pinned revision (crashed / lease_expired / budget 0); crash injected
+  between the park's two appends is repaired on resume, idempotently
+- `relayflowd/tests/crash_resume/manual_recovery.rs` (real `relayflowd serve`
+  binary over the protocol socket): same three cases, silence probe for
+  `step.dispatch`, daemon SIGKILL + restart + `resume`, journal unchanged,
+  `event.emit` answer redispatches attempt 2 on `rev-0` with the same
+  idempotency key
+
+`rustfmt --check` drift is unchanged from the PR head (20 files, none touched
+by this change beyond formatting the lines it added).
+
+Captured logs are verbatim except that trailing whitespace on captured lines
+was stripped (`sed 's/[ \t]*$//'`) so `git diff --check` passes; no other
+byte was edited.