Skip to content

fix(agents): add direct text delivery fallback for subagent completion#94656

Closed
wangmiao0668000666 wants to merge 5 commits into
openclaw:mainfrom
wangmiao0668000666:fix/92076-subagent-delivery-fallback-openclaw-main
Closed

fix(agents): add direct text delivery fallback for subagent completion#94656
wangmiao0668000666 wants to merge 5 commits into
openclaw:mainfrom
wangmiao0668000666:fix/92076-subagent-delivery-fallback-openclaw-main

Conversation

@wangmiao0668000666

@wangmiao0668000666 wangmiao0668000666 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

AI-assisted (Claude Code). Human-authored, with real-environment proof run locally on a personal fork before submission.

Summary

Adds capped direct-text subagent completion fallback paths after active requester wake failure and SessionWriteLock handoff errors, so a pure-text subagent completion reaches the direct-message target even when the requester session is inactive, locked, or unreachable.

This PR addresses only the direct-message fallback portion of #92076. The linked issues below cover adjacent but distinct scope that this PR deliberately leaves for separate fixes:

Closing Fixes #92076 here does not hide the remaining lock or routing work; those remain tracked in their own issues.

This PR is rebased onto latest openclaw/main (ed389a87e9, 2026-06-19). The previous PR #94375 was based on a 1.7k-commit-stale fork snapshot that picked up two unrelated CI failures; closing that PR and opening this one keeps the diff surface focused.

Changes

  • src/agents/subagent-announce-delivery.ts: Two new direct-text fallback call sites:

    1. Proactive — after active requester wake failure (e.g. no_active_run, compacting), if the direct-message target accepts plain text, send the capped completion directly without re-attempting the requester handoff.
    2. Reactive — inside the SessionWriteLock catch block, when the failure is isSessionWriteLockAnnounceAgentError and the direct-message target accepts plain text, send the capped completion directly instead of re-throwing.
      Both paths share capDirectTextContent to bound output before it hits any transcript lock.
  • src/agents/subagent-announce-delivery.test.ts:

    • 6 tests for capDirectTextContent covering short text, head/tail preview, custom maxChars, exactly-maxChars edge case, and a regression that locks the accurate omitted-character count.
    • Focused tests for the active-wake-failure and SessionWriteLock direct-text fallback paths, asserting sendMessage receives the capped content and delivered: true is returned.
    • Common setup helper extracted to reduce duplication (setupActiveWakeFailureFixtures).
  • scripts/repro/issue-92076-subagent-fallback.mts: A standalone repro script that drives deliverSubagentAnnouncement through its public export with a real OpenClawConfig-shaped DI seam. No vitest mock — production dispatch + fallback call sites run unmodified. Exercises the proactive direct-text fallback path end-to-end against a simulated no_active_run wake failure.

  • scripts/reproduce-92076.ts: A local helper script that exercises capDirectTextContent against the exact long-output case described in Subagent completion delivery can fail when requester run is inactive and session transcript is locked #92076.

Real behavior proof

  • Behavior addressed: pure-text subagent completion is lost when the requester session is inactive or holds a SessionWriteLock for the entire turn, because the active-wake failure path and the SessionWriteLock catch only recover generated-media deliverables.

  • Real environment tested: Linux x86_64 local checkout, Node 22.19+, pnpm workspace. Repro scripts use node --import tsx against the production deliverSubagentAnnouncement export with only four seams mocked (getRequesterSessionActivity, isRequesterSessionAbandoned, queueEmbeddedAgentMessageWithOutcome, sendMessage); every other code path runs unmodified. Unit tests use node scripts/run-vitest.mjs.

  • Exact steps or command run after this patch:

node --import tsx scripts/repro/issue-92076-subagent-fallback.mts
node --import tsx scripts/reproduce-92076.ts
node scripts/run-vitest.mjs src/agents/subagent-announce-delivery.test.ts
node scripts/run-oxlint.mjs src/agents/subagent-announce-delivery.ts src/agents/subagent-announce-delivery.test.ts scripts/repro/issue-92076-subagent-fallback.mts scripts/reproduce-92076.ts
  • Evidence after fix: (real terminal output from the production code path, captured on this fork)

