Skip to content

[codex] feat(mcp): share bundled runtime scope and fix preflight compaction abort#95563

Closed
yu-xin-c wants to merge 1 commit into
openclaw:mainfrom
yu-xin-c:codex/95553-95506-harness
Closed

[codex] feat(mcp): share bundled runtime scope and fix preflight compaction abort#95563
yu-xin-c wants to merge 1 commit into
openclaw:mainfrom
yu-xin-c:codex/95553-95506-harness

Conversation

@yu-xin-c

@yu-xin-c yu-xin-c commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #95553 by keeping required preflight/budget compaction from inheriting the reply operation abort signal. The compaction path now relies on the embedded compaction safety timeout, so agents.defaults.compaction.timeoutSeconds is no longer silently capped by a shorter reply-operation abort.

Addresses #95506 by adding an opt-in mcp.runtimeScope: "shared" mode for bundled MCP runtimes. Sessions with the same workspace and MCP config fingerprint can share one MCP runtime, collapsing duplicate stdio child processes, catalog caches, and repeated tools/list work. The default remains "session".

This overlaps the product direction in #79882. This draft rebases that shape onto current main, keeps the mode opt-in, and includes the primary configuration-reference warning that shared mode trades per-session MCP isolation for single-tenant runtime reuse.

Why This Change Was Made

Preflight compaction already has a configurable safety timeout, but the preflight caller passed replyOperation.abortSignal, letting unrelated reply-operation cancellation win before the compaction timeout could be honored.

Bundled MCP runtimes were keyed only by session id. In multi-session single-tenant deployments, identical workspace/config sessions therefore fanned out into duplicate MCP runtime objects, catalogs, stdio transports, and tools/list requests.

User Impact

  • Slow but valid preflight compactions can run up to the configured compaction timeout instead of failing around the reply-operation abort window.
  • Existing MCP behavior is unchanged unless mcp.runtimeScope is set.
  • Operators can set mcp.runtimeScope: "shared" in trusted single-tenant deployments to reuse warm bundled MCP clients across matching sessions.
  • Docs warn that shared mode lets MCP client, child-process, server-side, and failure state cross attached sessions.

Evidence

  • node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts src/agents/embedded-agent-runner.compaction-safety-timeout.test.ts src/agents/agent-bundle-mcp-runtime.test.ts src/config/schema.test.ts src/config/schema.hints.test.ts src/config/schema.help.quality.test.ts src/config/doc-baseline.test.ts src/config/doc-baseline.integration.test.ts
  • corepack pnpm tsgo:core
  • corepack pnpm tsgo:test:src
  • corepack pnpm config:docs:check
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime agents Agent runtime and tooling size: L labels Jun 21, 2026
@clawsweeper

clawsweeper Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 10:05 AM ET / 14:05 UTC.

Summary
The branch removes the reply-operation abort signal from required preflight compaction, adds an opt-in mcp.runtimeScope: "shared" setting for bundled MCP runtime reuse, and updates related config docs/tests.

PR surface: Source +156, Tests +261, Docs +14, Generated 0. Total +431 across 12 files.

Reproducibility: yes. at source level: current main passes the reply-operation abort into required budget compaction and keys bundled MCP runtimes by session id. I did not run a live slow-provider compaction or MCP process-reuse reproduction in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Config surface: 1 added. mcp.runtimeScope is a new openclaw.json runtime policy knob, so compatibility and isolation semantics need maintainer acceptance before merge.
  • Linked work streams: 2 combined. The branch combines a P1 compaction bug fix with a larger MCP runtime-sharing feature, so proof and review scope are materially broader than either change alone.

Stored data model
Persistent data-model change detected: persistent cache schema: src/config/schema.help.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted runtime logs or terminal output showing required preflight compaction running past the old reply-operation abort window and resolving by compaction.timeoutSeconds.
  • [P1] Add real setup proof that mcp.runtimeScope: "shared" reuses one bundled MCP runtime/catalog/process set for matching sessions while default session scope remains isolated.
  • [P1] Get maintainer direction on whether to split the P1 compaction fix from the opt-in shared MCP runtime feature.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists unit/type/docs/check commands, but it does not include after-fix real OpenClaw runtime proof for slow preflight compaction or shared MCP process/catalog reuse. 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.

Risk before merge

  • [P1] The PR adds a new operator-facing mcp.runtimeScope config surface and changes bundled MCP runtime lifecycle semantics when operators opt in.
  • [P1] Shared mode intentionally allows MCP clients, stdio child processes, server-side state, and failure/backoff state to cross attached sessions with the same workspace/config fingerprint.
  • [P2] Removing the reply-operation abort from required preflight compaction can make stop, restart, shutdown, or gateway-drain behavior wait for the compaction timeout unless maintainers define another cancellation route.
  • [P1] The PR body lists tests and checks, but it does not show after-fix real OpenClaw runtime proof for slow preflight compaction or shared MCP process/catalog reuse.

