From 9f43b5306dca66449cd4bf82b4229ed56c267adb Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Sun, 13 Sep 2026 18:45:50 +0300 Subject: [PATCH 01/25] 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 e85fd288..2bb89160 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 ed63bd65..892043ef 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 1639da23..660680a1 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 29a0687e..e92af132 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 9564268e..18c92088 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 167bfbe7..3edaeaa6 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 eb655d95..2f5fe2a0 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 30b7cbe8..ab1c849b 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 1a02ff83..59e4ba18 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 00000000..596c1616 --- /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 ef7f59c7..bcc0bb2f 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 00000000..c72ba29c --- /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 00000000..7fd4370b --- /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 2522a744..e4d9c5d0 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 00000000..7955a8d4 --- /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 e47cf737..debafeea 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 5d1abd2c..55435289 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 00000000..1267d2a7 --- /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 00000000..b882d6bf --- /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 f23eb66d..79e96edd 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 60d0ae1a..7a83319f 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 00000000..6e7eb217 --- /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 00000000..7bb255e7 --- /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 00000000..f3342dd6 --- /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 00000000..9dd9a5f5 --- /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 02/25] 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 2f5fe2a0..fded4543 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. From 09f184cfc921aca1c5b0990b806d51ba7b076555 Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Sun, 13 Sep 2026 18:49:05 +0300 Subject: [PATCH 03/25] fix(slack): preserve workspace context for question continuations Signed-off-by: Tiberiu Socaci --- TEST-PLAN.md | 2 +- src/slack/app.js | 2 +- src/slack/questions.js | 11 +++++++---- test/question-interactions.test.js | 21 ++++++++++++++++++++- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/TEST-PLAN.md b/TEST-PLAN.md index e92af132..18898ce2 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -5,7 +5,7 @@ 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 +pass 38 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. diff --git a/src/slack/app.js b/src/slack/app.js index debafeea..d1b5f843 100644 --- a/src/slack/app.js +++ b/src/slack/app.js @@ -1633,7 +1633,7 @@ async function connectAndWire(app) { }); for (const a of APPROVAL_ACTIONS) app.action(a, handleApprovalClick); registerBusyThreadChoiceActions(app, processMessageEvent); - registerQuestionActions(app, processMessageEvent); + registerQuestionActions(app, processMessageEvent, { botUserId, teamId }); 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/questions.js b/src/slack/questions.js index b882d6bf..12939ee1 100644 --- a/src/slack/questions.js +++ b/src/slack/questions.js @@ -204,8 +204,11 @@ export async function handleQuestionView({ ack, body, view = body?.view, client } } -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 })); +export function registerQuestionActions(app, processMessage, context = {}) { + // The live connection owns bot/workspace identity; the synthetic answer must retain it for + // mention hydration and Slack's recipient_team_id on streamed channel replies. + const continuation = (event, client, options) => processMessage(event, client, { ...context, ...options }); + app.action(/^cg_question_/, (payload) => handleQuestionAction(payload, { processMessage: continuation })); + app.view(QUESTION_FORM_CALLBACK, (payload) => handleQuestionView(payload, { processMessage: continuation })); + app.view(QUESTION_CUSTOM_CALLBACK, (payload) => handleQuestionView(payload, { processMessage: continuation })); } diff --git a/test/question-interactions.test.js b/test/question-interactions.test.js index 7bb255e7..0f817f6c 100644 --- a/test/question-interactions.test.js +++ b/test/question-interactions.test.js @@ -8,7 +8,7 @@ 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 { postQuestions, handleQuestionAction, handleQuestionView, refreshQuestionCard, registerQuestionActions } = await import("../src/slack/questions.js"); const { buildQuestionCard, buildQuestionModal, buildCustomAnswerModal } = await import("../src/slack/question-views.js"); const OWNER = "UQUESTION_OWNER"; @@ -79,6 +79,25 @@ test("posting is idempotent and visible card carries the persisted revision", as assert.equal(JSON.parse(elements(visible).find((el) => el.action_id === "cg_question_submit").value).revision, again.revision); }); +test("registered submit handlers preserve the live bot and workspace context", async () => { + const f = await fixture(); + await f.act("cg_question_choose:access:0"); + let handler; + let received; + registerQuestionActions({ action(_pattern, fn) { handler = fn; }, view() {} }, async (_event, _client, options) => { + received = options; + options.onQuestionSubmissionAccepted({ runId: randomUUID(), rec: f.context }); + }, { botUserId: "U_BOT_CONTEXT", teamId: "T_WORKSPACE_CONTEXT" }); + const record = f.current(); + const action = elements(buildQuestionCard(record)).find((el) => el.action_id === "cg_question_submit"); + await handler({ ack: async () => {}, action, client: f.client, body: { user: { id: OWNER }, channel: { id: CHANNEL }, message: { ts: record.messageTs } } }); + await settle(); + assert.equal(received.botUserId, "U_BOT_CONTEXT"); + assert.equal(received.teamId, "T_WORKSPACE_CONTEXT"); + assert.equal(received.bypassMention, true); + assert.equal(f.current().status, "submitted"); +}); + 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"); From 2e1da92e70ef46b3851ee08b1cf8b106cd3eadd3 Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Sun, 13 Sep 2026 21:43:45 +0300 Subject: [PATCH 04/25] Fix membership query encoding for Slack question cards Signed-off-by: Tiberiu Socaci --- CHANGELOG.md | 2 ++ TEST-PLAN.md | 5 +++- src/slack/questions.js | 14 +++++++---- test/question-interactions.test.js | 39 +++++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 892043ef..4fbc3abf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog — ChannelGate +- Fix Slack question-card posting by encoding channel membership checks as GET query parameters. + - 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. diff --git a/TEST-PLAN.md b/TEST-PLAN.md index 18898ce2..4612f33a 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -5,9 +5,12 @@ 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 38 tests using scratch SQLite, fake Slack interactions and fixture engines. They cover +pass 40 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. +Transport regression verifies GET-encoded membership queries (including pagination cursors), +JSON chat writes, authorization headers, and fail-closed API/HTTP errors. Live QST-01 must +post through the real MCP client so request-encoding failures cannot hide behind a fake client. 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 diff --git a/src/slack/questions.js b/src/slack/questions.js index 12939ee1..1ce8ef0b 100644 --- a/src/slack/questions.js +++ b/src/slack/questions.js @@ -6,17 +6,21 @@ import { buildQuestionCard, buildQuestionModal, buildCustomAnswerModal, parseQue // 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) => { + const call = async (method, body, read = false) => { 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), + // conversations.members requires query/form parameters; JSON POSTs lose its channel argument. + const url = new URL(`https://slack.com/api/${method}`); + if (read) url.search = new URLSearchParams(body).toString(); + const response = await fetchImpl(url.toString(), { + method: read ? "GET" : "POST", + headers: { Authorization: `Bearer ${token}`, ...(!read ? { "Content-Type": "application/json; charset=utf-8" } : {}) }, + ...(!read ? { 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) } }; + return { chat: { postMessage: (b) => call("chat.postMessage", b), update: (b) => call("chat.update", b) }, conversations: { members: (b) => call("conversations.members", b, true) } }; } export async function refreshQuestionCard(record, client) { diff --git a/test/question-interactions.test.js b/test/question-interactions.test.js index 0f817f6c..a4bdefd5 100644 --- a/test/question-interactions.test.js +++ b/test/question-interactions.test.js @@ -8,7 +8,7 @@ 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, registerQuestionActions } = await import("../src/slack/questions.js"); +const { postQuestions, handleQuestionAction, handleQuestionView, refreshQuestionCard, registerQuestionActions, questionSlackClient } = await import("../src/slack/questions.js"); const { buildQuestionCard, buildQuestionModal, buildCustomAnswerModal } = await import("../src/slack/question-views.js"); const OWNER = "UQUESTION_OWNER"; @@ -286,3 +286,40 @@ test("slow modal membership verification acknowledges an error before Slack's th assert.deepEqual(f.current().answers, {}); assert.equal(f.log.continuations.length, 0); }); + + +test("question Slack transport encodes member queries and preserves JSON chat writes", async () => { + const requests = []; + const client = questionSlackClient({ token: "test-only-token", fetchImpl: async (url, init) => { + requests.push({ url: new URL(url), init }); + return { ok: true, json: async () => ({ ok: true, members: ["UOWNER"] }) }; + } }); + await client.conversations.members({ channel: "CQUESTION", limit: 200, cursor: "next+/=&" }); + const { url, init } = requests[0]; + assert.equal(url.pathname, "/api/conversations.members"); + assert.equal(url.searchParams.get("channel"), "CQUESTION"); + assert.equal(url.searchParams.get("limit"), "200"); + assert.equal(url.searchParams.get("cursor"), "next+/=&"); + assert.equal(init.method, "GET"); + assert.equal(init.body, undefined); + assert.equal(init.headers.Authorization, "Bearer test-only-token"); + assert.equal(url.toString().includes("test-only-token"), false); + for (const method of ["postMessage", "update"]) { + const payload = { channel: "CQUESTION", text: "Demo" }; + await client.chat[method](payload); + const request = requests.at(-1); + assert.equal(request.url.pathname, `/api/chat.${method}`); + assert.equal(request.init.method, "POST"); + assert.deepEqual(JSON.parse(request.init.body), payload); + } +}); + +test("question Slack transport fails closed on API and HTTP errors", async () => { + for (const response of [ + { ok: true, json: async () => ({ ok: false, error: "invalid_arguments" }) }, + { ok: false, status: 503, json: async () => ({}) }, + ]) { + const client = questionSlackClient({ token: "test-only-token", fetchImpl: async () => response }); + await assert.rejects(client.conversations.members({ channel: "CQUESTION" }), /Slack conversations.members failed:/); + } +}); From 8e7ad486957bcb2f579e41fb4f5565573e0e19e1 Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Sun, 13 Sep 2026 22:03:36 +0300 Subject: [PATCH 05/25] fix: refresh Codex pricing and usage history Signed-off-by: Tiberiu Socaci --- CHANGELOG.md | 6 + FEATURES.md | 19 ++- TEST-PLAN.md | 26 +++- package.json | 1 + scripts/refresh-codex-pricing.mjs | 29 +++++ src/config/settings.js | 11 +- src/engines/adapters.js | 2 +- src/gateway/usage-pricing.js | 198 ++++++++++++++++++++++++++++++ src/gateway/usage.js | 9 +- src/server.js | 11 +- test/codex-rates.test.js | 26 ++-- test/engine-registry.test.js | 6 + test/usage-pricing.test.js | 114 +++++++++++++++++ 13 files changed, 429 insertions(+), 29 deletions(-) create mode 100644 scripts/refresh-codex-pricing.mjs create mode 100644 src/gateway/usage-pricing.js create mode 100644 test/usage-pricing.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fbc3abf..1e049b41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog — ChannelGate +- Refresh Codex Standard API-equivalent pricing from official OpenAI documentation: add + GPT-6 Astra at $10 / $1 cached / $50 per million tokens and reduce GPT-5.6 Sol (plus its + `gpt-5.6` alias) to $4 / $0.40 / $20. Keep the fallback picker aligned with the current Codex + CLI catalog, leave CLI-only Spark explicitly unpriced until an official rate exists, and + automatically back up and reprice component/request history since 2026-07-13 once on upgrade. + - Fix Slack question-card posting by encoding channel membership checks as GET query parameters. - Let agents ask clarification questions with Slack cards and paged forms: custom option buttons, diff --git a/FEATURES.md b/FEATURES.md index 660680a1..da7b9145 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -1466,7 +1466,10 @@ A categorized catalog of what's shipped. Cross-linked to `TEST-PLAN.md` checks. - Codex value is estimated, not billed spend: Settings exposes OpenAI Standard API-equivalent per-model `$/1M` rates. Root turns are priced from request-level rollout deltas and native child sessions are included without charging their copied fork prefix; Claude retains provider-reported - cost. The actual runtime model wins when a configured Claude model falls back to Codex. + cost. The actual runtime model wins when a configured Claude model falls back to Codex. The + fallback picker mirrors the current authenticated CLI catalog (`gpt-6-astra`, the GPT-5.6 + family, GPT-5.5 and CLI-only `gpt-5.3-codex-spark`); live discovery still wins. Spark remains + explicitly unpriced because OpenAI publishes no Standard API rate for that distinct model. - Session recovery: Codex auth/session state uses a stable, grant-free `CODEX_HOME` while private skills remain per-run under synthetic `HOME/.agents/skills`, so Codex 0.147+'s persisted rollout paths survive cleanup without leaking grants. Resuming a session that no longer exists (including @@ -2859,15 +2862,19 @@ are retired, bullet by bullet; everything else stands. no manual command. - **Per-model Codex Standard API-equivalent rates** (Settings → Behavior): Codex reports no dollar cost, so the ledger and reply footer estimate attribution value from an editable $/1M table — - input / cached-input / output per model (gpt-5.6-sol / gpt-5.6 alias / gpt-5.6-terra / + input / cached-input / output per model (gpt-6-astra, gpt-5.6-sol / gpt-5.6 alias / gpt-5.6-terra / gpt-5.6-luna, gpt-5.5, gpt-5.4, gpt-5.4-mini, gpt-5.4-nano, gpt-5.3-codex; defaults = - OpenAI's Standard pricing verified 2026-08-16). Cached reads are a subset of input and never + OpenAI's Standard pricing verified 2026-09-13). Cached reads are a subset of input and never double-counted; cache writes use 1.25× input, and eligible requests above 272K input use 2× input/cache plus 1.5× output. The threshold is evaluated per request, never against a turn aggregate. Official aliases/snapshots match on model boundaries; an unresolved CLI model remains - unpriced instead of being guessed. Retired full-table Terra/Luna defaults migrate to current rates - while genuine admin overrides survive. Claude runs are never priced with OpenAI rates. Legacy - blended rate remains a hidden last-resort fallback for explicitly unknown models. + unpriced instead of being guessed. Retired full-table GPT-5.6/Terra/Luna defaults migrate to + current rates while genuine admin overrides survive. On the first boot after this pricing basis + ships, the daemon backs up SQLite and reprices every request/component and its parent run since + 2026-07-13, after legacy accounting repair, so upgraded instances do not retain stale dashboard + history; the basis marker makes later boots no-ops. `npm run usage:reprice` previews the exact + rows/model deltas and `--apply` runs the same path manually. Claude runs are never priced with + OpenAI rates. Legacy blended rate remains a hidden last-resort fallback for explicitly unknown models. → TEST-PLAN: Observability. - Audit admin tab: monthly totals, per-channel rollups, and a recent-runs feed over the ledger + event log (`GET /api/audit`, `GET /api/audit/events`). No spend cap — visibility only. diff --git a/TEST-PLAN.md b/TEST-PLAN.md index 4612f33a..ad6057af 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -1437,6 +1437,10 @@ Google Workspace / Azure tenant and are unchecked until that drill runs. inside the refresh window, maps model-specific effort choices, and survives the next failed refresh with the last good snapshot; a cold failure retains the bundled fallback (`test/model-discovery.test.js`). +- [x] Automated: the bundled fallback matches the authenticated Codex CLI 0.153.4 catalog observed + 2026-09-13 (`gpt-6-astra`, GPT-5.6 Sol/Terra/Luna, GPT-5.5 and + `gpt-5.3-codex-spark`, plus the unresolved `codex` sentinel); retired picker entries and the + rejected `gpt-5.6` alias are absent (`test/engine-registry.test.js`). - [x] Automated: Claude's picker and browser fallback use rolling aliases (including `best`, `fable`, and `sonnet[1m]`); the Admin UI consumes the registry's model/effort manifests and keeps a valid saved same-engine custom ID available (`test/model-options.test.js`, @@ -5083,15 +5087,17 @@ Manual checks for the daemon-level behavior: ### Codex usage accounting and API-equivalent rates - [ ] Settings → Behavior shows the Codex/OpenAI rates table prefilled with the rates verified - 2026-08-16 against OpenAI Standard pricing + 2026-09-13 against OpenAI Standard pricing prices; editing a cell and saving persists it (reload shows the edited value; the others keep defaults). The old blended `$/1M` fallback input is not shown. - [ ] A Codex run's footer shows the estimated `$x.xx` API-equivalent value and Activity records the same figure with the estimated flag; a Claude run still shows the real `$` cost (never an OpenAI-rate estimate, even if total_cost_usd were missing). -- [x] Unit: Terra/Luna current defaults, retired-default migration with custom override preservation, - nested cache-read/cache-write pricing, official model-boundary matching, unresolved-model - behavior, and the 272K threshold applied per request (`test/codex-rates.test.js`). +- [x] Unit: GPT-6 Astra at $10/$1 cached/$50, GPT-5.6 Sol/alias at $4/$0.40/$20, + Terra/Luna current defaults, retired-default migration with custom override preservation, + nested cache-read/cache-write pricing, dated-snapshot-only inheritance, CLI-only Spark kept + unpriced, unresolved-model behavior, and the 272K threshold applied per request + (`test/codex-rates.test.js`). - [x] Unit: one provider session is serialized across gateway keys; aborted waiters do not strand the lock (`test/keyed-lock.test.js`). - [x] Unit: root rollout deltas, resumed baselines, actual runtime model/context metadata, child @@ -5108,6 +5114,18 @@ Manual checks for the daemon-level behavior: - [x] Unit: boot-time auto-repair triggers only when legacy codex rows exist past the last applied batch cutoff, applies the shared repair path with a backup, records a batch even when nothing matches, and never rescans settled history (`test/usage-repair.test.js`). +- [x] Unit: the 2026-09-13 pricing refresh selects only Codex usage at/after 2026-07-13, recomputes + request, component and parent-run values from stored cache/context/model evidence, preserves + older and Claude rows, leaves the internal `codex-auto-review` pseudo-model unpriced, writes + the new basis atomically, backs up once at boot, and skips the applied basis thereafter + (`test/usage-pricing.test.js`). +- [x] Live upgrade/history drill (Codex only, 2026-09-13): before upgrade run `npm run usage:reprice` and retain + its model/count/value summary; apply or restart the upgraded daemon, confirm the reported + backup opens, rerun the preview, and query the last-two-month ledger. Pass: every GPT-6 Astra + request/component is priced at $10/$1 cached/$50 with the per-request >272K uplift; every + GPT-5.6 Sol request/component uses $4/$0.40/$20; the parent run equals its priced component + sum; entries before 2026-07-13 and Claude/provider costs are byte-for-byte unchanged; only + `codex-auto-review` remains unpriced; a second boot changes no row. - [x] Unit: a DM resolves ONLY `composio-user` — the channel token and the organization default are both refused (`source: "none-dm"`) in Personal mode, and SDK mode mints no channel session at all; channels/mpims keep both identities (`test/composio-resolution.test.js`, diff --git a/package.json b/package.json index 2dfeaba1..acfc8970 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "restore:drill": "bash scripts/restore-drill.sh", "maintenance": "node scripts/runtime-maintenance.mjs", "usage:repair": "node scripts/repair-codex-usage-history.mjs", + "usage:reprice": "node scripts/refresh-codex-pricing.mjs", "release:artifacts": "node scripts/release-artifacts.mjs", "with-landing-lock": "node scripts/with-landing-lock.mjs --", "whisper:install": "node scripts/install-whisper.mjs", diff --git a/scripts/refresh-codex-pricing.mjs b/scripts/refresh-codex-pricing.mjs new file mode 100644 index 00000000..abac9b8a --- /dev/null +++ b/scripts/refresh-codex-pricing.mjs @@ -0,0 +1,29 @@ +#!/usr/bin/env node +// Dry-run by default. `--apply` backs up the gateway database and installs the current Codex +// Standard API-equivalent pricing basis over the declared historical window. Daemon boot runs the +// same idempotent refresh once after an upgraded instance repairs any legacy Codex accounting. +import { DatabaseSync } from "node:sqlite"; +import { dbFile } from "../src/config/paths.js"; +import { getDb } from "../src/db/index.js"; +import { backupGatewayDb } from "../src/gateway/usage-repair.js"; +import { + applyCodexPricingRefresh, + buildCodexPricingRefresh, + summarizeCodexPricingRefresh, +} from "../src/gateway/usage-pricing.js"; + +const apply = process.argv.includes("--apply"); +const sourcePath = dbFile(); +const source = new DatabaseSync(sourcePath, { readOnly: true }); +const plan = buildCodexPricingRefresh({ db: source }); +source.close(); +const summary = { mode: apply ? "apply" : "dry-run", database: sourcePath, ...summarizeCodexPricingRefresh(plan) }; + +if (!apply) { + console.log(JSON.stringify(summary, null, 2)); + process.exit(0); +} + +const backupPath = plan.usageRows ? await backupGatewayDb() : ""; +applyCodexPricingRefresh(getDb(), plan); +console.log(JSON.stringify({ ...summary, backupPath }, null, 2)); diff --git a/src/config/settings.js b/src/config/settings.js index 2b9798da..05e815e1 100644 --- a/src/config/settings.js +++ b/src/config/settings.js @@ -487,13 +487,14 @@ export function getCodexRatePer1MTokens() { } // Per-model Codex $/1M-token rates for the cost ESTIMATE (input / cached-input / output). -// Defaults verified against OpenAI's STANDARD API pricing table on 2026-08-16; admins can adjust +// Defaults verified against OpenAI's STANDARD API pricing table on 2026-09-13; admins can adjust // them in Settings → Integrations. `cachedInput` prices the cached_input_tokens subset of input. // Editable values are merged OVER these defaults, so a pricing change only needs the changed cell; // the model list itself is fixed and intentionally small. export const DEFAULT_CODEX_RATES = { - "gpt-5.6-sol": { input: 5, cachedInput: 0.5, output: 30 }, - "gpt-5.6": { input: 5, cachedInput: 0.5, output: 30 }, // alias for gpt-5.6-sol + "gpt-6-astra": { input: 10, cachedInput: 1, output: 50 }, + "gpt-5.6-sol": { input: 4, cachedInput: 0.4, output: 20 }, + "gpt-5.6": { input: 4, cachedInput: 0.4, output: 20 }, // alias for gpt-5.6-sol "gpt-5.6-terra": { input: 2, cachedInput: 0.2, output: 12 }, "gpt-5.6-luna": { input: 0.2, cachedInput: 0.02, output: 1.2 }, "gpt-5.5": { input: 5, cachedInput: 0.5, output: 30 }, @@ -505,9 +506,11 @@ export const DEFAULT_CODEX_RATES = { // The admin UI historically saved the complete displayed table, including untouched defaults. // When OpenAI changes a default, an old full snapshot would therefore shadow the corrected code -// forever. Treat only the two exact retired defaults as inherited values; genuinely customized +// forever. Treat only the exact retired defaults as inherited values; genuinely customized // cells (anything else) remain authoritative. A subsequent Settings save persists the new table. const RETIRED_CODEX_DEFAULTS = { + "gpt-5.6-sol": { input: 5, cachedInput: 0.5, output: 30 }, + "gpt-5.6": { input: 5, cachedInput: 0.5, output: 30 }, "gpt-5.6-terra": { input: 2.5, cachedInput: 0.25, output: 15 }, "gpt-5.6-luna": { input: 1, cachedInput: 0.1, output: 6 }, }; diff --git a/src/engines/adapters.js b/src/engines/adapters.js index 84dee860..0ca5d77a 100644 --- a/src/engines/adapters.js +++ b/src/engines/adapters.js @@ -199,7 +199,7 @@ const codex = validateEngineAdapter({ id: "codex", label: "Codex", cli: "codex", defaultModelKey: "defaultCodexModel", mcpMetaKey: "allowedCodexMcps", instructionFile: "AGENTS.md", skillsDir: ".agents/skills", mcpTransport: "argv", contextWindow: 272_000, efforts: ["none", "low", "medium", "high", "xhigh", "max", "ultra"], models: [ - ...["codex", "gpt-5.6-sol", "gpt-5.6", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.5", "gpt-5.4", "gpt-5.4-mini", "gpt-5.4-nano"].map((value) => ({ label: value === "codex" ? "Codex" : value.toUpperCase().replace("GPT-", "GPT-"), value, description: `${value} model.` })), + ...["codex", "gpt-6-astra", "gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.5", "gpt-5.3-codex-spark"].map((value) => ({ label: value === "codex" ? "Codex" : value.toUpperCase().replace("GPT-", "GPT-"), value, description: `${value} model.` })), ], mintsOwnSessionId: true, // The runner's own "the provider did not answer" kind (classifyCodexFailure), replayable in place. transientKinds: Object.freeze(["transient"]), diff --git a/src/gateway/usage-pricing.js b/src/gateway/usage-pricing.js new file mode 100644 index 00000000..f05a31dd --- /dev/null +++ b/src/gateway/usage-pricing.js @@ -0,0 +1,198 @@ +// One-time Codex pricing refreshes. The usage ledger stores token evidence and an estimated +// Standard API-equivalent dollar value; when OpenAI changes a published rate, updating only the +// forward estimator leaves the dashboard's existing period internally inconsistent. This module +// reprices the requested historical window from the stored component/request evidence, backs up +// the database first, and records a basis marker so every upgraded gateway applies it once. +import { CODEX_PRICING_BASIS, estimateCodexCost } from "./usage.js"; +import { backupGatewayDb } from "./usage-repair.js"; + +export const CODEX_PRICING_HISTORY_SINCE = "2026-07-13T00:00:00.000Z"; +export const CODEX_PRICING_META_KEY = "codex_usage_pricing_basis"; + +const tokenUsage = (row = {}) => ({ + input_tokens: Number(row.tokens_in) || 0, + cached_input_tokens: Number(row.tokens_cached) || 0, + cache_write_input_tokens: Number(row.tokens_cache_write) || 0, + output_tokens: Number(row.tokens_out) || 0, + reasoning_output_tokens: Number(row.reasoning_tokens) || 0, +}); + +const fixed = (value) => Number(Number(value || 0).toFixed(6)); + +export function pendingCodexPricingRefresh(db, { basis = CODEX_PRICING_BASIS } = {}) { + const appliedBasis = String(db.prepare("SELECT value FROM _meta WHERE key = ?").get(CODEX_PRICING_META_KEY)?.value || ""); + if (appliedBasis !== basis) return { pending: true, appliedBasis, basis }; + // A manual refresh can race an older daemon that is still recording turns. Once the marker is + // current, cheaply inspect only components written with another basis and refresh again if one + // of their models is now priceable. Permanently-unpriced internal pseudo-models stay ignored. + const stale = db.prepare( + `SELECT c.model, c.tokens_in, c.tokens_cached, c.tokens_cache_write, c.tokens_out, c.reasoning_tokens + FROM usage_components c + JOIN usage u ON u.id = c.usage_id + WHERE u.engine = 'codex' AND u.ts >= ? AND c.pricing_basis != ?` + ).all(CODEX_PRICING_HISTORY_SINCE, basis); + const rateableStaleComponent = stale.some((component) => ( + estimateCodexCost(tokenUsage(component), component.model).costUSD != null + )); + return { pending: rateableStaleComponent, appliedBasis, basis }; +} + +export function buildCodexPricingRefresh({ + db, + since = CODEX_PRICING_HISTORY_SINCE, + basis = CODEX_PRICING_BASIS, +} = {}) { + const components = db.prepare( + `SELECT c.* + FROM usage_components c + JOIN usage u ON u.id = c.usage_id + WHERE u.engine = 'codex' AND u.ts >= ? + ORDER BY c.id` + ).all(since); + const requests = db.prepare( + `SELECT r.* + FROM usage_requests r + JOIN usage_components c ON c.id = r.component_id + JOIN usage u ON u.id = c.usage_id + WHERE u.engine = 'codex' AND u.ts >= ? + ORDER BY r.component_id, r.request_index` + ).all(since); + const requestsByComponent = new Map(); + for (const request of requests) { + const list = requestsByComponent.get(Number(request.component_id)) || []; + list.push(request); + requestsByComponent.set(Number(request.component_id), list); + } + + const componentUpdates = []; + const usageCosts = new Map(); + const models = new Map(); + let storedEstimatedValue = 0; + let repricedEstimatedValue = 0; + let unpricedComponents = 0; + for (const component of components) { + const detail = requestsByComponent.get(Number(component.id)) || []; + const requestEvidence = detail.map((request) => ({ + model: request.model || component.model, + usage: tokenUsage(request), + })); + const estimate = estimateCodexCost(tokenUsage(component), component.model, requestEvidence); + const requestUpdates = detail.map((request) => ({ + id: Number(request.id), + costUSD: estimateCodexCost(tokenUsage(request), request.model || component.model, [{ + model: request.model || component.model, + usage: tokenUsage(request), + }]).costUSD, + })); + const update = { + id: Number(component.id), + usageId: Number(component.usage_id), + model: String(component.model || ""), + costUSD: estimate.costUSD, + costEstimated: estimate.estimated, + pricingBasis: estimate.estimated ? basis : "unpriced", + requestUpdates, + }; + componentUpdates.push(update); + + const usage = usageCosts.get(update.usageId) || { costUSD: 0, pricedComponents: 0 }; + if (update.costUSD != null) { + usage.costUSD += update.costUSD; + usage.pricedComponents += 1; + repricedEstimatedValue += update.costUSD; + } else { + unpricedComponents += 1; + } + usageCosts.set(update.usageId, usage); + + const model = models.get(update.model) || { components: 0, storedValue: 0, repricedValue: 0, unpricedComponents: 0 }; + model.components += 1; + if (component.cost_usd != null) { + const old = Number(component.cost_usd) || 0; + storedEstimatedValue += old; + model.storedValue += old; + } + if (update.costUSD == null) model.unpricedComponents += 1; + else model.repricedValue += update.costUSD; + models.set(update.model, model); + } + + const usageUpdates = [...usageCosts].map(([id, value]) => ({ + id, + costUSD: value.pricedComponents ? fixed(value.costUSD) : null, + costEstimated: value.pricedComponents > 0, + })); + const modelSummary = Object.fromEntries([...models].map(([model, value]) => [model, { + components: value.components, + storedValue: fixed(value.storedValue), + repricedValue: fixed(value.repricedValue), + delta: fixed(value.repricedValue - value.storedValue), + unpricedComponents: value.unpricedComponents, + }])); + + return { + basis, + since, + usageRows: usageUpdates.length, + components: componentUpdates.length, + requests: requests.length, + unpricedComponents, + storedEstimatedValue: fixed(storedEstimatedValue), + repricedEstimatedValue: fixed(repricedEstimatedValue), + delta: fixed(repricedEstimatedValue - storedEstimatedValue), + models: modelSummary, + componentUpdates, + usageUpdates, + }; +} + +export function summarizeCodexPricingRefresh(plan = {}) { + const { + basis = CODEX_PRICING_BASIS, + since = CODEX_PRICING_HISTORY_SINCE, + usageRows = 0, + components = 0, + requests = 0, + unpricedComponents = 0, + storedEstimatedValue = 0, + repricedEstimatedValue = 0, + delta = 0, + models = {}, + } = plan; + return { basis, since, usageRows, components, requests, unpricedComponents, storedEstimatedValue, repricedEstimatedValue, delta, models }; +} + +export function applyCodexPricingRefresh(db, plan) { + const updateRequest = db.prepare("UPDATE usage_requests SET cost_usd = ? WHERE id = ?"); + const updateComponent = db.prepare( + "UPDATE usage_components SET cost_usd = ?, cost_estimated = ?, pricing_basis = ? WHERE id = ?" + ); + const updateUsage = db.prepare("UPDATE usage SET cost_usd = ?, cost_estimated = ? WHERE id = ?"); + db.exec("BEGIN IMMEDIATE"); + try { + for (const component of plan.componentUpdates || []) { + for (const request of component.requestUpdates || []) updateRequest.run(request.costUSD, request.id); + updateComponent.run(component.costUSD, component.costEstimated ? 1 : 0, component.pricingBasis, component.id); + } + for (const usage of plan.usageUpdates || []) updateUsage.run(usage.costUSD, usage.costEstimated ? 1 : 0, usage.id); + db.prepare( + "INSERT INTO _meta(key, value) VALUES(?, ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value" + ).run(CODEX_PRICING_META_KEY, plan.basis || CODEX_PRICING_BASIS); + db.exec("COMMIT"); + } catch (error) { + try { db.exec("ROLLBACK"); } catch { /* already rolled back */ } + throw error; + } + return summarizeCodexPricingRefresh(plan); +} + +export async function autoRefreshCodexPricing({ db, makeBackup, log = console } = {}) { + const handle = db || (await import("../db/index.js")).getDb(); + const state = pendingCodexPricingRefresh(handle); + if (!state.pending) return { applied: false, basis: state.basis }; + const plan = buildCodexPricingRefresh({ db: handle }); + const backupPath = plan.usageRows ? await (makeBackup || backupGatewayDb)() : ""; + const summary = applyCodexPricingRefresh(handle, plan); + log.log?.(`[usage] Codex pricing refresh ${summary.basis}: ${summary.usageRows} run(s), ${summary.components} component(s), ${summary.requests} request(s); value ${summary.storedEstimatedValue} → ${summary.repricedEstimatedValue}${backupPath ? `; backup ${backupPath}` : ""}`); + return { applied: true, backupPath, ...summary }; +} diff --git a/src/gateway/usage.js b/src/gateway/usage.js index 47f142ac..486c492e 100644 --- a/src/gateway/usage.js +++ b/src/gateway/usage.js @@ -18,13 +18,16 @@ import { normalizeCodexTokenUsage } from "../engines/codex-usage.js"; // exact key → official dated-snapshot prefix. Empty/`codex` is a CLI sentinel, not an official API // model id, so it stays unpriced until the runtime model is resolved. The legacy blended rate is // retained only as an explicit admin fallback for an unknown non-empty runtime model. -const LONG_CONTEXT_RATE_KEYS = new Set(["gpt-5.6-sol", "gpt-5.6", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.5", "gpt-5.4"]); +export const CODEX_PRICING_BASIS = "openai-standard-2026-09-13"; +const LONG_CONTEXT_RATE_KEYS = new Set(["gpt-6-astra", "gpt-5.6-sol", "gpt-5.6", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.5", "gpt-5.4"]); function codexRateKey(rates, model) { const m = String(model || "").toLowerCase(); if (!m || m === "codex") return ""; if (rates[m]) return m; return Object.keys(rates) - .filter((key) => m.startsWith(`${key}-`)) + // Only inherit a base rate for an official dated snapshot. A named sibling such as + // gpt-5.3-codex-spark is a different model and stays unpriced until OpenAI publishes its rate. + .filter((key) => m.startsWith(`${key}-`) && /^\d{4}(?:-\d{2}){1,2}$/.test(m.slice(key.length + 1))) .sort((a, b) => b.length - a.length)[0] || ""; } @@ -132,7 +135,7 @@ export function componentRow(accounting, { sourceKind = "root", parentSourceId = requests, costUSD: estimate.costUSD, costEstimated: estimate.estimated, - pricingBasis: estimate.estimated ? "openai-standard-2026-08-16" : "unpriced", + pricingBasis: estimate.estimated ? CODEX_PRICING_BASIS : "unpriced", provenance: accounting?.provenance || (sourceKind === "root" ? "codex-rollout-root-delta" : "codex-rollout-fork-delta"), confidence: accounting?.exactRequests === false ? "verified-total" : "verified-requests", durationMs: accounting?.durationMs ?? durationMs, diff --git a/src/server.js b/src/server.js index ae607484..6bb1db95 100644 --- a/src/server.js +++ b/src/server.js @@ -45,6 +45,7 @@ import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import path from "node:path"; import { installConsoleRedaction } from "./util/redact.js"; import { autoRepairCodexUsageHistory } from "./gateway/usage-repair.js"; +import { autoRefreshCodexPricing } from "./gateway/usage-pricing.js"; import { postNotice } from "./platforms/notify.js"; import { startLicenseVerification } from "./ee/license.js"; import { startUsageReporting } from "./ee/limits.js"; @@ -347,14 +348,18 @@ async function main() { // Scheduled two-way Google Drive ↔ channel-folder sync (dormant unless enabled + configured). startDriveSync(); - // One-shot Codex usage-history repair. After an update introduces accounting schema v10 (which + // One-shot Codex usage-history maintenance. After an update introduces accounting schema v10 (which // marks pre-existing codex rows legacy-unverified), reconstruct per-turn + subagent usage from // surviving rollouts — the same idempotent path as `npm run usage:repair -- --apply`, with a DB // backup first. Fire-and-forget: rollout scanning can take a while and must not delay Slack. // Records a batch even when nothing matches, so later boots see nothing pending and skip. autoRepairCodexUsageHistory() - .then((r) => { if (!r.applied) return; console.log(`[gateway] usage history auto-repair done (batch ${r.batchId})`); }) - .catch((e) => console.error("[gateway] usage auto-repair failed:", e?.message || e)); + .then((r) => { + if (r.applied) console.log(`[gateway] usage history auto-repair done (batch ${r.batchId})`); + return autoRefreshCodexPricing(); + }) + .then((r) => { if (r.applied) console.log(`[gateway] Codex pricing history refresh done (${r.basis})`); }) + .catch((e) => console.error("[gateway] usage maintenance failed:", e?.message || e)); } for (const sig of ["SIGINT", "SIGTERM"]) { diff --git a/test/codex-rates.test.js b/test/codex-rates.test.js index e3d8a08b..1c259c80 100644 --- a/test/codex-rates.test.js +++ b/test/codex-rates.test.js @@ -12,7 +12,8 @@ const { DEFAULT_CODEX_RATES, getCodexModelRates, saveSettings } = await import(" test("default rates cover exactly the fixed model list", () => { const rates = getCodexModelRates(); assert.deepEqual(Object.keys(rates).sort(), Object.keys(DEFAULT_CODEX_RATES).sort()); - assert.equal(rates["gpt-5.6-sol"].input, 5); + assert.deepEqual(rates["gpt-6-astra"], { input: 10, cachedInput: 1, output: 50 }); + assert.deepEqual(rates["gpt-5.6-sol"], { input: 4, cachedInput: 0.4, output: 20 }); assert.equal(rates["gpt-5.6-terra"].output, 12); assert.equal(rates["gpt-5.6-luna"].cachedInput, 0.02); assert.equal(rates["gpt-5.5"].input, 5); @@ -41,12 +42,13 @@ test("matches official model variants by boundary and leaves an unresolved CLI s test("applies long-context rates per request, never to a turn aggregate", () => { const aggregate = { input_tokens: 400_000, cached_input_tokens: 200_000, output_tokens: 10_000 }; - assert.equal(estimateCodexCost(aggregate, "gpt-5.6-sol").costUSD, 1.4); - assert.equal(estimateCodexCost(aggregate, "gpt-5.6-sol", [{ usage: aggregate }]).costUSD, 2.65); + assert.equal(estimateCodexCost(aggregate, "gpt-5.6-sol").costUSD, 1.08); + assert.equal(estimateCodexCost(aggregate, "gpt-5.6-sol", [{ usage: aggregate }]).costUSD, 2.06); const atThreshold = { input_tokens: 272_000, output_tokens: 0 }; const overThreshold = { input_tokens: 272_001, output_tokens: 0 }; - assert.equal(estimateCodexCost(atThreshold, "gpt-5.6-sol", [{ usage: atThreshold }]).costUSD, 1.36); - assert.equal(estimateCodexCost(overThreshold, "gpt-5.6-sol", [{ usage: overThreshold }]).costUSD, 2.72001); + assert.equal(estimateCodexCost(atThreshold, "gpt-5.6-sol", [{ usage: atThreshold }]).costUSD, 1.088); + assert.equal(estimateCodexCost(overThreshold, "gpt-5.6-sol", [{ usage: overThreshold }]).costUSD, 2.176008); + assert.equal(estimateCodexCost(overThreshold, "gpt-6-astra", [{ usage: overThreshold }]).costUSD, 5.44002); }); test("supports nested cached/cache-write details and clamps malformed subsets", () => { @@ -55,10 +57,10 @@ test("supports nested cached/cache-write details and clamps malformed subsets", input_tokens_details: { cached_tokens: 400_000, cache_write_tokens: 100_000 }, output_tokens: 0, }; - // 500k full × $5 + 400k cached × $.50 + 100k write × $6.25 = $3.325. - assert.equal(estimateCodexCost(usage, "gpt-5.6-sol").costUSD, 3.325); + // 500k full × $4 + 400k cached × $.40 + 100k write × $5 = $2.66. + assert.equal(estimateCodexCost(usage, "gpt-5.6-sol").costUSD, 2.66); const malformed = { input_tokens: 10, input_tokens_details: { cached_tokens: 20, cache_write_tokens: 20 } }; - assert.equal(estimateCodexCost(malformed, "gpt-5.6-sol").costUSD, 0.000005); + assert.equal(estimateCodexCost(malformed, "gpt-5.6-sol").costUSD, 0.000004); }); test("normalizeUsage estimates only for codex; claude without cost stays null", () => { @@ -76,9 +78,13 @@ test("normalizeUsage estimates only for codex; claude without cost stays null", test("retired full-table defaults migrate while genuine admin overrides survive", () => { saveSettings({ codexModelRates: { + "gpt-5.6-sol": { input: 5, cachedInput: 0.5, output: 30 }, + "gpt-5.6": { input: 5, cachedInput: 0.5, output: 30 }, "gpt-5.6-terra": { input: 2.5, cachedInput: 0.25, output: 15 }, "gpt-5.6-luna": { input: 1, cachedInput: 0.1, output: 6 }, } }); + assert.deepEqual(getCodexModelRates()["gpt-5.6-sol"], { input: 4, cachedInput: 0.4, output: 20 }); + assert.deepEqual(getCodexModelRates()["gpt-5.6"], { input: 4, cachedInput: 0.4, output: 20 }); assert.deepEqual(getCodexModelRates()["gpt-5.6-terra"], { input: 2, cachedInput: 0.2, output: 12 }); assert.deepEqual(getCodexModelRates()["gpt-5.6-luna"], { input: 0.2, cachedInput: 0.02, output: 1.2 }); saveSettings({ codexModelRates: { @@ -86,3 +92,7 @@ test("retired full-table defaults migrate while genuine admin overrides survive" } }); assert.deepEqual(getCodexModelRates()["gpt-5.6-terra"], { input: 3, cachedInput: 0.3, output: 18 }); }); + +test("keeps CLI-only models without published API rates explicitly unpriced", () => { + assert.equal(estimateCodexCost({ input_tokens: 1_000_000 }, "gpt-5.3-codex-spark").costUSD, null); +}); diff --git a/test/engine-registry.test.js b/test/engine-registry.test.js index 8c5fdd29..c23aa23c 100644 --- a/test/engine-registry.test.js +++ b/test/engine-registry.test.js @@ -29,6 +29,12 @@ test("all shipped engines are registered with the facts callers depend on", () = } }); +test("Codex fallback models match the current selectable CLI catalog", () => { + assert.deepEqual(adapterFor("codex").models.map((model) => model.value), [ + "codex", "gpt-6-astra", "gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.5", "gpt-5.3-codex-spark", + ]); +}); + test("an unregistered engine is detectable rather than silently treated as Claude", () => { assert.equal(isEngineId(UNKNOWN), false); assert.equal(adapterFor(UNKNOWN), null, "the honest answer is 'I don't know this engine'"); diff --git a/test/usage-pricing.test.js b/test/usage-pricing.test.js new file mode 100644 index 00000000..337a584a --- /dev/null +++ b/test/usage-pricing.test.js @@ -0,0 +1,114 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { DatabaseSync } from "node:sqlite"; +import { ensureTestEnv } from "./helpers.js"; + +ensureTestEnv(); +const { runMigrations } = await import("../src/db/index.js"); +const { + CODEX_PRICING_HISTORY_SINCE, + applyCodexPricingRefresh, + autoRefreshCodexPricing, + buildCodexPricingRefresh, + pendingCodexPricingRefresh, +} = await import("../src/gateway/usage-pricing.js"); +const { CODEX_PRICING_BASIS } = await import("../src/gateway/usage.js"); + +function fixture() { + const db = new DatabaseSync(":memory:"); + db.exec("CREATE TABLE _meta (key TEXT PRIMARY KEY, value TEXT)"); + runMigrations(db); + const usage = db.prepare( + `INSERT INTO usage(ts, channel_id, slug, author_id, engine, model, task_kind, + tokens_in, tokens_out, cost_usd, cost_estimated, duration_ms, runtime_model, accounting_status) + VALUES(?, 'C', 'pricing', 'U', 'codex', ?, 'interactive', ?, ?, ?, 1, 1, ?, 'verified')` + ); + const component = db.prepare( + `INSERT INTO usage_components(usage_id, source_key, source_kind, model, tokens_in, + tokens_cached, tokens_cache_write, tokens_out, cost_usd, cost_estimated, pricing_basis) + VALUES(?, ?, 'root', ?, ?, ?, ?, ?, ?, 1, 'openai-standard-2026-08-16')` + ); + const request = db.prepare( + `INSERT INTO usage_requests(component_id, request_index, model, tokens_in, tokens_cached, + tokens_cache_write, tokens_out, reasoning_tokens, context_window, long_context, cost_usd) + VALUES(?, 0, ?, ?, ?, 0, ?, 0, 1050000, ?, ?)` + ); + + const sol = usage.run("2026-09-01T00:00:00.000Z", "gpt-5.6-sol", 300_000, 1_000, 2.145, "gpt-5.6-sol"); + const solComponent = component.run(Number(sol.lastInsertRowid), "sol", "gpt-5.6-sol", 300_000, 100_000, 0, 1_000, 2.145); + request.run(Number(solComponent.lastInsertRowid), "gpt-5.6-sol", 300_000, 100_000, 1_000, 1, 2.145); + component.run(Number(sol.lastInsertRowid), "review", "codex-auto-review", 1_000, 0, 0, 10, null); + + const astra = usage.run("2026-09-10T00:00:00.000Z", "gpt-6-astra", 1_000_000, 10_000, null, "gpt-6-astra"); + component.run(Number(astra.lastInsertRowid), "astra", "gpt-6-astra", 1_000_000, 500_000, 0, 10_000, null); + + const old = usage.run("2026-07-12T23:59:59.000Z", "gpt-5.6-sol", 100, 1, 9.99, "gpt-5.6-sol"); + component.run(Number(old.lastInsertRowid), "before-window", "gpt-5.6-sol", 100, 0, 0, 1, 9.99); + + const claude = db.prepare( + `INSERT INTO usage(ts, channel_id, slug, author_id, engine, model, task_kind, + tokens_in, tokens_out, cost_usd, cost_estimated, duration_ms, runtime_model, accounting_status) + VALUES('2026-09-11T00:00:00.000Z', 'C', 'pricing', 'U', 'claude', 'claude-opus-4-1', + 'interactive', 100, 10, 8.88, 0, 1, 'claude-opus-4-1', 'verified')` + ).run(); + component.run(Number(claude.lastInsertRowid), "claude", "claude-opus-4-1", 100, 0, 0, 10, 8.88); + return db; +} + +test("pricing refresh reprices the two-month evidence window and records an idempotent basis", () => { + const db = fixture(); + assert.deepEqual(pendingCodexPricingRefresh(db), { pending: true, appliedBasis: "", basis: CODEX_PRICING_BASIS }); + const plan = buildCodexPricingRefresh({ db }); + assert.equal(plan.since, CODEX_PRICING_HISTORY_SINCE); + assert.equal(plan.usageRows, 2); + assert.equal(plan.components, 3); + assert.equal(plan.requests, 1); + assert.equal(plan.unpricedComponents, 1); + assert.equal(plan.models["gpt-5.6-sol"].repricedValue, 1.71); + assert.equal(plan.models["gpt-6-astra"].repricedValue, 6); + assert.equal(plan.models["codex-auto-review"].unpricedComponents, 1); + + applyCodexPricingRefresh(db, plan); + assert.deepEqual(pendingCodexPricingRefresh(db), { pending: false, appliedBasis: CODEX_PRICING_BASIS, basis: CODEX_PRICING_BASIS }); + assert.deepEqual(db.prepare("SELECT source_key, cost_usd, cost_estimated, pricing_basis FROM usage_components ORDER BY id").all().map((row) => ({ ...row })), [ + { source_key: "sol", cost_usd: 1.71, cost_estimated: 1, pricing_basis: CODEX_PRICING_BASIS }, + { source_key: "review", cost_usd: null, cost_estimated: 0, pricing_basis: "unpriced" }, + { source_key: "astra", cost_usd: 6, cost_estimated: 1, pricing_basis: CODEX_PRICING_BASIS }, + { source_key: "before-window", cost_usd: 9.99, cost_estimated: 1, pricing_basis: "openai-standard-2026-08-16" }, + { source_key: "claude", cost_usd: 8.88, cost_estimated: 1, pricing_basis: "openai-standard-2026-08-16" }, + ]); + assert.deepEqual(db.prepare("SELECT model, cost_usd FROM usage ORDER BY id").all().map((row) => ({ ...row })), [ + { model: "gpt-5.6-sol", cost_usd: 1.71 }, + { model: "gpt-6-astra", cost_usd: 6 }, + { model: "gpt-5.6-sol", cost_usd: 9.99 }, + { model: "claude-opus-4-1", cost_usd: 8.88 }, + ]); + assert.equal(db.prepare("SELECT cost_usd FROM usage_requests").get().cost_usd, 1.71); + + db.prepare("UPDATE usage_components SET pricing_basis = 'unpriced' WHERE source_key = 'astra'").run(); + assert.deepEqual(pendingCodexPricingRefresh(db), { + pending: true, + appliedBasis: CODEX_PRICING_BASIS, + basis: CODEX_PRICING_BASIS, + }); + db.close(); +}); + +test("boot pricing refresh backs up once and then skips the applied basis", async () => { + const db = fixture(); + let backups = 0; + const first = await autoRefreshCodexPricing({ + db, + makeBackup: async () => { backups += 1; return "pricing-backup"; }, + log: { log: () => {} }, + }); + assert.equal(first.applied, true); + assert.equal(first.backupPath, "pricing-backup"); + assert.equal(backups, 1); + assert.deepEqual(await autoRefreshCodexPricing({ db, makeBackup: async () => { backups += 1; } }), { + applied: false, + basis: CODEX_PRICING_BASIS, + }); + assert.equal(backups, 1); + db.close(); +}); From 8d6251284cfa209ecd9698988cb0c88f9c4c41ca Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Mon, 14 Sep 2026 01:34:51 +0300 Subject: [PATCH 06/25] fix: honor Codex pricing effective dates Signed-off-by: Tiberiu Socaci --- CHANGELOG.md | 4 ++++ FEATURES.md | 4 +++- TEST-PLAN.md | 8 +++++++- src/gateway/usage-pricing.js | 37 +++++++++++++++++++++++++++++++----- src/gateway/usage.js | 5 ++--- test/usage-pricing.test.js | 19 ++++++++++++------ 6 files changed, 61 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e049b41..729c06c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog — ChannelGate +- Correct Codex history repricing to honor OpenAI's effective date for GPT-5.6 Sol: retain the + original $5 / $0.50 cached / $30 rate before 2026-08-21 and apply $4 / $0.40 / $20 from that + date onward. Upgraded instances rerun the backup-first correction under a new pricing basis. + - Refresh Codex Standard API-equivalent pricing from official OpenAI documentation: add GPT-6 Astra at $10 / $1 cached / $50 per million tokens and reduce GPT-5.6 Sol (plus its `gpt-5.6` alias) to $4 / $0.40 / $20. Keep the fallback picker aligned with the current Codex diff --git a/FEATURES.md b/FEATURES.md index da7b9145..1cf8b0f5 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -2872,7 +2872,9 @@ are retired, bullet by bullet; everything else stands. current rates while genuine admin overrides survive. On the first boot after this pricing basis ships, the daemon backs up SQLite and reprices every request/component and its parent run since 2026-07-13, after legacy accounting repair, so upgraded instances do not retain stale dashboard - history; the basis marker makes later boots no-ops. `npm run usage:reprice` previews the exact + history. Historical pricing follows official effective dates: GPT-5.6 Sol retains + $5/$0.50/$30 before 2026-08-21 and uses $4/$0.40/$20 from that date. The basis marker makes later + boots no-ops. `npm run usage:reprice` previews the exact rows/model deltas and `--apply` runs the same path manually. Claude runs are never priced with OpenAI rates. Legacy blended rate remains a hidden last-resort fallback for explicitly unknown models. → TEST-PLAN: Observability. diff --git a/TEST-PLAN.md b/TEST-PLAN.md index ad6057af..a21214e7 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -5117,8 +5117,14 @@ Manual checks for the daemon-level behavior: - [x] Unit: the 2026-09-13 pricing refresh selects only Codex usage at/after 2026-07-13, recomputes request, component and parent-run values from stored cache/context/model evidence, preserves older and Claude rows, leaves the internal `codex-auto-review` pseudo-model unpriced, writes - the new basis atomically, backs up once at boot, and skips the applied basis thereafter + the new basis atomically, applies GPT-5.6 Sol's $5/$0.50/$30 rate before the official + 2026-08-21 cutover and $4/$0.40/$20 at/after it, backs up once at boot, and skips the applied basis thereafter (`test/usage-pricing.test.js`). +- [x] Live dated-pricing correction (2026-09-14): apply the new basis to a production copy and the live ledger, + confirm every GPT-5.6 Sol component before 2026-08-21 uses $5/$0.50/$30 while every component + at/after the cutover uses $4/$0.40/$20, Astra remains $10/$1/$50, parent totals match their + priced components, non-Codex and pre-window rows are unchanged, both backups pass + `quick_check`, and a second dry run reports zero delta. - [x] Live upgrade/history drill (Codex only, 2026-09-13): before upgrade run `npm run usage:reprice` and retain its model/count/value summary; apply or restart the upgraded daemon, confirm the reported backup opens, rerun the preview, and query the last-two-month ledger. Pass: every GPT-6 Astra diff --git a/src/gateway/usage-pricing.js b/src/gateway/usage-pricing.js index f05a31dd..f3798e5f 100644 --- a/src/gateway/usage-pricing.js +++ b/src/gateway/usage-pricing.js @@ -5,10 +5,15 @@ // the database first, and records a basis marker so every upgraded gateway applies it once. import { CODEX_PRICING_BASIS, estimateCodexCost } from "./usage.js"; import { backupGatewayDb } from "./usage-repair.js"; +import { DEFAULT_CODEX_RATES, getCodexModelRates } from "../config/settings.js"; export const CODEX_PRICING_HISTORY_SINCE = "2026-07-13T00:00:00.000Z"; +export const CODEX_SOL_PRICE_CUTOVER = "2026-08-21T00:00:00.000Z"; export const CODEX_PRICING_META_KEY = "codex_usage_pricing_basis"; +const RETIRED_SOL_RATES = Object.freeze({ input: 5, cachedInput: 0.5, output: 30 }); +const SOL_RATE_KEYS = Object.freeze(["gpt-5.6-sol", "gpt-5.6"]); + const tokenUsage = (row = {}) => ({ input_tokens: Number(row.tokens_in) || 0, cached_input_tokens: Number(row.tokens_cached) || 0, @@ -19,6 +24,24 @@ const tokenUsage = (row = {}) => ({ const fixed = (value) => Number(Number(value || 0).toFixed(6)); +const sameRate = (left, right) => ( + left?.input === right?.input + && left?.cachedInput === right?.cachedInput + && left?.output === right?.output +); + +function codexRatesAt(ts) { + const current = getCodexModelRates(); + if (String(ts || "") >= CODEX_SOL_PRICE_CUTOVER) return current; + const historical = { ...current }; + for (const key of SOL_RATE_KEYS) { + // Respect genuine admin overrides. Only replace the shipped current default with the official + // rate that preceded OpenAI's August 21 reduction. + if (sameRate(current[key], DEFAULT_CODEX_RATES[key])) historical[key] = RETIRED_SOL_RATES; + } + return historical; +} + export function pendingCodexPricingRefresh(db, { basis = CODEX_PRICING_BASIS } = {}) { const appliedBasis = String(db.prepare("SELECT value FROM _meta WHERE key = ?").get(CODEX_PRICING_META_KEY)?.value || ""); if (appliedBasis !== basis) return { pending: true, appliedBasis, basis }; @@ -26,13 +49,16 @@ export function pendingCodexPricingRefresh(db, { basis = CODEX_PRICING_BASIS } = // current, cheaply inspect only components written with another basis and refresh again if one // of their models is now priceable. Permanently-unpriced internal pseudo-models stay ignored. const stale = db.prepare( - `SELECT c.model, c.tokens_in, c.tokens_cached, c.tokens_cache_write, c.tokens_out, c.reasoning_tokens + `SELECT c.model, c.tokens_in, c.tokens_cached, c.tokens_cache_write, c.tokens_out, c.reasoning_tokens, + u.ts AS usage_ts FROM usage_components c JOIN usage u ON u.id = c.usage_id WHERE u.engine = 'codex' AND u.ts >= ? AND c.pricing_basis != ?` ).all(CODEX_PRICING_HISTORY_SINCE, basis); const rateableStaleComponent = stale.some((component) => ( - estimateCodexCost(tokenUsage(component), component.model).costUSD != null + estimateCodexCost(tokenUsage(component), component.model, [], { + rates: codexRatesAt(component.usage_ts), + }).costUSD != null )); return { pending: rateableStaleComponent, appliedBasis, basis }; } @@ -43,7 +69,7 @@ export function buildCodexPricingRefresh({ basis = CODEX_PRICING_BASIS, } = {}) { const components = db.prepare( - `SELECT c.* + `SELECT c.*, u.ts AS usage_ts FROM usage_components c JOIN usage u ON u.id = c.usage_id WHERE u.engine = 'codex' AND u.ts >= ? @@ -71,18 +97,19 @@ export function buildCodexPricingRefresh({ let repricedEstimatedValue = 0; let unpricedComponents = 0; for (const component of components) { + const rates = codexRatesAt(component.usage_ts); const detail = requestsByComponent.get(Number(component.id)) || []; const requestEvidence = detail.map((request) => ({ model: request.model || component.model, usage: tokenUsage(request), })); - const estimate = estimateCodexCost(tokenUsage(component), component.model, requestEvidence); + const estimate = estimateCodexCost(tokenUsage(component), component.model, requestEvidence, { rates }); const requestUpdates = detail.map((request) => ({ id: Number(request.id), costUSD: estimateCodexCost(tokenUsage(request), request.model || component.model, [{ model: request.model || component.model, usage: tokenUsage(request), - }]).costUSD, + }], { rates }).costUSD, })); const update = { id: Number(component.id), diff --git a/src/gateway/usage.js b/src/gateway/usage.js index 486c492e..6cd9cb92 100644 --- a/src/gateway/usage.js +++ b/src/gateway/usage.js @@ -18,7 +18,7 @@ import { normalizeCodexTokenUsage } from "../engines/codex-usage.js"; // exact key → official dated-snapshot prefix. Empty/`codex` is a CLI sentinel, not an official API // model id, so it stays unpriced until the runtime model is resolved. The legacy blended rate is // retained only as an explicit admin fallback for an unknown non-empty runtime model. -export const CODEX_PRICING_BASIS = "openai-standard-2026-09-13"; +export const CODEX_PRICING_BASIS = "openai-standard-effective-dates-2026-09-14"; const LONG_CONTEXT_RATE_KEYS = new Set(["gpt-6-astra", "gpt-5.6-sol", "gpt-5.6", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.5", "gpt-5.4"]); function codexRateKey(rates, model) { const m = String(model || "").toLowerCase(); @@ -50,8 +50,7 @@ function priceCodexRequest(rawUsage, model, rates, { allowLongContext = true } = ) / 1_000_000; } -export function estimateCodexCost(u = {}, model = "", requests = []) { - const rates = getCodexModelRates(); +export function estimateCodexCost(u = {}, model = "", requests = [], { rates = getCodexModelRates() } = {}) { const detailed = Array.isArray(requests) ? requests.filter((request) => request?.usage) : []; if (detailed.length) { let cost = 0; diff --git a/test/usage-pricing.test.js b/test/usage-pricing.test.js index 337a584a..1a9bc6c9 100644 --- a/test/usage-pricing.test.js +++ b/test/usage-pricing.test.js @@ -7,6 +7,7 @@ ensureTestEnv(); const { runMigrations } = await import("../src/db/index.js"); const { CODEX_PRICING_HISTORY_SINCE, + CODEX_SOL_PRICE_CUTOVER, applyCodexPricingRefresh, autoRefreshCodexPricing, buildCodexPricingRefresh, @@ -34,11 +35,15 @@ function fixture() { VALUES(?, 0, ?, ?, ?, 0, ?, 0, 1050000, ?, ?)` ); - const sol = usage.run("2026-09-01T00:00:00.000Z", "gpt-5.6-sol", 300_000, 1_000, 2.145, "gpt-5.6-sol"); + const sol = usage.run(CODEX_SOL_PRICE_CUTOVER, "gpt-5.6-sol", 300_000, 1_000, 2.145, "gpt-5.6-sol"); const solComponent = component.run(Number(sol.lastInsertRowid), "sol", "gpt-5.6-sol", 300_000, 100_000, 0, 1_000, 2.145); request.run(Number(solComponent.lastInsertRowid), "gpt-5.6-sol", 300_000, 100_000, 1_000, 1, 2.145); component.run(Number(sol.lastInsertRowid), "review", "codex-auto-review", 1_000, 0, 0, 10, null); + const solBefore = usage.run("2026-08-20T23:59:59.999Z", "gpt-5.6-sol", 300_000, 1_000, 2.145, "gpt-5.6-sol"); + const solBeforeComponent = component.run(Number(solBefore.lastInsertRowid), "sol-before", "gpt-5.6-sol", 300_000, 100_000, 0, 1_000, 2.145); + request.run(Number(solBeforeComponent.lastInsertRowid), "gpt-5.6-sol", 300_000, 100_000, 1_000, 1, 2.145); + const astra = usage.run("2026-09-10T00:00:00.000Z", "gpt-6-astra", 1_000_000, 10_000, null, "gpt-6-astra"); component.run(Number(astra.lastInsertRowid), "astra", "gpt-6-astra", 1_000_000, 500_000, 0, 10_000, null); @@ -60,11 +65,11 @@ test("pricing refresh reprices the two-month evidence window and records an idem assert.deepEqual(pendingCodexPricingRefresh(db), { pending: true, appliedBasis: "", basis: CODEX_PRICING_BASIS }); const plan = buildCodexPricingRefresh({ db }); assert.equal(plan.since, CODEX_PRICING_HISTORY_SINCE); - assert.equal(plan.usageRows, 2); - assert.equal(plan.components, 3); - assert.equal(plan.requests, 1); + assert.equal(plan.usageRows, 3); + assert.equal(plan.components, 4); + assert.equal(plan.requests, 2); assert.equal(plan.unpricedComponents, 1); - assert.equal(plan.models["gpt-5.6-sol"].repricedValue, 1.71); + assert.equal(plan.models["gpt-5.6-sol"].repricedValue, 3.855); assert.equal(plan.models["gpt-6-astra"].repricedValue, 6); assert.equal(plan.models["codex-auto-review"].unpricedComponents, 1); @@ -73,17 +78,19 @@ test("pricing refresh reprices the two-month evidence window and records an idem assert.deepEqual(db.prepare("SELECT source_key, cost_usd, cost_estimated, pricing_basis FROM usage_components ORDER BY id").all().map((row) => ({ ...row })), [ { source_key: "sol", cost_usd: 1.71, cost_estimated: 1, pricing_basis: CODEX_PRICING_BASIS }, { source_key: "review", cost_usd: null, cost_estimated: 0, pricing_basis: "unpriced" }, + { source_key: "sol-before", cost_usd: 2.145, cost_estimated: 1, pricing_basis: CODEX_PRICING_BASIS }, { source_key: "astra", cost_usd: 6, cost_estimated: 1, pricing_basis: CODEX_PRICING_BASIS }, { source_key: "before-window", cost_usd: 9.99, cost_estimated: 1, pricing_basis: "openai-standard-2026-08-16" }, { source_key: "claude", cost_usd: 8.88, cost_estimated: 1, pricing_basis: "openai-standard-2026-08-16" }, ]); assert.deepEqual(db.prepare("SELECT model, cost_usd FROM usage ORDER BY id").all().map((row) => ({ ...row })), [ { model: "gpt-5.6-sol", cost_usd: 1.71 }, + { model: "gpt-5.6-sol", cost_usd: 2.145 }, { model: "gpt-6-astra", cost_usd: 6 }, { model: "gpt-5.6-sol", cost_usd: 9.99 }, { model: "claude-opus-4-1", cost_usd: 8.88 }, ]); - assert.equal(db.prepare("SELECT cost_usd FROM usage_requests").get().cost_usd, 1.71); + assert.deepEqual(db.prepare("SELECT cost_usd FROM usage_requests ORDER BY id").all().map((row) => row.cost_usd), [1.71, 2.145]); db.prepare("UPDATE usage_components SET pricing_basis = 'unpriced' WHERE source_key = 'astra'").run(); assert.deepEqual(pendingCodexPricingRefresh(db), { From 5db1dca1f41038cf075f3a88dac9eb4d9bfdc0c2 Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Mon, 14 Sep 2026 10:32:20 +0300 Subject: [PATCH 07/25] fix: remove successfully processed media uploads Signed-off-by: Tiberiu Socaci --- CHANGELOG.md | 5 ++ FEATURES.md | 13 +++- TEST-PLAN.md | 32 ++++++---- .../references/video-understanding.md | 12 ++++ .../scripts/cleanup_uploaded_media.py | 52 ++++++++++++++++ src/gateway/safe-fs.js | 26 ++++++++ src/gateway/transcribe.js | 45 ++++++++++---- src/platforms/ingest.js | 21 ++++++- src/platforms/voice.js | 7 ++- src/slack/message-pipeline.js | 10 ++++ test/managed-write-symlinks.test.js | 49 ++++++++++++++- test/platform-voice.test.js | 5 +- test/whisper-transcribe.test.js | 59 ++++++++++++++++++- 13 files changed, 304 insertions(+), 32 deletions(-) create mode 100755 src/gateway/gateway-usage/scripts/cleanup_uploaded_media.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 729c06c6..d3db1426 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog — ChannelGate +- Remove gateway-downloaded audio after a successful local or Slack-fallback transcript, while + retaining failed inputs for retry and refusing symlinks or paths outside managed uploads. Teach + the built-in video-understanding workflow to remove only successfully processed uploaded source + videos after all required re-sampling, never project files or failed inputs. + - Correct Codex history repricing to honor OpenAI's effective date for GPT-5.6 Sol: retain the original $5 / $0.50 cached / $30 rate before 2026-08-21 and apply $4 / $0.40 / $20 from that date onward. Upgraded instances rerun the backup-first correction under a new pricing basis. diff --git a/FEATURES.md b/FEATURES.md index 1cf8b0f5..5cae0dd8 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -447,7 +447,9 @@ A categorized catalog of what's shipped. Cross-linked to `TEST-PLAN.md` checks. - Downloadable audio is transcribed locally with the shared Whisper setting and cancellation; the portable path never calls Slack transcript APIs. Raw audio paths are withheld from the engine, ordinary files remain available, failed audio alone produces an explanation without an - engine turn, and typed text can continue with a visible missing-transcript note. Progress starts + engine turn, and typed text can continue with a visible missing-transcript note. A successfully + transcribed gateway-downloaded audio source is removed immediately; failed sources remain for a + retry, and symlinks or paths outside the managed `uploads/` root are refused. Progress starts before download/transcription. Every attachment intake has a unique storage directory so simultaneous same-name files or later edits cannot overwrite bytes another turn is reading. - The branch's Slack parity inventory and remaining surface-specific work live in @@ -641,7 +643,9 @@ A categorized catalog of what's shipped. Cross-linked to `TEST-PLAN.md` checks. Slack's completed full VTT transcript; absent transcripts prompt the user to click **Generate transcript** and re-trigger. Fresh installers ask whether to provision Whisper, updates honor the stored setting, and disabled mode never downloads raw audio. Typed text remains instructions and - raw audio is excluded from Claude/Codex. → TEST-PLAN: Voice prompts. + raw audio is excluded from Claude/Codex. Successfully resolved downloaded audio is removed from + the channel's `uploads/` folder, including when Slack transcript fallback completes after a local + failure; unresolved audio is retained for retry. → TEST-PLAN: Voice prompts. - Native Slack **channel file explorer**: the 📂 reply button opens a Block Kit modal rooted at the channel's effective working folder. Its title identifies the authoritative stored Slack channel name, and its subtitle shows the full absolute current directory, refreshed on every navigation. The @@ -2211,7 +2215,10 @@ are retired, bullet by bullet; everything else stands. image includes `ffmpeg`/`ffprobe`, pinned `opencv-python-headless` + `faster-whisper`, and a root-owned pre-cached Whisper `small` model. The always-present `gateway-usage` skill owns the video workflow, sampling guidance, dependency diagnostics, and analyzer script, so it can extract representative frames, build contact sheets and transcribe timestamped - speech without a per-channel install or first-use model download. `npm run setup` builds the + speech without a per-channel install or first-use model download. After successful analysis and + any needed re-sampling, the workflow removes only gateway-downloaded regular video files beneath + the channel's `uploads/` directory; failures and user-managed project files remain untouched. + `npm run setup` builds the image as part of a fresh install (`--skip-image` / `CG_BUILD_IMAGE=no` defers it and names `npm run build:image` as the remedy; a failed build never aborts the install), so a new gateway never reaches its first message without the toolchain. The former standalone catalog skill is diff --git a/TEST-PLAN.md b/TEST-PLAN.md index a21214e7..0d121af1 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -269,7 +269,8 @@ allowlisted drive identity, no `/shares` request, no bearer on the byte request, and bounded streams. Voice fixtures inject transcripts/failures, plus a cancellable local Node child; require no raw audio engine attachments, no engine on audio-only failure and preserved text/file fallback. Simultaneous flat messages named `audio.wav` plus a revision must retain -three different storage paths and each original byte sequence. +three different storage paths while processing; completed audio sources are then removed and a +failed source remains byte-for-byte available for retry. - [ ] UNEXECUTED live native-card gate, separately with Claude and Codex pinned: install the reviewed branch manifest in an owned personal chat, channel and external-member group. It must @@ -314,8 +315,9 @@ three different storage paths and each original byte sequence. `Reply TEXT_FALLBACK_OK` with unavailable audio: text must run and the failure remain visible. Stop a long local transcription and verify child exit and no later engine start. Send two simultaneous group messages each attaching `audio.wav` with distinct spoken markers, then - edit/retrigger one; require independent stored bytes, transcripts and group session roots. - Preserve ordinary attached files. There is no Slack transcript fallback on Teams. + edit/retrigger one; require independent transcripts and group session roots, successful audio + sources removed after processing, and the stopped/failed source retained for retry. Preserve + ordinary attached files. There is no Slack transcript fallback on Teams. Verification on 2026-09-09: full coverage suite passed (2,404 passed, 10 skipped); @@ -1717,8 +1719,11 @@ structural invariants are automated; rendered navigation and feature claims also the assistant status reads `is downloading 1 attachment(s) (… MB)…` while it fetches, the file lands under `uploads//` with its full size, the daemon's RSS does not grow by the file size (`systemctl --user status` memory line before/after), and the `video-understanding` skill - analyzes it. Attach a >500 MB file → the reply says ` exceeds the 500 MB attachment - limit` and `events.attachment_failed` carries the same reason. Both engines (QA: ATT-01). + analyzes it. After the evidence pack and any targeted re-sampling are complete, the original + upload is gone while the evidence pack remains. A deliberately failed decode keeps its source, + and a project video outside `uploads/` is never deleted. Attach a >500 MB file → the reply + says ` exceeds the 500 MB attachment limit` and `events.attachment_failed` carries the + same reason. Both engines (QA: ATT-01). - [ ] Live attachment smoke: upload XLSX, PDF, image, and multiple files with an `@bot` mention in both a root and a reply; edit a file message to add the mention; and confirm each turn receives the local path under the same thread folder exactly once. Then read the thread with @@ -1747,8 +1752,10 @@ structural invariants are automated; rendered navigation and feature claims also by `test/whisper-transcribe.test.js`. - [x] Unit: an unmentioned channel voice clip stays inert; mentioned, 🤖-reaction, and DM voice messages follow existing trigger semantics; authorization precedes both paths; typed text + - voice compose one prompt; raw audio is omitted; and no-transcript guidance exits before an - engine run (`test/slack-voice-prompts.test.js`). + voice compose one prompt; raw audio is omitted; successfully resolved downloads are removed; + failed originals remain for retry; cleanup refuses out-of-root files and symlinks; and + no-transcript guidance exits before an engine run (`test/slack-voice-prompts.test.js`, + `test/whisper-transcribe.test.js`, `test/managed-write-symlinks.test.js`). - [x] Unit: the backward-compatible setting and Admin UI/API wiring are covered by `test/whisper-settings.test.js`; installer flags/env/prompt/default behavior, platform/checksum selection, archive safety, persisted skip, and conditional updates are covered by @@ -1757,7 +1764,9 @@ structural invariants are automated; rendered navigation and feature claims also clip runs after an `@mention` or 🤖 reaction, while a DM follows current no-mention behavior. - [ ] Live: an unauthorized author cannot cause an audio download/transcription in a channel or DM. - [ ] Live: English and Romanian clips transcribe accurately enough to execute the spoken request; - typed text acts as instructions, and two clips appear in their original order. + typed text acts as instructions, and two clips appear in their original order. A successful + local transcript removes its downloaded source from `uploads/`; local failure plus successful + Slack fallback also removes it; total failure retains it for retry. - [ ] Privacy: with local mode enabled, observe only local `ffmpeg`/`whisper-cli`; with it disabled, observe Slack metadata/VTT reads but no raw-audio download. In both modes, neither raw audio nor an audio path reaches Claude/Codex. @@ -4275,9 +4284,10 @@ are the v0.8 production deployment gate and are executed in the QA loop that fol cache, passes every pin through the image builder, and bumps the daemon/image spec in lockstep (automated: `test/container-image.test.js`). - [x] Unit: `gateway-usage` materializes its video workflow, dependency reference, and analyzer - script into every surface; the former standalone slug is rejected at the grant boundary, - excluded from the catalog, removed from organization/template/channel/personal grants at - boot, and stale gateway-managed workspace copies are pruned (automated: + script into every surface, including the successful-analysis-only cleanup rule for regular + gateway downloads beneath `uploads/`; the former standalone slug is rejected at the grant + boundary, excluded from the catalog, removed from organization/template/channel/personal + grants at boot, and stale gateway-managed workspace copies are pruned (automated: `test/access-grants.test.js`, `test/managed-write-symlinks.test.js`, `test/skills-platform.test.js`). - [x] Unit: `npm run setup` builds the channel image as part of the install (`scripts/install.sh` diff --git a/src/gateway/gateway-usage/references/video-understanding.md b/src/gateway/gateway-usage/references/video-understanding.md index 3c83cbe7..697e7d9f 100644 --- a/src/gateway/gateway-usage/references/video-understanding.md +++ b/src/gateway/gateway-usage/references/video-understanding.md @@ -23,6 +23,18 @@ workflow from the transcript alone or from a few evenly spaced screenshots. evidence streams disagree. Prefer targeted extra frames over dense whole-video extraction. 6. Answer while distinguishing directly visible facts, spoken statements, combined inferences, and unresolved ambiguity. +7. After the evidence pack is complete and no more re-sampling is needed, remove the original only + when it is a gateway-downloaded regular file beneath the current channel's `uploads/` directory. + Use the bundled constrained cleanup helper, which resolves both paths and refuses symlinks or + paths outside `uploads/`: + + ```bash + python3 .claude/skills/gateway-usage/scripts/cleanup_uploaded_media.py VIDEO_PATH + ``` + + Delete only the source video—not the evidence pack. Keep the original when decoding, + transcription, or analysis failed so the user can retry. Never auto-delete a video supplied + from a project or user-managed path. For screen recordings, prioritize UI labels, selected cells, cursor focus, typed values, before/after state, and exact errors. Repeated table rows usually represent duplicates unless the diff --git a/src/gateway/gateway-usage/scripts/cleanup_uploaded_media.py b/src/gateway/gateway-usage/scripts/cleanup_uploaded_media.py new file mode 100755 index 00000000..4f77bdcf --- /dev/null +++ b/src/gateway/gateway-usage/scripts/cleanup_uploaded_media.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 +"""Delete one successfully processed media upload without escaping the channel uploads folder.""" + +from __future__ import annotations + +import argparse +import json +import stat +import sys +from pathlib import Path + + +def fail(message: str) -> None: + print(message, file=sys.stderr) + raise SystemExit(2) + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("source", type=Path) + args = parser.parse_args() + + channel_root = Path.cwd().resolve() + try: + uploads_root = (channel_root / "uploads").resolve(strict=True) + except OSError as exc: + fail(f"Managed uploads directory is unavailable: {exc}") + + source = args.source if args.source.is_absolute() else channel_root / args.source + try: + source_info = source.lstat() + except OSError as exc: + fail(f"Media source is unavailable: {exc}") + if stat.S_ISLNK(source_info.st_mode) or not stat.S_ISREG(source_info.st_mode): + fail("Refusing to remove a source that is not a regular file.") + + try: + resolved_source = source.resolve(strict=True) + resolved_source.relative_to(uploads_root) + except (OSError, ValueError): + fail("Refusing to remove a source outside the current channel's managed uploads directory.") + + try: + source.unlink() + except OSError as exc: + fail(f"Could not remove processed media source: {exc}") + + print(json.dumps({"removed": str(resolved_source)}, ensure_ascii=False)) + + +if __name__ == "__main__": + main() diff --git a/src/gateway/safe-fs.js b/src/gateway/safe-fs.js index cebbf09d..37b64365 100644 --- a/src/gateway/safe-fs.js +++ b/src/gateway/safe-fs.js @@ -153,6 +153,32 @@ export async function createExclusive(file, content, { mode = 0o644 } = {}) { } } +// Remove one gateway-managed regular file only when its resolved path remains beneath a trusted +// root. Attachment folders are agent-writable between turns, so a path that has been replaced by +// a symlink or moved outside the upload root is refused instead of followed. Missing files are +// already clean and therefore return false without failing the completed media operation. +export async function removeRegularFileWithin(root, file) { + try { + const resolvedRoot = await realpath(root); + const candidate = path.resolve(String(file || "")); + const resolvedParent = await realpath(path.dirname(candidate)); + const parentRelative = path.relative(resolvedRoot, resolvedParent); + if (parentRelative === ".." || parentRelative.startsWith(`..${path.sep}`) || path.isAbsolute(parentRelative)) return false; + + const info = await lstat(candidate); + if (!info.isFile() || info.isSymbolicLink()) return false; + const resolvedCandidate = await realpath(candidate); + const fileRelative = path.relative(resolvedRoot, resolvedCandidate); + if (fileRelative === ".." || fileRelative.startsWith(`..${path.sep}`) || path.isAbsolute(fileRelative)) return false; + + await rm(candidate); + return true; + } catch (error) { + if (error?.code === "ENOENT") return false; + throw error; + } +} + // A symlink is tolerable at a managed path ONLY when it points at a real directory that still // lives inside the trusted root. Returns that resolved directory, or null for a dangling link, a // link to a non-directory, or one that escapes the root — all of which the caller removes. diff --git a/src/gateway/transcribe.js b/src/gateway/transcribe.js index 07112a1a..9e0046fb 100644 --- a/src/gateway/transcribe.js +++ b/src/gateway/transcribe.js @@ -188,6 +188,7 @@ export async function transcribeSlackAudioFiles(files, { export async function resolveAudioTranscripts(files, { localEnabled = true, downloadLocal = async () => { throw new Error("Local audio download is unavailable."); }, + removeProcessed = null, localTranscriber = transcribeAudioFiles, localOptions = {}, slackTranscriber = transcribeSlackAudioFiles, @@ -196,31 +197,46 @@ export async function resolveAudioTranscripts(files, { const transcripts = []; const failed = []; const localFailed = []; + const cleanupFailed = []; for (const source of files || []) { const name = safeLabel(source?.name); + let saved = null; + let complete = false; if (localEnabled) { try { - const saved = await downloadLocal(source); + saved = await downloadLocal(source); if (!saved?.path) throw new Error(saved?.skipped || "Local audio download failed."); const local = await localTranscriber([saved], localOptions); if (local.transcripts?.[0]?.text) { transcripts.push({ name, text: local.transcripts[0].text }); - continue; + complete = true; + } else { + throw new Error(local.failed?.[0]?.reason || "Local Whisper produced no transcript."); } - throw new Error(local.failed?.[0]?.reason || "Local Whisper produced no transcript."); } catch (error) { localFailed.push({ name, reason: boundedReason(error) }); } } - const slack = await slackTranscriber([source], slackOptions); - if (slack.transcripts?.[0]?.text) { - transcripts.push({ name, text: slack.transcripts[0].text }); - } else { - failed.push({ name, reason: boundedReason(slack.failed?.[0]?.reason || "Slack transcript is not ready — click Generate transcript, then re-trigger the bot.") }); + if (!complete) { + const slack = await slackTranscriber([source], slackOptions); + if (slack.transcripts?.[0]?.text) { + transcripts.push({ name, text: slack.transcripts[0].text }); + complete = true; + } else { + failed.push({ name, reason: boundedReason(slack.failed?.[0]?.reason || "Slack transcript is not ready — click Generate transcript, then re-trigger the bot.") }); + } + } + if (complete && saved?.path && typeof removeProcessed === "function") { + try { + const removed = await removeProcessed(saved); + if (removed === false) throw new Error("downloaded source was no longer a managed regular upload"); + } catch (error) { + cleanupFailed.push({ name, reason: boundedReason(error) }); + } } } - return { transcripts, failed, localFailed }; + return { transcripts, failed, localFailed, cleanupFailed }; } export function composeVoicePrompt({ text = "", transcripts = [], failed = [] } = {}) { @@ -348,6 +364,7 @@ export async function transcribeAudioFile(filePath, options = {}) { export async function transcribeAudioFiles(files, options = {}) { const transcripts = []; const failed = []; + const cleanupFailed = []; const custom = typeof options.transcribe === "function" ? options.transcribe : null; for (const file of files || []) { options.signal?.throwIfAborted(); @@ -360,9 +377,17 @@ export async function transcribeAudioFiles(files, options = {}) { text = await transcribeAudioFile(file.path, options); } transcripts.push({ name: safeLabel(file.name), text }); + if (typeof options.removeProcessed === "function") { + try { + const removed = await options.removeProcessed(file); + if (removed === false) throw new Error("downloaded source was no longer a managed regular upload"); + } catch (error) { + cleanupFailed.push({ name: safeLabel(file?.name), reason: boundedReason(error) }); + } + } } catch (error) { failed.push({ name: safeLabel(file?.name), reason: boundedReason(error) }); } } - return { transcripts, failed }; + return { transcripts, failed, cleanupFailed }; } diff --git a/src/platforms/ingest.js b/src/platforms/ingest.js index 812649bc..c41f6dda 100644 --- a/src/platforms/ingest.js +++ b/src/platforms/ingest.js @@ -3,7 +3,7 @@ // policy/stores as Slack. Interactive approval escalation remains deliberately unavailable here. import { upsertChannelEntry, getChannelMeta, saveChannelMeta, defaultChannelMeta, getUser, setUser, isAdmin, isApproved } from "../config/store.js"; import { getDefaultChannelAccess, applyChannelTemplate, getDefaultNudges } from "../config/settings.js"; -import { ensureChannelFolder } from "../gateway/folders.js"; +import { effectiveWorkDir, ensureChannelFolder } from "../gateway/folders.js"; import { isAuthorized } from "../gateway/modes.js"; import { runMessage } from "../gateway/run.js"; import { createUsageBank } from "../gateway/usage.js"; @@ -12,6 +12,8 @@ import { platformOr, platformSupports } from "./registry.js"; import { postFormatted } from "./connector.js"; import { sessionKeyForMessage, rememberReplySession } from "./reply-sessions.js"; import { saveInboundAttachments } from "./attachments.js"; +import path from "node:path"; +import { removeRegularFileWithin } from "../gateway/safe-fs.js"; // Conversation kinds as the channel store spells them. The store's vocabulary is Slack's, and it is // a SECURITY value there (it decides whether a private channel's name may appear in App Home), so @@ -121,7 +123,22 @@ export function createIngest({ connector, log = console, run = runMessage, onCom skipped = saved.skipped; signal.throwIfAborted(); if (hasVoiceAttachments(message)) progress.phase('Transcribing voice locally'); - prepared = await voice(message, saved.paths, { signal }); + prepared = await voice(message, saved.paths, { + signal, + removeProcessed: (file) => removeRegularFileWithin( + path.join(effectiveWorkDir(entry.slug, meta), "uploads"), + file.path, + ), + }); + if (prepared.cleanupFailed?.length) { + await logEvent("attachment_cleanup_failed", { + channel: message.conversationId, + author: message.userId, + slug: entry.slug, + platform: adapter.id, + reasons: prepared.cleanupFailed.map((item) => `${item.name}: ${item.reason}`).join("; ").slice(0, 1000), + }); + } if (prepared.hasVoice && !prepared.hasPrompt && !prepared.paths.length) { await progress.stop(); await deliver(connector, message, placeholder, prepared.failureNotice || 'Could not transcribe this audio. Please send text or ask an administrator to check local Whisper.', rememberReply); diff --git a/src/platforms/voice.js b/src/platforms/voice.js index 38de13b4..bd6c092d 100644 --- a/src/platforms/voice.js +++ b/src/platforms/voice.js @@ -9,7 +9,7 @@ export function hasVoiceAttachments(message) { } export async function prepareVoiceAttachments(message, paths, { - signal, enabled = getWhisperEnabled(), transcribe = transcribeAudioFiles, + signal, enabled = getWhisperEnabled(), transcribe = transcribeAudioFiles, removeProcessed = null, } = {}) { signal?.throwIfAborted(); const pending = new Map(paths.map((file) => [path.basename(file), file])); @@ -30,11 +30,13 @@ export async function prepareVoiceAttachments(message, paths, { else audio.push({ name, path: saved }); } let transcripts = []; + let cleanupFailed = []; if (audio.length) { try { - const result = await transcribe(audio, { signal }); + const result = await transcribe(audio, { signal, removeProcessed }); transcripts = (result.transcripts || []).filter((item) => String(item.text || '').trim()); failed.push(...(result.failed || [])); + cleanupFailed = result.cleanupFailed || []; for (const file of audio) if (!transcripts.some((item) => item.name === file.name) && !failed.some((item) => item.name === file.name)) failed.push({ name: file.name, reason: 'Local Whisper detected no speech.' }); } catch (error) { signal?.throwIfAborted(); @@ -49,5 +51,6 @@ export async function prepareVoiceAttachments(message, paths, { hasPrompt: Boolean(String(message.text || '').trim() || transcripts.length), failed, failureNotice: failed.length ? composeVoicePrompt({ failed }) : '', + cleanupFailed, }; } diff --git a/src/slack/message-pipeline.js b/src/slack/message-pipeline.js index 55435289..833388eb 100644 --- a/src/slack/message-pipeline.js +++ b/src/slack/message-pipeline.js @@ -18,6 +18,7 @@ import { resolveRuntime } from "../runtimes/resolve.js"; import { setThreadEngine, getThreadEngine, resolveThreadEngine, setThreadClean, getThreadClean, setThreadModel, getThreadModel, setThreadEffort, getThreadEffort } from "../gateway/thread-engine.js"; import { abortPooled, pooledBusy, interruptPooled } from "../engines/session-pool.js"; import { logEvent } from "../util/logger.js"; +import { removeRegularFileWithin } from "../gateway/safe-fs.js"; // A typed `/mode` is a channel POLICY change like any admin-UI save — audited the same way. import { logChannelPolicyChange } from "../config/channel-audit.js"; import { createUsageBank } from "../gateway/usage.js"; @@ -1144,6 +1145,7 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI if (!local?.path) throw new Error(local?.skipped || "Slack audio download failed."); return local; }, + removeProcessed: (file) => removeRegularFileWithin(path.join(dest.root, "uploads"), file.path), slackOptions: { botToken }, }); } finally { @@ -1166,6 +1168,14 @@ export async function processMessageEvent(event, client, { botUserId = "", teamI reasons: voice.failed.map((item) => `${item.name}: ${item.reason}`).join("; ").slice(0, 1000), }); } + if (voice.cleanupFailed.length) { + await logEvent("attachment_cleanup_failed", { + channel: event.channel, + author: event.user, + slug: entry.slug, + reasons: voice.cleanupFailed.map((item) => `${item.name}: ${item.reason}`).join("; ").slice(0, 1000), + }); + } if (!voice.transcripts.length && !prompt.trim()) { await client.chat.postMessage({ channel: event.channel, diff --git a/test/managed-write-symlinks.test.js b/test/managed-write-symlinks.test.js index ba3b1ad5..314c4fbe 100644 --- a/test/managed-write-symlinks.test.js +++ b/test/managed-write-symlinks.test.js @@ -12,6 +12,8 @@ import assert from "node:assert/strict"; import { chmod, mkdtemp, mkdir, readFile, readdir, realpath, stat, writeFile, symlink, lstat, rm } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; import { ensureTestEnv } from "./helpers.js"; ensureTestEnv(); @@ -19,7 +21,7 @@ ensureTestEnv(); // test ever provisions a folder in the operator's real ~/Slack Agent. process.env.CG_WORKSPACE_DIR ||= await mkdtemp(path.join(os.tmpdir(), "cg-ws-")); -const [{ readNoFollow, writeNoFollow, writeStreamNoFollow, createExclusive, ensureRealDir }, memory, guide, folders, librarySkills, apiRuns, pipeline, paths, attachments, { ATTACHMENT_MAX_BYTES }] = +const [{ readNoFollow, writeNoFollow, writeStreamNoFollow, createExclusive, ensureRealDir, removeRegularFileWithin }, memory, guide, folders, librarySkills, apiRuns, pipeline, paths, attachments, { ATTACHMENT_MAX_BYTES }] = await Promise.all([ import("../src/gateway/safe-fs.js"), import("../src/gateway/channel-memory.js"), @@ -91,6 +93,25 @@ test("readNoFollow rethrows a REAL read failure instead of reporting the file as assert.equal(await readNoFollow(dir), null); }); +test("managed cleanup removes only regular files that remain inside its trusted root", async (t) => { + const { cwd, outside } = await scratch(t); + const uploads = path.join(cwd, "uploads"); + await mkdir(uploads); + const processed = path.join(uploads, "voice.wav"); + const outsideFile = path.join(outside, "keep.wav"); + const plantedLink = path.join(uploads, "swapped.wav"); + await writeFile(processed, "processed"); + await writeFile(outsideFile, "outside"); + await symlink(outsideFile, plantedLink); + + assert.equal(await removeRegularFileWithin(uploads, processed), true); + await assert.rejects(readFile(processed), { code: "ENOENT" }); + assert.equal(await removeRegularFileWithin(uploads, outsideFile), false); + assert.equal(await removeRegularFileWithin(uploads, plantedLink), false); + assert.equal(await readFile(outsideFile, "utf8"), "outside"); + assert.equal((await lstat(plantedLink)).isSymbolicLink(), true); +}); + test("writeNoFollow replaces a symlink node atomically and leaves no temp files behind", async (t) => { const { cwd, outside } = await scratch(t); const managed = path.join(cwd, "MANAGED.md"); @@ -234,7 +255,33 @@ test("the gateway-usage refresh replaces planted symlinks inside its own skill f await assertReplacedNode(path.join(skillDir, "SKILL.md"), skillVictim); assert.match(await readFile(path.join(skillDir, "SKILL.md"), "utf8"), /name: gateway-usage/); assert.match(await readFile(path.join(skillDir, "references", "video-understanding.md"), "utf8"), /two synchronized evidence streams/); + assert.match(await readFile(path.join(skillDir, "references", "video-understanding.md"), "utf8"), /remove the original only[\s\S]*beneath the current channel's `uploads\/` directory/); assert.match(await readFile(path.join(skillDir, "scripts", "analyze_video.py"), "utf8"), /timestamped visual\/audio evidence pack/); + assert.match(await readFile(path.join(skillDir, "scripts", "cleanup_uploaded_media.py"), "utf8"), /current channel's managed uploads directory/); +}); + +test("the video cleanup helper removes uploads and refuses outside files or symlinks", async (t) => { + const { cwd, outside } = await scratch(t); + const uploads = path.join(cwd, "uploads"); + const script = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "src", "gateway", "gateway-usage", "scripts", "cleanup_uploaded_media.py"); + await mkdir(uploads); + const uploaded = path.join(uploads, "recording.mp4"); + const outsideFile = path.join(outside, "project.mp4"); + const link = path.join(uploads, "linked.mp4"); + await writeFile(uploaded, "uploaded"); + await writeFile(outsideFile, "project"); + await symlink(outsideFile, link); + + const removed = spawnSync("python3", [script, uploaded], { cwd, encoding: "utf8" }); + assert.equal(removed.status, 0, removed.stderr); + await assert.rejects(readFile(uploaded), { code: "ENOENT" }); + + const outsideResult = spawnSync("python3", [script, outsideFile], { cwd, encoding: "utf8" }); + assert.equal(outsideResult.status, 2); + const linkResult = spawnSync("python3", [script, link], { cwd, encoding: "utf8" }); + assert.equal(linkResult.status, 2); + assert.equal(await readFile(outsideFile, "utf8"), "project"); + assert.equal((await lstat(link)).isSymbolicLink(), true); }); test("the gateway-usage refresh rebuilds a symlinked .claude/skills path as real directories", async (t) => { diff --git a/test/platform-voice.test.js b/test/platform-voice.test.js index 1e226703..c13b26c6 100644 --- a/test/platform-voice.test.js +++ b/test/platform-voice.test.js @@ -11,9 +11,10 @@ const { runProcess, transcribeAudioFiles } = await import('../src/gateway/transc const audio = (name = 'clip.wav') => ({ name, contentType: 'audio/wav', download: async () => Buffer.from('fixture audio') }); test('voice metadata follows original attachment index through download failures; files remain files', async () => { - let seen; - const result = await prepareVoiceAttachments({ text: 'Please summarize', attachments: [{ name: 'missing.pdf', contentType: 'application/pdf' }, audio(), { name: 'data.txt', contentType: 'text/plain' }] }, ['/fixture/2-clip.wav', '/fixture/3-data.txt'], { enabled: true, transcribe: async (files) => { seen = files; return { transcripts: [{ name: 'clip.wav', text: 'Book a meeting' }], failed: [] }; } }); + let seen; let removed; + const result = await prepareVoiceAttachments({ text: 'Please summarize', attachments: [{ name: 'missing.pdf', contentType: 'application/pdf' }, audio(), { name: 'data.txt', contentType: 'text/plain' }] }, ['/fixture/2-clip.wav', '/fixture/3-data.txt'], { enabled: true, removeProcessed: async (file) => { removed = file.path; return true; }, transcribe: async (files, options) => { seen = files; await options.removeProcessed(files[0]); return { transcripts: [{ name: 'clip.wav', text: 'Book a meeting' }], failed: [], cleanupFailed: [] }; } }); assert.equal(seen[0].path, '/fixture/2-clip.wav'); assert.deepEqual(result.paths, ['/fixture/3-data.txt']); assert.match(result.text, /Please summarize/); assert.match(result.text, /Book a meeting/); + assert.equal(removed, '/fixture/2-clip.wav'); assert.deepEqual(result.cleanupFailed, []); }); test('local-only disabled, failed, missing and empty speech paths explain failure', async () => { diff --git a/test/whisper-transcribe.test.js b/test/whisper-transcribe.test.js index 280ee5b4..4e2d306a 100644 --- a/test/whisper-transcribe.test.js +++ b/test/whisper-transcribe.test.js @@ -2,7 +2,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import os from "node:os"; import path from "node:path"; -import { mkdtemp, readdir, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; import { MAX_TRANSCRIPT_CHARS, composeVoicePrompt, @@ -131,6 +131,45 @@ test("audio batches serialize the daemon-wide Whisper work and preserve failures assert.deepEqual(two.transcripts, [{ name: "b.ogg", text: "b.ogg" }]); }); +test("completed local audio is cleaned while failed processing keeps its source for retry", async (t) => { + const dir = await mkdtemp(path.join(os.tmpdir(), "cg-whisper-cleanup-")); + t.after(() => rm(dir, { recursive: true, force: true })); + const good = path.join(dir, "good.wav"); + const bad = path.join(dir, "bad.wav"); + await writeFile(good, "good audio"); + await writeFile(bad, "bad audio"); + + const result = await transcribeAudioFiles([ + { name: "good.wav", path: good }, + { name: "bad.wav", path: bad }, + ], { + transcribe: async (filePath) => { + if (filePath === bad) throw new Error("decode failed"); + return "spoken text"; + }, + removeProcessed: async (file) => { + await rm(file.path); + return true; + }, + }); + + assert.deepEqual(result.transcripts, [{ name: "good.wav", text: "spoken text" }]); + assert.equal(result.failed[0].name, "bad.wav"); + assert.deepEqual(result.cleanupFailed, []); + await assert.rejects(readFile(good), { code: "ENOENT" }); + assert.equal(await readFile(bad, "utf8"), "bad audio"); +}); + +test("cleanup refusal is observable without discarding a successful transcript", async () => { + const result = await transcribeAudioFiles([{ name: "voice.wav", path: "/tmp/voice.wav" }], { + transcribe: async () => "spoken text", + removeProcessed: async () => false, + }); + assert.deepEqual(result.transcripts, [{ name: "voice.wav", text: "spoken text" }]); + assert.equal(result.failed.length, 0); + assert.match(result.cleanupFailed[0].reason, /no longer a managed regular upload/i); +}); + test("Slack WebVTT parsing removes cue metadata but keeps complete spoken text", () => { assert.equal( parseWebVtt("WEBVTT\n\n00:00:00.579 --> 00:00:01.700\n- How are you?\n\n00:00:02.000 --> 00:00:03.000\nSecond line."), @@ -210,6 +249,7 @@ test("disabled local Whisper never downloads audio and uses Slack transcripts in test("local failures fall back to Slack while local successes stay local", async () => { let slackCalls = 0; + const removed = []; const result = await resolveAudioTranscripts([ { id: "GOOD", name: "good.m4a" }, { id: "BAD", name: "bad.m4a" }, @@ -223,8 +263,25 @@ test("local failures fall back to Slack while local successes stay local", async slackCalls += 1; return { transcripts: [{ name: files[0].name, text: "Slack text" }], failed: [] }; }, + removeProcessed: async (file) => { removed.push(file.id); return true; }, }); assert.equal(slackCalls, 1); assert.deepEqual(result.transcripts.map((item) => item.text), ["local text", "Slack text"]); assert.equal(result.localFailed[0].name, "bad.m4a"); + assert.deepEqual(removed, ["GOOD", "BAD"]); + assert.deepEqual(result.cleanupFailed, []); +}); + +test("an audio source is retained when neither local nor Slack processing succeeds", async () => { + let removals = 0; + const result = await resolveAudioTranscripts([{ id: "BAD", name: "bad.m4a" }], { + localEnabled: true, + downloadLocal: async (file) => ({ ...file, path: "/tmp/BAD.m4a" }), + localTranscriber: async () => ({ transcripts: [], failed: [{ name: "bad.m4a", reason: "Whisper failed" }] }), + slackTranscriber: async () => ({ transcripts: [], failed: [{ name: "bad.m4a", reason: "Slack failed" }] }), + removeProcessed: async () => { removals += 1; return true; }, + }); + assert.equal(result.transcripts.length, 0); + assert.equal(result.failed.length, 1); + assert.equal(removals, 0); }); From 93a3826b31f704890fd595bbc96dc9583bb22198 Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Mon, 14 Sep 2026 20:31:48 +0300 Subject: [PATCH 08/25] feat: warn about shared working folders Signed-off-by: Tiberiu Socaci --- CHANGELOG.md | 3 + FEATURES.md | 5 ++ TEST-PLAN.md | 31 ++++++- public/app.js | 52 ++++++++++-- public/index.html | 1 + public/styles.css | 8 ++ src/gateway/workspace-assignments.js | 43 ++++++++++ src/web/routes/channels.js | 11 ++- src/web/routes/settings.js | 5 +- test/channel-workdir-ui.test.js | 2 +- test/workspace-conflict-admin.test.js | 107 ++++++++++++++++++++++++ test/workspace-conflict-browser.test.js | 81 ++++++++++++++++++ 12 files changed, 336 insertions(+), 13 deletions(-) create mode 100644 src/gateway/workspace-assignments.js create mode 100644 test/workspace-conflict-admin.test.js create mode 100644 test/workspace-conflict-browser.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index d3db1426..cf37388e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog — ChannelGate +- Highlight every conversation that shares its resolved working folder with another conversation + in red in the Admin UI, and warn in red while browsing a folder that is already assigned elsewhere. + - Remove gateway-downloaded audio after a successful local or Slack-fallback transcript, while retaining failed inputs for retry and refusing symlinks or paths outside managed uploads. Teach the built-in video-understanding workflow to remove only successfully processed uploaded source diff --git a/FEATURES.md b/FEATURES.md index 5cae0dd8..0f979a4f 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -3068,6 +3068,11 @@ are retired, bullet by bullet; everything else stands. - Skills admin saves refresh existing workspaces immediately. Boot and a five-second daemon reconciliation pass refresh changed catalog/template/grant state, including MCP changes. Failed writes are reported and retried; conflicting selections for a shared folder are reported. +- The Admin UI makes shared-folder assignments visible before they break a run. Every channel or + DM whose effective working folder is assigned to another conversation gets a red warning row in + Conversations. The working-folder browser shows a red, named warning whenever the directory being + viewed is already assigned elsewhere, before **Use this folder** can create another shared + assignment. Both views use the runtime's resolved path logic rather than raw string comparison. - Runtime settings provide **Reset to default** beside **Browse**. It clears only the custom working folder through the normal Save/Discard flow; the default remains `~/ChannelGate///`. Existing files stay in their original location. diff --git a/TEST-PLAN.md b/TEST-PLAN.md index 0d121af1..9626ac78 100644 --- a/TEST-PLAN.md +++ b/TEST-PLAN.md @@ -192,12 +192,37 @@ observed tool/process/filesystem evidence, and verdict. A missing, skipped, or b is not a pass. Maintain the private QA registry alongside these reusable public instructions. -## Slack shared-folder conflict replies +## Shared-folder conflict detection and warnings -Automated regression: `node --test test/workspace-conflict-reply.test.js test/skills-workspace-sync.test.js`. +Automated regression: `node --test test/workspace-conflict-reply.test.js test/skills-workspace-sync.test.js +test/workspace-conflict-admin.test.js`. Exercises real registration and conflict detection for skill and memory mismatches, root/thread routing, mention and typed `/mode`, duplicate delivery, unauthorized authors, Slack delivery failure, -and preservation of a workspace sentinel. No engine starts; this behavior is engine-independent. +preservation of a workspace sentinel, API annotations for shared channels and DMs, and folder-browser +assignment discovery. The UI regression pins the red list-row and browser-message states. No engine +starts; this behavior is engine-independent. + +- [x] Browser (engine-independent, disposable Chromium fixture): + `CG_BROWSER_MODULE=/path/to/playwright/index.mjs node --test + test/workspace-conflict-browser.test.js` creates two conversations on one real folder, a control + on a separate folder and one unused folder. It requires exactly the duplicate rows to render red, + requires the folder modal to name only other assignments, verifies used/control/unused navigation, + saves the unused folder and waits for every stale red row to clear without reload. Browser errors + fail the case; no engine is spawned. +- [x] Airtable: active engine-independent live definition `UI-WORKDIR-CONFLICT-01` mirrors the + Admin UI shared-folder setup, navigation, immediate refresh and evidence requirements below. + +Admin UI live release gate — **UNEXECUTED** (engine-independent). On a disposable deployment, create +three conversations: `folder-warning-a` and `folder-warning-b` use the same real folder, while +`folder-warning-control` uses a separate folder. Reload Conversations. Require A and B—but not the +control—to have red rows, a visible warning mark and “Shared working folder”; hovering each warning +must name the other assignment. Open A → Runtime → Browse and navigate to the shared folder: require +a red message naming B before selection. Navigate to the control's folder and an unused folder: the +warning must update to name the control for the former and disappear for the latter. Open the control +and browse the shared folder: require both A and B named before **Use this folder**. Reset B to its +default folder, save and reload; A and B must return to normal rows and browsing A's folder must no +longer warn about B. Record candidate revision, screenshots at desktop and narrow widths, API payloads +with no secret values, and browser-console output in the private QA registry before marking passed. Live release gate — **UNEXECUTED** (engine-independent: registration fails before engine selection). On a disposable deployment, create two synthetic Slack test channels and register both. Assign both diff --git a/public/app.js b/public/app.js index 26483430..51f63bc9 100644 --- a/public/app.js +++ b/public/app.js @@ -991,6 +991,18 @@ async function openActiveSessions() { } // ── Conversations (master–detail): templates + channels + DMs in one list ───────── +async function refreshConversationRows() { + try { + const [{ channels }, { dms }] = await Promise.all([api("/api/channels"), api("/api/dms")]); + CHANNELS = channels; + DMS = dms; + renderConvList(); + return true; + } catch { + return false; + } +} + async function loadConversations() { const [{ channels }, { dms }, s] = await Promise.all([api("/api/channels"), api("/api/dms"), api("/api/settings")]); CHANNELS = channels; @@ -1046,16 +1058,29 @@ function conversationExists(key) { return false; } -function convRow(key, color, name, sub, cost) { +function workDirConflictSummary(conflict) { + const others = Array.isArray(conflict?.conversations) ? conflict.conversations : []; + if (!others.length) return ""; + const names = others.map((item) => item.isDM || item.type === "im" + ? (item.name || item.slug) + : hashName(item.name || item.slug)); + return `Working folder is also assigned to ${names.join(", ")}`; +} + +function convRow(key, color, name, sub, cost, conflict = null) { const el = document.createElement("a"); - el.className = "list-item conv-item" + (selectedConv === key ? " active" : ""); + const conflictText = workDirConflictSummary(conflict); + el.className = "list-item conv-item" + (conflictText ? " workdir-conflict" : "") + (selectedConv === key ? " active" : ""); el.href = conversationPathForKey(key); + if (conflictText) el.title = conflictText; // Compact whole-dollar 30-day cost (skip sub-$1 rows so the list stays quiet). const dollars = cost == null ? null : Math.round(cost); const costHtml = dollars && dollars >= 1 ? `$${escapeHtml(dollars.toLocaleString())}` : ""; + const conflictHtml = conflictText ? '!' : ""; el.innerHTML = `` + - `${escapeHtml(name)}${sub ? `${escapeHtml(sub)}` : ""}` + + `${escapeHtml(name)}${sub ? `${escapeHtml(sub)}` : ""}${conflictText ? 'Shared working folder' : ""}` + + conflictHtml + costHtml; el.addEventListener("click", (e) => { if (e.button !== 0 || e.metaKey || e.ctrlKey || e.shiftKey || e.altKey) return; @@ -1090,7 +1115,7 @@ function renderConvList() { e.textContent = CHANNELS.length ? "No channels match." : "No channels yet — invite the bot and send a message."; list.appendChild(e); } else { - for (const c of chans) list.appendChild(convRow("ch:" + c.channelId, capColorOf(c.meta || {}), hashName(c.name || c.slug), capLabelOf(c.meta || {}), costFor(c.channelId, c.slug))); + for (const c of chans) list.appendChild(convRow("ch:" + c.channelId, capColorOf(c.meta || {}), hashName(c.name || c.slug), capLabelOf(c.meta || {}), costFor(c.channelId, c.slug), c.workDirConflict)); } } @@ -1112,7 +1137,7 @@ function renderConvList() { for (const d of dmItems) { const tplName = d.template === "admin" ? "Admin template" : d.template === "custom" ? "Custom" : "User template"; const capMeta = d.template === "custom" ? d.meta || {} : DM_TEMPLATES[d.template] || {}; - list.appendChild(convRow("dm:" + d.channelId, capColorOf(capMeta), d.userName || d.slug, tplName, costFor(d.channelId, d.slug))); + list.appendChild(convRow("dm:" + d.channelId, capColorOf(capMeta), d.userName || d.slug, tplName, costFor(d.channelId, d.slug), d.workDirConflict)); } } } @@ -1860,7 +1885,9 @@ function renderChannelDetail(ch) { makeToolboxState.textContent = ch.meta.hasMakeToolboxKey ? "saved" : "not configured"; clearMakeToolbox = false; paintModePill(ch.meta); - renderConvList(); + // A work-folder save can add or remove warnings on several rows at once. Refresh both + // conversation collections from the authoritative server rather than repainting stale flags. + if (!(await refreshConversationRows())) renderConvList(); detailDirty = false; savebarMsg.textContent = "Saved"; savebarMsg.classList.add("clean"); @@ -3624,6 +3651,19 @@ async function fsBrowse(p) { const data = await api(`/api/fs/list${p ? `?path=${encodeURIComponent(p)}` : ""}`); fsCurrentPath = data.path; document.getElementById("fs-current").textContent = data.path; + const conflict = document.getElementById("fs-conflict"); + const otherAssignments = (Array.isArray(data.assignedConversations) ? data.assignedConversations : []) + .filter((item) => `${item.isDM || item.type === "im" ? "dm" : "ch"}:${item.channelId}` !== selectedConv); + if (otherAssignments.length) { + const names = otherAssignments.map((item) => item.isDM || item.type === "im" + ? (item.name || item.slug) + : hashName(item.name || item.slug)); + conflict.textContent = `Already assigned to ${names.join(", ")}. Selecting it here will share one working folder between conversations.`; + conflict.hidden = false; + } else { + conflict.textContent = ""; + conflict.hidden = true; + } document.getElementById("fs-up").disabled = !data.parent; document.getElementById("fs-up").dataset.parent = data.parent || ""; const list = document.getElementById("fs-list"); diff --git a/public/index.html b/public/index.html index 8af15806..ac44bf0e 100644 --- a/public/index.html +++ b/public/index.html @@ -964,6 +964,7 @@

Danger zone