fix(delegate): bound delegated sessions by default by ramparte · Pull Request #298 · microsoft/amplifier-foundation

Sam Schillace (ramparte) · 2026-08-07T19:36:27Z

Summary

Real-world runaway delegated sessions have reached 17h34m / 8,036 calls and 3h49m / 992 calls. This adds a delegate-owned circuit breaker without exposing or depending on any private session data.

Default settings.timeout to 1800 seconds for both spawn and resume; an explicit null remains the opt-out for unbounded delegation.
Validate timeout configuration eagerly as a positive, finite, non-boolean number that the event loop can represent.
Enforce hard parent-release semantics: at the deadline, cancel the child and return the timeout result immediately rather than waiting for cancellation handling or slow persistence/cleanup to finish.
Retain detached child tasks strongly until completion, consume their terminal results, and release them without "task destroyed" or unobserved-exception leakage.
Report recovery state honestly as status: timed_out, resumable: false, and resume_status: pending_child_cleanup; the response never claims that interrupted-session persistence has completed.
Emit delegate:error with error_type: delegate_timeout, not delegate:agent_completed, while preserving ordinary external-cancellation semantics.
Return the child session ID and agent identity when available so recovery can be correlated after cleanup completes.
Document the default and explicit-null opt-out, add focused validation/deadline/detached-task/error-semantics coverage, and include module tests in default pytest collection.
Ignore .next/ working-session artifacts.

Coordinated prerequisite

This rollout depends on microsoft/amplifier-app-cli#260, which persists interrupted child sessions and guarantees shielded cleanup after the Foundation layer releases the parent at its deadline. The pending-cleanup response is deliberately conservative until that app-layer work completes.

Verification

30 focused timeout/circuit-breaker tests passed.
90 tool-delegate module tests passed.
1,639 full-suite tests passed with one pre-existing warning.
Ruff check clean.
Ruff format check clean.
Pyright clean.

Brian Krabach (bkrabach) · 2026-08-27T14:25:53Z

Holding this for a revision rather than merging or closing.

The machinery here is correct and is being kept verbatim — _await_child_with_deadline's hard parent release, strong retention of detached tasks with terminal-result consumption, and the honest resumable: false / pending_child_cleanup reporting are all good, and the 467-line test file stays.

Two problems with it as the primary bound:

