feat(cli): Mount the v2 tool operations on the Managed Runtime worker - #12671

Real-environment verification — PR #12671 @ a5b867e11d

Verdict: not mergeable as-is. The worker, executor, and journal behave well once the routes can be reached. But the PR's copy of the contract predates the version of #12630 that was merged. When the PR is merged into current main, git silently keeps main's attest-only route gate, and every execute, status, and cancel call returns 404. The fix is 3 lines plus flipping one test in main. Two smaller issues are also worth fixing before merge (the background shell and requestTimeout, both covered below).

Setup

• Tree: origin/main c33e1aad55 plus the PR head, merged with git merge. The five add/add and content conflicts were resolved to main (feat(cli): Declare the v2 execute/status/cancel Managed Runtime contract #12630 is merged and authoritative). All of the PR's own worker files merged cleanly.
• Real process: pnpm install and the production bundle, then node dist/cli.js managed-runtime-worker. The boot JSON goes over stdin, the worker prints ready, and every call below is a real HTTP request to that process. No mocks.
• Arms:
  • (A) merged as-is
  • (B) A plus a candidate gate fix
  • (C) control: the B bundle with requestTimeout patched back to main's 5_000
  • The PR's own head was also run as a reference.

Finding 1 — Blocking: after merge the tool routes are unreachable

On main, #12630 narrowed isOwnedManagedRuntimeRoute during review to method === ATTEST_ROUTE.method && url === ATTEST_ROUTE.path. It is pinned by the test rejects declared tool routes until their handlers land, and the design doc (§3.1) says the gate keeps returning 404 "until the real tool handlers are mounted". This PR is the change that mounts them. However, its branch still carries the pre-review OWNED_MANAGED_RUNTIME_ROUTES.some(...), which is identical to the merge base. So git takes main's side with no conflict, and the new handlers sit behind a gate that never lets requests reach them.

Candidate fix (verified in B; tsc --noEmit and eslint are clean):

--- a/packages/cli/src/serve/managed-runtime-attestation-contract.ts
+++ b/packages/cli/src/serve/managed-runtime-attestation-contract.ts
@@ export function isOwnedManagedRuntimeRoute(
-  return method === ATTEST_ROUTE.method && url === ATTEST_ROUTE.path;
+  return OWNED_MANAGED_RUNTIME_ROUTES.some(
+    (route) => method === route.method && url === route.path,
+  );
--- a/packages/cli/src/serve/managed-runtime-attestation-contract.test.ts
+++ b/packages/cli/src/serve/managed-runtime-attestation-contract.test.ts
-  it('rejects declared tool routes until their handlers land', async () => {
+  it('routes declared tool routes through the owned-route gate', async () => {
@@
-      expect(response.status).toBe(404);
-      expect(await response.text()).toBe('');
+      expect(response.status).toBe(200);
+      expect(await response.json()).toEqual({
+        protocolVersion: 2,
+        state: 'unknown',
+      });

The §3.1 sentence "Until the real tool handlers are mounted, ownedManagedRuntimeRouteGate admits only the exact attestation route…" also needs updating in both languages. Because this branch still contains its own copy of the #12630 commits, merging origin/main into it is the practical way to converge (not a rebase, since there are review threads). Take main's contract, schema, fixtures, Java, and doc files, then reapply the worker section of the doc.

What works (arm B, real process)

• Admitted tools:
  • read_file reads the workspace file.
  • write_file and edit changed the file on disk to alpha\ngamma\n.
  • edit with no match settles as executionStatus: "error" in 2 ms. It does not hang on the LLM-correction path, even with the synthetic model.
  • Foreground shell runs with cwd set to the attested workspace.
• Idempotency:
  • Two concurrent execute calls with the same callId both return after about 1039 ms with an identical result. A third call after settlement returns in 2 ms. The side-effect counter file has 1 line, so the command ran once.
  • The same callId with a different digest, or with a different input, returns 409 managed_runtime_identity_conflict.
  • An unadmitted tool (glob) returns 409.
• Status and cancel:
  • Unseen references return 200 unknown on both status and cancel.
  • status returns executing/lastSequence: 1 for a running call. After settlement it returns the result with lastSequence (2, or 3 after a cancel).
  • cancel on a running shell returns cancel_requested. The held execute then returns at 818 ms with executionStatus: "cancelled", and the shell PID is gone. A repeated cancel returns the settled result.
  • Responses follow the merged schema: execute and cancel carry no lastSequence, and non-settled states carry no result.
• Response size: Output stays well under the 1 MiB cap because the tools truncate themselves:
  • read_file of a 3 MB text file → 25 KB
  • 1500 lines × 1900 chars → 25 KB
  • 3 MB of shell stdout → 4.7 KB
• Shutdown: SIGTERM during a foreground execute exits in 520 ms, and the shell child is killed.

Finding 2 — Should fix: is_background: true gets past the "foreground only" admission

The executor admits run_shell_command without checking is_background. When a call sets it:

• execute settles immediately as success with Background shell started … pid: 58430.
• cancel then returns that settled result, and the process keeps running (alive after cancel=true).
• SIGTERM to the worker then takes 44.6 s to exit, because the worker waits for the background sleep 45 to finish.

In a provisioned environment, a background job would outlive its cancel and delay or block retirement of the generation. The PR text and the design doc both say "foreground run_shell_command". A small guard in ManagedToolExecutor.execute or the route closes this, for example: when toolName === 'run_shell_command' && input.is_background === true, reject it the same way an unadmitted tool is rejected. A test case for it would help too.

Finding 3 — Should fix: requestTimeout = 0 is unnecessary and removes a bound

The comment and the doc say requestTimeout has to be disabled because execute holds the connection until settlement. Node's requestTimeout only bounds how long it takes to receive the request, not the response. Measured in control arm C (identical bundle with main's 5_000):

• A 40 s foreground execute completed normally with 200 at 40153 ms. That span crosses Node's 30 s connection-check tick.
• An authenticated client that declares Content-Length: 1000 and never sends the body gets 408 at about 20 s on C. On this PR the socket was still open at 50 s.

Unauthenticated stalls are closed after about 2 s in both arms (401). So the only behavior change is that an authenticated slow upload is no longer bounded. Suggested fix: keep 5_000, and remove the rationale from the code comment and from doc §6.

Observations (not blocking)

1. Path confinement is purely Harness-side. The worker happily ran:

   • read_file /etc/hosts
   • write_file to a path outside workspaceCwd (the file was created)
   • cd / && pwd in the shell

   This matches "no further approval gate", but the tools' own outside-workspace ask is bypassed here. A sentence in §6 saying the Harness admission must include the workspace-boundary decision would make that contract explicit.

2. Image reads degrade. read_file of a PNG returns Unsupported image file … This model does not support image input because the worker's Config uses the synthetic model managed-runtime-worker. That differs from a local run with a vision model. It's fine for this slice, but worth listing as a follow-up.

3. The Java HttpRuntimeTransport.execute on main does not match the v2 execute body yet. It still posts the prepare-era body (tenantId, workspaceId, sessionId, turnKind, reference). It has no toolName or input, a 16 KiB response cap, and a 30 s timeout. This worker would answer that body with 400. Its class comment already defers execute to a later slice. The PR description's "the Java transport speaks it" should be scoped to attestation and the fixtures.

4. The prepared state can't be reached through HTTP. run() flips the entry to executing synchronously, so the cancel-on-prepared branch is currently dead code. It's harmless.

Recommendation

Merge after:

• (1) merging main into the branch, applying the gate change above, and updating the pinned test and the §3.1 wording
• (2) rejecting is_background: true
• (3) restoring requestTimeout = 5_000

After (1), the PR's suites plus main's shared fixtures all pass against the real worker process (114/114, 13/13).

@qwen-code /resolve

Qwen Code attempted to resolve merge conflicts but the run did not complete successfully.

address-summary.md

Root cause

This PR is stacked on the branch copy of #12630, which merged to main independently as 177c2f6415 after review rework, so the same files arrived twice, differently (4 add/add). Our broker commit 37fd3607e2 was likewise reimplemented by main's 3413e8cc57 (#12655). Nothing on main touched this PR's real subject, the worker mount.

Textual vs semantic

Textual — 6 files: the v2 schema/fixtures JSON, both contract/conformance tests, and the two broker Java files. Our side is byte-identical to pre-review bd1594e2ea/37fd3607e2; main's is a strict superset (per-route request bodies, additionalProperties, +66 fixture lines, mutation tests, our "unknown" cancel state included). Took main's.

Semantic — 2 files (both design docs): main rewrote §2–§5, our commit appended §6. Resolved as main's body + our §6, seam byte-verified in EN and zh-CN. git diff origin/main is now exactly the PR's 7-file worker mount (966+/11−).

Load-bearing — the merge is NOT green

Main's #12630 narrowed the raw gate to attest-only because the v2 routes were declared but unmounted:

return method === ATTEST_ROUTE.method && url === ATTEST_ROUTE.path;

This PR mounts execute/status/cancel behind that same gate (managed-runtime-attestation-worker.ts:130), so every v2 request 404s before Express. Main's doc states the rule: "A future handler change must expand gate admission in the same change." This is that change; the widening is missing. Not fixed here — managed-runtime-attestation-contract.ts auto-merged with no marker, and only conflicted files are editable in this run. Fix = restore the manifest scan:

return OWNED_MANAGED_RUNTIME_ROUTES.some(
  (route) => method === route.method && url === route.path,
);

Casualties in NON-conflicted files

• managed-runtime-tool-worker.test.ts — all tests fail (404 at the gate).
• managed-runtime-attestation-contract.test.ts — main's rejects declared tool routes until their handlers land still passes but is now false and contradicts the above; revert it with the gate.
• Docs §3.1/§5 still say the handlers are unmounted.

No build/typecheck/test run. Imports, executor core paths and the 66 new (200-status, skipped) fixtures check out.

Check the workflow run for full logs.

Addressed the conflict-resolution and verification comments in 1cecf1a. Main was merged into the existing branch without rewriting its review history. The reviewed schema, fixtures, and Java broker changes were preserved.

The three reported defects are fixed: all mounted tool routes pass the exact HTTP gate; background shell calls are rejected before a process or journal entry is created; and the 5-second request-receipt timeout remains enabled independently of execution duration. Both design languages and the PR description now accurately describe Harness-owned workspace boundaries, image limitations, deferred Java tool transport, and the internal-only prepared state.

Open-ended and reverse audits also found and reproduced three additional defects, now fixed: shared file-read caching hid content from another session; tool parameter normalization mutated the journal identity and rejected identical retries; and JSON-escaped SVG output exceeded the 1 MiB contract. The worker now disables conversation-dependent caching, executes with an input copy, and journals a bounded terminal error for oversized results so execute, status, cancel, and replay stay consistent. Regression tests were observed failing before each fix.

Validation on macOS, Node 22.22.2 and Java 21:

• Full build, bundle, and full typecheck passed; targeted ESLint and Prettier checks passed. Commit hooks passed, and the committed tree was verified byte-for-byte against the audited tree.
• CLI contract/worker suites: 121 passed. Java conformance/broker suites: 96 passed.
• Fresh production bundle, real worker and HTTP/TCP clients, no mocks: all six reproduced defects are fixed; read/write/edit, concurrent execution with one side effect, identity conflict, cancellation, and 13 shared negative fixtures passed.
• Incomplete authenticated upload: 408 after 28,799 ms. A complete request running a real 40-second foreground command: 200/success after 40,078 ms.
• The SVG that previously produced >1.2 MB responses now returns the same recorded error through execute/status/cancel/replay, with response sizes 160/177/160/160 bytes. Background rejection produced no marker file; worker shutdown exited normally.
• After the initial audit findings were fixed, two consecutive complete open-ended/reverse audit passes found no further actionable issue, including an independent review. No source edits were needed during those clean passes.

The global qwen 0.24.4 installation lacked this worker command, so baseline reproduction used a real source-worker process; final verification used only the freshly built production CLI. Windows and Linux were not run locally.

Real-environment re-verification (round 2) — PR #12671 @ 1cecf1a228

Verdict: two of the three round-1 findings are fixed, verified against the real worker process. The background-shell guard is still bypassable: is_background: "true" (a string) starts a background process that cancel can't stop and that holds worker shutdown for about 44 s. A 16-line candidate fix and a test extension are below. Both were verified A/B in unit tests and against the real process. Once that lands, I have no remaining blockers.

Setup

• Tree: PR head 1cecf1a228, merged locally with current origin/main 64c0453824. The merge was clean; the branch is MERGEABLE.
• Real process: pnpm install and the production bundle, then node dist/cli.js managed-runtime-worker with the boot JSON over stdin. Every call is real HTTP. The probe is the same as in round 1, plus new input variants.
• Before each run I checked that the bundle chunk contains the new code (OWNED_MANAGED_RUNTIME_ROUTES.some, requestTimeout = 5e3, and the background-guard string).

Round-1 findings

Retested with no regressions:

• read_file, write_file, and edit work; the file on disk ends up as alpha\ngamma\n.
• Concurrent duplicate calls join, and the side-effect file has 1 line.
• A different digest, a different input, or an unadmitted tool each return 409.
• cancel on a running shell settles as cancelled in 830 ms, and the process is gone.
• SIGTERM during a foreground execute exits in 516 ms.
• Large outputs are truncated by the tools themselves (3 MB → 25 KB / 4.7 KB). The new 1 MiB journal cap has its own unit test (journals a bounded error when JSON encoding exceeds the result limit).

Finding — the string "true" bypasses the background-shell guard

The guard compares the raw input: input['is_background'] === true. But tool.build() runs core's schemaValidator, whose fixBooleanValues pass converts "true"/"false" (case-insensitive) to booleans when the schema accepts boolean (packages/core/src/utils/schemaValidator.ts:1120-1136). So the tool runs with is_background: true while the guard sees a string. On the real worker:

A Harness that relays model-produced arguments can easily send "true", since models emit string booleans often enough that the validator has this coercion pass for them. The PR's test uses only the boolean true. With the test extended to [true, 'true', 'TRUE'], the PR's executor fails the 2 string cases, and the candidate fix passes all 123.

Candidate fix: decide on the parameters the tool will actually run with. It is verified with tsc --noEmit and eslint, both clean.

--- a/packages/cli/src/serve/managed-runtime-tool-executor.ts
+++ b/packages/cli/src/serve/managed-runtime-tool-executor.ts
@@ export class ManagedToolExecutor {
-    if (toolName === ShellTool.Name && input['is_background'] === true) {
+    if (toolName === ShellTool.Name && isBackgroundShell(tool, input)) {
       throw new ManagedToolConflictError(
         'Managed Runtime does not admit background shell execution.',
       );
@@
+// Decide on the parameters the tool will actually run with: build() coerces
+// "true"/"false" strings to booleans, so the raw input is not authoritative.
+function isBackgroundShell(
+  tool: AnyDeclarativeTool,
+  input: Record<string, unknown>,
+): boolean {
+  try {
+    const params = tool.build(structuredClone(input)).params as {
+      is_background?: unknown;
+    };
+    return params.is_background === true;
+  } catch {
+    return false;
+  }
+}

If build() throws, the call still goes through the existing path, which settles it as error without spawning anything. The test change turns the existing case into it.each([true, 'true', 'TRUE'])('rejects background shell execution (is_background=%j) without recording an invocation', …). The body is unchanged.

Minor (not blocking)

• A rejected background call returns the generic Managed Runtime invocation identity conflicts., because the route maps every ManagedToolConflictError to that one message. The executor's more specific text never reaches the caller. It's harmless, but the specific text would be easier to diagnose.
• The round-1 observations still apply unchanged, and the doc now scopes them: path confinement is Harness-side, and image reads degrade under the synthetic model name.

Recommendation

Merge after the isBackgroundShell change and the extended test. Everything else from round 1 is verified fixed on the real worker.

Addressed the round-2 review and merged main 688e8af in be47e55.

The previous assumption that non-boolean background inputs would all fail validation was incorrect. Admission now uses the same parameter normalization as the shell tool: boolean true and case-insensitive string true are rejected before journaling or process creation; boolean/string false remains foreground. The response now carries the specific background-admission error.

Pre-commit open-ended and reverse audits also exposed two deep-input error paths. Copy/validation failures now retain their recorded error settlement, and inputs the JSON encoder cannot represent are rejected with 400 before journaling. Accepted input is encoded once and retained for retry comparison, so replay cannot fail while re-encoding a recorded payload. Both design languages describe these boundaries.

Main advanced during verification and introduced conflicts in the two design documents. These are resolved while preserving both the worker implementation and the newly landed Java tool transport from #12637. Broker transport wiring remains follow-up work; the PR description has been updated accordingly.

Validation on the final merged tree (macOS, Node 22.22.2, Java 21):

• Full build, bundle, typecheck, targeted ESLint, Prettier, and Java Checkstyle passed.
• 135 CLI tests and 144 Java tests passed, including the new Java transport and provisioner tests.
• Fresh production bundle, real HTTP/TCP clients, no mocks: all four probe suites passed. Boolean/string variants, exact false-input replay with one side effect, file operations, concurrent duplicate execution, cancellation, session isolation, bounded results, and all 13 shared negative fixtures passed.
• A 4,000-level input returns the same recorded 200/error through execute, retry, status, and cancel. A 10,000-level input consistently returns 400 before journaling, with unknown status/cancel and no command side effects.
• Stalled upload: 408 after 28,822 ms. Complete 40-second foreground request: 200/success after 40,039 ms.
• After fixes and the latest-main merge, two consecutive complete open-ended/reverse audit passes found no further actionable issue, including an independent review. The committed tree was checked against the audited tree after commit hooks.

Real-environment re-verification (round 3) — PR #12671 @ be47e55274

Verdict: ready to merge from the verification side. The round-2 background-shell bypass is closed. The new invalid-input path works as described. Every earlier finding stays fixed, with no regressions on the real worker process.

Setup

• Tree: PR head be47e55274, merged locally with current origin/main 90232f0eb0. The merge was clean.
• Real process: a fresh pnpm install and production bundle, then node dist/cli.js managed-runtime-worker with the boot JSON over stdin. Every call is real HTTP.
• Before running, I checked that the bundle chunk contains the new admission code (validateToolParams(params) === null) and requestTimeout = 5e3.

Round-2 finding: is_background as a string

Admission now validates a structuredClone of the input with the shell tool's own validateToolParams. BaseDeclarativeTool.build() is exactly validateToolParams(params) followed by createInvocation(params) on the same object, and ShellTool does not override build. So admission now sees the same normalized parameters that execution uses.

Mutation check: reverting the guard to the raw input['is_background'] === true turns 3 PR tests red ("true"/"TRUE"/"TrUe"). The new test does detect the bypass.

New in this round: deep and unencodable input

• A 4000-level nested input returns the same recorded 200 settled/error across execute, retry, status, and cancel, with no command side effect.
• A 10000-level input returns 400 managed_runtime_attestation_invalid on both execute and retry, and status/cancel return unknown, so nothing was journaled. There is no side effect, and the worker still serves read_file afterwards.
• Mutation check: if the JSON.stringify failure no longer throws ManagedToolInvalidError, 2 PR tests go red.

Rounds 1–2 regression pass (real process)

• Routes are live, and main's 13 negative shared fixtures pass 13/13.
• read_file, write_file, and edit work.
• Concurrent duplicate calls join, and the side-effect file has 1 line.
• A different digest, a different input, or an unadmitted tool each return 409.
• cancel on a running shell settles as cancelled in 809 ms, and the process is gone.
• A 40 s foreground execute succeeds (40055 ms) with requestTimeout = 5_000. An authenticated stalled body gets 408 at about 16.8 s.
• SIGTERM during an execute exits in 515 ms, and the shell child is killed.
• Large outputs are still truncated by the tools (3 MB → 25 KB / 4.7 KB).
• The 3 suites pass 135/135. tsc --noEmit and eslint are clean.

Notes (not blocking)

• The round-1 observations are unchanged and now documented as out of scope: path confinement is Harness-side, image reads degrade under the synthetic model name, and broker transport wiring is follow-up work.

@qwen-code /triage