fix(exec): route notifyOnExit through exec-event and add heartbeat completion tracing by ChauncyZhang · Pull Request #42394 · openclaw/openclaw

ChauncyZhang · 2026-03-10T17:26:18Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem:
Async exec completion notification path was not using the canonical exec-event wake flow.
Completion event text/wake reason shape made exec-completion handling less consistent.
Why it matters:
Long-running task completion handoff could be unreliable, causing “started but no final completion notification” behavior in some flows.
What changed:
In notifyOnExit, route completion through emitExecSystemEvent(...) unified path.
Normalize completion event text to exec finished: ... so exec-completion filters can reliably detect it.
Normalize wake reason to canonical exec-event (instead of dynamic exec:<id>:exit) for consistent heartbeat reason handling.
Update tests in bash-tools.test.ts to match new completion text + wake reason.
What did NOT change (scope boundary):
No new tool capabilities.
No auth/token logic changes.
No API contract changes.
No new logging/tracing behavior included in this final cleaned PR.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #N/A
Related #N/A

User-visible / Behavior Changes

Improves reliability of async completion handoff for background exec tasks.
No UI changes.
Note: deployment config may need agents.defaults.heartbeat.target = "last" to relay async completion back to current chat.

Security Impact (required)

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)
If any Yes, explain risk + mitigation:
N/A

Repro + Verification

Environment

OS: Ubuntu (WSL2)
Runtime/container: OpenClaw source runtime (Node.js)
Model/provider: openai/gpt-5.3-codex
Integration/channel (if any): Feishu DM
Relevant config (redacted):
agents.defaults.heartbeat.target = "last"

Steps

Start gateway from source branch.
Trigger background exec flow (e.g., clone repo via chat task).
Verify completion is handed off and user receives completion message.

Expected

Start acknowledgement is sent.
Completion path is recognized as exec-event and completion message is relayed.

Actual

After fix: completion relay works in repeated real runs.

Evidence

Attach at least one:

Failing behavior before + passing after (real run logs)
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
Multiple real clone tasks from chat completed with final completion messages.
Completion wake path behaved consistently with canonical exec-event.
Edge cases checked:
notifyOnExit success path and test expectation alignment.
What you did not verify:
Non-Feishu channels end-to-end.
Multi-account routing permutations beyond current environment.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (Yes)
Migration needed? (Yes)
If yes, exact upgrade steps:

Deploy this code change.
Ensure config has agents.defaults.heartbeat.target = "last" if async completion should return to current chat.
Restart gateway.

Failure Recovery (if this breaks)

How to disable/revert this change quickly:
Revert this commit and restart gateway.
Files/config to restore:
src/agents/bash-tools.exec-runtime.ts
src/agents/bash-tools.test.ts
(optional runtime config) agents.defaults.heartbeat.target
Known bad symptoms reviewers should watch for:
Background exec finishes but no completion relay to user.
Completion path falls back to non-canonical reason and is skipped.

Risks and Mitigations

Risk:
Behavior relies on heartbeat delivery target config in deployment.
Mitigation:
Explicitly document and set agents.defaults.heartbeat.target = "last" in runtime config.

…mpletion tracing

greptile-apps · 2026-03-10T17:29:12Z

Greptile Summary

This PR standardizes the async exec completion notification path by routing notifyOnExit through the existing emitExecSystemEvent unified helper, normalizing the event text prefix to exec finished: ... and the heartbeat wake reason to the canonical "exec-event" string.

Key observations:

Heartbeat reason change: The wake reason changes from the per-session dynamic string (previously exec:<session-id>:exit) to the static "exec-event". Any code outside these two files that pattern-matched on the old dynamic reason format would silently stop matching — worth a quick grep across heartbeat consumer code before deploying.
contextKey now included on exit events: The new call path passes contextKey carrying the session-scoped exit identifier to enqueueSystemEvent, whereas the old inline call did not. This is a net improvement (per-session event deduplication at the queue level) and is consistent with how other exec events are emitted, but it is a subtle behavioral delta not called out in the PR description.
Test coverage is correct: Both wake-reason tests correctly drop the now-unused sessionId variable and assert on the new static reason. The toMatchObject vs toEqual distinction between the scoped and unscoped cases is preserved. OUTPUT_EXEC_COMPLETED is updated to match the new event text prefix.

Confidence Score: 4/5

Safe to merge; changes are logically correct and tests are updated, with one undocumented behavioral delta worth verifying before deployment.
The refactoring correctly consolidates duplicate inline logic into the existing emitExecSystemEvent helper, the heartbeat reason normalization is intentional and well-tested, and the session.exitNotified guard prevents duplicate emissions. The score is 4 rather than 5 because the implicit addition of contextKey to the exit system event (changing deduplication behavior) is not documented in the PR description, and there is no in-PR evidence that downstream heartbeat consumers relying on the old dynamic reason format have been audited.
No files require special attention beyond the noted behavioral delta in src/agents/bash-tools.exec-runtime.ts.

_{Last reviewed commit: d375663}

holgergruenhagen · 2026-03-25T00:27:11Z

We reproduced this bug live on Discord and confirmed that the minimal wake-reason change in #41479 resolves the local background-exec completion-notification failure for us.

So from our downstream validation:

fix: ensure exec notifyOnExit wakes heartbeat as exec-event #41479 matches the exact smallest fix we proved live
this PR looks like the broader/cleaner superset because it also routes notifyOnExit through the canonical exec-event helper and normalizes completion text

I’m linking this here because it appears architecturally aligned with what we saw in production, but the exact live-validated fix in our environment was the narrower change from exec:<session-id>:exit to exec-event.

No objection to the broader cleanup here — just noting that the concrete live proof we have maps most directly to #41479.

Related exact minimal fix: #41479

vincentkoc · 2026-04-05T07:44:18Z

closing this in favor of #41479. both fix the notifyOnExit to exec-event wake mismatch, but #41479 is the tighter branch and the better canonical path. if the extra tracing in this branch is still wanted, that should probably come back as a separate follow-up.

fix(exec): route notifyOnExit through exec-event and add heartbeat co…

d375663

…mpletion tracing

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Mar 10, 2026

holgergruenhagen mentioned this pull request Mar 25, 2026

fix: ensure exec notifyOnExit wakes heartbeat as exec-event #41479

Merged

20 tasks

vincentkoc added close:duplicate Closed as duplicate dedupe:child Duplicate issue/PR child in dedupe cluster labels Apr 5, 2026

vincentkoc closed this Apr 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(exec): route notifyOnExit through exec-event and add heartbeat completion tracing#42394

fix(exec): route notifyOnExit through exec-event and add heartbeat completion tracing#42394
ChauncyZhang wants to merge 1 commit into
openclaw:mainfrom
ChauncyZhang:fix/async-completion-ack

ChauncyZhang commented Mar 10, 2026

Uh oh!

greptile-apps Bot commented Mar 10, 2026

Uh oh!

holgergruenhagen commented Mar 25, 2026

Uh oh!

vincentkoc commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants