From d84ce13dab32925385e241ea4f1138bf9487ea88 Mon Sep 17 00:00:00 2001 From: Daniele Date: Thu, 3 Sep 2026 23:48:00 +0100 Subject: [PATCH] ssh: fix sudo failing with "password required" (v6 / 1.1.0) The connector asked for a sudo password on every privileged call and failed whenever nobody answered it, which is every unattended run. - Always probe `sudo -n` first, even for aliases set to sudo="prompt". `_sudo_prefix` used to elicit unconditionally, so a host granting this user NOPASSWD still opened an Agent Inbox prompt; with no human there it hit the client's 300s ELICITATION_DEADLINE, got back `cancel`, and surfaced as "sudo password required (user declined or timed out)". sudo refuses before running anything when it wants a password, so the probe is side-effect free. - Strip a leading `sudo` from `command` and turn it into sudo=true. Agents write `exec(command="sudo systemctl restart x")`: with sudo=false that ran a tty-less sudo, with sudo=true it nested `sudo -S ... sudo ...` whose inner prompt had no tty either. Handles -u/-n/-S/-E/-H/-i/-k/-p/--; an unknown flag leaves the command alone. sudo_user now implies sudo=true. - Run privileged commands as `sh -c ''`, so `&&`, pipes and redirections are elevated too instead of only the first word. - Add SSH_MCP_SUDO_PASSWORD (optional, secret) for unattended runs. It is consulted only after `sudo -n` proved a password is needed, so on a NOPASSWD host it never lands in the command's own stdin. - Actionable errors for every sudo failure mode, and a `hint` on a nested sudo we could not peel off. General review of the same server: - Drain stdout and stderr together and make timeout_sec a real wall-clock deadline. Both streams share one SSH channel window, so reading stdout to EOF first stalled once a chatty stderr filled it. Command stdin is now closed after the optional password. - Queue messages that arrive while awaiting an elicitation reply instead of discarding them, so a concurrent tools/call is not lost. - Record the client's `elicitation` capability at initialize and fail fast when it is absent rather than blocking on a prompt nobody can answer. - Tolerate null/string integer arguments (depth, max_results, context_lines, timeout_sec). - Realign the version across both manifests: fragment.json said 5/1.0.4 while connector.json said 2/1.0.1, so the feed was permanently ahead of the installed version and offered an update forever. Also lands the pending docs work: CONNECTOR_MANIFEST_GUIDE.md as the single source of truth, docs/connector.manifest_guide.md retired to a pointer, CLAUDE.md audited against the repo, compile.py docstring fixed, and an opencode.json config. --- CHANGELOG.md | 31 ++ CLAUDE.md | 123 +++++-- CONNECTOR_MANIFEST_GUIDE.md | 600 ++++++++++++++++++++----------- connectors/connectors.json | 14 +- connectors/ssh/connector.json | 17 +- connectors/ssh/fragment.json | 6 +- connectors/ssh/ssh_mcp_server.py | 372 +++++++++++++++---- docs/connector.manifest_guide.md | 385 +------------------- opencode.json | 27 ++ scripts/compile.py | 8 +- 10 files changed, 875 insertions(+), 708 deletions(-) create mode 100644 opencode.json diff --git a/CHANGELOG.md b/CHANGELOG.md index a1d2a46..444065f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **ssh: `sudo` almost always failed with "sudo password required (user declined or timed out)" (v6 / 1.1.0).** Three independent causes, all fixed: + - **`sudo -n` is now always tried first**, even when the alias is configured `sudo: "prompt"`. Previously `prompt` went straight to `sudo -S` and *unconditionally* elicited a password — so a host where the login user has a `NOPASSWD:` rule still opened an Agent Inbox prompt, and any unattended run (scheduled agent, no human watching the Inbox) hit the client's 300 s elicitation deadline and got back `cancel`, i.e. the reported error. A password is now requested only after `sudo -n` has proved the host actually demands one; `sudo` never runs the command when it refuses for a missing password, so the probe is side-effect free. + - **A leading `sudo` in `command` is stripped and turned into `sudo=true`.** Agents routinely write `exec(command="sudo systemctl restart x")`. With `sudo=false` that ran a bare `sudo` on a tty-less channel ("a terminal is required"); with `sudo=true` it nested `sudo -S … sudo …`, where the *inner* sudo prompts on a tty it does not have. The prefix (with `-u USER`, `-n`, `-S`, `-E`, `-H`, `-i`, `-k`, `-p PROMPT`, `--` and friends) is now peeled off and expressed as `sudo=true` + `sudo_user`; an unrecognised flag leaves the command untouched rather than mangling it. `sudo_user` alone also implies `sudo=true`. + - **Under sudo the command now runs as `sh -c ''`**, so pipes and redirections are privileged too — `sudo tee`/`sudo … > file` behaves as written instead of running the redirection as the login user. + - Errors are now actionable: sudo-disabled, `nopasswd`-but-password-required, password-rejected (the cached password is discarded so the next call re-prompts) and no-password-available each say what to change. A command that failed on a *nested* sudo we could not peel off (e.g. `cd /x && sudo …`) comes back with a `hint` telling the agent to use `sudo=true` instead. + +### Added + +- **ssh: `SSH_MCP_SUDO_PASSWORD`** — optional, secret, non-interactive sudo password for unattended runs where nobody can answer the Inbox prompt (mirrors the existing `SSH_MCP_KEY_PASSPHRASE`). Consulted only after `sudo -n` has shown the host requires a password, so on a NOPASSWD host it is never fed to sudo's stdin — where it would have landed in the command's own stdin instead. + ### Changed +- **ssh: version realigned to 6 / 1.1.0 in both manifests.** `fragment.json` carried `5` / `1.0.4` while `connector.json` carried `2` / `1.0.1`. Skald takes the installed version from the manifest and the feed version from the index, so the index was permanently 3 ahead: the connector advertised an update forever and re-installed to `2` every time. +- **ssh: stdout and stderr are drained together, and `timeout_sec` is a real wall-clock deadline.** Both streams share one SSH channel window, so reading stdout to EOF first stalled as soon as a chatty stderr filled that window (a >2 MB stderr deadlocked the call until the timeout). Command stdin is now always closed after the optional sudo password, so a remote command that reads stdin sees EOF instead of hanging. +- **ssh: messages arriving while the server waits for an elicitation reply are queued, not dropped.** A concurrent `tools/call` used to be discarded silently, leaving the client to time out on a request the server had thrown away. +- **ssh: the client's `elicitation` capability is recorded at `initialize`.** Without it the server no longer blocks on a prompt nobody can answer — it fails immediately and says so. +- **ssh: tool descriptions rewritten** to tell the agent explicitly not to put `sudo` in `command`, and to describe `prompt` as "tries `sudo -n` first, prompts only if the host demands it". Integer arguments (`depth`, `max_results`, `context_lines`, `timeout_sec`) now tolerate `null` and numeric strings instead of raising an internal error. + +- **Docs: `CONNECTOR_MANIFEST_GUIDE.md` is now the single source of truth, rewritten against the client implementation.** `~/projects/skald-circle` dropped its copy and references this file instead, so the guide was re-verified line by line against `src/frontend/api/{marketplace,mcp}.rs` and `crates/skald-core/src/mcp/{mod,install,verify,oauth}.rs`. What was wrong and is now fixed: + - **It told the author to hand-write `connectors.json` with `sha256sum`** (§1a, §8) and never mentioned `fragment.json`, `index.json` or `scripts/compile.py` — i.e. it described a workflow this repo abandoned. The build pipeline is now §1 in full, and `connectors.json` is documented as generated. + - **`tools[]` was said to go in `fragment.json`.** The client parses no `tools` field on the index entry — a block placed only there is inert. It must be in `connector.json` (this is why `google-trends`' four display names never reached the UI). + - **`auth` was said to resolve "the same way whether it appears in the index entry or the manifest".** The index's `auth` is never parsed; only the manifest's is, with `requires` as the sole coarse fallback. + - **`auth.type: "password"` was undocumented** while `email` ships it — the client recognizes only `none`/`api_key`/`oauth2`/`qr`/`ssh_key` and silently normalizes everything else to `none`. + - **Placeholder substitution was overstated.** It happens in exactly two places (`mcp_config.url` and `verify.command`) with *different* miss behaviour (literal token + api_key fallback vs empty string), and never in `mcp_config.env` — so an `env[].name` must be the real environment variable name. + - **`verify` was described as an `api_key`/`none` feature with a 15 s default.** Any auth type may use it; the script must be a shipped file whose basename appears in the command; the runtime applies a fixed 20 s and ignores `timeout_secs`. + - **`deliver: {as: "file"}` was labelled "legacy"** — it is rejected at activation, unimplemented. + - Added: the "who reads what" authority table (index vs manifest), the flat-folder constraint (`compile.py` does not recurse, so a subdirectory's files are silently unshipped), the version-desync trap (manifest wins, index-lower kills updates forever), exact dependency-install commands and paths, host assets not copied into containers, and the client's hard limits (8 MiB/file, path-safety rejects, no-`files[]` refusal). Also fixed an unbalanced code fence that swallowed the end of the file. +- **Docs: `docs/connector.manifest_guide.md` retired** to a pointer at the root guide — the duplicate copy is what allowed the drift. +- **Docs: `CLAUDE.md` audited against the repo.** Added § The consuming project (the client is the separate Rust repo at `~/projects/skald-circle`, with a map of the files that implement each part of the format) and corrected the same inaccuracies listed above where they also appeared here: placeholder scope and miss behaviour, verify workdir (`./connectors//` on the host, not `./scripts//`), verify timeout, `tools[]` location, `deliver: file`, `auth.type: "password"`, plus new invariants for version desync and flat folders. +- **`scripts/compile.py`: docstring corrected** — it claimed `connector.json` was auto-excluded from `files[]`, the opposite of what `EXCLUDE_FILES` does. + - **Docs: `SKALD.md` merged into `CLAUDE.md` and removed.** The two files had drifted apart — `CLAUDE.md` still described a hand-maintained `connectors.json` (pre-`compile.py`), an rsync deploy, 5 connectors, and a `files[]` that excluded `connector.json`. `CLAUDE.md` is now the single authoritative spec, carrying every `SKALD.md` section (full schemas for `connectors.json` / `fragment.json` / `connector.json`, reserved enums, `auth`/`deliver`/`env`/`verify` fields, placeholder syntax, icon conventions, file integrity, local workflow, deploy) corrected against the actual repo state: 18 connectors, the `compile.py` pipeline, and `connector.json` included in the hashed `files[]`. `CLAUDE.md` now also names `CONNECTOR_MANIFEST_GUIDE.md` (repo root) as the authoritative connector-authoring spec. ## 2026-08-24 diff --git a/CLAUDE.md b/CLAUDE.md index 7d60689..21d51b3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,21 +21,39 @@ The only tooling is `scripts/compile.py`, which regenerates the index. `main` is the **release** branch — only production-ready code lands here. Development/alpha versions will live on separate branches in the future. +### The consuming project + +The application that consumes this marketplace — the code that installs, verifies, and runs these +connectors — lives in a **separate repository at `~/projects/skald-circle`** (Rust workspace: `crates/`, +`Cargo.toml`, `blueprint/`, `agents/`). It is the client for everything documented here: it fetches +`connectors.json`, checks each file against its pinned SHA-256, resolves `auth` / `env` / `verify`, and +launches the MCP servers. The parts worth reading when a manifest question comes up: + +| Question | File in `~/projects/skald-circle` | +|----------|-----------------------------------| +| How the feed is parsed, hashed, installed | `src/frontend/api/marketplace.rs` | +| Activation, the env form, OAuth/QR handoff | `src/frontend/api/mcp.rs` | +| Server spec, URL placeholders, transports | `crates/skald-core/src/mcp/mod.rs` | +| Folder layout, container copy, deps install | `crates/skald-core/src/mcp/install.rs` | +| Verify runner + `{ENV:}`/`{SECRET:}` engine | `crates/skald-core/src/mcp/verify.rs` | + +It **no longer keeps a copy** of the spec: it references +[CONNECTOR_MANIFEST_GUIDE.md](CONNECTOR_MANIFEST_GUIDE.md) in this repo, which is the single source of +truth for the connector format. Edit the format here and nowhere else. + ### The spec The marketplace spec ships **inside this project**: [CONNECTOR_MANIFEST_GUIDE.md](CONNECTOR_MANIFEST_GUIDE.md) at the repo root is the authoritative, step-by-step specification for producing a correct connector — -the two documents, the MCP-over-stdio server contract, friendly tool names, placement & risk vocabulary, -the `auth.type` variants (`api_key` / `oauth2` / `qr`), dependency installation, verify-before-save, -versioning, and the new-connector checklist. Give this file to any agent that generates new connectors, -and update it whenever the connector format changes. +the folder layout, the three source documents + the compiler, which document the client reads each field +from, the MCP-over-stdio server contract, friendly tool names, placement & risk vocabulary, the +`auth.type` variants, the `env[]` form and placeholder engines, dependency installation, +verify-before-save, versioning, the limits the client enforces, and the new-connector checklist. Give +this file to any agent that generates new connectors, and update it whenever the connector format +changes — it is the **only** copy, referenced by `~/projects/skald-circle` rather than duplicated there. -Consumers outside this repo (e.g. `~/projects/skald-circle`) read that same file as the format reference. - -[docs/connector.manifest_guide.md](docs/connector.manifest_guide.md) is an earlier copy of the same guide; -it is the only one that documents the `fragment.json` + `scripts/compile.py` build pipeline, which -`CONNECTOR_MANIFEST_GUIDE.md` does not yet cover. Until the root guide absorbs that section, the compile -workflow is described below in § Architecture and § Local workflow. +[docs/connector.manifest_guide.md](docs/connector.manifest_guide.md) is a retired copy, now a pointer to +the root guide. Do not edit it. ## Architecture @@ -157,7 +175,7 @@ connectors/ | `version` | ✅ | Per-connector integer, +1 on every file change | | `version_string` | ✅ | Semver (display only) | | `version_release_date` | ✅ | ISO 8601 date (display only) | -| `tools` | ✅ | Array of `{name, display_name}` for friendly UI names | +| `tools` | optional | Array of `{name, display_name}` for friendly UI names. **Documentary here** — Skald reads `tools[]` from `connector.json` (see § Friendly tool names) | | `auth` | optional | Authentication configuration (if other than `"none"`) | The `files[]` array is added **automatically** by `compile.py` from the files present in the folder — it @@ -294,11 +312,15 @@ Structure describing how the connector handles authentication: {"type": "oauth2", "provider": "google", "scopes": ["…"], "deliver": {"as": "env", "format": "google_authorized_user", "env": "GMAIL_CREDS_JSON"}} -// OAuth2 with file-based deliver (legacy) +// OAuth2 with file-based deliver — ❌ NOT IMPLEMENTED: Skald rejects `as: "file"` at +// activation with an explicit error. Never ship it. {"type": "oauth2", "provider": "google", "scopes": ["…"], "deliver": {"as": "file", "format": "google_authorized_user", "path": "{secrets}/gmail_creds.json"}} -// Password / app-password provided via environment variables +// Password / app-password provided via environment variables — ⚠️ `password` is NOT one of +// the values Skald recognizes (`none`/`api_key`/`oauth2`/`qr`/`ssh_key`); it normalizes to +// `none`. `email` works only because its credentials travel through `env[]`. Prefer +// `api_key` or `none` for new connectors. {"type": "password", "delivery": "env"} // QR-code pairing at runtime (WhatsApp) @@ -314,9 +336,9 @@ Declares **how** Skald delivers the obtained OAuth credential to the MCP server | Field | Required | Description | |-------|----------|-------------| -| `as` | ✅ | `"file"` (on disk) or `"env"` (environment variable) | +| `as` | ✅ | `"env"` (environment variable). `"file"` parses but is **rejected at activation** — unimplemented | | `format` | ✅ | Name of the serialization — e.g. `"google_authorized_user"` (Google JSON that `from_authorized_user_file` reads), `"refresh_token"`, `"access_token"` | -| `path` | `as=file` only | Path with the `{secrets}` placeholder (Skald expands it to a per-user dir at runtime). MUST match the path in `mcp_config.env`. | +| `path` | `as=file` only | Unused while file delivery is unimplemented | | `env` | `as=env` only | Name of the environment variable into which Skald injects the entire authorized_user JSON. **Must not be declared in `mcp_config.env`** — Skald injects it at runtime. | The feed NEVER contains `client_id`, `client_secret`, the endpoint URL, or `redirect_uri`. These are @@ -364,8 +386,7 @@ The `email` connector is the reference example. `tavily`, `gmaps`, and `linkedin ## Placeholder syntax (unified) -Every value skald must fill at runtime with user-provided data uses **one of two tokens**, wherever it -appears (URL, `mcp_config.env`, `verify.command`): +Every value skald must fill at runtime with user-provided data uses **one of two tokens**: | Token | Meaning | Example | |-------|---------|---------| @@ -373,17 +394,27 @@ appears (URL, `mcp_config.env`, `verify.command`): | `{SECRET:NAME}` | Sensitive variable (password, API key, token) | `{SECRET:EMAIL_PASSWORD}` | `NAME` is the `name` field declared in the `env[]` array. skald collects the values via a form (masking -`{SECRET:}` fields), injects them as environment into the MCP server / verify process, and substitutes -the tokens in the manifest. +`{SECRET:}` fields) and injects them as environment into the MCP server / verify process. + +**Substitution happens in exactly two places** — there is no general template engine: + +| Where | Engine | Unknown `NAME` resolves to | +|-------|--------|----------------------------| +| `mcp_config.url` (remote connectors) | `skald-core/src/mcp/mod.rs` | `{SECRET:x}` falls back to the connector's api_key; otherwise the token is **left in the URL literally** | +| `verify.command` | `skald-core/src/mcp/verify.rs` | the **empty string** | + +⚠️ **`mcp_config.env` is NOT substituted.** What reaches a local server's environment is the form's +`env[]` values keyed by their `name`, verbatim — so `env[].name` must be exactly the variable name the +script reads. The `mcp_config.env` map survives only as a legacy fallback for the form schema (bare key +names); if it is ever the sole env source its `{ENV:…}` strings are injected literally. `email`, `ssh` +and `linkedin` still carry such a map — harmless today, but do not rely on it to rename a variable. Rules: -- Unrecognized tokens (`{secrets}/…`, legacy `{key}`, `{env:NAME}`) are **deprecated**: skald does not - substitute them and the manifest must be updated. +- Unrecognized tokens (`{secrets}/…`, `{env:NAME}`) are **deprecated**: skald does not substitute them + and the manifest must be updated. Legacy `{key}` still resolves to the api_key in a URL. - `{SECRET:}` is reserved for the primary key when `auth.type = "api_key"` (e.g. Tavily: `?tavilyApiKey={SECRET:tavilyApiKey}`). skald also treats that value as the API key for bearer/header - routing. -- A `{ENV:X}` or `{SECRET:X}` token whose `X` is not in the manifest's `env[]` is substituted with an - empty string (the host cannot guess it). + routing, and stops sending it as a bearer header once it has been spent on the URL. ### Deprecations @@ -408,7 +439,7 @@ the activation, to confirm the credentials just entered actually work. | Field | Required | Description | |-------|----------|-------------| | `command` | ✅ | Shell command. Runs in the same sandbox as the server: container `skald-{userid}` for `mcp_local` user, host for `mcp_remote` global. The declared env/secrets are injected | -| `timeout_secs` | optional | Default 15. skald kills the process at expiry | +| `timeout_secs` | optional | Declared for the record, but **not yet plumbed through**: the runtime applies a fixed 20 s to every verify (`VERIFY_TIMEOUT_SECS` in `src/frontend/api/mcp.rs`). Keep the probe well under that | ### Output convention @@ -431,10 +462,12 @@ fails). **Never print credentials** in `message`/`details`. ### Where to put the script If `command` references a file (e.g. `verify.py`), the file must be saved in the connector folder -(`/verify.py`); `compile.py` then picks it up into `files[]` with its SHA-256 and size. skald -downloads it, verifies the hash against the index, and makes it available at the same path as the main -server (container for `mcp_local`, `./scripts//` on the host for `mcp_remote`). A `verify` command -may also be fully inline (see `firecrawl`, which uses a `node -e "…"` one-liner). +(`/verify.py`); `compile.py` then picks it up into `files[]` with its SHA-256 and size. skald finds +the script by matching a **basename from `files[]`** against the command string, downloads it, verifies +the hash against the index, and runs it in the connector's own directory — inside the container +`skald-{userid}` (`~/.skald/mcp//`) for a per-user connector, on the host in `./connectors//` +for a global one. A `verify` command may also be fully inline (see `firecrawl`, which uses a +`node -e "…"` one-liner) — with no basename match, nothing extra is fetched. ### Without `verify` @@ -448,15 +481,20 @@ Every MCP tool must expose a friendly name for the Skald UI. Two ways, in order 1. **Via the MCP script (preferred)** — add `"title": "Friendly Name"` to each tool definition returned by `tools/list`. Works for all local scripts (Python/Node) that we control. -2. **Via the manifest (fallback)** — add `"tools": [{"name": "…", "display_name": "…"}]` in - `fragment.json` **and** in `connector.json`. Used only for remote connectors or external packages - (e.g. `npx -y firecrawl-mcp`). +2. **Via the manifest (fallback)** — add `"tools": [{"name": "…", "display_name": "…"}]` to + **`connector.json`**. Used only for remote connectors or external packages (e.g. + `npx -y firecrawl-mcp`). **Resolution order** used by Skald: 1. `tools[].display_name` from the manifest 2. `title` from the MCP server's `tools/list` 3. Automatic prettify of the raw name (`send_message` → "Send Message") +⚠️ **Skald reads `tools[]` from `connector.json` only** — the index entry carries no `tools` field in +the client's parser, so a block that lives only in `fragment.json` is inert (that is the state of +`google-trends` today, which is harmless because its script also sets `title`). Mirroring the block into +`fragment.json` is optional and purely documentary. + As of 2026-07-21 all marketplace connectors carry `title` in the script or `tools[]` in the manifest. ## MCP server conventions @@ -486,9 +524,14 @@ Connector-specific notes: - **Tavily** — `mcp_remote`/`global`; API key declared as an `env[]` secret and templated into the URL as `{SECRET:tavilyApiKey}`. - **SSH** — `mcp_local`/`user`; stores aliases in `~/.ssh_aliases.json` (auto-managed, 0600). Auth - per-alias: key/agent (default) or elicited password. Sudo via `nopasswd` or elicited password - (`sudo -S`). No setup-time credentials: `auth.type: "none"` with no `verify`. All `SSH_MCP_*` env vars - are optional with defaults. + per-alias: key/agent (default) or elicited password. Sudo always probes `sudo -n` first (free on a + NOPASSWD host, and side-effect free elsewhere since sudo refuses before running the command), then + falls back to `sudo -S` with a password from `SSH_MCP_SUDO_PASSWORD` or MCP elicitation; a leading + `sudo` inside `command` is stripped and turned into `sudo=true`, and under sudo the command runs as + `sh -c '…'` so pipes/redirections are privileged too. Elicitation needs a human in the Agent Inbox + within 300 s (`ELICITATION_DEADLINE` in `skald-core/src/elicitation/mod.rs`) — unattended runs must + rely on NOPASSWD or `SSH_MCP_SUDO_PASSWORD`. No setup-time credentials: `auth.type: "none"` with no + `verify`. All `SSH_MCP_*` env vars are optional with defaults. ## Current connectors @@ -539,9 +582,15 @@ A connector without `verify` is activated without any test — see § Without ve - **Never edit `connectors.json` by hand.** It is generated. Edit `fragment.json` / `index.json` / the connector files, then run `python3 scripts/compile.py`. - **Bump `version` (+1) and `version_string` on every file change** to a connector, in both - `fragment.json` and `connector.json`, and keep `version_release_date` current. + `fragment.json` and `connector.json`, and keep `version_release_date` current. When the two disagree + **the manifest wins** and Skald only logs a `marketplace feed version desync` warning — and if the + index carries the *lower* number, the strict `feed > installed` comparison can never fire again and + the connector silently stops offering updates. `compile.py` does not check this. - **Keep `id`, `name`, `type`, `scope`, `tags`, `requires`, and `auth` consistent** between - `fragment.json` and `connector.json`. + `fragment.json` and `connector.json`. Only the manifest's `auth` is parsed by Skald; the index's is + documentary, with `requires` as the sole coarse fallback (`OAUTH` → oauth, `API_KEY` → api_key). +- **Keep the connector folder flat.** `compile.py` scans only the folder's top level, so any file in a + subdirectory is silently absent from `files[]` — never downloaded, never installed. - **Adding a `verify` script means shipping the file in the connector folder** and recompiling — skald refuses to run a script whose SHA-256 is not pinned in the index. - **Always run `python3 scripts/compile.py` before deploying**; a stale hash breaks integrity diff --git a/CONNECTOR_MANIFEST_GUIDE.md b/CONNECTOR_MANIFEST_GUIDE.md index 0dbcdc8..19dabc2 100644 --- a/CONNECTOR_MANIFEST_GUIDE.md +++ b/CONNECTOR_MANIFEST_GUIDE.md @@ -1,156 +1,253 @@ # Skald Connector Authoring Guide -Instructions for generating a **correct connector** for the Skald marketplace -(`https://connectors.skaldagent.net`). Give this file to the agent that produces -new connectors. +**This file is the single source of truth for the connector format.** It lives in the +marketplace repo (`connectors.skaldagent.net`) and is referenced — not copied — by the +Skald application repo (`~/projects/skald-circle`). Change the format here, and here only. -A connector is a folder served by the marketplace. Skald installs it, verifies -every file against a SHA-256 pinned in the index, then either runs it on the host -(global connector) or copies it into the user's container and runs it there -(per-user connector, blueprint §6/§7). +Give this file to any agent that produces new connectors. + +A connector is a folder served by the marketplace. Skald installs it, verifies every file +against a SHA-256 pinned in the index, then either runs it on the host (global connector) +or copies it into the user's container and runs it there (per-user connector, blueprint +§6/§7). + +Everything below marked **"the client enforces"** was checked against the implementation in +`skald-circle` (`src/frontend/api/marketplace.rs`, `src/frontend/api/mcp.rs`, +`crates/skald-core/src/mcp/{mod,install,verify,oauth}.rs`). --- -## 1. The two documents +## 0. Folder layout -### 1a. The root index — `connectors.json` +One folder per connector, named exactly like its `id`, **flat**: -One array of entries, each pointing at a connector folder. **The index is the -signable root: it is the only place that lists a connector's files and their -SHA-256 digests.** Skald refuses any file whose bytes do not match. +``` +connectors/myconn/ +├── fragment.json ← index entry (compiler input, never served as-is) +├── connector.json ← the manifest Skald reads +├── server.py ← the entry file (or index.js…) +├── requirements.txt ← or package.json +├── verify.py ← optional +├── icon_sm.svg +└── icon_lg.svg +``` + +**Keep the folder flat.** The client accepts a relative sub-path (`pkg/server.py`) but +`scripts/compile.py` only scans the folder's **top level**: files in a subdirectory are +silently left out of `files[]`, never downloaded, and the connector breaks at runtime with +no error anywhere. If a tree is genuinely needed, teach the compiler to recurse first. + +--- + +## 1. The three source documents (+ one compiler) + +You maintain three files by hand. A fourth, `connectors.json`, is **generated**. + +### 1a. `connectors/index.json` — the order + +A flat JSON array of folder ids, in display order: + +```json +["gmail", "gcal", "myconn"] +``` + +This is the compiler's input list. A connector missing from it does not exist. + +### 1b. `connectors//fragment.json` — the index entry + +The connector's entry in the compiled index, **with every field except `files[]`** (the +compiler adds that one). ```jsonc { - "version": 1, - "connectors": [ - { - "id": "whatsapp", // unique slug = folder name - "name": "WhatsApp", - "version": 1, // INTEGER build number — the update key (§7) - "version_string": "2.0.1", // semver, display only - "version_release_date": "2026-07-19", // ISO date, display only - "type": "mcp_local", // mcp_local | mcp_remote (see §3) - "scope": "user", // user | global (see §3) - "icon_small": "whatsapp/icon_sm.svg", - "icon_large": "whatsapp/icon_lg.svg", - "user_description": "Send and read WhatsApp messages from your linked account.", - "requires": ["NODE"], // human hint: NODE | PYTHON | OAUTH | API_KEY - "tags": ["messaging", "mcp", "local", "whatsapp", "qr"], - "auth": { "type": "qr" }, // may be repeated here and in the manifest - "folder": "whatsapp", // defaults to id - "files": [ - { "path": "index.js", "sha256": "…", "size": 21258 }, - { "path": "package.json", "sha256": "…", "size": 302 }, - { "path": "connector.json", "sha256": "…", "size": 620 }, - { "path": "icon_sm.svg", "sha256": "…", "size": 306 }, - { "path": "icon_lg.svg", "sha256": "…", "size": 308 } - ] - } - ] + "id": "whatsapp", // unique slug = folder name + "name": "WhatsApp", + "version": 8, // INTEGER build number — the update key (§7) + "version_string": "2.2.0", // semver, display only + "version_release_date": "2026-08-23", // ISO date, display only + "type": "mcp_local", // mcp_local | mcp_remote (§3) + "scope": "user", // user | global (§3) + "icon_small": "whatsapp/icon_sm.png", // relative to the FEED ROOT, not the folder + "icon_large": "whatsapp/icon_lg.png", + "user_description": "Send and read WhatsApp messages from your linked account.", + "requires": ["NODE"], // API_KEY | ENV | NODE | PYTHON | OAUTH | DOCKER + "tags": ["messaging", "mcp", "local", "whatsapp", "qr"], + "folder": "whatsapp", // defaults to id + "auth": { "type": "qr" }, // informational here — see the table in §1e + "tools": [ … ] // informational here — see §2a } ``` -**Rules** +Note the **two icon vocabularies**: `icon_small` / `icon_large` in the index are relative to +the feed root (`whatsapp/icon_sm.png`), while `files[]` and `connector.json` name them from +inside the folder (`icon_sm.png`). The client reconciles the two, but only records an icon +whose file was actually installed — so **icons must be shipped in the folder**, never hot- +linked. -- `files[].path` is relative to the connector folder. List **every** file the - connector ships (server code, `package.json`/`requirements.txt`, icons, and the - `connector.json` itself). A missing or mismatched digest fails the install. -- Compute `sha256` over the exact bytes served: `sha256sum `. -- Do **not** list `node_modules/` or any generated deps — those are installed on - the box, not shipped (see §5). -- `size` is optional but recommended. +### 1c. `connectors//connector.json` — the manifest -### 1b. The per-connector manifest — `/connector.json` - -The richer document. Fetched per connector and mapped into Skald's catalog. +The richer document, fetched per connector and mapped into Skald's catalog. ```jsonc { "id": "whatsapp", "name": "WhatsApp", - "version": 1, // INTEGER build number — the update key (§7) - "version_string": "2.0.1", // semver, display only - "version_release_date": "2026-07-19", // ISO date, display only + "version": 8, // must match fragment.json (§7) + "version_string": "2.2.0", + "version_release_date": "2026-08-23", "type": "mcp_local", "scope": "user", - "auth": { "type": "qr" }, // none | api_key | oauth2 | qr (see §4) + "requires": ["NODE"], + "tags": ["messaging", "mcp", "local", "whatsapp", "qr"], + "auth": { "type": "qr" }, // none | api_key | oauth2 | qr | ssh_key (§4) + "launch_command": "node index.js", // human-readable; mcp_config is what runs + "transport": "stdio", // stdio | streamable-http (may also sit in mcp_config) "mcp_config": { "command": "node", // interpreter (local) … - "args": ["index.js"], // … args[0] MUST name the entry file - "transport": "stdio" // stdio (local) | streamable-http (remote) + "args": ["index.js"] // … args[0] MUST name the entry file }, + "dependencies": ["@whiskeysockets/baileys@7.0.0-rc.14"], // display only (§5) + "setup_instructions": ["Scan the QR code with WhatsApp → Linked devices"], "docs": [{ "lang": "en", "description": "Human blurb shown in the UI.", "llm_short_description": "One line the model reads to decide whether to use this connector." }], - "env": [], // form fields the user fills (see §4b) - "tools": [ // OPTIONAL — friendly UI names per tool (§2a) - { "name": "send_message", "display_name": "Send Message" } - ], - "homepage": "https://…", - "icon_small": "icon_sm.svg", // relative to the folder here - "icon_large": "icon_lg.svg", - "tags": ["messaging", "mcp", "local", "whatsapp", "qr"] + "env": [], // form fields the user fills (§4b) + "verify": { "command": "python3 verify.py", "timeout_secs": 20 }, // optional (§6) + "tools": [{ "name": "send_message", "display_name": "Send Message" }], // optional (§2a) + "homepage": "https://web.whatsapp.com", + "icon_small": "icon_sm.png", // relative to the FOLDER here + "icon_large": "icon_lg.png" } ``` -**`mcp_config.args[0]` is load-bearing for a local connector:** it is how Skald -learns which file to run. At activation Skald rewrites it to the file's path -inside the user's container (`/root/.skald/mcp//`), so keep it a -plain relative filename (`index.js`, `server.py`, `pkg/server.py`). +**`mcp_config.args[0]` is load-bearing for a local connector:** it is how Skald learns which +file to run — an install fails outright without it. At activation Skald rewrites it to the +file's path inside the user's container (`/root/.skald/mcp//`), so keep it a +plain relative filename (`index.js`, `server.py`). + +**`transport` may sit either at the top level or inside `mcp_config`** — the client reads +`mcp_config.transport` first, then the top-level one, then infers (`remote` → `http`, +otherwise `stdio`). Both spellings are in use in this repo; pick one per connector and keep +it consistent between the two files. `streamable-http` and `http` both normalise to HTTP; +anything unrecognised silently becomes stdio, which for a remote connector means trying to +spawn a command that does not exist. + +**`llm_short_description` is model-facing:** it is the one-liner injected into the LLM's +system prompt so the model knows what this connector does. Keep it short and functional +("Weather — current conditions, 16-day forecast, and AQI data for any location"), **not** a +list of tools (the model discovers those after `activate_tools`). + +### 1d. `connectors/connectors.json` — the compiled index (generated) + +**Never edit this file by hand.** `scripts/compile.py` reads `index.json`, loads each +`fragment.json`, scans the folder's real files, computes their SHA-256 + size, and writes the +result: + +```bash +python3 scripts/compile.py # regenerate +python3 scripts/compile.py --verify # fail if the committed index is stale +``` + +The compiler excludes `fragment.json`, `connectors.json`, `index.json`, `compile.sh`, +`compile.py`, `update_hashes.py`, `.DS_Store` and every subdirectory. **Everything else is +hashed, including `connector.json`.** + +```jsonc +"files": [ + { "path": "index.js", "sha256": "…", "size": 21258 }, + { "path": "package.json", "sha256": "…", "size": 302 }, + { "path": "connector.json", "sha256": "…", "size": 620 }, + { "path": "icon_sm.png", "sha256": "…", "size": 306 } +] +``` + +- `files[].path` is relative to the connector folder. +- Digests and sizes are computed by the compiler. **Never write them by hand.** +- The index is the **signable root**: it is the one document that names a connector's files + and their digests. The client refuses any file whose bytes do not match, all-or-nothing — + a mismatch leaves nothing on disk. +- Do **not** ship `node_modules/` or vendored wheels (§5). + +### 1e. Who reads what — the authority table + +The client hydrates a card from **two** documents, and they are not interchangeable. Getting +this wrong is the most common way a correct-looking connector misbehaves. + +| Field | Read from | Notes | +| --- | --- | --- | +| `id`, `folder`, `tags` | **index** (`fragment.json`) | `folder` defaults to `id` | +| `name` | index, falling back to manifest | | +| `user_description` | **index**, falling back to `docs[0].description` | the human blurb | +| `icon_small`, `icon_large` | **index** | feed-root-relative; the file must be in `files[]` | +| `files[]` | **index** (manifest's is a legacy fallback) | the trust root | +| `requires` | manifest, falling back to index | | +| `type`, `scope` | manifest, falling back to index | keep them identical | +| `version` trio | **manifest wins**, index is the fallback | a mismatch is logged as a desync warning (§7) | +| `auth` | **manifest only** | the index's `auth` is *never parsed*; only `requires` is used as a coarse fallback | +| `tools[]` | **manifest only** | a `tools[]` that lives only in `fragment.json` does nothing (§2a) | +| `env[]`, `verify`, `mcp_config`, `docs`, `dependencies`, `setup_instructions`, `homepage` | **manifest only** | | + +So: `auth` and `tools[]` in `fragment.json` are documentation for the catalog page and for +whoever reads the index — harmless, worth keeping in sync, but **the manifest is what runs**. --- ## 2. Server contract (MCP over stdio) -A **local** connector is a program speaking JSON-RPC 2.0 over stdin/stdout. It -MUST handle: +A **local** connector is a program speaking JSON-RPC 2.0 over stdin/stdout. It MUST handle: -- `initialize` → `{ protocolVersion, capabilities: { tools: {} }, serverInfo }` -- `notifications/initialized` → no response -- `tools/list` → `{ tools: [ { name, description, inputSchema } ] }` +- `initialize` → `{ protocolVersion: "2024-11-05", capabilities: { tools: {} }, serverInfo }` +- `notifications/initialized` → **no response** +- `ping` → `{}` +- `tools/list` → `{ "tools": [ { name, description, inputSchema, title? } ] }` — an + **object**, not a bare array - `tools/call` → `{ content: [ { type: "text", text } ], isError? }` -**stdout is reserved for JSON-RPC only.** Send all logs/diagnostics to **stderr**. -Anything a library prints to stdout (a logger, a banner) corrupts the protocol — -silence it (e.g. Baileys/pino → a silent logger; Python → `print(…, file=sys.stderr)`). +**Any message without an `id` is a notification and must produce no output at all.** +Answering one desynchronises a strict client. -A **remote** connector is an HTTP MCP endpoint (`mcp_config.url` + -`transport: "streamable-http"`); no code runs on the box. +**stdout is reserved for JSON-RPC only.** Send all logs/diagnostics to **stderr**. Anything a +library prints to stdout (a logger, a banner) corrupts the protocol — silence it (Baileys/pino +→ a silent logger; Python → `print(…, file=sys.stderr)`). Guard stdout writes with a lock if +the server has background threads. + +Report tool failures as a normal result carrying `isError: true` with a readable message — +not as a JSON-RPC error, and not as a "successful" result with an `{"status":"error"}` body +the model cannot distinguish from data. + +A **remote** connector is an HTTP MCP endpoint (`mcp_config.url` + `transport: +"streamable-http"`); no code runs on the box. ### 2a. Friendly tool names (`tools[]`) — optional -Raw MCP tool names are ugly in the chat UI (`search_files`, `send_message`). The -optional top-level `tools[]` block gives each one a human title shown as the tool -card's heading: +Raw MCP tool names are ugly in the chat UI (`search_files`, `send_message`). Two ways to fix +that, in order of preference: + +1. **`title` in your `tools/list` entries** (preferred for servers we control) — Skald uses it + automatically, no manifest change needed. +2. **The manifest's `tools[]` block** — for remote connectors and third-party packages + (`npx -y firecrawl-mcp`) whose `tools/list` we cannot edit: ```jsonc "tools": [ - { "name": "send_message", "display_name": "Send Message" }, - { "name": "list_chats", "display_name": "List Chats" }, - { "name": "download_media", "display_name": "Download Media" } + { "name": "send_message", "display_name": "Send Message" } ] ``` -- `name` — the **raw** tool name exactly as your server returns it from `tools/list`. +- `name` — the **raw** tool name exactly as the server returns it from `tools/list`. - `display_name` — the friendly card title (English only; not internationalized). -**Resolution order** for a tool's card title is **`tools[].display_name` → the MCP -`title` field → a prettified raw name**. So you have two ways to set a friendly -name, and can skip `tools[]` entirely: +**Resolution order:** `tools[].display_name` → the MCP `title` field → a prettified raw name +(`send_message` → "Send Message"). -1. **This block** — the authoritative override, curated in the manifest. -2. **The MCP `title` field** — if your `tools/list` entries already carry a - `title` (MCP 2025-06-18+), Skald uses it automatically; no manifest change - needed. `tools[]` wins if both are present. -3. If neither is set, Skald title-cases the raw name (`send_message` → "Send - Message"). +⚠️ **`tools[]` is read from `connector.json`.** The client parses no `tools` field on the +index entry, so a block placed only in `fragment.json` is inert. Put it in the manifest; +mirroring it into `fragment.json` is optional and purely documentary. -**Icons are per connector, not per tool.** Every tool of a connector shows that -connector's own `icon_small`; there is no per-tool icon field. Only list a tool in -`tools[]` when its prettified name isn't good enough — partial lists are fine -(unlisted tools fall through to steps 2–3). +**Icons are per connector, not per tool.** Every tool shows its connector's `icon_small`; +there is no per-tool icon. Partial `tools[]` lists are fine — unlisted tools fall through. --- @@ -158,44 +255,87 @@ connector's own `icon_small`; there is no per-tool icon field. Only list a tool | Manifest | Meaning | | --- | --- | -| `scope: "user"` | runs **once per user**, inside their container. Personal creds. | +| `scope: "user"` | runs **once per user**, inside their container. Personal creds. Stored as `per_user`. | | `scope: "global"` | runs **once for the household**, on the host. Shared, stateless. Admin enables it with a key. | -| `type: "mcp_local"` | ships code that will **execute on the box** — installing needs the admin `mcp.register_local_script` capability (RCE-bearing act, §14). | -| `type: "mcp_remote"` | just an HTTP URL; no local code. | +| `type: "mcp_local"` | ships code that will **execute on the box** — installing needs the admin `mcp.register_local_script` capability (RCE-bearing act, §14). Stored as `local_script`. | +| `type: "mcp_remote"` | just an HTTP URL; no local code. Stored as `remote`. | -Pick the narrowest: a personal messaging/email/calendar connector is -`scope: "user"`; a shared search API is `scope: "global"`. +Pick the narrowest: a personal messaging/email/calendar connector is `scope: "user"`; a shared +search API is `scope: "global"`. Both axes **fail closed** — an unreadable `scope` becomes +`per_user`, an unreadable `type` becomes `local_script`, the answer that demands *more* +authority. + +`requires[]` is a human hint rendered as tags on the catalog page: `API_KEY`, `ENV`, `NODE`, +`PYTHON`, `OAUTH`, `DOCKER`. The client only inspects it as a fallback when the manifest +declares no `auth` (`OAUTH` → oauth, `API_KEY` → api_key). `SECRETS_DIR` is **removed** — the +`secrets/` folder is no longer part of the model. --- ## 4. Authentication (`auth.type`) +### 4a. The recognized values + | `auth.type` | Flow | Ships | | --- | --- | --- | | `none` | nothing to sign in | — | | `api_key` | user pastes a key/secret into a form | an `env[]` schema (§4b) | -| `oauth2` | browser consent → paste code back | `auth.provider` + `auth.scopes` + `auth.deliver` (§4c) | +| `oauth2` | browser consent → token injected as an env var | `provider` + `scopes` + `deliver` (§4c) | | `qr` | server shows a QR, user scans with a phone | a `login_status` tool (§4d) | +| `ssh_key` | reserved for key-based remote access | — | -### 4b. `api_key` — the `env[]` schema +**Anything else normalises to `none`.** `email` declares `auth.type: "password"`, which the +client does not recognise and therefore treats as `none`; that connector works only because +its credentials arrive through `env[]` like any other form field. Prefer `api_key` or `none` +for new connectors and let `env[]` carry the credential. -Each entry drives one form field **and** is injected as an env var / URL token to -the server: +`auth.delivery` (`header` | `query` | `env`) is surfaced to the admin UI. For a remote +connector the **URL placeholder is what actually routes the key** (§4b) — the `param` name is +not read by the client. + +### 4b. `env[]` — the activation form, and what reaches the process + +Each entry drives one form field, and the collected values are injected into the server +process environment: ```jsonc "env": [{ - "name": "tavilyApiKey", - "label": "Tavily API key", - "description": "Create one at https://app.tavily.com.", + "name": "EMAIL_IMAP_HOST", // ← the ACTUAL environment variable name + "label": "IMAP host", + "description": "IMAP server hostname (e.g. imap.gmail.com).", "required": true, - "secret": true, // rendered masked, stored encrypted - "example": "tvly-xxxxxxxx" + "secret": false, // true → masked in the form, stored encrypted + "example": "imap.gmail.com", + "default": "" // non-required fields only }] ``` -The server reads each value from `process.env.` (or `os.environ`). For a -**remote** connector that wants the key in the URL, use a placeholder: -`"url": "https://mcp.example.com/?key={SECRET:tavilyApiKey}"`. +> **`name` must be exactly the environment variable your server reads.** The process receives +> the form's `name` → value map verbatim; there is no renaming layer. `mcp_config.env` is +> **not** substituted at runtime — it survives only as a legacy fallback for the form schema, +> and if it is ever the sole env source its `{ENV:…}` strings are injected *literally*. Do not +> use it to map one name onto another. + +The server reads each value from `os.environ` / `process.env`. Declare `requires: ["ENV"]` +when the connector needs user-supplied config. + +**Placeholders.** Two tokens, and exactly two places where they are substituted: + +| Token | Meaning | +| --- | --- | +| `{ENV:NAME}` | non-sensitive value (host, port, username…) | +| `{SECRET:NAME}` | sensitive value (password, API key, token) | + +| Substituted in | Engine behaviour on an unknown name | +| --- | --- | +| `mcp_config.url` (remote connectors) | `{SECRET:x}` falls back to the connector's api_key; if that is absent the token is **left in the URL literally**, so a misconfiguration is visible | +| `verify.command` (§6) | replaced with the **empty string** | + +Nowhere else — not in `mcp_config.env`, not in `args`. Legacy `{key}` still resolves to the +api_key in URLs; `{env:NAME}` and `{secrets}/…` are dead and are never substituted. + +Remote example: `"url": "https://mcp.tavily.com/mcp/?tavilyApiKey={SECRET:tavilyApiKey}"` +with a matching `env[]` entry named `tavilyApiKey`. ### 4c. `oauth2` — provider consent @@ -208,20 +348,27 @@ The server reads each value from `process.env.` (or `os.environ`). For a } ``` -The manifest names **only** the provider slug, scopes, and how the obtained token -is delivered — never client secrets or endpoint URLs (those are admin-entered, -kept off the public feed). Skald handles PKCE + code exchange and injects the -credential as the named env var. `format`: `google_authorized_user` (Google) or -`refresh_token`. Today only `as: "env"` is wired. +The manifest names **only** the provider slug, the scopes, and how the obtained token is +delivered — never client ids, client secrets, endpoint URLs or redirect URIs, which are +admin-entered and stay off the public feed. Skald runs PKCE + code exchange and injects the +credential as the named env var. -### 4d. `qr` / interactive device login — the generic contract +- `deliver.as`: **only `"env"` is implemented** — `"file"` is rejected at activation with an + explicit error rather than half-working. Do not ship it. +- `deliver.format`: `google_authorized_user` (the JSON `from_authorized_user_file` reads) or + `refresh_token`. +- `deliver.env` must **not** also appear in `mcp_config.env` — Skald injects it at runtime. +- An OAuth connector activates into a **pending** row; nothing starts until the consent + round-trip completes. The admin must have configured the provider first, or activation + fails early with a clear message. -For a connector whose credential is produced by **scanning/pairing** (WhatsApp -today), there is no code to paste. The rule: +### 4d. `qr` / interactive device login -> **Expose one extra tool, `login_status`, returning a JSON object** (as the -> `text` of a normal text result). Skald calls it directly (never the agent) and a -> login panel polls it. +For a connector whose credential is produced by **scanning/pairing** (WhatsApp today) there is +no code to paste. The contract: + +> **Expose one extra tool, `login_status`, returning a JSON object** (as the `text` of a normal +> text result). Skald calls it directly — never the agent — and a login panel polls it. ```jsonc // login_status result text (a JSON string): @@ -232,117 +379,166 @@ today), there is no code to paste. The rule: } ``` -- `activate` on a `qr` connector inserts a **pending** row and **starts the - server** (so it can produce the QR), then hands off to the login panel. -- The panel polls `POST /api/mcp/login/status`; when `state == "ready"` the - connector is marked ready and starts automatically on later logins. -- Also expose a `logout` tool (clears the session, forces a fresh QR) — the panel - calls it via `POST /api/mcp/login/reset` to re-link a different phone. -- The **credential is the on-disk session**, not a token. Persist it **inside the - connector's own directory** (e.g. `./auth/` next to the entry file). That folder - lives under the bind-mounted home, so it survives container recreates and - connector updates. Never store it under a shared/global path. - -Skald resolves `auth.type: "qr"` the same way whether it appears in the index -entry or the manifest. +- Activating a `qr` connector inserts a **pending** row and **starts the server** (so it can + produce the QR), then hands off to the login panel. +- The panel polls `POST /api/mcp/login/status`; when `state == "ready"` the connector is marked + ready and starts automatically on later logins. +- Also expose a **`logout`** tool (clears the session, forces a fresh QR) — the panel calls it + via `POST /api/mcp/login/reset` to re-link a different phone. +- The **credential is the on-disk session**, not a token. Persist it **inside the connector's + own directory** (e.g. `./auth/` next to the entry file). That folder lives under the bind- + mounted home, so it survives container recreates and connector updates. Never store it + under a shared or global path. --- ## 5. Dependencies (node & python) — how they get installed -**Do not ship `node_modules/` or vendored wheels.** Declare deps as a standard -manifest **file** and Skald installs them inside the container: +**Do not ship `node_modules/` or vendored wheels.** Declare deps as a standard manifest **file** +and Skald installs them where the server will run: - **node:** ship a `package.json` with a `dependencies` map. Skald runs - `npm ci --omit=dev` (falling back to `npm install --omit=dev`) in the connector - dir. `node_modules/` resolves automatically beside the entry file. -- **python:** ship a `requirements.txt`. Skald installs it with - `pip install --target .pydeps` and puts `.pydeps` on the server's `PYTHONPATH`. + `npm ci --omit=dev --no-audit --no-fund`, falling back to `npm install --omit=dev …` when + there is no lockfile. `node_modules/` resolves automatically beside the entry file. +- **python:** ship a `requirements.txt`. Skald runs + `python3 -m pip install --break-system-packages --target .pydeps -r requirements.txt` and + puts `.pydeps` on the server's `PYTHONPATH`. -This runs at activation **and** on every startup, guarded by a **content hash** of -the connector's source files: +A single install has a **300 s ceiling**; past that it fails rather than hanging a login. + +This runs at activation **and** on every startup, guarded by a **content hash** of the +connector's source files (`.skald-install.lock`): - first activation / a brand-new container → full install, - a connector **update** (any shipped file changed) → re-copy + re-install, - unchanged → skipped in microseconds. -So you never write install steps into the manifest — just ship the dep file, list -it in the index with its SHA-256, and set `requires: ["NODE"]` / `["PYTHON"]` as a -human hint. Pin versions in `package.json` / `requirements.txt` for reproducible -installs. Keep the dep tree lean (containers are slim; avoid native-heavy -packages where a pure alternative exists — e.g. Baileys instead of a browser). +So you never write install steps into the manifest — just ship the dep file. The manifest's +`dependencies[]` and `setup_instructions[]` are **display metadata** for the admin card; they +install nothing. Set `requires: ["NODE"]` / `["PYTHON"]` as a human hint. Pin versions for +reproducible installs, and keep the tree lean (containers are slim; prefer a pure-JS/Python +library over a native-heavy one — e.g. Baileys instead of a browser). + +**Host assets are not copied into the container.** Icons (`.svg/.png/.jpg/.webp/.gif/.ico`) and +`connector.json` stay on the host, so a server must never expect to read them at runtime. +Everything else in the folder — entry file, deps file, helper modules, `verify.py` — is copied. + +Where the files land: + +| | Host | Per-user container | +| --- | --- | --- | +| installed folder | `/connectors//` | `~/.skald/mcp//` (`/root/.skald/mcp//`) | +| python deps | `connectors//.pydeps` | `~/.skald/mcp//.pydeps` | --- -## 6. Verify-before-save (optional but recommended) +## 6. Verify-before-save (optional, strongly recommended) -Ship a `verify.py` / verify snippet and reference it: +Ship a `verify.py` / `verify.js` (or an inline snippet) and reference it: ```jsonc -"verify": { "command": "python3 verify.py", "timeout_secs": 15 } +"verify": { "command": "python3 verify.py", "timeout_secs": 20 } ``` -It runs with the collected env/secret injected and must print **one JSON object** -on stdout: `{"ok": bool, "message": string, "details"?: object}`, exit 0 on -success. Used for `api_key`/`none` connectors to test creds before activating. -(A `qr` connector needs no verify — its `login_status` is the live check.) +It runs **after** the user fills the form and **before** the activation is persisted, with the +collected env/secrets injected and `{ENV:…}` / `{SECRET:…}` substituted into the command. + +**Output contract** — one JSON object on stdout and nothing else: + +```json +{"ok": true, "message": "IMAP and SMTP authentication successful", "details": {"imap": "…"}} +{"ok": false, "message": "IMAP login failed: INVALID_CREDENTIALS"} +``` + +Exit 0 on success. If the JSON fails to parse the client falls back to the exit code (0 = ok) +and shows stderr. A timeout counts as a failure. **Never print credentials** in `message` or +`details`. + +Rules the client enforces: + +- **The script must be one of the connector's shipped files.** The client resolves it by + matching a **basename from `files[]`** against the command string — so `python3 verify.py` + works because `verify.py` is in `files[]`. A script that is not shipped is never downloaded + and activation fails; an inline command with no matching basename (firecrawl's `node -e "…"`) + is fine and runs as-is. +- **Where it runs:** inside the user's container, in the connector's directory, for a + `per_user` connector; on the host in `connectors//` for a `global` one. +- **Timeout:** declare `timeout_secs` for the record, but the runtime currently applies a fixed + **20 s** to every verify. Keep the probe well under that. +- Any `auth.type` may use verify — it is not limited to `api_key`. A `qr` connector needs none: + its `login_status` is the live check. + +**Without `verify` there is no test at all.** Activation goes straight to `ready` — including +for remote connectors, which get no handshake fallback. The manifest author decides. --- ## 7. Versioning & updates -Three fields, in **both** the index entry and the `connector.json`, kept identical: +Three fields, in **both** `fragment.json` and `connector.json`, kept identical: | field | type | role | | --- | --- | --- | -| `version` | **integer** | monotonic build number, **per connector** — the machine comparison key | +| `version` | **integer** | monotonic build number, per connector — the machine comparison key | | `version_string` | string (semver) | display only | | `version_release_date` | ISO date `YYYY-MM-DD` | display only | -- `version` is a **number, not a string** (`1`, not `"1"` or `"2.0.1"`). Start at - `1` for the first release under this scheme; **`+1` on every change** to any - shipped file **or to any manifest metadata** (description, icons, `version_string`). - Never reuse or decrement. -- Skald stores the installed `version` and compares it to the feed's: a strictly - greater feed `version` shows **"update available"** in the marketplace, and the - Install button becomes **Update**. Clicking it re-downloads the files and rewrites - the catalog row. -- **The integer is the *only* "is there an update?" signal** — it is compared - strictly (`feed > installed`). `version_string` (semver), icons and - `llm_short_description` are **never** compared, so a change to any of them that - does not also bump the integer is **invisible**: no "update available" badge - appears. This is the common trap — a "content-only" edit (e.g. a better - `llm_short_description`) that forgets the integer. +- `version` is a **number, not a string** (`8`, not `"8"` or `"2.2.0"`). **`+1` on every + change** to any shipped file **or** to any manifest metadata (description, icons, + `version_string`). Never reuse or decrement. +- **The integer is the only "is there an update?" signal**, compared strictly + (`feed > installed`). `version_string`, icons and `llm_short_description` are never + compared, so changing them without bumping the integer is **invisible** — no "update + available" badge. This is the classic trap. +- **The manifest wins over the index** when the two disagree, and the disagreement is only + visible as a log line (`marketplace feed version desync`). If the index carries the *lower* + number and the manifest the higher one, the strict comparison can never fire again and the + connector silently stops offering updates. Keep them equal — `compile.py` does not check + this for you. - **Two propagation paths, do not conflate them:** - - *Per-user code + deps* (the scripts, `package.json`/`requirements.txt`) reconcile - on a **content-hash** of the source files (§5), so new code lands at each user's - next login even without a reinstall. - - *Catalog metadata* (`llm_short_description` → the model's prompt, icons, friendly - name) is **not** in that hash — it lives in the catalog row and is rewritten only - by an explicit **reinstall/Update**. On reinstall Skald re-pulls the current feed - (never the browse cache) and pushes the new description live: enabled global - servers restart with it, and every logged-in user who activated the connector has - it restarted with the fresh `llm_short_description` — no re-login needed. -- So: to ship a new `llm_short_description`, **bump the integer** (so the admin sees - "update available") and the admin clicks **Update**. Nothing auto-propagates a - description change. -- `version_string` and `version_release_date` are display metadata only — never - compared. (Migration note: replace any legacy string `"version": "2.0.1"` with - the integer `version` + `version_string`.) + - *Per-user code + deps* reconcile on a **content hash** of the source files (§5), so new + code lands at each user's next login even without a reinstall. + - *Catalog metadata* (`llm_short_description` → the model's prompt, icons, friendly name) is + **not** in that hash. It lives in the catalog row and is rewritten only by an explicit + reinstall/Update, which re-pulls the current feed (never the 300 s browse cache) and + restarts enabled global servers and every live user's copy with the fresh description. +- So: to ship a new `llm_short_description`, **bump the integer** so the admin sees "update + available" and clicks Update. Nothing auto-propagates a description change. --- -## 8. Checklist for a new connector +## 8. Hard limits the client enforces -1. Folder `myconn/` with: entry file, `connector.json`, deps file - (`package.json`/`requirements.txt`), `icon_sm.svg`, `icon_lg.svg`, - optional `verify.*`. -2. Server speaks MCP over stdio (§2); **stdout = JSON-RPC only**. -3. `mcp_config.args[0]` names the entry file. -4. Correct `type` + `scope` (§3) and `auth.type` (§4). -5. For `qr`: implement `login_status` (+ `logout`), persist the session under the - connector dir (§4d). -6. Deps declared as a file, **not** vendored (§5). -7. Add the entry to `connectors.json` with a correct `sha256` for **every** file. -8. Bump `version`. -``` +- **Digest mismatch → refused**, all-or-nothing: nothing is written unless every file verifies. +- **8 MiB per file.** A `files[].size` above that is rejected before the download starts. +- **Path safety.** A `files[].path` or `mcp_config.args[0]` that is absolute, contains `..`, + a backslash or a colon is rejected outright. +- **A local connector with no `files[]` cannot be installed** ("refusing to install + unverifiable code"), and neither can one whose only file is `connector.json`. +- **`mcp_config.args[0]` is mandatory** for `mcp_local`. +- The feed is fetched as `GET /connectors.json` plus one `GET //connector.json` per + entry, cached 300 s for browsing; installs always refetch. A manifest that fails to load + degrades that one card to whatever the index said — it never fails the listing, which is + exactly how a broken manifest hides. + +--- + +## 9. Checklist for a new connector + +1. Create `connectors//` — flat — with: entry file, `connector.json`, `fragment.json`, + deps file (`package.json` / `requirements.txt`), `icon_sm.*`, `icon_lg.*`, optional + `verify.*`. +2. Server speaks MCP over stdio (§2); **stdout = JSON-RPC only**; notifications answered with + silence; `title` on every tool. +3. `mcp_config.args[0]` names the entry file; `transport` set consistently. +4. Correct `type` + `scope` (§3) and `auth.type` (§4); `auth`, `type`, `scope`, `tags`, + `requires` and the version trio **identical** in both documents. +5. `env[].name` = the real environment variable name (§4b). +6. For `qr`: implement `login_status` + `logout`, persist the session under the connector dir + (§4d). +7. Deps declared as a file, **not** vendored (§5). +8. `docs[0].llm_short_description` says what the connector *does*, not which tools it has. +9. Add `""` to `connectors/index.json`. +10. Bump `version` (+1), `version_string`, `version_release_date` in **both** documents. +11. Run `python3 scripts/compile.py`, then `python3 scripts/compile.py --verify`. +12. Record the change in `CHANGELOG.md` in the same commit, then commit and deploy. diff --git a/connectors/connectors.json b/connectors/connectors.json index 553254e..0963d65 100644 --- a/connectors/connectors.json +++ b/connectors/connectors.json @@ -619,14 +619,14 @@ "type": "none" }, "folder": "ssh", - "version": 5, - "version_string": "1.0.4", - "version_release_date": "2026-07-21", + "version": 6, + "version_string": "1.1.0", + "version_release_date": "2026-09-03", "files": [ { "path": "connector.json", - "sha256": "13ffbfaafab3092f5cc9e273a253f06ba50a5b89334ddd22bf1359dbd5b8feb7", - "size": 3986 + "sha256": "a8bbc923bedf484f3d3bdd24e77aeaac81209e42b17f96b4abd15ed0c8c3dbe8", + "size": 4494 }, { "path": "icon_lg.svg", @@ -645,8 +645,8 @@ }, { "path": "ssh_mcp_server.py", - "sha256": "68509094a35578ffaaf357e13b55e1bfcf397f16a3b993719c4c3b97ab2dc798", - "size": 50193 + "sha256": "ff3fb49dfd41eba6bd1cbfe5b314007876ce382a3a55fb28b96a03fa01fe292f", + "size": 60367 } ] }, diff --git a/connectors/ssh/connector.json b/connectors/ssh/connector.json index 5e561f7..394f2aa 100644 --- a/connectors/ssh/connector.json +++ b/connectors/ssh/connector.json @@ -63,6 +63,14 @@ "required": false, "secret": true, "example": "" + }, + { + "name": "SSH_MCP_SUDO_PASSWORD", + "label": "Sudo password (non-interactive override)", + "description": "Sudo password used when the remote host demands one and nobody can answer the Agent Inbox prompt (unattended/scheduled runs). Leave empty to always ask interactively. Only used after 'sudo -n' has proved the host really requires a password.", + "required": false, + "secret": true, + "example": "" } ], "setup_instructions": [ @@ -90,7 +98,8 @@ "SSH_MCP_CONNECT_TIMEOUT": "{ENV:SSH_MCP_CONNECT_TIMEOUT}", "SSH_MCP_LOGIN_PW_TTL": "{ENV:SSH_MCP_LOGIN_PW_TTL}", "SSH_MCP_SUDO_PW_TTL": "{ENV:SSH_MCP_SUDO_PW_TTL}", - "SSH_MCP_KEY_PASSPHRASE": "{SECRET:SSH_MCP_KEY_PASSPHRASE}" + "SSH_MCP_KEY_PASSPHRASE": "{SECRET:SSH_MCP_KEY_PASSPHRASE}", + "SSH_MCP_SUDO_PASSWORD": "{SECRET:SSH_MCP_SUDO_PASSWORD}" } }, "homepage": "", @@ -109,7 +118,7 @@ "auth": { "type": "none" }, - "version": 2, - "version_string": "1.0.1", - "version_release_date": "2026-07-21" + "version": 6, + "version_string": "1.1.0", + "version_release_date": "2026-09-03" } \ No newline at end of file diff --git a/connectors/ssh/fragment.json b/connectors/ssh/fragment.json index b5be722..e08d7e5 100644 --- a/connectors/ssh/fragment.json +++ b/connectors/ssh/fragment.json @@ -21,7 +21,7 @@ "type": "none" }, "folder": "ssh", - "version": 5, - "version_string": "1.0.4", - "version_release_date": "2026-07-21" + "version": 6, + "version_string": "1.1.0", + "version_release_date": "2026-09-03" } \ No newline at end of file diff --git a/connectors/ssh/ssh_mcp_server.py b/connectors/ssh/ssh_mcp_server.py index 59df2d5..287fefe 100644 --- a/connectors/ssh/ssh_mcp_server.py +++ b/connectors/ssh/ssh_mcp_server.py @@ -24,12 +24,22 @@ Elicited login secrets are kept only in this process's RAM with a short TTL they are dropped on an authentication failure so the next attempt re-prompts. sudo (two methods per alias, set on ``add_alias``): - * ``nopasswd`` — ``sudo -n``: non-interactive, fails fast if NOPASSWD is not - configured on the host (no hung channel). No secret stored anywhere. - * ``prompt`` — ``sudo -S``: the password is requested on demand via **MCP - elicitation** (Skald shows a masked field in the Agent Inbox), fed to - sudo's stdin, kept only in this process's RAM with a short TTL, never sent - to the LLM and never written to disk. + * ``nopasswd`` — ``sudo -n`` only: non-interactive, fails fast with an + explicit message if NOPASSWD is not configured on the host (no hung + channel). No secret stored anywhere. + * ``prompt`` — ``sudo -n`` is **still tried first**; only if the host really + demands a password is one requested via **MCP elicitation** (Skald shows a + masked field in the Agent Inbox), fed to sudo's stdin, kept only in this + process's RAM with a short TTL, never sent to the LLM and never written to + disk. ``SSH_MCP_SUDO_PASSWORD`` is a non-interactive override for + unattended runs (no human to answer the Inbox prompt). + +``exec``/``systemd`` never nest sudo: a leading ``sudo`` (with its usual flags, +including ``-u USER``) is stripped from ``command`` and turned into +``sudo=true``, so an agent that writes ``sudo systemctl restart x`` gets the +same, working behaviour as ``sudo=true`` + ``systemctl restart x``. Under sudo +the command runs as ``sh -c ''`` so pipes and redirections are also +privileged. Connections are pooled per alias with lazy TTL eviction. Host keys are verified against ``~/.ssh/known_hosts`` (unknown hosts are rejected unless the alias was @@ -66,6 +76,10 @@ LOGIN_PW_TTL = int(os.environ.get("SSH_MCP_LOGIN_PW_TTL", "300")) # in-RAM lo CONNECT_TIMEOUT = int(os.environ.get("SSH_MCP_CONNECT_TIMEOUT", "15")) DEFAULT_CMD_TIMEOUT = int(os.environ.get("SSH_MCP_COMMAND_TIMEOUT", "120")) +# Non-interactive sudo password (unattended runs, where nobody can answer the +# elicitation prompt in the Agent Inbox). Empty/unset ⇒ elicitation only. +SUDO_PASSWORD_ENV = os.environ.get("SSH_MCP_SUDO_PASSWORD") or None + # Mirror the native list_files skip set so remote listings match local ones. SKIP_DIRS = {"target", ".git", "node_modules", ".venv", "__pycache__", "secrets"} @@ -107,13 +121,32 @@ def readline() -> dict | None: _eid = itertools.count(1) +# Messages received while blocked on an elicitation reply — replayed by the main +# loop instead of being dropped, so a concurrent tools/call is never lost. +_deferred: list[dict] = [] + +# Whether the client advertised the `elicitation` capability at initialize. A +# client without it can never supply a password, so we fail fast with a useful +# message instead of blocking on a request nobody will answer. +_client_can_elicit = True + + +def next_message() -> dict | None: + """Next inbound message: deferred ones first, then stdin.""" + if _deferred: + return _deferred.pop(0) + return readline() + def elicit(message: str, requested_schema: dict) -> dict: """Send an ``elicitation/create`` request and block until the reply arrives. - Returns the JSON-RPC ``result`` ({"action": ..., "content": {...}}). While - waiting, any other inbound message is ignored (v1: serial processing). + Returns the JSON-RPC ``result`` ({"action": ..., "content": {...}}). Other + inbound messages are queued in ``_deferred`` and handled once the reply + lands (v1: still serial, but nothing is discarded). """ + if not _client_can_elicit: + return {"action": "cancel", "_reason": "unsupported"} eid = f"ssh-elicit-{next(_eid)}" send({ "jsonrpc": "2.0", @@ -124,10 +157,11 @@ def elicit(message: str, requested_schema: dict) -> dict: while True: msg = readline() if msg is None: - return {"action": "cancel"} + return {"action": "cancel", "_reason": "disconnected"} if msg.get("id") == eid: return msg.get("result", {"action": "cancel"}) - log(f"ignoring inbound while awaiting elicitation: {msg.get('method') or msg.get('id')}") + log(f"deferring inbound while awaiting elicitation: {msg.get('method') or msg.get('id')}") + _deferred.append(msg) def _ok(req_id: Any, result: Any) -> dict: @@ -141,6 +175,17 @@ def _text_result(req_id: Any, text: str, is_error: bool = False) -> dict: return {"jsonrpc": "2.0", "id": req_id, "result": res} +def _int(args: dict, key: str, default: int) -> int: + """Integer argument tolerant of null / numeric strings (LLMs send both).""" + v = args.get(key) + if v is None or v == "": + return default + try: + return int(v) + except (TypeError, ValueError): + return default + + # ── Alias store (auto-managed, 0600) ─────────────────────────────────────────── def _load_aliases() -> dict: @@ -216,6 +261,13 @@ def _login_password(alias: str, kind: str = "login") -> str | None: return None +def _no_secret_reason() -> str: + """Why an elicited secret never arrived — the two cases look identical to the + caller but need very different fixes from the user.""" + return ("this MCP client does not support elicitation" if not _client_can_elicit + else "the user declined it or the request timed out") + + def _clear_login_pw(alias: str) -> None: """Drop any cached login password / passphrase for ``alias``.""" for k in [k for k in _login_pw_cache if k.startswith(f"{alias}:")]: @@ -259,7 +311,8 @@ def _connect(cfg: dict, paramiko): password = _login_password(alias, "login") if password is None: raise ToolError( - f"login password required for alias '{alias}' (user declined or timed out)" + f"login password required for alias '{alias}' but none was provided " + f"({_no_secret_reason()})" ) def attempt(passphrase): @@ -301,7 +354,8 @@ def _connect(cfg: dict, paramiko): passphrase = _login_password(alias, "passphrase") if passphrase is None: raise ToolError( - f"key passphrase required for alias '{alias}' (user declined or timed out)" + f"key passphrase required for alias '{alias}' but none was provided " + f"({_no_secret_reason()}) — set SSH_MCP_KEY_PASSPHRASE for unattended runs" ) return attempt(passphrase) except (paramiko.AuthenticationException, paramiko.SSHException) as e: @@ -380,34 +434,89 @@ def _get_sftp(alias: str): def _run_with_stdin(client, command: str, timeout: int, stdin_data: str | None = None): - """Run a remote command; return (stdout, stderr, exit_code). Raises on timeout.""" + """Run a remote command; return (stdout, stderr, exit_code). + + stdout and stderr are drained **together**: they share one SSH channel + window, so reading stdout to EOF first stalls as soon as a chatty stderr + fills that window. ``timeout`` is a wall-clock deadline for the whole run + (not an idle timeout), and stdin is always closed so commands that read it + see EOF instead of hanging. + """ + chan = client.get_transport().open_session(timeout=CONNECT_TIMEOUT) try: - chan_in, chan_out, chan_err = client.exec_command(command, timeout=timeout) - if stdin_data is not None: - try: - chan_in.write(stdin_data) - chan_in.flush() - except Exception: - pass - out = chan_out.read().decode("utf-8", "replace") - err = chan_err.read().decode("utf-8", "replace") - code = chan_out.channel.recv_exit_status() - return out, err, code + chan.settimeout(timeout) + chan.exec_command(command) + try: + if stdin_data: + chan.sendall(stdin_data.encode()) + chan.shutdown_write() + except Exception as e: # closed early by the remote end + log(f"stdin write failed: {e}") + + out, err = bytearray(), bytearray() + deadline = time.time() + timeout + settled = 0 + while True: + idle = True + while chan.recv_ready(): + out += chan.recv(65536) + idle = False + while chan.recv_stderr_ready(): + err += chan.recv_stderr(65536) + idle = False + if not idle: + settled = 0 + continue + if chan.exit_status_ready(): + # Exit status can arrive before the last data. Wait for the + # remote EOF, or — should it close without one — for three + # consecutive empty polls, rather than truncating the output. + if chan.eof_received or chan.closed or settled >= 3: + break + settled += 1 + if time.time() > deadline: + raise ToolError(f"command timed out after {timeout}s") + time.sleep(0.02) + return (out.decode("utf-8", "replace"), + err.decode("utf-8", "replace"), + chan.recv_exit_status()) except socket.timeout: raise ToolError(f"command timed out after {timeout}s") + finally: + try: + chan.close() + except Exception: + pass # ── sudo ─────────────────────────────────────────────────────────────────────── -def _sudo_password(alias: str) -> str | None: - """Return the sudo password for ``alias`` from RAM cache, or elicit it. +def _cached_sudo_password(alias: str) -> str | None: + """Live RAM-cache entry for ``alias``, if any. - Never persisted. Returns None if the user declines/cancels/times out. + Deliberately ignores ``SSH_MCP_SUDO_PASSWORD``: a cache hit lets ``_run_sudo`` + skip its ``sudo -n`` probe, and on a NOPASSWD host sudo would then not read + the password line at all — feeding it straight into the command's stdin. A + RAM entry only ever exists because sudo already demanded a password once. + """ + cached = _sudo_pw_cache.get(alias) + if cached and (time.time() - cached[1] <= SUDO_PW_TTL): + return cached[0] + return None + + +def _sudo_password(alias: str) -> str | None: + """Return the sudo password for ``alias`` from RAM cache / env, or elicit it. + + Never persisted. Returns None if the user declines/cancels/times out, or if + the client cannot elicit at all. """ now = time.time() - cached = _sudo_pw_cache.get(alias) - if cached and (now - cached[1] <= SUDO_PW_TTL): - return cached[0] + cached = _cached_sudo_password(alias) + if cached is not None: + return cached + if SUDO_PASSWORD_ENV: + return SUDO_PASSWORD_ENV result = elicit( f"Enter the sudo password for SSH alias '{alias}'.", @@ -430,21 +539,119 @@ def _sudo_password(alias: str) -> str | None: return None -def _sudo_prefix(alias: str, cfg: dict, sudo_user: str | None): - """Build the sudo prefix for ``cfg``. Returns (prefix, stdin_password). +# sudo refusing to run because it wants a password. sudo prints these *instead* +# of running the command, so probing with `-n` is always side-effect free. +_SUDO_NEEDS_PW = ( + "a password is required", "no password was provided", "a terminal is required", + "no tty present", "askpass", +) +# sudo ran but the password we fed it was wrong. +_SUDO_BAD_PW = ("sorry, try again", "incorrect password attempt", "authentication failure") - Raises ToolError when sudo is disabled or the password is unavailable. + +def _sudo_says(err: str, code: int, markers: tuple) -> bool: + """True if a `sudo:`-prefixed stderr line matches one of ``markers``.""" + if code == 0: + return False + for line in err.splitlines(): + low = line.strip().lower() + if low.startswith("sudo:") and any(m in low for m in markers): + return True + return False + + +# Leading `sudo` (plus its common flags) that an agent put in `command` itself. +# Anything unrecognised simply doesn't match and is left untouched. +_SUDO_OPT = ( + r"""(?:-p\s*(?:'[^']*'|"[^"]*"|\S+)""" # -p PROMPT (takes an argument) + r"|--prompt(?:=|\s+)\S+" + r"|-[EHnSbik]+" # flag bundles without arguments + r"|--(?:preserve-env|set-home|non-interactive|stdin|login|shell|background|remove-timestamp))" +) +_LEADING_SUDO = re.compile( + rf"""^\s*(?:/usr/bin/|/bin/)?sudo + (?:\s+{_SUDO_OPT})* + (?:\s+(?:-u\s*|--user(?:=|\s+))(?P[A-Za-z0-9_.\-]+))? + (?:\s+{_SUDO_OPT})* + (?:\s+--)? + \s+(?P\S.*)$""", + re.X | re.S, +) + + +def _split_leading_sudo(command: str) -> tuple[str, bool, str | None]: + """Split a leading ``sudo …`` off ``command``. + + Returns ``(command_without_sudo, had_sudo, sudo_user)``. Agents routinely + write ``sudo systemctl restart x``; running that through the sudo machinery + would nest a second sudo, whose password prompt has no tty and dies. So the + prefix is peeled off here and expressed as ``sudo=true`` instead. + """ + m = _LEADING_SUDO.match(command) + if not m: + return command, False, None + rest = m.group("rest").strip() + if rest.startswith("-"): + # A sudo option we don't know about — stripping here would hand sudo a + # mangled command line, so leave the whole thing untouched. + return command, False, None + return rest, True, m.group("user") + + +def _sudo_wrap(flags: str, sudo_user: str | None, command: str) -> str: + """``sudo [-u user] sh -c ''`` — the whole command line, + pipes and redirections included, runs with the elevated privileges.""" + u = f"-u {shlex.quote(sudo_user)} " if sudo_user else "" + return f"sudo {flags} {u}sh -c {shlex.quote(command)}" + + +def _run_sudo(alias: str, cfg: dict, client, command: str, + sudo_user: str | None, timeout: int): + """Run ``command`` under sudo. Returns (stdout, stderr, exit_code). + + ``sudo -n`` is always attempted first: on a host that grants this user + NOPASSWD it succeeds outright, so no password is ever requested — which is + the only thing that works in unattended runs, where nobody is watching the + Agent Inbox. A password is elicited only when the host actually demands one. """ method = (cfg.get("sudo") or {}).get("method", "prompt") - u = f"-u {shlex.quote(sudo_user)} " if sudo_user else "" if method == "none": - raise ToolError(f"sudo is disabled for alias '{alias}'") - if method == "nopasswd": - return f"sudo -n {u}", None - pw = _sudo_password(alias) + raise ToolError( + f"sudo is disabled for alias '{alias}' — re-add it with sudo='prompt' to enable it" + ) + + pw = _cached_sudo_password(alias) if method != "nopasswd" else None if pw is None: - raise ToolError("sudo password required (user declined or timed out)") - return f"sudo -S -p '' {u}", pw + out, err, code = _run_with_stdin( + client, _sudo_wrap("-n", sudo_user, command), timeout) + if not _sudo_says(err, code, _SUDO_NEEDS_PW): + return out, err, code + if method == "nopasswd": + raise ToolError( + f"sudo on '{alias}' requires a password, but the alias is configured with " + f"sudo='nopasswd' (which only ever runs 'sudo -n' and never prompts). " + f"Re-add the alias with sudo='prompt', or grant this user a NOPASSWD rule " + f"in the remote /etc/sudoers." + ) + pw = _sudo_password(alias) + if pw is None: + raise ToolError( + f"sudo password required for alias '{alias}' but none was provided " + f"({_no_secret_reason()}). Either answer the sudo prompt in the Agent Inbox, or — for " + f"unattended runs — set SSH_MCP_SUDO_PASSWORD in the connector settings, or " + f"grant this user NOPASSWD sudo on the host." + ) + + out, err, code = _run_with_stdin( + client, _sudo_wrap("-S -p ''", sudo_user, command), timeout, pw + "\n") + if _sudo_says(err, code, _SUDO_BAD_PW) or _sudo_says(err, code, _SUDO_NEEDS_PW): + _sudo_pw_cache.pop(alias, None) # drop it so the next call re-prompts + raise ToolError( + f"sudo password rejected on '{alias}'" + + (" (from SSH_MCP_SUDO_PASSWORD)" if SUDO_PASSWORD_ENV else + " — the cached password was discarded, retry to be asked again") + ) + return out, err, code # ── SFTP helpers ─────────────────────────────────────────────────────────────── @@ -628,7 +835,7 @@ def _tool_list_files(args: dict) -> str: alias, path = args.get("alias"), args.get("path") if not alias or not path: return "Error: 'alias' and 'path' are required" - max_depth = int(args.get("depth", 3)) + max_depth = _int(args, "depth", 3) dirs_only = bool(args.get("dirs_only", False)) sftp = _get_sftp(alias) @@ -675,8 +882,8 @@ def _tool_grep_files(args: dict) -> str: if not alias or not path or pattern is None: return "Error: 'alias', 'path' and 'pattern' are required" mode = args.get("output_mode", "content") - ctx = min(int(args.get("context_lines", 0) or 0), 10) - maxr = int(args.get("max_results", 100)) + ctx = min(_int(args, "context_lines", 0), 10) + maxr = _int(args, "max_results", 100) client = _get_client(alias) flags = _grep_flags(args) @@ -840,32 +1047,38 @@ def _tool_exec(args: dict) -> str: return "Error: 'alias' and 'command' are required" sudo = bool(args.get("sudo", False)) sudo_user = args.get("sudo_user") - timeout = int(args.get("timeout_sec", DEFAULT_CMD_TIMEOUT)) + timeout = _int(args, "timeout_sec", DEFAULT_CMD_TIMEOUT) cfg = _find_alias(alias) if not cfg: return f"Error: unknown alias '{alias}'" - pw = None - wrapped = command - if sudo: - prefix, pw = _sudo_prefix(alias, cfg, sudo_user) - wrapped = prefix + command + # A `sudo` the agent typed into `command` is the same intent as sudo=true — + # honour it here rather than nesting a second, tty-less sudo remotely. + command, inline_sudo, inline_user = _split_leading_sudo(command) + if inline_sudo: + sudo = True + sudo_user = sudo_user or inline_user + if sudo_user: + sudo = True # `sudo -u X` is meaningless without sudo itself + if not command.strip(): + return "Error: 'command' is empty" client = _get_client(alias) - try: - chan_in, chan_out, chan_err = client.exec_command(wrapped, timeout=timeout) - if pw is not None: - try: - chan_in.write(pw + "\n") - chan_in.flush() - except Exception: - pass - out = chan_out.read().decode("utf-8", "replace") - err = chan_err.read().decode("utf-8", "replace") - code = chan_out.channel.recv_exit_status() - except socket.timeout: - return f"Error: command timed out after {timeout}s" - return json.dumps({"stdout": out, "stderr": err, "exit_code": code}) + if sudo: + out, err, code = _run_sudo(alias, cfg, client, command, sudo_user, timeout) + else: + out, err, code = _run_with_stdin(client, command, timeout) + + result = {"stdout": out, "stderr": err, "exit_code": code} + if _sudo_says(err, code, _SUDO_NEEDS_PW): + # A sudo buried mid-command (e.g. `cd /x && sudo …`) that we could not + # peel off. Don't silently re-run the whole line as root — tell the LLM. + result["hint"] = ( + "this command used sudo internally and sudo asked for a password on a " + "tty it does not have. Re-run it with sudo=true and no 'sudo' inside " + "`command` (with sudo=true the whole command line runs as root)." + ) + return json.dumps(result) def _tool_systemd(args: dict) -> str: @@ -884,11 +1097,8 @@ def _tool_systemd(args: dict) -> str: parts: list[str] = [] if action != "status": - prefix, pw = _sudo_prefix(alias, cfg, None) - out, err, code = _run_with_stdin( - client, f"{prefix}systemctl {action} {qsvc}", DEFAULT_CMD_TIMEOUT, - (pw + "\n") if pw else None, - ) + out, err, code = _run_sudo( + alias, cfg, client, f"systemctl {action} {qsvc}", None, DEFAULT_CMD_TIMEOUT) parts.append(f"$ systemctl {action} {service} (exit {code})") if out.strip(): parts.append(out.strip()) @@ -1064,7 +1274,7 @@ TOOLS = [ "auth": {"type": "string", "enum": ["key", "password"], "description": "Login auth. key: SSH key/agent (default). password: login password asked on demand via elicitation, kept only in RAM."}, "sudo": {"type": "string", "enum": ["nopasswd", "prompt", "none"], - "description": "How sudo authenticates. Use 'prompt' unless you KNOW otherwise — it is the safe default: runs 'sudo -S' and asks the user for the sudo password on demand via elicitation, so it works on any host where the login user is a normal sudoer. Only pick 'nopasswd' when the remote /etc/sudoers actually grants THIS user passwordless sudo (a NOPASSWD: rule): it runs 'sudo -n' and NEVER prompts, so on a normal host every sudo call fails immediately with 'a password is required'. 'none' disables sudo. Default prompt."}, + "description": "How sudo authenticates. Keep the default 'prompt' unless you KNOW otherwise: it tries 'sudo -n' first (so a host with a NOPASSWD rule never prompts) and only asks the user for the sudo password via elicitation if the host actually demands one. 'nopasswd' runs 'sudo -n' only and NEVER prompts — pick it just to forbid prompting, since on a host without a NOPASSWD rule every sudo call then fails. 'none' disables sudo entirely. Default prompt."}, "accept_new_host_key": {"type": "boolean", "description": "Trust the host key on first connect (TOFU). Default false."}, }, "required": ["alias", "hostname", "username"], @@ -1161,14 +1371,20 @@ TOOLS = [ { "name": "exec", "title": "Execute Command", - "description": "Run a command on the remote host. Set sudo=true to run via sudo (method per alias).", + "description": ( + "Run a command on the remote host. For anything needing root, set sudo=true and " + "write the command WITHOUT a 'sudo' prefix — with sudo=true the entire command " + "line (pipes and redirections included) runs as root. A leading 'sudo' left in " + "`command` is stripped and treated as sudo=true anyway, but a 'sudo' in the middle " + "of a command (e.g. 'cd /x && sudo …') has no tty and will fail." + ), "inputSchema": { "type": "object", "properties": { "alias": _ALIAS, - "command": {"type": "string", "description": "Shell command."}, - "sudo": {"type": "boolean", "description": "Run via sudo (default false)."}, - "sudo_user": {"type": "string", "description": "Target user for sudo -u (optional)."}, + "command": {"type": "string", "description": "Shell command. Do NOT prefix it with 'sudo' — use the sudo argument."}, + "sudo": {"type": "boolean", "description": "Run the whole command as root, via sudo (default false)."}, + "sudo_user": {"type": "string", "description": "Target user for sudo -u (optional; implies sudo)."}, "timeout_sec": {"type": "integer", "description": "Kill after N seconds (default 120)."}, }, "required": ["alias", "command"], @@ -1248,10 +1464,16 @@ def handle_message(msg: dict) -> dict | None: req_id = msg.get("id") if method == "initialize": + # Remember whether the client can collect input mid-call: without the + # elicitation capability we must never block on a password prompt. + global _client_can_elicit + caps = (msg.get("params") or {}).get("capabilities") or {} + _client_can_elicit = "elicitation" in caps + log(f"client elicitation capability: {'yes' if _client_can_elicit else 'no'}") return _ok(req_id, { "protocolVersion": "2025-06-18", "capabilities": {"tools": {}}, - "serverInfo": {"name": "ssh", "version": "1.0.0"}, + "serverInfo": {"name": "ssh", "version": "1.1.0"}, }) if method == "notifications/initialized": return None @@ -1283,7 +1505,7 @@ def main() -> None: log("starting SSH MCP server") try: while True: - msg = readline() + msg = next_message() if msg is None: break resp = handle_message(msg) diff --git a/docs/connector.manifest_guide.md b/docs/connector.manifest_guide.md index 927a7a9..284e001 100644 --- a/docs/connector.manifest_guide.md +++ b/docs/connector.manifest_guide.md @@ -1,380 +1,9 @@ -# Skald Connector Authoring Guide +# Skald Connector Authoring Guide — moved -Instructions for generating a **correct connector** for the Skald marketplace -(`https://connectors.skaldagent.net`). Give this file to the agent that produces -new connectors. +This copy has been retired. The authoring spec now lives, in a single place, at +[`CONNECTOR_MANIFEST_GUIDE.md`](../CONNECTOR_MANIFEST_GUIDE.md) in the repo root — it absorbed +everything this file documented (the `fragment.json` + `scripts/compile.py` build pipeline) +plus the client behaviour verified against `~/projects/skald-circle`. -A connector is a folder served by the marketplace. Skald installs it, verifies -every file against a SHA-256 pinned in the index, then either runs it on the host -(global connector) or copies it into the user's container and runs it there -(per-user connector, blueprint §6/§7). - ---- - -## 1. The two documents (+ one build script) - -### 1a. The compiled root index — `connectors.json` - -**This file is auto-generated.** Do not edit it by hand. It is produced by -[`scripts/compile.py`](../scripts/compile.py), which reads `index.json` + the -`fragment.json` in each connector folder, scans the physical files for SHA-256 -digests, and writes the final index. - -The output schema looks like this: - -```jsonc -{ - "version": 1, - "connectors": [ - { - "id": "whatsapp", // unique slug = folder name - "name": "WhatsApp", - "version": 1, // INTEGER build number — the update key (§7) - "version_string": "2.0.1", // semver, display only - "version_release_date": "2026-07-19", // ISO date, display only - "type": "mcp_local", // mcp_local | mcp_remote (see §3) - "scope": "user", // user | global (see §3) - "icon_small": "whatsapp/icon_sm.svg", - "icon_large": "whatsapp/icon_lg.svg", - "user_description": "Send and read WhatsApp messages from your linked account.", - "requires": ["NODE"], // human hint: NODE | PYTHON | OAUTH | API_KEY - "tags": ["messaging", "mcp", "local", "whatsapp", "qr"], - "auth": { "type": "qr" }, // may be repeated here and in the manifest - "folder": "whatsapp", // defaults to id - "files": [ - { "path": "index.js", "sha256": "…", "size": 21258 }, - { "path": "package.json", "sha256": "…", "size": 302 }, - { "path": "connector.json", "sha256": "…", "size": 620 }, - { "path": "icon_sm.svg", "sha256": "…", "size": 306 }, - { "path": "icon_lg.svg", "sha256": "…", "size": 308 } - ] - } - ] -} -``` - -**Rules** - -- `files[].path` is relative to the connector folder. List **every** file the - connector ships (server code, `package.json`/`requirements.txt`, icons, and the - `connector.json` itself). A missing or mismatched digest fails the install. -- SHA-256 digests and file sizes are **computed automatically** by `compile.py`. - Never write them by hand. -- The excluded files (`fragment.json`, `connectors.json`, `index.json`, - `.DS_Store`) are handled by `compile.py` — you don't need to think about them. -- Do **not** list `node_modules/` or any generated deps — those are installed on - the box, not shipped (see §5). -- `size` is optional but the compiler includes it. - -#### How the index is built - -Instead of editing `connectors.json` directly, you maintain **two lightweight -source files** and run one command: - -1. **`connectors/index.json`** — a flat JSON array of folder ids in display - order: `["gmail", "gcal", "myconn", …]` -2. **`connectors//fragment.json`** — the connector's index entry with - **every field except `files[]`** (same schema as above minus that array). -3. **`python3 scripts/compile.py`** — reads `index.json`, loads each - `fragment.json`, scans the folder for real files, computes SHA-256, and - writes `connectors.json`. - -The `tools[]` block (friendly UI names, §2a) goes into `fragment.json`. - -### 1b. The per-connector manifest — `/connector.json` - -The richer document. Fetched per connector and mapped into Skald's catalog. - -```jsonc -{ - "id": "whatsapp", - "name": "WhatsApp", - "version": 1, // INTEGER build number — the update key (§7) - "version_string": "2.0.1", // semver, display only - "version_release_date": "2026-07-19", // ISO date, display only - "type": "mcp_local", - "scope": "user", - "auth": { "type": "qr" }, // none | api_key | oauth2 | qr (see §4) - "mcp_config": { - "command": "node", // interpreter (local) … - "args": ["index.js"], // … args[0] MUST name the entry file - "transport": "stdio" // stdio (local) | streamable-http (remote) - }, - "docs": [{ - "lang": "en", - "description": "Human blurb shown in the UI.", - "llm_short_description": "One line the model reads to decide whether to use this connector." - }], - "env": [], // form fields the user fills (see §4b) - "tools": [ // OPTIONAL — friendly UI names per tool (§2a) - { "name": "send_message", "display_name": "Send Message" } - ], - "homepage": "https://…", - "icon_small": "icon_sm.svg", // relative to the folder here - "icon_large": "icon_lg.svg", - "tags": ["messaging", "mcp", "local", "whatsapp", "qr"] -} -``` - -**`mcp_config.args[0]` is load-bearing for a local connector:** it is how Skald -learns which file to run. At activation Skald rewrites it to the file's path -inside the user's container (`/root/.skald/mcp//`), so keep it a -plain relative filename (`index.js`, `server.py`, `pkg/server.py`). - -**`llm_short_description` is model-facing:** it's the one-liner injected into -the LLM's system prompt so the model knows what this connector does. Keep it -short and functional ("Weather — current conditions, 16-day forecast, and AQI -data for any location"), **not** a list of tools (the model discovers tools -after `activate_tools`). - ---- - -## 2. Server contract (MCP over stdio) - -A **local** connector is a program speaking JSON-RPC 2.0 over stdin/stdout. It -MUST handle: - -- `initialize` → `{ protocolVersion, capabilities: { tools: {} }, serverInfo }` -- `notifications/initialized` → no response -- `tools/list` → `{ tools: [ { name, description, inputSchema } ] }` -- `tools/call` → `{ content: [ { type: "text", text } ], isError? }` - -**stdout is reserved for JSON-RPC only.** Send all logs/diagnostics to **stderr**. -Anything a library prints to stdout (a logger, a banner) corrupts the protocol — -silence it (e.g. Baileys/pino → a silent logger; Python → `print(…, file=sys.stderr)`). - -A **remote** connector is an HTTP MCP endpoint (`mcp_config.url` + -`transport: "streamable-http"`); no code runs on the box. - -### 2a. Friendly tool names (`tools[]`) — optional - -Raw MCP tool names are ugly in the chat UI (`search_files`, `send_message`). The -optional top-level `tools[]` block gives each one a human title shown as the tool -card's heading: - -```jsonc -"tools": [ - { "name": "send_message", "display_name": "Send Message" }, - { "name": "list_chats", "display_name": "List Chats" }, - { "name": "download_media", "display_name": "Download Media" } -] -``` - -- `name` — the **raw** tool name exactly as your server returns it from `tools/list`. -- `display_name` — the friendly card title (English only; not internationalized). - -**Resolution order** for a tool's card title is **`tools[].display_name` → the MCP -`title` field → a prettified raw name**. So you have two ways to set a friendly -name, and can skip `tools[]` entirely: - -1. **This block** — the authoritative override, curated in the manifest. -2. **The MCP `title` field** — if your `tools/list` entries already carry a - `title` (MCP 2025-06-18+), Skald uses it automatically; no manifest change - needed. `tools[]` wins if both are present. -3. If neither is set, Skald title-cases the raw name (`send_message` → "Send - Message"). - -**Icons are per connector, not per tool.** Every tool of a connector shows that -connector's own `icon_small`; there is no per-tool icon field. Only list a tool in -`tools[]` when its prettified name isn't good enough — partial lists are fine -(unlisted tools fall through to steps 2–3). - ---- - -## 3. Placement & risk vocabulary (what the words mean) - -| Manifest | Meaning | -| --- | --- | -| `scope: "user"` | runs **once per user**, inside their container. Personal creds. | -| `scope: "global"` | runs **once for the household**, on the host. Shared, stateless. Admin enables it with a key. | -| `type: "mcp_local"` | ships code that will **execute on the box** — installing needs the admin `mcp.register_local_script` capability (RCE-bearing act, §14). | -| `type: "mcp_remote"` | just an HTTP URL; no local code. | - -Pick the narrowest: a personal messaging/email/calendar connector is -`scope: "user"`; a shared search API is `scope: "global"`. - ---- - -## 4. Authentication (`auth.type`) - -| `auth.type` | Flow | Ships | -| --- | --- | --- | -| `none` | nothing to sign in | — | -| `api_key` | user pastes a key/secret into a form | an `env[]` schema (§4b) | -| `oauth2` | browser consent → paste code back | `auth.provider` + `auth.scopes` + `auth.deliver` (§4c) | -| `qr` | server shows a QR, user scans with a phone | a `login_status` tool (§4d) | - -### 4b. `api_key` — the `env[]` schema - -Each entry drives one form field **and** is injected as an env var / URL token to -the server: - -```jsonc -"env": [{ - "name": "tavilyApiKey", - "label": "Tavily API key", - "description": "Create one at https://app.tavily.com.", - "required": true, - "secret": true, // rendered masked, stored encrypted - "example": "tvly-xxxxxxxx" -}] -``` - -The server reads each value from `process.env.` (or `os.environ`). For a -**remote** connector that wants the key in the URL, use a placeholder: -`"url": "https://mcp.example.com/?key={SECRET:tavilyApiKey}"`. - -### 4c. `oauth2` — provider consent - -```jsonc -"auth": { - "type": "oauth2", - "provider": "google", // slug into the admin's sign-in providers - "scopes": ["https://www.googleapis.com/auth/gmail.modify"], - "deliver": { "as": "env", "format": "google_authorized_user", "env": "GMAIL_CREDS_JSON" } -} -``` - -The manifest names **only** the provider slug, scopes, and how the obtained token -is delivered — never client secrets or endpoint URLs (those are admin-entered, -kept off the public feed). Skald handles PKCE + code exchange and injects the -credential as the named env var. `format`: `google_authorized_user` (Google) or -`refresh_token`. Today only `as: "env"` is wired. - -### 4d. `qr` / interactive device login — the generic contract - -For a connector whose credential is produced by **scanning/pairing** (WhatsApp -today), there is no code to paste. The rule: - -> **Expose one extra tool, `login_status`, returning a JSON object** (as the -> `text` of a normal text result). Skald calls it directly (never the agent) and a -> login panel polls it. - -```jsonc -// login_status result text (a JSON string): -{ - "state": "connecting" | "need_scan" | "ready" | "logged_out", - "qr": "data:image/png;base64,…", // present ONLY while state == need_scan - "message": "human-readable line" -} -``` - -- `activate` on a `qr` connector inserts a **pending** row and **starts the - server** (so it can produce the QR), then hands off to the login panel. -- The panel polls `POST /api/mcp/login/status`; when `state == "ready"` the - connector is marked ready and starts automatically on later logins. -- Also expose a `logout` tool (clears the session, forces a fresh QR) — the panel - calls it via `POST /api/mcp/login/reset` to re-link a different phone. -- The **credential is the on-disk session**, not a token. Persist it **inside the - connector's own directory** (e.g. `./auth/` next to the entry file). That folder - lives under the bind-mounted home, so it survives container recreates and - connector updates. Never store it under a shared/global path. - -Skald resolves `auth.type: "qr"` the same way whether it appears in the index -entry or the manifest. - ---- - -## 5. Dependencies (node & python) — how they get installed - -**Do not ship `node_modules/` or vendored wheels.** Declare deps as a standard -manifest **file** and Skald installs them inside the container: - -- **node:** ship a `package.json` with a `dependencies` map. Skald runs - `npm ci --omit=dev` (falling back to `npm install --omit=dev`) in the connector - dir. `node_modules/` resolves automatically beside the entry file. -- **python:** ship a `requirements.txt`. Skald installs it with - `pip install --target .pydeps` and puts `.pydeps` on the server's `PYTHONPATH`. - -This runs at activation **and** on every startup, guarded by a **content hash** of -the connector's source files: - -- first activation / a brand-new container → full install, -- a connector **update** (any shipped file changed) → re-copy + re-install, -- unchanged → skipped in microseconds. - -So you never write install steps into the manifest — just ship the dep file, list -it in the index with its SHA-256, and set `requires: ["NODE"]` / `["PYTHON"]` as a -human hint. Pin versions in `package.json` / `requirements.txt` for reproducible -installs. Keep the dep tree lean (containers are slim; avoid native-heavy -packages where a pure alternative exists — e.g. Baileys instead of a browser). - ---- - -## 6. Verify-before-save (optional but recommended) - -Ship a `verify.py` / verify snippet and reference it: - -```jsonc -"verify": { "command": "python3 verify.py", "timeout_secs": 15 } -``` - -It runs with the collected env/secret injected and must print **one JSON object** -on stdout: `{"ok": bool, "message": string, "details"?: object}`, exit 0 on -success. Used for `api_key`/`none` connectors to test creds before activating. -(A `qr` connector needs no verify — its `login_status` is the live check.) - ---- - -## 7. Versioning & updates - -Three fields, in **both** the index entry and the `connector.json`, kept identical: - -| field | type | role | -| --- | --- | --- | -| `version` | **integer** | monotonic build number, **per connector** — the machine comparison key | -| `version_string` | string (semver) | display only | -| `version_release_date` | ISO date `YYYY-MM-DD` | display only | - -- `version` is a **number, not a string** (`1`, not `"1"` or `"2.0.1"`). Start at - `1` for the first release under this scheme; **`+1` on every change** to any - shipped file **or to any manifest metadata** (description, icons, `version_string`). - Never reuse or decrement. -- Skald stores the installed `version` and compares it to the feed's: a strictly - greater feed `version` shows **"update available"** in the marketplace, and the - Install button becomes **Update**. Clicking it re-downloads the files and rewrites - the catalog row. -- **The integer is the *only* "is there an update?" signal** — it is compared - strictly (`feed > installed`). `version_string` (semver), icons and - `llm_short_description` are **never** compared, so a change to any of them that - does not also bump the integer is **invisible**: no "update available" badge - appears. This is the common trap — a "content-only" edit (e.g. a better - `llm_short_description`) that forgets the integer. -- **Two propagation paths, do not conflate them:** - - *Per-user code + deps* (the scripts, `package.json`/`requirements.txt`) reconcile - on a **content-hash** of the source files (§5), so new code lands at each user's - next login even without a reinstall. - - *Catalog metadata* (`llm_short_description` → the model's prompt, icons, friendly - name) is **not** in that hash — it lives in the catalog row and is rewritten only - by an explicit **reinstall/Update**. On reinstall Skald re-pulls the current feed - (never the browse cache) and pushes the new description live: enabled global - servers restart with it, and every logged-in user who activated the connector has - it restarted with the fresh `llm_short_description` — no re-login needed. -- So: to ship a new `llm_short_description`, **bump the integer** (so the admin sees - "update available") and the admin clicks **Update**. Nothing auto-propagates a - description change. -- `version_string` and `version_release_date` are display metadata only — never - compared. (Migration note: replace any legacy string `"version": "2.0.1"` with - the integer `version` + `version_string`.) - ---- - -## 8. Checklist for a new connector - -1. Folder `myconn/` with: entry file, `connector.json`, deps file - (`package.json`/`requirements.txt`), `icon_sm.svg`, `icon_lg.svg`, - optional `verify.*`. -2. Create `fragment.json` inside the folder (same schema as the index entry - without `files[]` — include `tools[]` if needed). -3. Add `"myconn"` to `connectors/index.json`. -4. Run `python3 scripts/compile.py` (auto-generates `connectors.json` with - fresh SHA-256 digests). -5. Server speaks MCP over stdio (§2); **stdout = JSON-RPC only**. -6. `mcp_config.args[0]` names the entry file. -7. Correct `type` + `scope` (§3) and `auth.type` (§4). -8. For `qr`: implement `login_status` (+ `logout`), persist the session under the - connector dir (§4d). -9. Deps declared as a file, **not** vendored (§5). -10. Bump `version`. -11. Run `python3 scripts/compile.py --verify` to confirm the index is fresh, - then commit. +Keeping two copies is what let them drift in the first place. Edit the root file only; the +history of this one is in git. diff --git a/opencode.json b/opencode.json new file mode 100644 index 0000000..fdcb0cb --- /dev/null +++ b/opencode.json @@ -0,0 +1,27 @@ +{ + "$schema": "https://opencode.ai/config.json", + "instructions": ["CLAUDE.md"], + "references": { + "docs": { + "path": "CONNECTOR_MANIFEST_GUIDE.md", + "description": "Format used by the marketplace. The marketplace is used by ~/projects/skald-circle" + } + }, + "permission": { + "bash": { + "cargo *": "allow", + "./run.sh": "allow", + "./run-docker.sh": "allow", + "./run-log.sh": "allow", + "./backup.sh": "allow", + "rg *": "allow", + "git status": "allow", + "git diff*": "allow", + "git log*": "allow", + "git show*": "allow", + "*": "ask" + }, + "external_directory": { "*": "allow" } + }, + "tool_output": { "max_lines": 200, "max_bytes": 16384 } +} diff --git a/scripts/compile.py b/scripts/compile.py index 808cfbf..5564186 100644 --- a/scripts/compile.py +++ b/scripts/compile.py @@ -13,8 +13,12 @@ Reads: Produces: - connectors/connectors.json — full index with files[] and SHA-256 -Auto-excluded: fragment.json, connector.json, compile.sh, compile.py, -update_hashes.py, .DS_Store, and scripts/ directory. +Auto-excluded: fragment.json, connectors.json, index.json, compile.sh, compile.py, +update_hashes.py, .DS_Store, and every subdirectory. `connector.json` IS hashed — +it ships with the connector and the index is what pins it. + +Note: only the folder's top level is scanned, so a connector must keep its files +flat; anything in a subdirectory never reaches files[]. """ from __future__ import annotations