Repository navigation
feat(subagent): unified timeout/retry/budget policy, result policy, roles - #327
Conversation
Add tests for token budget enforcement after the child returns, mapping of budget onto harness budget limits including cost, and call caps that only tighten a run config. Also cover the default policy of not retrying after tool calls. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
SubAgentBudget now carries input/output token and cost caps alongside the existing call caps, with builder methods, a helper that tightens a RunConfig's call caps, and a conversion to the harness BudgetLimits so a host can enforce cost mid-run. Token caps are checked after the child returns, and a new retry_after_tool_calls policy flag lets orchestration paths opt into retrying attempts that already ran tools. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add unit tests covering runtime library behaviour and the subagent result policy and role logic, extending test coverage for these modules. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Subagent roles now carry a configurable result policy that controls how their output is handled. This lets callers choose between returning the raw result and other handling modes instead of relying on a single hardcoded behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
PreparedSubagent now carries a delegation role, inherited tool ceiling, timeout/retry/budget policy, result policy, and an optional per-attempt context factory, with builder methods and a new constructor for the default plan. SubagentIncomplete gains a typed IncompleteKind so consumers no longer parse reason strings, SubagentOutcome records an optional schema error, and SubagentError gains a Transient variant that adapters use to mark retryable failures. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds the Cargo manifest for the new tinyagents-orchestration crate so it can participate in the workspace build. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…error Replace struct literals with the new PreparedSubagent::new and SubagentIncomplete::new constructors, and add the schema_error field to SubagentOutcome fixtures so the tests compile against the updated types. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add unit tests covering subagent module behaviour and runtime library entry points. These lock in current behaviour and give the new orchestration code a regression baseline. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Register a new test module for subagent driver policy and drop the unused RetriesExhausted variant from IncompleteKind, which no longer reflects how retry exhaustion is reported. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The driver now runs each subagent under a child cancellation token and retries transient failures according to the retry policy, rebuilding the plan from a captured template around a fresh context. Timeouts cancel only the child, and completed runs are checked against budget and result policies so over-budget or schema-invalid output is reported as incomplete rather than success. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The executor test double now records the cancellation token handed to each attempt instead of setting a flag when its own token fires, and the hanging behaviour waits forever rather than observing cancellation itself. The timeout test checks the captured child token directly, so it verifies the driver cancels the child rather than merely that the executor noticed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The driver now tracks whether a run ended because of its own timeout and only treats the execution token as cancelled when the timeout did not fire. Previously an executor that cancelled the token during a timeout was misreported as a cancellation rather than a timeout. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The token binding was initialized with a clone that the loop immediately overwrote on its first iteration, so the initial value was never read. Declaring it uninitialized removes the dead clone while keeping the same behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The subagent invocation code has been reorganised into separate modules for jobs, types, and policy runs so each concern lives in its own file. No behaviour changes; this is purely a structural cleanup to make the invocation path easier to navigate. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Inline and background subagent runs now go through the shared attempt runner and result settlement path instead of calling the hosted child directly, so retry and result policies apply consistently across all invocation modes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test module covering the delegation tool exposure policy and tightens the lifetime bounds on delegation_tools_exposed so the helper can be exercised from the new tests. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The leaf-refusal test now registers the subagent jobs tool through the harness dispatch API instead of the removed register_subagent_job_tools helper, keeping the test aligned with the current registration path. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Subagent transcripts are now rendered as part of the session view, with supporting types added to the view module. This makes subagent activity visible alongside the main conversation instead of being hidden. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Split the incomplete-delegation status test into separate cases for the legacy text marker and the typed status payload, extracting a shared helper that returns the projected status. This pins the new typed incomplete result alongside the marker it replaces. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Expand the subagent READMEs to cover timeout, retry, budget, incomplete status, result policy and role framing across both the graph node and orchestration paths, and note the API additions these features introduced. The graph node README now also records that token caps are checked after the run while call caps tighten the child RunConfig, and that max_cost is only enforced by the harness BudgetMiddleware. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document the policy governing subagent behaviour in the harness module. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted imports, long expressions, and assertions to satisfy rustfmt line-width and ordering rules. No behaviour changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reordered the planner re-export to sit with the other module re-exports and reflowed the types re-export list to fit the line width. Also wrapped an overlong tracing call in the invocation module. No behaviour change. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a policy run path that drives subagent invocation through configurable jobs, with supporting types and tests. This lets callers express invocation behaviour as policy rather than hard-coded flow. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…hecks Retry backoff now races against cancellation so a cancelled job stops promptly instead of sleeping out the delay. Leaf misconfiguration is detected once at construction and reported with clearer guidance, retry attempts reuse the first child's config with derived ids instead of minting new ordinals, and truncation now accounts for the marker's own width. Incomplete-result detection only trusts harness payloads that name Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds tests for retry-after-tool-calls gating, child cancellation on timeout, background retry job linking, token budget overruns, artifact overflow without a store, and leaf-role refusal. Also tightens the truncation tests to assert the cap holds including the marker, and checks that foreign JSON lacking harness job keys is not treated as an incomplete subagent result. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Relax the assertions in the subagent policy tests so they no longer depend on the exact omitted-character count embedded in the truncation marker, matching only the stable "chars omitted" suffix. This keeps the tests valid when the marker's count formatting changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The harness subagent-policy page now points at the crate README's policy and breaking-changes sections instead of duplicating them, and the README gains retry notes plus a full list of the API and behaviour changes hosts must adapt to. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Silence the clippy lint on run_attempts since the function has a single internal call site per mode and a params struct would only rename the arguments without improving clarity. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
This review includes 29 billable files and costs up to $7.25. Or wait 45 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (29)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 9 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Resolved this pass
Before merge
How this fits togetherflowchart LR
n0["SubAgentBudget<br/>changed"]:::changed
n1["SubAgentOutput<br/>changed"]:::changed
n2["SubAgentPolicy<br/>changed"]:::changed
n3["...binding_keeps_host_capabilities_live_only"]:::impacted
n4["from_fns"]:::impacted
n5["AgentInvocationBinding"]:::impacted
n6["SubAgentNode"]:::impacted
n2 -->|uses| n0
n3 -->|uses| n1
n3 -->|calls| n4
n3 -->|tests| n4
n3 -->|uses| n5
n3 -->|uses| n6
n4 -->|uses| n1
n6 -->|uses| n2
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8630b0bd11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0644 · 1,362,671 in / 59,891 out · 170,118 cached (12%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0341 · 653,816 in / 36,570 out · 71,967 cached (11%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0264 · 512,945 in / 16,891 out · 57,831 cached (11%) · gpt-5.6-luna
tests: $0.0007 · 98,211 in / 3,249 out · 40,320 cached (41%) · glm-5.3-flash
description: $0.0004 · 46,631 in / 107 out · 0 cached (0%) · glm-5.3-flash
tinysweeper
Bot
added
the
priority: p1
Introduce a result policy that decides how subagent job outcomes are handled, along with tests covering the new behaviour. This centralises the decision logic that was previously implicit in the invocation path. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move the policy execution path into its own module so the job runner only handles scheduling and result plumbing. This keeps the policy logic separate from the invocation lifecycle and makes both easier to follow. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…aces Add tests asserting that call caps apply to every retry attempt, that an executor which cancels its own token is not retried, and that an over-budget run still has its result policy applied. Also cover the case where a budget overrun races an owner cancellation and must settle as cancelled rather than incomplete. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
… text The result policy tests now check the omitted character count and the truncated length rather than matching a human-readable marker string in the output, so the assertions no longer depend on the exact wording of the truncation notice. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8de6424dd2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| applied.omitted_chars = omitted; | ||
| if self.overflow == ResultOverflow::Artifact { | ||
| match &self.artifact_store { | ||
| Some(store) => match store.store(task_id, full).await { |
There was a problem hiding this comment.
When an artifact store blocks or becomes unavailable, this unbounded await prevents both invocation modes from settling: a background cancel_owned call only cancels the child token, which ArtifactStore::store cannot observe, so the job remains Running and retains its spawn slot indefinitely even after cancellation is acknowledged. Race policy application against the job cancellation token, as the execution and retry backoff paths already do.
Useful? React with 👍 / 👎.
| Err(error @ TinyAgentsError::LimitExceeded(_)) => { | ||
| entry.job.status = SubAgentJobStatus::Incomplete; | ||
| entry.job.incomplete_kind = Some(IncompleteKind::BudgetExceeded); | ||
| entry.job.error = Some(error.to_string()); |
There was a problem hiding this comment.
When cancel_owned races with a child returning TinyAgentsError::LimitExceeded (for example from the newly tightened call caps), cancellation_requested is already true but this new arm still publishes Incomplete(BudgetExceeded) rather than Cancelled, contradicting the cancellation acknowledgement. Although mark_budget_overrun now handles this race, this separate Finished::Failed path remains; check the cancellation flag before settling these error variants.
Useful? React with 👍 / 👎.
| config.run_id = RunId::new(spare_id.clone()); | ||
| config.thread_id = parent | ||
| .thread_id() | ||
| .map(|thread| child_thread_id(thread, &spare_id)); |
There was a problem hiding this comment.
When an attempt is retried, assigning a distinct child thread derived from the -aN run ID creates a separate persisted child transcript for every attempt. The session projector enumerates each child transcript independently and has no retry-grouping logic, while the spawn payload can name only one attempt, so a single delegation can render as multiple or misassociated subagent cards after a retry. Preserve a stable projection identity across attempts or explicitly collapse attempt transcripts in the view.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0127 · 666,599 in / 51,895 out · 51,752 cached (8%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0054 · 274,462 in / 27,083 out · 31,798 cached (12%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0061 · 241,529 in / 19,932 out · 19,954 cached (8%) · gpt-5.6-luna
tests: $0.0004 · 48,685 in / 1,013 out · 0 cached (0%) · glm-5.3-flash
description: $0.0004 · 48,741 in / 1,036 out · 0 cached (0%) · glm-5.3-flash
| Some(limit) => match tokio::time::timeout(limit, run).await { | ||
| Ok(result) => result, | ||
| Err(_) => { | ||
| cancel.cancel(); |
There was a problem hiding this comment.
Keep child timeouts from cancelling the parent
The cancel token is shared with the caller, so a timeout in one child attempt cancels the parent execution as well as stopping this loop. A child timing out should return the timeout failure and cancel only the child attempt; otherwise an otherwise-independent parent run and its sibling work are terminated whenever this policy timeout fires.
Additional security observation
Keep child timeouts from cancelling the parent
[RULE] cancellation-propagation
The timeout path cancels the token supplied by the caller. If this is the executor or parent token, timing out one child also cancels the parent operation and its sibling work. Use a child-scoped cancellation token for the hosted run, while observing the caller token separately.
[RULE] cancellation-scope ·
| error: &TinyAgentsError, | ||
| tools_ran: bool, | ||
| ) -> bool { | ||
| policy.retry.should_retry_error(attempt, error) && (!tools_ran || policy.retry_after_tool_calls) |
There was a problem hiding this comment.
Exclude timeout and cancellation failures from retries
The module contract says a timed-out attempt is never retried, but this predicate only consults the general retry policy and tool-call state. If a timeout or cancellation error is classified as retryable by should_retry_error (and no tools ran), this returns true and permits another attempt. The retry decision needs an explicit non-retryable check for timeout and cancellation, or the caller must prove those errors never reach this function; that caller-side exclusion is not enforced here.
[RULE] retry-cancellation ·
| entry.job.status = SubAgentJobStatus::Cancelled; | ||
| entry.job.error = Some(TinyAgentsError::Cancelled.to_string()); | ||
| } | ||
| Err(error @ TinyAgentsError::LimitExceeded(_)) => { |
There was a problem hiding this comment.
Honor cancellation before classifying limit errors
If the owner requests cancellation while the child is finishing and the child reports LimitExceeded (or the analogous Timeout branch immediately below), this unconditionally publishes Incomplete instead of Cancelled. mark_budget_overrun explicitly gives a raced owner cancellation precedence, so these new error branches should apply the same cancellation_requested check before classifying the result.
[RULE] cancellation-propagation ·
| policy: &SubAgentPolicy, | ||
| result_policy: &ResultPolicy, | ||
| ) -> SubagentOutcome { | ||
| if !matches!(outcome.status, SubagentStatus::Completed) { |
There was a problem hiding this comment.
Apply result policies to budget-exceeded incomplete outcomes
The documented behavior says an over-budget run has its output trimmed and schema-checked according to result_policy, but this early return skips all processing whenever the executor already reports an Incomplete status. A call-cap overrun is an incomplete budget-exceeded outcome, so its output and schema errors are left unprocessed, unlike a completed run that exceeds the token budget and is converted below. Restrict the early return to non-budget incompletions, or otherwise continue through result-policy application for IncompleteKind::BudgetExceeded.
[RULE] incomplete-outcome-processing ·
| }; | ||
| return match policy.budget.check(&measured, subagent.name()) { | ||
| Ok(()) => Finished::Run(run), | ||
| Err(error) => Finished::OverBudget { run, error }, |
There was a problem hiding this comment.
Honor cancellation before settling a budget overrun
A successful child run is converted to Finished::OverBudget without checking cancel. If cancellation arrives before this branch, the caller still proceeds to settle the produced output as a budget overrun instead of honoring cancellation. Check the cancellation token before returning the over-budget outcome.
Additional critique observation
Honor cancellation before settling a budget overrun
[RULE] cancellation-check
A successful child that exceeds its budget is converted directly into Finished::OverBudget without checking cancel. If cancellation arrives after the child completes but before the result is settled, the caller still processes and records the over-budget output instead of honoring the cancellation. Check the cancellation state before returning this terminal result, or carry the cancellation state into settlement.
[RULE] cancellation-race ·
| ..AppliedResult::default() | ||
| } | ||
| }; | ||
| jobs.mark_budget_overrun(id, applied, error.to_string()); |
There was a problem hiding this comment.
Honor pending cancellation before marking a budget overrun
settle unconditionally records an over-budget result. Cancellation can arrive after the run finishes but before this asynchronous settlement completes, so the job may be published as a budget overrun even though cancellation is already pending. Pass the relevant cancellation token into settlement and check it before calling mark_budget_overrun.
Additional critique observation
Honor pending cancellation before marking a budget overrun
[RULE] cancellation-check
settle unconditionally records an over-budget result. Because this function has no cancellation check or token, a cancellation already pending when settlement begins is ignored and the job is marked as a budget overrun. The cancellation state must be checked before applying the result policy and marking the job, or settlement must receive the relevant cancellation token.
[RULE] cancellation-race ·
1 participant
Summary
Subagent policy work, items D5, D6 and D9 of the harness uplift plan (a comparison with pi and OpenClaw).
D6: one policy for every delegation path
SubAgentPolicy(timeout, retry, budget) used to apply only to the graphSubAgentNode. It is now re-exported from orchestration and applied inSubagentDriverand inSubAgentTool, in both inline and background mode.Timeouts. A timeout cancels the child's own token, never the caller's. The child ends
Incomplete(Timeout)with its output kept, and is never retried.Retries.
retry_after_tool_callsis set. The tool path detects "tools ran" through the attempt's event sink, and grandchild tool calls count too.{first}-a{n}, so turning on a retry policy never shifts the child ids of other subagents in the thread.Budgets.
RunConfig.Incomplete(BudgetExceeded).max_costis carried but not enforced; this is documented.D5: result policy and typed incomplete
Result size.
ResultPolicy { max_chars, overflow: Truncate | Artifact, schema, artifact_store }. The default leaves results unchanged.max_chars.ArtifactStoreand returns a preview plus a path-freeArtifactReference. A store failure is surfaced asartifact_error.Schema check. Optional, via
tinyinference_llm::tool::validate_json_value. A mismatch setsschema_errorbut never fails the run.Typed incomplete status.
IncompleteKindis new.SubAgentJobStatus::Incompleteand the session view'sSubagentStatus::Incompleteare new variants.[SUBAGENT_INCOMPLETE]marker.D9: roles and framing
Roles.
SubagentRole { Orchestrator (default), Leaf }.restrict_toolsstrips delegation tools from a leaf, and intersects with the ceiling so a child never widens the tool set it inherits.SubAgentToolrefuses to spawn. Its message tells the model to do the work itself, and it warns at construction when misconfigured.Framing.
subagent_framing(role, depth, task)is an optional preamble that hosts can prepend.Breaking changes
These are listed in full in the orchestration README's "Breaking changes" section:
PreparedSubagent,SubagentOutcome,SubagentIncomplete,SubAgentBudgetandSubAgentJob.SubAgentBudgetno longer derivesEq.LimitExceededorTimeoutnow endIncompleterather thanFailed.Incomplete.Host follow-up (OpenHuman bump)
PreparedSubagent/SubagentOutcomeliterals insubagent_host/lifecycle.rsneed the new fields.app/src/types/derivedTranscript.tsandmapDisplayItems.tsneed anincompletestatus. Otherwise these runs display as successful.Commit history note
The automatic checkpoint hook wrote most of these commits. The history is kept unsquashed on purpose; this description is the authoritative summary.
Tests
artifact_error.cargo test --workspacepasses. The one exception isworkspace::git::validate_repo_root_rejects_non_repo, which fails only when TMPDIR points inside a git checkout.-D warnings(default and all features) and fmt are clean.Co-authored-by: Medulla medulla@tinyhumans.ai