mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(outbound): stop schema-padded poll modifiers from blocking send (#89601)
Summary: - The PR changes shared poll-intent detection so `pollDurationHours` and `pollMulti` alone no longer make `send` actions fail, with focused unit and outbound validation coverage. - PR surface: Source -2, Tests +40. Total +38 across 3 files. - Reproducibility: yes. Source inspection shows current main and `v2026.5.28` expose `pollDurationHours` throu ... d message schema, classify non-zero shared duration as poll intent, and throw before a `send` can dispatch. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head0fd95756cd. - Required merge gates passed before the squash merge. Prepared head SHA:0fd95756cdReview: https://github.com/openclaw/openclaw/pull/89601#issuecomment-4606487310 Co-authored-by: Gabriel Fratica <gabriel@codez.ro> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> (cherry picked from commit8b546facaf)
This commit is contained in:
@@ -362,6 +362,29 @@ describe("runMessageAction send validation", () => {
|
||||
}),
|
||||
).rejects.toThrow(/use action "poll" instead of "send"/i);
|
||||
});
|
||||
|
||||
it("allows send when only schema-padded shared poll modifiers are present", async () => {
|
||||
// LLMs routinely echo the shared `message` tool schema's poll modifier
|
||||
// defaults (`pollDurationHours: 1`, `pollMulti: false`) on every plain
|
||||
// `send` call alongside the rest of the schema-padded slots. Without a
|
||||
// pollQuestion or pollOption present, these defaults are noise — not
|
||||
// poll intent — and must not block the send.
|
||||
const result = await runDrySend({
|
||||
cfg: workspaceConfig,
|
||||
actionParams: {
|
||||
channel: "workspace",
|
||||
target: "#C12345678",
|
||||
message: "hello",
|
||||
pollQuestion: "",
|
||||
pollOption: [],
|
||||
pollDurationHours: 1,
|
||||
pollMulti: false,
|
||||
},
|
||||
toolContext: { currentChannelId: "C12345678" },
|
||||
});
|
||||
|
||||
expect(result.kind).toBe("send");
|
||||
});
|
||||
});
|
||||
|
||||
describe("message body alias normalization", () => {
|
||||
|
||||
@@ -12,8 +12,8 @@ describe("poll params", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it.each([{ key: "pollMulti" }, { key: "pollAnonymous" }, { key: "pollPublic" }])(
|
||||
"treats $key=true as poll creation intent",
|
||||
it.each([{ key: "pollAnonymous" }, { key: "pollPublic" }])(
|
||||
"treats channel-extra $key=true as poll creation intent",
|
||||
({ key }) => {
|
||||
expect(
|
||||
hasPollCreationParams({
|
||||
@@ -23,27 +23,44 @@ describe("poll params", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("treats non-zero finite numeric poll params as poll creation intent", () => {
|
||||
it("treats non-zero finite numeric channel-extra poll params as poll creation intent", () => {
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: 60 })).toBe(true);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: "60" })).toBe(true);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: "+60" })).toBe(true);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: "1e3" })).toBe(true);
|
||||
expect(hasPollCreationParams({ pollDurationHours: -1 })).toBe(true);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: "-5" })).toBe(true);
|
||||
expect(hasPollCreationParams({ pollDurationHours: Number.NaN })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: Infinity })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: "60abc" })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: "0x10" })).toBe(false);
|
||||
});
|
||||
|
||||
it("does not treat zero-valued numeric poll params as poll creation intent", () => {
|
||||
it("does not treat zero-valued numeric channel-extra poll params as poll creation intent", () => {
|
||||
// Zero values are typically defaults/unset values from tool schemas,
|
||||
// not intentional poll creation. Fixes #52118.
|
||||
expect(hasPollCreationParams({ pollDurationHours: 0 })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationSeconds: 0 })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationHours: "0" })).toBe(false);
|
||||
expect(hasPollCreationParams({ poll_duration_seconds: 0 })).toBe(false);
|
||||
});
|
||||
|
||||
it("does not treat shared modifier params (pollDurationHours, pollMulti) as poll creation intent without a question or options", () => {
|
||||
// These two are exposed by the shared `message` tool schema for both
|
||||
// `send` and `poll` actions, so LLMs routinely schema-pad them on every
|
||||
// plain `send` call with their schema-implied defaults (1 for an integer
|
||||
// with `minimum: 1`, `false` for a boolean). Treating those defaults as
|
||||
// poll intent blocks routine sends — see the regression that motivated
|
||||
// this carve-out.
|
||||
expect(hasPollCreationParams({ pollDurationHours: 1 })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationHours: 1, pollMulti: false })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationHours: 0 })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationHours: -1 })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationHours: "0" })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollDurationHours: Number.NaN })).toBe(false);
|
||||
expect(hasPollCreationParams({ poll_duration_hours: "0" })).toBe(false);
|
||||
expect(hasPollCreationParams({ pollMulti: true })).toBe(false);
|
||||
});
|
||||
|
||||
it("still flags shared modifier params when accompanied by a question or options", () => {
|
||||
expect(hasPollCreationParams({ pollQuestion: "Ready?", pollDurationHours: 1 })).toBe(true);
|
||||
expect(hasPollCreationParams({ pollOption: ["Yes", "No"], pollMulti: true })).toBe(true);
|
||||
});
|
||||
|
||||
it("treats string-encoded boolean poll params as poll creation intent when true", () => {
|
||||
|
||||
@@ -74,8 +74,18 @@ function hasExplicitUnknownPollValue(key: string, value: unknown): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
export function hasPollCreationParams(params: Record<string, unknown>): boolean {
|
||||
for (const key of SHARED_POLL_CREATION_PARAM_NAMES) {
|
||||
// Among the shared poll params, only the content-bearing fields (pollQuestion,
|
||||
// pollOption) signal poll intent on their own. The modifier fields
|
||||
// (pollDurationHours, pollMulti) are exposed by the shared `message` tool
|
||||
// schema for both `send` and `poll` actions, so LLMs routinely echo their
|
||||
// schema-implied defaults (`1`, `false`) on plain `send` calls — see issue
|
||||
// for context. Treating those modifier defaults as "the agent meant to create
|
||||
// a poll" produces false positives and blocks routine sends. The modifiers
|
||||
// only count when accompanied by a content-bearing field.
|
||||
const CONTENT_BEARING_SHARED_POLL_PARAM_NAMES = ["pollQuestion", "pollOption"] as const;
|
||||
|
||||
function hasContentBearingPollCreationParam(params: Record<string, unknown>): boolean {
|
||||
for (const key of CONTENT_BEARING_SHARED_POLL_PARAM_NAMES) {
|
||||
const def = POLL_CREATION_PARAM_DEFS[key];
|
||||
const value = readPollParamRaw(params, key);
|
||||
if (def.kind === "string" && typeof value === "string" && value.trim().length > 0) {
|
||||
@@ -92,30 +102,18 @@ export function hasPollCreationParams(params: Record<string, unknown>): boolean
|
||||
return true;
|
||||
}
|
||||
}
|
||||
if (def.kind === "positiveInteger") {
|
||||
// Treat zero-valued numeric defaults as unset, but preserve any non-zero
|
||||
// numeric value as explicit poll intent so invalid durations still hit
|
||||
// the poll-only validation path.
|
||||
if (typeof value === "number" && Number.isFinite(value) && value !== 0) {
|
||||
return true;
|
||||
}
|
||||
if (typeof value === "string") {
|
||||
const trimmed = value.trim();
|
||||
const parsed = parseStrictFiniteNumber(trimmed);
|
||||
if (parsed !== undefined && parsed !== 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (def.kind === "boolean") {
|
||||
if (value === true) {
|
||||
return true;
|
||||
}
|
||||
if (typeof value === "string" && normalizeLowercaseStringOrEmpty(value) === "true") {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export function hasPollCreationParams(params: Record<string, unknown>): boolean {
|
||||
if (hasContentBearingPollCreationParam(params)) {
|
||||
return true;
|
||||
}
|
||||
// Channel-specific poll-prefixed params (e.g. pollDurationSeconds,
|
||||
// pollPublic) are not part of the shared schema, so an explicit value still
|
||||
// indicates deliberate poll intent and continues to trigger the validator
|
||||
// even without a pollQuestion/pollOption.
|
||||
for (const [key, value] of Object.entries(params)) {
|
||||
if (isChannelPollCreationParamName(key) && hasExplicitUnknownPollValue(key, value)) {
|
||||
return true;
|
||||
|
||||
Reference in New Issue
Block a user