fix(exec): track background exec liveness with CLI tasks#59719
fix(exec): track background exec liveness with CLI tasks#59719ringlochid wants to merge 1 commit into
Conversation
openclaw-barnacle
Bot
added
gateway
There was a problem hiding this comment.
Pull request overview
This PR fixes a liveness/visibility gap for background exec/process runs by linking them to first-class runtime:"cli" task records, ensuring task maintenance, session rows, and status surfaces reflect ongoing (or cancelled/failed) background work.
Changes:
- Create/update linked
runtime:"cli"task records for backgroundexecruns (including no-owner/system-scoped runs) and record progress/stall/terminal outcomes. - Adjust
process remove/killsemantics to preserve inspectability until exit finalizes, and propagate cancellation state consistently. - Reconcile orphaned CLI tasks via a runtime hook (
isCliRunActive) with a CLI-specific grace window, and derive gateway session row status/updatedAt from linked task state.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tasks/task-registry.test.ts | Adds regression coverage for CLI orphan reconciliation and grace behavior in maintenance. |
| src/tasks/task-registry.store.ts | Extends task registry hooks with isCliRunActive(runId) for CLI liveness checks. |
| src/tasks/task-registry.maintenance.ts | Uses CLI hook + CLI-specific grace when reconciling “lost” CLI tasks; makes reconcile helper accept now. |
| src/gateway/session-utils.ts | Drives session row status/timestamps from linked task snapshot (when no subagent run). |
| src/gateway/session-utils.test.ts | Adds tests ensuring session rows reflect active/stale/missing CLI-linked task state. |
| src/gateway/server.impl.ts | Wires isCliRunActive from process registry into task registry runtime hooks at gateway startup. |
| src/agents/bash-tools.test.ts | Adds exec backgrounding tests asserting CLI task creation/progress/cancel/terminal behavior. |
| src/agents/bash-tools.process.ts | Keeps removed sessions inspectable until exit, adds “termination requested” messaging, and surfaces killed status. |
| src/agents/bash-tools.process.supervisor.test.ts | Updates tests for new remove semantics (inspectable until exit) and killed terminal behavior. |
| src/agents/bash-tools.exec.ts | Creates linked CLI task on background yield (session-owned or system-scoped) via dynamic task-executor import. |
| src/agents/bash-tools.exec-runtime.ts | Records CLI task progress/stall notices and writes terminal state on completion/failure/cancel. |
| src/agents/bash-tools.exec-runtime.test.ts | Adds unit test for CLI stall notice (No output for 120s.) behavior. |
| src/agents/bash-process-registry.ts | Tracks output timestamps, removal/cancel flags, and exposes isCliRunActive for liveness checks. |
| src/agents/bash-process-registry.test.ts | Adds coverage for “removed but still alive” sessions: hidden from tools, still live for CLI liveness. |
| src/agents/bash-process-registry.test-helpers.ts | Extends process session fixtures with new CLI-task lifecycle fields. |
| return; | ||
| } | ||
| yielded = true; | ||
| markBackgrounded(run.session); | ||
| await createBackgroundCliTaskIfNeeded(); |
There was a problem hiding this comment.
onYieldNow awaits createBackgroundCliTaskIfNeeded() before resolving the tool result. Because createBackgroundCliTaskIfNeeded does a dynamic import + task write, this can delay the yield/"background" response beyond the configured yield window. Consider kicking off task creation without awaiting it (while still storing cliTaskCreationPromise so runExecProcess can await it on exit), then call resolveRunning() immediately to keep yield latency predictable.
| await createBackgroundCliTaskIfNeeded(); | |
| void createBackgroundCliTaskIfNeeded(); |
| const ownerTaskSnapshot = buildTaskStatusSnapshot( | ||
| listTasksForOwnerKey(key).map((task) => reconcileTaskRecordForOperatorInspection(task, now)), | ||
| { now }, | ||
| ); | ||
| const activeOwnerTask = ownerTaskSnapshot.active[0]; | ||
| const taskDrivenFocus = !subagentRun ? activeOwnerTask ?? ownerTaskSnapshot.focus : undefined; | ||
| const taskDrivenStatus = !subagentRun | ||
| ? mapTaskStatusToSessionRunStatus(taskDrivenFocus?.status) | ||
| : undefined; | ||
| const taskDrivenStartedAt = | ||
| typeof taskDrivenStatus === "string" | ||
| ? (taskDrivenFocus?.startedAt ?? taskDrivenFocus?.createdAt) | ||
| : undefined; | ||
| const taskDrivenEndedAt = | ||
| typeof taskDrivenStatus === "string" && taskDrivenStatus !== "running" | ||
| ? (taskDrivenFocus?.endedAt ?? taskDrivenFocus?.lastEventAt ?? now) | ||
| : undefined; | ||
| const taskDrivenRuntimeMs = | ||
| typeof taskDrivenStartedAt === "number" | ||
| ? Math.max( | ||
| 0, | ||
| (taskDrivenStatus === "running" ? now : (taskDrivenEndedAt ?? now)) - taskDrivenStartedAt, | ||
| ) | ||
| : undefined; | ||
| const taskDrivenUpdatedAt = | ||
| typeof taskDrivenStatus === "string" | ||
| ? (taskDrivenFocus?.lastEventAt ?? taskDrivenFocus?.endedAt ?? taskDrivenFocus?.startedAt ?? taskDrivenFocus?.createdAt) | ||
| : undefined; |
There was a problem hiding this comment.
buildGatewaySessionRow always builds an owner-task snapshot (including reconciliation) even when subagentRun is present, but then discards the task-driven fields. This adds unnecessary task-registry work per row. Wrap the listTasksForOwnerKey/buildTaskStatusSnapshot block in a if (!subagentRun) guard to avoid extra work when subagent metadata already drives the row.
| const ownerTaskSnapshot = buildTaskStatusSnapshot( | |
| listTasksForOwnerKey(key).map((task) => reconcileTaskRecordForOperatorInspection(task, now)), | |
| { now }, | |
| ); | |
| const activeOwnerTask = ownerTaskSnapshot.active[0]; | |
| const taskDrivenFocus = !subagentRun ? activeOwnerTask ?? ownerTaskSnapshot.focus : undefined; | |
| const taskDrivenStatus = !subagentRun | |
| ? mapTaskStatusToSessionRunStatus(taskDrivenFocus?.status) | |
| : undefined; | |
| const taskDrivenStartedAt = | |
| typeof taskDrivenStatus === "string" | |
| ? (taskDrivenFocus?.startedAt ?? taskDrivenFocus?.createdAt) | |
| : undefined; | |
| const taskDrivenEndedAt = | |
| typeof taskDrivenStatus === "string" && taskDrivenStatus !== "running" | |
| ? (taskDrivenFocus?.endedAt ?? taskDrivenFocus?.lastEventAt ?? now) | |
| : undefined; | |
| const taskDrivenRuntimeMs = | |
| typeof taskDrivenStartedAt === "number" | |
| ? Math.max( | |
| 0, | |
| (taskDrivenStatus === "running" ? now : (taskDrivenEndedAt ?? now)) - taskDrivenStartedAt, | |
| ) | |
| : undefined; | |
| const taskDrivenUpdatedAt = | |
| typeof taskDrivenStatus === "string" | |
| ? (taskDrivenFocus?.lastEventAt ?? taskDrivenFocus?.endedAt ?? taskDrivenFocus?.startedAt ?? taskDrivenFocus?.createdAt) | |
| : undefined; | |
| let taskDrivenFocus: | |
| | ReturnType<typeof buildTaskStatusSnapshot>["focus"] | |
| | ReturnType<typeof buildTaskStatusSnapshot>["active"][number] | |
| | undefined; | |
| let taskDrivenStatus: SessionRunStatus | undefined; | |
| let taskDrivenStartedAt: number | undefined; | |
| let taskDrivenEndedAt: number | undefined; | |
| let taskDrivenRuntimeMs: number | undefined; | |
| let taskDrivenUpdatedAt: number | undefined; | |
| if (!subagentRun) { | |
| const ownerTaskSnapshot = buildTaskStatusSnapshot( | |
| listTasksForOwnerKey(key).map((task) => | |
| reconcileTaskRecordForOperatorInspection(task, now), | |
| ), | |
| { now }, | |
| ); | |
| const activeOwnerTask = ownerTaskSnapshot.active[0]; | |
| taskDrivenFocus = activeOwnerTask ?? ownerTaskSnapshot.focus; | |
| taskDrivenStatus = mapTaskStatusToSessionRunStatus(taskDrivenFocus?.status); | |
| taskDrivenStartedAt = | |
| typeof taskDrivenStatus === "string" | |
| ? taskDrivenFocus?.startedAt ?? taskDrivenFocus?.createdAt | |
| : undefined; | |
| taskDrivenEndedAt = | |
| typeof taskDrivenStatus === "string" && taskDrivenStatus !== "running" | |
| ? taskDrivenFocus?.endedAt ?? taskDrivenFocus?.lastEventAt ?? now | |
| : undefined; | |
| taskDrivenRuntimeMs = | |
| typeof taskDrivenStartedAt === "number" | |
| ? Math.max( | |
| 0, | |
| (taskDrivenStatus === "running" ? now : taskDrivenEndedAt ?? now) - | |
| taskDrivenStartedAt, | |
| ) | |
| : undefined; | |
| taskDrivenUpdatedAt = | |
| typeof taskDrivenStatus === "string" | |
| ? (taskDrivenFocus?.lastEventAt ?? | |
| taskDrivenFocus?.endedAt ?? | |
| taskDrivenFocus?.startedAt ?? | |
| taskDrivenFocus?.createdAt) | |
| : undefined; | |
| } |
| if (!runId) { | ||
| return true; | ||
| } | ||
| return getTaskRegistryHooks()?.isCliRunActive?.(runId) ?? true; |
There was a problem hiding this comment.
hasBackingSession calls the configured isCliRunActive hook directly. If a hook implementation throws, it will bubble up and can break reconciliation/maintenance runs. Consider wrapping the hook call in a try/catch (similar to emitTaskRegistryHookEvent in task-registry.ts) and defaulting to true (or logging + true) on errors so maintenance remains resilient.
| return getTaskRegistryHooks()?.isCliRunActive?.(runId) ?? true; | |
| const hooks = getTaskRegistryHooks(); | |
| if (!hooks?.isCliRunActive) { | |
| return true; | |
| } | |
| try { | |
| return hooks.isCliRunActive(runId) ?? true; | |
| } catch (error) { | |
| // Ensure hook failures do not break maintenance/reconciliation runs. | |
| // eslint-disable-next-line no-console | |
| console.warn("task-registry: isCliRunActive hook threw, treating run as active", { | |
| runId, | |
| error, | |
| }); | |
| return true; | |
| } |
Greptile SummaryThis PR wires background One minor style issue was found (redundant second call to Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/bash-tools.process.ts
Line: 613-627
Comment:
**Redundant second `markCancelRequested` call**
`markCancelRequested` is already called on line 613 (before `cancelManagedSession`). The `else` branch on line 626 calls it again when `canceled` is `true`, but at that point `cancelRequestedByUser` is already `true` and the `stallTimer` was already cleared by the first call, so the second call is a no-op. The `else` branch can simply be removed.
```suggestion
markCancelRequested(scopedSession);
const canceled = cancelManagedSession(scopedSession.id);
if (!canceled) {
const terminated = terminateSessionFallback(scopedSession);
if (!terminated) {
scopedSession.cancelRequestedByUser = false;
return failText(
`Unable to terminate session ${params.sessionId}: no active supervisor run or process id.`,
);
}
markExited(scopedSession, null, "SIGKILL", "killed");
notifyExecSessionExit(scopedSession, "killed");
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Track background exec liveness with CLI ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| markCancelRequested(scopedSession); | ||
| const canceled = cancelManagedSession(scopedSession.id); | ||
| if (!canceled) { | ||
| const terminated = terminateSessionFallback(scopedSession); | ||
| if (!terminated) { | ||
| scopedSession.cancelRequestedByUser = false; | ||
| return failText( | ||
| `Unable to terminate session ${params.sessionId}: no active supervisor run or process id.`, | ||
| ); | ||
| } | ||
| markExited(scopedSession, null, "SIGKILL", "failed"); | ||
| markExited(scopedSession, null, "SIGKILL", "killed"); | ||
| notifyExecSessionExit(scopedSession, "killed"); | ||
| } else { | ||
| markCancelRequested(scopedSession); | ||
| } |
There was a problem hiding this comment.
markCancelRequested call
markCancelRequested is already called on line 613 (before cancelManagedSession). The else branch on line 626 calls it again when canceled is true, but at that point cancelRequestedByUser is already true and the stallTimer was already cleared by the first call, so the second call is a no-op. The else branch can simply be removed.
| markCancelRequested(scopedSession); | |
| const canceled = cancelManagedSession(scopedSession.id); | |
| if (!canceled) { | |
| const terminated = terminateSessionFallback(scopedSession); | |
| if (!terminated) { | |
| scopedSession.cancelRequestedByUser = false; | |
| return failText( | |
| `Unable to terminate session ${params.sessionId}: no active supervisor run or process id.`, | |
| ); | |
| } | |
| markExited(scopedSession, null, "SIGKILL", "failed"); | |
| markExited(scopedSession, null, "SIGKILL", "killed"); | |
| notifyExecSessionExit(scopedSession, "killed"); | |
| } else { | |
| markCancelRequested(scopedSession); | |
| } | |
| markCancelRequested(scopedSession); | |
| const canceled = cancelManagedSession(scopedSession.id); | |
| if (!canceled) { | |
| const terminated = terminateSessionFallback(scopedSession); | |
| if (!terminated) { | |
| scopedSession.cancelRequestedByUser = false; | |
| return failText( | |
| `Unable to terminate session ${params.sessionId}: no active supervisor run or process id.`, | |
| ); | |
| } | |
| markExited(scopedSession, null, "SIGKILL", "killed"); | |
| notifyExecSessionExit(scopedSession, "killed"); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/bash-tools.process.ts
Line: 613-627
Comment:
**Redundant second `markCancelRequested` call**
`markCancelRequested` is already called on line 613 (before `cancelManagedSession`). The `else` branch on line 626 calls it again when `canceled` is `true`, but at that point `cancelRequestedByUser` is already `true` and the `stallTimer` was already cleared by the first call, so the second call is a no-op. The `else` branch can simply be removed.
```suggestion
markCancelRequested(scopedSession);
const canceled = cancelManagedSession(scopedSession.id);
if (!canceled) {
const terminated = terminateSessionFallback(scopedSession);
if (!terminated) {
scopedSession.cancelRequestedByUser = false;
return failText(
`Unable to terminate session ${params.sessionId}: no active supervisor run or process id.`,
);
}
markExited(scopedSession, null, "SIGKILL", "killed");
notifyExecSessionExit(scopedSession, "killed");
}
```
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3abd6f2f51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1190,6 +1213,39 @@ export function buildGatewaySessionRow(params: { | |||
| const subagentStartedAt = subagentRun ? getSubagentSessionStartedAt(subagentRun) : undefined; | |||
| const subagentEndedAt = subagentRun ? subagentRun.endedAt : undefined; | |||
| const subagentRuntimeMs = subagentRun ? resolveSessionRuntimeMs(subagentRun, now) : undefined; | |||
| const ownerTaskSnapshot = buildTaskStatusSnapshot( | |||
| listTasksForOwnerKey(key).map((task) => reconcileTaskRecordForOperatorInspection(task, now)), | |||
There was a problem hiding this comment.
The new task-driven session mapping now pulls from listTasksForOwnerKey(key) across all runtimes, so any recent/active non-CLI task (for example ACP/subagent tasks owned by the same session) can overwrite the row's status, timing fields, and updatedAt. In practice this can make /sessions show a session as failed or running based on an unrelated task and keep it in activeMinutes results even when the session itself is done; the override should be scoped to the linked background CLI task(s) this change is targeting.
Useful? React with 👍 / 👎.
3abd6f2 to
723c6fe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 723c6fec85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const runId = task.runId?.trim(); | ||
| if (!runId) { | ||
| return true; | ||
| } | ||
| return getTaskRegistryObservers()?.isCliRunActive?.(runId) ?? true; |
There was a problem hiding this comment.
This branch now treats every runtime:"cli" task as backed only by isCliRunActive(runId), but the observer wired in src/gateway/server.impl.ts points to bash-process-registry and only knows exec/process sessions. Other CLI tasks (for example the gateway agent task created in src/gateway/server-methods/agent.ts:197-213) use different run IDs and will be reported missing, then reconciled to lost after the 5s CLI grace even while their real work is still active. That can incorrectly flip task/session status to failure.
Useful? React with 👍 / 👎.
| if (summary) { | ||
| const now = Date.now(); | ||
| session.lastTaskUpdateAt = now; | ||
| void loadTaskExecutorModule() | ||
| .then(({ recordTaskRunProgressByRunId }) => { |
There was a problem hiding this comment.
lastEventAt is only pushed to the task registry when summary is present, but callers pass undefined for normal ongoing output after the first chunk (only first output and stall-resume are summarized). For long-running commands that keep producing output, the task heartbeat stops advancing, so task-driven updatedAt becomes stale and active session filtering/status views can treat a live run as inactive.
Useful? React with 👍 / 👎.
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep this PR open because the background exec liveness problem is still meaningful and current main only partially covers adjacent stale-task maintenance, but this branch is not merge-ready: it conflicts with current main, lacks real behavior proof, and has correctness bugs in CLI task reconciliation and session-row status derivation. 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 detailsBest 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 established in this read-only review. Source inspection shows current main still lacks linked CLI task creation for background execs, while the PR provides targeted tests but not real-run proof. Is this the best way to solve the issue? No. The requested direction is valid, but this PR is currently too broad in task maintenance and session status projection; the safer fix must preserve current main's runtime-specific reconciliation and scope new status behavior to linked background exec tasks. Security review: Security review cleared: The diff changes existing exec/process and task-registry plumbing but adds no dependencies, workflows, secret handling, network calls, or new external code execution path beyond the existing exec tool surface. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against db5e415888ae. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
clawsweeper
Bot
added
P2
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
This pull request has been automatically marked as stale due to inactivity. |
clawsweeper
Bot
added
rating: 🌊 off-meta tidepool
clawsweeper
Bot
removed
the
rating: 🌊 off-meta tidepool
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
ClawSweeper applied the proposed close for this PR.
|
Summary
Describe the problem and fix in 2–5 bullets:
If this PR fixes a plugin beta-release blocker, title it
fix(<plugin-id>): beta blocker - <summary>and link the matchingBeta blocker: <plugin-name> - <summary>issue labeledbeta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.exec/processruns could continue after the parent lane looked idle/reset, but there was no first-class linkedruntime:"cli"task keeping liveness, cancellation, and terminal state visible./statussurface that looked done or dead while repo-mutating background work was still running, stalled, or only partially cancelled.process removepreserves visibility until actual exit, session rows/status surfaces map linked CLI task state, and task maintenance reconciles orphaned CLI runs through a runtime hook with a CLI-specific grace period.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause / Regression History (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.exec→processpath was not wired into the task registry/status surfaces, so background work could outlive the parent lane without a durable CLI task reflecting running/stalled/cancelled/completed state.git blame, prior PR, issue, or refactor if known):#58810fixed stale task rows in status cards, but it intentionally did not add CLI task creation/progress plumbing;#59672hardened task maintenance, which made the missing CLI-runtime seam more important to handle explicitly.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should have caught this. Otherwise write
N/A.src/agents/bash-tools.test.ts,src/agents/bash-tools.exec-runtime.test.ts,src/agents/bash-tools.process.supervisor.test.ts,src/tasks/task-registry.test.ts,src/gateway/session-utils.test.ts,src/agents/openclaw-tools.session-status.test.tsdone.session_statusand task-registry coverage helped validate the status surface once the new CLI-task path was linked in, but did not previously cover the background-exec seam itself.User-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.process removeno longer makes a live run immediately look gone; while termination is pending it remains inspectable and reportsTermination requested; process still running.killed/failedinstead of flattening them to plaindone.Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Security Impact (required)
Yes, explain risk + mitigation: N/ARepro + Verification
Environment
Steps
processbehavior plus linked task/session status while the run is active, silent, or being removed/cancelled.Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
killed/failed, and recent missing CLI runs stayingrunninguntil the CLI-specific grace expires.Review Conversations
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
Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.process removeas immediate disappearance now see a temporary running/termination-requested state until actual exit.isCliRunActive(runId)hook and applies a CLI-specific grace window before markinglost.