The 10 MiB cap on resolved stdin_source values is tight for wide
fan-in: a context.parallel.results batch from a large for_each round
carries tens of structured agent outputs, and a merge step that feeds
them to a deterministic command hits the ceiling as a hard
deterministic failure. Raise the ceiling to 30 MiB; it still bounds
peak memory and remote uploads, just with headroom matched to the
fan-out sizes for_each already allows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Workflow agent sessions run at PermissionLevel::Full with the whole
tool registry exposed, so blocking pre_tool_use hooks are the only
policy boundary they have. The child-session factory built for
spawn_agent dropped tool_hooks from the child's SessionOptions, so a
subagent's tool calls never reached the run's hooks: any agent that
could spawn a subagent got an unguarded read-write-shell escape from
every hook-enforced policy.
Clone the parent's tool_hooks into the factory, the same way the
permission level is already carried, so child sessions inherit the
parent's hook boundary. The new end-to-end test drives the real
create_session path against a scripted mock provider: the parent
spawns a child, the child executes read_file, and the hooks must see
both the parent's spawn_agent and the child's read_file.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CommandHandler also serves type="tool" nodes, so the runtime message now
says "Node '...'" to match the stdin_source_valid lint wording.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- 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>
These were untracked working-tree files unrelated to for_each item
injection. They were swept in by an over-broad `git add` and do not
belong on this branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses a Copilot review comment on #653.
The source array is runtime data, usually produced by a model, so its
length is not something a workflow author reviewed. Two changes, so an
over-long array degrades into a clear error rather than memory pressure.
Cap the item count at 1000. Above that the stage fails deterministically
before `parallel.started`, alongside the other for_each contract
violations, and the message says how to reduce the array.
Fork the parent context inside the branch task, after it acquires a
`max_parallel` slot, instead of at dispatch time. Live context copies now
track `max_parallel` rather than item count. Only the branch's own
preamble entry is moved into the task, so the shared stash is not cloned
per branch either.
The reviewer also suggested replacing spawn-all with `max_parallel`
workers pulling from a queue. Not done here: with the fork deferred, a
pending task holds little beyond its item, and reshaping the dispatch
loop would change cancellation and scope-reservation ordering, which
deserves its own review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses three Copilot review comments on #653.
`item_label` comes from a model or a workflow author, and it reaches the
terminal through the CLI progress display. A label could carry ANSI
escapes, newlines, or bidi overrides and rewrite what the operator sees.
It could also be whitespace-only, giving a branch a blank identity.
Add `text::sanitize_display_label`: strip ANSI sequences, drop control
and bidi-reordering characters, trim, and elide past 80 characters.
Return an empty string when nothing printable survives so callers fall
back to an identity they control.
Apply it where the label is created, so events, the store, and the web
UI all get a clean value instead of each consumer having to remember.
`parallel_branch_display` sanitizes again, because a run recorded before
this commit still has raw labels in its event log.
`emit_branch_retrying` now sets `stage.retrying`'s `index` from the
branch stage's execution ordinal, matching the envelope `stage_id` and
the meaning every other emitter gives that field. The branch's position
in the fan-out is already on `parallel.branch.started`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build_branch_plan read the runtime array before run_branches looked at
`simulated`, so every for_each workflow failed under --dry-run with
"for_each source '...' was not found in workflow context". Nothing had
populated the key yet: upstream LLM nodes take Handler::simulate, which
returns no context updates.
A dry run now stands in one placeholder item when the source is absent or
unusable, and simulates the template target once. Graph-shape mistakes
still fail, since catching those is the point of a dry run.
Also from review:
- ITEM_FENCE_PREFIX replaces the bare "untrusted-" literal that
render_item_data and its test each spelled out.
- ItemRecordingHandler no longer guesses an item label by substring
search. Nothing asserted it, and the third item's label "2" matched
stray hex from the random fence tag about two thirds of the time.
- The twin test reads keys::PARALLEL_RESULTS instead of the raw string.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
The schema advertises defaults for `block` and `timeout`, but the
executor errored when either was absent. Apply the advertised defaults
instead, and keep the type check for values that are present.
The required list stays as the Claude 5 contract declares it. Constants
now hold the defaults and the maximum so the schema and the executor
cannot drift.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reduce duplication and over-specification introduced with the Modal
provider, without changing shipped behavior.
- Extract enabled_provider_catalog and assert_deep_tool_round_trip in
the fabro-llm integration tests. The Poolside, Fireworks, OpenRouter,
and Modal deep round trips were four near-identical copies.
- Add ApiCredential::with_extra_headers for providers that authenticate
with request headers instead of an API key.
- Replace the unreachable require_env guards in the Modal e2e test with
the std::env::var form used by every sibling test, and register
MODAL_TOKEN_ID and MODAL_TOKEN_SECRET in EnvVars.
- Collapse modal_requires_both_vault_proxy_tokens to a single case. The
loop rebuilt the whole built-in catalog per iteration.
- Drop tautological and over-specified assertions: the api_key_url doc
URL, the forced default/probe lookups on a single-model provider, and
the get_on_provider loop that could not fail.
- Inline the single-use modal_env_catalog fixture and note why it
overrides the shipped secrets templates.
- Sort the MODAL_* keys in .env.example, and record in modal.toml why
api_id keeps the Hugging Face capitalization.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`parallel_group_id` used `oneOf: [$ref StageId, null]`, and the generator
drops a sibling description in that position, so the TypeScript client
documented the field as "Canonical stage execution identifier in
`node_id@visit` form" — the shared StageId text, which says nothing about
what this field means. Switching to `allOf` lets the field's own
description through.
Dropping `type: "null"` also makes the contract match the server, which
omits both fields rather than sending null (`skip_serializing_if` on
`Option`, pinned by list_run_stages_exposes_parallel_branch_identity).
The Rust types are unchanged — still `Option<StageId>` and `Option<u32>`,
which accept an explicit null on input either way — so this only narrows
what clients are told to expect on the wire. Wording updated to match,
and reworded to avoid an apostrophe the generator escapes into the
JSDoc.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses Copilot review feedback on #668.
Checking for `:` before the legacy `/` form broke `provider/model`
references whose selector contains a colon, which Bedrock API IDs do:
`bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0` parsed as the
whole path up to the last colon, then `0`. On main it is a pin to
bedrock with the full ID as the selector.
Now whichever separator appears first decides. A `/` before any `:` is
the legacy pin and its selector may contain colons. Otherwise the token
stays bare and `qualify` promotes it only when the prefix names a
provider, so `openrouter:moonshotai/kimi-k3` still qualifies.
Adds the regression test Copilot asked for, covering both Bedrock-style
legacy input and the colon-before-slash case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
