Commit graph

20 commits

Author SHA1 Message Date
Bryan Helmkamp
a049f94042
Add on_failure="succeed" as an explicit failure policy
A failed node with an effective `succeed` policy and no explicit recovery
route now finishes as `succeeded` and follows normal success routing. The
original failure stays on the outcome so the stage.completed event and the
checkpoint keep the diagnostic, and the outcome notes record which scope
promoted it.

- OnFailure gains a Succeed variant; Node::on_failure resolves the
  deprecated auto_status=true attribute as an alias, with an explicit
  on_failure winning
- The core executor applies the policy before the lifecycle observes the
  result, so the recorded outcome, context keys, goal gates, events, and
  routing all see the effective outcome; this replaces AutoStatusLifecycle
- Explicit routes take priority: a matching condition, preferred label,
  suggested next node, or handler jump keeps the outcome failed. A failed
  outcome takes an unconditional edge only under route, so under succeed
  any edge selection is an explicit route
- succeed applies only to failed, matching exit; the auto_status alias no
  longer promotes partially_succeeded
- Parallel branches promote after their retry loop, so a failed succeed
  branch counts as succeeded in the parent aggregate
- Validation accepts succeed and adds an auto_status_deprecated warning
  that suggests on_failure="succeed"
- Document the policy table, semantics, and deprecation; add a changelog
  entry

Closes #807

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-26 07:34:24 -04:00
Bryan Helmkamp
105f180d3d
Simplify node failure policy resolution 2026-08-25 20:10:05 -04:00
Bryan Helmkamp
491babe5da
Add node-level on_failure override
A node can now set its own on_failure attribute to override the
graph-level failed-node routing policy in either direction: a
best-effort node can keep route inside an exit graph, and a critical
node can exit while the rest of the graph keeps the default. An absent
node attribute inherits the graph policy.

- Node::on_failure returns Option<OnFailure> so absence means inherit
- Graph::resolve_on_failure(node_id) is the single resolution point,
  returning ResolvedOnFailure { policy, scope } so the executor's
  end-of-run message names the scope that stopped routing
- The core Graph trait method becomes resolve_on_failure(node_id); the
  graph-scope failure message is unchanged
- The failed-human-gate fallthrough block stays independent of a
  node-level route override
- Validation now accepts and value-checks node-level on_failure (it
  previously warned that node placement had no effect) and keeps the
  edge-placement warning with updated wording
- Document precedence in transitions, failures, and the DOT reference,
  and extend today's changelog entry

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Chraa21RK7i2KqHdZSJLb8
2026-08-25 18:55:39 -04:00
Bryan Helmkamp
15d1ef5b2c
Simplify on_failure validation rule and tests
- Dedup Diagnostic construction in the on_failure_valid rule
- Use the shared node_with_attrs test helper
- Drop an executor test that duplicated existing retry-target coverage
- Build on_failure integration test graphs from DOT and share a run
  harness, exercising the parser path for valid on_failure values

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J6MnJri6oSEMZaYeADY5dP
2026-08-25 14:49:05 -04:00
Bryan Helmkamp
b4092af89f
Add graph on_failure exit policy 2026-08-25 13:45:51 -04:00
Bryan Helmkamp
6e2a2ac652
Merge remote-tracking branch 'origin/main' into feat/command-stdin-source
# Conflicts:
#	docs/public/workflows/stages-and-nodes.mdx
#	lib/components/fabro-validate/src/rules/inert_attribute.rs
#	lib/components/fabro-workflow/src/handler/command.rs
2026-07-29 11:37:27 -04:00
Bryan Helmkamp
0eda219376
Simplify stdin_source plumbing after review
- ExecStreamingRequest: drop #[non_exhaustive] and the six Option-taking
  builder setters; call sites use struct literals over ::new(), matching
  GrepOptions/WalkOptions, and providers can destructure exhaustively
- Docker: pass ExecStreamingRequest through docker_exec_shell_streaming
  instead of seven positional args; revert the no-op StartExecOptions
- Daytona: stdin temp-file cleanup is now best-effort (mirrors
  DaytonaSession::close) so a failed delete cannot fail a completed
  command or double-delete from Drop; upload overlaps session creation;
  one shared DAYTONA_CLEANUP_TIMEOUT
- write_process_stdin tolerates ConnectionReset/ConnectionAborted so a
  command that stops reading stdin does not fail on TCP Docker daemons
- Local sandbox aborts the stdin writer after process exit instead of
  joining unbounded
- Cap stdin_source payloads at 10 MiB, mirroring the for_each bound
- Add Node::context_key_attr() tri-state so the handler and lint rule
  share one definition of a valid context-key attribute
- inert_attribute canonicalizes handler types via StageHandler, fixing
  false warnings for command attrs on tool nodes
- Share resolve_flat_context_value between command stdin and for_each;
  resolve_json_value takes Value by value, removing a deep clone
