fix(delegation): preserve redacted partial output when a subagent times out (rebase of #65824 + background path)

[sakhnenkoff]

What does this PR do?

Rebases #65824 onto current main and closes the one gap raised in review on it.

When a delegate_task child times out after doing real work, the parent gets {"status": "timeout", "summary": None} and every completed tool result is thrown away. A research child that finishes several tool calls and then hangs on the last one leaves the parent with nothing to continue from, even though the work existed. tools/delegate_tool.py hard-sets "summary": None on the timeout path and never looks at what the child produced.

This attaches the child's completed tool outputs to the timeout entry as partial: true + partial_output_tail[] (bounded to 8 entries × 600 chars), and renders that tail in the async re-injection path so delegate_task(background=true) surfaces it too.

Security: _extract_output_tail applies redact_sensitive_text(..., force=True) before truncation, so a credential can't be split at the boundary into an unrecognisable — and therefore unredacted — fragment. Any redaction failure drops the entire tail (fail closed) and preserves the legacy timeout schema.

Attribution

The implementation is Charles Cha's (@ypwcharles) from #65824 — authorship and Co-authored-by are preserved on the commit. That PR has been CONFLICTING since early August. This branch is the rebase plus the missing background rendering, opened so the work isn't lost. Happy to close this in favour of #65824 if @ypwcharles would rather push the rebase there.

Relative to #65824

1. Rebased onto current main. Conflict resolution keeps main's _late_pending_steer handling on the timeout entry alongside the new partial / partial_output_tail fields — neither side's behaviour is dropped.
2. Closes the background gap raised in review: _format_async_delegation() rendered timeout tasks from summary + error only and never read partial_output_tail, so the field existed on the entry but never reached the parent on the background=true path. _render_partial_output_tail() is now applied in both the batch and single-result branches.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Scope guards

• Only the timeout path is touched. Ordinary exceptions keep the existing error schema.
• The zero-API-call (before_first_llm_call) diagnostic behaviour is unchanged.
• The success-path result entry is untouched, so the wire shape stays byte-identical for completed children.

How to Test

Verified by reproduction, not by reading the diff.

A stub OpenAI-compatible endpoint lets a real child complete one real terminal tool call, then hangs, so delegation.child_timeout_seconds fires in the after_llm_calls phase with genuine completed work in the transcript. Same harness, same input, only tools/ swapped:

before (origin/main):  status=timeout  api_calls=2  summary=None  partial=None  tail=0
after  (this branch):  status=timeout  api_calls=2  summary=None  partial=True  tail=1
                       [terminal] {"output": "RESEARCH_FINDING_ALPHA: ...", "exit_code": 0}

Feeding that captured result entry through _format_async_delegation() — the exact path background=true uses to re-inject a finished fan-out — the rendered block contains the child's real output under a Partial output (redacted): label with this change, and neither without it.

Test runs:

1. scripts/run_tests.sh tests/tools/test_delegate.py tests/tools/test_delegate_subagent_timeout_diagnostic.py tests/tools/test_process_registry.py -q → 166 passed, 0 failed.
2. Sabotage check: reverting tools/delegate_tool.py + tools/process_registry.py to the merge base fails 14 of those tests, so the regressions genuinely bite.
3. Full scripts/run_tests.sh tests/tools/ -q → 6055 passed / 73 failed. The identical 73 also fail on unmodified origin/main on this host (Daytona, Discord and other environment-dependent suites) — no new failures.

Checklist

• Follows the project's code style
• Self-reviewed
• Commented in hard-to-understand areas
• New tests added and passing
• No new warnings introduced

[Lupin]

Code review — partial redacted output on timeout

Reviewed the full diff (4 files: delegate_tool.py, process_registry.py + 2 test files) against current main. Verified against the source tree: redact_sensitive_text(..., force=True) is a real parameter (agent/redact.py:772-775), and _session_messages is a genuine attribute assigned to the child agent in production (conversation_loop.py, run_agent.py), so getattr(child, "_session_messages", None) is legitimate — the new tail isn't dead code that only works in the stub.

What's good

• Redact before truncating — the comment at delegate_tool.py is exactly right: truncating first could split a credential at the max_chars boundary and leave an unredacted fragment. Correct ordering.
• is_error is computed from the raw content before redaction, so error detection still works on markers buried in block-wrapped content while the preview is sanitized. Good ordering.
• Fail-closed everywhere: malformed live messages, _session_messages raising, extractor throwing, and non-timeout exceptions all degrade to the legacy schema without leaking (tests cover each).
• Non-timeout exceptions never expose partial output — matches the security boundary the docstring promises.
• Bounded tail (8 entries / 600 chars) and dedup of preview into both the progress event and the error entry from one source variable.

Findings

WARNING — File:Line tools/delegate_tool.py:638-643 — _extract_output_tail is shared: it powers the normal delegation overlay "Output" section (cc-swarm-parity feature, non-timeout path), not just the new timeout tail. The new try/except returns [] on any redaction failure. That means a single redactor hiccup (config issue, pattern edge case) now blanks the entire existing output overlay for all delegations — not just the timeout case the PR targets. The timeout call-site explicitly wants fail-closed ("observability damage must never become exposure"), but the ordinary path previously degraded gracefully and now silently loses its whole Output section. Suggest scoping the fail-closed return [] to the timeout caller (or passing a fail_closed flag), keeping the normal path best-effort.

SUGGESTION — File:Line tools/process_registry.py:746-750 — _render_partial_output_tail uses entry.get("tool") / entry.get("preview") with defaults, but every consumer upstream already filters/validates these fields (delegate_tool.py:665-675). The defensive defaults are harmless, just slightly redundant; fine as-is.

SUGGESTION — Test gap — test_timeout_redacts_secrets_from_partial_tail and test_output_tail_preserves_plain_output_and_redacts_secrets both use a single sk- style secret. Consider one case with a secret split across the truncation boundary (e.g. prefix + max_chars - 3 + secret tail) to lock in the "redact-before-truncate" behavior that the implementation comment explicitly calls out — that's the subtle case that motivated the ordering and would otherwise regress silently.

Verdict

Solid, well-tested PR. The security ordering is correct and the fail-closed discipline is consistent. The one substantive point is the shared-function side effect in the Warning — worth confirming the non-timeout overlay path still shows output when the redactor is healthy, and ideally scoping the hard-fail to the timeout path.
COMMENT

[sakhnenkoff]

Thanks @Lupin — both findings actioned in 8dbecec, and the WARNING was a genuine regression, not a theoretical one.

WARNING (shared function) — fixed

Confirmed by reproduction before touching anything. With a redactor patched to raise, _extract_output_tail returned [] and the ordinary Output overlay lost content it had rendered moments earlier with a healthy redactor. Exactly as described: delegate_tool.py:2809 is the non-timeout success path using the same helper, and upstream degraded gracefully there.

Went with the fail_closed flag you suggested, defaulting to False:

• Default (display callers) — best-effort. continue past only the tool result that could not be redacted and keep rendering the rest, so one bad entry can't destroy a working feature.
• fail_closed=True — opted into solely by the timeout evidence call-site. Unchanged hard fail: evidence from a failed child is a security boundary, not a display nicety, so it emits nothing rather than risk an unredacted preview.

Verified with a three-result fixture where the redactor raises on the middle one:

overlay (default, best-effort): ['FINDING_ONE_worth_keeping', 'FINDING_THREE_worth_keeping']
timeout (fail_closed=True)    : []

The overlay keeps both healthy findings, drops the unredactable one, and the security-boundary caller still emits nothing.

Test gap (boundary-split secret) — added, after a false start worth reporting

My first attempt at this test was vacuous. It used an sk- style secret and passed even when I sabotaged the implementation to redact(content[:max_chars]) — it was locking in nothing. sk-, ghp_, AKIA and friends still match after truncation, so truncate-first redacts them anyway.

Only patterns with a minimum-length quantifier are actually vulnerable. Searched for one that bites: the Codex pattern gAAAA[A-Za-z0-9_=-]{20,}, cut to 25 chars, drops below the 20-char body minimum, stops matching, and leaks in the clear:

redact -> truncate:  'credential gAAAAB...BBBB '
truncate -> redact:  'credential gAAAABBBBBBBBB'   <- raw fragment

One further wrinkle: the redactor elides the middle of a matched token rather than deleting it, so assertNotIn("gAAAA", preview) is the wrong invariant — it fails on correct output too. The test asserts on the length of the surviving raw body run instead.

Non-vacuity is now proven both ways: green with the fix, and sabotaging the ordering fails it with AssertionError: '...' not found in 'credential gAAAABBBBBBBBB'.

SUGGESTION (redundant defaults in _render_partial_output_tail)

Left as-is, per your "fine as-is" — the defensive .get() defaults cost nothing and the function is now reachable from two render branches.

Test state

• tests/tools/test_delegate.py: 81 passed (79 before this round).
• Focused trio (test_delegate + test_delegate_subagent_timeout_diagnostic + test_process_registry): 168 passed, 0 failed.
• Full tests/tools/: 6057 passed / 73 failed — the identical 73 also fail on unmodified origin/main on this host (Daytona, Discord, other environment-dependent suites), so no new failures.
• The end-to-end timeout reproduction still returns partial=True carrying the child's real tool output, so scoping the fail-closed behaviour didn't weaken the case the PR exists for.

[ypwcharles]

Canonical consolidation is now on #65824.

I rebased the original implementation onto current main and preserved your follow-up hardening as a separate authored commit (Author: Matvii Sakhnenko). The consolidated branch includes:

• redact-before-truncate timeout evidence;
• timeout-only fail_closed=True while preserving best-effort ordinary overlays;
• async batch rendering and single-result notification rendering;
• single-background dispatch → queue transport with a second forced-redaction and bounds check;
• fail-closed handling for malformed/untrusted transport data;
• the non-vacuous truncation-boundary regression test.

Final related canonical validation on #65824: 205 passed, 0 failed, 4 Windows-only skipped; static/Windows-footgun gates passed across all 6 changed files. Full-suite status and exact head are recorded on #65824.

Thank you for doing the rebase and for catching the fail-closed scoping regression. Since #65824 now contains both contributions with git authorship intact, please close #84085 in favor of the canonical PR.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(delegation): preserve redacted partial output when a subagent times out — the redact-before-truncate fix and the fail-open/fail-closed split are well thought out, and the non-vacuity of the boundary test is demonstrated. Observations:

1. tools/delegate_tool.py::_extract_output_tail — from agent.redact import redact_sensitive_text is imported inside the per-message loop; it runs once per tool result. Hoisting to module level avoids a repeated import on every entry.
2. Redact-before-truncate also changes the ordinary (non-timeout) Output overlay: tool previews on the success path are now masked where raw output previously rendered. Presumably the point of the fix, but it's a visible behavior change on a display path shared by all delegation calls — worth confirming every preview consumer wants masking, not just the timeout evidence path.
3. fail_closed=True drops the entire tail on the first redactor failure, discarding entries that were already successfully redacted (and are therefore provably safe). Consistent with the "security boundary" rationale, but a flaky redactor hitting one late entry wipes all earlier evidence — consider whether fail-closed should retain the already-redacted entries and drop only the unredactable one.
4. Nice test coverage: minimum-length-quantifier pattern for truncation, malformed-message and snapshot-error fail-closed cases, and the batch/single/interrupted rendering matrix in process_registry.