From b4e4f96fd56800f19f3894f8d3494f451d2738d2 Mon Sep 17 00:00:00 2001 From: Jordan Brant Baker Date: Tue, 14 Apr 2026 10:06:14 -0600 Subject: [PATCH] fix: restore embedded-run param forwarding (#62675) (thanks @hexsprite) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: forward optional params dropped at the runEmbeddedAttempt call site runEmbeddedPiAgent in pi-embedded-runner/run.ts hand-enumerates ~85 fields when calling runEmbeddedAttempt({...}). Several optional fields on RunEmbeddedPiAgentParams were added to the type and to attempt.ts (the consumer) but were never wired at this specific call site. Because every field is declared as ?: optional on EmbeddedRunAttemptParams, TypeScript does not flag the missing fields and the attempt silently receives undefined for each. Four fields were affected: - toolsAllow (#58504, #62569): cron's --tools allow-list. Persisted in jobs.json by the CLI, forwarded by cron/isolated-agent/run-executor.ts to runEmbeddedPiAgent, but dropped here. Result: provider request ships the full tool catalog on every cron run regardless of toolsAllow, defeating the ~95% input-token reduction documented in #58504 and the --tools restriction documented in docs/automation/cron-jobs.md:85. - disableMessageTool: cron/isolated-agent/run-executor.ts:164 sets it from toolPolicy.disableMessageTool, derived at run.ts:110 as `params.deliveryContract === "cron-owned" ? true : params.deliveryRequested`. Every cron-owned delivery (the default per docs) is supposed to disable the message tool so the runner owns the final delivery path. Without forwarding, the agent can call messaging tools mid-cron and cause duplicate or wrong-channel sends. - requireExplicitMessageTarget: cron/isolated-agent/run-executor.ts:163 sets it from toolPolicy.requireExplicitMessageTarget. Has a fallback at attempt.ts:568-569 to `?? isSubagentSessionKey(params.sessionKey)`, so non-subagent crons silently get false instead of the intended value. - internalEvents: agents/command/attempt-execution.ts:478 passes it via params.opts.internalEvents. Different caller path from cron, but the same drop point. Internal events array silently dropped before reaching the consumer at attempt.ts:1480. The fix is four lines in the runEmbeddedAttempt({...}) call, immediately after the bootstrapContextMode/bootstrapContextRunKind lines added by PR #62264 (which fixed two more fields with the identical pattern at the same call site). A regression test (run.attempt-param-forwarding.test.ts) covers all six optional fields shown to have been bitten by this class of bug at this seam. The next ?: optional field added to RunEmbeddedPiAgentParams without wiring at the runEmbeddedAttempt call site will fail a test instead of silently shipping broken — addressing the missing-guardrail concern PR #60776's writeup explicitly noted. Verified locally: 6/6 forwarding tests pass, 258 pi-embedded-runner/run* tests pass, 176 cron/isolated-agent tests pass, oxlint and tsgo deltas versus origin/main are zero. Fixes #62569 * test: distill param forwarding guardrails * fix: restore embedded-run param forwarding (#62675) (thanks @hexsprite) --------- Co-authored-by: Ayaan Zaidi --- CHANGELOG.md | 1 + .../run.attempt-param-forwarding.test.ts | 80 +++++++++++++++++++ src/agents/pi-embedded-runner/run.ts | 4 + 3 files changed, 85 insertions(+) create mode 100644 src/agents/pi-embedded-runner/run.attempt-param-forwarding.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index df4f4639f19..9609adceca2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai - Video generation/live tests: bound provider polling for live video smoke, default to the fast non-FAL text-to-video path, and use a one-second lobster prompt so release validation no longer waits indefinitely on slow provider queues. - Memory-core/QMD `memory_get`: reject reads of arbitrary workspace markdown paths and only allow canonical memory files (`MEMORY.md`, `memory.md`, `DREAMS.md`, `dreams.md`, `memory/**`) plus exact paths of active indexed QMD workspace documents, so the QMD memory backend can no longer be used as a generic workspace-file read shim that bypasses `read` tool-policy denials. (#66026) Thanks @eleqtrizit. +- Cron/agents: forward embedded-run tool policy and internal event params into the attempt layer so `--tools` allowlists, cron-owned message-tool suppression, explicit message targeting, and command-path internal events all take effect at runtime again. (#62675) Thanks @hexsprite. ## 2026.4.14 diff --git a/src/agents/pi-embedded-runner/run.attempt-param-forwarding.test.ts b/src/agents/pi-embedded-runner/run.attempt-param-forwarding.test.ts new file mode 100644 index 00000000000..45f3029024c --- /dev/null +++ b/src/agents/pi-embedded-runner/run.attempt-param-forwarding.test.ts @@ -0,0 +1,80 @@ +import { beforeAll, beforeEach, describe, expect, it } from "vitest"; +import type { AgentInternalEvent } from "../internal-events.js"; +import { makeAttemptResult } from "./run.overflow-compaction.fixture.js"; +import { + loadRunOverflowCompactionHarness, + mockedRunEmbeddedAttempt, + overflowBaseRunParams, + resetRunOverflowCompactionHarnessMocks, +} from "./run.overflow-compaction.harness.js"; +import type { RunEmbeddedPiAgentParams } from "./run/params.js"; + +type ForwardingCase = { + name: string; + runId: string; + params: Partial; + expected: Record; +}; + +let runEmbeddedPiAgent: typeof import("./run.js").runEmbeddedPiAgent; +const internalEvents: AgentInternalEvent[] = []; +const forwardingCases = [ + { + name: "forwards toolsAllow so the per-job tool allowlist can be honored", + runId: "forward-toolsAllow", + params: { toolsAllow: ["exec", "read"] }, + expected: { toolsAllow: ["exec", "read"] }, + }, + { + name: "forwards bootstrapContextMode so lightContext cron jobs strip workspace bootstrap files", + runId: "forward-bootstrapContextMode", + params: { bootstrapContextMode: "lightweight" }, + expected: { bootstrapContextMode: "lightweight" }, + }, + { + name: "forwards bootstrapContextRunKind so the bootstrap filter knows the caller context", + runId: "forward-bootstrapContextRunKind", + params: { bootstrapContextRunKind: "cron" }, + expected: { bootstrapContextRunKind: "cron" }, + }, + { + name: "forwards disableMessageTool so cron-owned delivery suppresses the messaging tool", + runId: "forward-disableMessageTool", + params: { disableMessageTool: true }, + expected: { disableMessageTool: true }, + }, + { + name: "forwards requireExplicitMessageTarget so non-subagent callers can opt in explicitly", + runId: "forward-requireExplicitMessageTarget", + params: { requireExplicitMessageTarget: true }, + expected: { requireExplicitMessageTarget: true }, + }, + { + name: "forwards internalEvents so the agent command attempt path can deliver internal events", + runId: "forward-internalEvents", + params: { internalEvents }, + expected: { internalEvents }, + }, +] satisfies ForwardingCase[]; + +describe("runEmbeddedPiAgent forwards optional params to runEmbeddedAttempt", () => { + beforeAll(async () => { + ({ runEmbeddedPiAgent } = await loadRunOverflowCompactionHarness()); + }); + + beforeEach(() => { + resetRunOverflowCompactionHarnessMocks(); + }); + + it.each(forwardingCases)("$name", async ({ runId, params, expected }) => { + mockedRunEmbeddedAttempt.mockResolvedValueOnce(makeAttemptResult({ promptError: null })); + + await runEmbeddedPiAgent({ + ...overflowBaseRunParams, + ...params, + runId, + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledWith(expect.objectContaining(expected)); + }); +}); diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index 8f0398ed388..92522104099 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -754,6 +754,10 @@ export async function runEmbeddedPiAgent( silentExpected: params.silentExpected, bootstrapContextMode: params.bootstrapContextMode, bootstrapContextRunKind: params.bootstrapContextRunKind, + toolsAllow: params.toolsAllow, + disableMessageTool: params.disableMessageTool, + requireExplicitMessageTarget: params.requireExplicitMessageTarget, + internalEvents: params.internalEvents, bootstrapPromptWarningSignaturesSeen, bootstrapPromptWarningSignature: bootstrapPromptWarningSignaturesSeen[bootstrapPromptWarningSignaturesSeen.length - 1],