Maintainer options:

  1. Split The Narrow Bug Fix (recommended)
    Land the preflight abort repair separately, then review shared MCP runtime as its own opt-in product and security-boundary change with real proof.
  2. Approve And Prove The Combined Boundary
    Maintainers can keep the combined PR if they explicitly accept the shared-runtime isolation tradeoff and require redacted runtime proof for both changed behaviors before merge.
  3. Pause Shared Runtime Mode
    If per-session MCP client and child-process isolation must remain strict, pause or close the shared-runtime portion and pursue narrower latency/resource fixes instead.

Next step before merge

  • [P1] Human maintainer review is needed because the remaining blockers are proof, product/security acceptance of shared MCP scope, and cancellation policy rather than a narrow autonomous repair.

Security
Needs attention: The diff has no supply-chain change, but the opt-in shared MCP runtime changes a session isolation boundary and needs explicit maintainer acceptance.

Review details

Best possible solution:

Split or explicitly approve the two surfaces: land the preflight abort repair under a defined cancellation contract, and merge shared MCP runtime only after maintainers accept the opt-in single-tenant isolation boundary with real runtime proof.

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

Yes at source level: current main passes the reply-operation abort into required budget compaction and keys bundled MCP runtimes by session id. I did not run a live slow-provider compaction or MCP process-reuse reproduction in this read-only review.

Is this the best way to solve the issue?

Unclear as a combined PR: removing the preflight abort is a plausible call-site repair, but shared MCP runtime is a new opt-in isolation boundary that needs maintainer approval and real behavior proof before it can be called the best fix.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a P1 linked compaction bug that can repeatedly block large-session agent turns, even though it also includes a lower-priority feature.
  • merge-risk: 🚨 compatibility: The PR adds mcp.runtimeScope and changes bundled MCP runtime lifecycle behavior when operators enable shared mode.
  • merge-risk: 🚨 security-boundary: Shared mode lets matching sessions reuse the same MCP clients, child processes, server-side state, and failure state.
  • merge-risk: 🚨 availability: Shared MCP runtime failures can affect multiple attached sessions, and the preflight change can extend cancellation latency to the compaction timeout.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab 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 body lists unit/type/docs/check commands, but it does not include after-fix real OpenClaw runtime proof for slow preflight compaction or shared MCP process/catalog reuse. 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.
Evidence reviewed

PR surface:

Source +156, Tests +261, Docs +14, Generated 0. Total +431 across 12 files.

View PR surface stats
Area Files Added Removed Net
Source 6 219 63 +156
Tests 2 261 0 +261
Docs 3 23 9 +14
Config 0 0 0 0
Generated 1 2 2 0
Other 0 0 0 0
Total 12 505 74 +431

Security concerns:

  • [medium] Shared MCP runtime crosses session isolation — src/agents/agent-bundle-mcp-runtime.ts:1062
    When runtimeScope is shared, sessions with the same workspace and config attach to one MCP runtime, so MCP clients, child processes, server-side state, and failure state can cross session boundaries.
    Confidence: 0.88

Acceptance criteria:

  • [P2] node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts src/agents/embedded-agent-runner.compaction-safety-timeout.test.ts src/agents/agent-bundle-mcp-runtime.test.ts src/config/schema.test.ts src/config/schema.hints.test.ts src/config/schema.help.quality.test.ts src/config/doc-baseline.test.ts src/config/doc-baseline.integration.test.ts.
  • [P1] corepack pnpm tsgo:core.
  • [P1] corepack pnpm tsgo:test:src.
  • [P1] corepack pnpm config:docs:check.
  • [P1] git diff --check.

What I checked:

  • Repository policy read: Root AGENTS.md and scoped src/agents and docs AGENTS.md files were read; their config-surface, agent-runtime, proof, and Codex/dependency review guidance is relevant to this PR. (AGENTS.md:1, ac5d219be316)
  • Live PR state: Live metadata shows this PR is open, draft, mergeable clean, authored by a contributor, and currently lists only unit/type/docs/check commands as evidence. (cac2c72052d2)
  • Current main preflight abort path: Current main passes params.replyOperation.abortSignal into required budget/preflight compaction, matching the linked timeout report's source-level reproduction. (src/auto-reply/reply/agent-runner-memory.ts:981, ac5d219be316)
  • PR preflight change: The PR head removes the preflight abortSignal field, so required preflight compaction would rely on the compaction safety timeout rather than the reply operation abort signal. (src/auto-reply/reply/agent-runner-memory.ts:946, cac2c72052d2)
  • Current main MCP runtime ownership: Current main stores bundled MCP runtimes and in-flight creation by session id, so matching workspace/config sessions do not share runtime objects or catalog state. (src/agents/agent-bundle-mcp-runtime.ts:872, ac5d219be316)
  • PR shared runtime implementation: The PR head adds runtime-key indirection, shared runtime keys based on workspace/config fingerprint, and per-session attachment maps for the opt-in shared mode. (src/agents/agent-bundle-mcp-runtime.ts:1054, cac2c72052d2)

