diff --git a/PLAN.md b/PLAN.md index 7ec002850..d974b2b78 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1,164 +1,145 @@ -# ICI-680 — Jinn README refresh (copy + assets) - -**Branch** `build/ICI-680-readme-refresh` · **Base** `ebaac281` (main) -**Mode** direct · **Complexity** complex - -> This file replaces a leftover `PLAN.md` from ICI-225 that is tracked on `main`. - -## The request (operator's words) - -> Refresh README documentation, copy and assets. Assets including images are kind of -> old/stale compared to where jinn stands since then and how jinn looks like. -> Use agent-browser and what not or any skill that is available. -> **Do not refresh the gif.** - -## What is actually stale - -Verified against the repo at `ebaac281`, not assumed: - -**Copy — root `README.md`** (last rewritten at 0.26, `8237caff`; package is now **0.29.0**) -- "Highlights from **0.26**" and "See CHANGELOG.md for the full **0.26** notes" — three - releases behind. 0.27 (model-scoped Claude usage buckets), 0.28 (Workflows v2 explicit - completion contract + run canvas, collaborative Todo hierarchy with labels/comments/ - attachments/links, multiple isolated workspaces, instance-wide MCP file reads, auth - required by default) and 0.29 (grouped Todo work breakdowns in chat, one-root-per-outcome - doctrine, instance directory permissions hardening) are all missing. -- **False claim:** "The picker shows real model names out of the box (Opus 4.8, GPT-5.5, - Gemini 3.x…)". The shipped `DEFAULT_CONFIG` in `packages/jinn/src/cli/setup.ts` writes - `Opus (Latest)` / `Sonnet (Latest)` / `Fable (Latest)`, `GPT-5.5 Codex`, `Grok Build`, - `Gemini 3.5 Flash …`. Live discovery supplies the real Claude names; a fresh install does - not show "Opus 4.8". -- The `config.yaml` example omits `gateway.authRequired: true`, which 0.28 made the default - for new installs, and omits the `models:` registry block that is the actual answer to - "how do I add a model without a code change". -- Roadmap "On deck" still lists **REST API auth**, which shipped in 0.28. Multiple - workspaces shipped and is absent from "Shipped recently". -- Node prerequisite line ("22 or 24, avoid 25") must be re-checked against - `packages/jinn/package.json` (`node >=22`) and `.nvmrc` (`24.13.0`). - -**Copy — `packages/jinn/README.md`** (the page strangers see on npm) is far staler: it -describes Jinn as "Claude Code, Codex, Grok, and Antigravity" (no Pi, no Hermes) and never -mentions Todos, Workflows, or the MCP company surface. It is pre-0.26 text. - -**Assets** (`assets/*.png`) -- `chat.png`, `org-map.png` — 23 Jun. `todos.png`, `workflows.png` — 19 Jul. -- `packages/web/src/routes/chat` has 17 commits since, `todos` 102, `workflow` 43. -- `chat.png` shows the composer chip reading "Opus 4.8 · Medium" — a model label the product - no longer produces. -- `todos.png` predates hierarchy, labels, comments and links; it shows five flat rows. -- `org-map.png` truncates half its node labels ("Engineerin…", "QA & Rel…") and predates the - system Todo Dispatcher employee. -- `jinn-showcase.gif` is **not touched** (explicit operator instruction). - -## Acceptance criteria - -1. **Version truth.** The Features section is headed by 0.29 (not 0.26) and the CHANGELOG - link names 0.29. Every feature bullet that describes shipped behaviour is traceable to an - entry in `CHANGELOG.md` between 0.26.0 and 0.29.0. No bullet describes behaviour that - does not exist. -2. **Defaults match shipped code.** The model-names sentence matches the labels actually - written by `DEFAULT_CONFIG` in `packages/jinn/src/cli/setup.ts`; every key/value in the - README `config.yaml` example exists in that same `DEFAULT_CONFIG` with the same value, - including `gateway.authRequired: true`; the Node prerequisite matches - `packages/jinn/package.json` `engines.node` and `.nvmrc`. -3. **Roadmap is honest.** No item under "On deck" has already shipped (REST API auth is - moved or removed), and "Shipped recently" names Workflows v2's completion contract, the - Todo hierarchy, and multiple isolated workspaces. -4. **npm README is current.** `packages/jinn/README.md` names all six engines (claude, codex, - grok, antigravity, pi, hermes) and mentions Todos, Workflows, and the MCP company surface. - Its install and quickstart commands match the root README's. -5. **Assets regenerated.** `assets/todos.png`, `assets/workflows.png`, `assets/chat.png`, - `assets/org-map.png` are re-captured from a build of this branch, dark theme, 2× DPR - (≥2560px wide for the 880px-wide embeds), and each shows a surface that exists today: - Todos with a sub-task/label present, the Workflows v2 canvas, Chat with the current - composer and at least one activity receipt, the org map with **no ellipsis-truncated - node label**. -6. **The gif is untouched.** `git diff --stat main -- assets/jinn-showcase.gif` prints - nothing. -7. **Nothing personal ships.** The textual diff passes the leak grep, and no screenshot - contains a real person's name, a real product name, an absolute personal home path, a real Slack ID, - or any content from the live instance. All seeded demo data is invented and generic. -8. **Links resolve.** Every relative link and image path in both READMEs points at a file - that exists on this branch (`CHANGELOG.md`, `LICENSE`, `.github/CONTRIBUTING.md`, - `docs/engines-hermes.md`, `assets/*`). -9. **Scope is closed.** `git diff --stat main` lists only `README.md`, - `packages/jinn/README.md`, the four PNGs, and `PLAN.md`. No product code, no template. -10. **Safety honoured.** The sandbox gateway ran on a port ≥7778 from an explicit throwaway - `JINN_HOME`, its `config.yaml` `port:` was read and confirmed non-7777 *before* start, - and the instance was destroyed afterwards — including if the run failed. `~/.jinn` and - port 7777 were never written to, restarted, or stopped. - -## Files - -| File | Change | -|---|---| -| `README.md` | Features section retargeted to 0.29; model-name claim corrected; config example brought in line with `DEFAULT_CONFIG`; roadmap re-sorted; Node prerequisite verified; image captions checked against the new screenshots | -| `packages/jinn/README.md` | Rewritten to the current product: six engines, Todos, Workflows, MCP company surface, current quickstart | -| `assets/todos.png` | Re-captured | -| `assets/workflows.png` | Re-captured | -| `assets/chat.png` | Re-captured | -| `assets/org-map.png` | Re-captured, no truncated labels | -| `assets/jinn-showcase.gif` | **Untouched** | - -## How the assets get made - -1. Build this worktree: `pnpm install && pnpm build` inside - `~/Projects/.worktrees/jinn-build-ICI-680`. -2. Bring up an isolated sandbox from *this* build with a `mktemp -d` home outside `~/.jinn` - and an explicit port of 7793. Run setup with that exact `JINN_HOME`, edit only its generated - `config.yaml`, then read the file back and confirm the parsed port is neither 7777 nor 7788 - before starting the daemon. Record the PID and stop only that instance with the same explicit - `JINN_HOME`. -3. Seed a generic demo company into the sandbox home (invented names only — the existing - assets use a "Northwind" COO over Engineering / Growth / Research / Support; keep that - cast so the four images look like one company): - - employees as YAML in the throwaway home's `org/` directory; - - Todos, including one parent with sub-tasks and a label, through the sandbox gateway's - own API on its own port; - - a Workflow definition with sequential + parallel + approval nodes, and one completed - run so the canvas has state; - - a chat transcript inserted into the sandbox `messages` table - (`packages/jinn/src/sessions/migrate.ts`) with the gateway stopped, then restarted — - this is how a realistic conversation is staged without burning a real engine turn. - Seed scripts live in the sandbox home, **not** in the repo. -4. Capture with `agent-browser` (see the `browser-use` skill) at viewport 1440×900, DPR 2, - dark theme. Export a throwaway `AGENT_BROWSER_PROFILE` — `--session` does not isolate. -5. Crop only where the current assets are cropped (the org map is a wide strip). -6. Stop the daemon with the same explicit `JINN_HOME`, verify port 7793 is free, then remove the - exact `mktemp` home and isolated browser profile — even if the capture failed. - -## Verification - -No new unit tests: this change has no logic. The full repository typecheck, test, and build -gates still run after the final edit; task-specific proofs are mechanical checks plus eyes on -the images. - -```bash -cd ~/Projects/.worktrees/jinn-build-ICI-680 -git diff --stat main # AC9 — only the six files + PLAN.md -git diff --stat main -- assets/jinn-showcase.gif # AC6 — must be empty -# AC7 — run the required staged leak grep; only the repository-owner URLs may hit -pnpm typecheck -pnpm test -pnpm build +# ICI-682 — Connector identity gets one owner: the instance id + +**Branch** `simplify/ICI-682-connector-identity` · **Base** `3bf137e81cc2be8761d4035ab738eddd04e4f0d4` (origin/main) +**Worktree** `~/Projects/.worktrees/jinn-simplify-ICI-682` · **Phase** constrain (this file replaces a stale tracked PLAN.md from ICI-680) + +## The defect being removed + +The registry is keyed by connector **instance id** (`gateway/server.ts:749` +`connectorMap.set(instance.id, connector)`, id injected into every constructor config at +`server.ts:390`), but every connector except Discord stamps `session.connector` with its +**type name** (`slack/index.ts:17,180,258,374`, `telegram/index.ts:29,305`, +`whatsapp/index.ts:46` + literal `"whatsapp"` at `:312`, `discord/remote.ts:22`). For a +named instance (`{id: "slack-support", type: "slack"}`) the lookup in +`deliverConnectorReply` (`gateway/api.ts:6868-6869`) misses and **silently returns** — the +sole reply-delivery path for `runWebSession` turns (call sites `api.ts:7215, 7398, 7477, +7595`). Session-key prefixes hardcode the type too (`slack/threads.ts:11-23`, +`telegram/threads.ts:14`, `whatsapp/index.ts:308`, reaction key `slack/index.ts:371`), so +two instances of one type collide into one session. `/api/status` keys health by +`connector.name` (`api.ts:2556`) so same-type instances overwrite each other, while +`GET /api/connectors` (`api.ts:6301-6306`) already reads per instance. `server.ts:726` +keeps a parallel `connectors: Connector[]` array whose only uses are push (`:748`), splice +(`:783`), and the shutdown loop — all served by `connectorMap.values()`. + +**Load-bearing fact (verified):** legacy top-level config blocks get `id === type` +(`server.ts:397`, pinned by `gateway/__tests__/connectors.test.ts:22-31`), so for every +unnamed install this change is a behavior-preserving rename: stamps, session keys, and +status keys stay **byte-identical**. Only named `instances[]` users change — and their +replies are dropped today, so there is no working behavior to lose. + +## The change (mechanical steps) + +1. **`shared/types.ts` (~271)** — add `id: string` to the `Connector` interface with a + one-line JSDoc: the instance id, the registry key; equals the type for legacy top-level + config. No other interface change. (Sanctioned by the Todo's intended outcome; this is + the one deliberate public-surface addition.) +2. **Connector classes** — each sets `id` from `config.id` with its type literal as the + defensive fallback (same pattern Discord already uses): + - `slack/index.ts`: add `id`; stamp `connector: this.id` at `:180,:258,:374`; derive + session keys with the id prefix (thread keys via `threads.ts`, reaction key `:371`). + - `telegram/index.ts`: add `id`; stamp at `:305`; id-prefixed session key. + - `whatsapp/index.ts`: add `id`; replace literal at `:312`; prefix at `:308`. + - `discord/index.ts`: delete `instanceId` (`:41,:53`); keep `name = "discord"` as the + type constant, `id = config.id || "discord"`; replace `this.instanceId` uses + (`:274,:290`) with `this.id`; fix `proxyToRemote` (`:329`) to pass `this.id` so the + proxy path derives the same key as the inbound path (`:274`). + - `discord/remote.ts`: `RemoteDiscordConfig` gains `id?: string`; class sets + `id = config.id || "discord"`; `server.ts:422` passes `instance.config.id` through. + - `cron/index.ts`: `id = "cron"` (interface conformance; never in `connectorMap`). +3. **`slack/threads.ts` / `telegram/threads.ts`** — `deriveSessionKey` gains a prefix + parameter defaulting to the type literal, exactly the shape `discord/threads.ts:3` + already has. Existing tests keep passing unchanged (default = legacy behavior). +4. **`gateway/api.ts:6868-6869`** — replace the silent `return` with a `logger.warn` + naming the session id and the unresolved `session.connector` value, then return. + This failure class must never be invisible again. +5. **`gateway/api.ts:2555-2557`** — key `/api/status` connectors by the registry key + (iterate `.entries()`), not `connector.name`. Identical output for legacy configs. +6. **`gateway/server.ts:726,748,783` + shutdown loop (~1419)** — delete the parallel + `connectors: Connector[]` array; use `connectorMap` everywhere. +7. **Prose this change falsifies (taste §4 exception — part of the change, not adjacent):** + - `template/docs/connectors.md:24` — `connector: string; // Connector name` → instance id + (resolves the file's self-contradiction with its own line 90). + - `sessions/context.ts:399` and `:997` — `/api/connectors//send` → ``. + - `gateway/api.ts:6270` route comment and `gateway/server.ts:756-757` map comment — + "names" → "ids". No route or param renames. +8. **Tests (the evidence, per rubric §5.1 — a "hole is shut" claim needs a test that fails + on base and passes after):** + - `gateway/__tests__/run-web-session-connector-reply.test.ts` — add the named-instance + case: map keyed `"slack-support"`, session stamped `"slack-support"` → reply delivered + (fails on base only via the connector-side stamp change, so pin it at the unit seam: + lookup with matching id delivers). Rewrite the `:60-64` "missing from map" case — it + currently pins the silent drop as correct; it now asserts the drop is logged and still + does not throw. + - `gateway/__tests__/connectors.test.ts` — assert a constructed named instance carries + `id` from config and stamps it (slack or telegram, one case; this file already encodes + the instance-id model). + - `slack/threads.test.ts` / `telegram/__tests__/threads.test.ts` — one added case each: + custom prefix yields `:`-prefixed keys; existing literal-prefix assertions stay + untouched (they now pin legacy byte-identity). + - No test deletions: no covered behavior is deleted, only the silent-drop pin is + rewritten into a logged-drop pin. + +## Explicitly OUT of scope (report as follow-up Todos, do not touch) + +- Hardcoded discord proxy URLs (`discord/index.ts:347`, `remote.ts:100`) — cross-gateway + addressing, separate concern. +- `POST /api/connectors/:name/send` param rename and `mcp/connector-tools.ts` naming. +- `api.ts:6289-6291` hardcoded `get("whatsapp")` QR route. +- `template/docs/connectors.md:48` wrong channel-root key format (wrong today, not + falsified by this change) and the stale interface snippet at `:8-20`. +- Web settings cron-delivery picker hardcoding type options + (`packages/web/src/routes/settings/page.tsx:1776-1786`). +- `sessions/manager.ts:175` `connectorNames()` rename; session-fork connector carryover + (`registry.ts:1686-1700`); DB backfill of pre-existing named-instance rows + (`migrate.ts:372` `COALESCE(connector, source)` — legacy rows have `id === type` and + keep resolving; named-instance rows are already broken today). +- The legacy vs `instances[]` config merge (PLA-53, merged). + +## Budget (frozen) + +| Field | Value | Note | +|---|---|---| +| `netLineDelta` | **≤ +40** total, **≤ 0 excluding `*.test.ts`** | Relaxation from ≤0 is evidenced: the Todo itself estimates product at −55/+40 ("the win is ownership… not raw line count") and rubric §5.1 mandates new regression tests for the shut hole. All growth budget is test lines. | +| `maxFilesTouched` | **17** | 11 product + 2 prose (`context.ts`, `connectors.md`) + 4 test files. `PLAN.md` excluded from all measurements. | +| `maxNewFiles` | **0** | Tests go into existing files. | +| `maxFileLines` | **7675** | = `api.ts` (7668) + the logged-drop lines. Only `api.ts` (+≤7), `types.ts` (+≤3), `remote.ts` (+≤4), `cron/index.ts` (+≤2), and the 4 test files may grow; every other touched file must not grow. | +| New deps / config options / public exports / single-caller abstractions | **0** | Sole sanctioned surface change: the `id` field on the existing `Connector` interface. The `deriveSessionKey` prefix param copies the existing Discord shape, gaining a second caller pattern — not a new abstraction. | + +**budgetCommand** (run from the worktree; non-destructive): + +```sh +BASE=3bf137e81cc2be8761d4035ab738eddd04e4f0d4; git diff --numstat "$BASE" -- . ':(exclude)PLAN.md' | awk '{add+=$1; del+=$2; files++} END {printf "netLineDelta=%d\nfilesTouched=%d\n", add-del, files}'; printf "productNetLineDelta=%d\n" "$(git diff --numstat "$BASE" -- . ':(exclude)PLAN.md' ':(exclude)**/*.test.ts' | awk '{add+=$1; del+=$2} END {print add-del+0}')"; printf "newFiles=%d\n" "$(git diff --diff-filter=A --name-only "$BASE" -- . ':(exclude)PLAN.md' | wc -l | tr -d ' ')"; printf "maxFileLines=%d\n" "$(git diff --name-only "$BASE" -- . ':(exclude)PLAN.md' | while IFS= read -r f; do if [ -f "$f" ]; then wc -l < "$f"; else echo 0; fi; done | sort -n | tail -1 | tr -d ' ')" ``` -- **AC8**: extract every `](...)` and `src="..."` relative target from both READMEs and - `test -e` each one. -- **AC1/AC2/AC3**: read the new copy next to `CHANGELOG.md` (0.26→0.29) and - `packages/jinn/src/cli/setup.ts` `DEFAULT_CONFIG`, claim by claim. -- **AC5/AC7**: open each of the four PNGs and look at it. Check dimensions with - `sips -g pixelWidth -g pixelHeight`. - -## Out of scope - -- `assets/jinn-showcase.gif` — the operator said not to. -- **Stale defaults in product code.** `DEFAULT_CONFIG` still ships `gpt-5.5` and - `Opus (Latest)` while the current models are GPT-5.6 and Opus 5. That is a real problem - and it is *not* this ticket: this ticket makes the README describe what ships, and hands - the defaults back as a follow-up Todo. -- `CHANGELOG.md`, `.github/CONTRIBUTING.md`, `docs/**`, `packages/jinn/template/**`. -- Any product code or web UI change. If a screenshot exposes a UI bug, it is written down - and handed back, not fixed here. -- Light-theme variants of the assets. The README's existing assets and the gif are all dark; - the taste rule's dual-theme gate governs *design changes*, and this ticket changes no UI. +## Acceptance (mechanical) + +1. Worktree `~/Projects/.worktrees/jinn-simplify-ICI-682`, branch + `simplify/ICI-682-connector-identity`, based on `3bf137e81cc2be8761d4035ab738eddd04e4f0d4`. +2. `Connector` interface gains exactly one field, `id: string`, with JSDoc; no other + interface changes. +3. Every connector class (slack, telegram, whatsapp, discord, discord-remote, cron) sets + `id` from `config.id` with its type literal as fallback; `DiscordConnector.instanceId` + is deleted; `createConnector` passes `id` to the remote-discord config. +4. All five `IncomingMessage.connector` stamp sites use the instance id + (`slack/index.ts:180,258,374`, `telegram/index.ts:305`, `whatsapp/index.ts:312`, + discord via `this.id`). +5. All session-key prefixes derive from the instance id (slack threads + reaction key, + telegram threads, whatsapp `:308`, discord `proxyToRemote:329` now matching `:274`); + for legacy `id === type` configs the keys are byte-identical, pinned by the existing + unchanged threads-test assertions. +6. The `deliverConnectorReply` miss logs a warn with session id and connector value and + still returns without throwing; the old silent-drop test is rewritten, not deleted. +7. `/api/status` connectors are keyed by registry key (`.entries()`), identical output for + legacy configs. +8. The parallel `connectors: Connector[]` array in `server.ts` is deleted. +9. New tests: named-instance reply delivery; logged drop; one custom-prefix case each for + slack and telegram key derivation; one named-instance stamp case in + `connectors.test.ts`. No test deletions. +10. Prose fixed only where falsified: `connectors.md:24`, `context.ts:399,997`, + `api.ts:6270` comment, `server.ts:756` comment. Nothing from the out-of-scope list. +11. `budgetCommand` prints `netLineDelta ≤ 40`, `productNetLineDelta ≤ 0`, + `filesTouched ≤ 17`, `newFiles = 0`, `maxFileLines ≤ 7675`. +12. Zero new dependencies, config options, or single-caller abstractions; no public + exports beyond the `id` interface field. +13. After the final commit: `pnpm typecheck`, `pnpm test`, `pnpm build` all green, + verbatim tails quoted by whoever runs them. diff --git a/packages/jinn/src/connectors/cron/index.ts b/packages/jinn/src/connectors/cron/index.ts index 6c0955625..53b87bc5a 100644 --- a/packages/jinn/src/connectors/cron/index.ts +++ b/packages/jinn/src/connectors/cron/index.ts @@ -18,6 +18,7 @@ const capabilities: ConnectorCapabilities = { export class CronConnector implements Connector { name = "cron"; + id = "cron"; private handler: ((msg: IncomingMessage) => void) | null = null; constructor( diff --git a/packages/jinn/src/connectors/discord/index.ts b/packages/jinn/src/connectors/discord/index.ts index 36b5242ae..0d1d9d284 100644 --- a/packages/jinn/src/connectors/discord/index.ts +++ b/packages/jinn/src/connectors/discord/index.ts @@ -37,8 +37,8 @@ export interface DiscordConnectorConfig { } export class DiscordConnector implements Connector { - name: string; - instanceId: string; + name = "discord"; + id: string; private client: Client; private config: DiscordConnectorConfig; private handler: ((msg: IncomingMessage) => void) | null = null; @@ -49,8 +49,7 @@ export class DiscordConnector implements Connector { private typingIntervals = new Map>(); constructor(config: DiscordConnectorConfig) { - this.name = config.id || "discord"; - this.instanceId = config.id || "discord"; + this.id = config.id || "discord"; this.config = config; // Normalize Discord IDs to strings (YAML may parse large snowflake IDs as numbers) if (this.config.guildId) this.config.guildId = String(this.config.guildId); @@ -271,7 +270,7 @@ export class DiscordConnector implements Connector { if (!this.handler) return; - const sessionKey = deriveSessionKey(message, this.instanceId); + const sessionKey = deriveSessionKey(message, this.id); const replyContext = buildReplyContext(message); // Download attachments @@ -287,7 +286,7 @@ export class DiscordConnector implements Connector { ).then((results) => results.filter(Boolean) as Array<{ name: string; localPath: string; mimeType: string }>); const incomingMessage: IncomingMessage = { - connector: this.instanceId, + connector: this.id, source: "discord", sessionKey, channel: message.channel.id, diff --git a/packages/jinn/src/connectors/discord/remote.ts b/packages/jinn/src/connectors/discord/remote.ts index 5d27c480f..3df4b5f05 100644 --- a/packages/jinn/src/connectors/discord/remote.ts +++ b/packages/jinn/src/connectors/discord/remote.ts @@ -20,6 +20,7 @@ export interface RemoteDiscordConfig { */ export class RemoteDiscordConnector implements Connector { name = "discord"; + id = "discord"; private handler: ((msg: IncomingMessage) => void) | null = null; private baseUrl: string; diff --git a/packages/jinn/src/connectors/slack/index.ts b/packages/jinn/src/connectors/slack/index.ts index ad940d7e4..25b078828 100644 --- a/packages/jinn/src/connectors/slack/index.ts +++ b/packages/jinn/src/connectors/slack/index.ts @@ -15,6 +15,7 @@ import { logger } from "../../shared/logger.js"; export class SlackConnector implements Connector { name = "slack"; + id: string; private app: App; private handler: ((msg: IncomingMessage) => void) | null = null; private readonly allowedUsers: Set | null; @@ -58,6 +59,7 @@ export class SlackConnector implements Connector { } constructor(config: SlackConnectorConfig) { + this.id = config.id || "slack"; this.app = new App({ token: config.botToken, appToken: config.appToken, @@ -129,7 +131,7 @@ export class SlackConnector implements Connector { return; } - const sessionKey = deriveSessionKey(event as any); + const sessionKey = deriveSessionKey(event as any, this.id); const replyContext = buildReplyContext(event as any); // Fetch parent message for thread replies so the session has full context @@ -177,7 +179,7 @@ export class SlackConnector implements Connector { const channelName = await this.resolveChannelName((event as any).channel); const msg: IncomingMessage = { - connector: this.name, + connector: this.id, source: "slack", sessionKey, replyContext, @@ -228,7 +230,7 @@ export class SlackConnector implements Connector { return; } - const sessionKey = deriveSessionKey(event as any); + const sessionKey = deriveSessionKey(event as any, this.id); const replyContext = buildReplyContext(event as any); const channelName = await this.resolveChannelName(event.channel); @@ -255,7 +257,7 @@ export class SlackConnector implements Connector { } const msg: IncomingMessage = { - connector: this.name, + connector: this.id, source: "slack", sessionKey, replyContext, @@ -368,10 +370,10 @@ export class SlackConnector implements Connector { // Build the prompt with reaction context const prompt = `[Reaction :${emoji}: on message in ${channelDisplay}]\n\nOriginal message:\n"${messageText}"\n\nThe user reacted with :${emoji}: to this message. Interpret and act on the reaction.`; - const sessionKey = `slack:reaction:${channelId}:${messageTs}`; + const sessionKey = `${this.id}:reaction:${channelId}:${messageTs}`; const msg: IncomingMessage = { - connector: this.name, + connector: this.id, source: "slack", sessionKey, replyContext: { diff --git a/packages/jinn/src/connectors/slack/threads.test.ts b/packages/jinn/src/connectors/slack/threads.test.ts index 901a1de98..7c267d74e 100644 --- a/packages/jinn/src/connectors/slack/threads.test.ts +++ b/packages/jinn/src/connectors/slack/threads.test.ts @@ -40,6 +40,13 @@ test("deriveSessionKey treats same-ts thread_ts as root message", () => { expect(key).toBe("slack:C123:1700000000.000100"); }); +test("deriveSessionKey honours a custom instance prefix", () => { + const dm = { channel: "D123", user: "U123", channel_type: "im", ts: "1700000000.000100" }; + expect(deriveSessionKey(dm, "slack-support")).toBe("slack-support:dm:U123"); + const channel = { channel: "C123", user: "U123", ts: "1700000000.000100" }; + expect(deriveSessionKey(channel, "slack-support")).toBe("slack-support:C123:1700000000.000100"); +}); + test("buildReplyContext sets thread for channel root messages", () => { const context = buildReplyContext({ channel: "C123", diff --git a/packages/jinn/src/connectors/slack/threads.ts b/packages/jinn/src/connectors/slack/threads.ts index d75c0df99..0e1ce4e94 100644 --- a/packages/jinn/src/connectors/slack/threads.ts +++ b/packages/jinn/src/connectors/slack/threads.ts @@ -8,18 +8,11 @@ export interface SlackMessageEventLike { channel_type?: string; } -export function deriveSessionKey(event: SlackMessageEventLike): string { - if (event.channel_type === "im") { - return `slack:dm:${event.user || "unknown"}`; - } - - // Thread reply — use thread_ts (which is the root message's ts) - if (event.thread_ts && event.thread_ts !== event.ts) { - return `slack:${event.channel}:${event.thread_ts}`; - } - - // Root channel message — use ts so thread replies will match - return `slack:${event.channel}:${event.ts}`; +export function deriveSessionKey(event: SlackMessageEventLike, prefix = "slack"): string { + if (event.channel_type === "im") return `${prefix}:dm:${event.user || "unknown"}`; + // Thread replies key off thread_ts (the root's ts), so they land on the root's session. + const ts = event.thread_ts && event.thread_ts !== event.ts ? event.thread_ts : event.ts; + return `${prefix}:${event.channel}:${ts}`; } export function buildReplyContext(event: SlackMessageEventLike): ReplyContext { diff --git a/packages/jinn/src/connectors/telegram/__tests__/connector.test.ts b/packages/jinn/src/connectors/telegram/__tests__/connector.test.ts index 2ea692a8a..05b78db8c 100644 --- a/packages/jinn/src/connectors/telegram/__tests__/connector.test.ts +++ b/packages/jinn/src/connectors/telegram/__tests__/connector.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import type { IncomingMessage, Target } from "../../../shared/types.js"; +import type { IncomingMessage, Session, Target } from "../../../shared/types.js"; // Mock node-telegram-bot-api before importing connector const mockSendMessage = vi.fn().mockResolvedValue({ message_id: 1 }); @@ -32,6 +32,7 @@ vi.mock("../../../shared/logger.js", () => ({ // Import after mocks are set up const { TelegramConnector } = await import("../index.js"); +const { deliverConnectorReply } = await import("../../../gateway/api.js"); describe("TelegramConnector", () => { let connector: InstanceType; @@ -97,6 +98,43 @@ describe("TelegramConnector", () => { }); describe("onMessage", () => { + it("stamps and replies through a named connector instance id", async () => { + const named = new TelegramConnector({ + id: "telegram-support", + botToken: "123456:ABC-DEF", + }); + const handler = vi.fn(); + named.onMessage(handler); + await named.start(); + + const messageCallback = mockOn.mock.calls.find( + (call) => call[0] === "message", + )?.[1]; + await messageCallback({ + message_id: 42, + chat: { id: 12345, type: "private" as const }, + from: { id: 67890, username: "testuser", first_name: "Test", is_bot: false }, + date: Math.floor(Date.now() / 1000) + 10, + text: "Hello named bot!", + }); + + const incoming: IncomingMessage = handler.mock.calls[0][0]; + expect(incoming.connector).toBe("telegram-support"); + expect(incoming.sessionKey).toBe("telegram-support:12345"); + + await deliverConnectorReply({ + id: "session-named", + source: incoming.source, + connector: incoming.connector, + replyContext: incoming.replyContext, + } as Session, "Named reply", new Map([[named.id, named]])); + + expect(mockSendMessage).toHaveBeenCalledWith("12345", "Named reply", { + parse_mode: "Markdown", + reply_parameters: { message_id: 42 }, + }); + }); + it("routes incoming messages to the handler", async () => { const handler = vi.fn(); connector.onMessage(handler); diff --git a/packages/jinn/src/connectors/telegram/__tests__/threads.test.ts b/packages/jinn/src/connectors/telegram/__tests__/threads.test.ts index 740be6622..76bca6853 100644 --- a/packages/jinn/src/connectors/telegram/__tests__/threads.test.ts +++ b/packages/jinn/src/connectors/telegram/__tests__/threads.test.ts @@ -19,6 +19,12 @@ describe("deriveSessionKey", () => { deriveSessionKey({ chat: { id: -1001234, type: "supergroup" }, message_id: 1 }), ).toBe("telegram:-1001234"); }); + + it("honours a custom instance prefix", () => { + expect( + deriveSessionKey({ chat: { id: 12345, type: "private" }, message_id: 1 }, "telegram-support"), + ).toBe("telegram-support:12345"); + }); }); describe("buildReplyContext", () => { diff --git a/packages/jinn/src/connectors/telegram/index.ts b/packages/jinn/src/connectors/telegram/index.ts index 65f7c2fb4..d963d368c 100644 --- a/packages/jinn/src/connectors/telegram/index.ts +++ b/packages/jinn/src/connectors/telegram/index.ts @@ -27,6 +27,7 @@ type SendMessageOptions = Omit; export class TelegramConnector implements Connector { name = "telegram"; + id: string; private bot: TelegramBot; private handler: ((msg: IncomingMessage) => void) | null = null; private readonly allowedUsers: Set | null; @@ -48,6 +49,7 @@ export class TelegramConnector implements Connector { private sttPending = 0; constructor(config: TelegramConnectorConfig) { + this.id = config.id || "telegram"; this.bot = new TelegramBot(config.botToken, { polling: false }); this.ignoreOldMessagesOnBoot = config.ignoreOldMessagesOnBoot !== false; this.allowedUsers = @@ -101,7 +103,7 @@ export class TelegramConnector implements Connector { } } - const sessionKey = deriveSessionKey(telegramMsg); + const sessionKey = deriveSessionKey(telegramMsg, this.id); const replyContext = buildReplyContext(telegramMsg); const username = @@ -302,7 +304,7 @@ export class TelegramConnector implements Connector { } const msg: IncomingMessage = { - connector: this.name, + connector: this.id, source: "telegram", sessionKey, replyContext, diff --git a/packages/jinn/src/connectors/telegram/threads.ts b/packages/jinn/src/connectors/telegram/threads.ts index 8f78ac850..2c64067ee 100644 --- a/packages/jinn/src/connectors/telegram/threads.ts +++ b/packages/jinn/src/connectors/telegram/threads.ts @@ -7,12 +7,9 @@ export interface TelegramMessageLike { date?: number; } -/** - * Derive a session key from a Telegram message. - * Format: telegram: - */ -export function deriveSessionKey(msg: TelegramMessageLike): string { - return `telegram:${msg.chat.id}`; +/** Derive a session key from a Telegram message. Format: `:`. */ +export function deriveSessionKey(msg: TelegramMessageLike, prefix = "telegram"): string { + return `${prefix}:${msg.chat.id}`; } /** diff --git a/packages/jinn/src/connectors/whatsapp/__tests__/connector.test.ts b/packages/jinn/src/connectors/whatsapp/__tests__/connector.test.ts new file mode 100644 index 000000000..74a2932a8 --- /dev/null +++ b/packages/jinn/src/connectors/whatsapp/__tests__/connector.test.ts @@ -0,0 +1,69 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const testHome = fs.mkdtempSync(path.join(os.tmpdir(), "jinn-whatsapp-identity-")); +process.env.JINN_HOME = testHome; + +const mocks = vi.hoisted(() => { + const eventOn = vi.fn(); + const useMultiFileAuthState = vi.fn(async (_authDir: string) => ({ state: {}, saveCreds: vi.fn() })); + const socket = { + ev: { on: eventOn }, + end: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn(), + sendPresenceUpdate: vi.fn().mockResolvedValue(undefined), + }; + return { + eventOn, + useMultiFileAuthState, + makeWASocket: vi.fn(() => socket), + }; +}); + +vi.mock("@whiskeysockets/baileys", () => ({ + default: mocks.makeWASocket, + Browsers: { macOS: vi.fn(() => ["macOS", "Chrome", "test"]) }, + DisconnectReason: { loggedOut: 401 }, + fetchLatestWaWebVersion: vi.fn().mockResolvedValue({ version: undefined }), + useMultiFileAuthState: mocks.useMultiFileAuthState, + downloadMediaMessage: vi.fn(), +})); + +vi.mock("../../../shared/logger.js", () => ({ + logger: { + info: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + error: vi.fn(), + }, +})); + +const { WhatsAppConnector } = await import("../index.js"); + +describe("WhatsAppConnector identity", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("preserves legacy auth storage and isolates named default auth state", async () => { + const legacy = new WhatsAppConnector({}); + const support = new WhatsAppConnector({ id: "whatsapp-support" }); + const operations = new WhatsAppConnector({ id: "whatsapp-operations" }); + + await legacy.start(); + await support.start(); + await operations.start(); + + const authRoot = path.join(testHome, ".whatsapp-auth"); + expect(mocks.useMultiFileAuthState.mock.calls.map(([authDir]) => authDir)).toEqual([ + authRoot, + path.join(authRoot, "whatsapp-support"), + path.join(authRoot, "whatsapp-operations"), + ]); + expect(fs.existsSync(authRoot)).toBe(true); + expect(fs.existsSync(path.join(authRoot, "whatsapp-support"))).toBe(true); + expect(fs.existsSync(path.join(authRoot, "whatsapp-operations"))).toBe(true); + }); +}); diff --git a/packages/jinn/src/connectors/whatsapp/index.ts b/packages/jinn/src/connectors/whatsapp/index.ts index c574f4868..08da1fdfd 100644 --- a/packages/jinn/src/connectors/whatsapp/index.ts +++ b/packages/jinn/src/connectors/whatsapp/index.ts @@ -24,6 +24,8 @@ import path from "node:path"; import fs from "node:fs"; export interface WhatsAppConnectorConfig { + /** Unique instance identifier (e.g. "whatsapp-main") */ + id?: string; /** Where to store session credentials (default: JINN_HOME/.whatsapp-auth) */ authDir?: string; /** Allowed phone numbers in JID format (e.g. "447700900000@s.whatsapp.net") — empty = allow all */ @@ -44,6 +46,7 @@ const silentLogger = { export class WhatsAppConnector implements Connector { name = "whatsapp"; + id: string; private sock: WASocket | null = null; private config: WhatsAppConnectorConfig; private handler: ((msg: IncomingMessage) => void) | null = null; @@ -65,8 +68,10 @@ export class WhatsAppConnector implements Connector { }; constructor(config: WhatsAppConnectorConfig) { + this.id = config.id || "whatsapp"; this.config = config; - this.authDir = config.authDir ?? path.join(JINN_HOME, ".whatsapp-auth"); + const defaultAuthDir = path.join(JINN_HOME, ".whatsapp-auth"); + this.authDir = config.authDir ?? (this.id === "whatsapp" ? defaultAuthDir : path.join(defaultAuthDir, this.id)); this.allowedJids = new Set(config.allowFrom ?? []); fs.mkdirSync(this.authDir, { recursive: true }); } @@ -305,11 +310,11 @@ export class WhatsAppConnector implements Connector { } } - const sessionKey = `whatsapp:${jid}`; + const sessionKey = `${this.id}:${jid}`; const replyContext = { channel: jid, thread: null, messageTs: message.key.id ?? null }; const incomingMessage: IncomingMessage = { - connector: "whatsapp", + connector: this.id, source: "whatsapp", sessionKey, channel: jid, diff --git a/packages/jinn/src/gateway/__tests__/connectors.test.ts b/packages/jinn/src/gateway/__tests__/connectors.test.ts index 95fb70c33..037771553 100644 --- a/packages/jinn/src/gateway/__tests__/connectors.test.ts +++ b/packages/jinn/src/gateway/__tests__/connectors.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, it } from "vitest"; -import { connectorInstancesFromConfig, createConnector } from "../server.js"; +import { describe, expect, it, vi } from "vitest"; +import { connectorInstancesFromConfig, createConnector, reloadConnectorRegistry } from "../server.js"; import { SlackConnector } from "../../connectors/slack/index.js"; import { DiscordConnector } from "../../connectors/discord/index.js"; import { RemoteDiscordConnector } from "../../connectors/discord/remote.js"; @@ -60,6 +60,26 @@ describe("connectorInstancesFromConfig", () => { ); expect(normalized.map((entry) => entry.id)).toEqual(["slack", "telegram-support"]); }); + + it.each([ + "", + "Slack-Support", + "slack support", + "-slack", + "_slack", + `s${"x".repeat(64)}`, + ])("rejects invalid connector instance id %j during normalization", (id) => { + expect(() => connectorInstancesFromConfig(configWith({ + instances: [{ id, type: "slack", appToken: "xapp-test", botToken: "xoxb-test" }], + }))).toThrow(/invalid connector instance id/i); + }); + + it("accepts the 64-character lowercase connector id boundary", () => { + const id = `s${"x".repeat(63)}`; + expect(connectorInstancesFromConfig(configWith({ + instances: [{ id, type: "slack", appToken: "xapp-test", botToken: "xoxb-test" }], + }))[0].id).toBe(id); + }); }); describe("createConnector", () => { @@ -69,14 +89,79 @@ describe("createConnector", () => { it("dispatches every supported type to its connector class", () => { expect(build("slack", { appToken: "xapp-test", botToken: "xoxb-test" })).toBeInstanceOf(SlackConnector); expect(build("discord", { botToken: "d-test" })).toBeInstanceOf(DiscordConnector); - expect(build("discord", { proxyVia: "http://127.0.0.1:1" })).toBeInstanceOf(RemoteDiscordConnector); + expect(createConnector({ id: "discord", type: "discord", config: { id: "discord", proxyVia: "http://127.0.0.1:1" } })).toBeInstanceOf(RemoteDiscordConnector); expect(build("telegram", { botToken: "tg-test" })).toBeInstanceOf(TelegramConnector); expect(build("whatsapp")).toBeInstanceOf(WhatsAppConnector); }); + it("carries the instance id on id while name stays the type literal", () => { + const slack = build("slack", { appToken: "xapp-test", botToken: "xoxb-test" }); + expect([slack.id, slack.name]).toEqual(["slack-1", "slack"]); + const discord = build("discord", { botToken: "d-test" }); + expect([discord.id, discord.name]).toEqual(["discord-1", "discord"]); + }); + it("rejects an unknown type", () => { expect(() => build("carrier-pigeon")).toThrow(/Unknown connector type "carrier-pigeon"/); }); + + it("rejects named Remote Discord until authenticated identity is supported", () => { + expect(() => build("discord", { proxyVia: "http://127.0.0.1:1" })) + .toThrow(/named Remote Discord.*not supported/i); + }); + + it("keeps legacy Remote Discord identity and proxy addressing byte-identical", async () => { + const fetchMock = vi.fn().mockResolvedValue(new Response(JSON.stringify({ messageId: "m1" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + })); + vi.stubGlobal("fetch", fetchMock); + try { + const remote = createConnector({ + id: "discord", + type: "discord", + config: { id: "discord", proxyVia: "http://127.0.0.1:7788" }, + }); + expect([remote.id, remote.name]).toEqual(["discord", "discord"]); + await remote.sendMessage({ channel: "C1" }, "hello"); + expect(fetchMock).toHaveBeenCalledWith( + "http://127.0.0.1:7788/api/connectors/discord/proxy", + expect.objectContaining({ method: "POST" }), + ); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("keeps the local-to-remote Discord session key on the legacy discord prefix", async () => { + const fetchMock = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + try { + const local = build("discord", { botToken: "d-test" }) as DiscordConnector; + await (local as unknown as { proxyToRemote(url: string, message: unknown): Promise }).proxyToRemote( + "http://127.0.0.1:7788", + { + id: "M1", + content: "hello", + attachments: new Map(), + author: { id: "U1", username: "tester" }, + guild: { id: "G1" }, + channel: { + id: "C1", + name: "general", + isDMBased: () => false, + isThread: () => false, + isTextBased: () => true, + }, + }, + ); + const request = fetchMock.mock.calls[0][1] as RequestInit; + expect(JSON.parse(String(request.body)).sessionKey).toBe("discord:C1"); + expect(fetchMock.mock.calls[0][0]).toBe("http://127.0.0.1:7788/api/connectors/discord/incoming"); + } finally { + vi.unstubAllGlobals(); + } + }); }); describe("connector wiring", () => { @@ -124,4 +209,29 @@ describe("connector wiring", () => { registry.set("discord-ops", stubConnector(async () => {})); expect(names()).toEqual(["slack", "slack-second", "discord-ops"]); }); + + it("rejects an invalid reload config before stopping or deleting live connectors", async () => { + const stop = vi.fn(async () => {}); + const live = { + id: "telegram", + name: "telegram", + start: async () => {}, + stop, + sendMessage: async () => undefined, + onMessage: () => {}, + } as unknown as Connector; + const registry = new Map([[live.id, live]]); + + await expect(reloadConnectorRegistry({ + connectorMap: registry, + loadInstances: () => connectorInstancesFromConfig(configWith({ + instances: [{ id: "Invalid ID", type: "telegram", botToken: "tg-new" }], + })), + initConnector: async () => {}, + describeConnector: (instance) => `connector "${instance.id}"`, + })).rejects.toThrow(/invalid connector instance id/i); + + expect(stop).not.toHaveBeenCalled(); + expect([...registry.entries()]).toEqual([["telegram", live]]); + }); }); diff --git a/packages/jinn/src/gateway/__tests__/instance-migration-api.test.ts b/packages/jinn/src/gateway/__tests__/instance-migration-api.test.ts index 0d0a34700..3583b7a47 100644 --- a/packages/jinn/src/gateway/__tests__/instance-migration-api.test.ts +++ b/packages/jinn/src/gateway/__tests__/instance-migration-api.test.ts @@ -5,6 +5,7 @@ import path from "node:path" import { Readable } from "node:stream" import type { ServerResponse } from "node:http" import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest" +import type { Connector, IncomingMessage } from "../../shared/types.js" const registryHome = fs.mkdtempSync(path.join(os.tmpdir(), "jinn-migration-api-registry-")) process.env.JINN_HOME = registryHome @@ -117,10 +118,89 @@ beforeEach(() => { seedBundle() dispatched.length = 0 engineAvailable = true + context.connectors.clear() for (const session of registry.listSessions()) registry.deleteSession(session.id) }) +function connectorStub(id: string, name: string): Connector { + const capabilities = { threading: false, messageEdits: false, reactions: false, attachments: false } + return { + id, + name, + start: async () => {}, + stop: async () => {}, + getCapabilities: () => capabilities, + getHealth: () => ({ status: "running", capabilities }), + reconstructTarget: () => ({ channel: "test" }), + sendMessage: async () => undefined, + replyMessage: async () => undefined, + addReaction: async () => {}, + removeReaction: async () => {}, + editMessage: async () => {}, + onMessage: () => {}, + } +} + describe("instance migration API", () => { + it("keeps operator-chosen connector labels out of public status", async () => { + context.connectors.set("private-support-label", connectorStub("private-support-label", "slack")) + context.connectors.set("private-ops-label", connectorStub("private-ops-label", "telegram")) + + const status = await request("GET", "/api/status", undefined, false) + + expect(status.status).toBe(200) + expect(Object.keys(status.body.connectors).sort()).toEqual(["slack", "telegram"]) + expect(JSON.stringify(status.body)).not.toContain("private-support-label") + expect(JSON.stringify(status.body)).not.toContain("private-ops-label") + }) + + it("addresses the selected named WhatsApp instance when reading a QR code", async () => { + const legacyQr = vi.fn(() => "legacy-qr") + const supportQr = vi.fn(() => "support-qr") + context.connectors.set("whatsapp", Object.assign(connectorStub("whatsapp", "whatsapp"), { getQrCode: legacyQr })) + context.connectors.set("whatsapp-support", Object.assign(connectorStub("whatsapp-support", "whatsapp"), { getQrCode: supportQr })) + + const named = await request("GET", "/api/connectors/whatsapp-support/qr") + + expect(named.status).toBe(200) + expect(named.body.qr).toMatch(/^data:image\/png;base64,/) + expect(supportQr).toHaveBeenCalledOnce() + expect(legacyQr).not.toHaveBeenCalled() + }) + + it("rejects HTTP connector ids outside the normalized lowercase contract", async () => { + const invalid = await request("POST", "/api/connectors/Slack-Support/send", { channel: "C1", text: "hello" }) + expect(invalid.status).toBe(400) + expect(invalid.body.error).toMatch(/connector id/i) + }) + + it("keeps legacy Remote Discord inbound identity and rejects named route widening", async () => { + const legacyDelivery = vi.fn<(message: IncomingMessage) => void>() + context.connectors.set("discord", Object.assign(connectorStub("discord", "discord"), { deliverMessage: legacyDelivery })) + const body = { + sessionKey: "discord:C1", + channel: "C1", + user: "tester", + userId: "U1", + text: "hello", + messageId: "M1", + replyContext: { channel: "C1" }, + } + + const legacy = await request("POST", "/api/connectors/discord/incoming", body) + expect(legacy.status).toBe(200) + expect(legacyDelivery).toHaveBeenCalledWith(expect.objectContaining({ + connector: "discord", + sessionKey: "discord:C1", + })) + + const namedDelivery = vi.fn<(message: IncomingMessage) => void>() + context.connectors.set("discord-ops", Object.assign(connectorStub("discord-ops", "discord"), { deliverMessage: namedDelivery })) + const named = await request("POST", "/api/connectors/discord-ops/incoming", body) + expect(named.status).toBe(404) + expect(namedDelivery).not.toHaveBeenCalled() + }) + it("adds version and a compact migration summary to status and exposes the canonical contract", async () => { const status = await request("GET", "/api/status") const migration = await request("GET", "/api/instance-migration") diff --git a/packages/jinn/src/gateway/__tests__/run-web-session-connector-reply.test.ts b/packages/jinn/src/gateway/__tests__/run-web-session-connector-reply.test.ts index 9c2988ca6..babaa2c46 100644 --- a/packages/jinn/src/gateway/__tests__/run-web-session-connector-reply.test.ts +++ b/packages/jinn/src/gateway/__tests__/run-web-session-connector-reply.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { deliverConnectorReply } from "../api.js"; +import { logger } from "../../shared/logger.js"; import type { Connector, Session } from "../../shared/types.js"; /** Build a minimal mocked connector exposing the two methods the helper uses. */ @@ -13,8 +14,8 @@ function makeConnector(name: string) { /** Build the minimal slice of a Session the helper reads. */ function makeSession( - overrides: Partial> = {}, -): Pick { + overrides: Partial> & { id?: string } = {}, +): Pick & { id?: string } { return { source: "slack", connector: "slack", @@ -57,10 +58,23 @@ describe("deliverConnectorReply", () => { expect(slack.replyMessage).not.toHaveBeenCalled(); }); - it("does not throw and does not call when connector missing from map", async () => { - const session = makeSession({ connector: "telegram", source: "telegram" }); + it("delivers to a named connector instance keyed by instance id", async () => { + const support = makeConnector("slack"); + const named = new Map([["slack-support", support.connector]]); + await deliverConnectorReply(makeSession({ connector: "slack-support" }), "hi", named); + expect(support.reconstructTarget).toHaveBeenCalledTimes(1); + expect(support.replyMessage).toHaveBeenCalledTimes(1); + }); + + it("logs a warning and does not call when connector missing from map", async () => { + const warn = vi.spyOn(logger, "warn").mockImplementation(() => {}); + const session = makeSession({ id: "sess-42", connector: "telegram", source: "telegram" }); await expect(deliverConnectorReply(session, "hi", map)).resolves.toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0][0]).toContain("sess-42"); + expect(warn.mock.calls[0][0]).toContain("telegram"); expect(slack.replyMessage).not.toHaveBeenCalled(); + warn.mockRestore(); }); it("does not deliver when text is empty", async () => { diff --git a/packages/jinn/src/gateway/__tests__/session-attempt-race.test.ts b/packages/jinn/src/gateway/__tests__/session-attempt-race.test.ts index 1845a9911..26b039c00 100644 --- a/packages/jinn/src/gateway/__tests__/session-attempt-race.test.ts +++ b/packages/jinn/src/gateway/__tests__/session-attempt-race.test.ts @@ -98,6 +98,7 @@ function connectorStub(): Connector { const target: Target = { channel: "test" }; return { name: "test", + id: "test", start: async () => {}, stop: async () => {}, getCapabilities: () => ({ threading: false, messageEdits: false, reactions: false, attachments: false }), diff --git a/packages/jinn/src/gateway/api.ts b/packages/jinn/src/gateway/api.ts index 922b88c5c..d5ce85899 100644 --- a/packages/jinn/src/gateway/api.ts +++ b/packages/jinn/src/gateway/api.ts @@ -28,6 +28,7 @@ import type { SessionManager } from "../sessions/manager.js"; import { buildContext, buildPlatformContextSnapshot, type BuildContextOptions } from "../sessions/context.js"; import { buildPlatformContextRefresh, fingerprintPlatformContext } from "../engines/platform-context.js"; import { stripControlChars, hasControlBytes } from "../shared/sanitize.js"; +import { CONNECTOR_ID_REQUIREMENTS, isValidConnectorId } from "../shared/connector-id.js"; import { initDb } from "../shared/db.js"; import { listSessions, @@ -6167,12 +6168,9 @@ export async function handleApiRequest( } } - // POST /api/connectors/:id/incoming — receive proxied Discord messages from primary instance - // Supports both the legacy /api/connectors/discord/incoming and named instance ids - params = matchRoute("/api/connectors/:id/incoming", pathname); - if (method === "POST" && params && params.id) { - // Try the exact instance id first, then fall back to "discord" for the legacy path - const connector = context.connectors.get(params.id) ?? (params.id === "discord" ? context.connectors.get("discord") : undefined); + // POST /api/connectors/discord/incoming — receive proxied Discord messages from a primary instance + if (method === "POST" && pathname === "/api/connectors/discord/incoming") { + const connector = context.connectors.get("discord"); if (!connector) return notFound(res); if (!("deliverMessage" in connector)) { return json(res, { error: "Discord connector is not in remote mode" }, 400); @@ -6200,7 +6198,7 @@ export async function handleApiRequest( ); const incomingMsg: IncomingMessage = { - connector: params.id, + connector: "discord", source: "discord", sessionKey: body.sessionKey, channel: body.channel, @@ -6220,11 +6218,9 @@ export async function handleApiRequest( return json(res, { status: "delivered" }); } - // POST /api/connectors/:id/proxy — proxy connector operations from remote instances - // Supports both the legacy /api/connectors/discord/proxy and named instance ids - params = matchRoute("/api/connectors/:id/proxy", pathname); - if (method === "POST" && params && params.id) { - const connector = context.connectors.get(params.id) ?? (params.id === "discord" ? context.connectors.get("discord") : undefined); + // POST /api/connectors/discord/proxy — proxy connector operations from remote instances + if (method === "POST" && pathname === "/api/connectors/discord/proxy") { + const connector = context.connectors.get("discord"); if (!connector) return notFound(res); const _parsed = await readJsonBody(req, res); @@ -6269,9 +6265,10 @@ export async function handleApiRequest( return json(res, { status: "ok", messageId }); } - // POST /api/connectors/:name/send — send a message via a connector + // POST /api/connectors/:name/send — send via the connector with that instance id params = matchRoute("/api/connectors/:name/send", pathname); if (method === "POST" && params) { + if (!isValidConnectorId(params.name)) return badRequest(res, `connector id ${CONNECTOR_ID_REQUIREMENTS}`); const connector = context.connectors.get(params.name); if (!connector) return notFound(res); const _parsed = await readJsonBody(req, res); @@ -6286,10 +6283,12 @@ export async function handleApiRequest( return json(res, { status: "sent" }); } - // GET /api/connectors/whatsapp/qr — return current QR code as PNG data URL - if (method === "GET" && pathname === "/api/connectors/whatsapp/qr") { - const waConnector = context.connectors.get("whatsapp"); - if (!waConnector) return notFound(res); + // GET /api/connectors/:id/qr — return the selected WhatsApp instance QR as a PNG data URL + params = matchRoute("/api/connectors/:id/qr", pathname); + if (method === "GET" && params) { + if (!isValidConnectorId(params.id)) return badRequest(res, `connector id ${CONNECTOR_ID_REQUIREMENTS}`); + const waConnector = context.connectors.get(params.id); + if (!waConnector || waConnector.name !== "whatsapp" || !("getQrCode" in waConnector)) return notFound(res); const qrString = (waConnector as WhatsAppConnector).getQrCode(); if (!qrString) return json(res, { qr: null }); const dataUrl = await QRCode.toDataURL(qrString, { width: 256, margin: 2 }); @@ -6866,7 +6865,10 @@ export async function deliverConnectorReply( if (!text || NON_CONNECTOR_SOURCES.has(session.source)) return; if (!session.connector || !session.replyContext) return; const connector = connectors.get(session.connector); - if (!connector) return; + if (!connector) { + logger.warn(`Connector reply dropped for session ${session.id ?? "?"}: no connector registered as "${session.connector}"`); + return; + } try { const target = connector.reconstructTarget(session.replyContext); await connector.replyMessage(target, text); diff --git a/packages/jinn/src/gateway/server.ts b/packages/jinn/src/gateway/server.ts index 344117001..41be892a6 100644 --- a/packages/jinn/src/gateway/server.ts +++ b/packages/jinn/src/gateway/server.ts @@ -20,6 +20,7 @@ import { refreshPiModels, } from "../shared/models.js"; import { configureLogger, logger } from "../shared/logger.js"; +import { CONNECTOR_ID_REQUIREMENTS, isValidConnectorId } from "../shared/connector-id.js"; import { scheduleFtsBackfill, recoverStaleSessions, recoverStaleWorkflowAttemptSessions, recoverStaleQueueItems, clearAllPartialMessages, consumeRestartAcknowledgements, getInterruptedSessions, listSessions, updateSession, getSession, getMessages, getSessionSpend, RESTART_ACK_META_KEY } from "../sessions/registry.js"; import { initDb } from "../shared/db.js"; import { SessionManager, type RouteOptions } from "../sessions/manager.js"; @@ -381,7 +382,10 @@ export function connectorInstancesFromConfig(config: JinnConfig): NormalizedConn const instances: NormalizedConnector[] = []; const seen = new Set(); - const add = (id: string, type: string, raw: Record): void => { + const add = (id: unknown, type: string, raw: Record): void => { + if (!isValidConnectorId(id)) { + throw new Error(`Invalid connector instance id ${JSON.stringify(id)}: ${CONNECTOR_ID_REQUIREMENTS}`); + } if (seen.has(id)) { logger.warn(`Duplicate connector instance id "${id}", skipping`); return; @@ -400,7 +404,7 @@ export function connectorInstancesFromConfig(config: JinnConfig): NormalizedConn for (const instance of declared.instances ?? []) { const { id, type, ...rest } = instance; - if (!id || !type) { + if (id === undefined || id === null || !type) { logger.warn(`Skipping connector instance without id or type`); continue; } @@ -418,9 +422,13 @@ export function createConnector(instance: NormalizedConnector): Connector { return new SlackConnector(config as unknown as SlackConnectorConfig); case "discord": // Remote mode proxies all Discord I/O through the primary instance. - return config.proxyVia - ? new RemoteDiscordConnector({ proxyVia: String(config.proxyVia), channelId: config.channelId as string | undefined }) - : new DiscordConnector(config as unknown as DiscordConnectorConfig); + if (config.proxyVia) { + if (instance.id !== "discord") { + throw new Error("Named Remote Discord instances are not supported until the proxy protocol authenticates and validates instance identity"); + } + return new RemoteDiscordConnector({ proxyVia: String(config.proxyVia), channelId: config.channelId as string | undefined }); + } + return new DiscordConnector(config as unknown as DiscordConnectorConfig); case "telegram": return new TelegramConnector(config as unknown as TelegramConnectorConfig); case "whatsapp": @@ -430,6 +438,52 @@ export function createConnector(instance: NormalizedConnector): Connector { } } +interface ReloadConnectorRegistryOptions { + connectorMap: Map; + loadInstances: () => NormalizedConnector[]; + initConnector: (instance: NormalizedConnector) => Promise; + describeConnector: (instance: NormalizedConnector) => string; +} + +/** Reload a live connector registry from freshly normalized declarations. */ +export async function reloadConnectorRegistry({ + connectorMap, + loadInstances, + initConnector, + describeConnector, +}: ReloadConnectorRegistryOptions): Promise<{ started: string[]; stopped: string[]; errors: string[] }> { + const instances = loadInstances(); + const started: string[] = []; + const stopped: string[] = []; + const errors: string[] = []; + + for (const [id, connector] of [...connectorMap.entries()]) { + try { + await connector.stop(); + connectorMap.delete(id); + stopped.push(id); + logger.info(`Stopped connector "${id}" for reload`); + } catch (err) { + errors.push(`Failed to stop ${id}: ${err instanceof Error ? err.message : err}`); + } + } + + for (const instance of instances) { + // A connector that refused to stop is still live — leave it alone. + if (connectorMap.has(instance.id)) continue; + try { + await initConnector(instance); + started.push(instance.id); + logger.info(`Started ${describeConnector(instance)}`); + } catch (err) { + errors.push(`Failed to start "${instance.id}": ${err instanceof Error ? err.message : err}`); + logger.error(`Failed to start ${describeConnector(instance)}: ${err instanceof Error ? err.message : err}`); + } + } + + return { started, stopped, errors }; +} + export type GatewayCleanup = () => Promise; export async function startGateway( @@ -723,7 +777,6 @@ export async function startGateway( const sessionManager = new SessionManager(config, engines, bootId, (id) => employeeRegistry.get(id)); // Start connectors — one normalized list covers both config forms. - const connectors: Connector[] = []; const connectorMap = new Map(); /** @@ -745,7 +798,6 @@ export async function startGateway( logger.error(`${instance.id} route error: ${err instanceof Error ? err.message : err}`); }); }); - connectors.push(connector); connectorMap.set(instance.id, connector); return connector.start(); }; @@ -753,7 +805,7 @@ export async function startGateway( const describeConnector = (instance: NormalizedConnector): string => `connector "${instance.id}" (type: ${instance.type}, employee: ${instance.employee || "default"})`; - // Session context reads connector names off this map, so publish it before the + // Session context reads connector ids off this map, so publish it before the // first connector can deliver a message. sessionManager.setConnectorProvider(() => connectorMap); @@ -771,37 +823,12 @@ export async function startGateway( /** Stop every running connector and restart from fresh config (POST /api/connectors/reload). */ async function reloadConnectorInstances(): Promise<{ started: string[]; stopped: string[]; errors: string[] }> { - const started: string[] = []; - const stopped: string[] = []; - const errors: string[] = []; - - for (const [id, connector] of [...connectorMap.entries()]) { - try { - await connector.stop(); - connectorMap.delete(id); - const idx = connectors.indexOf(connector); - if (idx >= 0) connectors.splice(idx, 1); - stopped.push(id); - logger.info(`Stopped connector "${id}" for reload`); - } catch (err) { - errors.push(`Failed to stop ${id}: ${err instanceof Error ? err.message : err}`); - } - } - - for (const instance of connectorInstancesFromConfig(loadConfig())) { - // A connector that refused to stop is still live — leave it alone. - if (connectorMap.has(instance.id)) continue; - try { - await initConnector(instance); - started.push(instance.id); - logger.info(`Started ${describeConnector(instance)}`); - } catch (err) { - errors.push(`Failed to start "${instance.id}": ${err instanceof Error ? err.message : err}`); - logger.error(`Failed to start ${describeConnector(instance)}: ${err instanceof Error ? err.message : err}`); - } - } - - return { started, stopped, errors }; + return reloadConnectorRegistry({ + connectorMap, + loadInstances: () => connectorInstancesFromConfig(loadConfig()), + initConnector, + describeConnector, + }); } // Mutable config reference for hot-reload @@ -1414,7 +1441,7 @@ export async function startGateway( setTodoApprovalDecisionListener(null); // Stop connectors - for (const connector of connectors) { + for (const connector of connectorMap.values()) { try { await connector.stop(); } catch (err) { diff --git a/packages/jinn/src/mcp/connector-tools.ts b/packages/jinn/src/mcp/connector-tools.ts index 4811cf066..91e2875ed 100644 --- a/packages/jinn/src/mcp/connector-tools.ts +++ b/packages/jinn/src/mcp/connector-tools.ts @@ -1,6 +1,5 @@ import { assertBoundCaller, gatewayRequest, JinnMcpToolError, type JinnMcpTool } from "./toolkit.js"; - -const CONNECTOR_NAME = /^[a-z0-9][a-z0-9_-]{0,63}$/; +import { CONNECTOR_ID_REQUIREMENTS, isValidConnectorId } from "../shared/connector-id.js"; function requiredString(args: Record, name: string, max: number): string { const value = args[name]; @@ -41,8 +40,8 @@ export function buildConnectorTools(): JinnMcpTool[] { handler: async (args, ctx) => { assertBoundCaller(ctx); const connector = requiredString(args, "connector", 64); - if (!CONNECTOR_NAME.test(connector)) { - throw new JinnMcpToolError("connector must use lowercase letters, numbers, hyphens, or underscores"); + if (!isValidConnectorId(connector)) { + throw new JinnMcpToolError(`connector ${CONNECTOR_ID_REQUIREMENTS}`); } const channel = requiredString(args, "channel", 256); const text = requiredString(args, "text", 40_000); diff --git a/packages/jinn/src/sessions/__tests__/context.test.ts b/packages/jinn/src/sessions/__tests__/context.test.ts index c3332875a..70f08733b 100644 --- a/packages/jinn/src/sessions/__tests__/context.test.ts +++ b/packages/jinn/src/sessions/__tests__/context.test.ts @@ -426,7 +426,7 @@ describe("buildContext — audience scoping", () => { it("connector section is slim — recipe details live in CLAUDE.md", () => { const out = buildContext({ ...baseOpts, connectors: ["slack"] }); expect(out).toContain("## Available connectors: slack"); - expect(out).toContain("/api/connectors//send"); + expect(out).toContain("/api/connectors//send"); // The old per-connector recipe block is gone: expect(out).not.toContain("**Send threaded reply**"); }); diff --git a/packages/jinn/src/sessions/__tests__/platform-context-dispatch.test.ts b/packages/jinn/src/sessions/__tests__/platform-context-dispatch.test.ts index 75cdc588f..e5564f743 100644 --- a/packages/jinn/src/sessions/__tests__/platform-context-dispatch.test.ts +++ b/packages/jinn/src/sessions/__tests__/platform-context-dispatch.test.ts @@ -72,6 +72,7 @@ function connectorStub(): Connector { const target: Target = { channel: "test" }; return { name: "test", + id: "test", start: async () => {}, stop: async () => {}, getCapabilities: () => ({ threading: false, messageEdits: false, reactions: false, attachments: false }), diff --git a/packages/jinn/src/sessions/context.ts b/packages/jinn/src/sessions/context.ts index caf9b355a..fd32507ba 100644 --- a/packages/jinn/src/sessions/context.ts +++ b/packages/jinn/src/sessions/context.ts @@ -396,7 +396,7 @@ export function buildContext(opts: BuildContextOptions): string { content: buildConnectorContext(opts.connectors, gatewayUrl, opts.jinnMcpAttached), summary: opts.jinnMcpAttached ? `## Available connectors: ${opts.connectors.join(", ")}\nUse Jinn MCP/company routing for company operations; connector configuration lives in config.` - : `## Available connectors: ${opts.connectors.join(", ")}\nUse \`curl POST ${gatewayUrl}/api/connectors//send\` to send messages.`, + : `## Available connectors: ${opts.connectors.join(", ")}\nUse \`curl POST ${gatewayUrl}/api/connectors//send\` to send messages.`, }); } @@ -996,7 +996,7 @@ function buildConnectorContext(connectors: string[], gatewayUrl: string, jinnMcp } return [ `## Available connectors: ${connectors.join(", ")}`, - `Send a message: \`curl -X POST ${gatewayUrl}/api/connectors//send -H "Authorization: Bearer $JINN_GATEWAY_TOKEN" -H 'Content-Type: application/json' -d '{"channel":"CHANNEL_ID","text":"message"}'\` (add \`"thread":"THREAD_TS"\` for a threaded reply).`, + `Send a message: \`curl -X POST ${gatewayUrl}/api/connectors//send -H "Authorization: Bearer $JINN_GATEWAY_TOKEN" -H 'Content-Type: application/json' -d '{"channel":"CHANNEL_ID","text":"message"}'\` (add \`"thread":"THREAD_TS"\` for a threaded reply).`, `Channel IDs are in \`~/.jinn/config.yaml\`. You may send proactively (completed tasks, errors, status updates). Details: CLAUDE.md / AGENTS.md.`, ].join("\n"); } diff --git a/packages/jinn/src/sessions/manager.ts b/packages/jinn/src/sessions/manager.ts index b74de8583..fae548d23 100644 --- a/packages/jinn/src/sessions/manager.ts +++ b/packages/jinn/src/sessions/manager.ts @@ -68,7 +68,7 @@ export interface RouteOptions { } const WORKFLOW_CAPABILITIES = { threading: false, messageEdits: false, reactions: false, attachments: false }; -const WORKFLOW_CONNECTOR: Connector = { name: "workflow", async start() {}, async stop() {}, getCapabilities: () => WORKFLOW_CAPABILITIES, getHealth: () => ({ status: "running", capabilities: WORKFLOW_CAPABILITIES }), reconstructTarget: () => ({ channel: "workflow" }), async sendMessage() {}, async replyMessage() {}, async addReaction() {}, async removeReaction() {}, async editMessage() {}, onMessage() {} }; +const WORKFLOW_CONNECTOR: Connector = { name: "workflow", id: "workflow", async start() {}, async stop() {}, getCapabilities: () => WORKFLOW_CAPABILITIES, getHealth: () => ({ status: "running", capabilities: WORKFLOW_CAPABILITIES }), reconstructTarget: () => ({ channel: "workflow" }), async sendMessage() {}, async replyMessage() {}, async addReaction() {}, async removeReaction() {}, async editMessage() {}, onMessage() {} }; function maybeRevertEngineOverride(session: Session): Session { const meta = (session.transportMeta || {}) as Record; const override = meta["engineOverride"] as Record | undefined; diff --git a/packages/jinn/src/shared/connector-id.ts b/packages/jinn/src/shared/connector-id.ts new file mode 100644 index 000000000..7efdb06b3 --- /dev/null +++ b/packages/jinn/src/shared/connector-id.ts @@ -0,0 +1,7 @@ +export const CONNECTOR_ID_PATTERN = /^[a-z0-9][a-z0-9_-]{0,63}$/; + +export function isValidConnectorId(value: unknown): value is string { + return typeof value === "string" && CONNECTOR_ID_PATTERN.test(value); +} + +export const CONNECTOR_ID_REQUIREMENTS = "must use 1-64 lowercase letters, numbers, hyphens, or underscores and start with a letter or number"; diff --git a/packages/jinn/src/shared/types.ts b/packages/jinn/src/shared/types.ts index 779c4719e..48de97f0e 100644 --- a/packages/jinn/src/shared/types.ts +++ b/packages/jinn/src/shared/types.ts @@ -270,6 +270,8 @@ export type ReplyContext = JsonObject; export interface Connector { name: string; + /** Instance id — the connector's registry key; equals the type for legacy top-level config. */ + id: string; start(): Promise; stop(): Promise; getCapabilities(): ConnectorCapabilities; @@ -504,6 +506,7 @@ export interface CronJob { } export interface CronDelivery { + /** Connector instance id, matching the gateway registry key. */ connector: string; channel: string; } diff --git a/packages/jinn/template/docs/connectors.md b/packages/jinn/template/docs/connectors.md index 806933971..77dd83971 100644 --- a/packages/jinn/template/docs/connectors.md +++ b/packages/jinn/template/docs/connectors.md @@ -7,6 +7,7 @@ Connectors are modular adapters that bridge external messaging platforms with {{ ```typescript interface Connector { name: string; + id: string; // Connector instance id and registry key start(): Promise; stop(): Promise; sendMessage(sourceRef: string, text: string): Promise; @@ -21,7 +22,7 @@ interface IncomingMessage { text: string; // Message content userId: string; // Platform user ID userName: string; // Display name - connector: string; // Connector name + connector: string; // Connector instance id } ``` diff --git a/packages/jinn/template/docs/cron.md b/packages/jinn/template/docs/cron.md index 5fcdb02af..6f1ad3a57 100644 --- a/packages/jinn/template/docs/cron.md +++ b/packages/jinn/template/docs/cron.md @@ -16,7 +16,7 @@ interface CronJob { employee?: string; // Employee persona to use prompt: string; // The prompt to send to the engine delivery?: { // Optional output delivery - connector: string; // Connector name (e.g., "slack") + connector: string; // Connector instance id (e.g., "slack" or "slack-support") channel: string; // Target channel or user }; }