From 0e630f1bbfc6181ebc9370e7f9b88edf9db79d89 Mon Sep 17 00:00:00 2001 From: Darwin Wu Date: Thu, 23 Jul 2026 15:57:10 -0700 Subject: [PATCH 1/2] fix(message): enforce safe mode for CI compose In CI, message compose skips the editor and sends immediately. That bypasses safe mode, which is supposed to require human review. When safe mode and CI are both active, reject compose before resolving a workspace or calling Slack. Interactive compose and CI without safe mode are unchanged. --- README.md | 1 + skills/agent-slack/SKILL.md | 2 +- src/cli/message-command.ts | 5 +++++ src/index.ts | 2 +- test/safe-mode.test.ts | 45 ++++++++++++++++++++++++++++++++++++- 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7fb9c07..1d9545d 100644 --- a/README.md +++ b/README.md @@ -274,6 +274,7 @@ agent-slack --safe-mode message send "#general" "hello" While safe mode is active: - `message send` → redirected to the draft editor with the text pre-filled; you review and send from the browser. The output includes `"safe_mode": true` and `"redirected_from": "send"`, and a warning is printed to stderr. Flags the editor cannot represent (`--attach`, `--blocks`, `--schedule`, `--schedule-in`, `--reply-broadcast`) are rejected with an error instead of being silently dropped. +- `message compose` → opens the browser editor normally. In CI, where compose would skip the editor and send directly, it is blocked with an error. - `message edit` and `message delete` → blocked with an error. - All read operations (`get`, `list`, `search`, etc.) and reactions are unchanged. diff --git a/skills/agent-slack/SKILL.md b/skills/agent-slack/SKILL.md index c1143e1..972b94c 100644 --- a/skills/agent-slack/SKILL.md +++ b/skills/agent-slack/SKILL.md @@ -21,7 +21,7 @@ If a capability named here is absent from installed help, report version skew in - Read and search freely. - Perform write actions only when explicitly requested: sends, edits, deletes, reactions, invitations, channel or canvas creation/editing, mark-read operations, scheduling or canceling delivery, uploads, Later state/reminder changes, DM/group-DM creation, and `workflow run`. Workflow runs can execute downstream actions. - For compose- or review-only requests, return proposed text without invoking Slack, or use `message draft create` to add a Slack-native draft the user can review and send (nothing is posted). `message compose` is send-capable; use it only when the user explicitly asks to open the interactive editor. In CI or another noninteractive environment, do not invoke it without separate authorization to send immediately: CI skips the editor and sends supplied text. -- With `AGENT_SLACK_SAFE_MODE=1` (or the global `--safe-mode` flag) set, safe mode is enforced at the tool level: `message send` is redirected to the draft editor and `message edit`/`message delete` are blocked. Use it when nothing should post without human review. +- With `AGENT_SLACK_SAFE_MODE=1` (or the global `--safe-mode` flag) set, safe mode is enforced at the tool level: `message send` is redirected to the draft editor, the CI `message compose` direct-send shortcut is blocked, and `message edit`/`message delete` are blocked. Use it when nothing should post without human review. ## Workflow diff --git a/src/cli/message-command.ts b/src/cli/message-command.ts index b790e60..8504e79 100644 --- a/src/cli/message-command.ts +++ b/src/cli/message-command.ts @@ -295,6 +295,11 @@ export function registerMessageCommand(input: { program: Command; ctx: CliContex { workspace?: string; threadTs?: string; unfurl?: boolean }, ]; try { + if (safeModeActive() && process.env.CI) { + throw new Error( + 'Safe mode is active: "message compose" cannot skip the editor in CI because that would post without human review.', + ); + } const payload = await composeMessage({ ctx: input.ctx, targetInput, diff --git a/src/index.ts b/src/index.ts index 7e61630..fedbb7a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -59,7 +59,7 @@ program .version(getPackageVersion()) .option( "--safe-mode", - 'Human-in-the-loop enforcement: redirect "message send" to the draft editor and block "message edit"/"message delete" (also: AGENT_SLACK_SAFE_MODE=1)', + 'Human-in-the-loop enforcement: redirect "message send" to the draft editor, block the CI "message compose" direct-send shortcut, and block "message edit"/"message delete" (also: AGENT_SLACK_SAFE_MODE=1)', ); startCommandWatchdog(process.argv.slice(2)); diff --git a/test/safe-mode.test.ts b/test/safe-mode.test.ts index 5345621..328e104 100644 --- a/test/safe-mode.test.ts +++ b/test/safe-mode.test.ts @@ -1,5 +1,7 @@ -import { describe, expect, test } from "bun:test"; +import { describe, expect, mock, test } from "bun:test"; +import { Command } from "commander"; import type { CliContext } from "../src/cli/context.ts"; +import { registerMessageCommand } from "../src/cli/message-command.ts"; import { isSafeModeEnabled, redirectSendToDraft, @@ -35,6 +37,47 @@ describe("safeModeBlockedError", () => { }); }); +test("safe mode blocks CI compose before workspace or API work", async () => { + const originalCi = process.env.CI; + const originalExitCode = process.exitCode; + const originalLog = console.log; + const originalError = console.error; + const log = mock(() => {}); + const error = mock(() => {}); + const noWorkCtx = { + errorMessage: (err: unknown) => (err instanceof Error ? err.message : String(err)), + } as CliContext; + + try { + process.env.CI = "1"; + process.exitCode = 0; + console.log = log as typeof console.log; + console.error = error as typeof console.error; + + const program = new Command().option("--safe-mode"); + registerMessageCommand({ program, ctx: noWorkCtx }); + await program.parseAsync( + ["--safe-mode", "message", "compose", "C12345678", "review this first"], + { from: "user" }, + ); + + expect(log).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith( + 'Safe mode is active: "message compose" cannot skip the editor in CI because that would post without human review.', + ); + expect(process.exitCode).toBe(1); + } finally { + if (originalCi === undefined) { + delete process.env.CI; + } else { + process.env.CI = originalCi; + } + process.exitCode = originalExitCode ?? 0; + console.log = originalLog; + console.error = originalError; + } +}); + describe("redirectSendToDraft", () => { test.each([ [{ attach: ["./report.md"] }, "--attach"], From 91f8c56ce3162094335e3bcdf89eb9fe4cfc82d1 Mon Sep 17 00:00:00 2001 From: Darwin Wu Date: Fri, 11 Sep 2026 16:38:12 -0700 Subject: [PATCH 2/2] test: simplify safe mode CI setup --- test/safe-mode.test.ts | 44 +++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/test/safe-mode.test.ts b/test/safe-mode.test.ts index 328e104..fc20e52 100644 --- a/test/safe-mode.test.ts +++ b/test/safe-mode.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, mock, test } from "bun:test"; +import { describe, expect, spyOn, test } from "bun:test"; import { Command } from "commander"; import type { CliContext } from "../src/cli/context.ts"; import { registerMessageCommand } from "../src/cli/message-command.ts"; @@ -38,43 +38,31 @@ describe("safeModeBlockedError", () => { }); test("safe mode blocks CI compose before workspace or API work", async () => { - const originalCi = process.env.CI; const originalExitCode = process.exitCode; - const originalLog = console.log; - const originalError = console.error; - const log = mock(() => {}); - const error = mock(() => {}); + const error = spyOn(console, "error").mockImplementation(() => {}); const noWorkCtx = { errorMessage: (err: unknown) => (err instanceof Error ? err.message : String(err)), } as CliContext; try { - process.env.CI = "1"; - process.exitCode = 0; - console.log = log as typeof console.log; - console.error = error as typeof console.error; + await withEnvironment({ CI: "1" }, async () => { + process.exitCode = 0; - const program = new Command().option("--safe-mode"); - registerMessageCommand({ program, ctx: noWorkCtx }); - await program.parseAsync( - ["--safe-mode", "message", "compose", "C12345678", "review this first"], - { from: "user" }, - ); + const program = new Command().option("--safe-mode"); + registerMessageCommand({ program, ctx: noWorkCtx }); + await program.parseAsync( + ["--safe-mode", "message", "compose", "C12345678", "review this first"], + { from: "user" }, + ); - expect(log).not.toHaveBeenCalled(); - expect(error).toHaveBeenCalledWith( - 'Safe mode is active: "message compose" cannot skip the editor in CI because that would post without human review.', - ); - expect(process.exitCode).toBe(1); + expect(error).toHaveBeenCalledWith( + 'Safe mode is active: "message compose" cannot skip the editor in CI because that would post without human review.', + ); + expect(process.exitCode).toBe(1); + }); } finally { - if (originalCi === undefined) { - delete process.env.CI; - } else { - process.env.CI = originalCi; - } process.exitCode = originalExitCode ?? 0; - console.log = originalLog; - console.error = originalError; + error.mockRestore(); } });