Skip to content

Fix/send poll intent detection clean#2368

Open
BingqingLyu wants to merge 2 commits into
mainfrom
fork-pr-65301-fix-send-poll-intent-detection-clean
Open

Fix/send poll intent detection clean#2368
BingqingLyu wants to merge 2 commits into
mainfrom
fork-pr-65301-fix-send-poll-intent-detection-clean

Conversation

@BingqingLyu

@BingqingLyu BingqingLyu commented Apr 28, 2026

Copy link
Copy Markdown
Owner

Summary

Describe the problem and fix in 2–5 bullets:

If this PR fixes a plugin beta-release blocker, title it fix(<plugin-id>): beta blocker - <summary> and link the matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

  • Problem: message-action-runner treated any poll-related params on action="send" as poll intent, including metadata-only fields like duration and boolean flags.
  • Why it matters: normal sends could be rejected even when the payload did not actually contain a poll question or options.
  • What changed: send now only treats non-empty pollQuestion or pollOption as real poll intent, and regression tests were added for both the relaxed send path and the retained real-poll rejection path.
  • What did NOT change (scope boundary): poll creation behavior, poll validation rules, and poll-only parameter parsing for actual action="poll" requests were not changed.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write N/A. If the cause is unclear, write Unknown.

  • Root cause: the send-side guard reused broad poll-parameter detection that was designed to notice any poll-related fields, not to distinguish real poll payloads from metadata-only params.
  • Missing detection / guardrail: there was no narrower intent check requiring actual poll content such as a question or options before rejecting action="send".
  • Contributing context (if known): tool schemas and channel-specific params can populate poll metadata fields independently of a real poll payload, which made the broad check too aggressive.

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/poll-params.test.ts, src/infra/outbound/message-action-runner.core-send.test.ts
  • Scenario the test should lock in: action="send" must succeed when only poll metadata is present, and must still reject when pollQuestion or pollOption indicates a real poll payload.
  • Why this is the smallest reliable guardrail: the regression lives in local parameter classification and send routing, so focused unit tests cover the failure mode without requiring channel end-to-end setup.
  • Existing test that already covers this (if any): there was partial coverage for zero-valued poll params, but not for non-question metadata-only poll params on send.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

message send no longer rejects non-poll sends just because poll metadata fields are present. Real poll payloads on action="send" are still rejected and must use action="poll".

Diagram (if applicable)

For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write N/A.

Before:
[send action + poll metadata only] -> [broad poll param check] -> [rejected as poll]

After:
[send action + poll metadata only] -> [real poll intent check] -> [sent normally]
[send action + pollQuestion/pollOption] -> [real poll intent check] -> [rejected, use poll action]

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: Linux
  • Runtime/container: local repo checkout
  • Model/provider: N/A
  • Integration/channel (if any): outbound message action routing
  • Relevant config (redacted): minimal test config with outbound test plugin

Steps

  1. Run runMessageAction with action: "send" and metadata-only poll fields such as pollDurationHours, pollMulti, or pollAnonymous.
  2. Observe behavior before the fix: the request is rejected with Poll fields require action "poll".
  3. Run the same scenario after the fix and confirm the send succeeds.
  4. Run runMessageAction with action: "send" plus pollQuestion or pollOption.
  5. Confirm the request is still rejected and must use action: "poll".

Expected

  • Metadata-only poll params do not force action="send" into the poll-only rejection path.
  • Real poll payloads still require action="poll".

Actual

  • Before the fix, metadata-only poll params could incorrectly trigger poll rejection.
  • After the fix, only real poll content triggers that rejection.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: targeted unit tests for hasRealPollIntent and runMessageAction send routing were run locally.
  • Edge cases checked: empty question, empty option array, metadata-only params, and real poll question payloads.
  • What you did not verify: live channel delivery against real Telegram/Discord/other integrations was not manually exercised.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

  • Risk: some caller may have implicitly depended on metadata-only poll params being rejected on action="send".
    • Mitigation: the new tests lock the intended behavior to actual poll content only, which matches the error message and action contract more closely.

@BingqingLyu BingqingLyu added conflicting-group-2 Conflicting PR group 2 — review as a batch conflicting-pr Shares at least one cross-PR dependency with other PRs labels May 26, 2026
@BingqingLyu

Copy link
Copy Markdown
Owner Author

⚠️ Cross-PR Conflict Detected

This PR shares modified code with #2519.

Shared functions:

Function File Also modified by
runMessageAction src/infra/outbound/message-action-runner.ts #2519

Recommendation: Coordinate with #2519 before merging.


Auto-detected by codegraph — a code graph analysis tool built on neug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conflicting-group-2 Conflicting PR group 2 — review as a batch conflicting-pr Shares at least one cross-PR dependency with other PRs

Projects

None yet

2 participants