mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:30:43 +00:00
fix: restore embedded-run param forwarding (#62675) (thanks @hexsprite)
* 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 <hi@obviy.us>
This commit is contained in:
committed by
GitHub
parent
3bb9e5f580
commit
b4e4f96fd5
@@ -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
|
||||
|
||||
|
||||
@@ -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<RunEmbeddedPiAgentParams>;
|
||||
expected: Record<string, unknown>;
|
||||
};
|
||||
|
||||
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));
|
||||
});
|
||||
});
|
||||
@@ -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],
|
||||
|
||||
Reference in New Issue
Block a user