diff --git a/docs/configuration/flights.md b/docs/configuration/flights.md index 056dc66..a703a2a 100644 --- a/docs/configuration/flights.md +++ b/docs/configuration/flights.md @@ -28,7 +28,9 @@ A flight carries four details: while normal local and Hub authorization still applies. It never raises the identity's actual KEG role. `manage_flights` exposes flight mutation tools to the session, but Hub still requires the authenticated identity to own or - administer the target namespace. The capabilities are independent. + administer the target namespace. `manage_kegs` exposes `keg_create`, and + Hub still requires the identity to belong to the target namespace. The + capabilities are independent. Because a flight is not a KEG target selector, `tap mcp --flight` binds only the process flight identity. `tap mcp --keg` remains an independent default for @@ -113,14 +115,24 @@ current values. A Hub-backed active flight may edit or delete itself: - a successful self-delete immediately enters recovery-only mode; - editing or deleting another flight does not change current session authority. -Local `flights.d` manifests remain MCP read-only. Flight mutations always use -normal Hub authorization in addition to the active flight capability. +Local `flights.d` manifests remain MCP read-only — *reading* them is fully +supported (discovery, orientation, and cover enforcement all work off +`flights.d`), but create/update/delete is not implemented for local hubs and +refuses with a message naming the manifest path to write instead. Flight +mutations always use normal Hub authorization in addition to the active flight +capability. ## Behavior - MCP tools reject a keg outside the active flight's cover with a "keg … is not available in flight …" error. - MCP writes against a `viewer` cover row are rejected as viewer-only. +- Every cover and role-cap denial closes by telling the agent to call `orient`. + A session pins its flight snapshot until it re-orients, so a flight edited + elsewhere mid-session is the usual reason a call the agent expected to + succeed is refused, and the refusal alone cannot reveal that. Hosted `/mcp` + appends the same instruction when a Hub grant — rather than the cover — + is what denies the keg. - `keg_settings_edit` replaces the complete validated KEG YAML document and requires an `admin` cover (or `full_access`) plus editor/admin identity access to that KEG. An admin flight cap never creates a Hub admin identity. @@ -130,6 +142,20 @@ normal Hub authorization in addition to the active flight capability. - Without a selected flight, MCP starts in recovery-only mode and lists only `orient`, `list_flights`, `flight_show`, and credential-safe `auth_info`. After selecting a flight outside MCP, call `orient` on the same connection. +- When the session can reach **no flights at all**, it instead runs on a + synthetic **bootstrap flight**. Selecting from an empty list is not a + recovery, so the session is given the authority to populate it: the cover is + empty (every KEG tool stays locked) and the capabilities are `manage_flights` + plus `manage_kegs`, so `flight_create`, `flight_edit`, `flight_delete`, and + `keg_create` join the recovery four. The flight is never persisted, and its + instructions name the surface that owns selection for that transport — `tap` + configuration for stdio, the account page for hosted `/mcp`. +- Creating a flight from bootstrap does not select it. The next `orient` sees a + reachable flight and moves the session to recovery-only mode, where "select + one" has become the actionable step. +- On a local-only setup `flight_create` still fails: flight mutation is not + implemented for local hubs (see below). The bootstrap instructions say so and + point at the manifest path to write by hand. - Config-driven `tap mcp` reloads user, project, and environment configuration on every orientation. A successful orientation atomically replaces session authority; configuration changes alone do nothing. diff --git a/integrations/content/agent-orient.md b/integrations/content/agent-orient.md index aeacb5f..5da3931 100644 --- a/integrations/content/agent-orient.md +++ b/integrations/content/agent-orient.md @@ -1,11 +1,20 @@ # tapper Interact with Tapper KEGs (Knowledge Exchange Graphs) through the native MCP -server. At the start of KEG work, call `mcp__tapper__orient`, identify the -relevant covered KEGs from their titles and summaries, then call -`mcp__tapper__keg_settings` for those KEGs before operating. Treat the active -flight, cover, flight instructions, and targeted KEG instructions as the -authoritative context for the session. +server. + +**Call `mcp__tapper__orient` first, in every session, before doing anything +else — including answering the user.** Do not wait until KEG work looks like it +is starting. The active flight carries the instructions describing what this +session is for, so until you orient you cannot know whether the work is KEG +work, which KEGs you may touch, or what the user actually expects of you. A +message as small as "test" is not a reason to defer: orient, then respond with +that context in hand. + +After orienting, identify the relevant covered KEGs from their titles and +summaries and call `mcp__tapper__keg_settings` for those KEGs before operating +on them. Treat the active flight, cover, flight instructions, and targeted KEG +instructions as the authoritative context for the session. ## Rules @@ -21,6 +30,24 @@ authoritative context for the session. - **Treat the active flight as MCP authority.** It determines the instructions and KEGs available to the agent. `defaultKeg` does not grant authority for an MCP session. +- **Leave node 0 alone.** It is the keg's placeholder landing node, created with + the keg itself. + +## Node 0 + +Every keg has a node `0`. It is not an ordinary node and is not yours to write: + +- It is the **placeholder** a link to unwritten content lands on, so its content + is deliberately generic. +- It carries **no `type`**, on purpose. Do not add one, and do not read its + absence as a defect to repair — a node without a type is normally a schema + error, and node 0 is the documented exception. +- **Removing it breaks the keg.** Tapper treats a missing node 0 as an + uninitialized keg, so deleting it makes every other node unreachable. + +When you have content to write, create a new node. If node 0 genuinely needs to +change — a keg's landing page is a reasonable thing to want — say so and let the +user decide; do not fold it into unrelated work. ## Flight-first orientation @@ -37,11 +64,40 @@ restarting the MCP server. ## Bootstrapping a session -Call `mcp__tapper__orient` first, then load the selected KEG instructions with -`mcp__tapper__keg_settings`. When no flight is selected, the local MCP server connects in a -recovery-only state: KEG tools are locked, while `mcp__tapper__list_flights` -and `mcp__tapper__flight_show` remain available for discovery. Ask the user to -select a flight in Tapper configuration, then call `mcp__tapper__orient` again. +Orientation is unconditional and comes first, before any other tool call and +before your first reply. It is not a lookup step you reach for once KEG work is +identified — it is how the session learns what it is for. Then load the +selected KEG instructions with `mcp__tapper__keg_settings`. + +**Orient again after any context reset**, such as a clear or a compact. The MCP +connection survives those, so the server does not re-initialize and will not +re-send anything on its own — but the flight instructions you were operating +under are gone from your context. Re-orienting is cheap and idempotent, and it +also picks up any configuration change made since you connected. If you cannot +tell whether you have oriented in the current context, you have not; orient. + +**The newest orientation wins.** More than one copy can be present at once: the +connection's startup instructions are captured when the server connects and are +never refreshed afterwards, and a compaction summary may carry a paraphrase of +an earlier orientation. Both can be stale, and a stale copy may sit earlier in +your context than the fresh one. Treat the most recent `mcp__tapper__orient` +result as authoritative and discard the others outright rather than reconciling +them — in particular, a startup copy saying KEG tools are locked is wrong once +a later orientation has returned a flight. When no flight is selected, the MCP +server connects in a recovery-only state: KEG tools are locked, while +`mcp__tapper__list_flights` and `mcp__tapper__flight_show` remain available for +discovery. Ask the user to select a flight in Tapper configuration, then call +`mcp__tapper__orient` again. + +When there is no flight to select — a fresh machine or account — the session +instead starts on a temporary **bootstrap flight**. Its cover is empty, so the +KEG tools stay locked, but `mcp__tapper__keg_create` and the flight mutation +tools are available so you can create the first KEG and the first flight. +Setting that up is the session's work; do it before anything else. You still +cannot *select* a flight — that stays a human action — so hand the setup back +to the user and call `mcp__tapper__orient` again once they confirm. The +orientation payload names exactly where they should do it. + If `mcp__tapper__orient` is unavailable, report that the Tapper MCP connection is unavailable, ask the user to reconnect or restart the host session, and never kill or signal host-owned processes. A flight with an empty cover exposes diff --git a/integrations/content/linking.md b/integrations/content/linking.md index 739783b..1e412fa 100644 --- a/integrations/content/linking.md +++ b/integrations/content/linking.md @@ -14,3 +14,21 @@ Tapper supports two link forms in node bodies: Both forms appear in backlinks. Prefer intra-keg links when the target is in the same keg. + +## Attachments + +A node's uploaded files and images live in two directories inside the node's +own directory, so they are linked relative to it — the same base the `../NODEID` +form above counts from: + +- **File:** `[label](./assets/FILE)` — anything uploaded with + `mcp__tapper__upload_file`. +- **Image:** `![alt](./images/IMAGE)` — anything uploaded with + `mcp__tapper__upload_image`. + +**Both directory names are plural**: `assets/` and `images/`, never `asset/` or +`image/`. Uploading succeeds regardless of how you later write the link, so a +singular path fails silently as a broken reference rather than as an error. + +Use `mcp__tapper__list_files` and `mcp__tapper__list_images` to get the exact +stored names; the upload may normalize the filename you supplied. diff --git a/integrations/rendered/claude/tapper/skills/tapper/SKILL.md b/integrations/rendered/claude/tapper/skills/tapper/SKILL.md index f0d6350..eb6b7a3 100644 --- a/integrations/rendered/claude/tapper/skills/tapper/SKILL.md +++ b/integrations/rendered/claude/tapper/skills/tapper/SKILL.md @@ -6,11 +6,20 @@ description: Orient to Tapper flights and operate on KEGs through MCP-first safe # tapper Interact with Tapper KEGs (Knowledge Exchange Graphs) through the native MCP -server. At the start of KEG work, call `mcp__tapper__orient`, identify the -relevant covered KEGs from their titles and summaries, then call -`mcp__tapper__keg_settings` for those KEGs before operating. Treat the active -flight, cover, flight instructions, and targeted KEG instructions as the -authoritative context for the session. +server. + +**Call `mcp__tapper__orient` first, in every session, before doing anything +else — including answering the user.** Do not wait until KEG work looks like it +is starting. The active flight carries the instructions describing what this +session is for, so until you orient you cannot know whether the work is KEG +work, which KEGs you may touch, or what the user actually expects of you. A +message as small as "test" is not a reason to defer: orient, then respond with +that context in hand. + +After orienting, identify the relevant covered KEGs from their titles and +summaries and call `mcp__tapper__keg_settings` for those KEGs before operating +on them. Treat the active flight, cover, flight instructions, and targeted KEG +instructions as the authoritative context for the session. ## Rules @@ -26,6 +35,24 @@ authoritative context for the session. - **Treat the active flight as MCP authority.** It determines the instructions and KEGs available to the agent. `defaultKeg` does not grant authority for an MCP session. +- **Leave node 0 alone.** It is the keg's placeholder landing node, created with + the keg itself. + +## Node 0 + +Every keg has a node `0`. It is not an ordinary node and is not yours to write: + +- It is the **placeholder** a link to unwritten content lands on, so its content + is deliberately generic. +- It carries **no `type`**, on purpose. Do not add one, and do not read its + absence as a defect to repair — a node without a type is normally a schema + error, and node 0 is the documented exception. +- **Removing it breaks the keg.** Tapper treats a missing node 0 as an + uninitialized keg, so deleting it makes every other node unreachable. + +When you have content to write, create a new node. If node 0 genuinely needs to +change — a keg's landing page is a reasonable thing to want — say so and let the +user decide; do not fold it into unrelated work. ## Flight-first orientation @@ -42,11 +69,40 @@ restarting the MCP server. ## Bootstrapping a session -Call `mcp__tapper__orient` first, then load the selected KEG instructions with -`mcp__tapper__keg_settings`. When no flight is selected, the local MCP server connects in a -recovery-only state: KEG tools are locked, while `mcp__tapper__list_flights` -and `mcp__tapper__flight_show` remain available for discovery. Ask the user to -select a flight in Tapper configuration, then call `mcp__tapper__orient` again. +Orientation is unconditional and comes first, before any other tool call and +before your first reply. It is not a lookup step you reach for once KEG work is +identified — it is how the session learns what it is for. Then load the +selected KEG instructions with `mcp__tapper__keg_settings`. + +**Orient again after any context reset**, such as a clear or a compact. The MCP +connection survives those, so the server does not re-initialize and will not +re-send anything on its own — but the flight instructions you were operating +under are gone from your context. Re-orienting is cheap and idempotent, and it +also picks up any configuration change made since you connected. If you cannot +tell whether you have oriented in the current context, you have not; orient. + +**The newest orientation wins.** More than one copy can be present at once: the +connection's startup instructions are captured when the server connects and are +never refreshed afterwards, and a compaction summary may carry a paraphrase of +an earlier orientation. Both can be stale, and a stale copy may sit earlier in +your context than the fresh one. Treat the most recent `mcp__tapper__orient` +result as authoritative and discard the others outright rather than reconciling +them — in particular, a startup copy saying KEG tools are locked is wrong once +a later orientation has returned a flight. When no flight is selected, the MCP +server connects in a recovery-only state: KEG tools are locked, while +`mcp__tapper__list_flights` and `mcp__tapper__flight_show` remain available for +discovery. Ask the user to select a flight in Tapper configuration, then call +`mcp__tapper__orient` again. + +When there is no flight to select — a fresh machine or account — the session +instead starts on a temporary **bootstrap flight**. Its cover is empty, so the +KEG tools stay locked, but `mcp__tapper__keg_create` and the flight mutation +tools are available so you can create the first KEG and the first flight. +Setting that up is the session's work; do it before anything else. You still +cannot *select* a flight — that stays a human action — so hand the setup back +to the user and call `mcp__tapper__orient` again once they confirm. The +orientation payload names exactly where they should do it. + If `mcp__tapper__orient` is unavailable, report that the Tapper MCP connection is unavailable, ask the user to reconnect or restart the host session, and never kill or signal host-owned processes. A flight with an empty cover exposes @@ -169,6 +225,24 @@ Tapper supports two link forms in node bodies: Both forms appear in backlinks. Prefer intra-keg links when the target is in the same keg. +## Attachments + +A node's uploaded files and images live in two directories inside the node's +own directory, so they are linked relative to it — the same base the `../NODEID` +form above counts from: + +- **File:** `[label](./assets/FILE)` — anything uploaded with + `mcp__tapper__upload_file`. +- **Image:** `![alt](./images/IMAGE)` — anything uploaded with + `mcp__tapper__upload_image`. + +**Both directory names are plural**: `assets/` and `images/`, never `asset/` or +`image/`. Uploading succeeds regardless of how you later write the link, so a +singular path fails silently as a broken reference rather than as an error. + +Use `mcp__tapper__list_files` and `mcp__tapper__list_images` to get the exact +stored names; the upload may normalize the filename you supplied. + ## Secret handling - Never store credentials, API tokens, private keys, session cookies, customer diff --git a/integrations/rendered/codex/tapper/.mcp.json b/integrations/rendered/codex/tapper/.mcp.json index fc7f2ef..b46474e 100644 --- a/integrations/rendered/codex/tapper/.mcp.json +++ b/integrations/rendered/codex/tapper/.mcp.json @@ -4,6 +4,8 @@ "command": "tap", "args": ["mcp"], "env_vars": [ + "HOME", + "TAP_FLIGHT", "XDG_CONFIG_HOME", "XDG_DATA_HOME", "XDG_STATE_HOME", diff --git a/integrations/rendered/codex/tapper/skills/tapper/SKILL.md b/integrations/rendered/codex/tapper/skills/tapper/SKILL.md index f0d6350..eb6b7a3 100644 --- a/integrations/rendered/codex/tapper/skills/tapper/SKILL.md +++ b/integrations/rendered/codex/tapper/skills/tapper/SKILL.md @@ -6,11 +6,20 @@ description: Orient to Tapper flights and operate on KEGs through MCP-first safe # tapper Interact with Tapper KEGs (Knowledge Exchange Graphs) through the native MCP -server. At the start of KEG work, call `mcp__tapper__orient`, identify the -relevant covered KEGs from their titles and summaries, then call -`mcp__tapper__keg_settings` for those KEGs before operating. Treat the active -flight, cover, flight instructions, and targeted KEG instructions as the -authoritative context for the session. +server. + +**Call `mcp__tapper__orient` first, in every session, before doing anything +else — including answering the user.** Do not wait until KEG work looks like it +is starting. The active flight carries the instructions describing what this +session is for, so until you orient you cannot know whether the work is KEG +work, which KEGs you may touch, or what the user actually expects of you. A +message as small as "test" is not a reason to defer: orient, then respond with +that context in hand. + +After orienting, identify the relevant covered KEGs from their titles and +summaries and call `mcp__tapper__keg_settings` for those KEGs before operating +on them. Treat the active flight, cover, flight instructions, and targeted KEG +instructions as the authoritative context for the session. ## Rules @@ -26,6 +35,24 @@ authoritative context for the session. - **Treat the active flight as MCP authority.** It determines the instructions and KEGs available to the agent. `defaultKeg` does not grant authority for an MCP session. +- **Leave node 0 alone.** It is the keg's placeholder landing node, created with + the keg itself. + +## Node 0 + +Every keg has a node `0`. It is not an ordinary node and is not yours to write: + +- It is the **placeholder** a link to unwritten content lands on, so its content + is deliberately generic. +- It carries **no `type`**, on purpose. Do not add one, and do not read its + absence as a defect to repair — a node without a type is normally a schema + error, and node 0 is the documented exception. +- **Removing it breaks the keg.** Tapper treats a missing node 0 as an + uninitialized keg, so deleting it makes every other node unreachable. + +When you have content to write, create a new node. If node 0 genuinely needs to +change — a keg's landing page is a reasonable thing to want — say so and let the +user decide; do not fold it into unrelated work. ## Flight-first orientation @@ -42,11 +69,40 @@ restarting the MCP server. ## Bootstrapping a session -Call `mcp__tapper__orient` first, then load the selected KEG instructions with -`mcp__tapper__keg_settings`. When no flight is selected, the local MCP server connects in a -recovery-only state: KEG tools are locked, while `mcp__tapper__list_flights` -and `mcp__tapper__flight_show` remain available for discovery. Ask the user to -select a flight in Tapper configuration, then call `mcp__tapper__orient` again. +Orientation is unconditional and comes first, before any other tool call and +before your first reply. It is not a lookup step you reach for once KEG work is +identified — it is how the session learns what it is for. Then load the +selected KEG instructions with `mcp__tapper__keg_settings`. + +**Orient again after any context reset**, such as a clear or a compact. The MCP +connection survives those, so the server does not re-initialize and will not +re-send anything on its own — but the flight instructions you were operating +under are gone from your context. Re-orienting is cheap and idempotent, and it +also picks up any configuration change made since you connected. If you cannot +tell whether you have oriented in the current context, you have not; orient. + +**The newest orientation wins.** More than one copy can be present at once: the +connection's startup instructions are captured when the server connects and are +never refreshed afterwards, and a compaction summary may carry a paraphrase of +an earlier orientation. Both can be stale, and a stale copy may sit earlier in +your context than the fresh one. Treat the most recent `mcp__tapper__orient` +result as authoritative and discard the others outright rather than reconciling +them — in particular, a startup copy saying KEG tools are locked is wrong once +a later orientation has returned a flight. When no flight is selected, the MCP +server connects in a recovery-only state: KEG tools are locked, while +`mcp__tapper__list_flights` and `mcp__tapper__flight_show` remain available for +discovery. Ask the user to select a flight in Tapper configuration, then call +`mcp__tapper__orient` again. + +When there is no flight to select — a fresh machine or account — the session +instead starts on a temporary **bootstrap flight**. Its cover is empty, so the +KEG tools stay locked, but `mcp__tapper__keg_create` and the flight mutation +tools are available so you can create the first KEG and the first flight. +Setting that up is the session's work; do it before anything else. You still +cannot *select* a flight — that stays a human action — so hand the setup back +to the user and call `mcp__tapper__orient` again once they confirm. The +orientation payload names exactly where they should do it. + If `mcp__tapper__orient` is unavailable, report that the Tapper MCP connection is unavailable, ask the user to reconnect or restart the host session, and never kill or signal host-owned processes. A flight with an empty cover exposes @@ -169,6 +225,24 @@ Tapper supports two link forms in node bodies: Both forms appear in backlinks. Prefer intra-keg links when the target is in the same keg. +## Attachments + +A node's uploaded files and images live in two directories inside the node's +own directory, so they are linked relative to it — the same base the `../NODEID` +form above counts from: + +- **File:** `[label](./assets/FILE)` — anything uploaded with + `mcp__tapper__upload_file`. +- **Image:** `![alt](./images/IMAGE)` — anything uploaded with + `mcp__tapper__upload_image`. + +**Both directory names are plural**: `assets/` and `images/`, never `asset/` or +`image/`. Uploading succeeds regardless of how you later write the link, so a +singular path fails silently as a broken reference rather than as an error. + +Use `mcp__tapper__list_files` and `mcp__tapper__list_images` to get the exact +stored names; the upload may normalize the filename you supplied. + ## Secret handling - Never store credentials, API tokens, private keys, session cookies, customer diff --git a/pkg/cli/cmd_launch_test.go b/pkg/cli/cmd_launch_test.go index 89f0121..0773871 100644 --- a/pkg/cli/cmd_launch_test.go +++ b/pkg/cli/cmd_launch_test.go @@ -44,8 +44,9 @@ func TestLaunchCommand_DryRunResolvesOllamaThroughOpenAI(t *testing.T) { out := string(res.Stdout) require.Contains(t, out, "agent local -> ollama/qwen3.6:35b-mlx") require.Contains(t, out, "flight: @testuser/+scratch") - require.Contains(t, out, "codex --model qwen3.6:35b-mlx") - require.Contains(t, out, "OPENAI_BASE_URL=http://localhost:11434/v1") + require.Contains(t, out, "codex --oss --local-provider ollama --model qwen3.6:35b-mlx") + require.Contains(t, out, "CODEX_OSS_BASE_URL=http://localhost:11434/v1") + require.Contains(t, out, "TAP_FLIGHT=@testuser/+scratch") } @@ -69,7 +70,7 @@ func TestLaunchCommand_DryRunPassesThroughExtraArgs(t *testing.T) { res := NewProcess(t, false, "launch", "codex", "--agent", "local", "--dry-run", "--", "--sandbox", "read-only").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - require.Contains(t, string(res.Stdout), "codex --model qwen3.6:35b-mlx --sandbox read-only") + require.Contains(t, string(res.Stdout), "codex --oss --local-provider ollama --model qwen3.6:35b-mlx --sandbox read-only") } // Ollama serves the Anthropic Messages API as well, so Claude Code can drive it diff --git a/pkg/integrations/adapters/claude.go b/pkg/integrations/adapters/claude.go index 033c8c0..d8a9f14 100644 --- a/pkg/integrations/adapters/claude.go +++ b/pkg/integrations/adapters/claude.go @@ -110,8 +110,24 @@ func renderClaudeMarketplace() ([]byte, error) { return marshalIndented(v) } +// renderClaudeMCP writes the Claude Code MCP registration. +// +// Deliberately no env allowlist. Claude Code passes its own environment to the +// stdio servers it spawns, so tap resolves the same config, auth store, and +// data directories as the shell that launched it. Codex does not, which is why +// renderCodexMCP carries an explicit env_vars list — the asymmetry is a +// difference between the two hosts, not an oversight here. Adding a list to +// this one would only create something to go stale. func renderClaudeMCP() []byte { - return []byte("{\n \"mcpServers\": {\n \"tapper\": {\n \"command\": \"tap\",\n \"args\": [\"mcp\"]\n }\n }\n}\n") + return []byte(`{ + "mcpServers": { + "tapper": { + "command": "tap", + "args": ["mcp"] + } + } +} +`) } func stripLeadingH1(b []byte) []byte { diff --git a/pkg/integrations/adapters/codex.go b/pkg/integrations/adapters/codex.go index ad8b129..3442323 100644 --- a/pkg/integrations/adapters/codex.go +++ b/pkg/integrations/adapters/codex.go @@ -190,11 +190,40 @@ func renderCodexMarketplace() ([]byte, error) { return marshalIndented(v) } +// renderCodexMCP writes the Codex MCP registration. +// +// Codex hands stdio MCP servers an allowlisted environment rather than its own, +// so every variable tap needs must be named here. Claude Code forwards +// everything and needs no equivalent — see renderClaudeMCP. +// +// HOME is not redundant with the XDG roots. tap falls back to $HOME whenever a +// root is unset, and reaches for it directly when expanding "~" in config +// values and when resolving the default keg root. Forwarding the roots without +// HOME therefore works only where every path in play happens to be XDG-rooted; +// elsewhere tap mcp fails to authenticate while the same tap in the shell +// succeeds, which is precisely how this surfaced in a dev container. +// +// TAP_FLIGHT carries `tap launch --agent` flight selection. Without it the +// harness has the flight but the MCP server it spawns does not, so the session +// silently resolves the configured flight instead of the requested one. func renderCodexMCP() []byte { - // Codex filters the environment inherited by stdio MCP servers. Forward the - // XDG roots so tap resolves the same config, auth store, and data directories - // as the interactive shell that launched Codex (notably in dev containers). - return []byte("{\n \"mcpServers\": {\n \"tapper\": {\n \"command\": \"tap\",\n \"args\": [\"mcp\"],\n \"env_vars\": [\n \"XDG_CONFIG_HOME\",\n \"XDG_DATA_HOME\",\n \"XDG_STATE_HOME\",\n \"XDG_CACHE_HOME\"\n ]\n }\n }\n}\n") + return []byte(`{ + "mcpServers": { + "tapper": { + "command": "tap", + "args": ["mcp"], + "env_vars": [ + "HOME", + "TAP_FLIGHT", + "XDG_CONFIG_HOME", + "XDG_DATA_HOME", + "XDG_STATE_HOME", + "XDG_CACHE_HOME" + ] + } + } +} +`) } func pluginVersion(rt *toolkit.Runtime) string { diff --git a/pkg/integrations/adapters/codex_test.go b/pkg/integrations/adapters/codex_test.go index 1137afe..3196a4c 100644 --- a/pkg/integrations/adapters/codex_test.go +++ b/pkg/integrations/adapters/codex_test.go @@ -114,7 +114,11 @@ func TestCodexAdapter_RendersNativeMarketplaceAndTwoPlugins(t *testing.T) { if tapperMCP.Command != "tap" || strings.Join(tapperMCP.Args, " ") != "mcp" { t.Errorf("unexpected tapper MCP command: %+v", tapperMCP) } - wantEnvVars := "XDG_CONFIG_HOME,XDG_DATA_HOME,XDG_STATE_HOME,XDG_CACHE_HOME" + // HOME must be forwarded alongside the XDG roots: tap falls back to it when a + // root is unset and when expanding "~", so without it tap mcp fails to + // authenticate under Codex while the same tap works in the shell. TAP_FLIGHT + // carries `tap launch --agent` flight selection through to the server. + wantEnvVars := "HOME,TAP_FLIGHT,XDG_CONFIG_HOME,XDG_DATA_HOME,XDG_STATE_HOME,XDG_CACHE_HOME" if got := strings.Join(tapperMCP.EnvVars, ","); got != wantEnvVars { t.Errorf("tapper MCP env_vars = %q, want %q", got, wantEnvVars) } diff --git a/pkg/keg/schema.go b/pkg/keg/schema.go index 192512b..c6888d9 100644 --- a/pkg/keg/schema.go +++ b/pkg/keg/schema.go @@ -820,8 +820,20 @@ func (k *LocalKeg) validateNodeDataWithSchemas(ctx context.Context, id NodeId, n typeName, hasType := nodeType(node) result.Type = typeName if !hasType || strings.TrimSpace(typeName) == "" { - result.Issues = append(result.Issues, ValidationIssue{Level: "error", Field: "meta.type", Message: "missing required type"}) - result.Valid = false + // Node 0 is the keg's placeholder landing node and carries no type by + // design — Init writes it with empty meta. Requiring one made every + // schema-bearing keg permanently invalid, and the standing error told + // agents to fix node 0 the only way the message suggests: by giving it a + // type and content, destroying the placeholder. A node the schema + // contract cannot describe must not be reported as violating it. + // + // Scoped to this one rule. A node 0 that does declare a type is still + // validated against it below, and doctor's other node-0 checks (parse + // errors, broken links, stats) are untouched. + if id.ID != 0 { + result.Issues = append(result.Issues, ValidationIssue{Level: "error", Field: "meta.type", Message: "missing required type"}) + result.Valid = false + } return result, nil } diff --git a/pkg/keg/schema_test.go b/pkg/keg/schema_test.go index 2d9cbf3..efa1fe3 100644 --- a/pkg/keg/schema_test.go +++ b/pkg/keg/schema_test.go @@ -799,3 +799,116 @@ markdown: }) } } + +// TestZeroNodeExemptFromRequiredType covers the placeholder landing node. A keg +// with any schema requires meta.type on every node, but node 0 is created by +// Init with empty meta and can never satisfy that. Reporting it left a standing +// schema error on every schema-bearing keg — and the only fix the message +// suggests is to give node 0 a type and content, destroying the placeholder. +func TestZeroNodeExemptFromRequiredType(t *testing.T) { + f := sandbox.NewSandbox(t, &sandbox.Options{Home: "/home/testuser", User: "testuser"}) + ctx := context.Background() + k := kegpkg.NewLocalKeg(kegpkg.NewMemoryRepo(f.Runtime()), f.Runtime()) + if err := k.Init(ctx); err != nil { + t.Fatalf("Init: %v", err) + } + schema := []byte("type: task\nmeta:\n type: object\n required: [\"type\"]\n") + if err := k.WriteSchema(ctx, "task", schema); err != nil { + t.Fatalf("WriteSchema: %v", err) + } + + zero := kegpkg.NodeId{ID: 0} + result, err := k.ValidateNode(ctx, zero) + if err != nil { + t.Fatalf("ValidateNode zero: %v", err) + } + if !result.Valid || len(result.Issues) != 0 { + t.Fatalf("untyped node 0 must validate clean; result=%#v", result) + } + + // The exemption is one rule on one node, not a hole. A node 0 that declares + // a type is still held to it, on read... + result, err = k.ValidateNodePayload(ctx, kegpkg.NodeValidationPayload{ + ID: zero, Meta: []byte("type: nonexistent\n"), HasMeta: true, + }) + if err != nil { + t.Fatalf("ValidateNodePayload typed zero: %v", err) + } + if result.Valid { + t.Fatalf("node 0 declaring an unknown type must still fail; result=%#v", result) + } + + // ...and on write, where enforcement rejects it outright. + if err := k.SetMeta(ctx, zero, metaWithType(t, ctx, k, zero, "nonexistent")); !errors.Is(err, kegpkg.ErrSchemaInvalid) { + t.Fatalf("SetMeta with unknown type on node 0 = %v, want ErrSchemaInvalid", err) + } + + // And an ordinary node with no type still fails, so the exemption did not + // silently disable the rule for everyone. + id, err := k.Create(kegpkg.WithValidationActor(ctx, kegpkg.ValidationActorHuman), + &kegpkg.CreateOptions{Body: []byte("# Untyped\n")}) + if err != nil { + t.Fatalf("Create untyped: %v", err) + } + result, err = k.ValidateNode(ctx, id.ID) + if err != nil { + t.Fatalf("ValidateNode untyped: %v", err) + } + if result.Valid { + t.Fatalf("untyped non-zero node must still fail; result=%#v", result) + } +} + +func metaWithType(t *testing.T, ctx context.Context, k *kegpkg.LocalKeg, id kegpkg.NodeId, typeName string) *kegpkg.NodeMeta { + t.Helper() + meta, err := k.GetMeta(ctx, id) + if err != nil { + t.Fatalf("GetMeta: %v", err) + } + if err := meta.Set(ctx, "type", typeName); err != nil { + t.Fatalf("meta.Set: %v", err) + } + return meta +} + +// TestZeroNodeDoctorReportsRealProblemsOnly pins the other half: doctor stops +// reporting the schema error but keeps every other check live on node 0. +func TestZeroNodeDoctorReportsRealProblemsOnly(t *testing.T) { + f := sandbox.NewSandbox(t, &sandbox.Options{Home: "/home/testuser", User: "testuser"}) + ctx := context.Background() + k := kegpkg.NewLocalKeg(kegpkg.NewMemoryRepo(f.Runtime()), f.Runtime()) + if err := k.Init(ctx); err != nil { + t.Fatalf("Init: %v", err) + } + if err := k.WriteSchema(ctx, "task", []byte("type: task\nmeta:\n type: object\n")); err != nil { + t.Fatalf("WriteSchema: %v", err) + } + + issues, err := k.Doctor(ctx) + if err != nil { + t.Fatalf("Doctor: %v", err) + } + for _, issue := range issues { + if issue.NodeID == "0" && issue.Kind == "schema" { + t.Fatalf("doctor still reports a schema issue on node 0: %+v", issue) + } + } + + // A genuine node-0 defect must still surface. + if err := k.SetContent(ctx, kegpkg.NodeId{ID: 0}, []byte("# Zero\n\nLead.\n\n[gone](../4242)\n")); err != nil { + t.Fatalf("SetContent zero: %v", err) + } + issues, err = k.Doctor(ctx) + if err != nil { + t.Fatalf("Doctor after edit: %v", err) + } + var sawBrokenLink bool + for _, issue := range issues { + if issue.NodeID == "0" && issue.Kind == "broken-link" { + sawBrokenLink = true + } + } + if !sawBrokenLink { + t.Fatalf("doctor must still check node 0 for real defects; issues=%+v", issues) + } +} diff --git a/pkg/mcp/flight_authority_validation_test.go b/pkg/mcp/flight_authority_validation_test.go new file mode 100644 index 0000000..0fe0efd --- /dev/null +++ b/pkg/mcp/flight_authority_validation_test.go @@ -0,0 +1,222 @@ +package mcp_test + +import ( + "context" + "testing" + + sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/require" + + "github.com/jlrickert/tapper/pkg/mcp" + "github.com/jlrickert/tapper/pkg/tapper" +) + +// The tests in this file validate the four session-authority contracts that +// `tap mcp` (stdio, config-driven) and the hosted HTTP endpoint +// (provider-driven) must both honour: +// +// 1. Both transports gate KEG work through the same session flight gate. +// 2. orient reads the current flight and refreshes it on every call. +// 3. Any denied KEG operation reports a permission error that sends the agent +// back to orient, because the usual cause is a flight edited mid-session. +// 4. A flight holding manage_flights can edit itself, and the edit governs the +// very next call without a reconnect. + +// restrictionNudge is the recovery instruction every cover and role-cap denial +// must carry. recoveryNudge is its counterpart for a session with no flight at +// all, which is worded for a reader who has nothing to refresh yet. +const ( + restrictionNudge = "Call `orient` to refresh this session's flight authority" + recoveryNudge = "then orient again" +) + +// newValidationSession builds a provider-driven session (the hosted shape) over +// a sandbox holding two real local kegs, so widening a cover mid-session can be +// observed against a keg that actually exists. +func newValidationSession(t *testing.T) (*sdkmcp.ClientSession, context.Context, *fakeSessionBackend) { + t.Helper() + ctx := context.Background() + sb := newTestSandbox(t) + rt := sb.Runtime() + + tap, err := tapper.NewTap(tapper.TapOptions{Runtime: rt}) + require.NoError(t, err) + _, err = tap.InitKeg(ctx, tapper.InitOptions{Keg: "other", Namespace: "local"}) + require.NoError(t, err) + + provider := newFakeSessionBackend() + srv := mcp.NewServer(tap, "test", mcp.KegDefaults{}, mcp.ServerOptions{ + OrientationProvider: provider, FlightProvider: provider, + KegProvider: provider, IdentityProvider: provider, + }) + return connectFlightSession(t, ctx, srv, nil), ctx, provider +} + +func callCatKeg(t *testing.T, ctx context.Context, session *sdkmcp.ClientSession, keg string) *sdkmcp.CallToolResult { + t.Helper() + result, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "cat", + Arguments: map[string]any{ + "keg": keg, + "node_ids": []string{"0"}, + "content_only": true, + }, + }) + require.NoError(t, err) + return result +} + +// --- requirement 3: denials send the agent back to orient ------------------ + +func TestMCP_UncoveredKegDenialNudgesReorientation_LocalSurface(t *testing.T) { + t.Parallel() + session, ctx, privateID := newFlightLockedSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "cat", + Arguments: map[string]any{ + "keg": "private", "node_ids": []string{privateID}, "content_only": true, + }, + }) + require.NoError(t, err) + require.True(t, res.IsError) + text := extractText(t, res) + require.Contains(t, text, `keg "@local/private" is not available in flight`) + require.Contains(t, text, restrictionNudge, + "a cover denial must send the agent back to orient; the flight may have changed mid-session") +} + +func TestMCP_UncoveredKegDenialNudgesReorientation_ProviderSurface(t *testing.T) { + t.Parallel() + session, ctx, _ := newValidationSession(t) + + res := callCatKeg(t, ctx, session, "@local/other") + require.True(t, res.IsError) + text := extractText(t, res) + require.Contains(t, text, `keg "@local/other" is not available in flight`) + require.Contains(t, text, restrictionNudge, + "the hosted surface must nudge re-orientation exactly like the stdio surface") +} + +func TestMCP_RoleCapDenialNudgesReorientation(t *testing.T) { + t.Parallel() + // +focused covers @local/personal at viewer, so a write is refused on the + // role cap rather than on cover membership. + session, ctx, _ := newFlightLockedSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "create", + Arguments: map[string]any{"keg": "personal", "title": "Blocked by role cap"}, + }) + require.NoError(t, err) + require.True(t, res.IsError) + text := extractText(t, res) + require.Contains(t, text, "viewer-only") + require.Contains(t, text, restrictionNudge, "a role-cap denial must nudge re-orientation too") +} + +// TestMCP_MidSessionFlightWideningDeniesUntilReorient is the scenario the nudge +// exists for: the flight gained a keg after this session pinned its authority, +// so the next call is denied even though the stored flight now permits it, and +// only orient repairs the session. +func TestMCP_MidSessionFlightWideningDeniesUntilReorient(t *testing.T) { + t.Parallel() + session, ctx, provider := newValidationSession(t) + + require.False(t, callCatKeg(t, ctx, session, "@local/personal").IsError) + require.True(t, callCatKeg(t, ctx, session, "@local/other").IsError) + + // Someone else widens the flight while this session is live. + cover := []tapper.FlightCover{ + {Namespace: "local", Keg: "personal", Role: tapper.FlightRoleEditor}, + {Namespace: "local", Keg: "other", Role: tapper.FlightRoleEditor}, + } + _, err := provider.UpdateFlight(ctx, tapper.UpdateFlightOptions{Ref: "+active", Cover: &cover}) + require.NoError(t, err) + + denied := callCatKeg(t, ctx, session, "@local/other") + require.True(t, denied.IsError, "the pinned snapshot still governs until orient") + require.Contains(t, extractText(t, denied), restrictionNudge) + + callOrient(t, ctx, session) + require.False(t, callCatKeg(t, ctx, session, "@local/other").IsError, + "orient must adopt the widened cover on the same connection") +} + +// --- requirement 4: manage_flights can modify its own flight --------------- + +func TestMCP_SelfEditWideningOwnCoverTakesEffectImmediately(t *testing.T) { + t.Parallel() + session, ctx, _ := newValidationSession(t) + require.True(t, callCatKeg(t, ctx, session, "@local/other").IsError) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{Name: "flight_edit", Arguments: map[string]any{ + "ref": "+active", + "cover": []string{"@local/personal=editor", "@local/other=editor"}, + }}) + require.NoError(t, err) + require.False(t, res.IsError, extractText(t, res)) + + require.False(t, callCatKeg(t, ctx, session, "@local/other").IsError, + "a manage_flights self-edit must govern the next call without orient") + require.False(t, callCatKeg(t, ctx, session, "@local/personal").IsError) +} + +func TestMCP_WithoutManageFlightsSelfEditIsHiddenAndRefused(t *testing.T) { + t.Parallel() + session, ctx, provider := newValidationSession(t) + + provider.mu.Lock() + provider.active = "@local/+other" // +other carries no capabilities + provider.mu.Unlock() + callOrient(t, ctx, session) + + listed := listedToolNames(t, ctx, session) + require.NotContains(t, listed, "flight_create") + require.NotContains(t, listed, "flight_edit") + require.NotContains(t, listed, "flight_delete") + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{Name: "flight_edit", Arguments: map[string]any{ + "ref": "+other", "instructions": "should not apply", + }}) + require.NoError(t, err) + require.True(t, res.IsError) + require.Contains(t, extractText(t, res), "manage_flights") + + stored, err := provider.GetFlight(ctx, "+other") + require.NoError(t, err) + require.Equal(t, "other", stored.Instructions, "a refused self-edit must not persist") +} + +// --- requirements 1 + 2: both transports gate and recover identically ------ + +// TestMCP_BothSurfacesEnterTheSameRecoveryMode drives the config-driven and the +// provider-driven servers into a flight-less state by their own transport's +// mechanism — an emptied configuration versus a cleared account preference — +// and asserts they converge on one recovery contract. +func TestMCP_BothSurfacesEnterTheSameRecoveryMode(t *testing.T) { + recoveryTools := []string{"orient", "list_flights", "flight_show", "auth_info"} + + providerSession, providerCtx, provider := newValidationSession(t) + provider.mu.Lock() + provider.active = "" + provider.mu.Unlock() + require.Contains(t, callOrient(t, providerCtx, providerSession), "No KEGs are currently available") + require.ElementsMatch(t, recoveryTools, listedToolNames(t, providerCtx, providerSession)) + hosted := callCatKeg(t, providerCtx, providerSession, "@local/personal") + + localCtx, srv, rt := newOrientationServer(t, "") + localSession := connectFlightSession(t, localCtx, srv, nil) + writeProjectFlight(t, rt, "") + writeUserFlight(t, rt, "") + require.Contains(t, callOrient(t, localCtx, localSession), "No KEGs are currently available") + require.ElementsMatch(t, recoveryTools, listedToolNames(t, localCtx, localSession)) + local := callCat(t, localCtx, localSession) + + require.True(t, hosted.IsError) + require.True(t, local.IsError) + require.Equal(t, extractText(t, local), extractText(t, hosted), + "a flight-less session must read the same on both transports") + require.Contains(t, extractText(t, local), "no flight is selected") + require.Contains(t, extractText(t, local), recoveryNudge) +} diff --git a/pkg/mcp/providers.go b/pkg/mcp/providers.go index d9c4790..baf162b 100644 --- a/pkg/mcp/providers.go +++ b/pkg/mcp/providers.go @@ -47,12 +47,19 @@ type FlightProvider interface { DeleteFlight(context.Context, tapper.DeleteFlightOptions) error } -// KegDiscoveryProvider reports the kegs an identity can reach. +// KegDiscoveryProvider reports the kegs an identity can reach and creates new +// ones. Creation lives here rather than on a keg-agnostic surface because both +// operations answer to the same authenticated catalog. type KegDiscoveryProvider interface { // ListKegs returns every identity-authorized canonical keg ref. MCP applies // the immutable active-flight cover before releasing results, so // implementations do not filter by flight themselves. ListKegs(context.Context) ([]string, error) + // CreateKeg provisions a keg and returns its canonical @namespace/keg ref. + // The MCP gate has already checked the flight's manage_kegs capability; + // implementations apply their own transport's identity authorization, which + // the capability never substitutes for. + CreateKeg(context.Context, tapper.CreateKegOptions) (string, error) } // AuthIdentity is deliberately credential-free. Do not add token, email, @@ -79,6 +86,38 @@ type localOrientationProvider struct { staticFlight string } +// localBootstrapInstructions is the stdio half of the bootstrap nudge. `tap +// mcp` selects its flight from configuration the user owns, so recovery names +// config paths and CLI commands. skipped carries any hub that discovery could +// not reach: an unreachable hub is the most common reason a machine that does +// have flights reports none, and the reader cannot tell those apart otherwise. +func localBootstrapInstructions(skipped []string) string { + var b strings.Builder + b.WriteString("No flight is configured for this machine.\n\n") + if len(skipped) > 0 { + b.WriteString("Some hubs were skipped during discovery, so flights may exist that this\n") + b.WriteString("session cannot see. Resolve these before creating anything new:\n\n") + for _, warning := range skipped { + b.WriteString("- " + warning + "\n") + } + b.WriteString("\n") + } + b.WriteString("To set this session up, ask the user to:\n\n") + b.WriteString("1. Run `tap bootstrap` if they have never configured tapper on this machine.\n") + b.WriteString("2. Create a flight — either a local manifest at `~/kegs/flights.d/.yaml`,\n") + b.WriteString(" or `tap flight create @/+` against a hub they are logged\n") + b.WriteString(" in to. `flight_create` here only works against a remote hub; local\n") + b.WriteString(" manifests must be written by hand.\n") + b.WriteString("3. Select it by setting `flight: +` in `~/.config/tapper/config.yaml`\n") + b.WriteString(" (or the project's `.tapper/config.yaml`), exporting `TAP_FLIGHT=+`,\n") + b.WriteString(" or passing `tap mcp --flight +`.\n") + b.WriteString("4. Tell you when that is done, so you can call `orient` again on this same\n") + b.WriteString(" connection. Flights are selected outside MCP; you cannot select one yourself.\n\n") + b.WriteString("`keg_create` works now if the user wants a KEG created first, but a KEG is\n") + b.WriteString("unreadable until a flight's cover names it.\n") + return b.String() +} + func (p *localOrientationProvider) Load(ctx context.Context) (*Orientation, error) { if p.tap == nil || p.tap.ConfigService == nil || p.tap.FlightService == nil { return nil, errors.New("Tapper flight service is unavailable") @@ -94,8 +133,16 @@ func (p *localOrientationProvider) Load(ctx context.Context) (*Orientation, erro ref = p.tap.ActiveFlightName("") } if strings.TrimSpace(ref) == "" { - payload, err := tapper.BuildOrientationPayload(nil, "", nil, nil) - return &Orientation{Payload: payload}, err + // Nothing is selected. Whether the user can recover by picking one + // depends on whether anything exists to pick, so ask before choosing + // which of the two no-flight modes this session enters. + var warnings []string + flights, listErr := p.tap.ListFlights(ctx, tapper.ListFlightsOptions{Warnings: &warnings}) + if listErr == nil && len(flights) == 0 { + return p.Render(ctx, tapper.BootstrapFlight("", localBootstrapInstructions(warnings))) + } + payload, payloadErr := tapper.BuildOrientationPayload(nil, "", nil, warnings) + return &Orientation{Payload: payload, Warnings: warnings}, payloadErr } flight, err := p.tap.FlightService.GetFlightFresh(ctx, ref) if err != nil { @@ -136,6 +183,27 @@ func (p localKegDiscoveryProvider) ListKegs(ctx context.Context) ([]string, erro return p.tap.HubListKegs(ctx, tapper.HubListOptions{}) } +func (p localKegDiscoveryProvider) CreateKeg(ctx context.Context, opts tapper.CreateKegOptions) (string, error) { + target, err := p.tap.InitKeg(ctx, tapper.InitOptions{ + Keg: opts.Keg, + Namespace: opts.Namespace, + Title: opts.Title, + // MCP never prompts, and a config-driven create requires `tap bootstrap` + // exactly as keg resolution does (see resolveKegTarget). + NonInteractive: true, + RequireBootstrap: true, + }) + if err != nil { + return "", err + } + // Visibility is a hub concept; a filesystem keg has no such column, so it is + // silently unused here rather than rejected. + if ref := tapper.CanonicalKegRef(target); ref != "" { + return ref, nil + } + return opts.Keg, nil +} + type localIdentityProvider struct{ tap *tapper.Tap } func (p localIdentityProvider) Identities(ctx context.Context) ([]AuthIdentity, error) { diff --git a/pkg/mcp/session_bootstrap_test.go b/pkg/mcp/session_bootstrap_test.go new file mode 100644 index 0000000..42d3f3c --- /dev/null +++ b/pkg/mcp/session_bootstrap_test.go @@ -0,0 +1,186 @@ +package mcp_test + +import ( + "context" + "strings" + "testing" + + "github.com/jlrickert/cli-toolkit/toolkit" + sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/require" + + "github.com/jlrickert/tapper/pkg/mcp" + "github.com/jlrickert/tapper/pkg/tapper" +) + +// A session that can reach no flights at all runs on the synthetic bootstrap +// flight instead of the select-a-flight recovery mode: telling a user to pick +// from an empty list is a dead end, so the session instead carries the +// authority to create the first flight and the first KEG. + +// bootstrapTools is every tool a bootstrap session may see. The bootstrap +// flight carries both manage_flights and manage_kegs, so nothing is filtered +// out by capability on top of the allowlist. +var bootstrapTools = []string{ + "orient", "list_flights", "flight_show", "auth_info", + "flight_create", "flight_edit", "flight_delete", "keg_create", +} + +// flightSection returns the "## Flight" block of an orientation payload, which +// is where the session declares its own mode. Assertions must scope to it: the +// canonical guidance appended to every payload also describes bootstrap and +// recovery, so a whole-payload substring check passes in every mode. +func flightSection(t *testing.T, payload string) string { + t.Helper() + _, rest, ok := strings.Cut(payload, "## Flight\n") + require.True(t, ok, "payload has no Flight section: %q", payload) + section, _, _ := strings.Cut(rest, "## Guidance") + return section +} + +// newBootstrapSession builds the stdio surface over a configured but +// flight-less machine: the hub's basePath points at a directory with no +// flights.d, so discovery legitimately reports zero flights. +func newBootstrapSession(t *testing.T) (*sdkmcp.ClientSession, context.Context, *toolkit.Runtime) { + t.Helper() + ctx := context.Background() + sb := newTestSandbox(t) + require.NoError(t, sb.Setwd("/home/testuser")) + rt := sb.Runtime() + sb.MustWriteFile("~/.config/tapper/config.yaml", []byte(`defaultKeg: personal +fallbackNamespace: local +hubs: + home: + kind: local + defaultNamespace: local + basePath: ~/empty-kegs +`), 0o644) + + tap, err := tapper.NewTap(tapper.TapOptions{Runtime: rt}) + require.NoError(t, err) + srv := mcp.NewServer(tap, "test", mcp.KegDefaults{}) + return connectFlightSession(t, ctx, srv, nil), ctx, rt +} + +func TestMCP_NoFlightsAnywhereEntersBootstrapMode(t *testing.T) { + t.Parallel() + session, ctx, _ := newBootstrapSession(t) + + payload := session.InitializeResult().Instructions + flight := flightSection(t, payload) + require.Contains(t, flight, "temporary bootstrap flight") + require.NotContains(t, flight, "recovery mode", + "bootstrap must not present itself as the select-a-flight recovery mode") + require.Contains(t, flight, "tap bootstrap") + require.Contains(t, flight, "TAP_FLIGHT", + "the stdio surface must nudge toward configuration, not a web UI") + require.Equal(t, payload, callOrient(t, ctx, session), "orient is idempotent in bootstrap") + + require.ElementsMatch(t, bootstrapTools, listedToolNames(t, ctx, session)) + + denied := callCatKeg(t, ctx, session, "@local/personal") + require.True(t, denied.IsError, "an empty cover still denies every KEG") + require.Contains(t, extractText(t, denied), "bootstrap flight") +} + +// TestMCP_FlightsExistButUnselectedStaysInSelectMode guards the boundary +// between the two no-flight modes: a machine that has flights must keep asking +// the user to pick one rather than handing the agent admin authority. +func TestMCP_FlightsExistButUnselectedStaysInSelectMode(t *testing.T) { + ctx, srv, rt := newOrientationServer(t, "") + session := connectFlightSession(t, ctx, srv, nil) + writeProjectFlight(t, rt, "") + writeUserFlight(t, rt, "") + + flight := flightSection(t, callOrient(t, ctx, session)) + require.Contains(t, flight, "recovery mode") + require.NotContains(t, flight, "bootstrap flight") + require.ElementsMatch(t, + []string{"orient", "list_flights", "flight_show", "auth_info"}, + listedToolNames(t, ctx, session)) +} + +// TestMCP_BootstrapCreatesFirstKegThenAdoptsItsFlight walks the whole recovery +// the bootstrap instructions describe: create the KEG over MCP, have the user +// write and select a flight covering it, then orient into a working session. +func TestMCP_BootstrapCreatesFirstKegThenAdoptsItsFlight(t *testing.T) { + t.Parallel() + session, ctx, rt := newBootstrapSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{Name: "keg_create", Arguments: map[string]any{ + "keg": "first", "namespace": "local", "title": "First KEG", + }}) + require.NoError(t, err) + require.False(t, res.IsError, extractText(t, res)) + require.Contains(t, extractText(t, res), "@local/first") + + require.True(t, callCatKeg(t, ctx, session, "@local/first").IsError, + "creating a KEG does not add it to the active flight's cover") + + // The user does the part MCP deliberately cannot: write a flight and select it. + require.NoError(t, rt.AtomicWriteFile("/home/testuser/empty-kegs/flights.d/first.yaml", + []byte("title: First\ncover:\n - namespace: local\n keg: first\n role: editor\n"), 0o644)) + require.NoError(t, rt.AtomicWriteFile("/home/testuser/.config/tapper/config.yaml", + []byte("defaultKeg: personal\nfallbackNamespace: local\nflight: +first\nhubs:\n home:\n kind: local\n defaultNamespace: local\n basePath: ~/empty-kegs\n"), 0o644)) + + flight := flightSection(t, callOrient(t, ctx, session)) + require.Contains(t, flight, "+first") + require.NotContains(t, flight, "temporary bootstrap flight") + require.False(t, callCatKeg(t, ctx, session, "@local/first").IsError, + "orient must adopt the flight the user just selected") + require.NotContains(t, listedToolNames(t, ctx, session), "keg_create", + "a real flight without manage_kegs does not inherit bootstrap's authority") +} + +// TestMCP_LocalFlightCreateReportsNotImplemented pins the honest failure for +// the one thing bootstrap cannot do on a local-only machine. +func TestMCP_LocalFlightCreateReportsNotImplemented(t *testing.T) { + t.Parallel() + session, ctx, _ := newBootstrapSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{Name: "flight_create", Arguments: map[string]any{ + "ref": "@local/+attempt", "cover": []string{"@local/first=editor"}, + }}) + require.NoError(t, err) + require.True(t, res.IsError) + text := extractText(t, res) + require.Contains(t, text, "not implemented for local hubs") + require.Contains(t, text, "flights.d/attempt.yaml", + "the refusal must name the manifest the user should write instead") +} + +func TestMCP_KegCreateRequiresManageKegs(t *testing.T) { + t.Parallel() + session, ctx, provider := newValidationSession(t) + + // +active grants manage_flights only. + require.NotContains(t, listedToolNames(t, ctx, session), "keg_create") + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{Name: "keg_create", Arguments: map[string]any{ + "keg": "blocked", "namespace": "local", + }}) + require.NoError(t, err) + require.True(t, res.IsError) + require.Contains(t, extractText(t, res), "manage_kegs") + + provider.mu.Lock() + require.Empty(t, provider.createdKegs, "a refused keg_create must not reach the provider") + provider.mu.Unlock() + + capabilities := []tapper.FlightCapability{ + tapper.FlightCapabilityManageFlights, tapper.FlightCapabilityManageKegs, + } + _, err = provider.UpdateFlight(ctx, tapper.UpdateFlightOptions{Ref: "+active", Capabilities: &capabilities}) + require.NoError(t, err) + callOrient(t, ctx, session) + + require.Contains(t, listedToolNames(t, ctx, session), "keg_create") + res, err = session.CallTool(ctx, &sdkmcp.CallToolParams{Name: "keg_create", Arguments: map[string]any{ + "keg": "allowed", "namespace": "local", + }}) + require.NoError(t, err) + require.False(t, res.IsError, extractText(t, res)) + + provider.mu.Lock() + require.Equal(t, []string{"@local/allowed"}, provider.createdKegs) + provider.mu.Unlock() +} diff --git a/pkg/mcp/session_flight.go b/pkg/mcp/session_flight.go index cf40b59..66168e1 100644 --- a/pkg/mcp/session_flight.go +++ b/pkg/mcp/session_flight.go @@ -34,6 +34,38 @@ var recoveryToolNames = map[string]bool{ "auth_info": true, } +// bootstrapToolNames is what a session running on the synthetic bootstrap +// flight may call. Its cover is empty, so the KEG tools would fail anyway; +// hiding them keeps the agent from spending the session discovering that one +// refusal at a time. Like recoveryToolNames this is an allowlist, so a KEG tool +// added later is hidden by default rather than leaking into bootstrap. +var bootstrapToolNames = map[string]bool{ + "orient": true, + "list_flights": true, + "flight_show": true, + "auth_info": true, + "flight_create": true, + "flight_edit": true, + "flight_delete": true, + "keg_create": true, +} + +// sessionMode is the authority state of one MCP session. +type sessionMode int + +const ( + // modeActive: a real flight governs the session. + modeActive sessionMode = iota + // modeSelect: no flight is selected but flights exist to select. The agent + // cannot fix this itself, so only the recovery tools are offered. + modeSelect + // modeBootstrap: no flight exists at all. A synthetic admin flight governs + // the session so the agent can create the first flight and keg. + modeBootstrap +) + +var errMCPBootstrapOnly = errors.New("no flight is configured; this session is running on a temporary bootstrap flight and the KEG tools are locked. Create the first flight and KEG with `flight_create` and `keg_create`, ask the user to select the flight, then call `orient` again") + type flightSessionContextKey struct{} // orientationContext is immutable after publication. Tool calls capture its @@ -44,7 +76,7 @@ type orientationContext struct { payload string kegs []tapper.OrientationKeg warnings []string - recovery bool + mode sessionMode } type flightSessionState struct { @@ -100,7 +132,7 @@ func (g *sessionFlightGate) refresh(ctx context.Context, sessionID string) (*ori return current, err } // Initialization must remain connectable for recovery. - recovery := &orientationContext{payload: failedOrientationPayload(err), recovery: true, warnings: []string{err.Error()}} + recovery := &orientationContext{payload: failedOrientationPayload(err), mode: modeSelect, warnings: []string{err.Error()}} state := g.state(sessionID) state.mu.Lock() state.current = recovery @@ -115,12 +147,26 @@ func (g *sessionFlightGate) refresh(ctx context.Context, sessionID string) (*ori payload: candidate.Payload, kegs: append([]tapper.OrientationKeg(nil), candidate.Kegs...), warnings: append([]string(nil), candidate.Warnings...), - recovery: candidate.Flight == nil, + mode: modeFor(candidate.Flight), } g.publish(sessionID, next) return next, nil } +// modeFor classifies a loaded candidate. The provider decides *whether* to +// synthesize a bootstrap flight — it is the only layer that knows how to count +// its transport's flights — and the gate reads that decision off the manifest. +func modeFor(flight *tapper.Flight) sessionMode { + switch { + case flight == nil: + return modeSelect + case flight.Bootstrap: + return modeBootstrap + default: + return modeActive + } +} + func (g *sessionFlightGate) publish(sessionID string, next *orientationContext) { state := g.state(sessionID) state.mu.Lock() @@ -151,9 +197,38 @@ func (g *sessionFlightGate) notifyToolsChanged() { g.srv.RemoveTools("_orientation_transition_marker") } -func (g *sessionFlightGate) recoveryOnly(sessionID string) bool { +// mode reports the session's authority state. An unseen session is treated as +// modeSelect: nothing has been published for it, so it has no flight and no +// evidence that creating one would help. +func (g *sessionFlightGate) mode(sessionID string) sessionMode { current := g.current(sessionID) - return current == nil || current.recovery + if current == nil { + return modeSelect + } + return current.mode +} + +// allowedTools returns the allowlist governing sessionID, or nil when every +// registered tool is available. +func (g *sessionFlightGate) allowedTools(sessionID string) map[string]bool { + switch g.mode(sessionID) { + case modeSelect: + return recoveryToolNames + case modeBootstrap: + return bootstrapToolNames + default: + return nil + } +} + +// lockedError explains why a tool outside the allowlist was refused. The two +// modes need different text: one asks the reader to pick an existing flight, +// the other to create the first one. +func (g *sessionFlightGate) lockedError(sessionID string) error { + if g.mode(sessionID) == modeBootstrap { + return errMCPBootstrapOnly + } + return errMCPFlightRequired } func (g *sessionFlightGate) activeFlight(ctx context.Context) *tapper.Flight { @@ -183,13 +258,26 @@ func (g *sessionFlightGate) canManage(sessionID string) bool { return current != nil && current.flight != nil && current.flight.HasCapability(tapper.FlightCapabilityManageFlights) } +func (g *sessionFlightGate) canManageKegs(sessionID string) bool { + current := g.current(sessionID) + return current != nil && current.flight != nil && current.flight.HasCapability(tapper.FlightCapabilityManageKegs) +} + func (g *sessionFlightGate) authorizeMutation(sessionID string) error { + return g.authorizeCapability(sessionID, tapper.FlightCapabilityManageFlights) +} + +func (g *sessionFlightGate) authorizeKegCreation(sessionID string) error { + return g.authorizeCapability(sessionID, tapper.FlightCapabilityManageKegs) +} + +func (g *sessionFlightGate) authorizeCapability(sessionID string, capability tapper.FlightCapability) error { current := g.current(sessionID) if current == nil || current.flight == nil { return errMCPFlightRequired } - if !current.flight.HasCapability(tapper.FlightCapabilityManageFlights) { - return errors.New("active flight does not grant manage_flights") + if !current.flight.HasCapability(capability) { + return fmt.Errorf("active flight does not grant %s", capability) } return nil } @@ -220,7 +308,7 @@ func (g *sessionFlightGate) adoptEditedFlight(ctx context.Context, target string if renderErr != nil { warning := "flight update was applied, but orientation refresh failed: " + renderErr.Error() g.publish(sessionIDFromContext(ctx), &orientationContext{ - payload: errMCPFlightRequired.Error(), warnings: []string{warning}, recovery: true, + payload: errMCPFlightRequired.Error(), warnings: []string{warning}, mode: modeSelect, }) return true, errors.New(warning) } @@ -231,7 +319,7 @@ func (g *sessionFlightGate) adoptEditedFlight(ctx context.Context, target string flight: cloneFlight(flight), payload: candidate.Payload, kegs: append([]tapper.OrientationKeg(nil), candidate.Kegs...), warnings: append([]string(nil), candidate.Warnings...), - recovery: false, + mode: modeFor(flight), } g.publish(sessionIDFromContext(ctx), next) return true, nil @@ -248,7 +336,7 @@ func (g *sessionFlightGate) adoptDeletedFlight(ctx context.Context, target strin if payloadErr != nil { payload = errMCPFlightRequired.Error() } - g.publish(sessionIDFromContext(ctx), &orientationContext{payload: payload, recovery: true}) + g.publish(sessionIDFromContext(ctx), &orientationContext{payload: payload, mode: modeSelect}) return true, nil } @@ -340,19 +428,18 @@ func (g *sessionFlightGate) middleware(next sdkmcp.MethodHandler) sdkmcp.MethodH } copyResult := *listed copyResult.Tools = make([]*sdkmcp.Tool, 0, len(listed.Tools)) - if g.recoveryOnly(sessionID) { - for _, tool := range listed.Tools { - if recoveryToolNames[tool.Name] { - copyResult.Tools = append(copyResult.Tools, tool) - } - } - return ©Result, nil - } - canManage := g.canManage(sessionID) + allowed := g.allowedTools(sessionID) + canManage, canManageKegs := g.canManage(sessionID), g.canManageKegs(sessionID) for _, tool := range listed.Tools { + if allowed != nil && !allowed[tool.Name] { + continue + } if isFlightMutationTool(tool.Name) && !canManage { continue } + if isKegCreationTool(tool.Name) && !canManageKegs { + continue + } copyResult.Tools = append(copyResult.Tools, tool) } return ©Result, nil @@ -362,8 +449,8 @@ func (g *sessionFlightGate) middleware(next sdkmcp.MethodHandler) sdkmcp.MethodH if params != nil && params.Name == "orient" { return next(ctx, method, req) } - if params != nil && g.recoveryOnly(sessionID) && !recoveryToolNames[params.Name] { - return errorResult(errMCPFlightRequired), nil + if allowed := g.allowedTools(sessionID); params != nil && allowed != nil && !allowed[params.Name] { + return errorResult(g.lockedError(sessionID)), nil } if params != nil && isFlightMutationTool(params.Name) { if err := g.authorizeMutation(sessionID); err != nil { @@ -371,6 +458,12 @@ func (g *sessionFlightGate) middleware(next sdkmcp.MethodHandler) sdkmcp.MethodH } return next(ctx, method, req) } + if params != nil && isKegCreationTool(params.Name) { + if err := g.authorizeKegCreation(sessionID); err != nil { + return errorResult(err), nil + } + return next(ctx, method, req) + } g.calls.RLock() defer g.calls.RUnlock() } @@ -380,8 +473,10 @@ func (g *sessionFlightGate) middleware(next sdkmcp.MethodHandler) sdkmcp.MethodH } g.calls.RLock() defer g.calls.RUnlock() - if g.recoveryOnly(sessionID) { - return nil, errMCPFlightRequired + // Node resources are KEG reads, so bootstrap locks them exactly as + // it locks the KEG tools. + if g.mode(sessionID) != modeActive { + return nil, g.lockedError(sessionID) } } return next(ctx, method, req) @@ -391,3 +486,7 @@ func (g *sessionFlightGate) middleware(next sdkmcp.MethodHandler) sdkmcp.MethodH func isFlightMutationTool(name string) bool { return name == "flight_create" || name == "flight_edit" || name == "flight_delete" } + +func isKegCreationTool(name string) bool { + return name == "keg_create" +} diff --git a/pkg/mcp/session_transition_test.go b/pkg/mcp/session_transition_test.go index bae7c07..3ff4ac4 100644 --- a/pkg/mcp/session_transition_test.go +++ b/pkg/mcp/session_transition_test.go @@ -20,12 +20,13 @@ import ( ) type fakeSessionBackend struct { - mu sync.Mutex - flights map[string]*tapper.Flight - active string - renderErr error - listEnter chan struct{} - listWait chan struct{} + mu sync.Mutex + flights map[string]*tapper.Flight + active string + createdKegs []string + renderErr error + listEnter chan struct{} + listWait chan struct{} } func newFakeSessionBackend() *fakeSessionBackend { @@ -177,6 +178,18 @@ func (p *fakeSessionBackend) ListKegs(context.Context) ([]string, error) { return []string{"@local/personal", "@local/other"}, nil } +func (p *fakeSessionBackend) CreateKeg(_ context.Context, opts tapper.CreateKegOptions) (string, error) { + p.mu.Lock() + defer p.mu.Unlock() + ns := opts.Namespace + if ns == "" { + ns = "local" + } + ref := "@" + ns + "/" + opts.Keg + p.createdKegs = append(p.createdKegs, ref) + return ref, nil +} + func (p *fakeSessionBackend) Identities(context.Context) ([]mcp.AuthIdentity, error) { return []mcp.AuthIdentity{{Hub: "test", UserID: 1, Username: "tester", DefaultNamespace: "local", Namespaces: []string{"local"}}}, nil } diff --git a/pkg/mcp/tools_files.go b/pkg/mcp/tools_files.go index f910dce..ed329ed 100644 --- a/pkg/mcp/tools_files.go +++ b/pkg/mcp/tools_files.go @@ -212,10 +212,19 @@ func registerLocalUploadFile(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDe }) } +// linkHint tells the caller how to reference what it just uploaded. It lives on +// the tool rather than only in orientation because the moment of upload is when +// the reference is written, and a tool description is re-delivered with every +// tools/list — so it survives a context reset that discards orientation. +const ( + uploadFileLinkHint = " Link it from the node body as [label](./assets/FILENAME) — the directory is plural." + uploadImageLinkHint = " Link it from the node body as ![alt](./images/FILENAME) — the directory is plural." +) + func uploadFileTool(description string) *sdkmcp.Tool { return &sdkmcp.Tool{ Name: "upload_file", - Description: description, + Description: description + uploadFileLinkHint, Annotations: &sdkmcp.ToolAnnotations{ DestructiveHint: boolPtr(false), OpenWorldHint: boolPtr(false), @@ -345,7 +354,7 @@ func registerLocalUploadImage(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegD func uploadImageTool(description string) *sdkmcp.Tool { return &sdkmcp.Tool{ Name: "upload_image", - Description: description, + Description: description + uploadImageLinkHint, Annotations: &sdkmcp.ToolAnnotations{ DestructiveHint: boolPtr(false), OpenWorldHint: boolPtr(false), diff --git a/pkg/mcp/tools_keg.go b/pkg/mcp/tools_keg.go index d293fd4..537c598 100644 --- a/pkg/mcp/tools_keg.go +++ b/pkg/mcp/tools_keg.go @@ -12,10 +12,18 @@ import ( type kegListInput struct{} +type kegCreateInput struct { + Keg string `json:"keg" jsonschema:"alias for the new keg (1-64 lowercase letters, digits, or hyphens)"` + Namespace string `json:"namespace,omitempty" jsonschema:"target namespace without the @ sigil; empty uses the session default"` + Title string `json:"title,omitempty" jsonschema:"human-readable keg title"` + Visibility string `json:"visibility,omitempty" jsonschema:"keg visibility: private (default) or public"` +} + // registerKegTools exposes identity-authorized discovery filtered through the -// immutable active flight. Transport-specific hub selection is intentionally -// absent from the agent surface. -func registerKegTools(srv *sdkmcp.Server, _ KegDefaults, kegs KegDiscoveryProvider) { +// immutable active flight, plus keg creation for flights that carry +// manage_kegs. Transport-specific hub selection is intentionally absent from +// the agent surface. +func registerKegTools(srv *sdkmcp.Server, defaults KegDefaults, kegs KegDiscoveryProvider) { sdkmcp.AddTool(srv, &sdkmcp.Tool{ Name: "keg_list", Description: "List identity-authorized kegs covered by the active flight, qualified as @namespace/keg", @@ -30,6 +38,37 @@ func registerKegTools(srv *sdkmcp.Server, _ KegDefaults, kegs KegDiscoveryProvid } return linesResult(filterKegRefs(ctx, refs)), nil, nil }) + + sdkmcp.AddTool(srv, &sdkmcp.Tool{ + Name: "keg_create", + Description: "Create a new KEG. Requires the active flight to grant manage_kegs. " + + "The new KEG is not readable until a flight covers it — creating one does not " + + "add it to the active flight's cover.", + Annotations: &sdkmcp.ToolAnnotations{ + ReadOnlyHint: false, + DestructiveHint: boolPtr(false), + OpenWorldHint: boolPtr(true), + }, + }, func(ctx context.Context, _ *sdkmcp.CallToolRequest, in kegCreateInput) (*sdkmcp.CallToolResult, any, error) { + // The gate refuses this tool before dispatch; the check is repeated here + // so an embedded surface without the session gate cannot reach creation + // through a flight that never granted it. + if err := defaults.gate.authorizeKegCreation(sessionIDFromContext(ctx)); err != nil { + return errorResult(err), nil, nil + } + ref, err := kegs.CreateKeg(ctx, tapper.CreateKegOptions{ + Keg: in.Keg, + Namespace: in.Namespace, + Title: in.Title, + Visibility: in.Visibility, + }) + if err != nil { + return errorResult(err), nil, nil + } + return textResult("created keg " + ref + + "\n\nIt is not in this flight's cover yet, so KEG tools cannot reach it. " + + "Add it to a flight's cover, then call `orient`."), nil, nil + }) } func filterKegRefs(ctx context.Context, refs []string) []string { diff --git a/pkg/mcp/tools_orient.go b/pkg/mcp/tools_orient.go index ee60411..72a00fe 100644 --- a/pkg/mcp/tools_orient.go +++ b/pkg/mcp/tools_orient.go @@ -22,13 +22,23 @@ func registerOrient(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { sdkmcp.AddTool(srv, &sdkmcp.Tool{ Name: "orient", // The imperative lives here, not only in the server instructions, because - // tool descriptions are the one thing every model reads. A weaker model - // given only server instructions will not infer that it must orient. - Description: "Call this first, before any other KEG tool. Establishes this session's " + - "flight authority and returns the Tapper orientation payload listing the KEGs you " + - "may use. While no flight is active the KEG tools are hidden and only orient, " + - "list_flights, flight_show, and auth_info are available; call orient again after " + - "the user selects a flight to unlock them.", + // tool descriptions are the one channel that is re-delivered with every + // tools/list and therefore survives a context reset. The flight's own + // instructions arrive in this tool's *result*, which lives in the + // conversation and is exactly what a clear or compact discards — so the + // instruction to come back must live somewhere the reset cannot reach. + Description: "Call this first in every session, before any other tool and before " + + "replying to the user — not only when KEG work is expected. The active flight " + + "carries this session's operating instructions, so until you orient you do not " + + "know what the session is for or which KEGs you may touch. Returns that flight, " + + "its instructions, and the available KEGs. Call it again after any context reset " + + "such as a clear or a compact: the connection survives those but the flight " + + "instructions do not, and the server cannot detect the reset to re-send them. " + + "If you cannot tell whether you have oriented in the current context, you have " + + "not — orient. It is idempotent and also picks up configuration changed since " + + "you connected. While no flight is active the KEG tools are hidden and only " + + "orient, list_flights, flight_show, and auth_info are available; call orient " + + "again once the user selects a flight to unlock them.", Annotations: &sdkmcp.ToolAnnotations{ ReadOnlyHint: true, OpenWorldHint: boolPtr(false), diff --git a/pkg/tapper/config.go b/pkg/tapper/config.go index 5ba3fb8..a4d0e5a 100644 --- a/pkg/tapper/config.go +++ b/pkg/tapper/config.go @@ -187,12 +187,18 @@ type HubEntry struct { // and a project config may safely define them. // // Experimental: this shape is expected to change when agents move to the hub. +// ContextWindow caps the working context in tokens. Harnesses express this +// differently — Codex as model metadata, Claude Code as an auto-compact +// threshold — so the launcher translates it per harness rather than passing a +// raw flag, and reports rather than drops it where there is no equivalent. type AgentEntry struct { - Model string `yaml:"model,omitempty"` - Flight string `yaml:"flight,omitempty"` - BaseURL string `yaml:"baseUrl,omitempty"` - Auth string `yaml:"auth,omitempty"` - APIKeyEnv string `yaml:"apiKeyEnv,omitempty"` + Model string `yaml:"model,omitempty"` + Flight string `yaml:"flight,omitempty"` + BaseURL string `yaml:"baseUrl,omitempty"` + Auth string `yaml:"auth,omitempty"` + APIKeyEnv string `yaml:"apiKeyEnv,omitempty"` + ContextWindow int `yaml:"contextWindow,omitempty"` + Args []string `yaml:"args,omitempty"` } // KegRef is the (hub, namespace, name) triple a keg alias resolves to. An empty diff --git a/pkg/tapper/flight.go b/pkg/tapper/flight.go index 6e0251e..ae9b2b1 100644 --- a/pkg/tapper/flight.go +++ b/pkg/tapper/flight.go @@ -41,9 +41,47 @@ const ( FlightVisibilityPublic = "public" FlightCapabilityManageFlights FlightCapability = "manage_flights" + FlightCapabilityManageKegs FlightCapability = "manage_kegs" FlightCapabilityFullAccess FlightCapability = "full_access" + + // BootstrapFlightSlug names the synthetic flight a session runs on when no + // flight exists to select. It is never persisted, so the slug only has to be + // recognizable in orientation output and unambiguous against a real ref. + BootstrapFlightSlug = "bootstrap" ) +// BootstrapFlight returns the synthetic flight used when an identity can reach +// no flights at all. Its cover is empty, so every KEG operation is still +// denied; what it grants is the authority to create the first flight and the +// first keg. instructions carries the transport's own recovery text — the local +// and hosted surfaces nudge toward different places — and rides in the manifest +// rather than through BuildOrientationPayload's flightNote because a flight's +// instructions are already rendered and are inherently per-flight. +func BootstrapFlight(namespace, instructions string) *Flight { + ref := FlightRef{Namespace: strings.TrimPrefix(strings.TrimSpace(namespace), "@"), Slug: BootstrapFlightSlug} + // No Title: BuildOrientationPayload writes a flight's title verbatim, and a + // bare "Bootstrap" line adds nothing next to the paragraph it already emits + // for this mode. + m := FlightManifest{ + Visibility: FlightVisibilityPrivate, + Capabilities: []FlightCapability{ + FlightCapabilityManageFlights, + FlightCapabilityManageKegs, + }, + Instructions: instructions, + } + normalizeFlightManifest(&m) + return &Flight{ + Name: ref.Canonical(), + Namespace: ref.Namespace, + Slug: ref.Slug, + Source: "synthetic", + Bootstrap: true, + ManifestHash: hashFlightManifest(m), + FlightManifest: m, + } +} + // AtLeast reports whether r grants at least want within a flight cover. func (r FlightRole) AtLeast(want FlightRole) bool { r = normalizeFlightRole(r) @@ -91,10 +129,15 @@ type FlightManifest struct { // Flight is a discovered flight: its manifest plus provenance. type Flight struct { - Name string `yaml:"-" json:"name,omitempty"` - Namespace string `yaml:"-" json:"namespace,omitempty"` - Slug string `yaml:"-" json:"slug,omitempty"` - Source string `yaml:"-" json:"source,omitempty"` // "local" or a hub name + Name string `yaml:"-" json:"name,omitempty"` + Namespace string `yaml:"-" json:"namespace,omitempty"` + Slug string `yaml:"-" json:"slug,omitempty"` + Source string `yaml:"-" json:"source,omitempty"` // "local" or a hub name + // Bootstrap marks the synthetic flight from BootstrapFlight. It is a field + // rather than a Source sentinel because Source is provenance an operator + // reads, and a caller asking "is this real?" should not have to know which + // string means synthetic. + Bootstrap bool `yaml:"-" json:"bootstrap,omitempty"` ManifestHash string `yaml:"-" json:"-"` FlightManifest } @@ -114,7 +157,16 @@ func ParseFlightRef(raw string, defaultNamespace string) (FlightRef, error) { case strings.HasPrefix(raw, "@"): ns, rest, ok := strings.Cut(strings.TrimPrefix(raw, "@"), "/") if !ok || ns == "" || rest == "" { - return FlightRef{}, fmt.Errorf("invalid flight reference %q", raw) + return FlightRef{}, fmt.Errorf( + "invalid flight reference %q: use @namespace/+slug, or a bare +slug for the default namespace", raw) + } + // Validate here rather than letting the segment travel. The `+` sigil + // marks the slug, so "@+slug/..." puts it where the namespace belongs — + // an easy transposition that otherwise reaches the hub as a namespace + // that cannot exist and comes back as an opaque 404. + if err := ValidateNamespace(ns); err != nil { + return FlightRef{}, fmt.Errorf( + "invalid flight reference %q: %w; the + sigil marks the slug, so write @namespace/+slug", raw, err) } ref.Namespace = ns ref.Slug = strings.TrimPrefix(rest, "+") @@ -533,7 +585,9 @@ func validateFlightManifest(m *FlightManifest) error { seen := map[FlightCapability]struct{}{} for _, capability := range m.Capabilities { capability = FlightCapability(strings.TrimSpace(string(capability))) - if capability != FlightCapabilityManageFlights && capability != FlightCapabilityFullAccess { + switch capability { + case FlightCapabilityManageFlights, FlightCapabilityManageKegs, FlightCapabilityFullAccess: + default: return fmt.Errorf("unknown flight capability %q", capability) } if _, ok := seen[capability]; ok { diff --git a/pkg/tapper/flight_test.go b/pkg/tapper/flight_test.go index 4be6178..4aad0d4 100644 --- a/pkg/tapper/flight_test.go +++ b/pkg/tapper/flight_test.go @@ -209,6 +209,24 @@ func TestParseFlightRef(t *testing.T) { } } +// TestParseFlightRefRejectsUnusableNamespace pins that a malformed namespace +// fails here rather than travelling. The `+` sigil marks the slug, so +// "@+slug/..." transposes it into the namespace position — a namespace that can +// never exist. Left unvalidated it reaches the hub and returns as a bare 404, +// which reads as a missing flight and sends the author looking at permissions. +func TestParseFlightRefRejectsUnusableNamespace(t *testing.T) { + t.Parallel() + for _, raw := range []string{"@+agent-work/x", "@Foldwise/+agent-work", "@foldwise.dev/+agent-work"} { + t.Run(raw, func(t *testing.T) { + t.Parallel() + _, err := tapper.ParseFlightRef(raw, "jlrickert") + require.ErrorContains(t, err, "invalid flight reference") + require.ErrorContains(t, err, "@namespace/+slug", + "the refusal must show the form the author meant to write") + }) + } +} + func TestFlightRoleFor_CoverCapsWrites(t *testing.T) { t.Parallel() flight := &tapper.Flight{ diff --git a/pkg/tapper/hub_flights.go b/pkg/tapper/hub_flights.go index 42afae1..922477e 100644 --- a/pkg/tapper/hub_flights.go +++ b/pkg/tapper/hub_flights.go @@ -161,16 +161,26 @@ func doHubFlightJSON(ctx context.Context, method, hubURL, token, path string, pa } defer func() { _ = resp.Body.Close() }() + // Every branch carries the hub's own message. These statuses are not + // specific to the flight: the same endpoints answer 404 for an unresolvable + // *namespace* and 403 for an insufficient namespace role, so translating the + // status alone names the wrong subject and sends the reader off to fix + // something that was never wrong — "flight not found" on a create is not + // even a coherent claim. Naming the request and appending the body keeps the + // hub's diagnosis intact. switch resp.StatusCode { case http.StatusOK, http.StatusCreated: case http.StatusNoContent: return nil case http.StatusConflict: - return fmt.Errorf("hub: flight already exists: %w", keg.ErrExist) + return fmt.Errorf("hub: %s %s conflicts with existing state%s: %w", + method, path, readHubError(resp), keg.ErrExist) case http.StatusNotFound: - return fmt.Errorf("hub: flight not found: %w", keg.ErrNotExist) + return fmt.Errorf("hub: %s %s returned not found%s: %w", + method, path, readHubError(resp), keg.ErrNotExist) case http.StatusUnauthorized, http.StatusForbidden: - return fmt.Errorf("hub: %w (%s)", ErrTokenRejected, resp.Status) + return fmt.Errorf("hub: %w for %s %s (%s)%s", + ErrTokenRejected, method, path, resp.Status, readHubError(resp)) default: return fmt.Errorf("hub: flight request failed: %s%s", resp.Status, readHubError(resp)) } diff --git a/pkg/tapper/hub_flights_test.go b/pkg/tapper/hub_flights_test.go index 264a81b..588b7ff 100644 --- a/pkg/tapper/hub_flights_test.go +++ b/pkg/tapper/hub_flights_test.go @@ -94,3 +94,57 @@ func TestHubFlights_ClientRejectsUnknownCoverRole(t *testing.T) { _, err := tapper.GetHubFlight(context.Background(), srv.URL, "tok", "foldwise", "bad") require.ErrorContains(t, err, `invalid flight cover role "owner"`) } + +// TestHubFlights_ClientSurfacesHubDiagnosis pins that the hub's own message +// survives the status translation. The flight endpoints answer 404 for an +// unresolvable *namespace* and 403 for an insufficient namespace role, so a +// status-only message ("flight not found" on a create) names the wrong subject +// and sends the reader off to fix something that was never wrong. +func TestHubFlights_ClientSurfacesHubDiagnosis(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + status int + body string + wants []string + }{ + { + name: "namespace missing is not a missing flight", + status: http.StatusNotFound, + body: "namespace not found", + wants: []string{"namespace not found", "POST", "/api/v1/@foldwise/flights"}, + }, + { + name: "insufficient role is not a rejected token", + status: http.StatusForbidden, + body: "namespace owner access required", + wants: []string{"namespace owner access required", "/api/v1/@foldwise/flights"}, + }, + { + name: "conflict names the request", + status: http.StatusConflict, + body: "flight already exists", + wants: []string{"flight already exists", "/api/v1/@foldwise/flights"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.status) + _ = json.NewEncoder(w).Encode(map[string]string{"error": tc.body}) + })) + defer srv.Close() + + _, err := tapper.CreateHubFlight(context.Background(), srv.URL, "tok", "foldwise", + tapper.HubFlight{Namespace: "foldwise", Slug: "new"}) + require.Error(t, err) + for _, want := range tc.wants { + require.ErrorContains(t, err, want) + } + require.NotContains(t, err.Error(), "flight not found", + "a create cannot fail because the flight it would create is missing") + }) + } +} diff --git a/pkg/tapper/tap_flight.go b/pkg/tapper/tap_flight.go index 68aaaf7..39295f9 100644 --- a/pkg/tapper/tap_flight.go +++ b/pkg/tapper/tap_flight.go @@ -179,7 +179,17 @@ func (t *Tap) resolveWriteFlightRef(raw string) (FlightRef, HubEntry, string, er kind = HubKindRemote } if kind == HubKindLocal { - return FlightRef{}, HubEntry{}, "", fmt.Errorf("flight create/update/delete require a remote hub-backed namespace") + // Reading local manifests is fully supported — discovery, orientation, + // and cover enforcement all work off flights.d. Only mutation is + // unimplemented, so say that rather than describing it as a + // requirement the caller failed to meet. + dir, dirErr := t.FlightService.localFlightsDirFor(entry) + if dirErr != nil { + dir = "/" + flightsDirName + } + return FlightRef{}, HubEntry{}, "", fmt.Errorf( + "flight create/update/delete is not implemented for local hubs (hub %q); "+ + "write the manifest to %s/%s.yaml by hand instead", hubName, dir, ref.Slug) } if strings.TrimSpace(entry.URL) == "" { return FlightRef{}, HubEntry{}, "", fmt.Errorf("hub %q has no url configured", hubName) @@ -233,14 +243,26 @@ type FlightRestrictionError struct { Got FlightRole } +// flightRestrictionRecovery is appended to every cover/role-cap denial. The +// direct CLI bypasses flight restrictions entirely (see applyKegTargetProfile), +// so this error only ever reaches an agent over MCP — and an agent's session +// pins its flight snapshot until it orients again. A flight edited elsewhere +// mid-session is therefore the most common cause of a denial that the reader +// believes should have succeeded, and the reader cannot discover that from a +// bare "not available" line. +const flightRestrictionRecovery = ". Call `orient` to refresh this session's flight" + + " authority: it may have changed since you oriented. If orient still does not" + + " cover this keg, the flight genuinely excludes it — ask the user to widen the" + + " flight's cover rather than retrying." + func (e *FlightRestrictionError) Error() string { if e.Want == FlightRoleEditor && e.Got == FlightRoleViewer { - return fmt.Sprintf("keg %q is viewer-only in flight %q", e.Keg, e.Flight) + return fmt.Sprintf("keg %q is viewer-only in flight %q", e.Keg, e.Flight) + flightRestrictionRecovery } if e.Want == FlightRoleAdmin && (e.Got == FlightRoleViewer || e.Got == FlightRoleEditor) { - return fmt.Sprintf("keg %q requires admin flight authority in flight %q", e.Keg, e.Flight) + return fmt.Sprintf("keg %q requires admin flight authority in flight %q", e.Keg, e.Flight) + flightRestrictionRecovery } - return fmt.Sprintf("keg %q is not available in flight %q", e.Keg, e.Flight) + return fmt.Sprintf("keg %q is not available in flight %q", e.Keg, e.Flight) + flightRestrictionRecovery } // enforceFlight rejects a resolved keg that falls outside the active flight's @@ -305,6 +327,32 @@ func (t *Tap) enforceFlightSnapshot(flight *Flight, k keg.Keg, want FlightRole) return &FlightRestrictionError{Flight: flight.Name, Keg: label, Want: want, Got: role} } +// CanonicalKegRef returns the @namespace/keg reference for a resolved target, +// or "" when the target names no namespaced keg. A filesystem keg carries its +// identity in the path rather than in the Namespace/KegName fields, so this +// applies the same derivation enforceFlightSnapshot uses — callers reporting a +// keg back to an agent must name it the way the flight cover does. +func CanonicalKegRef(target *keg.Target) string { + if target == nil { + return "" + } + namespace, kegName := target.Namespace, target.KegName + if namespace == "" || kegName == "" { + if pathNamespace, pathKeg, ok := localHubPathKegIdentity(target); ok { + if namespace == "" { + namespace = pathNamespace + } + if kegName == "" { + kegName = pathKeg + } + } + } + if namespace == "" || kegName == "" { + return "" + } + return "@" + namespace + "/" + kegName +} + func localHubPathKegIdentity(target *keg.Target) (string, string, bool) { if target == nil { return "", "", false diff --git a/pkg/tapper/tap_init.go b/pkg/tapper/tap_init.go index 6278ab1..46fd7c8 100644 --- a/pkg/tapper/tap_init.go +++ b/pkg/tapper/tap_init.go @@ -57,6 +57,23 @@ func (o InitOptions) LocalDestination() bool { return o.Project || o.Cwd || strings.TrimSpace(o.Path) != "" } +// CreateKegOptions is the agent-facing keg creation request. It is deliberately +// narrower than InitOptions: the machine-local destination selectors (Project, +// Cwd, Path, User, Hub) describe a filesystem this MCP caller may not share +// with the server, so the agent surface names a namespace and nothing else. +type CreateKegOptions struct { + // Namespace is the target namespace without the @ sigil. Empty resolves to + // the transport's default namespace. + Namespace string + // Keg is the alias to create. + Keg string + // Title is the human-readable keg title. Optional. + Title string + // Visibility is "private" or "public". Empty means the backend's default, + // which is private everywhere. Local filesystem kegs ignore it. + Visibility string +} + // InitKeg creates a keg named options.Keg and initializes it at the resolved // destination. Destination resolution is namespace-centric: // diff --git a/pkg/tapper/tap_integrate.go b/pkg/tapper/tap_integrate.go index 4cd8363..96958ff 100644 --- a/pkg/tapper/tap_integrate.go +++ b/pkg/tapper/tap_integrate.go @@ -100,17 +100,22 @@ func (t *Tap) Integrate(ctx context.Context, opts IntegrateOptions) (*IntegrateR if err := t.extractIntegration(host, root); err != nil { return nil, err } - if !registered { - if err := t.runIntegrationCommand(ctx, executable, hostMarketplaceAddArgs(host, root, scope)); err != nil { + // The marketplace goes first: Codex installs plugins from its snapshot, so + // recapturing it has to happen before any plugin is added back. + for _, args := range hostMarketplaceCommands(host, root, scope, registered) { + if err := t.runIntegrationCommand(ctx, executable, args); err != nil { return nil, fmt.Errorf("integrate: register %s marketplace: %w", host, err) } } + // Each plugin's removal sits immediately before its own add, so a failure + // part-way through leaves at most one plugin missing rather than all of them. for _, name := range selected { id := name + "@" + localIntegrationMarketplace - args := hostPluginInstallArgs(host, id, scope, installed[id]) - if err := t.runIntegrationCommand(ctx, executable, args); err != nil { - return nil, fmt.Errorf("integrate: install %s with %s: %w", id, host, err) + for _, args := range hostPluginCommands(host, id, scope, installed[id]) { + if err := t.runIntegrationCommand(ctx, executable, args); err != nil { + return nil, fmt.Errorf("integrate: install %s with %s: %w", id, host, err) + } } } return result, nil @@ -164,13 +169,22 @@ func integrationPreview(host, root, scope string, selected []string) (*Integrate return nil, fmt.Errorf("integrate: inspect embedded %s marketplace: %w", host, err) } sort.Strings(targets) + // The preview is the fresh-install sequence. A dry run deliberately starts no + // host process (it returns before the list commands run), so it cannot know + // whether the marketplace is registered or a plugin already installed. A real + // run against an existing install inserts the corresponding removes — see + // hostMarketplaceCommands and hostPluginCommands. commands := [][]string{ append([]string{host}, hostMarketplaceListArgs(host)...), append([]string{host}, hostPluginListArgs(host)...), - append([]string{host}, hostMarketplaceAddArgs(host, root, scope)...), + } + for _, args := range hostMarketplaceCommands(host, root, scope, false) { + commands = append(commands, append([]string{host}, args...)) } for _, name := range selected { - commands = append(commands, append([]string{host}, hostPluginInstallArgs(host, name+"@"+localIntegrationMarketplace, scope, false)...)) + for _, args := range hostPluginCommands(host, name+"@"+localIntegrationMarketplace, scope, false) { + commands = append(commands, append([]string{host}, args...)) + } } return &IntegrateResult{Root: root, Paths: targets, Commands: commands}, nil } @@ -375,22 +389,49 @@ func hostPluginListArgs(host string) []string { return []string{"plugin", "list", "--json"} } -func hostMarketplaceAddArgs(host, root, scope string) []string { - args := []string{"plugin", "marketplace", "add", root} +// hostMarketplaceCommands returns the commands that make the host's view of the +// marketplace match the freshly extracted tree, given whether it is already +// registered. +// +// Codex serves plugins from a snapshot taken when the marketplace was added, so +// re-extracting files it has already snapshotted changes nothing. Its +// `marketplace upgrade` only refreshes Git sources, which a local marketplace is +// not, leaving remove-then-add as the sole way to recapture. Claude reads the +// registered path directly, so a re-register would be pure churn. +func hostMarketplaceCommands(host, root, scope string, registered bool) [][]string { + add := []string{"plugin", "marketplace", "add", root} if host == "claude" { - args = append(args, "--scope", scope) + if registered { + return nil + } + return [][]string{append(add, "--scope", scope)} } - return args + if registered { + return [][]string{ + {"plugin", "marketplace", "remove", localIntegrationMarketplace}, + add, + } + } + return [][]string{add} } -func hostPluginInstallArgs(host, id, scope string, installed bool) []string { +// hostPluginCommands returns the commands that install or refresh one plugin. +// +// Codex has no update verb and caches the plugin at install time, so an +// already-installed plugin must be removed before adding it back or the new +// content never lands — `plugin add` alone silently keeps the cached copy. +func hostPluginCommands(host, id, scope string, installed bool) [][]string { if host == "claude" { if installed { - return []string{"plugin", "update", id, "--scope", scope} + return [][]string{{"plugin", "update", id, "--scope", scope}} } - return []string{"plugin", "install", id, "--scope", scope} + return [][]string{{"plugin", "install", id, "--scope", scope}} + } + add := []string{"plugin", "add", id} + if installed { + return [][]string{{"plugin", "remove", id}, add} } - return []string{"plugin", "add", id} + return [][]string{add} } func checkMarketplaceState(host string, body []byte, expectedRoot, scope string) (bool, error) { diff --git a/pkg/tapper/tap_integrate_test.go b/pkg/tapper/tap_integrate_test.go index e71b0bc..8328816 100644 --- a/pkg/tapper/tap_integrate_test.go +++ b/pkg/tapper/tap_integrate_test.go @@ -116,7 +116,10 @@ func TestTap_Integrate_ClaudeUsesInstallThenUpdateForExistingPlugin(t *testing.T require.Contains(t, result.Root, filepath.FromSlash("integrations/claude")) } -func TestTap_Integrate_RefreshRemovesStaleFilesAndReusesMarketplace(t *testing.T) { +// Reusing the registration was the original intent, but Codex installs from a +// snapshot captured at `marketplace add`, so reuse meant a refresh rewrote the +// files on disk and shipped none of them. The marketplace is now recaptured. +func TestTap_Integrate_RefreshRemovesStaleFilesAndRecapturesMarketplace(t *testing.T) { tap, sb := newIntegrateTap(t) installFakeHost(t, sb, "codex") first, err := tap.Integrate(context.Background(), tapper.IntegrateOptions{Host: "codex"}) @@ -135,7 +138,59 @@ func TestTap_Integrate_RefreshRemovesStaleFilesAndReusesMarketplace(t *testing.T } calls, err := sb.ReadFile("calls") require.NoError(t, err) - require.Equal(t, 1, strings.Count(string(calls), "plugin marketplace add "+first.Root)) + // Once per run: the second run drops the stale registration and re-adds it so + // Codex snapshots the freshly extracted tree. + require.Equal(t, 2, strings.Count(string(calls), "plugin marketplace add "+first.Root)) + require.Equal(t, 1, strings.Count(string(calls), "plugin marketplace remove tapper-local")) +} + +// Codex has no update verb and serves an installed plugin from a local cache, +// so `plugin add` on an already-installed plugin is a no-op. Without the +// removes, `tap integrate codex` reports success while delivering nothing — +// which is how an MCP registration fix sat undelivered on every machine that +// already had the plugin. +func TestTap_Integrate_CodexRefreshReinstallsInsteadOfNoOp(t *testing.T) { + tap, sb := newIntegrateTap(t) + installFakeHost(t, sb, "codex") + root := "/home/testuser/.local/share/tapper/integrations/codex" + require.NoError(t, sb.Runtime().Env().Set("MARKETPLACES_JSON", + `{"marketplaces":[{"name":"tapper-local","root":"`+root+`"}]}`)) + require.NoError(t, sb.Runtime().Env().Set("PLUGINS_JSON", + `{"installed":[{"pluginId":"tapper@tapper-local"}]}`)) + + _, err := tap.Integrate(context.Background(), tapper.IntegrateOptions{Host: "codex"}) + require.NoError(t, err) + + calls, err := sb.ReadFile("calls") + require.NoError(t, err) + text := string(calls) + require.Contains(t, text, "plugin marketplace remove tapper-local") + require.Contains(t, text, "plugin marketplace add "+root) + require.Contains(t, text, "plugin remove tapper@tapper-local") + require.Contains(t, text, "plugin add tapper@tapper-local") + + // The snapshot must be recaptured before the plugin is installed from it. + require.Less(t, strings.Index(text, "plugin marketplace add "+root), + strings.Index(text, "plugin add tapper@tapper-local"), + "marketplace must be re-registered before the plugin is added back") + // And each plugin's removal immediately precedes its own add. + require.Less(t, strings.Index(text, "plugin remove tapper@tapper-local"), + strings.Index(text, "plugin add tapper@tapper-local")) +} + +func TestTap_Integrate_CodexFreshInstallRemovesNothing(t *testing.T) { + tap, sb := newIntegrateTap(t) + installFakeHost(t, sb, "codex") + + _, err := tap.Integrate(context.Background(), tapper.IntegrateOptions{Host: "codex"}) + require.NoError(t, err) + + calls, err := sb.ReadFile("calls") + require.NoError(t, err) + // Removing something never installed would fail the run for no reason. + require.NotContains(t, string(calls), "plugin remove") + require.NotContains(t, string(calls), "marketplace remove") + require.Contains(t, string(calls), "plugin add tapper@tapper-local") } func TestTap_Integrate_MarketplaceConflictFailsBeforeExtraction(t *testing.T) { diff --git a/pkg/tapper/tap_launch.go b/pkg/tapper/tap_launch.go index d85779f..49cb7ab 100644 --- a/pkg/tapper/tap_launch.go +++ b/pkg/tapper/tap_launch.go @@ -11,6 +11,7 @@ import ( "fmt" "os/exec" "sort" + "strconv" "strings" ) @@ -37,10 +38,16 @@ const ( AuthSubscription = "subscription" // AuthAPIKey forwards the variable named by an agent's apiKeyEnv. AuthAPIKey = "apiKey" + // AuthNone means the model needs no credential of ours. It strips the same + // inherited variables as AuthSubscription but does not imply a stored login + // to fall back to, which is what a local provider actually wants — and it + // leaves the placeholder key in place so the harness cannot fall back at + // all. It is the default for ollama models. + AuthNone = "none" ) // providerKeyEnv lists the credential variables each provider's clients read. -// AuthSubscription removes these from the child environment. +// AuthSubscription and AuthNone remove these from the child environment. var providerKeyEnv = map[string][]string{ ProviderAnthropic: {"ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN"}, ProviderOpenAI: {"OPENAI_API_KEY"}, @@ -92,6 +99,11 @@ type launchSpec struct { type harnessAdapter struct { command string providers map[string]func(spec launchSpec) ([]string, map[string]string) + // contextWindowArgs renders an agent's contextWindow into this harness's + // own flags. Nil means the harness has no equivalent, in which case a + // configured contextWindow is reported rather than silently dropped — + // quietly ignoring a context cap is how you discover it never applied. + contextWindowArgs func(tokens int) []string } // openAIBaseURL normalizes a base URL for OpenAI clients, which append @@ -127,16 +139,26 @@ func anthropicProtocol(command string) func(launchSpec) ([]string, map[string]st switch { case spec.apiKey != "": env["ANTHROPIC_API_KEY"] = spec.apiKey - case spec.provider == ProviderOllama && spec.auth != AuthSubscription: - // Ollama ignores the value, but without one the client would try - // the stored subscription login against the wrong host. + case spec.provider == ProviderOllama: + // Unconditional for a local provider. Ollama ignores the value, but + // without one the client falls back to its stored login and sends + // real subscription credentials to a host that is not Anthropic. + // This placeholder is the thing preventing that, so no auth mode may + // switch it off. env["ANTHROPIC_API_KEY"] = "ollama" } return []string{command}, env } } -// openAIProtocol builds the invocation for harnesses speaking the OpenAI API. +// openAIProtocol builds the invocation for harnesses that take their endpoint +// and key from the conventional OPENAI_* environment variables. +// +// Codex does NOT: it configures providers through ~/.codex/config.toml and its +// own CODEX_OSS_* variables, and ignores OPENAI_BASE_URL entirely. Setting +// OPENAI_API_KEY there is actively harmful — `codex doctor` reports "mixed auth +// signals: ChatGPT login plus API key env var" and switches to API-key billing. +// Codex therefore has its own builders below. func openAIProtocol(command string) func(launchSpec) ([]string, map[string]string) { return func(spec launchSpec) ([]string, map[string]string) { env := map[string]string{} @@ -146,13 +168,40 @@ func openAIProtocol(command string) func(launchSpec) ([]string, map[string]strin switch { case spec.apiKey != "": env["OPENAI_API_KEY"] = spec.apiKey - case spec.provider == ProviderOllama && spec.auth != AuthSubscription: + case spec.provider == ProviderOllama: + // Unconditional, for the same reason as the Anthropic builder: the + // placeholder is what stops the client reaching for a stored login + // and sending it to a host that is not the provider. env["OPENAI_API_KEY"] = "ollama" } return []string{command, "--model", spec.model}, env } } +// codexHosted drives Codex against OpenAI proper. Codex owns its own auth — a +// stored ChatGPT login or OPENAI_API_KEY from the environment — so nothing is +// injected here beyond the model. +func codexHosted(spec launchSpec) ([]string, map[string]string) { + env := map[string]string{} + if spec.apiKey != "" { + env["OPENAI_API_KEY"] = spec.apiKey + } + return []string{"codex", "--model", spec.model}, env +} + +// codexOSS drives Codex against a local Ollama server. Codex has first-class +// support for this through --oss/--local-provider and reads the endpoint from +// CODEX_OSS_BASE_URL, so the OPENAI_* variables are neither used nor set: an +// OPENAI_API_KEY here would only push Codex into API-key mode against the wrong +// provider. +func codexOSS(spec launchSpec) ([]string, map[string]string) { + env := map[string]string{} + if base := openAIBaseURL(spec.baseURL); base != "" { + env["CODEX_OSS_BASE_URL"] = base + } + return []string{"codex", "--oss", "--local-provider", "ollama", "--model", spec.model}, env +} + func harnessAdapters() map[string]harnessAdapter { return map[string]harnessAdapter{ "claude": { @@ -163,12 +212,24 @@ func harnessAdapters() map[string]harnessAdapter { // once ANTHROPIC_BASE_URL points at the server. ProviderOllama: anthropicProtocol("claude"), }, + // Claude Code has no model-metadata override; the nearest thing is + // the threshold at which it auto-compacts, which is what a context + // cap means in practice. It accepts roughly 100k-1M. + contextWindowArgs: func(tokens int) []string { + return []string{"--autocompact", strconv.Itoa(tokens)} + }, }, "codex": { command: "codex", providers: map[string]func(launchSpec) ([]string, map[string]string){ - ProviderOpenAI: openAIProtocol("codex"), - ProviderOllama: openAIProtocol("codex"), + ProviderOpenAI: codexHosted, + ProviderOllama: codexOSS, + }, + // Codex treats it as model metadata. Setting it also silences the + // "model metadata not found, defaulting to fallback" warning for a + // local tag Codex does not know. + contextWindowArgs: func(tokens int) []string { + return []string{"-c", "model_context_window=" + strconv.Itoa(tokens)} }, }, "pi": { @@ -250,7 +311,7 @@ func (t *Tap) ResolveLaunch(opts LaunchOptions) (*LaunchResult, error) { harness, provider, strings.Join(adapterProviders(adapter), ", ")) } - auth, err := resolveAuthMode(agent) + auth, err := resolveAuthMode(agent, provider) if err != nil { return nil, fmt.Errorf("agent %q: %w", agentName, err) } @@ -274,6 +335,16 @@ func (t *Tap) ResolveLaunch(opts LaunchOptions) (*LaunchResult, error) { apiKey: apiKey, auth: auth, }) + if agent.ContextWindow > 0 { + if adapter.contextWindowArgs == nil { + return nil, fmt.Errorf( + "agent %q sets contextWindow but harness %q has no way to apply it", + agentName, harness) + } + argv = append(argv, adapter.contextWindowArgs(agent.ContextWindow)...) + } + // Agent args first, then the invocation's own, so a one-off can override. + argv = append(argv, agent.Args...) argv = append(argv, opts.Args...) if env == nil { env = map[string]string{} @@ -287,7 +358,7 @@ func (t *Tap) ResolveLaunch(opts LaunchOptions) (*LaunchResult, error) { // Subscription mode has to remove inherited credentials, which an overlay // cannot express: appending can override a variable but never unset one. var strip []string - if auth == AuthSubscription { + if auth == AuthSubscription || auth == AuthNone { for _, name := range providerKeyEnv[provider] { if _, set := env[name]; !set { strip = append(strip, name) @@ -311,19 +382,40 @@ func (t *Tap) ResolveLaunch(opts LaunchOptions) (*LaunchResult, error) { }, nil } -// resolveAuthMode validates an agent's auth field, defaulting to inherit. -func resolveAuthMode(agent AgentEntry) (string, error) { - switch mode := strings.TrimSpace(agent.Auth); mode { - case "": - if strings.TrimSpace(agent.APIKeyEnv) != "" { +// resolveAuthMode validates an agent's auth field and supplies the default. +// +// A local provider defaults to none rather than inherit: cloud credentials have +// no business reaching a model running on your own hardware, and inheriting +// them is how an exported OPENAI_API_KEY ends up confusing a harness that is +// not talking to OpenAI at all. +func resolveAuthMode(agent AgentEntry, provider string) (string, error) { + mode := strings.TrimSpace(agent.Auth) + if mode == "" { + switch { + case strings.TrimSpace(agent.APIKeyEnv) != "": return AuthAPIKey, nil + case provider == ProviderOllama: + return AuthNone, nil + default: + return AuthInherit, nil } - return AuthInherit, nil - case AuthInherit, AuthSubscription, AuthAPIKey: + } + switch mode { + case AuthSubscription: + if provider == ProviderOllama { + // There is no subscription behind a local model, and honouring the + // request would mean withholding the placeholder key that stops the + // harness reaching for a real stored login. + return "", fmt.Errorf( + "auth %q is meaningless for a local %s model; use %q (the default) instead", + AuthSubscription, ProviderOllama, AuthNone) + } + return mode, nil + case AuthInherit, AuthAPIKey, AuthNone: return mode, nil default: - return "", fmt.Errorf("unknown auth mode %q (want %s, %s, or %s)", - mode, AuthInherit, AuthSubscription, AuthAPIKey) + return "", fmt.Errorf("unknown auth mode %q (want %s, %s, %s, or %s)", + mode, AuthInherit, AuthSubscription, AuthAPIKey, AuthNone) } } @@ -338,8 +430,8 @@ func (t *Tap) resolveAPIKey(agent AgentEntry, auth string) (key, source string, } return "", "", nil } - if auth == AuthSubscription { - return "", "", fmt.Errorf("auth %q cannot be combined with apiKeyEnv", AuthSubscription) + if auth == AuthSubscription || auth == AuthNone { + return "", "", fmt.Errorf("auth %q cannot be combined with apiKeyEnv", auth) } value := strings.TrimSpace(t.Runtime.Env().Get(name)) if value == "" { diff --git a/pkg/tapper/tap_launch_test.go b/pkg/tapper/tap_launch_test.go index 1bce16f..5ce3885 100644 --- a/pkg/tapper/tap_launch_test.go +++ b/pkg/tapper/tap_launch_test.go @@ -48,6 +48,13 @@ agents: badauth: model: openai/gpt-5 auth: nonsense + localsub: + model: ollama/qwen3.6:35b-mlx + auth: subscription + capped: + model: ollama/qwen3.6:35b-mlx + contextWindow: 150000 + args: ['--search'] ` func TestParseAgentModel(t *testing.T) { @@ -93,7 +100,11 @@ func TestResolveLaunch_AnthropicOnClaude(t *testing.T) { require.Equal(t, "+dev", got.Env["TAP_FLIGHT"]) } -func TestResolveLaunch_OllamaOnCodexUsesOpenAIProtocol(t *testing.T) { +// Codex has first-class local-provider support and configures it through +// --oss/--local-provider plus CODEX_OSS_BASE_URL. It ignores OPENAI_BASE_URL, +// and an OPENAI_API_KEY would push it into API-key billing against the wrong +// provider — `codex doctor` calls that "mixed auth signals". +func TestResolveLaunch_OllamaOnCodexUsesOSSProvider(t *testing.T) { t.Parallel() tap := newLaunchTap(t, launchUserConfig) @@ -101,8 +112,12 @@ func TestResolveLaunch_OllamaOnCodexUsesOpenAIProtocol(t *testing.T) { require.NoError(t, err) require.Equal(t, tapper.ProviderOllama, got.Provider) - require.Equal(t, []string{"codex", "--model", "qwen3.6:35b-mlx"}, got.Argv) - require.Equal(t, "http://localhost:11434/v1", got.Env["OPENAI_BASE_URL"]) + require.Equal(t, + []string{"codex", "--oss", "--local-provider", "ollama", "--model", "qwen3.6:35b-mlx"}, + got.Argv) + require.Equal(t, "http://localhost:11434/v1", got.Env["CODEX_OSS_BASE_URL"]) + require.NotContains(t, got.Env, "OPENAI_BASE_URL") + require.NotContains(t, got.Env, "OPENAI_API_KEY") require.Equal(t, "@testuser/+scratch", got.Env["TAP_FLIGHT"]) } @@ -147,7 +162,7 @@ func TestResolveLaunch_BaseURLNormalizesPerProtocol(t *testing.T) { viaCodex, err := tap.ResolveLaunch(tapper.LaunchOptions{Harness: "codex", Agent: "lab"}) require.NoError(t, err) - require.Equal(t, "http://192.168.50.197:11434/v1", viaCodex.Env["OPENAI_BASE_URL"]) + require.Equal(t, "http://192.168.50.197:11434/v1", viaCodex.Env["CODEX_OSS_BASE_URL"]) } func TestResolveLaunch_RejectsIncompatibleProvider(t *testing.T) { @@ -166,6 +181,29 @@ func TestResolveLaunch_RejectsIncompatibleProvider(t *testing.T) { require.Contains(t, err.Error(), "cannot use a openai model") } +// A local model must never cause real credentials to be sent to it. The +// placeholder key is what stops the harness falling back to a stored login and +// posting it to the ollama host, so no auth mode may suppress it — and asking +// for subscription auth on a local model is rejected rather than honoured. +func TestResolveLaunch_LocalModelNeverLeaksRealCredentials(t *testing.T) { + t.Parallel() + tap := newLaunchTap(t, launchUserConfig) + + got, err := tap.ResolveLaunch(tapper.LaunchOptions{Harness: "claude", Agent: "local"}) + require.NoError(t, err) + + // Defaults to none for a local provider without the user asking. + require.Equal(t, tapper.AuthNone, got.Auth) + require.Equal(t, "ollama", got.Env["ANTHROPIC_API_KEY"]) + // And the ambient cloud credentials are removed rather than passed along. + require.Contains(t, got.StripEnv, "OPENAI_API_KEY") + require.Contains(t, got.StripEnv, "ANTHROPIC_AUTH_TOKEN") + + _, err = tap.ResolveLaunch(tapper.LaunchOptions{Harness: "claude", Agent: "localsub"}) + require.Error(t, err) + require.Contains(t, err.Error(), "meaningless for a local ollama model") +} + func TestResolveLaunch_SubscriptionStripsInheritedKeys(t *testing.T) { t.Parallel() tap := newLaunchTap(t, launchUserConfig) @@ -269,6 +307,31 @@ func TestResolveLaunch_ReadsAgentsFromProjectConfig(t *testing.T) { require.Equal(t, "+proj", got.Env["TAP_FLIGHT"]) } +// A context cap means the same thing to a user on either harness but is spelled +// differently by each, so the launcher translates rather than passing a raw +// flag through. +func TestResolveLaunch_ContextWindowTranslatesPerHarness(t *testing.T) { + t.Parallel() + tap := newLaunchTap(t, launchUserConfig) + + viaCodex, err := tap.ResolveLaunch(tapper.LaunchOptions{Harness: "codex", Agent: "capped"}) + require.NoError(t, err) + require.Contains(t, viaCodex.Argv, "model_context_window=150000") + // Agent args ride along, before any one-off passed at the call site. + require.Contains(t, viaCodex.Argv, "--search") + + viaClaude, err := tap.ResolveLaunch(tapper.LaunchOptions{Harness: "claude", Agent: "capped"}) + require.NoError(t, err) + require.Contains(t, viaClaude.Argv, "--autocompact") + require.Contains(t, viaClaude.Argv, "150000") + + // pi has no known equivalent, so the cap is reported rather than dropped — + // silently ignoring it is how you find out later that it never applied. + _, err = tap.ResolveLaunch(tapper.LaunchOptions{Harness: "pi", Agent: "capped"}) + require.Error(t, err) + require.Contains(t, err.Error(), "no way to apply it") +} + func TestLaunchHarnesses(t *testing.T) { t.Parallel() require.Equal(t, []string{"claude", "codex", "pi"}, tapper.LaunchHarnesses()) diff --git a/pkg/tapper/tap_orient.go b/pkg/tapper/tap_orient.go index c45d583..b0f5e32 100644 --- a/pkg/tapper/tap_orient.go +++ b/pkg/tapper/tap_orient.go @@ -16,10 +16,15 @@ import ( const orientPurpose = "Tapper provides an MCP interface for KEG (Knowledge Exchange Graph) systems. A KEG is a numbered collection of markdown nodes with metadata, links, tags, and snapshot history. Agents operate on a KEG through the `mcp__tapper__*` tools; reading or writing node files directly bypasses indexing, locking, and snapshots." const orientRulesSummary = "Rules:\n" + + "- Call `orient` first in every session, before any other tool and before replying. The active flight carries this session's instructions, so until you orient you do not know what the session is for.\n" + + "- Call `orient` again after any context reset such as a clear or a compact. These instructions were delivered into the conversation and are discarded with it; the connection survives, so nothing re-sends them on its own. If you cannot tell whether you have oriented in the current context, you have not.\n" + + "- This payload supersedes every earlier copy of itself. An older copy can still be present — the connection's startup instructions are captured once and never refreshed, and a compaction summary may paraphrase a previous orientation — so if anything you remember about the flight, its cover, or its instructions disagrees with what you are reading here, this is current and that is stale. Do not merge them; replace.\n" + "- Use the `mcp__tapper__*` tools for every KEG operation; never read or write node files directly.\n" + "- The target keg resolves from the working directory unless the `keg` parameter overrides it.\n" + "- Take a snapshot before non-trivial edits. Snapshots do not protect against `remove`; preserve content some other way before deletion.\n" + - "- Intra-keg links use `[title](../NODEID)`; cross-keg links use `keg:ALIAS/NODEID` through active configuration or fully qualified `keg:@NAMESPACE/ALIAS/NODEID`.\n" + "- Node 0 is the keg's placeholder landing node. Leave it alone: it carries no `type` on purpose, it is where links to unwritten content land, and removing it makes the keg read as uninitialized. Write your content in a new node instead.\n" + + "- Intra-keg links use `[title](../NODEID)`; cross-keg links use `keg:ALIAS/NODEID` through active configuration or fully qualified `keg:@NAMESPACE/ALIAS/NODEID`.\n" + + "- Attachments on a node are linked relative to that node's own directory: `[label](./assets/FILE)` for files and `![alt](./images/IMAGE)` for images. Both directory names are plural.\n" // OrientOptions is the input to Tap.Orient. Flight is the only selector used // by orientation; the embedded target options remain for CLI profile @@ -393,7 +398,17 @@ func BuildOrientationPayload(flight *Flight, flightNote string, kegs []Orientati if flight != nil { b.WriteString("## Flight\n\n") - if flight.Name != "" { + switch { + case flight.Bootstrap: + // Never say "active flight" for the synthetic one: a reader who + // believes a flight was selected will not go set one up, which is + // the entire point of this mode. + b.WriteString("No flight is configured, so this session is running on a ") + b.WriteString("temporary bootstrap flight. Its cover is empty, so every KEG ") + b.WriteString("tool stays locked; what it grants is the authority to create ") + b.WriteString("the first flight and the first KEG. Setting this up is the ") + b.WriteString("session's work — do it before anything else.\n\n") + case flight.Name != "": b.WriteString("Active flight: `") b.WriteString(flight.Name) b.WriteString("`\n\n") diff --git a/pkg/tapper/tap_orient_test.go b/pkg/tapper/tap_orient_test.go index 74d7a21..aaed895 100644 --- a/pkg/tapper/tap_orient_test.go +++ b/pkg/tapper/tap_orient_test.go @@ -458,3 +458,34 @@ func TestTap_Orient_RecoveryPayloadStatesTheSituation(t *testing.T) { // The payload is the MCP-facing surface and never names CLI commands. require.NotContains(t, payload, "`tap ") } + +// TestTap_Orient_StatesZeroNodeAndAttachmentPaths pins two things the payload +// must carry. Node 0 is the placeholder landing node agents kept overwriting, +// and the attachment directories are plural — `assets/` and `images/`, per +// keg.NodeAttachmentsDir and keg.NodeImagesDir. A singular path in the guidance +// would produce links that upload fine and silently resolve to nothing, so the +// spelling is asserted rather than trusted. +func TestTap_Orient_StatesZeroNodeAndAttachmentPaths(t *testing.T) { + t.Parallel() + tap := newOrientTap(t) + + payload, err := tap.Orient(context.Background(), tapper.OrientOptions{}) + require.NoError(t, err) + + rules := payload[:strings.Index(payload, "## Available KEGs")] + require.Contains(t, rules, "Node 0 is the keg's placeholder landing node", + "the compact rules survive a context reset; node 0 belongs there") + require.Contains(t, rules, "(./assets/FILE)") + require.Contains(t, rules, "(./images/IMAGE)") + + guidance := payload[strings.Index(payload, "## Guidance"):] + require.Contains(t, guidance, "./assets/FILE") + require.Contains(t, guidance, "./images/IMAGE") + require.Contains(t, guidance, "## Node 0") + + // Guard the exact spelling: `asset/` or `image/` singular would be a silent + // break, since uploads succeed no matter how the link is later written. + for _, wrong := range []string{"./asset/", "./image/", "(assets/", "(images/"} { + require.NotContains(t, payload, wrong) + } +} diff --git a/schemas/flight-manifest.json b/schemas/flight-manifest.json index a2e190b..4d7c7ee 100644 --- a/schemas/flight-manifest.json +++ b/schemas/flight-manifest.json @@ -17,7 +17,7 @@ }, "capabilities": { "type": "array", - "items": {"type": "string", "enum": ["full_access", "manage_flights"]}, + "items": {"type": "string", "enum": ["full_access", "manage_flights", "manage_kegs"]}, "uniqueItems": true, "description": "Explicit agent capabilities granted to sessions using this flight." },