Skip to content

Commit 989f828

Browse files
authored
fix(runner): an inapplicable cap is not an unsafe one (#185)
* fix(runner): an inapplicable cap is not an unsafe one Codex preflight refused `agent:` and `budget_usd` with the same sentence, and they are not the same kind of thing. `agent:` names a Claude Code subagent — without it the node runs without that agent's system prompt, so it is a different node, and refusing stays correct. `budget_usd` is a USD ceiling over a runtime that reports no USD: there is no quantity to bound, which makes it inapplicable rather than unsafe. ValidateGraphForRuntime now returns warnings alongside its error. A budgeted node under codex loads and warns that the cap cannot apply, naming the guard still in force — its own `timeout:` or the runner's default — quoted from the constant the CLIRunner applies. The runaway guard was never the budget: graphs/fragments/e2e-verify.yaml says so beside its own `budget_usd: 10.00` ("The hang guard is `timeout:`, not the budget"). Measured, `--runtime codex lint graphs/*.yaml`: five of eight graphs refused before, one after — adr-driven-dev, for `agent:`. `auto --max-goal-budget-usd` stays refused, deliberately: a goal ceiling is the ONLY bound on an iterating loop, so an unmeasurable one would stop at its first cycle boundary with StopBudgetUnmeasurable having bought a cycle to learn what preflight says for free. All five call sites surface the warnings through one helper; `run` alone discards executeGraph's second copy, having already printed the list in its pre-run disclosure. A nil writer there means stderr, never silence. shipped_graphs_runtime_test.go lints every embedded graph under both runtimes and asserts the verdict per named graph, with its reason, so a graph that becomes unloadable under Codex fails `make test` by name. The Claude path does not move: ValidateGraphForRuntime still returns on its first line for RuntimeClaude. --help, version, lint and --dry-run over all eight shipped graphs are byte-identical to HEAD's binary. Refs: ADR 0026, amending ADR 0025. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> * fix(runner): the goal ceiling is not the only bound, and say so ADR 0026 justified refusing `auto --max-goal-budget-usd` on a premise the code denies: that a goal ceiling is the ONLY bound on an iterating loop. flags.go refuses the flag unless --max-cycles >= 2, and its own comment says the cycle flag IS the bound with no unbounded spelling (ADR 0011 §1). Drop the ceiling and the loop is still hard-bounded — the exact parallel to a node keeping its `timeout:` that the record spends a section denying. The sound argument was already there and is verifiable: goal.go checks the ceiling only inside its `cycle > 1` block, so accepting an unmeasurable one buys a whole cycle before StopBudgetUnmeasurable says what preflight says for free, while an inapplicable node cap costs nothing extra. That is now the reason in all six places the claim appears — preflight.go's doc, noteCodexRuntimePolicy, ADR 0026 §2, CHANGELOG, LIMITATIONS, EXAMPLES — with "only bound" demoted to "only spend-shaped bound". README.ko.md still said all three declarations are refused before execution; the English paragraph was rewritten in 773ba51 and its mirror was not. Ported. This mirror has drifted twice before (CHANGELOG:487, :1336). Three new tests in cmd/oh-my-graph/runtime_test.go pin the claim that made the split worth making — that every call site PRINTS. `lint` and `--dry-run` are pinned at their spawn-free sites, and `run` is pinned to print the list EXACTLY once counted across stdout and stderr together, so the fix for a double print cannot be to move one copy where the test is not looking. Mutation-checked: `_ = runtimeWarnings` at either spawn-free site, or dropping `run`'s io.Discard, turns each red; before this they stayed green. shipped_graphs_runtime_test.go counts a graph's warnings instead of asserting presence, because ADR 0026 §1's table records counts: stripping one of review-loop's two caps was green under the bool and now fails by name with both numbers. `run` also passes flags.graphPath rather than "", so its warning carries the same `<path>: ` prefix as the other four sites, and warnRuntimePreflight now documents why `run` alone routes to stdout — the disclosure block below refers back to those lines, and a reference split from its referent across two streams reads worse than the inconsistency. The 2026-08-14 plan record gets a banner for the one item ADR 0026 reversed. The Claude path still does not move: lint and --dry-run over all eight shipped graphs, --help, bare `run` and bare `auto` are byte-identical in stdout, stderr and exit to 773ba51's binary. Refs: ADR 0026. Co-Authored-By: oh-my-graph <graphs@oh-my-graph.dev> Signed-off-by: jitokim <pigberger70@gmail.com> --------- Signed-off-by: jitokim <pigberger70@gmail.com>
1 parent 5e1085a commit 989f828

19 files changed

Lines changed: 729 additions & 41 deletions

CHANGELOG.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,28 @@ oh-my-graph is **alpha software**. The graph YAML schema, the CLI, and the
1010

1111
## [Unreleased]
1212

13+
### Changed
14+
15+
- **A node's `budget_usd` no longer refuses a Codex graph
16+
([ADR 0026](docs/adr/0026-an-inapplicable-cap-is-not-an-unsafe-one.md)).**
17+
Preflight had one sentence for two different facts: `agent:` names a subagent
18+
whose system prompt the node would otherwise lose (a different node — still
19+
refused), while `budget_usd` is a USD ceiling a runtime that reports no USD
20+
has nothing to bound. Inapplicable is not unsafe, so the graph now loads and
21+
warns per node, naming the guard still in force — that node's `timeout:`, or
22+
the runner's 20m default. Measured on `graphs/*.yaml`: **five refused under
23+
`--runtime codex` before, one after** (`adr-driven-dev`, for its `agent:`).
24+
`auto --max-goal-budget-usd` stays refused and that is not an inconsistency:
25+
it is checked only at a cycle boundary, so an unmeasurable ceiling would buy a
26+
whole cycle before stopping to say it cannot be checked, where an inapplicable
27+
node cap costs nothing extra. The loop stays bounded either way —
28+
`--max-cycles` is what bounds iterations.
29+
`internal/runner/shipped_graphs_runtime_test.go` now lints every shipped
30+
graph under both runtimes and asserts the verdict by name, so a graph that
31+
becomes unloadable under Codex fails `make test` instead of a user's run.
32+
**The Claude path is unchanged**: `ValidateGraphForRuntime` still returns on
33+
its first line for `RuntimeClaude`, warning nothing and refusing nothing.
34+
1335
## [v0.8.0] - 2026-08-15
1436

1537
**Minor because the CLI grew a token you may type.** Compared by name rather

DESIGN.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,10 @@ the selected protocol reports and uses it for `handoff: session`.
3333

3434
Codex reports token usage but no USD total. `CostUnknown` is durable through
3535
the ledger, `state.json`, `events.jsonl`, CLI history views and web UI; it must
36-
never render as `$0`. Positive `budget_usd`, `agent:`, and the goal-level USD
37-
budget are refused at preflight for Codex.
36+
never render as `$0`. `agent:` and the goal-level USD budget are refused at
37+
preflight for Codex; a node's `budget_usd` is not (ADR 0026) — it loads with a
38+
warning saying the cap cannot apply and naming the guard that still holds, that
39+
node's own `timeout:` or the runner's default.
3840

3941
## Node runtime mechanics (ground truth — use exactly)
4042

README.ko.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ Codex는 `permission_mode: plan`을 read-only sandbox로, 일반 모드를
7676
`workspace-write`로, `bypassPermissions``danger-full-access`로 매핑합니다.
7777
이 sandbox는 네트워크 경계이기도 해서, Codex 그래프는 push하거나 `gh`
7878
호출하는 첫 번째 노드에서 멈춥니다.
79-
Codex에는 Claude의 호출별 `budget_usd``agent:` 선택자가 없고 USD도 보고하지
80-
않으므로, 이 두 graph field와 `--max-goal-budget-usd`는 실행 전에 거부합니다.
79+
Codex는 USD를 보고하지 않고 Claude의 `agent:` 선택자도 구현하지 않으므로,
80+
`agent:``--max-goal-budget-usd`는 Codex run이 무언가를 쓰기 전에 거부합니다.
81+
노드의 `budget_usd`는 거부하지 않습니다. 묶을 USD가 없어 그저 적용될 수 없을
82+
뿐이므로, 그래프는 로드되고 노드마다 경고 한 줄이 그 사실과 함께 여전히 그
83+
노드를 지키는 `timeout:`을 알려줍니다.
8184
Claude Code agent mapping과 skill activation은 Claude 전용이며, Codex `auto`
8285
Codex sandbox isolation을 사용합니다.
8386

README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,11 @@ Codex maps `permission_mode: plan` to its read-only sandbox, ordinary modes to
7272
`workspace-write`, and `bypassPermissions` to `danger-full-access`. That sandbox
7373
is also a network boundary, so a Codex graph halts at its first node that
7474
pushes or calls `gh`. Codex does
75-
not report USD or implement Claude's per-call `budget_usd` and `agent:`
76-
selector, so those two graph fields and `--max-goal-budget-usd` are rejected
77-
before a Codex run spends anything. Claude Code agent mapping and skill
75+
not report USD or implement Claude's `agent:` selector, so `agent:` and
76+
`--max-goal-budget-usd` are rejected before a Codex run spends anything. A
77+
node's `budget_usd` is not rejected: with no USD to bound it simply cannot
78+
apply, so the graph loads and one warning per node says so and names the
79+
`timeout:` that still guards it. Claude Code agent mapping and skill
7880
activation are Claude-only; Codex `auto` runs use Codex sandbox isolation
7981
instead.
8082

SECURITY.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,10 @@ assessor do not. A hand-written graph also keeps the user's normal config,
151151
matching the existing reviewed-artifact boundary.
152152

153153
This is a filesystem sandbox stance, not granular enforcement of
154-
`allowed_tools`. Positive `budget_usd`, `agent:`, and the goal-level USD budget
155-
flag are rejected for Codex rather than silently ignored. Claude agent mapping
154+
`allowed_tools`. `agent:` and the goal-level USD budget flag are rejected for
155+
Codex rather than silently ignored. A node's `budget_usd` is neither rejected
156+
nor silently ignored: it is accepted with a warning that the cap cannot apply
157+
and that the node's `timeout:` is the guard still in force (ADR 0026). Claude agent mapping
156158
and skill activation are not attempted. Codex USD cost is recorded as unknown;
157159
its provider-reported token counts are the accounting surface.
158160

cmd/oh-my-graph/dryrun.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ func dryRunGraphForRuntime(w, warnW io.Writer, path string, inputs map[string]st
4040
}
4141

4242
g := loaded.Graph
43-
if err := runner.ValidateGraphForRuntime(runtime, g); err != nil {
43+
runtimeWarnings, err := runner.ValidateGraphForRuntime(runtime, g)
44+
warnRuntimePreflight(warnW, path, runtimeWarnings)
45+
if err != nil {
4446
return err
4547
}
4648
warnAdvisories(warnW, path, g)

cmd/oh-my-graph/flags.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ type commonRunFlags struct {
2323
noWeb bool
2424
planningCostUnknown bool
2525
planningUsage runner.TokenUsage
26+
// runtimeWarnW receives the runtime-preflight warnings executeGraph's own
27+
// runner.ValidateGraphForRuntime call produces (ADR 0026). Not a flag: the
28+
// caller's answer to "have these already been shown?". `run` sets it to
29+
// io.Discard because it surfaced the identical list at load, as part of the
30+
// pre-run Codex disclosure; every other entry to executeGraph leaves it nil,
31+
// which means os.Stderr — nil must never mean silence, or the one path that
32+
// forgets to set it drops the warning.
33+
runtimeWarnW io.Writer
2634
}
2735

2836
func (c *commonRunFlags) register(set *flag.FlagSet) {

cmd/oh-my-graph/lint.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ func lintGraphForRuntime(w, warnW io.Writer, path string, runtime runner.Runtime
7676
return err
7777
}
7878
if len(issues) == 0 {
79-
if err := runner.ValidateGraphForRuntime(runtime, loaded.Graph); err != nil {
79+
runtimeWarnings, err := runner.ValidateGraphForRuntime(runtime, loaded.Graph)
80+
warnRuntimePreflight(warnW, path, runtimeWarnings)
81+
if err != nil {
8082
return err
8183
}
8284
printFragmentResolutions(w, loaded.Resolutions)
@@ -118,6 +120,33 @@ func warnAdvisories(warnW io.Writer, path string, g *graph.Graph) {
118120
}
119121
}
120122

123+
// warnRuntimePreflight prints one `warning:` line per runtime-preflight
124+
// warning — today, a `budget_usd` the selected runtime cannot evaluate
125+
// (ADR 0026). It is called at EVERY runner.ValidateGraphForRuntime call site,
126+
// including the ones whose call also returns an error: the two verdicts are
127+
// independent (a graph refused for `agent:` may also carry an inapplicable
128+
// cap), and a warning nobody prints is exactly the silent drop the split
129+
// exists to avoid. path may be empty, for the callers that judge a graph in
130+
// memory rather than a file on disk. Advice only: never an exit code.
131+
//
132+
// The writer is the caller's, and the callers do not all pick the same stream:
133+
// `lint`, `--dry-run`, `resume` and executeGraph's default put these on stderr
134+
// with every other `warning:` line, while `run` deliberately routes them to the
135+
// stdout its pre-run Codex DISCLOSURE prints on. That is not drift. The
136+
// disclosure block immediately below them (noteCodexRuntimePolicy) refers back
137+
// to these very lines — "each such node is warned by name" — and a reference
138+
// that lands on a different stream than its referent is a worse read than a
139+
// warning that is not on the advisory channel.
140+
func warnRuntimePreflight(warnW io.Writer, path string, warnings []string) {
141+
for _, warning := range warnings {
142+
if path == "" {
143+
fmt.Fprintf(warnW, "warning: %s\n", warning)
144+
continue
145+
}
146+
fmt.Fprintf(warnW, "warning: %s: %s\n", path, warning)
147+
}
148+
}
149+
121150
// warnFragmentAdvisories prints one `warning:` line per fragment-file
122151
// advisory (ADR 0013 — e.g. a declared substitution point the fragment body
123152
// never references). Same standing as warnAdvisories: advice only, never an

cmd/oh-my-graph/main.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,19 @@ func runGraphWithRuntime(runtime runner.Runtime, args []string, nodeRunner runne
298298
return err
299299
}
300300
g := loaded.Graph
301-
if err := runner.ValidateGraphForRuntime(runtime, g); err != nil {
301+
runtimeWarnings, err := runner.ValidateGraphForRuntime(runtime, g)
302+
// Part of the pre-run disclosure, so it prints where the rest of the Codex
303+
// policy prints and not on a stream the reader may not be watching. The
304+
// identical list is produced again inside executeGraph, which is the gate
305+
// `auto` and a chat-started graph reach WITHOUT passing here; runtimeWarnW
306+
// below tells that call not to print this run's copy twice.
307+
warnRuntimePreflight(stdout, flags.graphPath, runtimeWarnings)
308+
if err != nil {
302309
return err
303310
}
304311
noteCodexRuntimePolicy(stdout, runtime, g, false)
305312
flags.runtime = runtime
313+
flags.runtimeWarnW = io.Discard
306314
printFragmentResolutions(os.Stdout, loaded.Resolutions)
307315
// The advisory half of the same disclosure, through the same helper `lint`
308316
// and `--dry-run` use: a run that announces which fragments it spliced must
@@ -739,7 +747,17 @@ func executeGraph(ctx context.Context, runID string, g *graph.Graph, nodeRunner
739747
runtime = runner.RuntimeClaude
740748
}
741749
flags.runtime = runtime
742-
if err := runner.ValidateGraphForRuntime(runtime, g); err != nil {
750+
runtimeWarnings, err := runner.ValidateGraphForRuntime(runtime, g)
751+
// The last gate before anything spends, and the ONLY one an `auto` or
752+
// chat-started graph passes through, so it surfaces its own copy (ADR 0026).
753+
// A nil writer means stderr; `run` alone passes io.Discard, having already
754+
// printed this list beside the rest of its Codex disclosure.
755+
runtimeWarnW := flags.runtimeWarnW
756+
if runtimeWarnW == nil {
757+
runtimeWarnW = os.Stderr
758+
}
759+
warnRuntimePreflight(runtimeWarnW, graphSourcePath, runtimeWarnings)
760+
if err != nil {
743761
return err
744762
}
745763
// The leg holds the run's resume.lock for its whole duration — the same
@@ -1085,6 +1103,7 @@ func noteCodexRuntimePolicy(w io.Writer, runtime runner.Runtime, g *graph.Graph,
10851103
fmt.Fprintln(w, " Last node: adr-driven-dev (finalize), and every user of graphs/fragments/pr-publish.yaml (self-dev, dev-review-pr, backlog-batch). First node: apply-flags (dev pushes before verify reads). Every node: merge-shepherd, which is `gh` end to end and fails at node 1 having done nothing.")
10861104
fmt.Fprintln(w, " Two remedies, both per node: permission_mode: bypassPermissions maps to danger-full-access, which is no sandbox — that node keeps network AND keyring. Or Codex's sandbox_workspace_write.network_access=true, which lifts the block for `git push`/`git ls-remote` but not for `gh` on a machine where gh's token is in an OS keyring the sandbox denies (measured 2026-08-14, macOS: \"no oauth token found for github.com\"; where no keyring exists gh reads ~/.config/gh/hosts.yml, which the sandbox can read).")
10871105
fmt.Fprintln(w, " Cost is unknown for every Codex node: tokens are counted, USD never is, so this run reports no dollar figure per node or in total.")
1106+
fmt.Fprintln(w, " A node's budget_usd therefore loads but cannot apply — there is no spend to compare it against, and that node's runaway guard is its timeout: (each such node is warned by name). `auto --max-goal-budget-usd` is refused instead, because it is checked only at a cycle boundary: an unmeasurable ceiling would buy a whole cycle before stopping to say it cannot be checked, where an inapplicable node cap costs nothing extra. The loop stays bounded either way — --max-cycles is what bounds iterations (ADR 0026).")
10881107
fmt.Fprintln(w, " approval_policy=\"never\" is passed on every node: a non-interactive run cannot answer a prompt, so nothing is escalated for approval.")
10891108
fmt.Fprintln(w, " No session-limit pause: ADR 0009's resumable pause is Claude-only, so a Codex session limit is an ordinary node failure (ADR 0009 scopes it to the Claude runtime).")
10901109
if isolated {

cmd/oh-my-graph/resume.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,13 @@ func continueRun(flags *resumeFlags, snap runstate.Snapshot, records map[string]
355355
if err != nil {
356356
return fmt.Errorf("resume run %q: %w", runID, err)
357357
}
358-
if err := runner.ValidateGraphForRuntime(runtime, g); err != nil {
358+
runtimeWarnings, err := runner.ValidateGraphForRuntime(runtime, g)
359+
// A resumed leg re-surfaces these for the same reason it re-warns about
360+
// bypassPermissions below: the terminal that saw the first leg's copy may
361+
// be long gone, and a cap that cannot apply is a fact about the nodes this
362+
// leg is about to spend on (ADR 0026).
363+
warnRuntimePreflight(os.Stderr, snap.GraphSourcePath, runtimeWarnings)
364+
if err != nil {
359365
return fmt.Errorf("resume run %q: %w", runID, err)
360366
}
361367
// A resumed leg re-warns exactly as `run` did at load: the warning is

0 commit comments

Comments
 (0)