Skip to content

Commit 0fd9575

Browse files
codezzcodezz
committed
fix(outbound): stop schema-padded poll modifiers from blocking send
`hasPollCreationParams` previously treated *any* of the shared poll params (`pollQuestion`, `pollOption`, `pollDurationHours`, `pollMulti`) as poll intent if present with a non-zero / non-default value. The shared `message` tool schema exposes those last two for both `send` and `poll` actions, and agent models routinely echo the schema's implied defaults (`1` for the `minimum: 1` integer, `false` for the boolean) on every plain `send` call alongside the rest of the schema-padded slots. The previous logic interpreted `pollDurationHours: 1` (or any non-zero duration / `pollMulti: true`) on its own as "the agent meant to create a poll", so `runMessageAction` threw Poll fields require action "poll"; use action "poll" instead of "send". on routine sends, blocking thread replies from agents whose tool calls include the full schema-padded param surface. Narrow the shared-param branch to the content-bearing fields (`pollQuestion`, `pollOption`). Channel-extra `poll*` params (`pollDurationSeconds`, `pollPublic`, `pollAnonymous`, ...) remain strict because they're not part of the shared tool-schema surface, so an explicit value still signals deliberate intent. Add a unit test covering the schema-pad shape and an integration test on `runMessageAction` confirming `send` is no longer rejected when only schema-padded modifier defaults are present.
1 parent 28b1ea7 commit 0fd9575

3 files changed

Lines changed: 71 additions & 33 deletions

File tree

‎src/infra/outbound/message-action-runner.send-validation.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,29 @@ describe("runMessageAction send validation", () => {
362362
}),
363363
).rejects.toThrow(/use action "poll" instead of "send"/i);
364364
});
365+
366+
it("allows send when only schema-padded shared poll modifiers are present", async () => {
367+
// LLMs routinely echo the shared `message` tool schema's poll modifier
368+
// defaults (`pollDurationHours: 1`, `pollMulti: false`) on every plain
369+
// `send` call alongside the rest of the schema-padded slots. Without a
370+
// pollQuestion or pollOption present, these defaults are noise — not
371+
// poll intent — and must not block the send.
372+
const result = await runDrySend({
373+
cfg: workspaceConfig,
374+
actionParams: {
375+
channel: "workspace",
376+
target: "#C12345678",
377+
message: "hello",
378+
pollQuestion: "",
379+
pollOption: [],
380+
pollDurationHours: 1,
381+
pollMulti: false,
382+
},
383+
toolContext: { currentChannelId: "C12345678" },
384+
});
385+
386+
expect(result.kind).toBe("send");
387+
});
365388
});
366389

