test(channels): cover ask_question option style normalization

Reviewers on #2933 asked for tests on the button-style plumbing. Two layers:

- src/channels/ask-question.test.ts (new): normalizeOption/normalizeOptions
  style whitelist — primary/danger/default pass through; unknown strings,
  case variants, and non-string values drop to undefined; string-shorthand
  options carry no style; style coexists with the label/selectedLabel/value
  defaulting. This is the load-bearing gate: an invalid style reaching
  Slack Block Kit fails the whole card with invalid_blocks, which in the
  approval flow is an effective auto-deny.

- src/channels/chat-sdk-bridge.test.ts: ask_question delivery passes each
  normalized option style into Button() and omits it when unset; an invalid
  style in the raw payload is stripped before the card is built.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
gavrielc
2026-07-04 16:39:04 +03:00
parent 2bce84781b
commit e3d156f800
2 changed files with 147 additions and 0 deletions
+76
View File
@@ -0,0 +1,76 @@
import { describe, expect, it } from 'vitest';
import { normalizeOption, normalizeOptions } from './ask-question.js';
describe('normalizeOption — style whitelist', () => {
// The style value flows straight into the Chat SDK Button() and from there
// into Slack Block Kit. Slack rejects the *entire* message with
// invalid_blocks if a button carries an unknown style, which in the
// approval flow means the card never renders — an effective auto-deny.
// So anything outside the whitelist must drop to undefined here.
it.each(['primary', 'danger', 'default'] as const)('passes through the known style %j', (style) => {
expect(normalizeOption({ label: 'Approve', style }).style).toBe(style);
});
it('drops unknown style strings to undefined', () => {
for (const bad of ['success', 'warning', 'PRIMARY', 'Danger', ' primary', 'primary ', '', 'red']) {
const opt = normalizeOption({ label: 'Approve', style: bad as never });
expect(opt.style, `style ${JSON.stringify(bad)} should be dropped`).toBeUndefined();
}
});
it('drops non-string style values to undefined', () => {
for (const bad of [1, true, null, {}, ['primary']]) {
const opt = normalizeOption({ label: 'Approve', style: bad as never });
expect(opt.style, `style ${JSON.stringify(bad)} should be dropped`).toBeUndefined();
}
});
it('leaves style undefined when the object option omits it', () => {
expect(normalizeOption({ label: 'Approve' }).style).toBeUndefined();
});
it('gives string-shorthand options no style', () => {
const opt = normalizeOption('Approve');
expect(opt).toEqual({ label: 'Approve', selectedLabel: 'Approve', value: 'Approve' });
expect('style' in opt && opt.style !== undefined).toBe(false);
});
it('style coexists with the label/selectedLabel/value defaulting', () => {
// Defaults still fill in around an explicit style…
expect(normalizeOption({ label: 'Approve', style: 'primary' })).toEqual({
label: 'Approve',
selectedLabel: 'Approve',
value: 'Approve',
style: 'primary',
});
// …and explicit fields are untouched by the style whitelist.
expect(normalizeOption({ label: 'Deny', selectedLabel: 'Denied', value: 'deny-1', style: 'danger' })).toEqual({
label: 'Deny',
selectedLabel: 'Denied',
value: 'deny-1',
style: 'danger',
});
// An invalid style must not disturb the rest of the normalization.
expect(normalizeOption({ label: 'Deny', value: 'deny-1', style: 'bogus' as never })).toEqual({
label: 'Deny',
selectedLabel: 'Deny',
value: 'deny-1',
style: undefined,
});
});
});
describe('normalizeOptions', () => {
it('normalizes mixed string and object options, preserving order and per-option styles', () => {
const out = normalizeOptions([
'Skip',
{ label: 'Approve', style: 'primary' },
{ label: 'Deny', style: 'danger' },
{ label: 'Later', style: 'lime' as never },
]);
expect(out.map((o) => o.label)).toEqual(['Skip', 'Approve', 'Deny', 'Later']);
expect(out.map((o) => o.style)).toEqual([undefined, 'primary', 'danger', undefined]);
});
});
+71
View File
@@ -238,6 +238,77 @@ describe('createChatSdkBridge.setup — webhook route and state namespace', () =
});
});
describe('createChatSdkBridge.deliver — ask_question cards (button styles)', () => {
// Approval cards color their buttons (Slack: primary→green, danger→red).
// The bridge must forward the normalized option style into Button() and
// omit it when unset — an invalid style surviving to Block Kit would fail
// the whole card with invalid_blocks (effective auto-deny).
interface CapturedButton {
type?: string;
id?: string;
label?: string;
value?: string;
style?: string;
}
function buttonsFrom(calls: PostCall[]): CapturedButton[] {
const msg = calls[0].message as {
card?: { children?: Array<{ type?: string; children?: CapturedButton[] }> };
};
const actionsRow = msg.card?.children?.find((c) => c.type === 'actions');
expect(actionsRow).toBeDefined();
return actionsRow?.children ?? [];
}
it('passes each option style through to the Button, and omits it when unset', async () => {
const { calls, postMessage } = makePostCapture();
const bridge = createChatSdkBridge({
adapter: stubAdapter({ postMessage }),
supportsThreads: false,
});
await bridge.deliver('slack:C1', null, {
kind: 'chat-sdk',
content: {
type: 'ask_question',
questionId: 'q-1',
title: 'Approval needed',
question: 'Allow the tool call?',
options: [
{ label: 'Approve', style: 'primary' },
{ label: 'Deny', style: 'danger' },
'Skip', // string shorthand — never styled
],
},
});
expect(calls).toHaveLength(1);
const buttons = buttonsFrom(calls);
expect(buttons.map((b) => b.label)).toEqual(['Approve', 'Deny', 'Skip']);
expect(buttons.map((b) => b.style)).toEqual(['primary', 'danger', undefined]);
});
it('drops invalid styles before they reach the Button (delivery goes through normalizeOptions)', async () => {
const { calls, postMessage } = makePostCapture();
const bridge = createChatSdkBridge({
adapter: stubAdapter({ postMessage }),
supportsThreads: false,
});
await bridge.deliver('slack:C1', null, {
kind: 'chat-sdk',
content: {
type: 'ask_question',
questionId: 'q-2',
title: 'Approval needed',
question: 'Allow the tool call?',
options: [{ label: 'Approve', style: 'chartreuse' }],
},
});
const buttons = buttonsFrom(calls);
expect(buttons).toHaveLength(1);
expect(buttons[0].style).toBeUndefined();
});
});
describe('createChatSdkBridge.deliver — display cards (send_card)', () => {
// The send_card MCP tool writes outbound rows with `{ type: 'card', card, fallbackText }`.
// Before this branch existed the bridge silently dropped them: cards have no