feat(nodes): continue Claude catalog sessions on paired nodes via streaming CLI agent runs#105833
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ec156f205
ℹ️ 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 child = spawn(invocation.command, invocation.args, { | ||
| cwd: params.cwd, | ||
| env: params.env, |
There was a problem hiding this comment.
Return a result when node Claude spawn throws
If the catalog-reported cwd is replaced with a regular file after decodeClaudeCliNodeRunParams validates it (or another spawn option throws synchronously), spawn(...) throws before the error/close handlers are attached. That rejection bubbles to handleInvoke's top-level logger without sending node.invoke.result, so the gateway-side streaming invoke sits until its hard timeout instead of failing the turn immediately; wrap this spawn in the same try/catch style used by system.run and resolve a failed RunResult.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 9:50 AM ET / 13:50 UTC. Summary PR surface: Source +2025, Tests +1469, Docs +62, Generated 0, Other +30. Total +3586 across 68 files. Reproducibility: not applicable. this is a new paired-node execution capability rather than a current-main bug with a failing reproduction path. The existing view-only behavior is the documented starting limitation. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Keep the Claude node runner behind a dedicated non-generic seam, require operator.admin for adopting node-bound sessions, preserve only explicitly safe backend-prepared environment, resolve the current conflicts, and demonstrate the complete flow on a real paired node before merge. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a new paired-node execution capability rather than a current-main bug with a failing reproduction path. The existing view-only behavior is the documented starting limitation. Is this the best way to solve the issue? No, not yet. The dedicated paired-node execution direction is plausible, but the current generic invocability claim, write-versus-admin authorization mismatch, and skipped backend preparation make this implementation unsafe as the permanent contract. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a965d28e0409. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +2025, Tests +1469, Docs +62, Generated 0, Other +30. Total +3586 across 68 files. View PR surface stats
Security concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (9 earlier review cycles; latest 8 shown)
|
7a0429d to
2e3cbe2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e3cbe2852
ℹ️ 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".
| if ( | ||
| nodeRunTruncated && | ||
| result.exitCode === 0 && | ||
| !result.timedOut && | ||
| !streamingParser?.getOutput() | ||
| ) { |
There was a problem hiding this comment.
Reject truncated node streams without terminal results
When a node Claude run streams enough assistant text to hit the node-side cap and then loses the terminal result line (for example because the final result line is too large to re-inject), streamingParser.getOutput() can still return partial assistant text from earlier deltas, so this guard does not fire. The turn is then accepted as a clean exit with truncated content and no terminal result/session/usage proof; this needs to check whether a terminal result was actually observed or re-injected, not just whether any partial output parsed.
Useful? React with 👍 / 👎.
2e3cbe2 to
038dbf6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 038dbf6562
ℹ️ 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".
| }); | ||
| }); | ||
| child.stderr.on("data", (raw: Buffer) => { | ||
| retain(raw); |
There was a problem hiding this comment.
Keep stderr out of the stdout stream cap
When the node-side Claude process writes a large stderr burst before or during stdout, this call consumes the shared outputBytes budget used by retain(). Once that 200KB budget is exhausted, later stdout chunks are dropped or reduced to heartbeats, so a valid Claude JSONL response can arrive without the assistant deltas even though stderr is already tailed separately in stderrTail; track stderr tail bytes independently from the stdout progress cap.
Useful? React with 👍 / 👎.
038dbf6 to
81b401d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81b401d14c
ℹ️ 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".
| ...(record.cwd ? { spawnedCwd: record.cwd } : {}), | ||
| ...(nodeId ? { execNode: nodeId, ...(record.cwd ? { execCwd: record.cwd } : {}) } : {}), |
There was a problem hiding this comment.
Keep node cwd out of gateway spawnedCwd
When hostId is a paired node, record.cwd is the node-local project path. This node binding still leaves that same value in spawnedCwd from the previous line, but gateway callers such as resolveSessionRuntimeCwd and resolveIngressWorkspaceOverrideForSessionRun treat spawnedCwd as a local gateway workspace; adopting a node session whose cwd does not exist (or points elsewhere) on the gateway can break reset/bootstrap/file/diff flows or run them against the wrong local path. Store node paths only in execCwd for node-bound sessions (as sessions.create does) and avoid setting spawnedCwd for this branch.
Useful? React with 👍 / 👎.
81b401d to
a00f104
Compare
…ated drift into surface budget
34646a9 to
8c9fb66
Compare
|
Merged via squash.
|
…eaming CLI agent runs (openclaw#105833) * feat(gateway): stream node.invoke progress with idle timeouts and cancel * feat(node-host): opt-in approval-gated claude cli agent run command * feat(agents): run node-placed claude-cli turns through streaming node invoke * feat(anthropic): continue claude catalog sessions on advertising nodes * docs(nodes): document paired-node claude agent runs and continuation * chore(plugin-sdk): admit spawn-invocation helper into surface budget * fix(nodes): harden streaming invoke and node run policy from review findings * test(node-host): prove multi-token tool-policy argv fails closed * fix(nodes): allowlist cancel event for apps and break node-host type cycle * chore(protocol): regenerate Swift gateway models for node invoke progress * chore(node-host): drop test-only export surface flagged by deadcode ratchet * style(node-host): merge invoke-types imports * refactor(nodes): split node agent run modules to satisfy the LOC ratchet * chore(protocol): regenerate android gateway models and sync export baseline * chore(plugin-sdk): admit spawn-invocation helper and inherited deprecated drift into surface budget * chore(plugin-sdk): tolerate inherited agent-core deprecated export * fix(anthropic): drop node param parser orphaned by upstream catalog split * ci: sync unused-export baseline with healed main * test(agents): adopt renamed cli backend prepare helper * style(node-host): format claude run spawn invocation * ci: retrigger checks * chore(plugin-sdk): pin surface budgets to healed-base counts * fix(protocol): restore untyped lazy validators after upstream style change
…eaming CLI agent runs (openclaw#105833) * feat(gateway): stream node.invoke progress with idle timeouts and cancel * feat(node-host): opt-in approval-gated claude cli agent run command * feat(agents): run node-placed claude-cli turns through streaming node invoke * feat(anthropic): continue claude catalog sessions on advertising nodes * docs(nodes): document paired-node claude agent runs and continuation * chore(plugin-sdk): admit spawn-invocation helper into surface budget * fix(nodes): harden streaming invoke and node run policy from review findings * test(node-host): prove multi-token tool-policy argv fails closed * fix(nodes): allowlist cancel event for apps and break node-host type cycle * chore(protocol): regenerate Swift gateway models for node invoke progress * chore(node-host): drop test-only export surface flagged by deadcode ratchet * style(node-host): merge invoke-types imports * refactor(nodes): split node agent run modules to satisfy the LOC ratchet * chore(protocol): regenerate android gateway models and sync export baseline * chore(plugin-sdk): admit spawn-invocation helper and inherited deprecated drift into surface budget * chore(plugin-sdk): tolerate inherited agent-core deprecated export * fix(anthropic): drop node param parser orphaned by upstream catalog split * ci: sync unused-export baseline with healed main * test(agents): adopt renamed cli backend prepare helper * style(node-host): format claude run spawn invocation * ci: retrigger checks * chore(plugin-sdk): pin surface budgets to healed-base counts * fix(protocol): restore untyped lazy validators after upstream style change
What Problem This Solves
Claude Code catalog sessions on paired nodes were view-only:
continueClaudeSessionrejected anyhostIdother thangateway:local, so a session whose files live on another machine could be browsed but never continued. Closes #105529.Why This Change Was Made
Continuing a Claude session must run the real
claudeCLI on the machine that owns the session files (its~/.claude, its auth) — OpenClaw stays a frontend and never substitutes its own inference. This PR ships the full TS stack for that, opt-in and approval-gated end to end:node.invoke.progressevents ({invokeId, nodeId, seq, chunk}) with strict seq ordering, bounded out-of-order buffering, idle-timeout reset on progress, hard-cap timeout, andnode.invoke.cancel. Legacy single-result invokes stay byte-identical (cancel frames are gated to streaming invokes). Progress handlers are node-scoped and bind invokeId to the owning node's connection — no cross-node injection.agent.cli.claude.run.v1(TS node host): advertised only when the node-local confignodeHost.agentRuns.claude.enabledis true AND theclaudebinary resolves on the node — the gateway can never force it on. Requests are validated against a strict Claude flag allowlist (bypassPermissionsrejected in both arg forms), env passthrough is allowlisted, cwd must exist, and every run flows through the existingsystem.runexec-approval policy with the exact validated argv. Output is capped (200KB, 16KB chunks with backpressure), stderr tailed, process tree SIGKILLed on cancel/timeout, and the system prompt is materialized in a node-local 0600 temp file that is always cleaned up.execHost === "node"run one-shotclaude -pturns through the streaming invoke, feeding the same JSONL parser as local runs (session-id capture, fork markers, tool events unchanged). Warm stdio, gateway-loopback MCP, skills dirs, reseed-from-transcript, and attachments are cleanly disabled for node runs (rejected or omitted, double-enforced by the node-side allowlist); local runs are byte-identical.execNode/execCwdthrough the trusted creation seam. First-turn--fork-sessionsemantics identical to local adoption.Mac-app (Swift) nodes simply never advertise the command and remain view-only until the app ships it.
User Impact
With a headless node host opted in (
nodeHost.agentRuns.claude.enabled: true+claudeinstalled), Claude Code sessions on that machine appear in the session catalog and can be continued like native sessions: each turn runs on the node with the node's Claude auth and settings, streams output live, and respects the node's exec approval policy. Nothing changes for nodes that do not opt in; existing node/gateway deployments see zero wire-format changes.Evidence
check:changed(12m21s),pnpm build— all green pre-review.node.invoke.cancelfor legacy invokes too) is fixed — cancel frames now gate on the streaming contract.--tools+ MCP deny projected;--allowedToolsauto-approval still never crosses the node boundary); fresh node CLI sessions reseed conversation history from the gateway-side transcript; and a truncated node stream that lost the terminal result line now fails the turn instead of passing as a clean exit.plugin-sdk:api:checkandplugin-sdk:surface:checkgreen (surface budget +1 forresolveChildProcessInvocation, now public via theprocess-runtimewildcard — needed by the node host for Windows shim-correct spawns).git diff --checkclean.Known follow-ups (deliberate v1 scope, in the tracking issue): Mac-app Swift command, warm stdio on nodes, gateway-loopback MCP over node transport, Codex app-server on nodes, approval-request withdrawal when a run aborts mid-wait, FailoverError classification for node disconnects.