From 9d04a4fb02b31a5c56b482cfdbdc3a89609994da Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 4 Nov 2026 17:02:05 +0100 Subject: [PATCH] feat: drop the typed first-message hint from the Discord bridge (#741) Owner decision. Typing the note into a brand-new non-Claude tab spent a model turn per tab and, for a spawned worker or a scheduled run, landed with the first real prompt. Claude keeps its hint through the system prompt, and every other CLI is told nothing. The setting goes with the path it gated: sessionIdle, the fresh set, isHintableSession and the session:idle wiring. sendInput loses `fromUser`, which was only ever false for the hint, and the deps test pins that a reply is a person's own input. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- docs/api-reference.md | 2 +- docs/discord-bridge.md | 3 +- docs/security-architecture.md | 2 +- docs/wiki/Discord-Bridge.md | 4 +- src/discord-bridge.ts | 41 +++----------- src/types/discord.ts | 5 -- src/web/public/index.html | 7 --- src/web/public/settings-ui.js | 2 - src/web/schemas.ts | 1 - src/web/server.ts | 41 +++----------- test/discord-bridge.test.ts | 90 +++++------------------------- test/discord-push-glue.test.ts | 32 ++++------- test/routes/discord-routes.test.ts | 8 +-- 14 files changed, 51 insertions(+), 189 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 685c3f5c..49040c40 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -131,7 +131,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Agent CLI (`codeman agent …`, `src/cli-agent.ts`)**: the command-line client for the wait primitives and the session verbs, for agents the skill does not reach (pi, shell, no `jq`; `seedAgentSessionPreamble` covers every local session whose CLI takes the skill, `capabilities.agentSkillDir`; every pane still exports `CODEMAN_MUX`/`CODEMAN_SESSION_ID`/`CODEMAN_API_URL`). It is an HTTP CLIENT: inspection and guarded stage/submit use the ownership-checked `agent-session-routes.ts` module (experimental, behind `agentCoordinationEnabled`, SYNCED, default OFF: all three routes answer `403 FORBIDDEN` naming it while off, and the CLI shows that verbatim); the other verbs reuse existing routes, with no second transport (the plan's "Socket API" non-goal stands), every call carries `X-Codeman-Parent-Session`, only `spawn`'s quick-start carries `X-Codeman-Agent-Origin` (pinned in test/cli-agent.test.ts), while `spawn` without a case (cwd or `--working-dir`) is `POST /api/v1/sessions` plus `/interactive` or `/shell` without that header, so no agent-case marker, and refuses (exit 4, before any request) an empty `--working-dir` and a bare spawn from a HOME cwd, since it always sends an explicit `workingDir` and the server's no-`workingDir` hooks skip cannot apply (test/cli-agent-project-spawn.test.ts), auth (`src/codeman-credentials.ts`, shared with `codeman attach` and the TUI) from `CODEMAN_PASSWORD` or the data dir's `.env` (the same reader `codeman attach` uses — `readCodemanEnvFile`). Verbs return exit codes (`0` ok / `1` error / `2` timeout / `3` exited / `4` refused) instead of calling `process.exit`, so `test/cli-agent.test.ts` drives every verb against a recording fake transport plus the real `httpRequest` against a local server. ⚠️ Three guards live in code, not prose, and the tests pin them: refuse without `CODEMAN_MUX=1`+`CODEMAN_API_URL` (never guess a URL); `send` transmits printable text + `\r` only (`inputRefusal`: any byte < 0x20 or DEL refuses BEFORE the transport — opencode's `Ctrl+C` is `app_exit`), ESC exists solely as `interrupt` with no `\r`; `rm` fails closed (`deleteRefusal`: empty id, self shorter than 8, prefix match in EITHER direction, re-checked on the resolved id). ⚠️ The routes accept FULL ids only — an 8-char prefix is a 404 (measured live) — while `ls` prints prefixes, so `resolveSessionId` maps a short id through `GET /api/v1/sessions` and refuses an ambiguous one. It also refuses any id under 8 characters (exit 4, on every verb, before any request), the same floor as the server's `PARENT_SESSION_ID_MIN_PREFIX`, so `rm 9` can never pick an arbitrary session. ⚠️ A 400 for `--until stop` on a hook-less mode is passed through verbatim, never papered over: the marker path is the answer there. `spawn` on a readiness timeout exits 2 and LEAVES the session for inspection (the skill's `spawn_worker` deletes it; the CLI's caller is often a human); its stdout is the id ALONE (prose on stderr) so `SID=$(…)` works. ⚠️ Two server answers that look like success and are not: `delivered:false` without `duplicate` (the bytes went into a dead pane — the field exists so a client says "restart" instead of "wait longer") and `wait.ended:true, signal:null` (the worker died during a `stop`/`idle` wait: the registry satisfies only waiters that listed `exit`, then cancels the rest) — both exit 3, checked BEFORE the happy paths in `waitExitCode`. → README "`codeman agent`", `docs/api-reference.md` -**Discord bridge (`src/discord-bridge.ts`, `src/web/routes/discord-routes.ts`, `src/cli-notify.ts`)**: opt-in two-way Discord (a bot, not the one-way webhook), one thread per tab. Split like `webhook-notify.ts`: pure helpers, a 0600 store (`discord.json` holds the bot token and is never returned; `discord-bridge.json` maps sessionId to thread id), then IO (`DiscordApi` REST with injected fetch, `DiscordGateway` inbound WebSocket with injected socket, `DiscordBridge` rules). Outbound hangs off `WebServer.sendPushNotifications` beside the webhook (so it works with zero push subscriptions) and tab events off `WebServer.broadcast` (`bridgeSessionEvent`); a finished turn (Claude's idle and stop hooks, DeepSeek's stop; the other CLIs raise no turn end, so theirs arrive with an error, a blocked respawn, a crash loop or `codeman notify`) or an error posts the CLI's WHOLE last message as markdown, read from its transcript through the response viewer's reader (`WebServer.lastResponseFor`, set by the session routes: the CLI's own transcript, or the pane segmented into turns for the CLIs that keep none; never for a shell), split over posts of at most 1900 characters and not posted twice (keyed on the text plus its turn, recorded only after the first post succeeds), while a dialog and every CLI without a transcript post the RENDERED tmux screen (`screenTail`, with the composer row and the status lines under it cut), never the raw stream, which a full-screen UI repaints in place; `staleThreads` (default `keep`) archives or deletes threads the bot owns that no tab maps to, after boot and then daily, never one under an hour old, never with an empty map and at most 25 per run. Inbound replies are typed through `session.writeViaMux`. ⚠️ Guards that live in code and tests pin: only `allowedUserIds`, in `guildId`, inside a thread the bridge created, are acted on, everything else is dropped unread; a reply is one printable line plus Enter (a lone digit while a permission or question is pending goes as the key alone, since the dialog acts on it), capped and rate limited, and `discord-notify` is a per-session token bucket (6 a minute, 429 `RATE_LIMITED`; `fileProblem` probes the folder with the bounded probe first); posts send `allowed_mentions` limited to the allowlisted ids; received files go only from Discord's own HTTPS hosts (max 5 x 8 MB, names reduced to `[A-Za-z0-9._-]`) into `/.codeman-discord/`, and a sent file's REAL path must lie inside the tab's folder (`fileProblem`, symlinks followed); a Discord failure never reaches a session; the alert tail is a function the bridge calls only once it will post (no tmux exec per event while off); a delete reaches the bridge from the cleanup path with `killMux` true, never from `session:deleted`, which a detach also fires; `.codeman-discord/` is probed with the bounded probe, refused if a symlink, and ignores itself with a `.gitignore` of `*`; a config restore of `discord.json` reloads the running bridge (`afterRestore`). The startup hint ("how to reach the user") rides the CLI registry as the engine value `appendSystemPrompt` (Claude's `--append-system-prompt`, undefined unless the bridge and `startupHint` are on, so every other launch is byte-identical) and, only with the separate `typedHint` setting (default OFF), is typed once as a first message to brand-new external CLI tabs on their first idle (`sessionIdle`; never a shell, a restored tab, or a remote (ssh) or Docker tab, which have no `codeman` on their PATH). `codeman notify "" [--urgent] [--file ]` wraps `POST /api/sessions/:id/discord-notify`, which accepts a unique 8-character id prefix (a Docker case exports it cut). → `docs/discord-bridge.md`, `docs/wiki/Discord-Bridge.md`, `docs/api-reference.md` +**Discord bridge (`src/discord-bridge.ts`, `src/web/routes/discord-routes.ts`, `src/cli-notify.ts`)**: opt-in two-way Discord (a bot, not the one-way webhook), one thread per tab. Split like `webhook-notify.ts`: pure helpers, a 0600 store (`discord.json` holds the bot token and is never returned; `discord-bridge.json` maps sessionId to thread id), then IO (`DiscordApi` REST with injected fetch, `DiscordGateway` inbound WebSocket with injected socket, `DiscordBridge` rules). Outbound hangs off `WebServer.sendPushNotifications` beside the webhook (so it works with zero push subscriptions) and tab events off `WebServer.broadcast` (`bridgeSessionEvent`); a finished turn (Claude's idle and stop hooks, DeepSeek's stop; the other CLIs raise no turn end, so theirs arrive with an error, a blocked respawn, a crash loop or `codeman notify`) or an error posts the CLI's WHOLE last message as markdown, read from its transcript through the response viewer's reader (`WebServer.lastResponseFor`, set by the session routes: the CLI's own transcript, or the pane segmented into turns for the CLIs that keep none; never for a shell), split over posts of at most 1900 characters and not posted twice (keyed on the text plus its turn, recorded only after the first post succeeds), while a dialog and every CLI without a transcript post the RENDERED tmux screen (`screenTail`, with the composer row and the status lines under it cut), never the raw stream, which a full-screen UI repaints in place; `staleThreads` (default `keep`) archives or deletes threads the bot owns that no tab maps to, after boot and then daily, never one under an hour old, never with an empty map and at most 25 per run. Inbound replies are typed through `session.writeViaMux`. ⚠️ Guards that live in code and tests pin: only `allowedUserIds`, in `guildId`, inside a thread the bridge created, are acted on, everything else is dropped unread; a reply is one printable line plus Enter (a lone digit while a permission or question is pending goes as the key alone, since the dialog acts on it), capped and rate limited, and `discord-notify` is a per-session token bucket (6 a minute, 429 `RATE_LIMITED`; `fileProblem` probes the folder with the bounded probe first); posts send `allowed_mentions` limited to the allowlisted ids; received files go only from Discord's own HTTPS hosts (max 5 x 8 MB, names reduced to `[A-Za-z0-9._-]`) into `/.codeman-discord/`, and a sent file's REAL path must lie inside the tab's folder (`fileProblem`, symlinks followed); a Discord failure never reaches a session; the alert tail is a function the bridge calls only once it will post (no tmux exec per event while off); a delete reaches the bridge from the cleanup path with `killMux` true, never from `session:deleted`, which a detach also fires; `.codeman-discord/` is probed with the bounded probe, refused if a symlink, and ignores itself with a `.gitignore` of `*`; a config restore of `discord.json` reloads the running bridge (`afterRestore`). The startup hint ("how to reach the user") rides the CLI registry as the engine value `appendSystemPrompt` (Claude's `--append-system-prompt`, undefined unless the bridge and `startupHint` are on, so every other launch is byte-identical) No other CLI is told. `codeman notify "" [--urgent] [--file ]` wraps `POST /api/sessions/:id/discord-notify`, which accepts a unique 8-character id prefix (a Docker case exports it cut). → `docs/discord-bridge.md`, `docs/wiki/Discord-Bridge.md`, `docs/api-reference.md` **MCP server (`codeman mcp`, `src/mcp-server.ts`)**: a Model Context Protocol server on stdio for any MCP client, a CLIENT of the HTTP API like `codeman agent` (reuses its `httpRequest`, `inputRefusal`, `buildSendBody`; the MCP server adds no routes). Seven tools: `list_sessions`, `start_session`, `send_prompt`, `wait_for_session`, `read_output`, `interrupt_session`, `delete_session`. ⚠️ Dual-era: answers the legacy `initialize` handshake (2024-11-05 to 2025-11-25) AND the 2026-07-28 revision (per-request `_meta` with `protocolVersion` + `clientCapabilities`, `server/discover`, `resultType: "complete"` on every modern result); a modern `tools/list`/`server/discover` MUST carry `ttlMs` + `cacheScope` (Claude Code 2.1.296 drops every tool without them: measured). ⚠️ stdout is protocol only; logs go to stderr. ⚠️ The URL comes from `--url`/`CODEMAN_API_URL`, never a guess, and unconfigured it still lists tools (registries introspect) while every call answers how to configure it. ⚠️ Prompt/interrupt/delete only sessions the PROCESS started (in-memory allowlist, so a restart fails closed) or, inside a pane, the calling session's own children, unless `--allow-any-session`; never its own session. ⚠️ `read_output` returns text ONLY, no `structuredContent` (a client that prefers it showed the model metadata instead of the output: measured). Waits default 45 s (Codex's MCP tool timeout is 60 s). Registry manifest `config/mcp-registry/server.json` (`mcpName` in package.json, version kept by `scripts/sync-plugin.mjs`). Tests: `test/mcp-server.test.ts`. diff --git a/docs/api-reference.md b/docs/api-reference.md index f60eff70..c8304635 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1530,7 +1530,7 @@ One thread per tab in a private Discord channel, alerts out and replies typed ba | Method | Path | Body | Notes | | ------ | ------ | ------ | ------ | | `GET` | `/api/discord` | none | Config without the token, plus `hasToken`, `tokenMasked`, `gateway` (`off` \| `connecting` \| `connected` \| `error`), `gatewayError` and `lastResult`. | -| `PUT` | `/api/discord` | `{ enabled?, token?, guildId?, parentChannelId?, allowedUserIds?, threadCreation?, onSessionDelete?, staleThreads?, pingMode?, scope?, startupHint?, typedHint? }` (strict) | `threadCreation`: `lazy` \| `onSessionStart`. `onSessionDelete`: `keep` \| `archive` \| `delete`. `pingMode`: `off` \| `critical` \| `all`. `scope`: `attention` \| `all`. An absent `token` keeps the saved one; `""` clears it. `400` for a non-numeric id or enabling without a token, guild, channel and at least one allowed user. | +| `PUT` | `/api/discord` | `{ enabled?, token?, guildId?, parentChannelId?, allowedUserIds?, threadCreation?, onSessionDelete?, staleThreads?, pingMode?, scope?, startupHint? }` (strict) | `threadCreation`: `lazy` \| `onSessionStart`. `onSessionDelete`: `keep` \| `archive` \| `delete`. `pingMode`: `off` \| `critical` \| `all`. `scope`: `attention` \| `all`. An absent `token` keeps the saved one; `""` clears it. `400` for a non-numeric id or enabling without a token, guild, channel and at least one allowed user. | | `POST` | `/api/discord/test` | none | Posts one message to the parent channel with the saved config, even while disabled. `200` with `data.ok`; `400` if no token or channel is saved. | | `POST` | `/api/sessions/:id/discord-notify` | `{ message, urgent?, file? }` (strict, message 1 to 1500 characters) | A CLI asks for the user: posts into that tab's thread, pinging when `urgent`. `file` is an absolute path inside the tab's working folder (real path, regular file, at most 8 MB, local sessions only) uploaded with the post; `400` otherwise. A `200` carries the delivery result: `data.ok` is `false` (with `data.error`) when Discord refused or failed the post. `409` while the bridge is disabled, `429` past 6 posts a minute per tab. The `codeman notify "message" [--urgent] [--file path]` command wraps this route. | diff --git a/docs/discord-bridge.md b/docs/discord-bridge.md index a36088d3..161f533f 100644 --- a/docs/discord-bridge.md +++ b/docs/discord-bridge.md @@ -97,14 +97,13 @@ In the Discord mobile app set the server to **All Messages**. Discord's own push | `pingMode` | `off`, `critical`, `all` | `critical` | `critical` pings you for permission prompts, questions, errors and urgent posts | | `scope` | `attention`, `all` | `attention` | `attention` skips the quiet "response complete" event. `all` sends it too | | `startupHint` | on, off | on | Tell Claude about `codeman notify` in its system prompt at launch | -| `typedHint` | on, off | off | Also send the note once, as a first message, to brand-new non-Claude CLI tabs | ## Telling the CLIs A CLI only posts deliberately if it knows it can. While the bridge is on: - **Claude** (`startupHint`, on by default) is launched with `--append-system-prompt`, so nothing shows in the conversation. This reaches local tabs only: remote (ssh) and Docker launches are built outside the CLI registry, and have no `codeman` command anyway. -- **Every other CLI** is sent nothing unless you turn on `typedHint` (off by default: it costs one model turn per tab, and for a spawned worker or a scheduled run it arrives together with the first real prompt). With it on, the same text is sent once, as a message, the first time a brand-new tab becomes idle. Tabs that are restored when Codeman starts, shell tabs, and remote (ssh) and Docker tabs (which have no `codeman` command) are never sent it. +- **Every other CLI** is sent nothing: there is no silent way to tell it, and typing the note as a message would cost a model turn per tab. It can still run `codeman notify` if you tell it to. The text explains `codeman notify "message" --urgent`. `codeman` is on the PATH of every session from the installer or an npm install (a git checkout run without the installer has no `codeman` command) and reads `CODEMAN_SESSION_ID` and `CODEMAN_API_URL`, so no other setup is needed. Without a bridge it prints that the bridge is off and exits cleanly. Tabs started before the bridge was enabled are not told; use the skill or `CLAUDE.md` for those. diff --git a/docs/security-architecture.md b/docs/security-architecture.md index 3c68a096..f25f843f 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -582,7 +582,7 @@ Opt-in and off by default: the server POSTs the Web Push events (permission prom - **A second server-side outbound channel through the web-tab egress guard (§10b).** Delivery goes through `webviewFetch`, so link-local and cloud-metadata targets are refused at save time and again on the RESOLVED address at connect time; redirects are not followed (`redirect: 'manual'`) and each send is bounded by a 5 s timeout. Loopback and RFC1918 stay allowed on purpose (a self-hosted ntfy is the point), so **Send test** works as a blind reachability probe (status, refused or timed out, never a response body) for whoever may call it. Web tabs already give that caller full LAN reach with bodies, so nothing new is exposed. - **The URL is a bearer secret** (anyone holding a Slack or Discord webhook URL can post as it). It lives in `~/.codeman/webhook.json` (0600, tmp+rename), is kept out of `settings.json` (which every logged-in user reads through `GET /api/settings`), is never returned (`GET /api/webhook` gives scheme + host only), and never appears in a log line, a delivery result or an error message. -- **Discord bridge** (`src/discord-bridge.ts`, [setup](discord-bridge.md)): the bot token lives in `~/.codeman/discord.json` (0600, tmp+rename) and is never returned, logged or put in an error. Inbound replies are typed into a session only from `allowedUserIds`, in the configured guild, inside a thread the bridge created; anything else is dropped unread. A reply becomes one printable line plus Enter, capped at 1000 characters and one per second per tab. Posts send `allowed_mentions` limited to the allowlisted ids, so agent output cannot ping a server. **Files:** an attachment is fetched only from Discord's own HTTPS hosts (no redirects followed), at most 5 per reply and 8 MB each, saved 0600 under `/.codeman-discord/` with the name reduced to `[A-Za-z0-9._-]`; a file a CLI sends must be a regular file whose real path (symlinks followed) lies inside its own tab's folder, which stops a file outside that folder being uploaded by mistake, and remote sessions send and receive none. A received file is untrusted content to whatever opens it. **What the checks do not stop:** the agent runs as the same OS user, so the path check prevents accidental uploads, not a determined agent that pastes a file's contents into the message. **What leaves the machine:** a permission or question alert sends the rendered screen (whatever the agent printed), and a finished turn or an error sends the CLI's whole last message, read from its transcript (up to 8 posts of 1,900 characters). `idle_prompt` is a warning, so with the default scope every Claude turn left unanswered for a minute is posted, and with **Which events** set to Everything every turn end is; treat the Discord server as holding that output. **Multi-user:** the bridge is one allowlist for the whole server, so an allowlisted Discord account can type into every user's sessions; only an admin can configure it. **Startup hint:** a fixed text (no user input) is added to Claude's system prompt, or, only when `typedHint` is on, typed once to a brand-new external CLI tab, never to a shell. The bridge only dials out (REST and the gateway WebSocket), so it needs no inbound exposure. A Discord account on the allowlist can drive agents that run shell commands: it needs 2FA. +- **Discord bridge** (`src/discord-bridge.ts`, [setup](discord-bridge.md)): the bot token lives in `~/.codeman/discord.json` (0600, tmp+rename) and is never returned, logged or put in an error. Inbound replies are typed into a session only from `allowedUserIds`, in the configured guild, inside a thread the bridge created; anything else is dropped unread. A reply becomes one printable line plus Enter, capped at 1000 characters and one per second per tab. Posts send `allowed_mentions` limited to the allowlisted ids, so agent output cannot ping a server. **Files:** an attachment is fetched only from Discord's own HTTPS hosts (no redirects followed), at most 5 per reply and 8 MB each, saved 0600 under `/.codeman-discord/` with the name reduced to `[A-Za-z0-9._-]`; a file a CLI sends must be a regular file whose real path (symlinks followed) lies inside its own tab's folder, which stops a file outside that folder being uploaded by mistake, and remote sessions send and receive none. A received file is untrusted content to whatever opens it. **What the checks do not stop:** the agent runs as the same OS user, so the path check prevents accidental uploads, not a determined agent that pastes a file's contents into the message. **What leaves the machine:** a permission or question alert sends the rendered screen (whatever the agent printed), and a finished turn or an error sends the CLI's whole last message, read from its transcript (up to 8 posts of 1,900 characters). `idle_prompt` is a warning, so with the default scope every Claude turn left unanswered for a minute is posted, and with **Which events** set to Everything every turn end is; treat the Discord server as holding that output. **Multi-user:** the bridge is one allowlist for the whole server, so an allowlisted Discord account can type into every user's sessions; only an admin can configure it. **Startup hint:** a fixed text (no user input) is added to Claude's system prompt; no other CLI is sent it. The bridge only dials out (REST and the gateway WebSocket), so it needs no inbound exposure. A Discord account on the allowlist can drive agents that run shell commands: it needs 2FA. - **It carries session data to a third party.** Titles and bodies include session names, tool names and error text, all agent- or user-controlled, so Discord gets `allowed_mentions: { parse: [] }` and Slack's `& < >` are escaped: agent output cannot ping a channel. In multi-user mode all three routes are admin-only and the channel is instance-wide: it receives every user's session events, the same reach an admin's own Web Push has, which means non-admins' session details leave the box at the admin's choice. --- diff --git a/docs/wiki/Discord-Bridge.md b/docs/wiki/Discord-Bridge.md index edd2a21b..e566889b 100644 --- a/docs/wiki/Discord-Bridge.md +++ b/docs/wiki/Discord-Bridge.md @@ -47,7 +47,6 @@ only), never in the shared settings, and never shown again once saved. | @ ping | Never, critical events only, every post | Critical only | | Which events | Needs attention, or everything | Needs attention | | Tell Claude how to reach you | On or off | On | -| Also tell other CLIs, as a first message | On or off | Off | A finished turn, and an error, is posted as the CLI's whole last message, with its formatting and questions kept, split over several posts if it is long. A shell, and permission or question dialogs, show the last lines @@ -69,8 +68,7 @@ permission prompt, a question, an error, or a post an agent marked urgent. An agent only posts deliberately if it knows it can: - **Claude** is launched with the instruction as part of its system prompt (**Tell Claude how to reach you**, on by default), so nothing shows in the chat. This reaches local tabs only (remote and Docker tabs have no `codeman` command). -- **Every other CLI** is told only if you turn on **Also tell other CLIs, as a first message** (off by default: it costs one model turn per tab, and for a spawned worker or a scheduled run it arrives with the first real prompt). Then the same text is sent once, as a message, the first time a brand-new tab goes idle. - Restored tabs, shell tabs, and remote (ssh) and Docker tabs (which have no `codeman` command) are never sent it. +- **Every other CLI** is not told. It can still run `codeman notify` if you tell it to. Tabs that were already open when you turned the bridge on are not told. The alerts still reach Discord for them; only the agent-initiated `codeman notify` habit is missing. diff --git a/src/discord-bridge.ts b/src/discord-bridge.ts index 840e91ff..56fd34c0 100644 --- a/src/discord-bridge.ts +++ b/src/discord-bridge.ts @@ -77,7 +77,6 @@ export const DEFAULT_DISCORD_CONFIG: DiscordConfig = { pingMode: 'critical', scope: 'attention', startupHint: true, - typedHint: false, }; export interface DiscordMessage { @@ -169,9 +168,8 @@ const DOWNLOAD_TIMEOUT_MS = 30_000; export const RECEIVED_DIR = '.codeman-discord'; /** - * What a session is told about the bridge. Claude gets it as `--append-system-prompt`; every other - * CLI is sent it once as its first message, only when `typedHint` is on (see DiscordBridge.sessionIdle). No apostrophes: it is - * rendered into a shell command. + * What a session is told about the bridge. Claude gets it as `--append-system-prompt`; no other CLI + * is told. No apostrophes: it is rendered into a shell command. */ export const DISCORD_STARTUP_HINT = 'Codeman note: if you are blocked on something only the user can do (a physical step such as ' + @@ -399,7 +397,6 @@ function coerce(raw: unknown): DiscordConfig { pingMode: oneOf(DISCORD_PING_MODES, r.pingMode, d.pingMode), scope: oneOf(WEBHOOK_SCOPES, r.scope, d.scope), startupHint: r.startupHint === undefined ? d.startupHint : r.startupHint === true, - typedHint: r.typedHint === true, }; } @@ -803,11 +800,10 @@ export interface DiscordBridgeDeps { configDir: string; fetchImpl: DiscordFetch; /** - * Types `text` into the session (a line ending in Enter, or a lone digit for a dialog). False when the - * session is gone. `fromUser` is true only for what a person typed (a Discord reply), false for text - * Codeman types itself. + * Types a person's Discord reply into the session (a line ending in Enter, or a lone digit for a + * dialog). False when the session is gone. */ - sendInput: (sessionId: string, text: string, fromUser: boolean) => Promise; + sendInput: (sessionId: string, text: string) => Promise; /** True once the tab's agent has exited: tmux still accepts keys for a dead pane, so a tick would lie. */ agentExited?: (sessionId: string) => boolean; /** True while the session shows a permission or question dialog, which acts on a bare keypress. */ @@ -832,7 +828,6 @@ export class DiscordBridge { /** The last name seen per session, so only a real rename touches the thread. */ private names = new Map(); /** Brand-new sessions that have not yet been sent the startup hint (non-Claude CLIs). */ - private fresh = new Set(); private pending = new Map>(); /** Tabs closed while their thread was still being created: the creation applies `onSessionDelete` to it. */ private closedWhilePending = new Set(); @@ -1005,7 +1000,6 @@ export class DiscordBridge { this.lastSent.clear(); this.lastReply.clear(); this.names.clear(); - this.fresh.clear(); this.lastFull.clear(); this.inbound.clear(); } @@ -1186,33 +1180,13 @@ export class DiscordBridge { } /** Session tab created: make its thread now when `threadCreation` is `onSessionStart`. */ - async sessionCreated(sessionId: string, name: string | undefined, createdAt?: number): Promise { + async sessionCreated(sessionId: string, name: string | undefined): Promise { await this.ensureLoaded(); - // Only a tab made just now: the same event also fires for sessions restored at server start. - if (createdAt !== undefined && this.now() - createdAt < 60_000) this.fresh.add(sessionId); if (!this.cfg.enabled || this.cfg.threadCreation !== 'onSessionStart') return; const made = await this.threadFor(sessionId, name); if (typeof made !== 'string') this.last = made; } - /** - * First idle of a brand-new non-Claude session means its prompt is up: type the startup hint once. - * Claude gets the same text as a launch flag instead, and a shell must never receive it. - */ - async sessionIdle(sessionId: string, hintable: boolean): Promise { - if (!this.fresh.has(sessionId)) return; - if (!hintable || !this.cfg.enabled || !this.cfg.startupHint || !this.cfg.typedHint) { - this.fresh.delete(sessionId); // no longer new: turning the hint on later must not reach this tab - return; - } - // A dialog is up (codex's trust prompt reads as idle): Enter would pick its highlighted option, so wait for the next idle. - if (this.deps.awaitingChoice?.(sessionId)) return; - this.fresh.delete(sessionId); - const text = sanitiseReply(`${DISCORD_STARTUP_HINT} Reply with just: ok`); - // fromUser false: this is Codeman talking, so it must not auto-name the tab or count as the person's input. - if (text) await this.deps.sendInput(sessionId, text, false).catch(() => false); - } - async sessionRenamed(sessionId: string, name: string | undefined): Promise { const seen = this.names.get(sessionId); this.names.set(sessionId, name ?? ''); @@ -1231,7 +1205,6 @@ export class DiscordBridge { forgetTab(sessionId: string): void { this.lastReply.delete(sessionId); this.names.delete(sessionId); - this.fresh.delete(sessionId); this.lastFull.delete(sessionId); } @@ -1343,7 +1316,7 @@ export class DiscordBridge { if (saved.refused) await this.say(m.channel_id, saved.refused); // A lone digit answers a dialog by itself: an Enter after it would land on whatever comes next. const keys = /^\d\r$/.test(text) && this.deps.awaitingChoice?.(sessionId) ? text.slice(0, -1) : text; - const ok = await this.deps.sendInput(sessionId, keys, true).catch(() => false); + const ok = await this.deps.sendInput(sessionId, keys).catch(() => false); const reply = ok ? this.api.react(m.channel_id, m.id, '✅') : this.api.post(m.channel_id, { diff --git a/src/types/discord.ts b/src/types/discord.ts index ded66bbc..42782aca 100644 --- a/src/types/discord.ts +++ b/src/types/discord.ts @@ -37,11 +37,6 @@ export interface DiscordConfig { scope: WebhookScope; /** Tell each session how to reach the user (see DISCORD_STARTUP_HINT). On by default while the bridge is on. */ startupHint: boolean; - /** - * Also type the hint, once, as the first message to a brand-new non-Claude CLI tab. Off by default: it spends a - * model turn per tab, and for a spawned worker or a cron session it lands on the same first idle as the real prompt. - */ - typedHint: boolean; } export interface DiscordResult { diff --git a/src/web/public/index.html b/src/web/public/index.html index 3cc2552d..82a06f6f 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -3319,13 +3319,6 @@ -
-
- Also tell other CLIs, as a first message - Sends the same note once as a message to a brand-new Codex, OpenCode or similar tab when it first becomes idle. Costs one model turn per tab, and for a spawned worker or a scheduled run it arrives with the first real prompt. Never sent to shell, restored, remote or Docker tabs. Off by default. -
- -
Which events diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index f923bd46..380a46bd 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -1771,7 +1771,6 @@ Object.assign(CodemanApp.prototype, { document.getElementById('discordPingMode').value = d.pingMode; document.getElementById('discordScope').value = d.scope; document.getElementById('discordStartupHint').checked = d.startupHint !== false; - document.getElementById('discordTypedHint').checked = d.typedHint === true; const clearBtn = document.getElementById('discordClearBtn'); if (clearBtn) clearBtn.style.display = d.hasToken ? '' : 'none'; const gw = { off: 'Not connected (disabled).', connecting: 'Connecting to Discord…', connected: 'Connected: replies are being read.', error: `Not connected: ${d.gatewayError || 'error'}` }; @@ -1797,7 +1796,6 @@ Object.assign(CodemanApp.prototype, { pingMode: document.getElementById('discordPingMode').value, scope: document.getElementById('discordScope').value, startupHint: document.getElementById('discordStartupHint').checked, - typedHint: document.getElementById('discordTypedHint').checked, }; const token = document.getElementById('discordToken').value.trim(); if (token) payload.token = token; // blank = keep the saved one (Remove token is the way to clear it) diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 1e507414..5c72e74a 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -2268,7 +2268,6 @@ export const DiscordUpdateSchema = z pingMode: z.enum(DISCORD_PING_MODES).optional(), scope: z.enum(WEBHOOK_SCOPES).optional(), startupHint: z.boolean().optional(), - typedHint: z.boolean().optional(), }) .strict(); diff --git a/src/web/server.ts b/src/web/server.ts index 15ae32d5..2a679a10 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -520,13 +520,12 @@ export class WebServer extends EventEmitter { private discordBridge = new DiscordBridge({ configDir: getDataDir(), fetchImpl: webviewFetch, - sendInput: async (sessionId, text, fromUser) => { + sendInput: async (sessionId, text) => { const session = this.sessions.get(sessionId); if (!session) return false; - // fromUser is true only for a Discord reply (a person typed it); the startup hint is Codeman's own - // text and must not auto-name the tab or count as the person's input. - if (await session.writeViaMux(text, { fromUser }).catch(() => false)) return true; - return session.write(text, { fromUser }); + // A Discord reply is a person's own input, so it names the tab like any other prompt. + if (await session.writeViaMux(text, { fromUser: true }).catch(() => false)) return true; + return session.write(text, { fromUser: true }); }, agentExited: (id) => !!this.sessions.get(id)?.paneExit, awaitingChoice: (id) => approvalInbox.listPending().some((item) => item.sessionId === id && item.kind !== 'idle'), @@ -2786,41 +2785,19 @@ export class WebServer extends EventEmitter { this.bridgeSessionEvent(event, data); } - /** - * Whether the startup hint may be typed into this tab: an external CLI (never Claude, which has it as a - * launch flag, and never a shell), running locally. A remote (ssh) or Docker tab has no `codeman` on its - * PATH, so the hint would tell the agent to run a command it does not have. - */ - private isHintableSession(sessionId: string): boolean { - const session = this.sessions.get(sessionId); - return !!session && isExternalCliMode(session.mode) && !session.remote && !session.docker; - } - /** Keep the Discord thread in step with the tab: created, renamed, deleted. Never throws. */ private bridgeSessionEvent(event: string, data: unknown): void { // Deletion is NOT handled here: `session:deleted` also fires for a detach (killMux=false), where the // session keeps running and can be re-attached. The real delete calls the bridge from the cleanup path. - if (event !== SseEvent.SessionCreated && event !== SseEvent.SessionUpdated && event !== SseEvent.SessionIdle) { - return; - } - const d = (data ?? {}) as { - id?: string; - name?: string; - createdAt?: number; - session?: { id?: string; name?: string; createdAt?: number }; - }; + if (event !== SseEvent.SessionCreated && event !== SseEvent.SessionUpdated) return; + const d = (data ?? {}) as { id?: string; name?: string; session?: { id?: string; name?: string } }; const s = d.session ?? d; if (!s.id) return; const id = s.id; const run = - event === SseEvent.SessionIdle - ? // Claude is told at launch; a shell, a remote or a Docker tab must never be typed at. - this.discordBridge.sessionIdle(id, this.isHintableSession(id)) - : event === SseEvent.SessionCreated - ? this.discordBridge - .sessionCreated(id, s.name, s.createdAt) - .then(() => this.discordBridge.sessionRenamed(id, s.name)) - : this.discordBridge.sessionRenamed(id, s.name); + event === SseEvent.SessionCreated + ? this.discordBridge.sessionCreated(id, s.name).then(() => this.discordBridge.sessionRenamed(id, s.name)) + : this.discordBridge.sessionRenamed(id, s.name); void run.catch(() => undefined); } diff --git a/test/discord-bridge.test.ts b/test/discord-bridge.test.ts index 1af96e63..d42ce749 100644 --- a/test/discord-bridge.test.ts +++ b/test/discord-bridge.test.ts @@ -75,6 +75,10 @@ beforeEach(() => { afterEach(() => rmSync(dir, { recursive: true, force: true })); describe('pure rules', () => { + it('names the command in the hint Claude is launched with', () => { + expect(DISCORD_STARTUP_HINT).toContain('codeman notify'); + }); + it('sanitiseReply makes one printable line plus Enter', () => { expect(sanitiseReply('yes')).toBe('yes\r'); expect(sanitiseReply('line one\nline two\r\nthree')).toBe('line one line two three\r'); @@ -282,7 +286,7 @@ describe('DiscordGateway', () => { describe('DiscordBridge', () => { let posts: Array<{ path: string; method: string; body: Record | undefined }>; - let sendInput: ReturnType Promise>>; + let sendInput: ReturnType Promise>>; let t: number; function bridge(overrides: Partial = {}, fetchOverride?: DiscordFetch) { @@ -447,7 +451,7 @@ describe('DiscordBridge', () => { it('types an allowed reply into the mapped session and ticks it', async () => { const b = await ready(); await b.onGatewayMessage(reply()); - expect(sendInput).toHaveBeenCalledWith('s1', 'press y\r', true); // a person typed it + expect(sendInput).toHaveBeenCalledWith('s1', 'press y\r'); expect(posts[0].method).toBe('PUT'); expect(posts[0].path).toContain('/reactions/'); b.stop(); @@ -472,7 +476,7 @@ describe('DiscordBridge', () => { await b.onGatewayMessage(reply({ content: 'a\nb' })); await b.onGatewayMessage(reply({ content: 'again' })); expect(sendInput).toHaveBeenCalledTimes(1); - expect(sendInput).toHaveBeenCalledWith('s1', 'a b\r', true); + expect(sendInput).toHaveBeenCalledWith('s1', 'a b\r'); // The second reply was not typed, and the person is told so instead of it vanishing. expect(posts.some((p) => p.method === 'PUT' && p.path.includes(encodeURIComponent('⏳')))).toBe(true); b.stop(); @@ -848,28 +852,26 @@ describe('DiscordBridge', () => { describe('per-tab state', () => { const internals = (b: DiscordBridge) => - b as unknown as { names: Map; lastReply: Map; fresh: Set }; + b as unknown as { names: Map; lastReply: Map }; it('a deleted tab that never had a thread still frees its entries', async () => { const b = bridge(); await b.reload(); - await b.sessionCreated('nothread', 'w', t - 1000); + await b.sessionCreated('nothread', 'w'); await b.sessionRenamed('nothread', 'w'); internals(b).lastReply.set('nothread', 1); await b.sessionDeleted('nothread'); expect(internals(b).names.has('nothread')).toBe(false); expect(internals(b).lastReply.has('nothread')).toBe(false); - expect(internals(b).fresh.has('nothread')).toBe(false); b.stop(); }); it('stop() clears the maps and the gateway', async () => { const b = bridge(); await b.reload(); - await b.sessionCreated('x', 'w', t - 1000); + await b.sessionCreated('x', 'w'); await b.sessionRenamed('x', 'w'); b.stop(); - expect(internals(b).fresh.size).toBe(0); expect(internals(b).names.size).toBe(0); expect(b.gatewayState).toBe('off'); }); @@ -1025,7 +1027,7 @@ describe('DiscordBridge files and startup hint', () => { ...over, }); let work: string; - let sent: Array<[string, string, boolean]>; + let sent: Array<[string, string]>; let posted: Array<{ path: string; body?: Record }>; let downloads: string[]; @@ -1047,7 +1049,7 @@ describe('DiscordBridge files and startup hint', () => { posted.push({ path, body: init.body ? JSON.parse(String(init.body)) : undefined }); return path.endsWith('/threads') ? jsonRes({ id: THREAD }) : jsonRes({ id: 'msg' }); }, - sendInput: async (id, text, fromUser) => (sent.push([id, text, fromUser]), true), + sendInput: async (id, text) => (sent.push([id, text]), true), sessionDir: dirFor, awaitingChoice, agentExited, @@ -1055,7 +1057,7 @@ describe('DiscordBridge files and startup hint', () => { now: () => t, sleep: async () => undefined, }); - return writeDiscordConfig(dir, { ...CFG, typedHint: true, ...cfg }).then(async () => { + return writeDiscordConfig(dir, { ...CFG, ...cfg }).then(async () => { await b.reload(); await b.notify(MSG); posted.length = 0; @@ -1078,7 +1080,7 @@ describe('DiscordBridge files and startup hint', () => { const dest = join(work, '.codeman-discord', '555555555555555555-shot_one.png'); expect(readFileSync(dest)).toEqual(Buffer.from([7, 8, 9])); expect(statSync(dest).mode & 0o777).toBe(0o600); - expect(sent).toEqual([['s1', `[attached: ${dest}]\r`, true]]); + expect(sent).toEqual([['s1', `[attached: ${dest}]\r`]]); b.stop(); }); @@ -1117,48 +1119,6 @@ describe('DiscordBridge files and startup hint', () => { b.stop(); }); - it('sends the startup hint once, only to a fresh external session, and never to a shell', async () => { - const b = await make(); - await b.sessionCreated('s2', 'w2', t - 1000); - await b.sessionIdle('s2', false); // shell or Claude: nothing - expect(sent).toHaveLength(0); - await b.sessionCreated('s3', 'w3', t - 1000); - await b.sessionIdle('s3', true); - await b.sessionIdle('s3', true); // second idle: nothing - expect(sent).toHaveLength(1); - expect(sent[0][0]).toBe('s3'); - expect(sent[0][1]).toContain('codeman notify'); - expect(sent[0][1].endsWith('\r')).toBe(true); - expect(sent[0][2]).toBe(false); // Codeman's own text: not a person's input, so it cannot auto-name the tab - expect(DISCORD_STARTUP_HINT).toContain('codeman notify'); - b.stop(); - }); - - it('holds the startup hint while a choice is pending and sends it at the next idle', async () => { - let choice = true; - const b = await make({}, undefined, () => choice); - await b.sessionCreated('s6', 'w6', t - 1000); - await b.sessionIdle('s6', true); - expect(sent).toHaveLength(0); - choice = false; - await b.sessionIdle('s6', true); - expect(sent).toHaveLength(1); - await b.sessionIdle('s6', true); - expect(sent).toHaveLength(1); - b.stop(); - }); - - it('does not hint a tab that was already past its first idle while the hint was off', async () => { - const off = await make({ typedHint: false }); - await off.sessionCreated('s8', 'w8', t - 1000); - await off.sessionIdle('s8', true); // first idle with the hint off: no longer new - await writeDiscordConfig(dir, { ...CFG, typedHint: true }); - await off.reload(); - await off.sessionIdle('s8', true); - expect(sent).toHaveLength(0); - off.stop(); - }); - it('says the agent has exited and types nothing when the tab reports it', async () => { const b = await make({}, undefined, undefined, () => true); await b.onGatewayMessage(reply({ content: 'hello', attachments: [] })); @@ -1202,28 +1162,6 @@ describe('DiscordBridge files and startup hint', () => { expect(String(posted.at(-1)?.body?.content)).toMatch(/could not be saved/); b.stop(); }); - - it('sends the typed hint only when typedHint is on (the default is off)', async () => { - const off = await make({ typedHint: false }); - await off.sessionCreated('s7', 'new', t - 1000); - await off.sessionIdle('s7', true); - expect(sent).toHaveLength(0); - off.stop(); - expect(DEFAULT_DISCORD_CONFIG.typedHint).toBe(false); - }); - - it('does not hint a restored (old) session, or when startupHint is off', async () => { - const b = await make(); - await b.sessionCreated('s4', 'old', 1); // created long ago: restored at server start - await b.sessionIdle('s4', true); - expect(sent).toHaveLength(0); - b.stop(); - const off = await make({ startupHint: false }); - await off.sessionCreated('s5', 'new', t - 1000); - await off.sessionIdle('s5', true); - expect(sent).toHaveLength(0); - off.stop(); - }); }); describe('tablesToCodeBlocks', () => { diff --git a/test/discord-push-glue.test.ts b/test/discord-push-glue.test.ts index 0a8d86d1..01210b75 100644 --- a/test/discord-push-glue.test.ts +++ b/test/discord-push-glue.test.ts @@ -38,7 +38,6 @@ function server(): Internals { sessionRenamed: vi.fn(async () => undefined), sessionDeleted: vi.fn(async () => undefined), forgetTab: vi.fn(), - sessionIdle: vi.fn(async () => undefined), stop: vi.fn(), }; return s; @@ -58,6 +57,17 @@ describe('bridge deps', () => { expect(agentExited('alive')).toBe(false); expect(agentExited('missing')).toBe(false); }); + + it("types a reply as the person's own input, so it can name the tab", async () => { + const s = new WebServer(0, false, true, '127.0.0.1', 'box') as unknown as Internals & { + discordBridge: { deps: { sendInput: (id: string, text: string) => Promise } }; + }; + const writeViaMux = vi.fn(async () => true); + s.sessions.set('s1', { writeViaMux }); + expect(await s.discordBridge.deps.sendInput('s1', 'yes\r')).toBe(true); + expect(writeViaMux).toHaveBeenCalledWith('yes\r', { fromUser: true }); + expect(await s.discordBridge.deps.sendInput('missing', 'yes\r')).toBe(false); + }); }); describe('sendPushNotifications -> Discord', () => { @@ -152,7 +162,7 @@ describe('broadcast -> Discord tab lifecycle', () => { it('routes created, updated and deleted by kind, reading both payload shapes', () => { const s = server(); s.broadcast(SseEvent.SessionCreated, { session: { id: 's1', name: 'w1', createdAt: 123 } }); - expect(s.discordBridge.sessionCreated).toHaveBeenCalledWith('s1', 'w1', 123); + expect(s.discordBridge.sessionCreated).toHaveBeenCalledWith('s1', 'w1'); s.broadcast(SseEvent.SessionUpdated, { id: 's1', name: 'renamed' }); expect(s.discordBridge.sessionRenamed).toHaveBeenCalledWith('s1', 'renamed'); }); @@ -190,24 +200,6 @@ describe('broadcast -> Discord tab lifecycle', () => { expect(s.discordBridge.sessionRenamed).not.toHaveBeenCalled(); }); - it('allows the typed hint only for a local external CLI: not Claude, a shell, a remote or a Docker tab', () => { - const s = server(); - s.sessions.set('cx', { mode: 'codex' }); - s.sessions.set('cl', { mode: 'claude' }); - s.sessions.set('sh', { mode: 'shell' }); - s.sessions.set('rm', { mode: 'codex', remote: { label: 'box' } }); - s.sessions.set('dk', { mode: 'codex', docker: { image: 'x' } }); - for (const id of ['cx', 'cl', 'sh', 'rm', 'dk', 'missing']) s.broadcast(SseEvent.SessionIdle, { id }); - expect(s.discordBridge.sessionIdle.mock.calls).toEqual([ - ['cx', true], - ['cl', false], - ['sh', false], - ['rm', false], - ['dk', false], - ['missing', false], - ]); - }); - it('closes the bridge when the server stops', async () => { const s = server(); await (s as unknown as { stop: () => Promise }).stop(); diff --git a/test/routes/discord-routes.test.ts b/test/routes/discord-routes.test.ts index 288b54c7..7ea15bc3 100644 --- a/test/routes/discord-routes.test.ts +++ b/test/routes/discord-routes.test.ts @@ -84,16 +84,16 @@ describe('/api/discord', () => { bridge.stop(); }); - it('stores startupHint (default on) and typedHint (default off) and returns them', async () => { + it('stores startupHint (default on) and returns it', async () => { const { app, bridge } = await harness(); const first = (await app.inject({ method: 'GET', url: '/api/discord' })).json().data; - expect(first).toMatchObject({ startupHint: true, typedHint: false }); + expect(first).toMatchObject({ startupHint: true }); const put = await app.inject({ method: 'PUT', url: '/api/discord', - payload: { typedHint: true, startupHint: false }, + payload: { startupHint: false }, }); - expect(put.json().data).toMatchObject({ startupHint: false, typedHint: true }); + expect(put.json().data).toMatchObject({ startupHint: false }); bridge.stop(); });