Likely related people:

  • jared596: Commit c6d8318 expanded runPreflightCompactionIfNeeded around transcript-estimate-triggered preflight compaction, making it a central ancestor of the touched bug path. (role: preflight compaction feature contributor; confidence: high; commits: c6d8318d07f5; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner.ts)
  • wangmiao0668000666: Commit bb6e477 changed the default compaction timeout while preserving explicit timeoutSeconds, which is central to the preflight timeout contract this PR changes. (role: timeout contract contributor; confidence: high; commits: bb6e47729cf8; files: src/agents/embedded-agent-runner/compaction-safety-timeout.ts, src/agents/embedded-agent-runner.compaction-safety-timeout.test.ts, docs/gateway/config-agents.md)
  • dutifulbob: Commit 3f68402 unified reply lifecycle behavior around stop, rotation, and restart, which is relevant to deciding which reply-operation aborts should still cancel preflight compaction. (role: reply lifecycle contributor; confidence: medium; commits: 3f6840230b86; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner.ts)
  • steipete: Prior MCP operability and operator workflow commits shaped the bundled MCP runtime, config, and docs surfaces now extended by shared runtime scope. (role: MCP runtime and config history contributor; confidence: high; commits: 99ce71ddbbdb, 38d3d11cbc0c; files: src/agents/agent-bundle-mcp-runtime.ts, src/agents/agent-bundle-mcp-materialize.ts, src/config/types.mcp.ts)
  • openperf: Commit f2530de changed lease-release idle-eviction behavior in the same runtime manager and tests, an invariant the shared runtime path must preserve. (role: recent MCP lifecycle contributor; confidence: high; commits: f2530de8320e; files: src/agents/agent-bundle-mcp-runtime.ts, src/agents/agent-bundle-mcp-runtime.test.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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 21, 2026
This was referenced Jun 21, 2026
Closed
Closed
@yu-xin-c yu-xin-c mentioned this pull request Jun 21, 2026
Closed
@yu-xin-c

yu-xin-c commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Superseding this combined draft with split PRs per ClawSweeper feedback: #95590 covers the narrow #95553 preflight compaction fix, and #95591 covers the opt-in #95506 MCP shared runtime feature. Closing this combined draft so review/proof can stay scoped.

@yu-xin-c yu-xin-c closed this Jun 21, 2026
This was referenced Jun 21, 2026
Closed
Merged
wangmiao0668000666 added a commit to wangmiao0668000666/openclaw that referenced this pull request Jun 22, 2026
@wangmiao0668000666
feat(compaction): add compaction.preflight.enabled config gate (openc…
bc2c49c 
…law#95553)

Adds a user-facing config gate to disable preflight (budget-triggered)
compaction. When compaction.preflight.enabled === false, the pre-turn
threshold check in runPreflightCompactionIfNeeded short-circuits to the
existing session entry, so subsequent turns fall through to overflow
recovery (which already honors compaction.timeoutSeconds with a 15min
budget).

The check is '=== false' against the literal value, so an unset key
preserves the existing default-on behavior — the fix is strictly
additive and does not change shipped behavior for any current user.

This complements the four open PRs (openclaw#95561, openclaw#95563, openclaw#95580, openclaw#95590)
that are all working on the abortSignal source for the preflight
path. None of those PRs addresses the user-facing config dimension
that the issue explicitly requests.

Changes:
- src/config/types.agent-defaults.ts: new AgentCompactionPreflightConfig
  type with optional enabled boolean
- src/config/zod-schema.agent-defaults.ts: zod schema with strict mode
- src/config/schema.help.ts + schema.labels.ts: help text + label
- src/auto-reply/reply/agent-runner-memory.ts: 6-line early return in
  runPreflightCompactionIfNeeded, placed after isHeartbeat/isCli and
  before codex runtime so the gate is the only OpenClaw-specific path
  affected
- src/auto-reply/reply/agent-runner-memory.test.ts: 1 new test
  (skips preflight compaction when enabled === false) using the
  same test fixture pattern as adjacent tests
- scripts/repro/issue-95553-preflight-disabled-gate.mts: standalone
  real-environment proof that exercises the production gate

Refs openclaw#95553
This was referenced Jun 22, 2026
Closed
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation gateway Gateway runtime merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: L status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: preflight (budget-triggered) compaction hard-capped at ~60s, ignores compaction.timeoutSeconds

1 participant

@yu-xin-c