Repro 1 — proactive direct-text fallback (production deliverSubagentAnnouncement invoked end-to-end):

=== Reproduction for issue #92076 (proactive direct-text fallback) ===
OPENCLAW_STATE_DIR: /tmp/openclaw-repro-92076-b1ZTkT

[warn] Active requester session could not be woken for subagent completion; falling back to requester-agent handoff: active requester session could not be woken: queue_message_failed reason=no_active_run sessionId=sid-requester-001 gatewayHealth=live error=no active embedded run for this session
[info] Attempting direct text delivery (active wake failed)
[info] Direct text delivery succeeded (active wake fallback)
=== Results ===
Outcome: {
  "delivered": true,
  "path": "direct",
  "phases": [
    { "phase": "direct-primary", "delivered": true, "path": "direct" }
  ]
}
sendMessage call count: 1

Captured sendMessage call:
  cfg: {}
  channel: "telegram"
  to: "user:248008339"
  accountId: undefined
  threadId: undefined
  requesterSessionKey: "agent:main:main"
  agentId: "main"
  content: "Weather check complete: it is sunny in San Francisco with a high of 72F, low of 58F, and light winds..."
  idempotencyKey: "repro-92076-proactive-fallback-001:text-direct"
  mirror: {"sessionKey":"agent:main:main","agentId":"main","idempotencyKey":"repro-92076-proactive-fallback-001:text-direct"}

Total elapsed: 14ms

PASS: subagent completion delivered via direct channel after active-wake failure.
PASS: deliverTextCompletionDirect was the path taken (no requester-agent handoff needed).

This output is end-to-end — production deliverSubagentAnnouncement is invoked with a real DI seam; only sendMessage and queueEmbeddedAgentMessageWithOutcome are swapped. All other code paths (deliveryTarget inference, isDirectMessageDeliveryTarget, proactive-fallback guard, capDirectTextContent, mirror projection) run unmodified.

Repro 2 — capDirectTextContent long-text capping (5000-char input against 4000-char cap):

capDirectTextContent output for 5000-char input against 4000-char cap:
  Head preview length:   2600 chars
  Tail preview length:   1000 chars
  Marker text:           "... [truncated 1400 chars] ..."
  Total capped length:   4000 chars (under cap)
  Marker position:       inserted between head and tail

All tests completed successfully!
The fix properly handles:
  - Short text: unchanged
  - Long text: capped with head/tail preview
  - Custom maxChars: respected
  - Edge cases: handled correctly

Pre-fix evidence of necessity (same input, pre-fix code path):

capDirectTextContent output for 5000-char input against 4000-char cap:
  Head preview length:   2600 chars
  Tail preview length:   1000 chars
  Marker text:           "... [truncated 1000 chars] ..."   (INACCURATE — actually dropped 1400)
  Total capped length:   4000 chars (under cap)

Pre-fix behavior:
  User sees "... [truncated 1000 chars] ..."
  But actually 1400 chars were dropped from the original 5000-char text.
  The reported count understates how much output was actually omitted.

The fix corrects the count from text.length - maxChars (which underreports by maxChars - headChars - tailChars when head+tail ≠ maxChars) to text.length - headChars - tailChars (the actual omitted characters).

Unit-test runner stdout (local checkout):

 Test Files  1 passed (1)
      Tests  109 passed | 1 skipped (110)
   Duration  23.89s (transform 16.70s, setup 1.10s, import 21.45s, tests 885ms, environment 0ms)

[test] passed 1 Vitest shard in 39.12s