- Reuse MockSandbox in command handler stdin tests instead of extending
  SpySandbox with a hand-rolled streaming override

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-29 11:14:48 -04:00
Bryan Helmkamp
d37fc0027c
fix(workflow): align inferred command behavior 2026-07-29 10:54:43 -04:00
Bryan Helmkamp
cb24f47b59
Merge remote-tracking branch 'origin/main' into feat/infer-command-node-from-script
# Conflicts:
#	docs/public/workflows/stages-and-nodes.mdx
#	lib/foundation/fabro-types/src/graph.rs
2026-07-29 10:40:26 -04:00
Bryan Helmkamp
5d72f9a538
Add context-sourced command stdin 2026-07-29 10:25:51 -04:00
Bryan Helmkamp
a369ea7fc4
Merge remote-tracking branch 'origin/main' into feat/for-each-item-injection
# Conflicts:
#	apps/fabro-web/app/components/stage-renderers/parallel-children.tsx
2026-07-28 20:03:25 -04:00
Bryan Helmkamp
ee9189ddb5
Reuse existing helpers in the for_each fan-out
Simplification pass over the for_each branch. No behavior change.

Share what was duplicated:
- Node::prompt_or_label replaces the "prompt, else label" fallback that
  agent, prompt, and the for_each item injector each wrote out.
- context::lookup_flat replaces the "exact key, then strip context."
  lookup that condition.rs had twice and the for_each source had again.
- is_llm_handler_type replaces the inline agent/prompt match, so the
  runtime and the for_each_contract rule agree by construction.
- find_join_node now takes branch ids, so a for_each fan-out passes its
  template target instead of needing find_join_for_target.
- collect_events moves to test_support; parallel and integration tests
  shared one copy already.
- One ScriptedHandler replaces four test handlers that differed only in
  what they returned.

Straighten the branch retry loop:
- Reserve the branch scope once before the loop instead of guarding it
  with an Option, which removes three expect() calls.
- acquire_branch_permit and backoff_or_cancel replace the cancel-aware
  select! blocks the loop repeated verbatim.
- Keep the match arms in Executor::execute_with_retry order so the two
  loops stay easy to compare.

Drop redundant state:
- BranchPlan::is_for_each derives from template_target_id.
- for_each_contract checks node type before source shape, so one
  mistake reports one diagnostic.

Prove the new wire fields survive the OpenAPI boundary: the fabro-api
round-trip fixture now carries index and item_label.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 18:42:23 -04:00
Bryan Helmkamp
e6cf174578
Merge origin/main into feat/human-review-target
Conflict in apps/fabro-web/app/components/interview-dock.tsx. Main moved
the dock onto the shared collapsible `RunDockShell` and replaced the
local button constants with shared ones.

Kept main's structure whole and re-applied the review target rendering
onto it: the question paragraph in the shell's `body` becomes the linked
`ReviewTargetQuestion` when the target passes `safeReviewTarget`, and
plain text otherwise. Both now share main's paragraph classes through
`QUESTION_TEXT`, so the two renderings stay visually identical.

`peek` keeps using `question.text`, which is the correct plain-text
collapsed summary for a review target question.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 16:48:02 -04:00
Bryan Helmkamp
04c931e9d3
Simplify edge-only node validation 2026-07-28 14:14:32 -04:00
Bryan Helmkamp
010c8d50c1
Add structured review targets to human gates 2026-07-28 13:24:12 -04:00
Release Repro
69a51e65b9
feat(workflow): infer command nodes from the script attribute
A node with no `shape` defaulted to `box`, which resolves to the agent
handler. That made a shapeless `script` node run as an LLM call prompted
with its own label, while the `script` was reported as inert — wrong
behavior behind a warning.

`script` is read by the command handler and by nothing else, so a
shapeless node that sets it is unambiguously a command node. `shape()`
now infers `parallelogram` in that case. An explicit `shape` still wins.

Two rules keep the inference honest:

- `script_prompt_conflict` — setting both `script` and `prompt` is an
  error. No handler reads both. It fires regardless of shape so that
  adding one cannot downgrade the error to a warning.
- `command_requires_script` — a command node without a script is an
  error. Without this the original trap just moves: a node meant as a
  command that omits its script silently becomes an agent again.

Also drops the `tool_command` alias in favor of `script` alone, routing
the six read sites through a new `Node::script()` accessor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 14:59:31 -04:00
Bryan Helmkamp
59b1c2e59f
Reject nodes referenced by an edge but never declared
The DOT parser created a node for every edge endpoint, and nothing
recorded whether a node came from a declaration or was synthesized from
an edge. The edge_target_exists rule only checked whether the node id
was present in the graph, which was always true by then, so a misspelled
endpoint became an attribute-free node that defaulted to shape=box — an
LLM stage. Validation emitted a prompt_on_llm_nodes warning and exited 0.

Node now carries `implicit`, set only when the parser synthesizes the
node from an edge endpoint. A declaration anywhere in the workflow
clears it, so order does not matter and subgraph declarations count.
Node::new leaves it false, so programmatic construction and graphs
deserialized from older checkpoints read as declared.

edge_target_exists treats an endpoint as valid only when it exists and
is declared, reporting each undeclared node once. The near-identical
missing-source and missing-target branches collapse into one path. The
import transform copies the flag onto spliced nodes so an edge-only node
inside an imported fragment is caught too.

parse_and_validate_human_gate had two edge-only nodes and now declares
them; it was an instance of the bug rather than a casualty of the fix.
No shipped workflow, docs example, or CLI fixture relied on the old
behavior.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 13:53:54 -04:00
Bryan Helmkamp
b53045a1ac
Add runtime for_each item injection 2026-07-27 11:55:55 -04:00
Bryan Helmkamp
85f3286c66
Merge branch 'main' into feat/shared-checkout-parallel 2026-07-24 06:29:57 -04:00
Scott Werner
47bc772f7b refactor: organize crates into three layers 2026-07-23 17:59:34 -04:00