Skip to content

tasks: extract detached task lifecycle runtime#68886

Merged
mbelinky merged 2 commits intomainfrom
mariano/detached-executor-seam
Apr 19, 2026
Merged

tasks: extract detached task lifecycle runtime#68886
mbelinky merged 2 commits intomainfrom
mariano/detached-executor-seam

Conversation

@mbelinky
Copy link
Copy Markdown
Contributor

Summary

  • Problem: detached task lifecycle calls were imported directly from src/tasks/task-executor.ts across ACP, cron, gateway, subagent, and background-task paths, leaving no clean seam for alternate detached-work execution.
  • Why it matters: we want to explore plugin-backed or externally managed detached execution without forking Tasks/TaskFlow or duplicating task state logic in core.
  • What changed: added src/tasks/detached-task-runtime.ts as a behavior-preserving lifecycle seam, rerouted detached-run callers to it, and added seam contract coverage.
  • What did NOT change (scope boundary): no task schema changes, no TaskFlow changes, no executor semantics changes, no retry/crash-recovery/stall-detection implementation, and no plugin registration yet.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: detached-work lifecycle was coupled directly to core task-executor imports in each caller, so there was no narrow override point for alternate executor/runtime ownership.
  • Missing detection / guardrail: no seam-contract test existed to prove callers routed through an overridable lifecycle runtime.
  • Contributing context (if known): PR minions: durable SQLite-backed job queue for subagents, ACP, CLI, and cron #68718 explored a second detached execution substrate; this PR is the first extraction step so that future work can extend Tasks rather than bypass them.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file:
    • src/tasks/detached-task-runtime.test.ts
    • src/gateway/server-methods/agent.test.ts
    • Existing touched-surface tests in ACP, cron, agents, and gateway
  • Scenario the test should lock in: detached-run callers use the seam, and existing task-registry behavior is preserved after rerouting imports.
  • Why this is the smallest reliable guardrail: it proves both the seam dispatch itself and one representative caller path, while existing behavior tests cover the rest of the touched surfaces.
  • Existing test that already covers this (if any): ACP manager, gateway async agent runs, cron manual/timer runs, ACP spawn, subagent lifecycle, media background flows, and context-engine maintenance already covered preserved behavior.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

None.

Diagram (if applicable)

Before:
[detached caller] -> task-executor lifecycle functions

After:
[detached caller] -> detached-task-runtime seam -> default task-executor lifecycle functions

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:

Repro + Verification

Environment

  • OS: macOS on mb-server
  • Runtime/container: Node 22 / pnpm workspace
  • Model/provider: N/A
  • Integration/channel (if any): gateway, ACP, cron, subagent runtime paths
  • Relevant config (redacted): default test harness config

Steps

  1. Add src/tasks/detached-task-runtime.ts as a delegating lifecycle seam.
  2. Reroute detached lifecycle call sites to import the seam instead of task-executor directly.
  3. Add a seam-contract unit test and a gateway caller-level seam dispatch test.
  4. Rebase onto latest origin/main and run the targeted touched-surface tests.

Expected

  • Existing detached task behavior remains unchanged.
  • Callers can be intercepted through the seam.

Actual

  • Targeted tests passed.
  • pnpm build on mb-server hit an existing runtime-postbuild bundled-plugin dependency staging issue in the installed workspace graph, not an error in the seam diff itself.

Evidence

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

Human Verification (required)

  • Verified scenarios:
    • targeted tests for tasks seam, gateway agent, ACP manager, cron ops/timer, ACP spawn, subagent lifecycle, sessions-spawn lifecycle, media background flows, and context-engine maintenance
    • rebase onto latest origin/main
    • branch push to origin
  • Edge cases checked:
    • seam dispatch can be overridden in tests
    • gateway async run still creates task records through the seam
    • cron still tolerates ledger create/update failures through the seam
  • What you did not verify:
    • full repo-wide build on mb-server due unrelated runtime-postbuild dependency staging issue in the installed workspace graph
    • any alternate executor/plugin implementation beyond the seam extraction

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.

