diff --git a/container/agent-runner/src/mcp-tools/interactive.ts b/container/agent-runner/src/mcp-tools/interactive.ts index 330c50c..82833c7 100644 --- a/container/agent-runner/src/mcp-tools/interactive.ts +++ b/container/agent-runner/src/mcp-tools/interactive.ts @@ -37,26 +37,53 @@ export const askUserQuestion: McpToolDefinition = { tool: { name: 'ask_user_question', description: - 'Ask the user a multiple-choice question and wait for their response. This is a blocking call — execution pauses until the user responds or the timeout expires.', + 'Ask the user a multiple-choice question and wait for their response. This is a blocking call — execution pauses until the user responds or the timeout expires. Provide a short card title (e.g. "Confirm deletion") and an array of options — each option may be a plain string (used as both button label and result value) or an object { label, selectedLabel?, value? } where selectedLabel is the text shown on the card after the user clicks.', inputSchema: { type: 'object' as const, properties: { + title: { type: 'string', description: 'Short card title shown above the question' }, question: { type: 'string', description: 'The question to ask' }, options: { type: 'array', - items: { type: 'string' }, - description: 'Button labels for the user to choose from', + items: { + oneOf: [ + { type: 'string' }, + { + type: 'object', + properties: { + label: { type: 'string' }, + selectedLabel: { type: 'string' }, + value: { type: 'string' }, + }, + required: ['label'], + }, + ], + }, + description: 'Options for the user to choose from (string or {label, selectedLabel?, value?})', }, timeout: { type: 'number', description: 'Timeout in seconds (default: 300)' }, }, - required: ['question', 'options'], + required: ['title', 'question', 'options'], }, }, async handler(args) { + const title = args.title as string; const question = args.question as string; - const options = args.options as string[]; + const rawOptions = args.options as unknown[]; const timeout = ((args.timeout as number) || 300) * 1000; - if (!question || !options?.length) return err('question and options are required'); + if (!title || !question || !rawOptions?.length) { + return err('title, question, and options are required'); + } + + const options = rawOptions.map((o) => { + if (typeof o === 'string') return { label: o, selectedLabel: o, value: o }; + const obj = o as { label: string; selectedLabel?: string; value?: string }; + return { + label: obj.label, + selectedLabel: obj.selectedLabel ?? obj.label, + value: obj.value ?? obj.label, + }; + }); const questionId = generateId(); const r = routing(); @@ -71,6 +98,7 @@ export const askUserQuestion: McpToolDefinition = { content: JSON.stringify({ type: 'ask_question', questionId, + title, question, options, }), diff --git a/docs/v2-agent-runner-details.md b/docs/v2-agent-runner-details.md index 1059213..30f8af8 100644 --- a/docs/v2-agent-runner-details.md +++ b/docs/v2-agent-runner-details.md @@ -512,8 +512,9 @@ Send an interactive question and wait for the user's response. This is a **block { name: 'ask_user_question', params: { + title: string, // short card title, e.g. "Confirm deletion" question: string, - options: string[], // button labels + options: (string | { label: string; selectedLabel?: string; value?: string })[], timeout?: number, // seconds (default: 300) } } diff --git a/docs/v2-api-details.md b/docs/v2-api-details.md index 02ba7c5..37b3188 100644 --- a/docs/v2-api-details.md +++ b/docs/v2-api-details.md @@ -309,8 +309,13 @@ function createWhatsAppChannel(): ChannelAdapter { { "operation": "ask_question", "questionId": "q-123", + "title": "Failing Test", "question": "How should we handle the failing test?", - "options": ["Skip it", "Fix and retry", "Abort deployment"] + "options": [ + "Skip it", + { "label": "Fix and retry", "selectedLabel": "✅ Fixing", "value": "fix" }, + { "label": "Abort deployment", "selectedLabel": "❌ Aborted", "value": "abort" } + ] } ``` diff --git a/docs/v2-architecture-draft.md b/docs/v2-architecture-draft.md index b3a9db9..271a2b0 100644 --- a/docs/v2-architecture-draft.md +++ b/docs/v2-architecture-draft.md @@ -350,7 +350,7 @@ Agent calls `add_reaction` tool with message ID and emoji. Agent-runner writes m { "text": "LGTM" } // Interactive card -{ "operation": "ask_question", "question": "Approve deployment?", "options": ["Yes", "No", "Defer"] } +{ "operation": "ask_question", "title": "Deploy", "question": "Approve deployment?", "options": ["Yes", "No", "Defer"] } // Edit existing message { "operation": "edit", "messageId": "3", "text": "Updated: LGTM with minor comments" } diff --git a/src/channels/ask-question.ts b/src/channels/ask-question.ts new file mode 100644 index 0000000..9f71417 --- /dev/null +++ b/src/channels/ask-question.ts @@ -0,0 +1,46 @@ +/** + * Shared ask_question payload schema + normalization. + * + * Producers (host-side approvals, container-side ask_user_question MCP tool) + * emit an `ask_question` payload. Options may be bare strings for ergonomics, + * but are normalized here into a consistent shape before delivery, persistence, + * and rendering. + */ + +export interface OptionInput { + label: string; + selectedLabel?: string; + value?: string; +} + +export type RawOption = string | OptionInput; + +export interface NormalizedOption { + label: string; + selectedLabel: string; + value: string; +} + +export function normalizeOption(raw: RawOption): NormalizedOption { + if (typeof raw === 'string') { + return { label: raw, selectedLabel: raw, value: raw }; + } + const label = raw.label; + return { + label, + selectedLabel: raw.selectedLabel ?? label, + value: raw.value ?? label, + }; +} + +export function normalizeOptions(raws: RawOption[]): NormalizedOption[] { + return raws.map(normalizeOption); +} + +export interface AskQuestionPayload { + type: 'ask_question'; + questionId: string; + title: string; + question: string; + options: NormalizedOption[]; +} diff --git a/src/channels/chat-sdk-bridge.ts b/src/channels/chat-sdk-bridge.ts index 21c5088..e4ba6b5 100644 --- a/src/channels/chat-sdk-bridge.ts +++ b/src/channels/chat-sdk-bridge.ts @@ -21,6 +21,8 @@ import { import { log } from '../log.js'; import { SqliteStateAdapter } from '../state-sqlite.js'; import { registerWebhookAdapter } from '../webhook-server.js'; +import { getPendingQuestion } from '../db/sessions.js'; +import { normalizeOptions, type NormalizedOption } from './ask-question.js'; import type { ChannelAdapter, ChannelSetup, ConversationConfig, InboundMessage } from './adapter.js'; /** Adapter with optional gateway support (e.g., Discord). */ @@ -243,11 +245,17 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter const selectedOption = event.value || ''; const userId = event.user?.userId || ''; + // Look up the pending question BEFORE dispatching onAction (which deletes it). + const pq = getPendingQuestion(questionId); + const title = pq?.title ?? '❓ Question'; + const matched = pq?.options.find((o) => o.value === selectedOption); + const selectedLabel = matched?.selectedLabel ?? selectedOption ?? '(clicked)'; + // Update the card to show the selected answer and remove buttons try { const tid = event.threadId; await adapter.editMessage(tid, event.messageId, { - markdown: `❓ **Question**\n\n${selectedOption ? `✅ **${selectedOption}**` : '(clicked)'}`, + markdown: `${title}\n\n${selectedLabel}`, }); } catch (err) { log.warn('Failed to update card after action', { err }); @@ -342,17 +350,27 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter // Ask question card — render as Card with buttons if (content.type === 'ask_question' && content.questionId && content.options) { const questionId = content.questionId as string; - const options = content.options as string[]; + const title = content.title as string; + const question = content.question as string; + if (!title) { + log.error('ask_question missing required title — skipping delivery', { questionId }); + return; + } + const options: NormalizedOption[] = normalizeOptions(content.options as never); const card = Card({ - title: '❓ Question', + title, children: [ - CardText(content.question as string), - Actions(options.map((opt) => Button({ id: `ncq:${questionId}:${opt}`, label: opt, value: opt }))), + CardText(question), + Actions( + options.map((opt) => + Button({ id: `ncq:${questionId}:${opt.value}`, label: opt.label, value: opt.value }), + ), + ), ], }); const result = await adapter.postMessage(tid, { card, - fallbackText: `${content.question}\nOptions: ${options.join(', ')}`, + fallbackText: `${title}\n\n${question}\nOptions: ${options.map((o) => o.label).join(', ')}`, }); return result?.id; } @@ -502,6 +520,10 @@ async function handleForwardedEvent( const originalEmbeds = ((interaction.message as Record)?.embeds as Array>) || []; const originalDescription = (originalEmbeds[0]?.description as string) || ''; + const pq = questionId ? getPendingQuestion(questionId) : undefined; + const cardTitle = pq?.title ?? ((originalEmbeds[0]?.title as string) || '❓ Question'); + const matchedOpt = pq?.options.find((o) => o.value === selectedOption); + const selectedLabel = matchedOpt?.selectedLabel ?? selectedOption ?? customId; try { await fetch(`https://discord.com/api/v10/interactions/${interactionId}/${interactionToken}/callback`, { method: 'POST', @@ -511,8 +533,8 @@ async function handleForwardedEvent( data: { embeds: [ { - title: '❓ Question', - description: `${originalDescription}\n\n✅ **${selectedOption || customId}**`, + title: cardTitle, + description: `${originalDescription}\n\n${selectedLabel}`, }, ], components: [], // remove buttons diff --git a/src/channels/whatsapp.ts b/src/channels/whatsapp.ts index af43c72..e5723df 100644 --- a/src/channels/whatsapp.ts +++ b/src/channels/whatsapp.ts @@ -32,6 +32,7 @@ import { ASSISTANT_HAS_OWN_NUMBER, ASSISTANT_NAME, DATA_DIR } from '../config.js import { readEnvFile } from '../env.js'; import { log } from '../log.js'; import { registerChannelAdapter } from './channel-registry.js'; +import { normalizeOptions, type NormalizedOption } from './ask-question.js'; import type { ChannelAdapter, ChannelSetup, @@ -69,7 +70,7 @@ const SENT_MESSAGE_CACHE_MAX = 256; const RECONNECT_DELAY_MS = 5000; const PENDING_QUESTIONS_MAX = 64; -/** Normalize an option name to a slash command: "Approve" → "/approve" */ +/** Normalize an option label to a slash command: "Approve" → "/approve" */ function optionToCommand(option: string): string { return '/' + option.toLowerCase().replace(/\s+/g, '-'); } @@ -183,7 +184,7 @@ registerChannelAdapter('whatsapp', { string, { questionId: string; - options: string[]; + options: NormalizedOption[]; } >(); @@ -549,15 +550,17 @@ registerChannelAdapter('whatsapp', { const pending = pendingQuestions.get(chatJid); if (pending && content.startsWith('/')) { const cmd = content.trim().toLowerCase(); - const matched = pending.options.find((o) => optionToCommand(o) === cmd); + const matched = pending.options.find((o) => optionToCommand(o.label) === cmd); if (matched) { const voterName = msg.pushName || sender.split('@')[0]; - setupConfig.onAction(pending.questionId, matched, sender); + setupConfig.onAction(pending.questionId, matched.value, sender); pendingQuestions.delete(chatJid); - // Past tense for common actions: Approve→Approved, Reject→Rejected - const label = matched.endsWith('e') ? `${matched}d` : `${matched}ed`; - await sendRawMessage(chatJid, `*${label}* by ${voterName}`); - log.info('Question answered', { questionId: pending.questionId, matched, voterName }); + await sendRawMessage(chatJid, `${matched.selectedLabel} by ${voterName}`); + log.info('Question answered', { + questionId: pending.questionId, + value: matched.value, + voterName, + }); continue; // Don't forward this reply to the agent } } @@ -621,11 +624,16 @@ registerChannelAdapter('whatsapp', { // Ask question → text with slash command replies if (content.type === 'ask_question' && content.questionId && content.options) { const questionId = content.questionId as string; + const title = content.title as string; const question = content.question as string; - const options = content.options as string[]; + if (!title) { + log.error('ask_question missing required title — skipping delivery', { questionId }); + return; + } + const options: NormalizedOption[] = normalizeOptions(content.options as never); - const optionLines = options.map((o) => ` ${optionToCommand(o)}`).join('\n'); - const text = `${question}\n\nReply with:\n${optionLines}`; + const optionLines = options.map((o) => ` ${optionToCommand(o.label)}`).join('\n'); + const text = `*${title}*\n\n${question}\n\nReply with:\n${optionLines}`; const msgId = await sendRawMessage(platformId, text); if (msgId) { pendingQuestions.set(platformId, { questionId, options }); diff --git a/src/db/db-v2.test.ts b/src/db/db-v2.test.ts index 8e7f05d..52e89ea 100644 --- a/src/db/db-v2.test.ts +++ b/src/db/db-v2.test.ts @@ -375,11 +375,15 @@ describe('pending questions', () => { platform_id: 'chan-1', channel_type: 'discord', thread_id: null, + title: 'Test', + options: [{ label: 'Yes', selectedLabel: 'Yes', value: 'yes' }], created_at: now(), }); const result = getPendingQuestion('q-1'); expect(result).toBeDefined(); expect(result!.session_id).toBe('sess-1'); + expect(result!.title).toBe('Test'); + expect(result!.options[0].value).toBe('yes'); }); it('should delete', () => { @@ -390,6 +394,8 @@ describe('pending questions', () => { platform_id: null, channel_type: null, thread_id: null, + title: 'Test', + options: [{ label: 'Yes', selectedLabel: 'Yes', value: 'yes' }], created_at: now(), }); deletePendingQuestion('q-1'); diff --git a/src/db/migrations/001-initial.ts b/src/db/migrations/001-initial.ts index d32b3c2..5dec523 100644 --- a/src/db/migrations/001-initial.ts +++ b/src/db/migrations/001-initial.ts @@ -61,6 +61,8 @@ export const migration001: Migration = { platform_id TEXT, channel_type TEXT, thread_id TEXT, + title TEXT NOT NULL, + options_json TEXT NOT NULL, created_at TEXT NOT NULL ); `); diff --git a/src/db/schema.ts b/src/db/schema.ts index 6f8a803..9473c42 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -64,6 +64,8 @@ CREATE TABLE pending_questions ( platform_id TEXT, channel_type TEXT, thread_id TEXT, + title TEXT NOT NULL, + options_json TEXT NOT NULL, created_at TEXT NOT NULL ); `; diff --git a/src/db/sessions.ts b/src/db/sessions.ts index 75aacd2..129f6da 100644 --- a/src/db/sessions.ts +++ b/src/db/sessions.ts @@ -75,16 +75,29 @@ export function deleteSession(id: string): void { export function createPendingQuestion(pq: PendingQuestion): void { getDb() .prepare( - `INSERT INTO pending_questions (question_id, session_id, message_out_id, platform_id, channel_type, thread_id, created_at) - VALUES (@question_id, @session_id, @message_out_id, @platform_id, @channel_type, @thread_id, @created_at)`, + `INSERT INTO pending_questions (question_id, session_id, message_out_id, platform_id, channel_type, thread_id, title, options_json, created_at) + VALUES (@question_id, @session_id, @message_out_id, @platform_id, @channel_type, @thread_id, @title, @options_json, @created_at)`, ) - .run(pq); + .run({ + question_id: pq.question_id, + session_id: pq.session_id, + message_out_id: pq.message_out_id, + platform_id: pq.platform_id, + channel_type: pq.channel_type, + thread_id: pq.thread_id, + title: pq.title, + options_json: JSON.stringify(pq.options), + created_at: pq.created_at, + }); } export function getPendingQuestion(questionId: string): PendingQuestion | undefined { - return getDb().prepare('SELECT * FROM pending_questions WHERE question_id = ?').get(questionId) as - | PendingQuestion - | undefined; + const row = getDb() + .prepare('SELECT * FROM pending_questions WHERE question_id = ?') + .get(questionId) as (Omit & { options_json: string }) | undefined; + if (!row) return undefined; + const { options_json, ...rest } = row; + return { ...rest, options: JSON.parse(options_json) }; } export function deletePendingQuestion(questionId: string): void { diff --git a/src/delivery.ts b/src/delivery.ts index fdfc837..51bb7b5 100644 --- a/src/delivery.ts +++ b/src/delivery.ts @@ -40,6 +40,7 @@ import { resumeTask, } from './db/session-db.js'; import { log } from './log.js'; +import { normalizeOptions, type RawOption } from './channels/ask-question.js'; import { openInboundDb, openOutboundDb, @@ -110,11 +111,17 @@ function notifyAgent(session: Session, text: string): void { * The admin's button click routes via the existing ncq: card infrastructure to * handleApprovalResponse in index.ts, which completes the action. */ +const APPROVAL_OPTIONS: RawOption[] = [ + { label: 'Approve', selectedLabel: '✅ Approved', value: 'approve' }, + { label: 'Reject', selectedLabel: '❌ Rejected', value: 'reject' }, +]; + async function requestApproval( session: Session, agentName: string, action: 'install_packages' | 'request_rebuild' | 'add_mcp_server', payload: Record, + title: string, question: string, ): Promise { const adminGroup = getAdminAgentGroup(); @@ -145,8 +152,9 @@ async function requestApproval( JSON.stringify({ type: 'ask_question', questionId: approvalId, + title, question, - options: ['Approve', 'Reject'], + options: APPROVAL_OPTIONS, }), ); } catch (err) { @@ -356,16 +364,26 @@ async function deliverMessage( // Track pending questions for ask_user_question flow if (content.type === 'ask_question' && content.questionId) { - createPendingQuestion({ - question_id: content.questionId, - session_id: session.id, - message_out_id: msg.id, - platform_id: msg.platform_id, - channel_type: msg.channel_type, - thread_id: msg.thread_id, - created_at: new Date().toISOString(), - }); - log.info('Pending question created', { questionId: content.questionId, sessionId: session.id }); + const title = content.title as string | undefined; + const rawOptions = content.options as unknown; + if (!title || !Array.isArray(rawOptions)) { + log.error('ask_question missing required title/options — not persisting', { + questionId: content.questionId, + }); + } else { + createPendingQuestion({ + question_id: content.questionId, + session_id: session.id, + message_out_id: msg.id, + platform_id: msg.platform_id, + channel_type: msg.channel_type, + thread_id: msg.thread_id, + title, + options: normalizeOptions(rawOptions as never), + created_at: new Date().toISOString(), + }); + log.info('Pending question created', { questionId: content.questionId, sessionId: session.id }); + } } // Channel delivery @@ -584,7 +602,8 @@ async function handleSystemAction( args: (content.args as string[]) || [], env: (content.env as Record) || {}, }, - `Agent "${agentGroup.name}" requests a new MCP server:\n${serverName} (${command})`, + 'Add MCP Request', + `Agent "${agentGroup.name}" is attempting to add a new MCP server:\n${serverName} (${command})`, ); break; } @@ -633,7 +652,8 @@ async function handleSystemAction( agentGroup.name, 'install_packages', { apt, npm, reason }, - `Agent "${agentGroup.name}" requests package install + container rebuild:\n${packageList}${reason ? `\nReason: ${reason}` : ''}`, + 'Install Packages Request', + `Agent "${agentGroup.name}" is attempting to install a package + rebuild container:\n${packageList}${reason ? `\nReason: ${reason}` : ''}`, ); break; } @@ -650,7 +670,8 @@ async function handleSystemAction( agentGroup.name, 'request_rebuild', { reason }, - `Agent "${agentGroup.name}" requests a container rebuild.${reason ? `\nReason: ${reason}` : ''}`, + 'Rebuild Request', + `Agent "${agentGroup.name}" is attempting to rebuild container.${reason ? `\nReason: ${reason}` : ''}`, ); break; } diff --git a/src/index.ts b/src/index.ts index 77f4cb6..fada49a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -262,7 +262,7 @@ async function handleApprovalResponse( }); }; - if (selectedOption !== 'Approve') { + if (selectedOption !== 'approve') { notify(`Your ${approval.action} request was rejected by admin.`); log.info('Approval rejected', { approvalId: approval.approval_id, action: approval.action, userId }); deletePendingApproval(approval.approval_id); diff --git a/src/onecli-approvals.ts b/src/onecli-approvals.ts index c8d6558..7030d7e 100644 --- a/src/onecli-approvals.ts +++ b/src/onecli-approvals.ts @@ -71,7 +71,7 @@ export function resolveOneCLIApproval(approvalId: string, selectedOption: string pending.delete(approvalId); clearTimeout(state.timer); - const decision: Decision = selectedOption === 'Approve' ? 'approve' : 'deny'; + const decision: Decision = selectedOption === 'approve' ? 'approve' : 'deny'; updatePendingApprovalStatus(approvalId, decision === 'approve' ? 'approved' : 'rejected'); // Card is auto-edited to "✅