(MOT-4102, MOT-4103) feat(harness): one-way reactive orchestration — fire-and-forget spawn, terminal turns, hardened doctrine#540
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (54)
📝 WalkthroughWalkthroughThe change adds read-only database query enforcement, an ChangesRead-only execution
Harness orchestration
Wake-aware completion
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fp/src/util.rs (1)
833-839: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTest name/coverage is stale after adding
WHEN_IDtois_transform.
is_transformnow matches 18 ids (Line 708 addsWHEN_ID), but this test still says "seventeen_ops" and its loop omitsWHEN_ID, so the newly added branch is unasserted. AddWHEN_IDto the list and rename accordingly.💚 Update the assertion set and name
- fn is_transform_matches_exactly_the_seventeen_ops() { + fn is_transform_matches_exactly_the_eighteen_ops() { for id in [ GET_ID, PICK_ID, OMIT_ID, TAKE_ID, DROP_ID, MAP_ID, FILTER_ID, SPLIT_ID, JOIN_ID, UNIQ_ID, SIZE_ID, COMPACT_ID, NTH_ID, GET_OR_ID, FLATTEN_ID, SORT_BY_ID, REVERSE_ID, + WHEN_ID, ] {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fp/src/util.rs` around lines 833 - 839, Update is_transform_matches_exactly_the_seventeen_ops to include WHEN_ID in the asserted ID list, and rename the test to reflect all 18 supported transform operations.
🧹 Nitpick comments (2)
harness/src/functions/send.rs (1)
80-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify: provider inheritance only applies when
modelis also omitted.This comment reads as if provider inheritance is independent of whether
modelwas overridden, but Line 150-151 ((Some(m), _) => (m, req.provider.clone())) only inheritsproviderfrom the prior turn whenmodelis ALSO omitted — mirroringchild_providerinsubagent.rs(different model may need a different provider). A caller who explicitly setsmodelbut omitsproviderwill silently getNone, not the inherited provider. Worth spelling out explicitly so API consumers don't assume provider always survives a model override.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/src/functions/send.rs` around lines 80 - 85, Clarify the documentation above the model field to state that provider inheritance occurs only when model is omitted; when a caller overrides model without specifying provider, provider remains unset. Keep the existing behavior in the request handling logic unchanged, including the model-and-provider resolution in the line-150 flow.harness/src/functions/react.rs (1)
488-498: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUnconditional
parentresolution wastes work on call-mode fires.
parent(including the asyncresolve_root(deps, &sid).awaitwalk) is computed for every fire, but it's never consulted bycall_edge(line 520-523:call_edge(deps, &spec, &call, &event, event.clone())doesn't takeparent) nor byjoin_edge's call-downstream branch (dispatch_callalso ignoresparent). Call reactions are documented as "deterministic, token-free, milliseconds" (seedefault.txt/provider prompts); an unconditional ancestor-chain lookup undercuts that for every call-mode fire.⚡ Proposed fix: skip parent resolution for call-mode
- let parent = match spec.parent_session_id.clone() { - Some(p) => Some(p), - None => match parent_anchor(&event, &spec) { - Some(sid) => Some(resolve_root(deps, &sid).await), - None => None, - }, - }; + let parent = if spec.call.is_some() { + None + } else { + match spec.parent_session_id.clone() { + Some(p) => Some(p), + None => match parent_anchor(&event, &spec) { + Some(sid) => Some(resolve_root(deps, &sid).await), + None => None, + }, + } + };Also applies to: 520-523, 724-737
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harness/src/functions/react.rs` around lines 488 - 498, Restrict the `parent` computation in the reactive fire flow to edge modes that consume it, skipping `parent_anchor` and the async `resolve_root` walk for call-mode fires. Preserve explicit parent and anchor-root resolution for join or other modes that pass `parent` into downstream handling, including the `join_edge` path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fp/src/pipe.rs`:
- Around line 319-324: Update the clobber-note guard in the pipe handling logic
around args.pointer(pointer) to exclude empty container values, including the
documented empty-object padding pattern, while retaining detection for non-empty
literals. Keep the existing clobber_note message and behavior unchanged for
meaningful non-null values.
In `@harness/src/deferred.rs`:
- Around line 103-147: In the SPAWN branch of the deferred resolver, mark the
matching call’s state as Triggered and persist the updated turn with put_turn
before invoking spawn_from_turn, mirroring the sibling execute path. Keep the
existing completion logic that records the child identifiers and marks the call
Done after the spawn result is appended.
---
Outside diff comments:
In `@fp/src/util.rs`:
- Around line 833-839: Update is_transform_matches_exactly_the_seventeen_ops to
include WHEN_ID in the asserted ID list, and rename the test to reflect all 18
supported transform operations.
---
Nitpick comments:
In `@harness/src/functions/react.rs`:
- Around line 488-498: Restrict the `parent` computation in the reactive fire
flow to edge modes that consume it, skipping `parent_anchor` and the async
`resolve_root` walk for call-mode fires. Preserve explicit parent and
anchor-root resolution for join or other modes that pass `parent` into
downstream handling, including the `join_edge` path.
In `@harness/src/functions/send.rs`:
- Around line 80-85: Clarify the documentation above the model field to state
that provider inheritance occurs only when model is omitted; when a caller
overrides model without specifying provider, provider remains unset. Keep the
existing behavior in the request handling logic unchanged, including the
model-and-provider resolution in the line-150 flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fea0a639-8e86-4cc5-a81c-7405ce68cbc5
📒 Files selected for processing (54)
console/web/src/lib/backend/translate.test.tsconsole/web/src/lib/backend/translate.tsconsole/web/src/lib/sessions/entry-mapper.test.tsconsole/web/src/lib/sessions/entry-mapper.tsconsole/web/src/types/iii-agent-event.tsdatabase/src/driver/mysql.rsdatabase/src/driver/postgres.rsdatabase/src/driver/sqlite.rsfp/src/guidance.rsfp/src/main.rsfp/src/pipe.rsfp/src/util.rsharness/README.mdharness/architecture/README.mdharness/prompts/cli.txtharness/prompts/default.txtharness/prompts/subagent.txtharness/src/config.rsharness/src/deferred.rsharness/src/events.rsharness/src/functions/mod.rsharness/src/functions/react.rsharness/src/functions/send.rsharness/src/functions/spawn.rsharness/src/functions/status.rsharness/src/functions/stop.rsharness/src/functions/subscribe.rsharness/src/policy.rsharness/src/prompt/mod.rsharness/src/prompt/tests.rsharness/src/prompt/variants.rsharness/src/subagent.rsharness/src/subscriptions/mod.rsharness/src/subscriptions/registry.rsharness/src/turn_loop.rsharness/src/types/turn.rsharness/tests/golden/schemas/harness.react.jsonharness/tests/golden/schemas/harness.send.jsonharness/tests/golden/schemas/harness.spawn.jsonharness/tests/golden/schemas/harness.status.jsonprovider-anthropic/prompts/identity.txtprovider-kimi/prompts/identity.txtprovider-llamacpp/prompts/identity.txtprovider-openai-codex/prompts/identity.txtprovider-openai/prompts/identity.txtprovider-xai/prompts/identity.txtprovider-zai/prompts/identity.txtslack/src/functions/bindings.rsslack/src/types.rsslack/tests/golden/schemas/slack.on-turn-completed.jsontelegram-bot/src/functions/bindings/turn_completed.rstelegram-bot/src/types.rstelegram-bot/tests/golden/schemas/telegram-bot.on-turn-completed.jsonworkflow/src/reconcile.rs
💤 Files with no reviewable changes (2)
- harness/prompts/cli.txt
- harness/tests/golden/schemas/harness.spawn.json
In-turn harness::spawn no longer parks the parent: it seeds the child and
returns {child_session_id, child_turn_id} immediately as a Done checkpoint
(child ids retained for the fan-out guard, status, and the stop cascade).
The child's result is never delivered upward — consumers observe it only
through registered triggers and the state the child writes. ParentLink and
console linkage are kept for event filters and nesting.
- subagent: spawn_pending -> spawn_from_turn (same guards, no pending
plumbing) + spawned_result with an explicit fire-and-forget hint line
- deferred: approval-released spawns route through the same guarded helper
instead of the generic dispatcher (they previously lost ParentLink,
depth/fan-out guards, and policy subsetting); resolve_parent + sweep kept
so legacy pre-deploy parked turns still resolve
- types/turn: live_children -> spawned_children (any state with child ids);
fan-out becomes a per-turn spawn total
- spawn: SpawnOptions.pending_timeout_ms removed; golden re-blessed
- events: harness::turn-completed gains terminal:bool — false while the
session owns an armed wake (a one-shot notify subscription), so consumers
can finalize a logical exchange only on the terminal turn
A one-way run's first turn completes as a wiring ack; the real outcome arrives on a later notification-woken turn in the same session. Consumers that treated the first turn-completed as final now finalize on the `terminal` flag: - harness::status gains `expects_wake` (mirrors `!terminal`) so pollers can see a completed-but-continuing session - console: keep the stream open past non-terminal completions (each still closes its assistant bubble); legacy flag-less events stay terminal - telegram: flush the stream but skip the outcome toast, the FIFO drain, and the active-turn release until the terminal turn - slack: flush the stream; hold outcome reporting for the terminal turn - workflow: reconcile nodes by session terminal state instead of the exact initial turn id (which would have skipped woken turns forever); node stays Running while `expects_wake` holds
Every spawned child (in-turn, react-fired, direct/CLI) now gets the
embedded minimal prompts/subagent.txt identity instead of the router-served
orchestrator prompt: do the ONE task, write the named state destination as
an {ok, value | error} envelope before replying, stop. No knowledge of
larger goals, siblings, or callers — by design. Spawn options.system_prompt
(+ override strategy) remains the escape hatch; harness::send main agents
keep the orchestrator identity.
Dumbness is enforced, not just prompted: children lose harness::spawn and
engine::register/unregister_trigger via a default deny-set applied to every
seeded child. An exact id in the requested allow list re-grants; a glob
never does. The parentless read-mostly baseline also drops trigger
registration — children are leaves.
Rewrite the orchestrator doctrine in the harness fallback prompt and all
seven provider identity variants (4 unique bodies: terse
anthropic/llamacpp/zai, prose openai/xai, kimi, codex — md5 groups
preserved):
- delete the parked-spawn YES/NO decision rule; harness::spawn never waits
and its result never returns
- new doctrine: wire consumers FIRST (state triggers / turn-completed
edges & joins), spawn fully self-contained child tasks (exact inputs,
exact state scope+key, {ok, value | error} envelope, zero context about
the goal or siblings), then end the turn — notifications wake you
- new "Finishing: the turn-complete key" pattern: run-scoped one-shot
notify on turn_complete + a validator reaction pinned to its own session
(continue_on_error, always writes done:true/false) + a one-shot cron
deadline; compose the final answer FROM STATE on the wake
- children documented as policy-denied orchestration (leaves)
- final-checklist items for consumers-before-producers, self-contained
tasks, and the finish path
Also: delete dead prompts/cli.txt (zero references), update README config/
prompt/trigger-table wording, and the send.rs inject doc comment.
New guard transform fp::when { value, path?, op, to? } — ops ==, !=, >, >=,
<, <=, exists, not_empty; path defaults to the whole value; a pointer miss
FAILS the guard instead of erroring ("not there yet" is what guards are
for); exists counts a stored null as a hit (getOr rule), not_empty doesn't.
Direct calls return { passed, value }.
As a fp::pipe step the guard is dispatched by the pipe loop itself: a pass
threads the ORIGINAL value onward unchanged; a fail stops the pipe and the
response carries short_circuited: true — so a trailing write step (e.g.
state::set) runs only when the condition holds. This is the primitive for
zero-token mechanical validation pipes (react call mode lands next).
harness::react gains a second reaction mode: metadata.call
{ function_id, payload?, event_into? } (mutually exclusive with task)
dispatches a function with the fired event injected at event_into (default
/event) instead of spawning a sub-agent — deterministic, zero-token
reactions; with fp::pipe + fp::when this is the mechanical-validator
primitive. A completed join's downstream call receives the accumulated
{ results } map at the same pointer.
- registration (agent path): call.function_id is validated against the
REGISTRANT's dispatch policy — a call reaction can only target functions
the session could call itself — and harness-internal targets are refused
(self-dispatch loops); model inheritance/validation skipped in call mode
- fire path: no model check, no reactive-depth propagation (nothing
spawns; call chains stay bounded by the per-subscription fire-rate gate,
whose spec-hash now covers call specs); __once retire, continue_on_error
gating, fired records (target: "call") and reaction outcomes preserved
- ReactSpec.model/task become optional (exactly-one enforced by
validate_spec, task mode still requires a model); ReactResult gains
`called`; react golden re-blessed
…OT-4103)
default.txt and all seven provider identity variants (md5 groups preserved:
terse anthropic/llamacpp/zai, prose openai/xai, kimi, codex):
- Reacting to events: a reaction can be a FUNCTION CALL — metadata.call
{ function_id, payload?, event_into? } instead of task; event injected at
/event; policy-checked at registration, harness-internal targets refused;
join downstreams receive { results }; prefer call for mechanical work
(paired with fp::pipe / fp::when), task only for judgment
- Finishing: validator KIND picked by the rule — MECHANICAL (call edge
running fp::pipe: database::query -> fp::get -> fp::when -> state::set
turn_complete; zero tokens, standing once:false edge passes exactly once
at the threshold) vs JUDGMENT (the pinned-session LLM validator, kept)
…fy (MOT-4103) Organic live test caught the doctrine gap: with call-mode salient, an agent wired its own turn-complete wake AND its cron deadline as react->call (state::get into the void) — the pipe validator fired perfectly but nothing woke the parent and the run stranded. Add the hard rule to default.txt and all seven identity variants, in both the call-mode paragraph and the finishing wake step: a call's result is discarded and never lands in any session; anything that must WAKE you is ALWAYS a plain notify (no function_id), never a react/call edge.
…k (MOT-4103) The reaction outcome record's new status "called" (react call mode) fell through entry-mapper's label chain to "reaction failed" while its tone stayed info — the chat showed "REACTION FAILED — Reaction dispatched fp::pipe." for a successful dispatch. Map "called" explicitly.
Live edge test (6-wide fan-out vs cap 5): the guard's bare "5 children spawned this turn at or above max_children 5" taught no recovery, and the model invented an escape hatch (state->react edge fired by a state write) to launch the sixth child. It worked, but a mechanical error should name the sanctioned paths: consolidate into running children, or spawn the remainder from a later turn — the cap is per turn.
Live edge test: a 6-wide fan-out ("one worker per planet") is a mundane
ask that hit the cap of 5 and forced a react-edge workaround. 8 covers the
ordinary case; the per-turn guard still stops runaways.
…dening) Live edge test caught a policy escalation: children granted only the "read-only" database::query ran raw INSERTs through it — the query/execute split was response-shape only, unenforced. database::query is the surface narrowed agent policies grant, so enforcement must be server/driver-side: - sqlite: sqlite3_stmt_readonly on the prepared statement (authoritative — also catches data-modifying CTEs and writing PRAGMAs) - postgres: run inside a read-only transaction (writes fail with 25006); rolled back on timeout, committed after rows are collected - mysql: SET TRANSACTION READ ONLY before the statement (consumed by its implicit transaction; pooled conn reverts afterwards) Known limitation: transactionQuery inside an explicitly-begun pg/mysql transaction still trusts the caller (the transaction surface is a separate, rarely-granted capability); sqlite's is enforceable the same way in a follow-up.
…ing) Live reminder test: the model armed a cron notify without once:true. Cron is the one trigger type that defaults to RECURRING, so the "one-shot" deadline would fire every period forever (the model had to hand-unregister it) and is not tracked as the session's armed wake, misreporting the ack turn as terminal. Spell it out in the deadline step of default.txt and all seven identity variants.
…session (MOT-4103 hardening) Live edge test: steering a running one-way run with a plain follow-up message failed with a raw serde "missing field `model`". In the reactive model sessions live across wakes, so a nudge should not have to re-name the model every time: `model` is now optional — an existing session inherits its last turn's model (and provider, unless overridden), the same rule the notification inject path already uses; a NEW session still requires it, with a teachable error either way. Send golden re-blessed.
… JSON (MOT-4103 hardening)
Live corrective-loop run (loop1c): one retry child wrote its result envelope
as a string of serialized JSON ("{\"ok\":true,...}") instead of an object.
LLM validators shrug it off but pointer-based consumers (fp::when /ok) miss
and mechanical pipes fail the item. One explicit right/wrong line in the
deliverable section closes the gap.
… fallback only (MOT-4103 hardening) Live A/B: the loop1c run wired cron-only wakes and every corrective round stalled to the next 10-minute boundary (~30 min for a 3-round loop); the loop2 run armed the one-shot state notify on turn_complete and closed each round instantly. Model variance, same doctrine — the finish-path checklist now names the notify explicitly so cron-only wiring reads as a miss.
…03 hardening) Live L3 run: a zero-token retry machine (state trigger -> call-mode fp::pipe -> harness::spawn stage) silently stalled after one attempt — the pipe's spawn is parentless, inherits no model, and the bare "spawn requires a model" error died unread in the reaction record while the run's deadline masked the dead loop. The guard now says exactly which contexts inherit (in-turn only) and that reaction/pipe/CLI spawns must name model explicitly.
… (MOT-4103 hardening) Companion to the teachable spawn guard: the L3 retry machine (call-mode fp::pipe whose final step is harness::spawn) stalled because the pipe's spawn payload named no model — parentless dispatch inherits nothing and the failure lands only in the reaction record. One sentence in the mechanical- validator doctrine of all 8 prompt files; identity md5 groups stay 3/2/1/1.
…-4103 hardening)
Live L4 capstone: a bare mid-loop nudge ("add a 6th word") seeded a fresh
turn record that inherited the model (76edd89) but reset `functions` to
None — fail-closed deny-all — permanently disarming the run: every wake
after the steer reported "function dispatch is disabled" and the loop could
never absorb the change. A steer now inherits the prior turn's policy when
the send names none, mirroring the model rule; explicit `{ allow: [] }`
still strips. Send golden re-blessed (doc text only).
…n instead (MOT-4103 hardening) Live P2 run: models followed the pipe-spawn doctrine (2fd15c9) into fp's authority guard — "harness/turn-control functions are not supported in a pipe — steps run with WORKER authority, which would bypass the agent-level deny on them" — so every mechanical retry machine died at registration- correct fp behavior, wrong doctrine. All 8 prompt files now state the pipe's real surface (observe + write only) and the sanctioned mechanical retry: failures go to their own key and a task-mode react edge on that key IS the respawn, running with agent authority. Spawn guard error drops the impossible "call-mode pipe" context. Identity md5 groups stay 3/2/1/1.
…o` (MOT-4103 hardening)
Live P2b run: the model's 16-step all-ok pipe ended in state::set with a
literal value {done:true} and no `into` — the threaded guard output landed
at the default /value and silently replaced the literal, so turn_complete
held a bare boolean (the model caught it via a scratch-scope repro). The
injection now flags any replacement of a non-null literal on that step's
receipt, turning the silent clobber into a visible, teachable note.
…ch subfield (MOT-4103 hardening) Companion doctrine for the fp clobber note: one sentence in the mechanical- validator section of all 8 prompt files stating that the threaded value lands AT `into` (default /value) and replaces any literal there, so write steps must aim it at a scratch subfield. Identity md5 groups stay 3/2/1/1.
…wed (MOT-4103 hardening, security)
Live probe: fp::pipe { through: [{ function: "database::execute",
payload: { sql: "DELETE FROM t" } }] } wiped a row (affected_rows:1) —
pipe steps run with fp's WORKER authority, so a child capped at read-only
database::query plus fp::pipe could wrap execute/transaction/prepared-write
steps and defeat the query/execute read-only split (2e5d8d2) outright. Same
authority-laundering class fp already refuses for harness/session/config/
router. forbidden_step now allows only database::query and refuses every
other database::* with a teachable reason; database::query (the mechanical
validator's COUNT) stays supported. Test matrix + positive case added.
… calls (MOT-4103 hardening) The 4493032 doctrine said a pipe "can only observe and write (state/db/fp)", which read as "db writes are fine in a pipe". Live X3 run followed it: the model registered a done-check reaction ending in `database::execute` (insert a COMPLETE marker row). With the new fp guard (804c68f) that step is refused at fire time, so the mechanical marker never landed and the run limped to its result only via the cron-deadline fallback (~8 min instead of seconds). All 8 prompt files now state the pipe's real surface: it READS state/db-query/fp and writes ONLY state::set; a db marker row or respawn happens on the woken turn or a react edge, with agent authority. Identity md5 groups stay 3/2/1/1.
…job (MOT-4103 hardening)
Live scrapling run (scrap1-deadurl): asked to "retry unsucceeded URLs in
later rounds", the model wrote each child a LOOPING task ("fetch → on
failure sleep 5s, retry, up to 40 attempts, until a deadline") instead of
letting the parent respawn on a failure key. The dead-URL child then burned
~6 fetch attempts across many turns inside itself — a dumb-leaf violation,
and a child stuck in its own loop ignores the parent's deadline (it can only
compose a partial answer, never kill the stuck child). The doctrine taught
parent-driven retries only in the mechanical-validator section, never in the
task-writing step. Now the fan-out step and the child identity both state:
a child task is SINGLE-SHOT (do it once, write the envelope, stop); retrying
over rounds is the parent's failure-key respawn; a flaky call uses the
worker's own timeout/retries, not a hand-rolled loop. Identity md5 groups
stay 3/2/1/1.
Per product decision: by default a subagent has the SAME permissions as its main agent. Reverses the policy-level orchestration strip (6b61628): an in-turn child already inherited the parent's allow via subset_policy — the only divergence was deny_child_orchestration stripping harness::spawn + engine::register_trigger/unregister_trigger unless re-granted. That strip is removed, so an in-turn child now mirrors the parent's policy exactly. subset_policy still forbids escalation (a child never exceeds its parent) and still honours an explicit narrower request (least privilege stays available). Dumbness moves from a capability wall to a prompt/data-flow property: the child identity and the orchestrator doctrine now say a child INHERITS the permission to spawn/register but is a LEAF by role and must not use it — retries/orchestration are the parent's job. Parentless (direct/CLI/ trigger-fired) spawns are unchanged: they have no parent to mirror and keep the read-only baseline, granted up explicitly. policy.rs: drop CHILD_ORCHESTRATION_DENY + deny_child_orchestration and their test; add children_inherit_parent_policy_including_orchestration (parity + no-escalation + parentless deny-all). All 9 prompt files reframed; identity md5 groups stay 3/2/1/1. cargo test -p harness green, clippy clean.
… it (MOT-4103 hardening) Live 3-level-tree test after the permission-parity change: an L2 child at depth 2 held allow:["*"] with empty deny (full parity) and 493 free turns, yet REFUSED to spawn L3 — "FAILED: harness::spawn is denied by policy" — because subagent.txt said "you inherited the permission, ignore it, never spawn". The parity change gave children the capability; the doctrine still categorically forbade using it, so a deliberate coordinator task (the only way to build a sub-tree) fails at the second hop. subagent.txt now carves the exception: no orchestration on your OWN initiative, but when the TASK explicitly tells you to spawn/register, you inherited the permission — do it, and hand your child the same single-shot discipline. default.txt + the 7 identities soften "children never orchestrate" to "leaf by default, may coordinate when explicitly tasked; flat is usually better, hierarchy when the work needs it, bounded by the spawn-depth limit." Re-ran: chain completes to depth 3, every level inherits allow:["*"]. Identity md5 groups stay 3/2/1/1.
… (MOT-4103 hardening) Live 3-level aggregation tree (treeC): the tree BUILT perfectly — 15 agents, depth 3, every level inheriting allow:["*"] — but the numbers came out wrong. District coordinators tried to SUM two store values inside a token-free fp::pipe (state::get store1 -> ... -> state::get store2 -> state::set), but fp has no arithmetic transform, so the pipe just threaded the LAST store, not the total: district-A1 = 20 (store A1b) instead of 30 (A1a+A1b). Worse, the validators were one-shot watchers on ONE store key, so they fired before the sibling landed, short-circuited, and stranded — region totals never wrote. Both bugs are the "mechanical validator" framing inviting aggregation into a pipe. The doctrine now states plainly: a pipe GATES and moves ONE value; it cannot compute across several (fp has no sum/avg/reduce). To aggregate, gate on all inputs PRESENT (a join, or a count) which WAKES the coordinator to compute the total in its turn; never gate a multi-input condition with a one-shot single-key watcher — use a join that accumulates. default.txt + 7 identities; md5 groups stay 3/2/1/1. All prompt suites green.
…or grant the join task (MOT-4103 hardening) A/B re-run of the aggregation tree on the fc1d7ff doctrine (treeCb): stores all wrote, but every district subtotal silently never appeared and the tree stalled. Root cause: the coordinator gated with a join whose DOWNSTREAM TASK computed the sum — but a trigger-fired downstream is PARENTLESS, so it starts from the read-only baseline and its state::set is denied ("FAILED: state::set is denied by policy"). The permission-parity change only covers IN-TURN children; a react/join downstream inherits nothing (the deliberate leave-read-only choice). fc1d7ff already said "wake the coordinator via a plain notify so the sum runs in its own permissioned turn" — this makes the trap explicit: do NOT put the compute in a join downstream without granting metadata.options write access; prefer waking yourself. default.txt + 7 identities; md5 groups stay 3/2/1/1. Prompt suites green.
…before extras (MOT-4103 hardening) Both fixes come from a live end-to-end verification (200-file security-review prompt: git worker -> state -> db -> parallel sub-agents -> loop-until-count -> UI). Two orchestration failures stalled it and needed manual rescue: 1. TABLE-NAME MISMATCH: the root created tables `vulnerabilities` / `dc_mcp_security_findings` but every batch child wrote to `findings`, so the loop-gate counted an empty table and could never terminate. Doctrine now: pin the shared destination (state key or db table) ONCE and use that IDENTICAL string in every child task AND the gate — never a second name. 2. HALF-DONE STALL: the root spawned one wave (5, the max_children cap), then wandered into building the UI, chased non-existent docs, and never spawned the remaining 8 batches — frozen at 80/205. Doctrine now: a fan-out larger than the per-turn cap is a WAVE — record the work-list + cursor in the run scope and spawn the next wave on each completion-wake until ALL dispatched; and do NOT start a big downstream deliverable (UI, report) until the loop is finished and the gate is satisfied. Finish the loop, then build extras. default.txt + 7 identities; md5 groups stay 3/2/1/1. All prompt suites green.
…ompletions (MOT-4103 hardening) Scaled A/B of the previous wave-dispatch fix (12 items, one child each, 3 waves past the max_children:5 cap): the run drove correctly to 10/12 across waves — canonical single table, no interleaving — then DEADLOCKED. Root cause in my own 5c8a5da wording: "on each completion-wake spawn the next wave" told the model to pace dispatch on child-turn-completed events, but those stop firing once a wave's children all finish (and are rate-capped ~10/min), so the last wave (s11,s12) was never dispatched and no wake ever came — the model even claimed a cron it never armed. Doctrine now: after spawning a wave, arm a RELIABLE dispatch-wake — a short recurring cron or a self-notify re-armed each turn — and never pace dispatch on child completions. default.txt + 7 identities; md5 groups stay 3/2/1/1. All prompt suites green.
…n-out turns (MOT-4103 hardening) Two live findings against b6bf960's wave doctrine, one autonomous A/B each: 1. Stacked dispatch-crons: offered "a recurring cron OR a self-notify you re-arm", a run armed a FRESH every-minute cron per wave (crons 0->3) and left them all firing — 62 turns for a 12-item fan-out. Doctrine now: arm ONE dispatch-wake, ONCE, for the whole run; never a fresh cron per wave. 2. My first cut of the fix said "unregister the dispatch-cron in the same turn you dispatch the final wave — the turn_complete gate notify wakes you next". A run that had skipped the gate (pacing purely on one-shot poll-wakes) obeyed and dropped its ONLY wake: 12/12 rows written, no one left to collect them, run stranded (needed a manual nudge). Replaced the unconditional unregister with an END-OF-TURN INVARIANT: while children are in flight or the gate hasn't passed, at least ONE armed wake must survive the turn — gate notify, dispatch-wake, or deadline. Verified live (claude-sonnet-5, 12 snippets, cap 5 -> 3 waves, zero nudges): peak simultaneous dispatch-crons 1 (was 3), gate + deadline + single dispatch-wake all armed before wave 1, dispatch-wake dropped only at cursor exhaustion with gate still standing, deadline dropped only after the gate passed, 12/12 + UI in 25 turns / 8.7 min (was 62 turns). default.txt + 7 identities; md5 groups stay 3/2/1/1. All prompt suites green.
d57f953 to
5074b74
Compare
skill-check — worker0 verified, 46 skipped (no docs/).
Four for four. Nicely done. |
…patches The released-SPAWN branch called spawn_from_turn while the checkpoint was still Pending, only persisting Done after session.append. A crash or redelivered resolve in that window re-entered the branch (the Pending check at the top of resolve) and seeded a SECOND child for the same call — the fan-out guard can't catch it because the first child's ids were never persisted. Mirror the sibling non-spawn release path: mark the checkpoint Triggered and put_turn BEFORE dispatching, so a redelivery is a no-op. Trades the duplicate spawn for the same recoverable stuck-at-Triggered tradeoff the sibling path already accepts. Found by CodeRabbit on PR #540.
`{"value": {}}` is the documented padding for threaded steps, but the
receipt note treated any non-null value at the pointer as an authored
literal — so every padded step warned "piped value REPLACED the literal".
Count only non-empty objects/arrays (and scalars) as occupied.
Found by CodeRabbit on PR #540.
Summary
Reworks sub-agent orchestration into one-way reactive flow (React-style): a parent hands work DOWN as self-contained spawn tasks, results flow back only through state and the triggers registered to consume them, and a validator-written
turn_completekey tells the top-level agent when to stop. Delegation never parks; approvals still can.Covers MOT-4102 (fire-and-forget spawn + terminal contract + child identity) and MOT-4103 (validator/call-mode reactions + doctrine hardening). Every behavioral commit was found and A/B-verified by adversarial runs against the live engine (
claude-sonnet-5).Core changes
Fire-and-forget spawn + terminal turn contract (
07b58270)harness::spawnreturns{child_session_id, child_turn_id}immediately and never parks; all guards (depth, fan-out, policy subsetting) andParentLinkseeding kept in a sharedspawn_from_turnhelper; approval-released spawns route through the same helper (previously lost guards + linkage).harness::turn-completedgains an additiveterminal: bool: a completing turn is non-terminal iff its session still owns an armed self-notify — so wiring-ack turns and corrective-loop turns don't finalize consumers.Consumer migration (
870e1277,e55f1147)terminal: true; non-terminal completions render as progress. Workflow correlates steps by session instead of the initial turn id.Child identity & policy (
6b616280, then superseded by544be27c)subagent.txtidentity: single task, single shot,{ok, value, error}envelope written to the named state destination before the final reply.harness::spawn/ trigger registration — and can never escalate past it. Parentless spawns (direct/CLI/trigger-fired) start from the read-onlydefault_functionsbaseline. A child coordinates only when its task explicitly directs it (4edfa941).Mechanical validation: call-mode reactions + fp pipes (
f33f2121,87056a5c,1cb4e656)harness::reactcall mode: an event fires a FUNCTION (not an agent turn) — e.g.database::queryCOUNT →fp::filter/fp::size→fp::when→state::set turn_complete. Loops close without burning model turns.fp::whenguard + pipe short-circuit; pipe step receipts note when a piped value clobbers a literal (e3dfd534).Security hardening (found by live probing)
database::queryenforces read-only SQL (2e5d8d22).fp::pipesteps run with worker authority, which let a pipe smuggledatabase::executepast an agent-level deny — pipes now forbid alldatabase::*exceptdatabase::query(804c68ff). Hit organically by a model mid-run before the fix; refused after.Harness fixes from live failures
76edd89b,d859534f).fb2f1279,9a06cb22);default_max_children5 → 8 (18dbb45c).Orchestrator doctrine (
51a99bfd+ the hardening tail,default.txt+ 7 provider identities, md5 groups kept 3/2/1/1)Each rule below exists because a live run broke without it:
5c8a5da8).fc1d7ff3,cd511c22); pipes read db, write only state (555b149d); pipes cannot spawn (44930325,2fd15c9a).b6bf9609,0c8f529b).once: true(b6909933); wakes must be plain notifies, never react/call (10b49dd4); subagent envelope is a JSON object, not serialized JSON (0b2533c2); child tasks are single-shot — retries are the parent's job (9513ad76).Verification
cargo testgreen across harness (incl. new spawn/terminal/policy tests + re-blessed goldens), all 7 providers, console/web, telegram-bot, slack, workflow. Identity md5 invariant3/2/1/1holds.804c68ff.Follow-ups (not this PR)
resolve_parent, the non-held sweep arm, anddefault_pending_timeout_msonce no pre-deploy parked turns remain.Summary by CodeRabbit
New Features
fp::whenguards to control pipeline execution.harness::react.Bug Fixes