Skip to content

fix(subagents): reconcile timed-out waits against child session state#75786

Closed
carlos-443-diaz wants to merge 1 commit into
openclaw:mainfrom
carlos-443-diaz:fix/subagent-timeout-reconcile-session-state
Closed

fix(subagents): reconcile timed-out waits against child session state#75786
carlos-443-diaz wants to merge 1 commit into
openclaw:mainfrom
carlos-443-diaz:fix/subagent-timeout-reconcile-session-state

Conversation

@carlos-443-diaz

Copy link
Copy Markdown

Fixes #75785.

Summary

Reconcile timed-out agent.wait results against the persisted child session state before announcing subagent completion.

This fixes a false-timeout path where the child run can finish successfully, but the parent-side completion handoff still reports timed out and falls back to a partial-progress stub.

Problem / Motivation

The announce flow currently trusts the immediate agent.wait timeout result. In the observed failure mode, that timeout can be stale relative to the child session store state. When that happens, the parent announces a timeout even though the child session has already completed successfully.

What Changed

  • src/agents/subagent-announce-output.ts
    • waitForSubagentRunOutcome(runId, timeoutMs, sessionKey?) now optionally re-checks session-store state after a timeout
    • added helpers to load and interpret the child session entry
  • src/agents/subagent-announce.ts
    • pass childSessionKey into waitForSubagentRunOutcome(...)
    • do the same for the immediate recheck path
  • src/agents/subagent-announce.timeout.test.ts
    • adds a regression test covering a timed-out wait that should reconcile to persisted child success before announcing

Why this is the smallest reliable guardrail

The reconciliation only runs:

  • when agent.wait returns timeout
  • when the child session key is available
  • for a short bounded retry loop

Normal successful and failed paths are unchanged.

User-visible / Behavior Changes

Parent sessions stop receiving false timed out completion events for already-finished child runs in this path.

Diagram

Before:
child finishes successfully -> agent.wait returns timeout -> parent announces timeout -> actual child result is replaced by partial progress

After:
child finishes successfully -> agent.wait returns timeout -> announce flow rechecks child session state -> parent announces success -> child result is delivered

Security Impact

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: Linux 6.8
  • Runtime/container: npm global install
  • Model/provider: openai-codex/gpt-5.4
  • Integration/channel: subagent completion announce path
  • Relevant config: default subagent announce flow with completion handoff enabled

Steps

  1. Spawn a subagent that performs many tool calls and completes successfully.
  2. Let the child session reach a terminal successful state.
  3. Have the parent-side announce flow observe agent.wait return timeout.
  4. Compare the child session state with the parent completion event.

Expected

  • Parent completion announce resolves to child success and includes the child result.

Actual

  • Parent completion announce can resolve to timeout and replace the child result with a partial-progress stub.

Evidence

  • Trace/log snippets
  • Failing test/log before + passing after
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification

  • Verified scenarios:
    • observed a child run with successful terminal state while the parent announce path still emitted a timeout result
    • applied equivalent reconciliation logic locally in the installed build and confirmed it corrected the announce outcome for that bug path
  • Edge cases checked:
    • no reconciliation path is used unless agent.wait returns timeout
    • still falls back to original timeout result if the child session store is not terminal
  • What I did not verify:
    • full upstream CI suite
    • multi-provider or multi-channel coverage beyond the local reproduction path

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Risks and Mitigations

  • Risk: session-store terminal state could be briefly stale in the opposite direction
    • Mitigation: reconciliation runs only after a timeout and only upgrades the announce outcome when the child session entry is already terminal
  • Risk: additional small read on timeout path
    • Mitigation: only occurs on timeout, with a short bounded retry loop

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels May 1, 2026
@clawsweeper

clawsweeper Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep this PR open, but it is not merge-ready because the branch conflicts, lacks inspectable real behavior proof, and duplicates the current shared session reconciliation contract without its freshness guard.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #75785
Summary: This PR is the open candidate fix for the false subagent timeout report; the broader lifecycle PR is adjacent but does not safely replace this direct timeout reconciliation work.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

Do we have a high-confidence way to reproduce the issue?

No high-confidence live reproduction was run in this read-only review. Source inspection shows the direct announce wait path can still apply a raw timeout, and the linked report supplies one observed real run.

Is this the best way to solve the issue?

No, not as submitted. The target seam is plausible, but current main already has a shared freshness-checked reconciliation helper, so the maintainable fix is to reuse it instead of adding a second parser.

Security review:

Security review cleared: No concrete security or supply-chain concern was found; the diff changes in-repo agent runtime and test code only.

AGENTS.md: found and applied where relevant.

What I checked:

  • stale F-rated PR: PR was opened 2026-05-01T19:23:12Z, is older than 60 days, and the latest review rated it F.
  • proof blocker: real behavior proof is missing and proof tier is F, so this branch is not merge-ready without contributor follow-up.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • vincentkoc: Commit 34c5748 introduced the existing timed-out subagent announce wait recheck behavior that this PR extends. (role: announce-path feature-history contributor; confidence: high; commits: 34c57487b44c; files: src/agents/subagent-announce.ts, src/agents/subagent-announce.format.e2e.test.ts)
  • steipete: Commit 540b98b split subagent announce output helpers into the current module where the PR changes waitForSubagentRunOutcome. (role: announce helper refactor contributor; confidence: medium; commits: 540b98b23f7b; files: src/agents/subagent-announce-output.ts, src/agents/subagent-announce.ts)
  • pick-cat: Current blame attributes the shared session reconciliation helper and run-manager timeout reconciliation path to commit eb5fb2a, which the PR should reuse or align with. (role: recent shared-helper contributor; confidence: medium; commits: eb5fb2aa69f4; files: src/agents/subagent-session-reconciliation.ts, src/agents/subagent-registry-run-manager.ts, src/agents/subagent-announce-output.ts)

Codex review notes: model internal, reasoning high; reviewed against 6cb82eaab865.

@barnacle-openclaw

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@barnacle-openclaw barnacle-openclaw Bot added the stale Marked as stale due to inactivity label May 30, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels May 30, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 30, 2026
@clawsweeper clawsweeper Bot added merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 30, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label May 31, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 31, 2026
@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 14, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 14, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 15, 2026
@clawsweeper clawsweeper Bot added merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 15, 2026
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 17, 2026
@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Subagent completion can be announced as timed out even when the child run finished successfully

1 participant