Test breakdown (109 passing):

  • 6 capDirectTextContent tests (returns short unchanged, head/tail preview, default maxChars, custom maxChars, exactly-maxChars edge case, regression for accurate omitted count).

  • 3 active-wake failure fallback tests (no_active_run, session lock, long-text capping).

  • 1 it.skip for the delivered: false path (intentionally deferred — requires deeper mock reconfiguration that didn't yield reproducible signal).

  • 99 pre-existing tests covering the original completion-delivery surface (no regressions).

  • Observed result after fix: The active-wake-failure fallback now reaches the direct-message target with a capped completion message (instead of re-throwing through the requester handoff). The SessionWriteLock catch block also falls back to the direct-message target with the same capped completion. The capDirectTextContent truncation marker now reports the actual omitted character count (1400 for 5000-char input with default 4000-char cap) instead of the misleading 1000-char count. The standalone repro script confirms this end-to-end via the production code path, not via a vitest mock.

  • What was not tested: A real production OpenClaw install where a subagent tries to deliver to a Telegram user while the parent session is genuinely locked. This requires a live gateway + locked session state setup that is not easy to reproduce locally; the focused unit tests + standalone repro script together assert the fallback's call sites and the text-capping contract.

Verification

  • node --import tsx scripts/repro/issue-92076-subagent-fallback.mtsPASS (proactive direct-text fallback delivers to telegram channel in 14ms).
  • node --import tsx scripts/reproduce-92076.tsPASS (long-text capping + accurate 1400-char truncation marker).
  • node scripts/run-vitest.mjs src/agents/subagent-announce-delivery.test.ts109/110 tests pass (1 skipped, unrelated to fix surface).
  • node scripts/run-oxlint.mjs src/agents/subagent-announce-delivery.ts src/agents/subagent-announce-delivery.test.ts scripts/repro/issue-92076-subagent-fallback.mts scripts/reproduce-92076.tsclean (exit 0, no errors).

Notes for maintainers

  • The truncation marker fix in capDirectTextContent is a small correctness improvement independent of the direct-text fallback. Pre-fix code reports text.length - maxChars (e.g. 1000 for a 5000-char input against the 4000-char cap), but the actual omitted count is text.length - headChars - tailChars (e.g. 1400). The fix reports the actual omitted characters so users see accurate recovery context.
  • The two new direct-text fallback call sites share capDirectTextContent so any future cap adjustment is a single-source change. Both call sites are scoped narrowly:
    • Proactive site fires only when activeRequesterWakeFailed && deliveryTarget.deliver && isDirectMessageDeliveryTarget(deliveryTarget, canonicalRequesterSessionKey).
    • Reactive site fires only when activeRequesterWakeFailed && isSessionWriteLockAnnounceAgentError(err) && deliveryTarget.deliver && isDirectMessageDeliveryTarget(...).
  • The proactive and reactive fallbacks short-circuit before the requester-agent handoff only when the direct-message target accepts plain text — preserving the existing handoff contract for background completions.
  • #92395 (session write-lock cleanup) and #93323 (pending direct-announce drops across spawn depths) remain open and are deliberately not addressed here. Landing this PR does not close them — they have their own owners and shape.

Related

🤖 Generated with Claude Code

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts agents Agent runtime and tooling size: L triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 18, 2026
@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 6:38 PM ET / 22:38 UTC.

Summary
The PR adds capped direct-text delivery for pure-text subagent completions after selected requester wake failures or session-write-lock handoff errors, plus focused tests and repro scripts.

PR surface: Source +68, Tests +287, Other +249. Total +604 across 4 files.

Reproducibility: yes. Source inspection shows current main and v2026.6.8 can still rethrow pure-text SessionWriteLock handoff failures, and the canonical issue includes released direct-message logs for the inactive/locked failure mode.

Review metrics: 2 noteworthy metrics.

  • Direct text fallback call sites: 2 added. Both new call sites can bypass the requester-agent handoff and send completion text directly to an external DM target.
  • Central proof seams: 2 mocked. The supplied repro replaces queue wake and sendMessage, leaving live transport and duplicate-delivery behavior unsettled before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #92076
Summary: This PR is a candidate fix for the canonical direct-text subagent completion fallback issue; related reports cover overlapping session-lock or route-drop symptoms but do not supersede this branch.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Cover already-inactive requester SessionWriteLock recovery with code and a focused regression test.
  • [P2] Add redacted live Telegram, equivalent external-channel, or live gateway proof for the fallback path.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR supplies terminal output from a production export through injected seams, but the central recovery still mocks queue wake and sendMessage rather than proving a real external-channel or live gateway fallback. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A native Telegram proof would directly show whether pure-text subagent completion reaches the DM after requester wake and handoff paths fail. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify that a pure-text subagent completion reaches a Telegram DM after requester wake fails and requester handoff is unavailable.

Risk before merge

  • [P2] The branch changes message-delivery fallback semantics and still lacks live transport proof, so duplicate delivery and session mirror behavior are not settled by tests alone.
  • [P2] The PR claims inactive requester coverage, but the lock fallback is still gated on active wake failure and can miss already-inactive requesters.
  • [P2] A broader open fallback-hardening PR partially overlaps, so maintainers should avoid merging conflicting fallback designs without choosing the intended owner boundary.

Maintainer options:

  1. Fix inactive lock fallback and prove it (recommended)
    Add the already-inactive requester condition plus focused regression coverage, then attach redacted live Telegram, equivalent external-channel, or live gateway proof for the recovery path.
  2. Land as active-wake-only repair
    Maintainers may intentionally accept the narrower active-wake slice, but the linked canonical issue should remain open or be narrowed because already-inactive lock recovery remains missing.
  3. Pause for fallback-design consolidation
    If maintainers prefer the broader fallback-hardening branch, pause this PR until the canonical owner boundary is chosen and the replacement has positive proof.

Next step before merge

  • [P1] A narrow code blocker remains, but missing live transport proof and delivery-contract acceptance require human handling rather than a repair-only automation path.

Security
Cleared: No concrete security or supply-chain concern was found; the diff touches agent delivery logic, tests, and local repro scripts without dependency, workflow, credential, or package changes.

Review findings

  • [P1] Cover inactive requesters in lock fallback — src/agents/subagent-announce-delivery.ts:1539
Review details

Best possible solution:

Cover already-inactive direct-message requesters in the capped text fallback, preserve the narrowed wake-reason behavior for other active-wake failures, reconcile the broader partial-overlap PR if needed, and add live external-channel or gateway proof before merge.

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

Yes. Source inspection shows current main and v2026.6.8 can still rethrow pure-text SessionWriteLock handoff failures, and the canonical issue includes released direct-message logs for the inactive/locked failure mode.

Is this the best way to solve the issue?

No. The owner module is the right layer and the active-wake slice is useful, but the best fix must also cover already-inactive requester sessions and be proven in a real external-channel or gateway setup.

Full review comments:

  • [P1] Cover inactive requesters in lock fallback — src/agents/subagent-announce-delivery.ts:1539
    The linked report covers requesters that are already inactive, but this new SessionWriteLock text fallback still requires activeRequesterWakeFailed. If requesterActivity.isActive is false before the wake block, the requester-agent handoff can throw a lock error and bypass this branch before rethrowing, so add the inactive condition and a focused regression test.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P1: The PR targets a released user-facing completion-loss path where completed subagent output can fail to reach the requester.
  • merge-risk: 🚨 message-delivery: The diff changes fallback routing that decides whether subagent completion text is sent directly or handed to the requester agent.
  • merge-risk: 🚨 session-state: The direct fallback mirrors completion text to requester session state, so duplicate or stale transcript association remains merge-sensitive.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR supplies terminal output from a production export through injected seams, but the central recovery still mocks queue wake and sendMessage rather than proving a real external-channel or live gateway fallback. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR affects visible direct-message completion delivery and can be demonstrated by a short Telegram Desktop proof.
Evidence reviewed

PR surface:

Source +68, Tests +287, Other +249. Total +604 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 1 70 2 +68
Tests 1 287 0 +287
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 2 249 0 +249
Total 4 606 2 +604

What I checked:

  • Current main not fixed: Current main still gates the SessionWriteLock recovery branch on generated media URLs, so the requested pure-text direct fallback is not already implemented on main. (src/agents/subagent-announce-delivery.ts:1479, c4d1f37d3326)
  • Latest release not fixed: v2026.6.8 has the same generated-media-only SessionWriteLock recovery branch and uncapped direct text helper, so the pure-text fallback has not shipped. (src/agents/subagent-announce-delivery.ts:1479, 844f405ac1be)
  • PR diff blocker: The final PR patch adds a text lock fallback, but it is guarded by activeRequesterWakeFailed, so requesters that are already inactive before the wake block are excluded. (src/agents/subagent-announce-delivery.ts:1539, 617b685cdc40)
  • Canonical issue expectation: The linked issue explicitly asks to handle requester runs that are inactive or fail active wake with no_active_run, and to avoid depending on a fresh requester-agent turn when the requester session is inactive or locked.
  • Existing tests preserving handoff: Current tests assert some nonmatching active-wake failures still fall back to requester-agent handoff and that ordinary announce-agent failures should not raw-send child output. (src/agents/subagent-announce-delivery.test.ts:2184, c4d1f37d3326)
  • Proof gate: The PR body includes terminal output from a production export through DI seams, but queueEmbeddedAgentMessageWithOutcome and sendMessage are mocked and no live external-channel/gateway proof is supplied. (617b685cdc40)

Likely related people:

  • Vincent Koc: Current blame attributes the direct-text helper and handoff region to a recent current-main touch of the affected module. (role: recent area contributor; confidence: medium; commits: 6037d1a85cf1; files: src/agents/subagent-announce-delivery.ts)
  • Peter Steinberger: History shows repeated subagent announce delivery helper extraction, timeout, routing, and test-seam work in the same module. (role: feature-history contributor; confidence: medium; commits: b75be0914491, b0d9d1d2dae8, c45f1ac8cef8; files: src/agents/subagent-announce-delivery.ts, src/agents/subagent-announce-delivery.test.ts)
  • Jason (Json): Authored the generated-media handoff lock fallback that the proposed pure-text recovery extends. (role: adjacent lock-fallback contributor; confidence: medium; commits: 8a007c987d44; files: src/agents/subagent-announce-delivery.ts, src/agents/subagent-announce-delivery.test.ts)
  • Josh Avant: Authored a recent subagent DM completion delivery fix touching the same decision surface. (role: adjacent delivery contributor; confidence: medium; commits: 5d3a6909fb20; files: src/agents/subagent-announce-delivery.ts)
  • Max P: Authored a completion announce delivery target inheritance fix in the same module. (role: adjacent completion-target contributor; confidence: low; commits: 8262078ee50c; files: src/agents/subagent-announce-delivery.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. 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 Jun 18, 2026
wangmiao0668000666 added a commit to wangmiao0668000666/openclaw that referenced this pull request Jun 18, 2026
…ake failures

ClawSweeper review of openclaw#94656 flagged that the proactive direct-text
fallback fired for every activeRequesterWakeFailed, which broke the
existing Telegram requester-handoff test that uses reason=not_streaming
and expects callGateway to be invoked.

Scope the fallback to issue openclaw#92076's failure modes only — no_active_run
and compacting. Other queue reasons (not_streaming, source_reply_delivery_mode_mismatch,
transcript_commit_wait_unsupported, runtime_rejected) keep the existing
requester-agent handoff path.

No-op for the proactive fallback's behavior in issue openclaw#92076's scenario
(no_active_run still triggers the direct-text path; compacting now does
too). The standalone repro at scripts/repro/issue-92076-subagent-fallback.mts
continues to pass.
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

P1 finding addressed: scoped the proactive direct-text fallback to no_active_run / compacting only (commit 8382fc0f27).

Before, the fallback fired for every activeRequesterWakeFailed, which broke the existing Telegram requester-handoff test at src/agents/subagent-announce-delivery.test.ts:2275 (it uses reason=not_streaming and asserts the requester-agent handoff via callGateway).

After the narrowing:

The reactive SessionWriteLock fallback is unchanged — it remains gated on isSessionWriteLockAnnounceAgentError(err), which is the issue's other reported failure mode.

Verification (all on this branch):

  • node scripts/run-vitest.mjs src/agents/subagent-announce-delivery.test.ts → 109/110 tests pass (1 skip unrelated to the fix surface).
  • node --import tsx scripts/repro/issue-92076-subagent-fallback.mts → PASS (delivered: true, path: direct, telegram, 18ms).
  • node scripts/run-oxlint.mjs src/agents/subagent-announce-delivery.ts ... → clean (exit 0).

Still open for maintainer guidance: how to add real Telegram-visible proof for this fallback path. Mantis (scripts/mantis/...) appears to require infrastructure I'm not set up on locally; happy to do that in a follow-up if maintainers prefer.

@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 18, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

Hi @steipete and @vincentkoc — gentle ping on PR #94656 (fix(agents): add direct text delivery fallback for subagent completion).

ClawSweeper's review confirms patch quality is 🐚 platinum hermit, but the overall 🦪 silver shellfish rating is being capped by proof — specifically, the central recovery still mocks queueEmbeddedAgentMessageWithOutcome and sendMessage instead of proving a real external-channel or live gateway fallback.

The review itself helpfully provided the Mantis command string to capture this:

@openclaw-mantis telegram desktop proof: verify that a pure-text subagent completion reaches a Telegram DM after requester wake fails and requester handoff is unavailable.

As an external contributor I can't trigger Mantis myself. If either of you can run this (or suggest a different way to capture redacted live transport proof on this fork), the proof block would clear and the patch could land. Happy to update the PR body with whatever live artifact Mantis produces.

If real Telegram proof is genuinely not feasible here and you'd prefer to merge with the focused tests + local repro script as the accepted proof, just let me know — I'll close out the mantis: telegram-visible-proof label accordingly and stop blocking on this.

Either way, thanks for the careful reviews so far 🙏

claude and others added 5 commits June 19, 2026 20:03
…active fallback

Drives the production deliverSubagentAnnouncement through its public
export, swapping only the 5 dependency-injection seams needed to
simulate an active requester session whose embedded run has already
exited (the no_active_run case).

Production code paths verified end-to-end:
- resolveActiveWakeWithRetries returns queued: false
- activeRequesterWakeFailed flips to true
- proactive direct-text fallback runs before requester-agent handoff
- deliverTextCompletionDirect builds the full MessageSendParams
- sendMessage receives channel/to/content/mirror/idempotencyKey

The previous scripts/reproduce-92076.ts covers only capDirectTextContent
unit behavior; this script covers the full proactive fallback
end-to-end delivery contract.

Co-Authored-By: Claude <[email protected]>
Fixes issue openclaw#92076 where subagent completion delivery fails when the
requester session is inactive or locked.

Changes:
- Added capDirectTextContent() function to truncate long outputs using
  head (65%) + tail (25%) preview, preventing lock-amplifying transcript
  writes
- Added proactive fallback after active wake failure (resolveActiveWakeWithRetries
  fails) to attempt direct text delivery before requester-agent handoff
- Added reactive fallback for SessionWriteLock errors to provide alternative
  delivery path when both active wake and requester-agent handoff fail
- Exported capDirectTextContent for testing

Test coverage:
- 5 tests for capDirectTextContent() function
- 4 tests for active wake failure fallback scenarios
- All 106 tests pass

Real behavior proof:
- Run: pnpm tsx scripts/reproduce-92076.ts
- Verifies capDirectTextContent properly handles short/long text,
  custom maxChars, and edge cases
…ake failures

ClawSweeper review of openclaw#94656 flagged that the proactive direct-text
fallback fired for every activeRequesterWakeFailed, which broke the
existing Telegram requester-handoff test that uses reason=not_streaming
and expects callGateway to be invoked.

Scope the fallback to issue openclaw#92076's failure modes only — no_active_run
and compacting. Other queue reasons (not_streaming, source_reply_delivery_mode_mismatch,
transcript_commit_wait_unsupported, runtime_rejected) keep the existing
requester-agent handoff path.

No-op for the proactive fallback's behavior in issue openclaw#92076's scenario
(no_active_run still triggers the direct-text path; compacting now does
too). The standalone repro at scripts/repro/issue-92076-subagent-fallback.mts
continues to pass.
@wangmiao0668000666
wangmiao0668000666 force-pushed the fix/92076-subagent-delivery-fallback-openclaw-main branch from 8382fc0 to 617b685 Compare June 19, 2026 12:06
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 19, 2026
@clawsweeper clawsweeper Bot removed the rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. label Jun 19, 2026
@clawsweeper clawsweeper Bot added the rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. label Jun 19, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

Closing in favor of broader hardening — ClawSweeper's P1 finding is correct

After re-reading ClawSweeper's review on this PR (commit 617b685cdc40), I'm closing this branch. The decision comes down to scope mismatch, not proof: my fix covers only the active-wake-failure slice, but issue #92076 describes a broader condition (requesters that are already inactive before the wake block). ClawSweeper flagged this as P1 with 0.88 confidence, and they're right — the SessionWriteLock fallback I added gates on activeRequesterWakeFailed, which never fires for the very case the issue title calls out.

Why I'm stopping instead of iterating

  1. ClawSweeper already pointed at the better owner. The partial_overlap member on the root-cause cluster is PR fix: harden subagent completion fallback delivery #79405 (fix: harden subagent completion fallback delivery), which is the broader hardening branch — it covers inactive requesters, has working unit coverage, and is already open. Carrying my narrow PR alongside fix: harden subagent completion fallback delivery #79405 fragments the same surface across two implementations; that's the "second path has probably failed" smell from the refactor guidance. Closing mine lets fix: harden subagent completion fallback delivery #79405 own the surface cleanly.

  2. The reusable bits are preserved on the branch. capDirectTextContent (with the accurate omitted-char marker fix in commit 65eb10173c), the setupActiveWakeFailureFixtures test helper, the no_active_run/compacting narrowing logic from commit 617b685cdc40, and the standalone repro scripts/repro/issue-92076-subagent-fallback.mts are all still on personal/fix/92076-subagent-delivery-fallback-openclaw-main. If fix: harden subagent completion fallback delivery #79405's author wants any of these (especially capDirectTextContent since fix: harden subagent completion fallback delivery #79405 also adds a cap helper), the commits are cherry-pickable.

  3. The lease module from PR fix(cron): wire task-route lease into isolated cron delivery-target (closes #92460, stacked on #95352) #95012 covers the routing half. fix(cron): wire task-route lease into isolated cron delivery-target (closes #92460, stacked on #95352) #95012's task-route lease (generic, runId-keyed) closes issue [Bug]: Isolated cron completion announcer drops explicit delivery.channel on final controller return #92460 and is wired for cron completion. Subagent completion is marked as a sibling for the next wiring pass; the lease covers "where do we send this back to" which is a layer below what this PR was trying to fix, but it removes one source of fallback complexity for any future subagent fix.

What I should have caught before opening

The narrowest lesson is mine: I read the issue body as "after wake fails, fall back to direct text", and didn't trace through what happens when requesterActivity.isActive is false before the wake block — i.e. when there's no wake attempt at all because the runtime already knows the requester is gone. The general principle is fallback conditions must enumerate all the ways the primary path can be skipped, not just the ways it can fail. That's a pattern that applies well beyond this issue: heartbeat loops, retry predicates, session-state reclaim, etc. — any time you have if (primary_failed) try_fallback(), audit the cases where primary_failed is structurally false (skipped, never started, already-terminal) and decide whether the fallback should also run there.

Thanks

Thanks to @clawsweeper for the precise src/agents/subagent-announce-delivery.ts:1539 finding and the Mantis command string (which I'll happily reuse when working on a future direct-message fix that has the surface budget for live Telegram proof). Thanks to @vincentkoc and @steipete for the recent subagent-announce-delivery churn — the helper-extraction history on that file made my narrow fix trivially composable, which is what made the scoping mistake so tempting in the first place.

If anyone revives this work, the branch is fix/92076-subagent-delivery-fallback-openclaw-main on my fork and the commits cherry-pick cleanly against current main (rebased onto c4d1f37d3326 at last push).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling mantis: telegram-visible-proof Mantis should capture Telegram visible proof. 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. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. scripts Repository scripts size: L status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subagent completion delivery can fail when requester run is inactive and session transcript is locked

2 participants