Compatibility / Migration

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

Risks and Mitigations

  • Risk: future callers might bypass the seam and import task-executor directly again.
    • Mitigation: added direct seam dispatch coverage plus a representative caller-level seam test.
  • Risk: this seam alone does not solve executor ownership, crash recovery, retry, or stall detection.
    • Mitigation: kept scope explicit; those belong in follow-up work, not hidden inside this PR.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels Apr 19, 2026
@mbelinky mbelinky marked this pull request as ready for review April 19, 2026 08:56
@mbelinky mbelinky merged commit 0787266 into main Apr 19, 2026
37 of 38 checks passed
@mbelinky mbelinky deleted the mariano/detached-executor-seam branch April 19, 2026 08:56
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 19, 2026

Greptile Summary

This PR extracts a detachedTaskLifecycleRuntime seam module (src/tasks/detached-task-runtime.ts) that wraps the seven task-executor lifecycle functions and routes all detached-run callers (gateway, ACP, cron, subagent, ACP spawn, media background, context-engine) through it. The change is purely structural — runtime behavior is delegated to the existing task-executor core by default — with a setDetachedTaskLifecycleRuntime override point and matching seam-contract tests added. No schema, TaskFlow, or executor semantics are altered.

Confidence Score: 5/5

Safe to merge — behavior-preserving refactor with thorough seam-contract coverage and consistent caller rerouting.

All findings are P2 or lower. The seam implementation is correct, delegation is faithful to the original calls, reset hygiene in tests is sound, and the gateway seam-dispatch test proves one representative caller path in addition to the unit-level contract test.

No files require special attention.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/tasks/detached-task-runtime.ts
Line: 41-43

Comment:
**Test helper exported from production module**

`resetDetachedTaskLifecycleRuntimeForTests` is exported from the main runtime module rather than a test-only entry point, so it will be included in the production bundle. Consider co-locating it in a `detached-task-runtime.test-helpers.ts` file (as done elsewhere in the codebase) to keep the production surface clean. The function name suffix `ForTests` signals the intent, but the export boundary does not enforce it.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "tests: relax gateway seam expectation" | Re-trigger Greptile