1800s sits inside our measured healthy distribution. Legitimate sub-sessions measure 996–1168s; ollama/local runs at ~1.0x that margin. This default would fire on real work.
At the deadline the parent gets no partial result. The session is marked resumable: false and the prerequisite that would fix that (fix(session): persist interrupted child sessions amplifier-app-cli#260) is still open, so the work is unrecoverable. Separately, a child that suppresses CancelledError keeps running after detach — this bounds parent waiting, not child spend.

The revision puts an LLM-call budget in front of it (default 300 calls per session leg, enforced in the orchestrator loop, exiting on the child's own turn boundary with a complete transcript and status: budget_exhausted, resumable: true) and moves this timeout to a 14400s backstop that only fires when that first layer did not apply.

Net change to this diff: 1800 → 14400 plus the docs text. Everything else is preserved.

…terations (Layer 1) Layered Bounding for Delegated Sessions (spec: 298-replacement, replacing the wall-clock-only default in #298). Adds a per-session-leg LLM-call budget as the first line of defense in front of the delegate's existing settings.timeout wall-clock backstop, delivered with zero new kernel surface: tool-delegate writes max_iterations (and a new budget_warn_ratio) into the orchestrator_config dict it already passes to spawn_fn, and amplifier-app-cli's session_spawner already does a caller-wins .update() into the child's config -- zero app-cli changes needed. Enforcement itself lives in the orchestrator loop (see companion PR microsoft/amplifier-module-loop-streaming#43), which already counts LLM calls via max_iterations and already exits exhaustion via a normal return (graceful wrap-up), so the resulting transcript is complete and resumable -- unlike a cancellation-based timeout. Ships DARK: settings.max_llm_calls defaults to None, so no budget is injected into any child session and orchestrator_config is byte-for-byte what it was before this change. Nothing here changes behavior until an operator explicitly sets settings.max_llm_calls. Precedence chain (highest first): 1. Per-call tool input (`max_llm_calls`) -- implemented 2. Per-agent frontmatter (`agents[name]["budget"]["max_llm_calls"]`) -- NOT implemented, see below 3. This module's settings.max_llm_calls (default None) -- implemented 4. Inherited parent orchestrator_config's max_iterations -- implemented (the pre-existing inheritance path, left untouched when no budget applies) Per-agent frontmatter override (rank 2) does not ship: verified empirically (not just read from source) that a top-level `budget:` block in an agent .md's frontmatter is dropped by amplifier_foundation.bundle._dataclass._load_agent_file_metadata, which only forwards a fixed allowlist of top-level keys (tools, providers, hooks, session, provider_preferences, model_role, agents) -- budget is not among them. Reproduced in tests/test_delegate_call_budget.py::test_agent_frontmatter_budget_key_is_dropped. Ranks 1, 3, and 4 ship; rank 2 is a follow-up requiring a change to the frontmatter loader itself, documented in this module's README "Known gaps" section. Also adds: - Eager validation (_validate_call_budget / _check_call_budget_type): reject bool, non-int, and negative values at the point supplied (module construction for the settings default, execute() for the per-call override) -- never at spawn time. - Negotiated-feature warning (spec §4.4): if a budget was requested but the child's orchestrator reports no llm_call_budget telemetry (e.g. a third-party orchestrator with no max_iterations support), logs a warning and sets metadata.budget_enforced = false on the returned ToolResult, so the gap is loud rather than silent. - max_llm_calls entry in the tool's input schema (kept a pure literal for the static token-cost estimator). Files: - modules/tool-delegate/amplifier_module_tool_delegate/__init__.py: _check_call_budget_type / _validate_call_budget module functions; settings.max_llm_calls / budget_warn_ratio in __init__; per-call max_llm_calls parsing + validation in execute(); _resolve_call_budget method; orchestrator_config build (copy-not-mutate + budget injection) and negotiated-feature warning in _spawn_new_session; max_llm_calls schema entry - modules/tool-delegate/README.md: "Layer 1 call budget" section + "Known gaps" - modules/tool-delegate/tests/test_delegate_call_budget.py (new): T2.1, T2.2, T2.4, T2.5, T2.6, T2.7, T2.8, T2.9, T2.10, T2.11 + the frontmatter round-trip verification test (14 tests) Testing: - New tests: 14 passed - Full tool-delegate module suite: 80 passed (was ~66; zero regressions) - Full foundation repo suite (tests/): 1634 passed, 1 skipped -- matches pre-change baseline exactly - ruff/pyright: no new issues vs baseline Part of the 298-replacement design (Layer 1 of 3). Companion PR: microsoft/amplifier-module-loop-streaming#43 (orchestrator-side enforcement). #298 is being revised separately to reframe its wall-clock default as the Layer 3 backstop behind this budget. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>

…terations (Layer 1) (#325) Layered Bounding for Delegated Sessions (spec: 298-replacement, replacing the wall-clock-only default in #298). Adds a per-session-leg LLM-call budget as the first line of defense in front of the delegate's existing settings.timeout wall-clock backstop, delivered with zero new kernel surface: tool-delegate writes max_iterations (and a new budget_warn_ratio) into the orchestrator_config dict it already passes to spawn_fn, and amplifier-app-cli's session_spawner already does a caller-wins .update() into the child's config -- zero app-cli changes needed. Enforcement itself lives in the orchestrator loop (see companion PR microsoft/amplifier-module-loop-streaming#43), which already counts LLM calls via max_iterations and already exits exhaustion via a normal return (graceful wrap-up), so the resulting transcript is complete and resumable -- unlike a cancellation-based timeout. Ships DARK: settings.max_llm_calls defaults to None, so no budget is injected into any child session and orchestrator_config is byte-for-byte what it was before this change. Nothing here changes behavior until an operator explicitly sets settings.max_llm_calls. Precedence chain (highest first): 1. Per-call tool input (`max_llm_calls`) -- implemented 2. Per-agent frontmatter (`agents[name]["budget"]["max_llm_calls"]`) -- NOT implemented, see below 3. This module's settings.max_llm_calls (default None) -- implemented 4. Inherited parent orchestrator_config's max_iterations -- implemented (the pre-existing inheritance path, left untouched when no budget applies) Per-agent frontmatter override (rank 2) does not ship: verified empirically (not just read from source) that a top-level `budget:` block in an agent .md's frontmatter is dropped by amplifier_foundation.bundle._dataclass._load_agent_file_metadata, which only forwards a fixed allowlist of top-level keys (tools, providers, hooks, session, provider_preferences, model_role, agents) -- budget is not among them. Reproduced in tests/test_delegate_call_budget.py::test_agent_frontmatter_budget_key_is_dropped. Ranks 1, 3, and 4 ship; rank 2 is a follow-up requiring a change to the frontmatter loader itself, documented in this module's README "Known gaps" section. Also adds: - Eager validation (_validate_call_budget / _check_call_budget_type): reject bool, non-int, and negative values at the point supplied (module construction for the settings default, execute() for the per-call override) -- never at spawn time. - Negotiated-feature warning (spec §4.4): if a budget was requested but the child's orchestrator reports no llm_call_budget telemetry (e.g. a third-party orchestrator with no max_iterations support), logs a warning and sets metadata.budget_enforced = false on the returned ToolResult, so the gap is loud rather than silent. - max_llm_calls entry in the tool's input schema (kept a pure literal for the static token-cost estimator). Files: - modules/tool-delegate/amplifier_module_tool_delegate/__init__.py: _check_call_budget_type / _validate_call_budget module functions; settings.max_llm_calls / budget_warn_ratio in __init__; per-call max_llm_calls parsing + validation in execute(); _resolve_call_budget method; orchestrator_config build (copy-not-mutate + budget injection) and negotiated-feature warning in _spawn_new_session; max_llm_calls schema entry - modules/tool-delegate/README.md: "Layer 1 call budget" section + "Known gaps" - modules/tool-delegate/tests/test_delegate_call_budget.py (new): T2.1, T2.2, T2.4, T2.5, T2.6, T2.7, T2.8, T2.9, T2.10, T2.11 + the frontmatter round-trip verification test (14 tests) Testing: - New tests: 14 passed - Full tool-delegate module suite: 80 passed (was ~66; zero regressions) - Full foundation repo suite (tests/): 1634 passed, 1 skipped -- matches pre-change baseline exactly - ruff/pyright: no new issues vs baseline Part of the 298-replacement design (Layer 1 of 3). Companion PR: microsoft/amplifier-module-loop-streaming#43 (orchestrator-side enforcement). #298 is being revised separately to reframe its wall-clock default as the Layer 3 backstop behind this budget. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>

Reframes this PR's timeout as Layer 3 of the "Layered Bounding for Delegated Sessions" design (spec: 298-replacement) -- the orchestrator- independent wall-clock backstop that sits behind a per-leg LLM-call budget (Layer 1, see microsoft#325 and companion PR microsoft/amplifier-module-loop-streaming#43), not the primary bound. Net functional change: `1800` -> `14400` for `settings.timeout`'s default. Everything else in this PR is kept verbatim: `_DelegateTimeoutExpired`, `_validate_timeout`, `_await_child_with_deadline`, the hard parent-release semantics, the honest `resumable: false` / `resume_status: pending_child_cleanup` reporting, and all 30 focused timeout tests (only the default-value assertions are retargeted). Why 14400s: ~12x the measured healthy sub-session upper bound (996-1168s), and ~2x below the worst observed runaway (17h34m) -- generous enough that a working Layer 1 budget should make this backstop fire zero times in practice. If it ever fires with Layer 1 active, that's a Layer 1 bug report, not evidence this default is wrong. Docs updated to frame this as the backstop: module docstring, README's "Delegate Timeout" section (retitled "Layered bounding: call budget (Layer 1) + wall-clock backstop (Layer 3)"), and the settings.timeout config comment. Files: - modules/tool-delegate/amplifier_module_tool_delegate/__init__.py: module docstring `settings.timeout` description; default 1800 -> 14400 - modules/tool-delegate/README.md: "Delegate Timeout" section rewritten as "Layered bounding"; config example comment - modules/tool-delegate/tests/test_delegate_timeout.py: default-value assertions retargeted to 14400 (T3.1) Testing: - modules/tool-delegate/tests/test_delegate_timeout.py: 30 passed - Full tool-delegate module suite: 90 passed (zero regressions) - Full foundation repo suite (tests/, this branch's own base): 1549 passed - python_check: no new issues vs this branch's own baseline (pre-existing I001 import-sort warning unchanged, confirmed via stash diff) 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>

Brian Krabach (bkrabach) · 2026-08-28T15:57:25Z

Revision rebased onto current `main` — ready to merge

This branch has been rebased onto current main (was 27 commits behind) and re-validated end to end. Summary below.

Credit

The core insight and machinery here are Sam Schillace (@ramparte)'s. The measured production runaways (17h34m / 8,036 LLM calls, and 3h49m / 992 LLM calls) are what motivated a hard bound on delegated sessions in the first place, and _await_child_with_deadline / _cancel_and_detach_child / _consume_detached_child_result — the hard parent-release semantics, strong retention of detached tasks, and the honest status: timed_out / resumable: false reporting — are good, careful async engineering that is kept verbatim in this merge.

The adjudication (already applied to this branch prior to rebase)

Investigation found the machinery was right but the default was fighting the healthy population: 1800s sits inside the measured healthy sub-session distribution (996–1168s), so it risked firing on legitimate work (ollama/local sessions in particular run at ~1.0x that margin).

The resolution is a layered-bounding design:

Layer 1 — per-leg LLM-call budget, merged separately to main as feat(delegate): flag-gated per-leg call budget via orchestrator max_iterations (Layer 1) #325 (max_llm_calls via the child's own orchestrator max_iterations, off by default). This is the layer meant to actually catch a runaway agent: exhaustion is a normal turn ending, so the transcript stays complete and resumable.
Layer 2 — provider HTTP timeouts. Already shipped by every provider (120–600s); not implemented here.
Layer 3 — this PR's wall-clock backstop. Orchestrator-independent, intentionally generous. Default raised from 1800 → 14400 (4h): ~12x the measured healthy upper bound, ~2x below the worst observed runaway — generous enough that a working Layer 1 budget should make this backstop fire ~never in practice.

Everything else in the original PR — _DelegateTimeoutExpired, _validate_timeout, the cancellation/detach mechanics, and all 30 focused tests in test_delegate_timeout.py — is unchanged; only the default value and the docs framing (module docstring, README's "Delegate Timeout" → "Layered bounding" section) were updated to reflect Layer 3's backstop role.

Rebase (this pass)

Six PRs touched modules/tool-delegate/amplifier_module_tool_delegate/__init__.py on main since this branch was cut: #319 (resume agent identity + delegate:agent_resumed), #322 (model_role_unresolved + strict_model_role), #324 (flag-gated structured return contract), #325 (the Layer 1 call budget above), and #327/#328 (tool-description text edit + revert). All six conflicted with this branch's two commits.

Resolved with keep-both discipline — every mainline feature preserved intact, this PR's timeout machinery preserved intact with the 14400s default:

Module docstring, imports, and settings block: merged so settings.timeout (Layer 3) and settings.max_llm_calls / settings.strict_model_role (mainline) all document correctly side by side.
Resume-path agent-identity resolution: kept main's superior _resolve_agent_for_session (cache + session-id-suffix fallback) instead of this PR's original inline suffix parsing; the timeout exception path (_DelegateTimeoutExpired) and the rich error_payload / timeout_output shape (status, resumable, resume_status, recovery_message) are this PR's, unchanged.
pyproject.toml / .gitignore: the testpaths/pythonpath and .next/ additions from this PR are still novel relative to current main (not superseded by any of the six PRs) — kept as-is.

Test evidence after rebase:

test_delegate_timeout.py: 30/30 passed, asserting the 14400 default.
Full modules/tool-delegate/tests/ suite: 150 passed (0 regressions across all six intervening mainline features).
Full repo suite (uv run pytest tests/): 1640 passed, 1 skipped.
python_check on the touched file: no new issues vs. main's own baseline (same 9 pre-existing warnings, same categories/positions; confirmed by diffing against main's copy of the file).
CI on this PR: all 6 matrix legs (ubuntu/windows × 3.11/3.12/3.13) green, plus license/cla.

Merge

This PR is being self-merged (squash) at the maintainer's direction, per the documented admin-merge pattern, since branch protection requires a review this repository has no other assigned reviewer to provide right now. Squashing preserves Sam Schillace (@ramparte) as the commit author.

…ot bool(text) (#356) `_partial_output_fields` picked its timeout guidance from `bool(text)` alone, so a partial recovered from the REASONING channel got the sentence written for unfinished prose: "the text in 'partial_response' is unfinished work salvaged from the agent mid-flight -- it has NOT been checked, concluded, or self-reviewed" True of unfinished assistant prose. False of raw private reasoning, which was never addressed to a reader at all -- and framing it as unreviewed draft output invites the calling model to read it as a draft answer. Reachable only since app-cli 8c83a9b (PR #298) widened the accumulator to recover `thinking` + `tool_call` traces when no assistant text exists (k64: recoverable window 0.05% -> 82.2% of a leg). That half is the producer; this is the consumer. Branches on `partial.source`, never on the prose: no text -> _NO_PARTIAL_GUIDANCE (byte-identical) "spawn-accumulator:reasoning"-> _REASONING_PARTIAL_GUIDANCE (new) anything else -> _PARTIAL_GUIDANCE (byte-identical) Exact match, deliberately: an unknown or non-string source degrades to the incumbent behaviour rather than inheriting a frame that may be wrong for it. `source` is compared, never parsed, so it cannot raise on the timeout path -- the one path where raising discards every completed sibling in a batch. Byte-identity verified against the parent blob, not against this module's own constants: _PARTIAL_GUIDANCE sha256 b1d9796d1a9adf29 (416 B) and _NO_PARTIAL_GUIDANCE sha256 d73f51f164c545d3 (245 B) are unchanged, and the parent's selector re-run against this build agrees on every case except the reasoning one. Tests land in tests/ (not modules/tool-delegate/tests/) because CI runs `pytest tests/` only. Fail-before on 5d8db2f: 3 failed / 16 passed; after: 19 passed. Full suite 1939 -> 1958 passed, 1 skipped. Refs: model_performance-yiy Co-authored-by: amplifier-lane <amplifier-lane@localhost>

Sam Schillace (ramparte) requested review from Brian Krabach (bkrabach) and Salil Das (sadlilas) August 7, 2026 19:36

Sam Schillace (ramparte) mentioned this pull request Aug 13, 2026

fix: validate timeouts and clean up cancellation microsoft/amplifier-module-tool-bash#16

Merged

Brian Krabach (bkrabach) mentioned this pull request Aug 27, 2026

feat(delegate): flag-gated per-leg call budget via orchestrator max_iterations (Layer 1) #325

Merged

Sam Schillace (ramparte) and others added 2 commits August 28, 2026 08:50

fix(delegate): bound delegated sessions by default

b455a44

Brian Krabach (bkrabach) force-pushed the fix/delegate-default-timeout branch from 57a2778 to 6eb0fda Compare August 28, 2026 15:54

Brian Krabach (bkrabach) merged commit 14d5a52 into microsoft:main Aug 28, 2026
7 checks passed

Brian Krabach (bkrabach) mentioned this pull request Sep 3, 2026

fix(tool-delegate): select the partial guidance string on the KIND of partial, not bool(text) #356

Merged

Navigation Menu

Uh oh!

fix(delegate): bound delegated sessions by default - #298

fix(delegate): bound delegated sessions by default#298
Brian Krabach (bkrabach) merged 2 commits into
microsoft:mainfrom
ramparte:fix/delegate-default-timeout

Sam Schillace (ramparte) commented Aug 7, 2026

Uh oh!

Brian Krabach (bkrabach) commented Aug 27, 2026

Uh oh!

Brian Krabach (bkrabach) commented Aug 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Sam Schillace (ramparte) commented Aug 7, 2026

Summary

Coordinated prerequisite

Verification

Uh oh!

Brian Krabach (bkrabach) commented Aug 27, 2026

Uh oh!

Brian Krabach (bkrabach) commented Aug 28, 2026

Revision rebased onto current main — ready to merge

Credit

The adjudication (already applied to this branch prior to rebase)

Rebase (this pass)

Merge

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Revision rebased onto current `main` — ready to merge