From 2e1da92e70ef46b3851ee08b1cf8b106cd3eadd3 Mon Sep 17 00:00:00 2001 From: Tiberiu Socaci Date: Sun, 13 Sep 2026 21:43:45 +0300 Subject: [PATCH] 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 892043e..4fbc3ab 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 18898ce..4612f33 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 12939ee..1ce8ef0 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 0f817f6..a4bdefd 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:/); + } +});