From 9f43b5306dca66449cd4bf82b4229ed56c267adb Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Sun, 13 Sep 2026 18:45:50 +0300 Subject: [PATCH 1/2] feat(slack): add durable agent question cards and forms Signed-off-by: Tiberiu Socaci --- AGENTS.md | 4 +- CHANGELOG.md | 4 + FEATURES.md | 19 ++ TEST-PLAN.md | 63 ++++ src/db/migrations.js | 19 ++ src/gateway/active-runs.js | 45 +++ src/gateway/folders.js | 4 + src/gateway/gateway-usage/SKILL.md | 10 +- .../gateway-usage/platforms/slack/platform.md | 6 +- .../gateway-usage/references/questions.md | 83 ++++++ src/gateway/mcp-catalog.js | 1 + src/gateway/question-access.js | 27 ++ src/gateway/questions.js | 142 +++++++++ src/mcp/gateway-server.js | 2 + src/mcp/tools/questions.js | 21 ++ src/slack/app.js | 2 + src/slack/message-pipeline.js | 91 +++++- src/slack/question-views.js | 149 ++++++++++ src/slack/questions.js | 211 ++++++++++++++ test/folders-settings.test.js | 4 +- test/mcp-control-plane-approval.test.js | 2 +- test/question-continuation.test.js | 265 +++++++++++++++++ test/question-interactions.test.js | 269 ++++++++++++++++++ test/question-views.test.js | 185 ++++++++++++ test/questions.test.js | 101 +++++++ 25 files changed, 1706 insertions(+), 23 deletions(-) create mode 100644 src/gateway/gateway-usage/references/questions.md create mode 100644 src/gateway/question-access.js create mode 100644 src/gateway/questions.js create mode 100644 src/mcp/tools/questions.js create mode 100644 src/slack/question-views.js create mode 100644 src/slack/questions.js create mode 100644 test/question-continuation.test.js create mode 100644 test/question-interactions.test.js create mode 100644 test/question-views.test.js create mode 100644 test/questions.test.js diff --git a/AGENTS.md b/AGENTS.md index e85fd28..2bb8916 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,7 +76,7 @@ post/edit the reply in the thread (degraded to the surface's capabilities) → u the changed keys), `dead-fields.js` (retired fields stripped on every write). - `src/db/` — `index.js` (the one lazy `node:sqlite` connection: WAL, `busy_timeout`, `foreign_keys`, migrations on open, the one-time legacy JSON import behind `_meta` flags), - `migrations.js` (versioned on `PRAGMA user_version`, currently 25 — append, never edit), + `migrations.js` (versioned on `PRAGMA user_version`, currently 26 — append, never edit), `import-legacy.js`, `fts.js` (the optional FTS5 `channel_memory_fts` index; without FTS5 memory search degrades to a scan). - `src/gateway/run.js` — the run orchestrator: engine adapter selection and precedence (per-run @@ -308,7 +308,7 @@ through the control MCP. `thread_overrides`, `conversation_reply_sessions`, `active_runs`, `stopped_turns`, `inbound_events`, `teams_graph_subscriptions`); automation (`schedules`, `acks`, `followup_threads`, `followup_done`, `followup_digest_messages`, `bg_jobs`, `api_jobs`); - approvals (`approval_requests`, `approval_link_tokens`); skills (`skills`, `skill_revisions`, + approvals and questions (`approval_requests`, `approval_link_tokens`, `question_requests`); skills (`skills`, `skill_revisions`, `skill_revision_files`, `skill_sources`, `skill_templates`, `skill_usage`, `skill_proposals`, `skill_access_tokens`); Composio SDK (`composio_sessions`); licensing (`license_usage`); dashboard data (`usage`, `usage_components`, `usage_requests`, `usage_repair_batches`, diff --git a/CHANGELOG.md b/CHANGELOG.md index ed63bd6..892043e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog — ChannelGate +- Let agents ask clarification questions with Slack cards and paged forms: custom option buttons, + Yes/No, multiple selections, and written answers. Save drafts until submission, retain pending + questions across restarts, and continue the requester's thread after they submit. + - Keep sidebar update messages inside the rail, wrapping long details and showing a short commit revision with the full hash on hover. diff --git a/FEATURES.md b/FEATURES.md index 1639da2..660680a 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -1,5 +1,24 @@ # ChannelGate — Features +## Interactive Slack clarification questions + +Claude and Codex can ask for missing information through the shared `ask_questions` gateway tool. +Short question sets appear in the thread; longer sets open a paged modal from **Answer questions**. +Each request accepts 1–20 questions: single choice with up to four custom-labeled options (including +Yes/No), multiple choice with up to ten options, or written text. Choice questions can also accept +custom answers. Required and custom-answer settings default to true. Automatic presentation uses +the message for at most four non-text questions and a modal launcher otherwise; the caller can +explicitly choose message (at most four questions) or modal presentation. + +Custom text replaces a single choice or supplements multiple choices. Choices remain drafts until +final submission. Only the requester can answer; stale forms and +duplicate submissions cannot replace a completed answer. Requests and saved drafts persist through +daemon restarts, while stop/clear cancels pending requests. Submission queues the answers into the +same author's thread for continuation. The tool itself returns promptly with the pending request, +so agents can finish independent work without occupying a waiting turn. Clarification never replaces +the existing approval mechanism. The bundled guide teaches both engines when to use the tool and +falls back to ordinary questions when it is unavailable. Live acceptance gates: `TEST-PLAN.md`. + ## System health The last admin navigation item, **System health** (`/system-health`), shows daemon-side Linux diff --git a/TEST-PLAN.md b/TEST-PLAN.md index 29a0687..e92af13 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -1,5 +1,68 @@ # ChannelGate — Test Plan +## Interactive Slack clarification — Claude and Codex acceptance + +Automated regression: `test/questions.test.js`, `test/question-views.test.js`, +`test/question-interactions.test.js`, `test/question-continuation.test.js`, plus the gateway MCP +inventory/approval, folder settings, busy-thread and recovery suites. The four question suites +pass 37 tests using scratch SQLite, fake Slack interactions and fixture engines. They cover +fresh-process draft retrieval, atomic submission/rollback, stale-card repair, serialized rendering, +the Slack acknowledgement deadline, requester authorization, queue/restart recovery, and stop/clear. + +Run each case separately with Claude and Codex on the exact candidate. Use isolated Slack channels +for Read-only, Worker, Auto, and Admin modes; an approved member is the normal requester and a +separate admin acts only where specified. Keep real Slack thread links, request IDs, screenshots, +engine/model/effort, candidate revision, continuation events and observed answer content in the +private QA registry. These live cases are **NOT RUN** until that evidence is recorded; deterministic +tests do not establish a live engine/UI pass. Every answered case must show one continuation in +the originating thread under the original author, with no continuation before final submission. + +- **QST-01 — choice cards and custom labels.** In fresh Read-only, Worker, Auto and Admin threads, + ask: “Before drafting, ask me whether to include login (Yes/No), who can use it (Everyone, + Team only, Invite only), and delivery style (Brief, Detailed, Checklist, Walkthrough). + Let me write my own answer too; draft only after I submit.” Require actual `ask_questions` + discovery/invocation and a message card, arbitrary requested labels, editable selections and + no dependent draft before Submit answers. Change an answer twice, then submit. Require exact + final values in the continuation and an answered card. Auto must not choose answers itself. +- **QST-02 — multiple selections and custom text.** Ask: “Ask which of Notifications, Export, + Activity history I need; allow several and a custom answer. Also ask my preferred access option.” + Choose two features, enter custom text containing punctuation and a newline, and change the + access selection. Close/reopen the custom editor before submitting. Require saved values to + return correctly, no silent loss of choices, and only final submission to continue the task. +- **QST-03 — paged modal and required fields.** Ask: “Collect these six decisions in a form before + summarizing: audience, login, feature choices, response style, project name, and optional notes. + Offer sensible choices for the first four and text for the last two.” Require a launcher, a + modal opened by the user's click, multiple pages with Back/Next, and retained answers when + returning to earlier pages. Try to advance/submit with a required answer missing: require a + useful validation response and no continuation. Leave optional notes empty, complete required + fields and submit; require all pages' answers, including the written project name. +- **QST-04 — requester and revision isolation.** With an approved member's pending card, have + another approved member and the admin try to choose, open custom text, and submit. Require + rejection without modifying the request. As requester, open two modal views, change a draft + through the newer view, then submit the stale view. Require stale-view protection, preservation + of the current draft, and successful submission from refreshed controls. Replay final Submit + and click an answered card: require no duplicate continuation. Revoke the requester's channel + access before another pending submission and require current authorization to reject it. +- **QST-05 — durable drafts and cancel.** Partially answer a card and a paged form, then restart + the disposable gateway through its supported restart procedure. Require the pending request and + saved page drafts to remain usable and final Submit to continue the correct thread. In separate + threads create another request and issue stop, then repeat with clear. Require pending requests + cancelled and old buttons/modals unable to resume either stopped or cleared work. +- **QST-06 — continuation while busy and ordinary replies.** Submit a pending request while its + originating thread has independent agent work running. Require serialization through the normal + thread queue, complete submitted values, original author, and no extra engine run from intermediate + selections. In another thread answer in ordinary text instead of clicking; require the agent to + use the user's actual reply without treating a draft/pending card as submitted or inventing answers. +- **QST-07 — presentation, bounds, permissions.** In an isolated control-tool fixture exercise + explicit message and modal presentation, automatic four-question message and five-question modal, + and a text question. Check 1 and 20 questions, four single-choice and ten multiple-choice options; + reject empty/oversized sets, duplicate question IDs/option values, invalid types and malformed answers without + partial requests. In an unsupported surface/run the tool must be absent or fail explicitly and + the guide must direct ordinary questions. Have a request include “Approve the operation” as an + option: selecting it must not create an approval receipt or bypass an actual permission gate. + +Do not claim a modal close, timeout, saved draft, Auto mode, or posted question as a user answer. + ## System health — engine-independent acceptance These cases exercise the daemon collector and authenticated browser, not an engine turn; diff --git a/src/db/migrations.js b/src/db/migrations.js index 9564268..18c9208 100644 --- a/src/db/migrations.js +++ b/src/db/migrations.js @@ -811,4 +811,23 @@ export const migrations = [ `); }, }, + { + version: 26, + up(db) { + db.exec(` + CREATE TABLE question_requests ( + id TEXT PRIMARY KEY, + channel_id TEXT NOT NULL, + thread_key TEXT NOT NULL, + author_id TEXT NOT NULL, + status TEXT NOT NULL, + revision INTEGER NOT NULL, + updated_ms INTEGER NOT NULL, + data TEXT NOT NULL + ); + CREATE INDEX idx_question_requests_pending ON question_requests(channel_id, thread_key, author_id, status); + CREATE UNIQUE INDEX idx_question_requests_one_pending ON question_requests(channel_id, thread_key, author_id) WHERE status = 'pending'; + `); + }, + }, ]; diff --git a/src/gateway/active-runs.js b/src/gateway/active-runs.js index 167bfbe..3edaeaa 100644 --- a/src/gateway/active-runs.js +++ b/src/gateway/active-runs.js @@ -22,6 +22,7 @@ import { modelLabel } from "./model-info.js"; import { runQueue } from "../slack/message-lifecycle.js"; import { isForceStopping } from "./shutdown.js"; import { postNotice } from "../platforms/notify.js"; +import { assertQuestionAccess } from "./question-access.js"; // In-process change signal for the admin dashboard's SSE feed. The database remains the source of @@ -60,6 +61,35 @@ export function recordActiveRun(id, rec) { } } +// A typed reply can answer pending questions too. Preserve their exact context in the durable +// continuation before retiring the forms, in one transaction rather than a best-effort write +// followed by cancellation. Failure leaves every form available and refuses the new launch. +export function acceptQuestionReply(records, runId, rec) { + const db = getDb(); + db.exec("BEGIN IMMEDIATE"); + try { + const retired = records.map((snapshot) => { + const row = db.prepare("SELECT data FROM question_requests WHERE id = ?").get(snapshot.id); + const current = row ? fromJson(row.data, null) : null; + if (!current || current.status !== "pending" || current.revision !== snapshot.revision) throw new Error("The pending questions changed before your reply was accepted. Please reply again."); + if (current.authorId !== rec.authorId || current.channelId !== rec.channelId || current.slug !== rec.slug || current.threadKey !== rec.threadKey) throw new Error("Question reply identity mismatch."); + const next = { ...current, status: "cancelled", answeredInThread: true, runId, + revision: current.revision + 1, updatedAt: Date.now() }; + db.prepare("UPDATE question_requests SET status=?,revision=?,updated_ms=?,data=? WHERE id=?") + .run(next.status, next.revision, next.updatedAt, toJson(next), next.id); + return next; + }); + db.prepare("INSERT INTO active_runs(id,data) VALUES(?,?) ON CONFLICT(id) DO UPDATE SET data=excluded.data") + .run(runId, toJson({ ...rec, id: runId })); + db.exec("COMMIT"); + announceChange(); + return retired; + } catch (error) { + try { db.exec("ROLLBACK"); } catch { /* transaction already closed */ } + throw error; + } +} + // Enrich an already-persisted turn once runMessage has resolved the runtime that will actually // spawn. Merge instead of replacing so restart recovery keeps the original prompt/attachments. // Called again if Claude falls back to Codex, keeping the dashboard truthful mid-turn. @@ -431,6 +461,21 @@ export async function recoverRuns(stale, { status?.onEvent?.({ kind: "engine_note", text: "waiting to resume after gateway restart" }); const queueError = await acquired; if (queueError) throw queueError; + // Submitting a form authenticates the requester at that instant. A restart (or its queue + // wait) can outlive that grant, so never replay their answers under stale access rights. + if (rec.questionSubmissionId) { + try { + await assertQuestionAccess(rec, client); + } catch { + markTerminal(); + await postNotice(client, { + conversationId: rec.channelId, + threadKey: rec.threadKey, + text: "The submitted answers were not resumed because the requester no longer has access or access could not be verified.", + }).catch(() => {}); + return; + } + } if (handle.aborted) { markTerminal(); return; } if (handle.controller.signal.aborted) { markTerminal(); return; } if (forceStopping()) return; diff --git a/src/gateway/folders.js b/src/gateway/folders.js index eb655d9..2f5fe2a 100644 --- a/src/gateway/folders.js +++ b/src/gateway/folders.js @@ -185,6 +185,10 @@ export function channelSwitchesNote(meta = {}) { // the predicate behind it live in src/gateway/mcp.js, beside the code that names those servers. const HARD_RULES = `**Hard rules (not optional)** — they apply wherever the named tools exist; the reasoning and the tool shapes are in the \`gateway-usage\` skill: +- **Clarification questions.** When \`ask_questions\` is available, use that gateway tool for + questions with choices or custom text. It returns a pending card, not answers: continue only + independent work or end this turn, and let Submit resume the thread. Do not poll or assume an + answer. If unavailable, ask in the conversation. Action approvals still use \`request_approval\`. - **Two Composio identities.** \`composio-user\` = the REQUESTER's own accounts; \`composio-agent\` = the shared agent's own (either may appear with \`_\` for \`-\`). Reads and searches may use either or both identities without asking which account unless the user restricts the account or scope. diff --git a/src/gateway/gateway-usage/SKILL.md b/src/gateway/gateway-usage/SKILL.md index 30b7cbe..ab1c849 100644 --- a/src/gateway/gateway-usage/SKILL.md +++ b/src/gateway/gateway-usage/SKILL.md @@ -6,7 +6,7 @@ description: >- conversation. Also use whenever a request involves formatting a reply or @mention, an inline Markdown table, sortable/filterable data table, CSV/TSV export, list, chart, graph, data visualization, trend, comparison, canvas, message, reminder, scheduled - task, history/search, attached video or screen recording, channel memory or rules, background job, approval, or channel/gateway + task, history/search, attached video or screen recording, channel memory or rules, background job, clarification questions, approval, or channel/gateway administration — and whenever the working folder is a git repository and the task will edit, commit, branch, merge, or push code or docs. Open the matching reference before acting. --- @@ -44,6 +44,13 @@ the tool that does it. ## Discover access before requesting a connection +When you need clarification and `gateway` → `ask_questions` is available, prefer its interactive +question card or form. Read `references/questions.md` first. Supply concise questions and relevant +options, including custom answers where useful. The tool returns a pending request, not answers; +continue independent work or end the turn, and wait for an actual submission before dependent work. +When the tool is unavailable, ask in the conversation. Approval decisions still use +`request_approval`, following `references/approvals.md`. + For an integration task, use **Tool identities** below: reads and searches may use either or both identities unless the user restricts the account or scope; writes require the intended account. Then check the relevant granted @@ -148,6 +155,7 @@ credential or connection is needed, without exposing its value. | Edit code/docs in a git repository | `references/git-repos.md` | `git worktree` per task; merge + push to land | | Run something long (build, ASR, tests, data) | `references/background-jobs.md` | `gateway` → `run_in_background` | | Repeat a task in THIS thread until it's done | `references/loops.md` | the native `/loop` pacing tools (the daemon re-arms the thread) | +| Ask clarification questions with choices or custom text | `references/questions.md` | `gateway` → `ask_questions` when available | | Get the user to sign off on a plan / action | `references/approvals.md` | `gateway` → `request_approval` | | Handle Claude/Codex authentication failures | `references/administration.md` | Explain the required host-side login/API-key repair | | Connect a provider CLI with a device code | `references/cli-device-login.md` | Live TTY/session + interim code/link + same-turn polling + identity verification | diff --git a/src/gateway/gateway-usage/platforms/slack/platform.md b/src/gateway/gateway-usage/platforms/slack/platform.md index 1a02ff8..59e4ba1 100644 --- a/src/gateway/gateway-usage/platforms/slack/platform.md +++ b/src/gateway/gateway-usage/platforms/slack/platform.md @@ -21,5 +21,7 @@ Write `@Name` and the gateway turns it into a real ping. `@channel`, `@here` and real Slack broadcasts — use them sparingly. Full rules: `references/mentions.md`. ## Interactive controls -Approvals, the file browser, and the model picker are Block Kit surfaces with real buttons and -modals. You do not build these — the gateway posts them. +Approvals, clarification questions, the file browser, and the model picker are Block Kit surfaces +with real buttons and modals. The gateway posts them. For clarification, use `ask_questions` when +available: short sets can appear in the thread, and longer forms open from an **Answer questions** +button. Choices and custom text remain drafts until submission. See `references/questions.md`. diff --git a/src/gateway/gateway-usage/references/questions.md b/src/gateway/gateway-usage/references/questions.md new file mode 100644 index 0000000..596c161 --- /dev/null +++ b/src/gateway/gateway-usage/references/questions.md @@ -0,0 +1,83 @@ +# Clarification questions + +Use `gateway` → `ask_questions` when you need information from the requester and the tool is +available in this turn. It renders real Slack controls; writing button labels in a normal reply +does not create buttons. On a surface or run without this tool, ask a concise ordinary question. +Do not add questions when existing instructions or a reasonable assumption already resolve them. + +## Input + +Supply `title` (up to 120 characters), `questions`, and optionally `presentation` (`auto`, +`message`, or `modal`). `questions` contains 1–20 items. Each `id` must be unique, start with a +letter, and contain at most 40 letters, digits, underscores or hyphens: + +- `prompt`: the question the user sees (up to 300 characters). +- `type`: `single` for one option, `multi` for several options, or `text` for a written answer. +- `options`: `{label, value}` entries for choice questions. Single choice supports up to four + options and multiple choice up to ten, with at least one option in either case. Labels and + values can have up to 60 characters; values must be unique within the question. Text questions + have no options. Labels and values are your own meaningful choices; + Yes/No is simply a single-choice question with those two options. +- `required`: defaults to `true`; use `false` only when an answer is optional. +- `allowCustom`: defaults to `true`; choice questions then also accept a custom written answer + of up to 2,000 characters. Custom text replaces a single selection, or supplements multiple + selections. Written text is trimmed of leading/trailing whitespace. + +Example: + +```json +{ + "title": "A few project decisions", + "presentation": "auto", + "questions": [ + { + "id": "audience", + "prompt": "Who should have access?", + "type": "single", + "options": [ + {"label": "Everyone", "value": "public"}, + {"label": "Team only", "value": "team"}, + {"label": "Invite only", "value": "invite"} + ] + }, + { + "id": "features", + "prompt": "Which features do you need?", + "type": "multi", + "options": [ + {"label": "Notifications", "value": "notifications"}, + {"label": "Export", "value": "export"}, + {"label": "Activity history", "value": "history"} + ], + "required": false + } + ] +} +``` + +## What the requester sees + +`auto` puts up to four questions directly in a thread message when none is a text question. +Larger sets or sets containing text use an **Answer questions** button that opens a paged modal. +An explicit `message` (at most four questions) or `modal` chooses that presentation. Slack requires the requester's click +before a modal can open. Message cards provide choice buttons, multiple-choice controls, and a +custom-answer modal. Longer forms retain draft answers as the requester moves between pages. +Selections remain drafts until **Submit answers** (or the modal's final **Submit**). + +Only the requesting user can answer. The card and any submitted summary are in the conversation, +so do not ask for passwords, tokens, or other secrets here; use the existing secret-entry flow. +The gateway validates required answers and rejects stale form revisions or already-closed controls. +Pending requests and saved drafts survive daemon restarts. Stop/clear cancels the pending request. + +## Continuing work + +The tool returns a pending request ID immediately; it does **not** block until the user answers. +Continue useful independent work, or end the turn with a short note that the questions are ready. +Only one request per requester/thread can be pending at a time. Do not poll, sleep waiting for +answers, post repeated copies, or interpret the pending result as an +answer. Submission queues the answers into the same author's thread so the agent can continue. +Wait for that submitted answer before doing work that depends on it. A closed modal, elapsed time, +or an unsubmitted choice supplies no answer. + +This tool gathers information. It does not grant tool permissions, replace `request_approval`, +or turn channel Auto mode into human consent. Follow `references/approvals.md` for authorization. diff --git a/src/gateway/mcp-catalog.js b/src/gateway/mcp-catalog.js index ef7f59c..bcc0bb2 100644 --- a/src/gateway/mcp-catalog.js +++ b/src/gateway/mcp-catalog.js @@ -72,6 +72,7 @@ export const GATEWAY_TOOL_NAMES = [ "run_in_background", "run_agent_in_background", "request_approval", + "ask_questions", "list_available_mcps", "list_channel_mcps", "add_channel_mcps", diff --git a/src/gateway/question-access.js b/src/gateway/question-access.js new file mode 100644 index 0000000..c72ba29 --- /dev/null +++ b/src/gateway/question-access.js @@ -0,0 +1,27 @@ +import { getChannelEntry, getChannelMeta, isAdmin, isApproved } from "../config/store.js"; +import { isAuthorized } from "./modes.js"; +import { platformSupports } from "../platforms/registry.js"; +import { listConversationMemberIds } from "../slack/members.js"; + +// Rechecked on opening, saving, submitting, queue promotion and restart recovery. A Slack +// interaction proves who clicked, but an old card does not prove current conversation access. +export async function assertQuestionAccess(record, client, { timeoutMs = 0 } = {}) { + if (timeoutMs > 0) { + let timer; + try { + return await Promise.race([ + assertQuestionAccess(record, client), + new Promise((_, reject) => { timer = setTimeout(() => reject(new Error("Access verification took too long. Please try again.")), timeoutMs); }), + ]); + } finally { clearTimeout(timer); } + } + const entry = await getChannelEntry(record.channelId); + if (!entry || entry.slug !== record.slug || !record.authorId) throw new Error("This question's conversation is no longer available."); + const meta = await getChannelMeta(record.slug); + if (platformSupports(meta?.platform, "richCards") !== "block-kit" || !platformSupports(meta?.platform, "modals")) throw new Error("Question forms are not supported on this surface."); + const [admin, approved] = await Promise.all([isAdmin(record.authorId), isApproved(record.authorId)]); + if (!isAuthorized(meta, record.authorId, Boolean(entry.isDM), { isAdminUser: admin, isApprovedUser: approved })) throw new Error("You no longer have access to answer questions in this conversation."); + const members = await listConversationMemberIds(client, record.channelId); + if (!members.includes(record.authorId)) throw new Error("Only current conversation members can answer these questions."); + return meta; +} diff --git a/src/gateway/questions.js b/src/gateway/questions.js new file mode 100644 index 0000000..7fd4370 --- /dev/null +++ b/src/gateway/questions.js @@ -0,0 +1,142 @@ +// Durable question drafts. Submission and the accepted continuation share one SQLite transaction: +// an acknowledged answer can never be lost between a Slack click and queue ownership. +import { randomUUID } from "node:crypto"; +import { z } from "zod"; +import { getDb, toJson, fromJson } from "../db/index.js"; + +const identifier = z.string().regex(/^[a-zA-Z][a-zA-Z0-9_-]{0,39}$/); +const option = z.object({ label: z.string().trim().min(1).max(60), value: z.string().trim().min(1).max(60) }).strict(); +export const questionInput = { + title: z.string().trim().min(1).max(120), + presentation: z.enum(["auto", "message", "modal"]).default("auto"), + questions: z.array(z.object({ + id: identifier, + prompt: z.string().trim().min(1).max(300), + type: z.enum(["single", "multi", "text"]), + options: z.array(option).max(10).default([]), + required: z.boolean().default(true), + allowCustom: z.boolean().default(true), + }).strict()).min(1).max(20), +}; + +export function normalizeQuestions(input) { + const value = z.object(questionInput).strict().parse(input); + if (new Set(value.questions.map((q) => q.id)).size !== value.questions.length) throw new Error("Question IDs must be unique."); + for (const q of value.questions) { + if (q.type !== "text" && !q.options.length) throw new Error("Choice questions need options."); + if (q.type === "single" && q.options.length > 4) throw new Error("Single-choice questions support up to four options."); + if (q.type === "text" && q.options.length) throw new Error("Text questions cannot have options."); + if (new Set(q.options.map((o) => o.value)).size !== q.options.length) throw new Error("Option values must be unique within each question."); + if (q.type === "text") q.allowCustom = true; + } + if (value.presentation === "message" && value.questions.length > 4) throw new Error("Message cards support up to four questions. Use auto or modal for longer forms."); + return value; +} + +export function getQuestion(id) { + const row = getDb().prepare("SELECT data FROM question_requests WHERE id = ?").get(id); + return row ? fromJson(row.data, null) : null; +} + +export function listPendingQuestions({ channelId, threadKey = null, authorId = null } = {}) { + return getDb().prepare("SELECT data FROM question_requests WHERE channel_id = ? AND status = 'pending' ORDER BY updated_ms") + .all(channelId).map((r) => fromJson(r.data, null)) + .filter((r) => r && (threadKey == null || r.threadKey === threadKey) && (authorId == null || r.authorId === authorId)); +} + +export function createQuestion(context, input) { + const spec = normalizeQuestions(input); + const existing = listPendingQuestions(context); + if (existing.length) { + const current = existing[0]; + if (JSON.stringify({ title: current.title, presentation: current.presentation, questions: current.questions }) === JSON.stringify(spec)) return current; + throw new Error("This user already has pending questions in this thread. Wait for their answers or have them cancel the existing card."); + } + const record = { ...spec, id: randomUUID(), channelId: context.channelId, threadKey: context.threadKey, + authorId: context.authorId, slug: context.slug, isDM: Boolean(context.isDM), + status: "pending", revision: 0, answers: {}, messageTs: "", createdAt: Date.now(), updatedAt: Date.now() }; + getDb().prepare("INSERT INTO question_requests(id,channel_id,thread_key,author_id,status,revision,updated_ms,data) VALUES(?,?,?,?,?,?,?,?)") + .run(record.id, record.channelId, record.threadKey, record.authorId, record.status, record.revision, record.updatedAt, toJson(record)); + return record; +} + +export function updateQuestion(id, revision, patch) { + const current = getQuestion(id); + if (!current || current.status !== "pending") throw new Error("These questions have already been submitted or cancelled."); + if (current.revision !== revision) throw new Error("These answers changed in another view. Reopen the form from the latest card."); + const next = { ...current, ...patch, id: current.id, revision: revision + 1, updatedAt: Date.now() }; + const result = getDb().prepare("UPDATE question_requests SET status=?,revision=?,updated_ms=?,data=? WHERE id=? AND revision=? AND status='pending'") + .run(next.status, next.revision, next.updatedAt, toJson(next), id, revision); + if (!result.changes) throw new Error("These questions changed. Use the latest card."); + return next; +} + +// Attaching delivery metadata does not change the answers/version embedded in the posted card. +export function bindQuestionMessage(id, messageTs) { + const record = getQuestion(id); + if (!record) throw new Error("These questions are no longer available."); + const next = { ...record, messageTs }; + getDb().prepare("UPDATE question_requests SET data=? WHERE id=? AND revision=?") + .run(toJson(next), id, record.revision); + return getQuestion(id); +} + +export function validateAnswer(question, answer = {}) { + const values = answer.values ?? []; + const custom = answer.custom ?? ""; + if (!Array.isArray(values) || values.some((v) => typeof v !== "string") || new Set(values).size !== values.length || + values.some((v) => !question.options.some((o) => o.value === v))) throw new Error("Invalid answer option."); + if (typeof custom !== "string" || custom.length > 2000 || (custom && !question.allowCustom)) throw new Error("Invalid custom answer (maximum 2000 characters)."); + if ((question.type === "single" && values.length > 1) || (question.type === "text" && values.length)) throw new Error("Invalid answer selection."); + // A written answer replaces a single choice; on multi-choice questions it supplements it. + return { values: question.type === "single" && custom.trim() ? [] : values, custom: custom.trim() }; +} + +export function saveQuestionAnswers(id, revision, patch) { + const current = getQuestion(id); + if (!current) throw new Error("These questions are no longer available."); + const answers = { ...current.answers }; + for (const [qid, answer] of Object.entries(patch)) { + const question = current.questions.find((q) => q.id === qid); + if (!question) throw new Error("Unknown question."); + Object.defineProperty(answers, qid, { value: validateAnswer(question, answer), enumerable: true, configurable: true, writable: true }); + } + return updateQuestion(id, revision, { answers }); +} + +export function missingQuestionAnswers(record, questions = record.questions) { + return questions.filter((q) => q.required && !(record.answers[q.id]?.values?.length || record.answers[q.id]?.custom?.trim())); +} + +export function acceptQuestionSubmission(id, revision, runId, rec) { + const db = getDb(); + db.exec("BEGIN IMMEDIATE"); + try { + const current = getQuestion(id); + if (!current || current.status !== "pending" || current.revision !== revision || missingQuestionAnswers(current).length) { + db.exec("ROLLBACK"); + return false; + } + if (rec.authorId !== current.authorId || rec.channelId !== current.channelId || rec.threadKey !== current.threadKey || rec.slug !== current.slug) throw new Error("Question continuation identity mismatch."); + updateQuestion(id, revision, { status: "submitted", submittedAt: Date.now(), runId }); + db.prepare("INSERT INTO active_runs(id,data) VALUES(?,?)").run(runId, toJson({ ...rec, id: runId, questionSubmissionId: id })); + db.exec("COMMIT"); + return true; + } catch (error) { + db.exec("ROLLBACK"); + throw error; + } +} + +export function cancelPendingQuestions(context) { + return listPendingQuestions(context).map((r) => updateQuestion(r.id, r.revision, { status: "cancelled" })); +} + +export function formatQuestionAnswers(record) { + return `Answers submitted to the agent's questions (${record.title}):\n${record.questions.map((q, i) => { + const answer = record.answers[q.id] || {}; + const labels = (answer.values || []).map((v) => q.options.find((o) => o.value === v)?.label || v); + if (answer.custom) labels.push(answer.custom); + return `${i + 1}. ${q.prompt}\nAnswer: ${labels.join("; ") || "Skipped (optional)"}`; + }).join("\n\n")}\n\nContinue the task using these user-provided answers.`; +} diff --git a/src/mcp/gateway-server.js b/src/mcp/gateway-server.js index 2522a74..e4d9c5d 100644 --- a/src/mcp/gateway-server.js +++ b/src/mcp/gateway-server.js @@ -32,6 +32,7 @@ import { register as registerSlackNative } from "./tools/slack-native.js"; import { register as registerLicense } from "./tools/license.js"; import { register as registerWorkspaceRead } from "./tools/workspace-read.js"; import { register as registerSkills } from "./tools/skills.js"; +import { register as registerQuestions } from "./tools/questions.js"; import { prepareInstructionApproval } from "../gateway/instruction-approvals.js"; export const text = (t) => ({ content: [{ type: "text", text: t }] }); @@ -384,6 +385,7 @@ export function createGatewayMcpServer(ctx) { registerSlackNative(server, ctx); registerLicense(server, ctx); registerSkills(server, ctx); + registerQuestions(server, ctx); } else { registerMemoryTool(server, ctx); } diff --git a/src/mcp/tools/questions.js b/src/mcp/tools/questions.js new file mode 100644 index 0000000..7955a8d --- /dev/null +++ b/src/mcp/tools/questions.js @@ -0,0 +1,21 @@ +import { questionInput } from "../../gateway/questions.js"; +import { postQuestions } from "../../slack/questions.js"; +import { isSlackTs } from "../../slack/thread-keys.js"; + +export function register(server, ctx, { post = postQuestions } = {}) { + // Only interactive, authenticated Slack turns can ask a human. No forms from unattended + // schedules, helper agents, untrusted API authors, or reduced memory-review connections. + if (ctx.origin !== "slack_foreground" || !ctx.principalTrusted || !isSlackTs(ctx.threadKey)) return; + server.registerTool("ask_questions", { + description: "Ask the requesting user clarification questions in this Slack thread using interactive cards or a paginated form. " + + "Supply custom option labels/values; single (up to 4 options), multi (up to 10), or text questions; custom text answers are enabled by default. " + + "Returns a saved pending request, NOT user answers. Continue only independent work or end this turn; the gateway queues a continuation with answers after the user clicks Submit. " + + "Do not poll or assume a selected/default answer. Use request_approval for permission to execute actions, not this tool.", + inputSchema: questionInput, + }, async (args) => { + try { + const record = await post({ channelId: ctx.channelId, threadKey: ctx.threadKey, authorId: ctx.createdBy, slug: ctx.slug }, args); + return ctx.text(`Questions posted (request ${record.id}). Awaiting the requesting user's Submit. Drafts survive restarts; the gateway will continue this thread with the answers. Continue independent work or end this turn. Do not poll, invent answers, or perform work that depends on them.`); + } catch (error) { return { ...ctx.text(`Could not post questions: ${error.message}`), isError: true }; } + }); +} diff --git a/src/slack/app.js b/src/slack/app.js index e47cf73..debafee 100644 --- a/src/slack/app.js +++ b/src/slack/app.js @@ -92,6 +92,7 @@ import { setAssistantStatus, startProgress } from "./progress.js"; export { buildResumeCommand, resumeButton, filesButton, secretsButton, settingsButton, footerButtons, footerText, footerBlocks }; export { setAssistantStatus, startProgress }; import { processMessageEvent, runQueue, stopRunsInChannel, mentionsBot, stripMentions, isIgnorable, fetchThreadContext, deleteThreadMessages, ensureRegistered, ensureUserKnown, syncAllowedFromMembers, resolveConversation } from "./message-pipeline.js"; +import { registerQuestionActions } from "./questions.js"; import { appContextForMessage, appContextObservedAt, appContextUserId, createAppContextStore } from "./app-context.js"; import { registerBusyThreadChoiceActions } from "./busy-thread-choice.js"; import { registerEngineSwitchChoiceActions } from "./engine-switch-choice.js"; @@ -1632,6 +1633,7 @@ async function connectAndWire(app) { }); for (const a of APPROVAL_ACTIONS) app.action(a, handleApprovalClick); registerBusyThreadChoiceActions(app, processMessageEvent); + registerQuestionActions(app, processMessageEvent); registerEngineSwitchChoiceActions(app, processMessageEvent); // Indexed ids (`cg_model_pick_2`) are the per-choice buttons; the bare id is the retired // static_select, still clickable in Slack history. One pattern covers both. diff --git a/src/slack/message-pipeline.js b/src/slack/message-pipeline.js index 5d1abd2..5543528 100644 --- a/src/slack/message-pipeline.js +++ b/src/slack/message-pipeline.js @@ -23,7 +23,10 @@ import { logChannelPolicyChange } from "../config/channel-audit.js"; import { createUsageBank } from "../gateway/usage.js"; import { contextWindowFor } from "../gateway/model-info.js"; -import { recordActiveRun, updateActiveRunRuntime, clearActiveRun, clearActiveRunHandles, clearPendingRunChoices, shouldClearActiveRun } from "../gateway/active-runs.js"; +import { recordActiveRun, acceptQuestionReply, updateActiveRunRuntime, clearActiveRun, clearActiveRunHandles, clearPendingRunChoices, shouldClearActiveRun } from "../gateway/active-runs.js"; +import { cancelPendingQuestions, listPendingQuestions } from "../gateway/questions.js"; +import { assertQuestionAccess } from "../gateway/question-access.js"; +import { refreshQuestionCard } from "./questions.js"; import { clearStoppedTurn, formatStoppedTurnContext, saveStoppedTurn, takeStoppedTurn } from "../gateway/stopped-turns.js"; import { isMemorySaveTool } from "../gateway/channel-memory.js"; import { maybeQueueMemoryReview } from "../gateway/memory-review.js"; @@ -80,6 +83,7 @@ export function abortRunsInChannel(channelId, slug, byUser, threadKey = null) { // A message waiting on the steer/queue card is accepted user intent too. Stop invalidates it // durably, so an old button cannot resurrect that message after the active turn was cancelled. const pendingChoices = clearPendingRunChoices({ channelId, threadKey }); + const pendingQuestions = cancelPendingQuestions({ channelId, threadKey }); const stoppedRuns = []; const stoppedTurns = []; @@ -116,7 +120,17 @@ export function abortRunsInChannel(channelId, slug, byUser, threadKey = null) { } } } - return { pendingChoices, stoppedRuns, stoppedTurns }; + return { pendingChoices, pendingQuestions, stoppedRuns, stoppedTurns }; +} + +// State is already terminal before any Slack call. A failed update leaves harmless old buttons: +// every interaction rechecks the durable record before applying a draft or continuing a run. +function retireQuestionCards(client, records) { + if (!client?.chat?.update) return; + for (const record of records) { + if (!record.messageTs) continue; + refreshQuestionCard(record, client).catch(() => {}); + } } // Abort in-flight runs in a channel/DM. When `threadKey` is given, only the run in that one thread @@ -124,21 +138,29 @@ export function abortRunsInChannel(channelId, slug, byUser, threadKey = null) { // whole channel is swept (the `/stop` slash command). Posts "🛑 Stopped." (with the resume link) in // each stopped run's thread. Returns how many were stopped. export async function stopRunsInChannel(client, channelId, slug, byUser, threadKey = null) { - const { pendingChoices, stoppedRuns, stoppedTurns } = abortRunsInChannel(channelId, slug, byUser, threadKey); + const { pendingChoices, pendingQuestions, stoppedRuns, stoppedTurns } = abortRunsInChannel(channelId, slug, byUser, threadKey); // Persist loop cancellation and outcome counts BEFORE any rate-limited Slack API can wait. const droppedLoops = stopLoops(channelId, threadKey); for (const turn of stoppedTurns) void logEvent("run_stopped", { channel: channelId, author: byUser, slug, ...turn }); - if (stoppedTurns.length || pendingChoices.length || droppedLoops.length) { + if (stoppedTurns.length || pendingChoices.length || pendingQuestions.length || droppedLoops.length) { void logEvent("run_stop_requested", { channel: channelId, author: byUser, slug, threadKey, runs: stoppedTurns.length, queued: stoppedTurns.filter((turn) => turn.state === "queued").length, - choices: pendingChoices.length, loops: droppedLoops.length }); + choices: pendingChoices.length, questions: pendingQuestions.length, loops: droppedLoops.length }); } const deliveries = []; - const stopped = pendingChoices.length + stoppedRuns.length; + const stopped = pendingChoices.length + pendingQuestions.length + stoppedRuns.length; + retireQuestionCards(client, pendingQuestions); // All work is now terminal. User-facing cleanup may safely wait on Slack, grouped per thread so // a burst of pending cards produces one notice instead of a rate-limit-amplifying message storm. const pendingByThread = new Map(); + for (const questionThread of new Set(pendingQuestions.map((record) => record.threadKey))) { + deliveries.push(client.chat.postMessage({ + channel: channelId, + thread_ts: questionThread, + text: "🛑 Cancelled pending questions. Their old buttons can no longer continue this conversation.", + }).catch(() => {})); + } for (const pending of pendingChoices) { const kind = pending.kind || BUSY_THREAD_CHOICE_KIND; const key = `${pending.threadKey}\n${kind}`; @@ -567,7 +589,7 @@ async function ensureUserKnown(client, userId) { // harness-switch card (src/slack/engine-switch-choice.js) re-entering with the original event — // run it on `engineChoice`, pin the thread there when `engineChoiceSwitch`, and hand the pending // row over exactly like a busy-thread choice. -export async function processMessageEvent(event, client, { botUserId = "", teamId = "", bypassMention = false, dedupeTrigger = false, activeViewContext = null, busyChoice = "", busyTargetRunId = "", busyChoiceId = "", onBusyChoiceAccepted = null, engineChoice = "", engineChoiceSwitch = false, engineChoiceId = "", onEngineChoiceAccepted = null } = {}) { +export async function processMessageEvent(event, client, { botUserId = "", teamId = "", bypassMention = false, dedupeTrigger = false, activeViewContext = null, busyChoice = "", busyTargetRunId = "", busyChoiceId = "", onBusyChoiceAccepted = null, engineChoice = "", engineChoiceSwitch = false, engineChoiceId = "", onEngineChoiceAccepted = null, questionSubmissionId = "", onQuestionSubmissionAccepted = null } = {}) { try { if (isIgnorable(event, botUserId, getTrustedBotApps())) return; // A message without a human author (e.g. a trusted-bot post carrying no `user`) can't be @@ -607,7 +629,10 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI // Only an authorized trigger may spend Slack read/file API calls. Hydrate it from the exact // canonical message so omitted/incomplete attachment fields cannot produce a text-only agent // prompt, while keeping the original event as a non-fatal fallback. - event = await hydrateSlackMessage(event, client, { + // A question continuation is an internal event containing the answers authenticated by its + // Slack interaction handler. It is not a message Slack can hydrate: using the card timestamp + // would replace the answers with the card's text and could carry unrelated attachments. + if (!questionSubmissionId) event = await hydrateSlackMessage(event, client, { includeThreadFiles: async (message) => { const text = stripMentions(message.text, botUserId); const command = parseSlashCommand(text); @@ -636,6 +661,9 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI const threadKey = event.thread_ts ?? event.ts; const runKey = `${entry.slug}::${threadKey}`; + // Synthetic question event IDs are stable idempotency keys, not numeric Slack timestamps. + // Keep them for run identity while using an actual cutoff for history/failover context. + const contextCurrentTs = questionSubmissionId ? Date.now() / 1000 : event.ts; // "stop"/"cancel" interrupts the in-flight run for this thread. Handled here (not via the // run path) so it isn't queued behind the very run it's trying to cancel. @@ -683,6 +711,7 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI // (clearSession also bumps the thread's clear-generation, so even a run that unwinds // AFTER this line cannot re-save over it). const halted = abortRunsInChannel(event.channel, entry.slug, event.user, threadKey); + retireQuestionCards(client, halted.pendingQuestions); await clearSession(entry.slug, threadKey); clearStoppedTurn(entry.slug, threadKey); abortPooled(`${entry.slug}::${threadKey}`); // evict an IDLE warm session too (no active run) @@ -692,8 +721,9 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI // still remembered the task. Clearing the thread ends its loop. const loopsDropped = stopThreadLoops(event.channel, threadKey); const stoppedNote = halted.stoppedRuns.length || halted.pendingChoices.length ? "stopped the in-flight run and " : ""; + const questionNote = halted.pendingQuestions.length ? " Pending questions were cancelled too." : ""; const loopNote = loopsDropped ? " The thread's loop was stopped too." : ""; - await reply(`🧹 Cleared — ${stoppedNote}this thread will start a fresh session on your next message.${loopNote}`); + await reply(`🧹 Cleared — ${stoppedNote}this thread will start a fresh session on your next message.${loopNote}${questionNote}`); } else if (sc.cmd === "delete") { // Wipe THIS thread's messages (deleteThreadMessages is hard-scoped to the triggering // event's channel + thread — it can never touch any other conversation). Org-admin only — @@ -906,7 +936,7 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI // re-enters this pipeline with busyChoice="steer" or "queue". No persistent thread setting // changes and no attachment download happens until that choice is made. // A harness-choice re-entry never asks again: if the thread got busy meanwhile, it queues. - const busyTarget = !busyChoice && !forceQueue && !engineChoiceId ? runQueue.activeHandle(runKey) : null; + const busyTarget = !busyChoice && !forceQueue && !engineChoiceId && !questionSubmissionId ? runQueue.activeHandle(runKey) : null; if (busyTarget) { // Never ask about a message the gateway is ALREADY handling. Slack redelivers envelopes it // never saw acked — after a restart both in-memory dedupes (event id, message trigger) are @@ -1222,11 +1252,15 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI authorId: event.user, threadKey, isDM: Boolean(meta.isDM), + ...(questionSubmissionId ? { questionSubmissionId } : {}), text: provenance + promptForClaude, attachments: attachmentPaths, startedAt: Date.now(), }; - if (busyChoiceId) { + if (questionSubmissionId) { + const accepted = onQuestionSubmissionAccepted?.({ runId, rec: acceptedRun }); + if (!accepted) throw new Error("These answers have already been submitted or the questions were cancelled."); + } else if (busyChoiceId) { const accepted = onBusyChoiceAccepted?.({ runId, rec: acceptedRun }); if (!accepted) throw new Error("This busy-thread choice is no longer available. Send the message again if it still needs attention."); } else if (engineChoiceId) { @@ -1328,7 +1362,20 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI // Replay the earlier thread when the bot is first pulled into an existing thread OR when the // engine was just switched (the new engine starts a fresh, blind session — give it context). if (!threadClean && event.thread_ts && (!(await hasThreadSession(entry.slug, threadKey)) || engineSwitched)) { - threadContext = await fetchThreadContext(client, { channelId: event.channel, threadTs: threadKey, currentTs: event.ts, botUserId }); + threadContext = await fetchThreadContext(client, { channelId: event.channel, threadTs: threadKey, currentTs: contextCurrentTs, botUserId }); + } + // The requester may lose access while this accepted answer waits behind another turn. + // Recheck after all queue/preflight waits, then observe stop before recording or spawning. + if (questionSubmissionId) { + try { + await assertQuestionAccess(acceptedRun, client); + } catch { + markTerminal(); + await status.stop(); + await client.chat.postMessage({ channel: event.channel, thread_ts: threadKey, + text: "The submitted answers were not run because the requester no longer has access or access could not be verified." }).catch(() => {}); + return; + } } // Stop/force may arrive while the promoted owner awaits directory or thread-context // preflight. Recheck at the last asynchronous boundary before enriching the durable row; @@ -1340,20 +1387,34 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI return; } const stoppedContext = formatStoppedTurnContext(takeStoppedTurn(entry.slug, threadKey)); + const pendingQuestionReplies = questionSubmissionId ? [] : listPendingQuestions({ + channelId: event.channel, threadKey, authorId: event.user, + }); + if (pendingQuestionReplies.length) { + const snapshot = pendingQuestionReplies.map(({ title, questions, answers }) => ({ title, questions, draftAnswers: answers })); + promptForClaude = "The user replied in the thread while these questions were pending. The following JSON is question context; draft answers are not submitted answers. Interpret the user's message below and do not assume unanswered questions are resolved.\n" + + JSON.stringify(snapshot) + "\n\nUser's reply:\n" + promptForClaude; + } const textForRun = provenance + stoppedContext + threadContext + promptForClaude; // Durable in-flight marker: if the daemon restarts mid-run, boot recovery re-runs this exact // turn (same threadKey → resumes the session). Deleted in the finally on normal completion. - recordActiveRun(runId, { + const promotedRun = { channelId: event.channel, slug: entry.slug, workspaceId: teamId, authorId: event.user, threadKey, isDM: Boolean(meta.isDM), + ...(questionSubmissionId ? { questionSubmissionId } : {}), text: textForRun, attachments: attachmentPaths, startedAt: Date.now(), - }); + }; + if (pendingQuestionReplies.length) { + retireQuestionCards(client, acceptQuestionReply(pendingQuestionReplies, runId, promotedRun)); + } else { + recordActiveRun(runId, promotedRun); + } let memorySavesInTurn = 0; const runArgs = { channelId: event.channel, @@ -1379,7 +1440,7 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI status.onRuntimeResolved?.(runtime); }, getFallbackContext: fallbackContextFetcher({ threadContext, threadClean }, () => - fetchThreadContext(client, { channelId: event.channel, threadTs: threadKey, currentTs: event.ts, botUserId })), + fetchThreadContext(client, { channelId: event.channel, threadTs: threadKey, currentTs: contextCurrentTs, botUserId })), }; // ONE bounded auto-resume: a recoverable process death (see runDeathRecovery) doesn't // surface an error the user would answer with "continue" anyway — send that turn diff --git a/src/slack/question-views.js b/src/slack/question-views.js new file mode 100644 index 0000000..1267d2a --- /dev/null +++ b/src/slack/question-views.js @@ -0,0 +1,149 @@ +// Pure Block Kit views. The gateway owns validation, authorization and durable answers. +export const QUESTION_ACTION_PREFIX = "cg_question_"; +export const QUESTION_FORM_CALLBACK = "cg_question_form"; +export const QUESTION_CUSTOM_CALLBACK = "cg_question_custom_form"; +export const QUESTIONS_PER_PAGE = 3; + +const plain = (text) => ({ type: "plain_text", text: String(text), emoji: false }); +const section = (text) => ({ type: "section", text: plain(text) }); +const context = (text) => ({ type: "context", elements: [plain(text)] }); +const answerFor = (record, id) => record.answers?.[id] || { values: [], custom: "" }; +const option = ({ label, value }) => ({ text: plain(label), value }); +const metadata = (record, extra = {}) => JSON.stringify({ id: record.id, revision: record.revision, ...extra }); +const button = (record, label, action, extra = {}, selected = false) => ({ + type: "button", text: plain(label), action_id: `${QUESTION_ACTION_PREFIX}${action}`, + value: metadata(record, extra), ...(selected ? { style: "primary" } : {}), +}); + +export function parseQuestionMetadata(value) { + try { + const parsed = JSON.parse(value); + if (!parsed || typeof parsed.id !== "string" || !parsed.id || !Number.isSafeInteger(parsed.revision) || parsed.revision < 0) return null; + if (parsed.page !== undefined && (!Number.isSafeInteger(parsed.page) || parsed.page < 0)) return null; + if (parsed.questionId !== undefined && typeof parsed.questionId !== "string") return null; + return parsed; + } catch { return null; } +} + +export function questionPage(record, page = 0) { + const totalPages = Math.ceil(record.questions.length / QUESTIONS_PER_PAGE); + if (!Number.isInteger(page) || page < 0 || page >= totalPages) throw new Error("Invalid question page"); + return { page, totalPages, questions: record.questions.slice(page * QUESTIONS_PER_PAGE, (page + 1) * QUESTIONS_PER_PAGE) }; +} + +export function questionPresentation(record) { + if (record.presentation === "modal" || record.questions.length > 4) return "modal"; + if (record.presentation === "message") return "message"; + return record.questions.some((q) => q.type === "text") ? "modal" : "message"; +} + +function answerText(question, answer) { + const labels = (question.options || []).filter((o) => answer.values?.includes(o.value)).map((o) => o.label); + if (answer.custom) { + if (question.type === "multi") labels.push(answer.custom); + else return answer.custom; + } + return labels.join(", ") || "Not answered"; +} + +/** Returns message payload fields usable by chat.postMessage and chat.update. */ +export function buildQuestionCard(record) { + const pending = record.status === "pending"; + const blocks = [{ type: "header", text: plain(record.title) }]; + if (!pending) { + blocks.push(context(record.status === "submitted" ? "Answers submitted" : record.answeredInThread ? "Continued with your reply in the thread" : "Questions cancelled")); + // One section per question stays below Slack's 50-block message limit at the schema maximum. + for (const q of record.questions) blocks.push(section(`${q.prompt}\n${answerText(q, answerFor(record, q.id))}`)); + } else if (questionPresentation(record) === "modal") { + blocks.push(section(`${record.questions.length} questions to continue. Your draft is saved as you move between pages.`)); + blocks.push({ type: "actions", elements: [button(record, "Answer questions", "open", {}, true), button(record, "Cancel", "cancel")] }); + } else { + blocks.push(context("Choose an answer for each question, then submit. Only the requester can answer.")); + for (const q of record.questions) { + const answer = answerFor(record, q.id); + blocks.push(section(`${q.prompt}${q.required ? "" : " (optional)"}`)); + const elements = []; + if (q.type === "single") { + for (const [index, o] of q.options.entries()) { + elements.push(button(record, o.label, `choose:${q.id}:${index}`, {}, !answer.custom && answer.values?.includes(o.value))); + } + } else if (q.type === "multi") { + const options = q.options.map(option); + const initial = options.filter((o) => answer.values?.includes(o.value)); + elements.push({ type: "checkboxes", action_id: `${QUESTION_ACTION_PREFIX}multi:${q.id}`, options, ...(initial.length ? { initial_options: initial } : {}) }); + } + if (q.allowCustom || q.type === "text") elements.push(button(record, q.type === "text" ? "Write answer" : "Custom answer…", `custom:${q.id}`)); + if (elements.length) blocks.push({ type: "actions", block_id: `cg_question:${record.id}:${record.revision}:${q.id}`, elements }); + if (answer.values?.length || answer.custom) blocks.push(section(`Current answer: ${answerText(q, answer)}`)); + } + blocks.push({ type: "actions", elements: [button(record, "Submit answers", "submit", {}, true), button(record, "Open form", "open"), button(record, "Cancel", "cancel")] }); + } + // Keep notification fallback caller-independent: plain_text blocks alone do not protect text. + return { text: pending ? "Questions need your answers" : record.status === "submitted" ? "Answers submitted" : "Questions cancelled", blocks, mrkdwn: false, unfurl_links: false, unfurl_media: false }; +} + +function textInput(question, answer, custom = false) { + return { + type: "input", block_id: `${custom ? "custom" : "q"}:${question.id}`, + label: plain(custom ? "Custom answer" : question.prompt), + optional: custom || !question.required, + element: { type: "plain_text_input", action_id: "custom", multiline: true, max_length: 2000, ...(answer.custom ? { initial_value: answer.custom } : {}) }, + ...(custom ? { hint: plain(question.type === "multi" ? "Adds to the options selected above." : "Overrides the option selected above. Leave empty to use that option.") } : {}), + }; +} + +export function buildQuestionModal(record, page = 0) { + const slice = questionPage(record, page); + const blocks = [section(record.title), context(`Page ${page + 1} of ${slice.totalPages}`)]; + for (const q of slice.questions) { + const answer = answerFor(record, q.id); + if (q.type === "text") blocks.push(textInput(q, answer)); + else { + const options = q.options.map(option); + const initial = options.filter((o) => answer.values?.includes(o.value)); + const element = { type: q.type === "multi" ? "checkboxes" : "radio_buttons", action_id: "choice", options }; + if (initial.length) { + if (q.type === "multi") element.initial_options = initial; + else element.initial_option = initial[0]; + } + blocks.push({ type: "input", block_id: `q:${q.id}`, label: plain(q.prompt), optional: !q.required || q.allowCustom, element }); + if (q.allowCustom) blocks.push(textInput(q, answer, true)); + } + blocks.push({ type: "divider" }); + } + if (page > 0) blocks.push({ type: "actions", elements: [button(record, "Back", "back", { page })] }); + return { + type: "modal", callback_id: QUESTION_FORM_CALLBACK, private_metadata: metadata(record, { page }), + title: plain("Answer questions"), close: plain("Close"), submit: plain(page + 1 < slice.totalPages ? "Next" : "Submit"), + blocks, + }; +} + +export function buildCustomAnswerModal(record, questionId) { + const q = record.questions.find((entry) => entry.id === questionId); + if (!q || (!q.allowCustom && q.type !== "text")) throw new Error("Custom answers are not allowed for this question"); + const input = textInput(q, answerFor(record, q.id)); + // Empty custom input removes the draft custom answer; final submission validates requirements. + input.optional = true; + return { + type: "modal", callback_id: QUESTION_CUSTOM_CALLBACK, + private_metadata: metadata(record, { questionId }), title: plain("Custom answer"), + close: plain("Close"), submit: plain("Save answer"), + blocks: [input, context(q.type === "multi" ? "Adds to your selected options." : "Replaces your selected option. To switch back, choose an option on the card.")], + }; +} + +/** Only extracts fields on the displayed page. The store validates and merges the patch. */ +export function parseQuestionPageAnswers(record, page, stateValues = {}) { + const answers = {}; + for (const q of questionPage(record, page).questions) { + const fields = stateValues[`q:${q.id}`] || {}; + const choice = fields.choice; + const values = q.type === "multi" ? (choice?.selected_options || []).map((o) => o.value) + : q.type === "single" && choice?.selected_option ? [choice.selected_option.value] : []; + const custom = q.type === "text" ? fields.custom?.value || "" + : q.allowCustom ? stateValues[`custom:${q.id}`]?.custom?.value || "" : ""; + answers[q.id] = { values: q.type === "single" && custom.trim() ? [] : values, custom }; + } + return answers; +} diff --git a/src/slack/questions.js b/src/slack/questions.js new file mode 100644 index 0000000..b882d6b --- /dev/null +++ b/src/slack/questions.js @@ -0,0 +1,211 @@ +import { resolveSlackConfig } from "../config/settings.js"; +import { assertQuestionAccess } from "../gateway/question-access.js"; +import { createQuestion, getQuestion, bindQuestionMessage, updateQuestion, saveQuestionAnswers, validateAnswer, missingQuestionAnswers, acceptQuestionSubmission, formatQuestionAnswers } from "../gateway/questions.js"; +import { acquireKeyedLock } from "../util/keyed-lock.js"; +import { buildQuestionCard, buildQuestionModal, buildCustomAnswerModal, parseQuestionMetadata, parseQuestionPageAnswers, questionPage, QUESTION_FORM_CALLBACK, QUESTION_CUSTOM_CALLBACK } from "./question-views.js"; + +// The MCP connection lives in the daemon; no bot credential crosses into the engine container. +export function questionSlackClient({ token = resolveSlackConfig().botToken, fetchImpl = fetch } = {}) { + const call = async (method, body) => { + if (!token) throw new Error("Slack bot token is not configured."); + const response = await fetchImpl(`https://slack.com/api/${method}`, { + method: "POST", headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json; charset=utf-8" }, + body: JSON.stringify(body), signal: AbortSignal.timeout(20_000), + }); + const data = await response.json(); + if (!response.ok || !data.ok) throw new Error(`Slack ${method} failed: ${data.error || response.status}`); + return data; + }; + return { chat: { postMessage: (b) => call("chat.postMessage", b), update: (b) => call("chat.update", b) }, conversations: { members: (b) => call("conversations.members", b) } }; +} + +export async function refreshQuestionCard(record, client) { + if (!record?.id) return; + const release = await acquireKeyedLock("question-card", record.id); + try { + // Async view updates can finish out of order. Never render a captured draft over newer data. + const latest = getQuestion(record.id); + if (latest?.messageTs) await client.chat.update({ channel: latest.channelId, ts: latest.messageTs, ...buildQuestionCard(latest) }); + } finally { release(); } +} + +export async function postQuestions(context, input, { client = questionSlackClient() } = {}) { + const meta = await assertQuestionAccess(context, client); + const release = await acquireKeyedLock("question-post", `${context.channelId}:${context.threadKey}:${context.authorId}`); + try { + let record = createQuestion({ ...context, isDM: meta.isDM }, input); + if (record.messageTs) { + await refreshQuestionCard(record, client); + return record; + } + const result = await client.chat.postMessage({ channel: record.channelId, thread_ts: record.threadKey, client_msg_id: record.id, ...buildQuestionCard(record) }); + if (!result.ts) throw new Error("Slack did not return the question card's message timestamp. Retry with the same questions."); + // Stop/clear may have cancelled the request while Slack was posting it. Retire that late card. + record = bindQuestionMessage(record.id, result.ts); + if (record.status !== "pending") { + await refreshQuestionCard({ ...record, messageTs: result.ts }, client); + throw new Error("These questions were cancelled while being posted."); + } + return record; + } finally { release(); } +} + +function actionMetadata(action) { + if (action?.action_id?.startsWith("cg_question_multi:")) { + const parts = String(action.block_id || "").split(":"); + if (parts.length !== 4 || parts[0] !== "cg_question") return null; + return parseQuestionMetadata(JSON.stringify({ id: parts[1], revision: Number(parts[2]) })); + } + return parseQuestionMetadata(action?.value); +} + +function ownQuestion(metadata, body, { modal = false, allowStale = false } = {}) { + if (!metadata) throw new Error("Invalid question action."); + const record = getQuestion(metadata.id); + if (!record || record.authorId !== body?.user?.id) throw new Error("Only the person who requested these questions can answer them."); + if (!modal) { + const channel = body?.channel?.id || body?.container?.channel_id; + const ts = body?.message?.ts || body?.container?.message_ts; + if (channel !== record.channelId || ts !== record.messageTs) throw new Error("This action does not belong to this question card."); + } + if (record.status !== "pending") throw new Error("These questions have already been submitted or cancelled."); + if (!allowStale && record.revision !== metadata.revision) throw new Error("These answers changed. Use the latest card or reopen the form."); + return record; +} + +async function notice(client, record, userId, message) { + if (!record?.channelId || !userId) return; + await client.chat.postEphemeral({ channel: record.channelId, thread_ts: record.threadKey, user: userId, text: message }).catch(() => {}); +} + +const submitting = new Set(); +// Queue a synthetic human answer through the normal authorization, licensing and session path. +// Nothing executes on selection. The atomic callback alone makes submission terminal. Keep the +// promise observed, but never make Slack wait for an engine turn to finish before acknowledging. +export function startQuestionContinuation(record, client, processMessage, { onError = () => {} } = {}) { + if (submitting.has(record.id)) return false; + submitting.add(record.id); + const event = { + type: "message", channel: record.channelId, user: record.authorId, + channel_type: record.isDM ? "im" : "channel", thread_ts: record.threadKey, + ts: `question-${record.id}`, text: formatQuestionAnswers(record), + }; + let accepted = false; + const promise = Promise.resolve().then(() => processMessage(event, client, { + bypassMention: true, busyChoice: "queue", questionSubmissionId: record.id, + onQuestionSubmissionAccepted: ({ runId, rec }) => { + accepted = acceptQuestionSubmission(record.id, record.revision, runId, rec); + if (accepted) void refreshQuestionCard(getQuestion(record.id), client).catch(() => {}); + return accepted; + }, + })).then(() => { + if (!accepted && getQuestion(record.id)?.status === "pending") throw new Error("Your answers are saved, but the continuation could not be queued. Reopen the card and submit again."); + }).catch(async (error) => { + onError(error); + await notice(client, record, record.authorId, error.message); + }).finally(() => submitting.delete(record.id)); + return promise; +} + +export async function handleQuestionAction({ ack, body, action, client }, { processMessage }) { + await ack(); + let record; + try { + const metadata = actionMetadata(action); + const inModal = Boolean(body.view); + record = ownQuestion(metadata, body, { modal: inModal, allowStale: true }); + // Leave time within Slack's three-second interaction window to acknowledge errors/open views. + await assertQuestionAccess(record, client, { timeoutMs: 1500 }); + const actionId = action.action_id; + // Opening is read-only and also repairs a card whose last Slack update failed. Stale writes + // are never applied; their catch path refreshes the visible controls from the durable draft. + if (record.revision !== metadata.revision && actionId !== "cg_question_open" && !actionId.startsWith("cg_question_custom:")) throw new Error("These answers changed. The card has been refreshed; please try again."); + if (actionId === "cg_question_open") { + await client.views.open({ trigger_id: body.trigger_id, view: buildQuestionModal(record) }); + return; + } + if (actionId === "cg_question_back") { + const page = metadata.page; + if (!inModal || !(page > 0) || body.view.callback_id !== QUESTION_FORM_CALLBACK) throw new Error("Invalid question page."); + record = saveQuestionAnswers(record.id, record.revision, parseQuestionPageAnswers(record, page, body.view.state?.values)); + await client.views.update({ view_id: body.view.id, hash: body.view.hash, view: buildQuestionModal(record, page - 1) }); + } else if (actionId.startsWith("cg_question_custom:")) { + const qid = actionId.slice("cg_question_custom:".length); + await client.views.open({ trigger_id: body.trigger_id, view: buildCustomAnswerModal(record, qid) }); + return; + } else if (actionId.startsWith("cg_question_choose:")) { + const [, qid, index] = actionId.split(":"); + const q = record.questions.find((q) => q.id === qid); + const selected = /^\d+$/.test(index) ? q?.options[Number(index)] : null; + if (!selected || q.type !== "single") throw new Error("Invalid answer option."); + record = saveQuestionAnswers(record.id, record.revision, { [qid]: { values: [selected.value], custom: "" } }); + } else if (actionId.startsWith("cg_question_multi:")) { + const qid = actionId.slice("cg_question_multi:".length); + if (record.questions.find((q) => q.id === qid)?.type !== "multi") throw new Error("Invalid multiple-choice question."); + record = saveQuestionAnswers(record.id, record.revision, { [qid]: { values: (action.selected_options || []).map((o) => o.value), custom: record.answers[qid]?.custom || "" } }); + } else if (actionId === "cg_question_cancel") { + record = updateQuestion(record.id, record.revision, { status: "cancelled" }); + } else if (actionId === "cg_question_submit") { + if (missingQuestionAnswers(record).length) throw new Error("Please answer every required question before submitting."); + startQuestionContinuation(record, client, processMessage); + return; + } else throw new Error("Unknown question action."); + await refreshQuestionCard(record, client); + } catch (error) { + if (record) await refreshQuestionCard(record, client).catch(() => {}); + const target = record || { channelId: body?.channel?.id, threadKey: body?.message?.thread_ts }; + await notice(client, target, body?.user?.id, error.message); + } +} + +export async function handleQuestionView({ ack, body, view = body?.view, client }, { processMessage }) { + let acknowledged = false; + const respond = async (value) => { acknowledged = true; await ack(value); }; + let record; + try { + const metadata = parseQuestionMetadata(view?.private_metadata); + record = ownQuestion(metadata, body, { modal: true }); + await assertQuestionAccess(record, client, { timeoutMs: 1500 }); + if (view.callback_id === QUESTION_CUSTOM_CALLBACK) { + const qid = metadata.questionId; + const q = record.questions.find((q) => q.id === qid); + if (!q || !q.allowCustom) throw new Error("Custom answers are not allowed."); + const custom = view.state?.values?.[`q:${qid}`]?.custom?.value || ""; + record = saveQuestionAnswers(record.id, record.revision, { [qid]: { values: record.answers[qid]?.values || [], custom } }); + await respond(); + } else if (view.callback_id === QUESTION_FORM_CALLBACK) { + const page = metadata.page; + const slice = questionPage(record, page); + const patch = parseQuestionPageAnswers(record, page, view.state?.values); + const answers = { ...record.answers }; + for (const q of slice.questions) answers[q.id] = validateAnswer(q, patch[q.id]); + const missing = missingQuestionAnswers({ ...record, answers }, slice.questions); + if (missing.length) { + await respond({ response_action: "errors", errors: Object.fromEntries(missing.map((q) => [`q:${q.id}`, "Choose an option or write an answer."])) }); + return; + } + record = saveQuestionAnswers(record.id, record.revision, patch); + if (page + 1 < slice.totalPages) await respond({ response_action: "update", view: buildQuestionModal(record, page + 1) }); + else if (missingQuestionAnswers(record).length) { + // Back permits incomplete drafts. Return to the first missing page instead of dropping it. + const first = record.questions.findIndex((q) => missingQuestionAnswers(record).some((m) => m.id === q.id)); + await respond({ response_action: "update", view: buildQuestionModal(record, Math.floor(first / 3)) }); + } else { + await respond(); + startQuestionContinuation(record, client, processMessage); + } + } else throw new Error("Unknown question form."); + await refreshQuestionCard(getQuestion(record.id), client); + } catch (error) { + if (!acknowledged) { + const block = view?.blocks?.find((b) => b.type === "input")?.block_id; + await respond(block ? { response_action: "errors", errors: { [block]: error.message } } : {}); + } else await notice(client, record, body?.user?.id, error.message); + } +} + +export function registerQuestionActions(app, processMessage) { + app.action(/^cg_question_/, (payload) => handleQuestionAction(payload, { processMessage })); + app.view(QUESTION_FORM_CALLBACK, (payload) => handleQuestionView(payload, { processMessage })); + app.view(QUESTION_CUSTOM_CALLBACK, (payload) => handleQuestionView(payload, { processMessage })); +} diff --git a/test/folders-settings.test.js b/test/folders-settings.test.js index f23eb66..79e96ed 100644 --- a/test/folders-settings.test.js +++ b/test/folders-settings.test.js @@ -228,11 +228,11 @@ test("gateway MCP permission list tracks registered gateway tools", () => { // Tool registrations live in the per-group modules under src/mcp/tools/ (registered by the // gateway-server.js entry). Group order differs from the flat pre-split file, so compare the // registered names as a sorted list — same set, no duplicates, nothing lost. - const toolModules = ["schedules.js", "background.js", "channel-admin.js", "tokens.js", "slack-native.js", "skills.js"]; + const toolModules = ["schedules.js", "background.js", "channel-admin.js", "tokens.js", "slack-native.js", "skills.js", "questions.js"]; const source = toolModules .map((file) => readFileSync(new URL(`../src/mcp/tools/${file}`, import.meta.url), "utf8")) .join("\n"); - const registered = [...source.matchAll(/server\.registerTool\(\s*\n\s*"([^"]+)"/g)].map((m) => m[1]); + const registered = [...source.matchAll(/server\.registerTool\(\s*"([^"]+)"/g)].map((m) => m[1]); assert.deepEqual([...GATEWAY_TOOL_NAMES].sort(), [...registered].sort()); }); diff --git a/test/mcp-control-plane-approval.test.js b/test/mcp-control-plane-approval.test.js index 60d0ae1..7a83319 100644 --- a/test/mcp-control-plane-approval.test.js +++ b/test/mcp-control-plane-approval.test.js @@ -378,7 +378,7 @@ test("every registered gateway tool is consciously classified as gated or open ( "slack_channel_history", "slack_thread_replies", "slack_download_file", // lands only in this thread's uploads/ folder, this channel's files only "run_in_background", "run_agent_in_background", // shell kind has its own admin-click gate in background.js - "request_approval", "permission_prompt", "report_progress", + "request_approval", "permission_prompt", "report_progress", "ask_questions", "update_channel_memory", // operator decision 2026-08-07: memory is agent-owned, never approval-gated // operator decision 2026-08-19: reminders/scheduled tasks are an ordinary channel request and // are never approval-gated. A schedule fires with origin "schedule" (cannot escalate — A2), in diff --git a/test/question-continuation.test.js b/test/question-continuation.test.js new file mode 100644 index 0000000..6e7eb21 --- /dev/null +++ b/test/question-continuation.test.js @@ -0,0 +1,265 @@ +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { setTimeout as delay } from "node:timers/promises"; +import test from "node:test"; +import assert from "node:assert/strict"; +import { ensureTestEnv } from "./helpers.js"; + +ensureTestEnv(); +const projectRoot = fileURLToPath(new URL("..", import.meta.url)); +process.env.PATH = `${path.join(projectRoot, "test", "fixtures", "prompt-echo")}${path.delimiter}${process.env.PATH || ""}`; +process.env.SESSION_KEEPALIVE = "0"; +process.env.PROGRESS_VIEW = "shimmer"; +const { useFakeRuntime } = await import("./runtime-fake.js"); +const runtime = await useFakeRuntime(); +const { setUser, upsertChannelEntry, defaultChannelMeta, saveChannelMeta } = await import("../src/config/store.js"); +const { saveSettings } = await import("../src/config/settings.js"); +const { getDb } = await import("../src/db/index.js"); +const { processMessageEvent, runQueue, stopRunsInChannel } = await import("../src/slack/message-pipeline.js"); +const { acceptQuestionReply, clearActiveRun, listActiveRuns, recordActiveRun, recoverRuns } = await import("../src/gateway/active-runs.js"); +const { createQuestion, getQuestion, updateQuestion, saveQuestionAnswers, acceptQuestionSubmission } = await import("../src/gateway/questions.js"); + +const USER = "U_QUESTION_CONTINUATION"; +const CHANNEL = "D_QUESTION_CONTINUATION"; +let sequence = 0; + +function fakeSlack() { + const posted = []; + const updated = []; + const ok = async () => ({ ok: true }); + const client = { + posted, updated, members: [USER], + chat: { + postMessage: async (message) => { posted.push(message); return { ok: true, ts: `9000.${++sequence}` }; }, + update: async (message) => { updated.push(message); return { ok: true }; }, + postEphemeral: ok, + }, + users: { + info: async ({ user }) => ({ user: { id: user, real_name: "Question User" } }), + list: async () => ({ members: [{ id: USER, real_name: "Question User" }], response_metadata: {} }), + }, + conversations: { + history: async () => ({ messages: [] }), + replies: async () => ({ messages: [] }), + info: async ({ channel }) => ({ channel: { id: channel } }), + members: async () => ({ members: client.members, response_metadata: {} }), + }, + apiCall: ok, + }; + client.chatStream = () => ({ + ts: `9000.${++sequence}`, append: ok, + stop: async ({ markdown_text = "" } = {}) => { posted.push({ text: markdown_text }); return { ok: true }; }, + }); + return client; +} + +async function setup(threadKey) { + saveSettings({ engine: "claude", composioMode: "personal" }); + await setUser(USER, { name: "Question User", approved: true, isAdmin: false }); + const entry = await upsertChannelEntry(CHANNEL, { name: "question-continuation", type: "im", isDM: true, platform: "slack" }); + await saveChannelMeta(entry.slug, { ...defaultChannelMeta({ channelId: CHANNEL, name: entry.name, type: "im", isDM: true, platform: "slack" }), dmUserId: USER }); + return { channelId: CHANNEL, authorId: USER, slug: entry.slug, threadKey, isDM: true }; +} + +function question(context) { + return createQuestion(context, { title: "Access preferences", questions: [ + { id: "access", prompt: "Who should have access?", type: "single", options: [{ label: "Team only", value: "team" }, { label: "Everyone", value: "everyone" }] }, + ] }); +} + +function event(context, text = "Answers submitted to the agent's questions: Team only") { + return { type: "message", channel: context.channelId, channel_type: "im", user: context.authorId, + text, thread_ts: context.threadKey, ts: `${Number(context.threadKey) + 1}.001` }; +} + +async function queued(key) { + for (let attempt = 0; attempt < 200 && runQueue.count(key) < 2; attempt++) await delay(5); + assert.equal(runQueue.count(key), 2, "the answers should be accepted into the existing thread queue"); +} + +test("submitted answers bypass canonical card hydration and queue durably without steering", async () => { + const context = await setup("6000.001"); + const client = fakeSlack(); + const request = saveQuestionAnswers(question(context).id, 0, { access: { values: ["team"] } }); + const input = event(context); + let hydrated = 0; + client.conversations.replies = async () => { hydrated++; return { messages: [{ ts: input.ts, text: "WRONG canonical card text" }] }; }; + const key = `${context.slug}::${context.threadKey}`; + const active = { aborted: false, controller: new AbortController(), authorId: USER }; + await runQueue.acquire(key, active); + const running = processMessageEvent(input, client, { + botUserId: "U_BOT", teamId: "T_QUESTIONS", bypassMention: true, questionSubmissionId: request.id, + onQuestionSubmissionAccepted: ({ runId, rec }) => acceptQuestionSubmission(request.id, request.revision, runId, rec), + }); + try { + await queued(key); + assert.equal(hydrated, 0); + assert.equal(active.controller.signal.aborted, false); + const stored = listActiveRuns().find((r) => r.questionSubmissionId === request.id); + assert.equal(stored.authorId, USER); + assert.equal(stored.threadKey, context.threadKey); + assert.match(stored.text, /Team only/); + assert.doesNotMatch(stored.text, /WRONG/); + assert.equal(getQuestion(request.id).status, "submitted"); + assert.equal(acceptQuestionSubmission(request.id, request.revision, "duplicate", stored), false); + await stopRunsInChannel(client, CHANNEL, context.slug, USER, context.threadKey); + } finally { + runQueue.release(key, active); + await running; + } + assert.equal(listActiveRuns().some((r) => r.questionSubmissionId === request.id), false); +}); + +test("requester access revoked while answers are queued prevents the engine spawn", async () => { + const context = await setup("6100.001"); + const client = fakeSlack(); + const request = saveQuestionAnswers(question(context).id, 0, { access: { values: ["team"] } }); + const key = `${context.slug}::${context.threadKey}`; + const active = { aborted: false, controller: new AbortController(), authorId: USER }; + await runQueue.acquire(key, active); + const spawnsBefore = runtime.calls.spawn.length; + const running = processMessageEvent(event(context), client, { + botUserId: "U_BOT", questionSubmissionId: request.id, + onQuestionSubmissionAccepted: ({ runId, rec }) => acceptQuestionSubmission(request.id, request.revision, runId, rec), + }); + try { + await queued(key); + await setUser(USER, { approved: false }); + } finally { + runQueue.release(key, active); + await running; + } + assert.equal(runtime.calls.spawn.length, spawnsBefore); + assert.equal(listActiveRuns().some((r) => r.questionSubmissionId === request.id), false); + assert.ok(client.posted.some((m) => /no longer has access|could not be verified/.test(m.text || ""))); +}); + +test("live promotion preserves submission marker before the fixture engine starts", async () => { + const context = await setup("6150.001"); + const client = fakeSlack(); + const request = saveQuestionAnswers(question(context).id, 0, { access: { values: ["team"] } }); + const input = { ...event(context), ts: `question-${request.id}` }; + client.conversations.replies = async () => ({ messages: [{ ts: context.threadKey, user: USER, text: "Original task needs thread context." }] }); + const spawn = runtime.spawn; + let observed = false; + runtime.spawn = (target, spec) => { + const stored = listActiveRuns().find((r) => r.questionSubmissionId === request.id); + assert.equal(stored?.threadKey, context.threadKey); + assert.match(stored.text, /Team only/); + assert.match(stored.text, /Original task needs thread context/); + observed = true; + return spawn(target, spec); + }; + try { + await processMessageEvent(input, client, { + botUserId: "U_BOT", questionSubmissionId: request.id, + onQuestionSubmissionAccepted: ({ runId, rec }) => acceptQuestionSubmission(request.id, request.revision, runId, rec), + }); + } finally { + runtime.spawn = spawn; + } + assert.equal(observed, true); + assert.ok(client.posted.some((m) => /Team only/.test(m.text || ""))); +}); + +test("a typed thread answer carries pending question context into the fixture engine", async () => { + const context = await setup("6175.001"); + const client = fakeSlack(); + const request = question(context); + await processMessageEvent(event(context, "Actually, invite only please."), client, { botUserId: "U_BOT" }); + assert.equal(getQuestion(request.id).answeredInThread, true); + assert.ok(client.posted.some((m) => /Who should have access/.test(m.text || "") && /invite only please/.test(m.text || ""))); +}); + +test("restart recovery rechecks current membership before replaying submitted answers", async () => { + const context = await setup("6200.001"); + const client = fakeSlack(); + client.members = []; + const rec = { ...context, id: "question-recovery-revoked", questionSubmissionId: "saved-question", text: "saved answers", attachments: [] }; + recordActiveRun(rec.id, rec); + let calls = 0; + await recoverRuns([rec], { + slack: { snapshot: () => ({ connected: true }), getClient: () => client }, + runner: async () => { calls++; throw new Error("must never spawn"); }, + forceStopping: () => false, + }); + assert.equal(calls, 0); + assert.equal(listActiveRuns().some((r) => r.id === rec.id), false); + assert.ok(client.posted.some((m) => /not resumed/.test(m.text || ""))); +}); + +test("authorized restart recovery retains the submission identity in the durable run", async () => { + const context = await setup("6300.001"); + const client = fakeSlack(); + const rec = { ...context, id: "question-recovery-valid", questionSubmissionId: "saved-valid-question", text: "saved answers", attachments: [] }; + recordActiveRun(rec.id, rec); + let calls = 0; + await recoverRuns([rec], { + slack: { snapshot: () => ({ connected: true }), getClient: () => client }, + runner: async (args) => { + calls++; + assert.equal(args.authorId, USER); + assert.equal(args.threadKey, context.threadKey); + assert.equal(listActiveRuns().find((r) => r.id === rec.id).questionSubmissionId, rec.questionSubmissionId); + return { content: "continued", engine: "claude", usage: { output_tokens: 1 } }; + }, + deliver: async () => {}, usageRecorder: async () => {}, progressFactory: () => null, forceStopping: () => false, + }); + assert.equal(calls, 1); + assert.equal(listActiveRuns().some((r) => r.id === rec.id), false); +}); + +test("stop cancels questions before Slack acknowledgement and retires their controls", async () => { + const context = await setup("6400.001"); + const client = fakeSlack(); + const request = updateQuestion(question(context).id, 0, { messageTs: "6400.002" }); + let release; + const gate = new Promise((resolve) => { release = resolve; }); + client.chat.postMessage = async (message) => { client.posted.push(message); await gate; return { ok: true }; }; + const stopping = stopRunsInChannel(client, CHANNEL, context.slug, USER, context.threadKey); + assert.equal(getQuestion(request.id).status, "cancelled"); + release(); + assert.equal(await stopping, 1); + await delay(0); + assert.ok(client.updated.some((m) => m.ts === request.messageTs && !m.blocks.some((b) => b.type === "actions"))); +}); + +test("clear cancels only questions in the cleared thread", async () => { + const context = await setup("6500.001"); + const current = question(context); + const other = question({ ...context, threadKey: "6501.001" }); + const client = fakeSlack(); + await processMessageEvent(event(context, "/clear"), client, { botUserId: "U_BOT" }); + assert.equal(getQuestion(current.id).status, "cancelled"); + assert.equal(getQuestion(other.id).status, "pending"); + await stopRunsInChannel(client, CHANNEL, context.slug, USER, "6501.001"); +}); + +test("ordinary reply atomically retires only matching forms and keeps their prompt context", async () => { + const context = await setup("6600.001"); + const current = question(context); + const other = question({ ...context, authorId: "U_OTHER" }); + const text = `Question context: ${JSON.stringify(current.questions)}\nUser reply: invite only`; + const retired = acceptQuestionReply([current], "question-typed-reply", { ...context, text }); + assert.equal(retired[0].answeredInThread, true); + assert.equal(getQuestion(current.id).status, "cancelled"); + assert.equal(getQuestion(other.id).status, "pending"); + assert.equal(listActiveRuns().find((r) => r.id === "question-typed-reply").text, text); + clearActiveRun("question-typed-reply"); + await stopRunsInChannel(fakeSlack(), CHANNEL, context.slug, USER, context.threadKey); +}); + +test("ordinary reply persistence failure rolls back form cancellation", async () => { + const context = await setup("6700.001"); + const request = question(context); + const db = getDb(); + db.exec("CREATE TRIGGER question_reply_failure BEFORE INSERT ON active_runs BEGIN SELECT RAISE(ABORT, 'fixture write failure'); END"); + try { + assert.throws(() => acceptQuestionReply([request], "question-write-failure", { ...context, text: "answer" }), /fixture write failure/); + assert.equal(getQuestion(request.id).status, "pending"); + assert.equal(listActiveRuns().some((r) => r.id === "question-write-failure"), false); + } finally { + db.exec("DROP TRIGGER question_reply_failure"); + } + await stopRunsInChannel(fakeSlack(), CHANNEL, context.slug, USER, context.threadKey); +}); diff --git a/test/question-interactions.test.js b/test/question-interactions.test.js new file mode 100644 index 0000000..7bb255e --- /dev/null +++ b/test/question-interactions.test.js @@ -0,0 +1,269 @@ +// Real durable store and authorization with captured Slack payloads; no engine/network access. +import test from "node:test"; +import assert from "node:assert/strict"; +import { randomUUID } from "node:crypto"; +import { ensureTestEnv } from "./helpers.js"; +ensureTestEnv(); + +const { setUser, upsertChannelEntry, saveChannelMeta, getChannelMeta } = await import("../src/config/store.js"); +const { getDb } = await import("../src/db/index.js"); +const { getQuestion, saveQuestionAnswers } = await import("../src/gateway/questions.js"); +const { postQuestions, handleQuestionAction, handleQuestionView, refreshQuestionCard } = await import("../src/slack/questions.js"); +const { buildQuestionCard, buildQuestionModal, buildCustomAnswerModal } = await import("../src/slack/question-views.js"); + +const OWNER = "UQUESTION_OWNER"; +const OTHER = "UQUESTION_OTHER"; +const CHANNEL = "CQUESTION_INTERACTIONS"; +await setUser(OWNER, { name: "Question owner", approved: true, isAdmin: false }); +await setUser(OTHER, { name: "Other member", approved: true, isAdmin: false }); +const entry = await upsertChannelEntry(CHANNEL, { name: "question-interactions", type: "channel", isDM: false }); +const SLUG = entry.slug; +await saveChannelMeta(SLUG, { ...await getChannelMeta(SLUG), channelId: CHANNEL, access: "approved", platform: "slack", isDM: false }); + +const choice = (id = "access", extra = {}) => ({ id, prompt: "Who can access this?", type: "single", required: true, + allowCustom: true, options: [{ label: "Team", value: "team" }, { label: "Everyone", value: "all" }], ...extra }); +const elements = (view) => view.blocks.flatMap((b) => (b.elements || []).map((el) => ({ ...el, block_id: b.block_id }))); +const settle = async () => { for (let i = 0; i < 12; i++) await new Promise((resolve) => setImmediate(resolve)); }; + +async function fixture(questions = [choice()]) { + const log = { posts: [], updates: [], notices: [], opens: [], viewUpdates: [], continuations: [], accepted: [] }; + const control = { members: [OWNER, OTHER], updateError: null }; + const client = { + conversations: { members: async () => ({ members: control.members }) }, + chat: { + postMessage: async (payload) => { log.posts.push(payload); return { ok: true, ts: `1900.${log.posts.length}` }; }, + update: async (payload) => { if (control.updateError) throw control.updateError; log.updates.push(payload); return { ok: true }; }, + postEphemeral: async (payload) => { log.notices.push(payload); return { ok: true }; }, + }, + views: { + open: async (payload) => { log.opens.push(payload); return { ok: true }; }, + update: async (payload) => { log.viewUpdates.push(payload); return { ok: true }; }, + }, + }; + const context = { channelId: CHANNEL, authorId: OWNER, slug: SLUG, threadKey: `1800.${randomUUID()}` }; + const input = { title: "Choose the behavior", questions }; + const initial = await postQuestions(context, input, { client }); + const current = () => getQuestion(initial.id); + const processMessage = async (event, usedClient, options) => { + assert.equal(usedClient, client); + log.continuations.push({ event, options }); + log.accepted.push(options.onQuestionSubmissionAccepted({ runId: randomUUID(), rec: { ...context, userId: OWNER } })); + }; + const act = async (actionId, { data = current(), user = OWNER, channel = CHANNEL, ts = data.messageTs, selected_options, view } = {}) => { + const source = view || buildQuestionCard(data); + const action = elements(source).find((el) => el.action_id === actionId); + assert.ok(action, `missing action ${actionId}`); + if (selected_options) action.selected_options = selected_options; + const acks = []; + const body = { user: { id: user }, channel: { id: channel }, message: { ts, thread_ts: data.threadKey }, trigger_id: "trigger-test", ...(view ? { view } : {}) }; + await handleQuestionAction({ ack: async (result) => acks.push(result), body, action, client }, { processMessage }); + assert.equal(acks.length, 1); + return acks; + }; + const submitView = async (view, values, user = OWNER) => { + const acks = []; + view = { ...view, id: "VQUESTION", hash: "hash-test", state: { values } }; + await handleQuestionView({ ack: async (result) => acks.push(result), body: { user: { id: user }, view }, view, client }, { processMessage }); + assert.equal(acks.length, 1); + return acks[0]; + }; + return { log, control, client, context, input, initial, current, act, submitView }; +} + +test("posting is idempotent and visible card carries the persisted revision", async () => { + const f = await fixture(); + const again = await postQuestions(f.context, f.input, { client: f.client }); + assert.equal(again.id, f.initial.id); + assert.equal(f.log.posts.length, 1); + const visible = f.log.updates.at(-1) || f.log.posts.at(-1); + assert.equal(JSON.parse(elements(visible).find((el) => el.action_id === "cg_question_submit").value).revision, again.revision); +}); + +test("options and custom save are drafts; duplicate submit creates one durable continuation", async () => { + const f = await fixture(); + await f.act("cg_question_choose:access:0"); + assert.deepEqual(f.current().answers.access, { values: ["team"], custom: "" }); + await f.act("cg_question_custom:access"); + const custom = f.log.opens.at(-1).view; + const result = await f.submitView(custom, { "q:access": { custom: { type: "plain_text_input", value: "Invited guests" } } }); + assert.equal(result, undefined); + assert.deepEqual(f.current().answers.access, { values: [], custom: "Invited guests" }); + assert.equal(f.log.continuations.length, 0); + const before = f.current(); + await Promise.all([f.act("cg_question_submit", { data: before }), f.act("cg_question_submit", { data: before })]); + await settle(); + assert.equal(f.log.continuations.length, 1); + assert.deepEqual(f.log.accepted, [true]); + assert.equal(f.current().status, "submitted"); + const continuation = f.log.continuations[0]; + assert.equal(continuation.event.channel, CHANNEL); + assert.equal(continuation.event.user, OWNER); + assert.equal(continuation.event.thread_ts, f.context.threadKey); + assert.match(continuation.event.text, /Invited guests/); + assert.equal(continuation.options.busyChoice, "queue"); + assert.equal(getDb().prepare("SELECT COUNT(*) AS n FROM active_runs WHERE id=?").get(f.current().runId).n, 1); +}); + +test("other user, wrong channel/message, stale buttons and departed members cannot mutate drafts", async () => { + const f = await fixture(); + const initial = f.current(); + for (const args of [{ user: OTHER }, { channel: "CWRONG" }, { ts: "wrong.timestamp" }]) { + await f.act("cg_question_choose:access:0", args); + assert.equal(f.current().revision, initial.revision); + } + await f.act("cg_question_choose:access:0"); + const selected = f.current(); + await f.act("cg_question_choose:access:1", { data: initial }); + assert.deepEqual(f.current().answers, selected.answers); + f.control.members = [OTHER]; + await f.act("cg_question_choose:access:1"); + assert.deepEqual(f.current().answers, selected.answers); + assert.equal(f.log.notices.length, 5); + assert.equal(f.log.continuations.length, 0); +}); + +test("modal owner and revision checks reject replayed custom answers", async () => { + const f = await fixture(); + const stale = buildCustomAnswerModal(f.current(), "access"); + const values = { "q:access": { custom: { value: "Untrusted answer" } } }; + const denied = await f.submitView(stale, values, OTHER); + assert.equal(denied.response_action, "errors"); + assert.deepEqual(f.current().answers, {}); + await f.act("cg_question_choose:access:0"); + const rejected = await f.submitView(stale, values); + assert.equal(rejected.response_action, "errors"); + assert.deepEqual(f.current().answers.access, { values: ["team"], custom: "" }); + assert.equal(f.log.continuations.length, 0); +}); + +test("multi checkbox payload identity persists options and custom text together", async () => { + const f = await fixture([choice("features", { type: "multi" })]); + await f.act("cg_question_multi:features", { selected_options: [{ text: { type: "plain_text", text: "Everyone" }, value: "all" }] }); + await f.submitView(buildCustomAnswerModal(f.current(), "features"), { "q:features": { custom: { value: "One more" } } }); + assert.deepEqual(f.current().answers.features, { values: ["all"], custom: "One more" }); + await f.act("cg_question_multi:features", { selected_options: [] }); + assert.deepEqual(f.current().answers.features, { values: [], custom: "One more" }); + assert.equal(f.log.continuations.length, 0); +}); + +test("Next validates required answers, Back saves incomplete drafts and Submit resumes once", async () => { + const f = await fixture(Array.from({ length: 4 }, (_, i) => choice(`q${i}`, { allowCustom: false }))); + await f.act("cg_question_open"); + const first = f.log.opens.at(-1).view; + const missing = await f.submitView(first, {}); + assert.equal(missing.response_action, "errors"); + assert.deepEqual(Object.keys(missing.errors), ["q:q0", "q:q1", "q:q2"]); + const firstValues = Object.fromEntries([0, 1, 2].map((i) => [`q:q${i}`, { choice: { type: "radio_buttons", selected_option: { value: "team" } } }])); + const next = await f.submitView(first, firstValues); + assert.equal(next.response_action, "update"); + assert.equal(JSON.parse(next.view.private_metadata).page, 1); + const page1 = { ...next.view, id: "VQUESTION", hash: "hash-test", state: { values: {} } }; + await f.act("cg_question_back", { view: page1 }); + assert.equal(JSON.parse(f.log.viewUpdates.at(-1).view.private_metadata).page, 0); + assert.deepEqual(f.current().answers.q3, { values: [], custom: "" }); + assert.equal(f.log.continuations.length, 0); + const secondNext = await f.submitView(f.log.viewUpdates.at(-1).view, firstValues); + await f.submitView(secondNext.view, { "q:q3": { choice: { type: "radio_buttons", selected_option: { value: "all" } } } }); + await settle(); + assert.equal(f.current().status, "submitted"); + assert.equal(f.log.continuations.length, 1); + assert.deepEqual(f.current().answers.q3, { values: ["all"], custom: "" }); +}); + +test("cancellation retires a form without invoking the agent", async () => { + const f = await fixture(); + await f.act("cg_question_cancel"); + assert.equal(f.current().status, "cancelled"); + assert.equal(f.log.continuations.length, 0); + assert.ok(!f.log.updates.at(-1).blocks.some((b) => b.type === "actions")); +}); + +test("retry repairs a failed card refresh without duplicate posting", async () => { + const f = await fixture(); + f.control.updateError = new Error("temporary update failure"); + await f.act("cg_question_choose:access:0"); + const updated = f.current(); + assert.deepEqual(updated.answers.access, { values: ["team"], custom: "" }); + assert.match(f.log.notices.at(-1).text, /temporary update failure/); + // A retry of the original request must redraw this persisted revision, even if an earlier + // interactive card edit failed after the draft was saved. + f.control.updateError = null; + await postQuestions(f.context, f.input, { client: f.client }); + assert.equal(f.log.posts.length, 1); + const visible = f.log.updates.at(-1); + assert.equal(JSON.parse(elements(visible).find((el) => el.action_id === "cg_question_submit").value).revision, updated.revision); +}); + +test("stale Open form recovers the durable draft after a card update fails", async () => { + const f = await fixture(); + const original = f.current(); + f.control.updateError = new Error("temporary card update failure"); + await f.act("cg_question_choose:access:1"); + assert.ok(f.current().revision > original.revision); + f.control.updateError = null; + await f.act("cg_question_open", { data: original }); + assert.equal(f.log.opens.length, 1); + const view = f.log.opens[0].view; + assert.equal(JSON.parse(view.private_metadata).revision, f.current().revision); + assert.equal(view.blocks.find((b) => b.block_id === "q:access").element.initial_option.value, "all"); + assert.equal(f.log.continuations.length, 0); +}); + +test("overlapping card refreshes serialize and reread the newest durable answers", async () => { + const f = await fixture(); + const saved = saveQuestionAnswers(f.current().id, f.current().revision, { access: { values: ["team"], custom: "" } }); + let releaseFirst; + let announceFirst; + const firstEntered = new Promise((resolve) => { announceFirst = resolve; }); + const firstGate = new Promise((resolve) => { releaseFirst = resolve; }); + const completed = []; + let inFlight = 0; + let maxInFlight = 0; + let calls = 0; + f.client.chat.update = async (payload) => { + inFlight++; + maxInFlight = Math.max(maxInFlight, inFlight); + if (++calls === 1) { + announceFirst(); + await firstGate; + } + completed.push(payload); + inFlight--; + return { ok: true }; + }; + const first = refreshQuestionCard(saved, f.client); + await firstEntered; + const newest = saveQuestionAnswers(saved.id, saved.revision, { access: { values: ["all"], custom: "" } }); + // Intentionally pass the old record: rendering must reread after the first update finishes. + const second = refreshQuestionCard(saved, f.client); + await settle(); + assert.equal(calls, 1); + releaseFirst(); + await Promise.all([first, second]); + assert.equal(maxInFlight, 1); + assert.equal(completed.length, 2); + const final = completed.at(-1); + assert.equal(JSON.parse(elements(final).find((el) => el.action_id === "cg_question_submit").value).revision, newest.revision); + assert.equal(elements(final).find((el) => el.action_id === "cg_question_choose:access:1").style, "primary"); + assert.equal(f.log.continuations.length, 0); +}); + +test("slow modal membership verification acknowledges an error before Slack's three-second deadline", async () => { + const f = await fixture(); + const original = f.current(); + let resolveMembership; + f.client.conversations.members = () => new Promise((resolve) => { resolveMembership = resolve; }); + const started = performance.now(); + const response = await f.submitView(buildCustomAnswerModal(original, "access"), { + "q:access": { custom: { value: "Must not be saved after timeout" } }, + }); + const elapsed = performance.now() - started; + assert.equal(response.response_action, "errors"); + assert.match(response.errors["q:access"], /verification took too long/); + assert.ok(elapsed < 3000, `Slack acknowledgement took ${elapsed}ms`); + assert.equal(f.current().revision, original.revision); + resolveMembership({ members: [OWNER] }); + await settle(); + assert.deepEqual(f.current().answers, {}); + assert.equal(f.log.continuations.length, 0); +}); diff --git a/test/question-views.test.js b/test/question-views.test.js new file mode 100644 index 0000000..f3342dd --- /dev/null +++ b/test/question-views.test.js @@ -0,0 +1,185 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + buildQuestionCard, buildQuestionModal, buildCustomAnswerModal, questionPage, + questionPresentation, parseQuestionMetadata, parseQuestionPageAnswers, + QUESTION_FORM_CALLBACK, QUESTION_CUSTOM_CALLBACK, +} from "../src/slack/question-views.js"; + +const single = (id = "access", extra = {}) => ({ + id, prompt: "Who should have access?", type: "single", required: true, allowCustom: true, + options: [{ label: "Team only", value: "team" }, { label: "Everyone", value: "all" }], ...extra, +}); +const record = (extra = {}) => ({ + id: "question-form-id", revision: 2, title: "A few decisions", status: "pending", + questions: [single()], answers: {}, ...extra, +}); +const allElements = (view) => view.blocks.flatMap((block) => block.elements || (block.element ? [block.element] : [])); + +function assertSlackBounds(view, modal = false) { + assert.ok(view.blocks.length <= (modal ? 100 : 50)); + if (modal) { + assert.ok(view.title.text.length <= 24); + assert.ok(view.submit.text.length <= 24); + assert.ok(view.close.text.length <= 24); + assert.ok(view.private_metadata.length <= 3000); + } + const visit = (obj) => { + if (!obj || typeof obj !== "object") return; + assert.notEqual(obj.type, "mrkdwn"); + if (obj.action_id) assert.ok(obj.action_id.length <= 255); + if (obj.block_id) assert.ok(obj.block_id.length <= 255); + if (obj.type === "section") assert.ok(obj.text.text.length <= 3000); + if (obj.type === "header") assert.ok(obj.text.text.length <= 150); + if (obj.type === "actions") assert.ok(obj.elements.length <= 25); + if (obj.type === "button") { + assert.ok(obj.text.text.length <= 75); + assert.ok(obj.value.length <= 2000); + } + if (obj.type === "input") assert.ok(obj.label.text.length <= 2000); + if (["radio_buttons", "checkboxes"].includes(obj.type)) { + assert.ok(obj.options.length <= 10); + for (const option of obj.options) { + assert.ok(option.text.text.length <= 75); + assert.ok(option.value.length <= 150); + } + for (const initial of obj.initial_options || (obj.initial_option ? [obj.initial_option] : [])) { + assert.ok(obj.options.some((o) => JSON.stringify(o) === JSON.stringify(initial))); + } + } + for (const value of Object.values(obj)) { + if (Array.isArray(value)) value.forEach(visit); + else if (value && typeof value === "object") visit(value); + } + }; + visit(view); +} + +test("message buttons carry form/revision identity, dynamic labels and selected state", () => { + const data = record({ answers: { access: { values: ["team"], custom: "" } } }); + const card = buildQuestionCard(data); + const elements = allElements(card); + const team = elements.find((el) => el.action_id === "cg_question_choose:access:0"); + assert.equal(team.text.text, "Team only"); + assert.equal(team.style, "primary"); + assert.deepEqual(JSON.parse(team.value), { id: data.id, revision: 2 }); + assert.equal(elements.find((el) => el.action_id === "cg_question_choose:access:1").style, undefined); + assert.ok(elements.some((el) => el.action_id === "cg_question_custom:access")); + assert.ok(elements.some((el) => el.action_id === "cg_question_submit")); + assert.ok(card.blocks.some((b) => b.block_id === `cg_question:${data.id}:2:access`)); + assertSlackBounds(card); +}); + +test("multi choices use checkboxes with exact initial option objects and custom text", () => { + const card = buildQuestionCard(record({ questions: [single("features", { type: "multi" })], + answers: { features: { values: ["team"], custom: "Another feature" } } })); + const checkbox = allElements(card).find((el) => el.type === "checkboxes"); + assert.deepEqual(checkbox.initial_options, [checkbox.options[0]]); + assert.equal(checkbox.action_id, "cg_question_multi:features"); + assert.ok(card.blocks.some((b) => b.text?.text === "Current answer: Team only, Another feature")); + assertSlackBounds(card); +}); + +test("auto presentation opens long/text forms and explicit short text message gets write button", () => { + const long = record({ questions: Array.from({ length: 5 }, (_, i) => single(`q${i}`)) }); + assert.equal(questionPresentation(long), "modal"); + assert.equal(questionPresentation({ ...long, presentation: "message" }), "modal"); + const text = record({ questions: [single("notes", { type: "text", options: [] })] }); + assert.equal(questionPresentation(text), "modal"); + const launch = allElements(buildQuestionCard(long)); + assert.ok(launch.some((el) => el.action_id === "cg_question_open")); + assert.ok(!launch.some((el) => el.action_id === "cg_question_submit")); + const explicit = buildQuestionCard({ ...text, presentation: "message" }); + assert.ok(allElements(explicit).some((el) => el.action_id === "cg_question_custom:notes")); +}); + +test("submitted and cancelled cards remove all controls and show answers as plain text", () => { + for (const status of ["submitted", "cancelled"]) { + const card = buildQuestionCard(record({ status, answers: { access: { values: ["team"], custom: "My choice" } } })); + assert.equal(allElements(card).length, 1); // Header excluded, single context text included. + assert.ok(!card.blocks.some((b) => b.type === "actions")); + assert.ok(card.blocks.some((b) => b.text?.text.endsWith("\nMy choice"))); + assertSlackBounds(card); + } +}); + +test("paginated form restores each page's draft and exposes Back/Next/Submit", () => { + const data = record({ questions: Array.from({ length: 7 }, (_, i) => single(`q${i}`)), + answers: { q3: { values: ["all"], custom: "" }, q4: { values: [], custom: "Custom value" } } }); + assert.deepEqual(questionPage(data, 1).questions.map((q) => q.id), ["q3", "q4", "q5"]); + for (const invalid of [-1, 3, 0.2, NaN]) assert.throws(() => questionPage(data, invalid), /Invalid question page/); + const first = buildQuestionModal(data); + assert.equal(first.callback_id, QUESTION_FORM_CALLBACK); + assert.equal(first.submit.text, "Next"); + assert.ok(!allElements(first).some((el) => el.action_id === "cg_question_back")); + const second = buildQuestionModal(data, 1); + assert.deepEqual(JSON.parse(second.private_metadata), { id: data.id, revision: 2, page: 1 }); + assert.equal(second.blocks.find((b) => b.block_id === "q:q3").element.initial_option.value, "all"); + assert.equal(second.blocks.find((b) => b.block_id === "custom:q4").element.initial_value, "Custom value"); + assert.ok(allElements(second).some((el) => el.action_id === "cg_question_back")); + const last = buildQuestionModal(data, 2); + assert.equal(last.submit.text, "Submit"); + assert.equal(last.blocks.filter((b) => b.type === "input").length, 2); + [first, second, last].forEach((view) => assertSlackBounds(view, true)); +}); + +test("custom modal preserves text, permits clearing and disallows unsupported questions", () => { + const data = record({ answers: { access: { values: [], custom: "Specific team" } } }); + const view = buildCustomAnswerModal(data, "access"); + assert.equal(view.callback_id, QUESTION_CUSTOM_CALLBACK); + assert.deepEqual(JSON.parse(view.private_metadata), { id: data.id, revision: 2, questionId: "access" }); + assert.equal(view.blocks[0].optional, true); + assert.equal(view.blocks[0].element.initial_value, "Specific team"); + assert.equal(view.blocks[0].element.max_length, 2000); + assert.throws(() => buildCustomAnswerModal(data, "unknown"), /not allowed/); + assert.throws(() => buildCustomAnswerModal(record({ questions: [single("access", { allowCustom: false })] }), "access"), /not allowed/); + assertSlackBounds(view, true); +}); + +test("page parsing gives custom single answer precedence, supplements multi and excludes other pages", () => { + const data = record({ questions: [single(), single("features", { type: "multi" }), single("notes", { type: "text" }), single("later")] }); + const answers = parseQuestionPageAnswers(data, 0, { + "q:access": { choice: { selected_option: { value: "team" } } }, + "custom:access": { custom: { value: "A custom team" } }, + "q:features": { choice: { selected_options: [{ value: "team" }, { value: "all" }] } }, + "custom:features": { custom: { value: "One more" } }, + "q:notes": { custom: { value: "Notes" } }, + "q:later": { choice: { selected_option: { value: "all" } } }, + }); + assert.deepEqual(answers, { access: { values: [], custom: "A custom team" }, features: { values: ["team", "all"], custom: "One more" }, notes: { values: [], custom: "Notes" } }); + assert.deepEqual(parseQuestionPageAnswers(data, 0), { access: { values: [], custom: "" }, features: { values: [], custom: "" }, notes: { values: [], custom: "" } }); + assert.deepEqual(parseQuestionPageAnswers(data, 0, { + "q:access": { choice: { selected_option: { value: "team" } } }, + "custom:access": { custom: { value: " " } }, + }).access, { values: ["team"], custom: " " }); +}); + +test("modal input requirements permit custom alternatives and preserve strict required choice", () => { + const view = buildQuestionModal(record({ questions: [single(), single("strict", { allowCustom: false }), single("notes", { type: "text" })] })); + assert.equal(view.blocks.find((b) => b.block_id === "q:access").optional, true); + assert.equal(view.blocks.find((b) => b.block_id === "q:strict").optional, false); + assert.equal(view.blocks.find((b) => b.block_id === "q:notes").optional, false); +}); + +test("metadata parser rejects malformed, negative and noninteger identities", () => { + for (const value of [null, "no json", "null", "{}", '{"id":"x","revision":-1}', '{"id":"x","revision":1.5}', '{"id":"x","revision":1,"page":-1}', '{"id":"x","revision":1,"questionId":3}']) { + assert.equal(parseQuestionMetadata(value), null); + } + assert.deepEqual(parseQuestionMetadata('{"id":"x","revision":0}'), { id: "x", revision: 0 }); +}); + +test("maximum schema sizes remain Slack-valid and caller text cannot inject mentions or mrkdwn", () => { + const questions = Array.from({ length: 20 }, (_, i) => single(`q${i}`, { + prompt: "<@U123> *unsafe* ".padEnd(300, "x"), type: "multi", + options: Array.from({ length: 10 }, (_, j) => ({ label: "<@U123>".padEnd(60, "x"), value: String(j).padEnd(60, "v") })), + })); + const data = record({ title: "".padEnd(150, "x"), questions, + answers: Object.fromEntries(questions.map((q) => [q.id, { values: q.options.map((o) => o.value), custom: "".padEnd(2000, "x") }])) }); + for (let page = 0; page < 7; page++) assertSlackBounds(buildQuestionModal(data, page), true); + assertSlackBounds(buildQuestionCard({ ...data, status: "submitted" })); + const card = buildQuestionCard({ ...data, questions: questions.slice(0, 4) }); + assertSlackBounds(card); + assert.equal(card.mrkdwn, false); + assert.ok(!card.text.includes("")); + assert.ok(card.blocks.some((b) => b.text?.text.includes(""))); +}); diff --git a/test/questions.test.js b/test/questions.test.js new file mode 100644 index 0000000..9dd9a5f --- /dev/null +++ b/test/questions.test.js @@ -0,0 +1,101 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { ensureTestEnv } from "./helpers.js"; +ensureTestEnv(); +const store = await import("../src/gateway/questions.js"); +const { getDb } = await import("../src/db/index.js"); +const { register } = await import("../src/mcp/tools/questions.js"); + +let nextThread = 0; +const context = () => ({ channelId: "C_QUESTIONS", slug: "questions-test", authorId: "U_REQUESTER", threadKey: `${++nextThread}.00001` }); +const input = () => ({ title: "Choices", questions: [ + { id: "access", prompt: "Access?", type: "single", options: [{ label: "Team only", value: "team" }, { label: "Invite only", value: "invite" }] }, +] }); + +test("schema rejects duplicate IDs, forged fields, unsupported choices and invalid lengths", () => { + const value = input(); + assert.equal(store.normalizeQuestions(value).questions[0].allowCustom, true); + assert.throws(() => store.normalizeQuestions({ ...value, channelId: "C_OTHER" })); + assert.throws(() => store.normalizeQuestions({ ...value, questions: [...value.questions, ...value.questions] }), /unique/); + assert.throws(() => store.normalizeQuestions({ ...value, questions: [{ ...value.questions[0], options: Array(5).fill({ label: "x", value: "x" }) }] }), /four/); + assert.throws(() => store.normalizeQuestions({ ...value, questions: [{ ...value.questions[0], options: [{ label: "x", value: "x" }, { label: "y", value: "x" }] }] }), /unique/); + assert.throws(() => store.normalizeQuestions({ ...value, title: "x".repeat(121) })); + assert.throws(() => store.normalizeQuestions({ ...value, questions: [{ id: "__proto__", prompt: "bad", type: "text" }] })); + assert.throws(() => store.normalizeQuestions({ ...value, presentation: "message", questions: Array.from({ length: 5 }, (_, i) => ({ id: `q${i}`, prompt: "x", type: "text" })) }), /four/); +}); + +test("creation retries reuse the same pending card, drafts are readable by a fresh process", () => { + const ctx = context(); + let record = store.createQuestion(ctx, input()); + record = store.bindQuestionMessage(record.id, "123.001"); + assert.equal(record.revision, 0, "delivery binding must not stale the posted controls"); + record = store.saveQuestionAnswers(record.id, record.revision, { access: { values: ["team"], custom: "" } }); + assert.equal(store.createQuestion(ctx, input()).id, record.id); + assert.throws(() => store.createQuestion(ctx, { ...input(), title: "Different" }), /pending/); + const moduleUrl = new URL("../src/gateway/questions.js", import.meta.url).href; + const fresh = JSON.parse(execFileSync(process.execPath, ["--input-type=module", "-e", `const {getQuestion}=await import(${JSON.stringify(moduleUrl)}); process.stdout.write(JSON.stringify(getQuestion(${JSON.stringify(record.id)})));`], { encoding: "utf8" })); + assert.deepEqual(fresh.answers.access.values, ["team"]); + assert.equal(fresh.messageTs, "123.001"); +}); + +test("draft validation rejects forged options, stale writes and custom text where disabled", () => { + let record = store.createQuestion(context(), input()); + assert.throws(() => store.saveQuestionAnswers(record.id, 0, { access: { values: ["forged"] } }), /Invalid/); + assert.throws(() => store.saveQuestionAnswers(record.id, 0, { missing: { custom: "x" } }), /Unknown/); + record = store.saveQuestionAnswers(record.id, 0, { access: { values: ["team"], custom: " My answer " } }); + assert.deepEqual(record.answers.access, { values: [], custom: "My answer" }); + assert.throws(() => store.saveQuestionAnswers(record.id, 0, { access: { values: ["invite"] } }), /changed/); + assert.throws(() => store.validateAnswer({ ...record.questions[0], allowCustom: false }, { custom: "No" }), /Invalid/); + assert.throws(() => store.validateAnswer(record.questions[0], { custom: "x".repeat(2001) }), /2000/); + assert.deepEqual(store.validateAnswer(record.questions[0], { values: ["team"], custom: " " }), { values: ["team"], custom: "" }); +}); + +test("Submit validates required answers and atomically creates exactly one continuation", () => { + const ctx = context(); + let record = store.createQuestion(ctx, input()); + const run = { ...ctx, text: "Answers", questionSubmissionId: record.id }; + assert.equal(store.acceptQuestionSubmission(record.id, 0, "unanswered", run), false); + record = store.saveQuestionAnswers(record.id, 0, { access: { values: ["invite"] } }); + assert.throws(() => store.acceptQuestionSubmission(record.id, record.revision, "wrong-author", { ...run, authorId: "U_OTHER" }), /identity/); + assert.equal(store.getQuestion(record.id).status, "pending"); + assert.equal(store.acceptQuestionSubmission(record.id, record.revision, "submitted-once", run), true); + assert.equal(store.acceptQuestionSubmission(record.id, record.revision, "submitted-twice", run), false); + assert.equal(store.getQuestion(record.id).status, "submitted"); + assert.ok(getDb().prepare("SELECT id FROM active_runs WHERE id=?").get("submitted-once")); + assert.equal(getDb().prepare("SELECT id FROM active_runs WHERE id=?").get("submitted-twice"), undefined); +}); + +test("failed continuation persistence rolls back submission, and cancellation is scoped", () => { + const ctx = context(); + let record = store.createQuestion(ctx, input()); + record = store.saveQuestionAnswers(record.id, 0, { access: { values: ["team"] } }); + getDb().exec("CREATE TEMP TRIGGER reject_question_run BEFORE INSERT ON active_runs BEGIN SELECT RAISE(ABORT, 'test failure'); END"); + try { assert.throws(() => store.acceptQuestionSubmission(record.id, record.revision, "rollback", ctx), /test failure/); } + finally { getDb().exec("DROP TRIGGER reject_question_run"); } + assert.equal(store.getQuestion(record.id).status, "pending"); + assert.equal(store.getQuestion(record.id).revision, record.revision); + const other = store.createQuestion({ ...ctx, authorId: "U_OTHER" }, input()); + assert.equal(store.cancelPendingQuestions(ctx).length, 1); + assert.equal(store.getQuestion(other.id).status, "pending"); + assert.equal(store.acceptQuestionSubmission(record.id, record.revision, "after-stop", ctx), false); +}); + +test("question tool is available to both engines only for a trusted foreground thread", async () => { + const base = { origin: "slack_foreground", principalTrusted: true, threadKey: "123.456", channelId: "C_THIS", slug: "this", createdBy: "U_THIS", text: (s) => ({ content: [{ type: "text", text: s }] }) }; + for (const activeEngine of ["claude", "codex"]) { + const calls = []; + let handler; + register({ registerTool(name, def, fn) { assert.equal(name, "ask_questions"); handler = fn; } }, { ...base, activeEngine }, { + post: async (ctx, args) => { calls.push({ ctx, args }); return { id: "request-id" }; }, + }); + const result = await handler(input()); + assert.deepEqual(calls[0].ctx, { channelId: "C_THIS", threadKey: "123.456", authorId: "U_THIS", slug: "this" }); + assert.match(result.content[0].text, /Awaiting.*Submit/); + } + for (const override of [{ origin: "schedule" }, { origin: "recovery" }, { origin: "background_agent" }, { principalTrusted: false }, { threadKey: "123.456::agent-1" }]) { + let registered = false; + register({ registerTool() { registered = true; } }, { ...base, ...override }); + assert.equal(registered, false); + } +}); From a7b5bc24cbf931685d83a12b76edd37db8d1b616 Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Sun, 13 Sep 2026 18:46:42 +0300 Subject: [PATCH 2/2] fix(gateway): keep clarification guidance within prompt budget Signed-off-by: Tiberiu Socaci --- src/gateway/folders.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/gateway/folders.js b/src/gateway/folders.js index 2f5fe2a..fded454 100644 --- a/src/gateway/folders.js +++ b/src/gateway/folders.js @@ -185,10 +185,7 @@ export function channelSwitchesNote(meta = {}) { // the predicate behind it live in src/gateway/mcp.js, beside the code that names those servers. const HARD_RULES = `**Hard rules (not optional)** — they apply wherever the named tools exist; the reasoning and the tool shapes are in the \`gateway-usage\` skill: -- **Clarification questions.** When \`ask_questions\` is available, use that gateway tool for - questions with choices or custom text. It returns a pending card, not answers: continue only - independent work or end this turn, and let Submit resume the thread. Do not poll or assume an - answer. If unavailable, ask in the conversation. Action approvals still use \`request_approval\`. +- Use \`ask_questions\` for clarification. - **Two Composio identities.** \`composio-user\` = the REQUESTER's own accounts; \`composio-agent\` = the shared agent's own (either may appear with \`_\` for \`-\`). Reads and searches may use either or both identities without asking which account unless the user restricts the account or scope.