Comment on lines +41 to +43
export function resetDetachedTaskLifecycleRuntimeForTests(): void {
detachedTaskLifecycleRuntime = DEFAULT_DETACHED_TASK_LIFECYCLE_RUNTIME;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test helper exported from production module

resetDetachedTaskLifecycleRuntimeForTests is exported from the main runtime module rather than a test-only entry point, so it will be included in the production bundle. Consider co-locating it in a detached-task-runtime.test-helpers.ts file (as done elsewhere in the codebase) to keep the production surface clean. The function name suffix ForTests signals the intent, but the export boundary does not enforce it.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/tasks/detached-task-runtime.ts
Line: 41-43

Comment:
**Test helper exported from production module**

`resetDetachedTaskLifecycleRuntimeForTests` is exported from the main runtime module rather than a test-only entry point, so it will be included in the production bundle. Consider co-locating it in a `detached-task-runtime.test-helpers.ts` file (as done elsewhere in the codebase) to keep the production surface clean. The function name suffix `ForTests` signals the intent, but the export boundary does not enforce it.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

jinon86 pushed a commit to jinon86/openclaw that referenced this pull request Apr 19, 2026
* test: stabilize standalone Parallels smoke lanes

* fix: strip orphaned OpenAI reasoning blocks before responses API call (openclaw#55787)

Merged via squash.

Prepared head SHA: 263b952
Co-authored-by: suboss87 <11032439+suboss87@users.noreply.github.com>
Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com>
Reviewed-by: @jalehman

* fix: stabilize release smoke reruns

* test: tolerate empty fireworks live responses

* test: make OpenWebUI smoke deterministic

* tasks: extract detached task lifecycle runtime (openclaw#68886)

* tasks: extract detached task lifecycle runtime

* tests: relax gateway seam expectation

---------

Co-authored-by: Mariano Belinky <mariano@mb-server-643.local>

* test: complete workspace setup in update smokes

* fix: parse PowerShell cron tools allow-list (openclaw#68858) (thanks @chen-zhang-cs-code)

* fix(cron): parse PowerShell tools allow list

* fix(cron): clarify tools allow-list help

* fix: parse PowerShell cron tools allow-list (openclaw#68858) (thanks @chen-zhang-cs-code)

---------

Co-authored-by: Ayaan Zaidi <hi@obviy.us>

* fix(browser): discover CDP websocket from bare ws:// URL before attach (openclaw#68715)

* fix(browser): discover CDP websocket from bare ws:// URL before attach

When browser.cdpUrl is set to a bare ws://host:port (no /devtools/ path), ensureBrowserAvailable would call isChromeReachable -> canOpenWebSocket against the URL verbatim. Chrome only accepts WebSocket upgrades at the specific path returned by /json/version, so the handshake failed immediately with HTTP 400. With attachOnly: true, that surfaced as:

  Browser attachOnly is enabled and profile "openclaw" is not running.

even though the CDP endpoint was reachable and the profile was healthy. Reproduced by the new tests in chrome.test.ts and cdp.test.ts (openclaw#68027).

Fix: introduce isDirectCdpWebSocketEndpoint(url) — true only when a ws/wss URL has a /devtools/<kind>/<id> handshake path. Route any other ws/wss cdpUrl (including the bare ws://host:port shape) through HTTP /json/version discovery by normalising the scheme via the existing normalizeCdpHttpBaseForJsonEndpoints helper. Apply this in isChromeReachable, getChromeWebSocketUrl, and createTargetViaCdp. Direct WS endpoints with a /devtools/ path are still opened without an extra discovery round-trip.

Fixes openclaw#68027

* test(browser): add seeded fuzz coverage for CDP URL helpers

Adds property-based / seeded-fuzz tests for the URL helpers the
attachOnly CDP fix depends on (openclaw#68027):

  - isWebSocketUrl
  - isDirectCdpWebSocketEndpoint
  - normalizeCdpHttpBaseForJsonEndpoints
  - parseBrowserHttpUrl
  - redactCdpUrl
  - appendCdpPath
  - getHeadersWithAuth

Follows the existing repo convention (see
src/gateway/http-common.fuzz.test.ts): no fast-check dep, small
mulberry32 PRNG + hand-rolled generators, deterministic per-describe
seeds so failures are reproducible.

Lifts cdp.helpers.ts coverage from 77.77% -> 89.54% statements,
67.9% -> 80.24% branches, 78% -> 90% lines. Remaining uncovered
lines are inside the WS sender internals (createCdpSender,
withCdpSocket, fetchCdpChecked rate-limit branch), which require
integration-style mocks and are unrelated to the attachOnly fix.

* test(browser): drive cdp.helpers/cdp/chrome to 100% coverage

Lifts the three files touched by the openclaw#68027 attachOnly fix to 100% statements/branches/functions/lines across the extensions test suite. Adds cdp.helpers.internal.test.ts, cdp.internal.test.ts, and chrome.internal.test.ts covering error paths, branch matrices, CDP session helpers, Chrome spawn/launch/stop flows, and canRunCdpHealthCommand. Defensively unreachable guards are annotated with c8 ignore + inline justifications.

* fix(browser): restore WS fallback for non-/devtools ws:// CDP URLs

When /json/version discovery is unavailable (or returns no
webSocketDebuggerUrl), fall back to treating the original bare ws/wss
URL as a direct WebSocket endpoint. This preserves the openclaw#68027 fix for
Chrome's debug port while restoring compatibility with Browserless/
Browserbase-style providers that expose a direct WebSocket root without
a /json/version endpoint.

Priority order for bare ws/wss cdpUrl inputs:
  1. /devtools/<kind>/<id> URL \u2192 direct handshake, no discovery (unchanged)
  2. bare ws/wss root \u2192 try HTTP discovery first; if discovery returns a
     webSocketDebuggerUrl use it; otherwise fall back to the original URL
     as a direct WS endpoint
  3. HTTP/HTTPS URL \u2192 HTTP discovery only, no fallback (unchanged)

Affected call sites: isChromeReachable, getChromeWebSocketUrl,
createTargetViaCdp.

Also renames a misleading test ('still enforces SSRF policy for direct
WebSocket URLs') to accurately describe what it tests: SSRF enforcement
on the navigation target URL, not on the CDP endpoint.

New tests added for all three fallback paths. Coverage remains 100% on
all three touched files (238 tests).

* fix: browser attachOnly bare ws CDP follow-ups (openclaw#68715) (thanks @visionik)

* test(tasks): align detached runtime mock return types

* browser: route existing-session user profile through browser nodes (openclaw#68891)

* browser: route user profile through browser nodes

* browser: align existing-session node docs

* browser: preserve host fallback on node discovery errors

* browser: preserve configured node pin errors

* browser: widen config mock in node pin test

* fix: default kimi thinking to off (openclaw#68907)

Co-authored-by: termtek <termtek@ubuntu.tail2b72cd.ts.net>

* docs(changelog): note kimi thinking default fix

* docs(changelog): add thanks for kimi fix

* tasks: add detached runtime plugin registration contract (openclaw#68915)

* tasks: register detached runtime plugins

* tasks: harden detached runtime ownership

* tasks: extract detached runtime contract types

* changelog: note detached runtime contract

* changelog: attribute detached runtime contract

* feat(ui): add a2a operator dashboard shell

* fix(ui): align a2a baseline with current read contract

* feat(ui): add A2A case inspector panels

* CI: route small workflows to ubuntu-latest

* CI: route fork-blocked workflows to GitHub-hosted runners

* CI: fall back to GITHUB_TOKEN for fork automation

* CI: skip app-token steps when fork secrets are absent

* CI: gate fork app-token steps through env guards

* CI: fix stale fallback env guard

* fix(ui): sync A2A locale snapshots

* fix(ui): sync A2A locale metadata

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: Subash Natarajan <suboss87@gmail.com>
Co-authored-by: suboss87 <11032439+suboss87@users.noreply.github.com>
Co-authored-by: jalehman <550978+jalehman@users.noreply.github.com>
Co-authored-by: Mariano <132747814+mbelinky@users.noreply.github.com>
Co-authored-by: Mariano Belinky <mariano@mb-server-643.local>
Co-authored-by: ZC <chenzhangcode@163.com>
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
Co-authored-by: Viz <visionik@pobox.com>
Co-authored-by: Frank Yang <frank.ekn@gmail.com>
Co-authored-by: termtek <termtek@ubuntu.tail2b72cd.ts.net>
Co-authored-by: bangtong-ai <bangtong-ai@users.noreply.github.com>
Co-authored-by: OpenClaw Agent <openclaw-agent@users.noreply.github.com>
Co-authored-by: OpenClaw Bot <bot@openclaw.local>
Mquarmoc pushed a commit to Mquarmoc/openclaw that referenced this pull request Apr 20, 2026
* tasks: extract detached task lifecycle runtime

* tests: relax gateway seam expectation

---------

Co-authored-by: Mariano Belinky <mariano@mb-server-643.local>
lovewanwan pushed a commit to lovewanwan/openclaw that referenced this pull request Apr 28, 2026
* tasks: extract detached task lifecycle runtime

* tests: relax gateway seam expectation

---------

Co-authored-by: Mariano Belinky <mariano@mb-server-643.local>
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
* tasks: extract detached task lifecycle runtime

* tests: relax gateway seam expectation

---------

Co-authored-by: Mariano Belinky <mariano@mb-server-643.local>
medaimane pushed a commit to medaimane/openclaw that referenced this pull request May 4, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
* tasks: extract detached task lifecycle runtime

* tests: relax gateway seam expectation

---------

Co-authored-by: Mariano Belinky <mariano@mb-server-643.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling gateway Gateway runtime maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant