Skip to content

refactor(execute): single exit paths for pipe/wait and cache update#442

Open
wan9chi wants to merge 2 commits into
mainfrom
execute-spawn-exit-paths
Open

refactor(execute): single exit paths for pipe/wait and cache update#442
wan9chi wants to merge 2 commits into
mainfrom
execute-spawn-exit-paths

Conversation

@wan9chi

@wan9chi wan9chi commented Jun 11, 2026

Copy link
Copy Markdown
Member

Motivation

execute_spawn reports errors through leaf_reporter.finish() — but from three different places, each duplicating the finish-and-return dance: a pipe failure reported inside the drain section, a wait failure had its own identical arm, and an archive-creation failure called finish() from deep inside the cache-update expression. Every change to this function has to keep those exits consistent by hand, and the runner-aware IPC work (#441) is about to thread per-task server state through exactly these paths — each early exit is one more place to forget a drain or a signal.

Approach

Two exit-point consolidations, no behavior change:

  • One error sink for pipe + wait. pipe_stdio and child.wait fold into a single child_work future; a pipe failure cancels (so the wait kills the child instead of orphaning it — same ordering as before) and surfaces through the same wait_result match as a wait failure. Box::pin(child_work) keeps the pipe stack off the enclosing future, which shrinks execute_spawn enough that the large_futures expects at both call sites become removable — clippy's unfulfilled-expectation lint enforces that this is true, not just claimed.
  • One exit for the cache-update section. The section becomes a 'cache_update: labeled block where every outcome — including the archive-creation failure that previously returned early — breaks with a (status, error) pair, reported by the single finish() at the end.

Also renames the post-execution fspy summary local from tracking to fspy_outcome, freeing the name for #441's per-task tracking state.

Same statuses, same errors, same messages: the entire snapshot suite is byte-identical. One nuance worth a reviewer's glance: a pipe failure's finish() now happens after the unified match rather than inline in the drain section — observably identical, deliberately so.

Second commit: split scheduling and cache-update out of execute_spawn

execute/mod.rs mixed three concerns — the DAG scheduler (which leaf runs when), the spawn pipeline (how one leaf runs), and the post-run cache decision — with reporting sprinkled through all of them (finish() called from seven places).

  • scheduler.rs: ExecutionContext + the dependency-ordered, fast-fail graph walking, plus Session::execute_graph. Pure move.
  • cache_update.rs: the may-this-run-be-cached decision flattened into guard clauses that name each refusal (cancelled, non-zero exit, read-write overlap, fspy unsupported), then fingerprint → archive → store. The labeled-block breaks become plain returns.
  • mod.rs keeps the typed ExecutionMode/CacheState and a pipeline whose phases (lookup_cache, replay_cache_hit, ExecutionMode::build, run_child, update_cache) construct a Report instead of reporting in place — the reporter-consuming finish() now happens in exactly one spot, in execute_spawn.

Still zero behavior change: snapshot suite byte-identical. #441 on top now integrates IPC as a Tracking field, one pipeline arm, and one guard clause, with its execute/mod.rs diff almost purely additive.

execute_spawn had three early error exits that each duplicated the
finish-and-return dance: a pipe failure reported and returned inside the
drain section, a wait failure had its own identical arm, and an
archive-creation failure called finish() from deep inside the
cache-update expression.

Fold pipe-drain and child-wait into one child_work future whose pipe
failure cancels, reaps the child, and surfaces through the same
wait_result match as a wait failure. Convert the cache-update section to
a labeled block where every outcome breaks with a (status, error) pair,
reported by the single finish() at the end.

Box::pin(child_work) keeps the pipe stack off the enclosing future,
which shrinks execute_spawn enough that the large_futures expects at
both call sites become removable (clippy enforces this). Also rename
the post-execution fspy summary local from tracking to fspy_outcome —
clearer, and it frees the name for the upcoming per-task IPC tracking
state (#441).

No behavior change: same statuses, same errors, same messages; the
entire snapshot suite is byte-identical.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@wan9chi wan9chi marked this pull request as ready for review June 11, 2026 17:43

wan9chi commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

…pawn

execute/mod.rs mixed three concerns: the DAG scheduler (which leaf runs
when), the spawn pipeline (how one leaf runs), and the post-run cache
decision — plus reporting sprinkled through all of them, with finish()
called from seven different places.

- scheduler.rs: ExecutionContext and the dependency-ordered, fast-fail
  graph walking, plus Session::execute_graph. Pure move.
- cache_update.rs: the may-this-run-be-cached decision flattened into
  guard clauses that name each refusal (cancelled, non-zero exit,
  read-write overlap, fspy unsupported), then fingerprint + archive +
  store. The labeled-block breaks become plain returns.
- mod.rs keeps the typed ExecutionMode/CacheState and a pipeline whose
  phases (lookup_cache, replay_cache_hit, ExecutionMode::build,
  run_child, update_cache) construct a Report instead of reporting in
  place; the reporter-consuming finish() now happens in exactly one
  spot, in execute_spawn.

No behavior change: same statuses, errors, and messages; the snapshot
suite is byte-identical. Sets up #441's IPC integration to slot in as a
Tracking field, one pipeline arm, and one guard clause.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant