Repository navigation
aggregator(actions): one answer action may name the same survey question twice — duplicate questionId entries with contradictory values pass validateNewAction and checkActionAgainstItem #4291
Description
Problem
One answer action may carry the same survey question twice in a single payload.answers array — with contradictory values — and every layer accepts it:
validateNewAction(pm-stack/aggregator/src/actions.ts:275-303) converts each answers entry independently (array of at most 50) and never tracks whichquestionIds it has already seen.checkActionAgainstItem(pm-stack/aggregator/src/tier-items.ts:348-362) checks each entry's membership/value against the published question, again per entry, with no duplicate detection.
The item side enforces the opposite invariant: sanitizeQuestions / sanitizeOptions (tier-items.ts) drop later entries whose id repeats (seen sets). The answer side has no mirror of that rule.
Evidence (verified this run, laptop probe against real code @ cc819fe, md5-matched)
Probe: ~/git/fleet/probe-2026-11-02-laneB-answeroption/ (verbatim imports of src/tier-items.ts; item fixture mirrors test/needs-user.test.ts — single question audience, options internal/public):
- control —
answers: [{questionId: "audience", value: "internal"}]→null(accepted, correctly) - control — an answer naming an unknown question or an unpublished option value is rejected by the same function (existing tests, test/needs-user.test.ts:316-330)
- bug —
answers: [{questionId: "audience", value: "internal"}, {questionId: "audience", value: "public"}]→null(accepted, both entries stored)
Why it matters
The stored action is self-contradictory, and its two readers resolve the contradiction differently and silently:
cardState(src/card-state.ts:89-99) merges answers into aMapper question, latest entry wins — the rendered card shows onlypublic, with no trace that the same submit also saidinternal.- The consumer receives the raw payload (actions.ts header: the orchestration pass "delivers payload text/option/answers like its own classifications") — which of the two values it applies is an accident of its own iteration, not a contract.
A double-submit race, a stale form, or a buggy client can therefore record two opposite answers to a required question in one auditable action, and the dashboard's merged view will disagree with what a consumer reading the payload in order applies first.
Acceptance criteria
- A single
answeraction whoseanswersarray names the samequestionIdmore than once is rejected at the boundary — either invalidateNewActionor incheckActionAgainstItem— with a legible error naming the duplicated question. (Rejecting, not silently deduping, matches the boundary's fail-legible style; if dedupe is chosen instead, the surviving value must be the one both the card merge and the consumer see, and that choice must be tested.) - Regression tests in pm-stack/aggregator/test/needs-user.test.ts: duplicate questionId with the same value and with contradictory values; distinct questions in one submit still pass; partial submits (feat(dashboard): multi-question cards — render every question, submit one answers array #1451) are unaffected.
- Aggregator test suite passes (
bun testin pm-stack/aggregator).
Distinct from: #1476 (the dashboard UI presenting one question through multiple answer widgets — this is the POST boundary accepting duplicates from any client), and from the optionId-shielding gap filed this run as #4286.
Activity
dezren39 commentedon Nov 2, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 2, 2026
🤖 Agent note · picked up
validateNewAction/checkActionAgainstItem), and route the fix.dezren39 commentedon Nov 2, 2026
🤖 Agent note · done
Evidence (fresh this shift)
d18d61735(laptop primary, HEAD == origin/main):validateNewAction(pm-stack/aggregator/src/actions.ts:275-303) converts eachanswersentry independently — array cap 50 only, no seen-questionId set;checkActionAgainstItem(src/tier-items.ts:340+) loops entries with per-entry membership/value checks, no duplicate detection. (The file changed since the creator probe only via the aggregator: proposal expiry is clocked from the delegation's updatedAt, not the proposal's draft time — a 72h-old proposal stays approvable, a 1h-old one reads expired #1623 commit4d05b191c, unrelated to this function.)probe.tsimports the primary clone's current sources directly:BUG2 duplicate questionId contradictory => null(one action answeringaudience=internalandaudience=publicis accepted). Controls correct: unpublishedoptionIdalone is rejected; a valid single answer is accepted.cardStatemerges answers latest-wins per question, so the card would show onlypublicwhile a raw-payload consumer sees both values — the silent divergence the issue describes.Routing
questionIdwithin oneanswersarray at the validation boundary with a legible error; regression tests intest/needs-user.test.ts(same-value + contradictory duplicates rejected; distinct questions and feat(dashboard): multi-question cards — render every question, submit one answers array #1451 partial submits unchanged).4 remaining items
dezren39 commentedon Nov 2, 2026
Pickup by repo queue worker lane H for task 6167 (small_fix for #4291). Fresh revalidation 2026-11-02T17:12Z: #4291 OPEN, no hard-stop labels, no open PR owns this fix. Implementing the duplicate-questionId rejection at the validation boundary (validateNewAction in src/actions.ts and checkActionAgainstItem in src/tier-items.ts) with regression tests in test/needs-user.test.ts, on branch queue/6167-4291-dup-question-id.
dezren39 commentedon Nov 2, 2026
Implementation by repo queue worker lane H for task 6167: PR #4482 opened (branch
queue/6167-4291-dup-question-id, commit 710442b). A duplicate questionId in oneanswersarray is now rejected at both boundaries —validateNewAction(src/actions.ts) andcheckActionAgainstItem(src/tier-items.ts) — with a legible error naming the question. Regression tests in test/needs-user.test.ts: 27 pass, 0 fail (bun teston the laptop). Full aggregator suite: 1172 pass; the brief-surfaces failure and the web-push module error are pre-existing on the clean tree (verified via stash). #1451 partial submits across separate actions are unchanged.Merge pull request #4482 from developing-today/queue/6167-4291-dup-qu…
dezren39 commentedon Nov 2, 2026
Done by repo queue worker lane H (task 6167): PR #4482 merged (merge commit cefcdce) and #4291 is closed. Codex review on PR #4482 completed with no findings. Verification as recorded on the PR:
bun test test/needs-user.test.ts27 pass / 0 fail on the laptop; the full-suite brief-surfaces failure and web-push module error are pre-existing on the clean tree (stash-verified).