Skip to content

Commit d250d8b

Browse files
authored
fix(auto): the spawn retry window was shorter than the thing it covers (#235)
#214 gave the assessor a bounded spawn retry and #226 extended it to the planner. Both shipped at 3 attempts, 300ms apart -- a 600ms window -- and two more lanes died on the same failure AFTER that shipped: assessor run: claude run: spawn failed: exec: "claude": executable file not found in $PATH planner run: claude run: spawn failed: exec: "claude": executable file not found in $PATH Four occurrences in one day, two of them post-fix. 600ms is less than a package manager takes to relink a binary, so the retry was correct in shape and useless in size. 5 attempts, 2s apart. A machine that genuinely lacks the CLI still fails -- eight seconds later, saying what it said before. Only a spawn that never happened is retried; a refused reply, a non-zero exit, a timeout and a cancelled context all still stop on the first answer. Signed-off-by: jitokim <pigberger70@gmail.com>
1 parent f8eaa4f commit d250d8b

2 files changed

Lines changed: 45 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,29 @@ oh-my-graph is **alpha software**. The graph YAML schema, the CLI, and the
1212

1313
### Changed
1414

15+
- **The spawn retry now waits long enough to be useful.** #214 gave the
16+
assessor a bounded retry for a CLI that never started, and #226 extended it to
17+
the planner. Both shipped with a **3-attempt, 300ms** bound — a 600ms window —
18+
and then two more lanes died on the same failure *after* the retry was in
19+
place:
20+
21+
```
22+
assessor run: claude run: spawn failed: exec: "claude": executable file not found in $PATH
23+
planner run: claude run: spawn failed: exec: "claude": executable file not found in $PATH
24+
```
25+
26+
Four occurrences in one day, two of them post-fix, say the bound was **correct
27+
in shape and useless in size**: 600ms is less than a package manager takes to
28+
relink a binary. Widened to **5 attempts, 2s apart** — eight seconds of
29+
patience, which buys the common case. A machine that genuinely has no CLI
30+
installed still fails, eight seconds later, saying exactly what it said before.
31+
32+
Nothing else changed: only a spawn that never happened is retried. A refused
33+
reply, a non-zero exit, a timeout and a cancelled context still stop on the
34+
first answer.
35+
36+
### Changed
37+
1538
- **An unresolvable `{{ inputs.x }}` or `{{ artifacts.id }}` now says that a
1639
merely-quoted placeholder is resolved too, and how to quote one.** The two
1740
reasons were written as if the graph had meant the token — the artifact one

internal/coordinator/assess.go

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,17 +195,28 @@ func assessmentFailure(reason string, outcome runner.NodeOutcome) *AssessError {
195195
// claim is measured, not assumed: E8 (assess_manual_test.go, `-tags manual`;
196196
// recorded in ADR 0011's Measurement outcome, 2026-08-02) fed this stance a
197197
// read-this-file lure and the file's content never reached the verdict.
198-
// assessorSpawnAttempts bounds how many times the assessor's subprocess may be
199-
// launched. Small on purpose: this covers a binary that is momentarily absent —
200-
// the case that named it was an npm update replacing `claude` on PATH mid-run
201-
// (#214) — not a machine that has no CLI installed, which fails all three just
202-
// as fast and reports the same thing.
203-
const assessorSpawnAttempts = 3
204-
205-
// assessorSpawnRetryDelay separates the attempts. Short, because the failure it
206-
// covers is a file being replaced rather than a service being down; long enough
207-
// that three attempts do not all land inside one `mv`.
208-
var assessorSpawnRetryDelay = 300 * time.Millisecond
198+
// assessorSpawnAttempts bounds how many times a coordinator call's subprocess
199+
// may be launched. It covers a binary that is momentarily absent — an npm update
200+
// replacing `claude` on PATH mid-run (#214) — not a machine with no CLI
201+
// installed, which exhausts the attempts just as fast and reports the same thing.
202+
//
203+
// Widened from 3×300ms to 5×2s on 2026-08-22, because the first window was
204+
// shorter than the thing it was covering. Four occurrences in one day, two of
205+
// them AFTER the retry shipped:
206+
//
207+
// assessor run: claude run: spawn failed: exec: "claude": executable file not found in $PATH
208+
// planner run: claude run: spawn failed: exec: "claude": executable file not found in $PATH
209+
//
210+
// 600ms is less than an npm install takes to relink a binary, so the retry was
211+
// correct in shape and useless in size. 8s of total patience buys the common
212+
// case; a machine that genuinely lacks the CLI still fails, just eight seconds
213+
// later, and says the same thing it said before.
214+
const assessorSpawnAttempts = 5
215+
216+
// assessorSpawnRetryDelay separates the attempts. Long enough that the attempts
217+
// do not all land inside one package manager's relink — the failure this covers
218+
// is a file being replaced, and the replacement is not instantaneous.
219+
var assessorSpawnRetryDelay = 2 * time.Second
209220

210221
// runAssessorWithSpawnRetry launches a coordinator call, retrying ONLY when the
211222
// CLI never started.

0 commit comments

Comments
 (0)