367390
describe("message body alias normalization", () => {

‎src/poll-params.test.ts‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ describe("poll params", () => {
1212
).toBe(false);
1313
});
1414

15-
it.each([{ key: "pollMulti" }, { key: "pollAnonymous" }, { key: "pollPublic" }])(
16-
"treats $key=true as poll creation intent",
15+
it.each([{ key: "pollAnonymous" }, { key: "pollPublic" }])(
16+
"treats channel-extra $key=true as poll creation intent",
1717
({ key }) => {
1818
expect(
1919
hasPollCreationParams({
@@ -23,27 +23,44 @@ describe("poll params", () => {
2323
},
2424
);
2525

26-
it("treats non-zero finite numeric poll params as poll creation intent", () => {
26+
it("treats non-zero finite numeric channel-extra poll params as poll creation intent", () => {
2727
expect(hasPollCreationParams({ pollDurationSeconds: 60 })).toBe(true);
2828
expect(hasPollCreationParams({ pollDurationSeconds: "60" })).toBe(true);
2929
expect(hasPollCreationParams({ pollDurationSeconds: "+60" })).toBe(true);
3030
expect(hasPollCreationParams({ pollDurationSeconds: "1e3" })).toBe(true);
31-
expect(hasPollCreationParams({ pollDurationHours: -1 })).toBe(true);
3231
expect(hasPollCreationParams({ pollDurationSeconds: "-5" })).toBe(true);
33-
expect(hasPollCreationParams({ pollDurationHours: Number.NaN })).toBe(false);
3432
expect(hasPollCreationParams({ pollDurationSeconds: Infinity })).toBe(false);
3533
expect(hasPollCreationParams({ pollDurationSeconds: "60abc" })).toBe(false);
3634
expect(hasPollCreationParams({ pollDurationSeconds: "0x10" })).toBe(false);
3735
});
3836

39-
it("does not treat zero-valued numeric poll params as poll creation intent", () => {
37+
it("does not treat zero-valued numeric channel-extra poll params as poll creation intent", () => {
4038
// Zero values are typically defaults/unset values from tool schemas,
4139
// not intentional poll creation. Fixes #52118.
42-
expect(hasPollCreationParams({ pollDurationHours: 0 })).toBe(false);
4340
expect(hasPollCreationParams({ pollDurationSeconds: 0 })).toBe(false);
44-
expect(hasPollCreationParams({ pollDurationHours: "0" })).toBe(false);
4541
expect(hasPollCreationParams({ poll_duration_seconds: 0 })).toBe(false);
42+
});
43+
44+
it("does not treat shared modifier params (pollDurationHours, pollMulti) as poll creation intent without a question or options", () => {
45+
// These two are exposed by the shared `message` tool schema for both
46+
// `send` and `poll` actions, so LLMs routinely schema-pad them on every
47+
// plain `send` call with their schema-implied defaults (1 for an integer
48+
// with `minimum: 1`, `false` for a boolean). Treating those defaults as
49+
// poll intent blocks routine sends — see the regression that motivated
50+
// this carve-out.
51+
expect(hasPollCreationParams({ pollDurationHours: 1 })).toBe(false);
52+
expect(hasPollCreationParams({ pollDurationHours: 1, pollMulti: false })).toBe(false);
53+
expect(hasPollCreationParams({ pollDurationHours: 0 })).toBe(false);
54+
expect(hasPollCreationParams({ pollDurationHours: -1 })).toBe(false);
55+
expect(hasPollCreationParams({ pollDurationHours: "0" })).toBe(false);
56+
expect(hasPollCreationParams({ pollDurationHours: Number.NaN })).toBe(false);
4657
expect(hasPollCreationParams({ poll_duration_hours: "0" })).toBe(false);
58+
expect(hasPollCreationParams({ pollMulti: true })).toBe(false);
59+
});
60+
61+
it("still flags shared modifier params when accompanied by a question or options", () => {
62+
expect(hasPollCreationParams({ pollQuestion: "Ready?", pollDurationHours: 1 })).toBe(true);
63+
expect(hasPollCreationParams({ pollOption: ["Yes", "No"], pollMulti: true })).toBe(true);
4764
});
4865

4966
it("treats string-encoded boolean poll params as poll creation intent when true", () => {

‎src/poll-params.ts‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,18 @@ function hasExplicitUnknownPollValue(key: string, value: unknown): boolean {
7474
return false;
7575
}
7676

77-
export function hasPollCreationParams(params: Record<string, unknown>): boolean {
78-
for (const key of SHARED_POLL_CREATION_PARAM_NAMES) {
77+
// Among the shared poll params, only the content-bearing fields (pollQuestion,
78+
// pollOption) signal poll intent on their own. The modifier fields
79+
// (pollDurationHours, pollMulti) are exposed by the shared `message` tool
80+
// schema for both `send` and `poll` actions, so LLMs routinely echo their
81+
// schema-implied defaults (`1`, `false`) on plain `send` calls — see issue
82+
// for context. Treating those modifier defaults as "the agent meant to create
83+
// a poll" produces false positives and blocks routine sends. The modifiers
84+
// only count when accompanied by a content-bearing field.
85+
const CONTENT_BEARING_SHARED_POLL_PARAM_NAMES = ["pollQuestion", "pollOption"] as const;
86+
87+
function hasContentBearingPollCreationParam(params: Record<string, unknown>): boolean {
88+
for (const key of CONTENT_BEARING_SHARED_POLL_PARAM_NAMES) {
7989
const def = POLL_CREATION_PARAM_DEFS[key];
8090
const value = readPollParamRaw(params, key);
8191
if (def.kind === "string" && typeof value === "string" && value.trim().length > 0) {
@@ -92,30 +102,18 @@ export function hasPollCreationParams(params: Record<string, unknown>): boolean
92102
return true;
93103
}
94104
}
95-
if (def.kind === "positiveInteger") {
96-
// Treat zero-valued numeric defaults as unset, but preserve any non-zero
97-
// numeric value as explicit poll intent so invalid durations still hit
98-
// the poll-only validation path.
99-
if (typeof value === "number" && Number.isFinite(value) && value !== 0) {
100-
return true;
101-
}
102-
if (typeof value === "string") {
103-
const trimmed = value.trim();
104-
const parsed = parseStrictFiniteNumber(trimmed);
105-
if (parsed !== undefined && parsed !== 0) {
106-
return true;
107-
}
108-
}
109-
}
110-
if (def.kind === "boolean") {
111-
if (value === true) {
112-
return true;
113-
}
114-
if (typeof value === "string" && normalizeLowercaseStringOrEmpty(value) === "true") {
115-
return true;
116-
}
117-
}
118105
}
106+
return false;
107+
}
108+
109+
export function hasPollCreationParams(params: Record<string, unknown>): boolean {
110+
if (hasContentBearingPollCreationParam(params)) {
111+
return true;
112+
}
113+
// Channel-specific poll-prefixed params (e.g. pollDurationSeconds,
114+
// pollPublic) are not part of the shared schema, so an explicit value still
115+
// indicates deliberate poll intent and continues to trigger the validator
116+
// even without a pollQuestion/pollOption.
119117
for (const [key, value] of Object.entries(params)) {
120118
if (isChannelPollCreationParamName(key) && hasExplicitUnknownPollValue(key, value)) {
121119
return true;

0 commit comments

Comments
 (0)