diff --git a/CLAUDE.md b/CLAUDE.md index 9976eb5c..cc1ca80e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -420,8 +420,10 @@ validation. lists) in one operation — a single round trip on RemoteKeg. - The keg settings file is named `keg` (no extension), though `keg.yaml` and `keg.yml` are also accepted. -- Node IDs are allocated by the Hub. `GET /nodes/next` is only a read-only - probe; creation uses `POST /nodes` with complete content. +- Node IDs are allocated by the Hub, which owns a per-keg counter that only + ever rises: a removed id is never handed out again. There is no way to read + the next id ahead of time — creation uses `POST /nodes` with complete + content, and the response carries every allocated id. - **Cobra skips PersistentPostRunE when RunE returns an error.** Any cleanup or logging that must run on both success and failure paths cannot rely on PersistentPostRunE. In tapper, invocation logging and log file cleanup are diff --git a/cliff.toml b/cliff.toml index 9eda2758..ef01168e 100644 --- a/cliff.toml +++ b/cliff.toml @@ -1,16 +1,33 @@ [changelog] header = "# Changelog\n\nAll notable changes to this project are documented in this file.\n" +# Breaking changes get their own section ahead of the type groups, and are +# marked inline within them. Without this a `feat(keg)!:` reads as an ordinary +# feature bullet, so a consumer scanning the changelog never learns their +# client broke. `breaking_description` is the `BREAKING CHANGE:` footer; +# git-cliff falls back to the subject when there is no footer, so it is only +# printed when it says something the subject did not. body = """ {% if version %} ## {{ version }} - {{ timestamp | date(format=\"%Y-%m-%d\") }} {% else %} ## Unreleased {% endif %} +{%- set breaking = commits | filter(attribute=\"breaking\", value=true) %} +{%- if breaking | length > 0 %} + +### ⚠️ Breaking Changes +{% for commit in breaking %} +**{% if commit.scope %}{{ commit.scope }}: {% endif %}{{ commit.message | trim }}** +{% if commit.breaking_description and commit.breaking_description | trim != commit.message | trim %} +{{ commit.breaking_description | trim }} +{% endif -%} +{%- endfor -%} +{%- endif %} {% for group, commits in commits | group_by(attribute=\"group\") %} ### {{ group }} {% for commit in commits -%} -- {% if commit.scope %}**{{ commit.scope }}:** {% endif %}{{ commit.message | trim }} +- {% if commit.breaking %}⚠️ {% endif %}{% if commit.scope %}**{{ commit.scope }}:** {% endif %}{{ commit.message | trim }} {% endfor %} {% endfor %} """ diff --git a/docs/ai-coding-agents/agent-conventions.md b/docs/ai-coding-agents/agent-conventions.md index 35af432b..2a105881 100644 --- a/docs/ai-coding-agents/agent-conventions.md +++ b/docs/ai-coding-agents/agent-conventions.md @@ -42,10 +42,10 @@ Always route through tapper's interfaces: - `mcp__tapper__cat` to read (supports `content_only`, `meta_only`, `stats_only`). -- `mcp__tapper__edit` to write content — accepts markdown with - frontmatter and separates it into `README.md` and `meta.yaml` - automatically. -- `mcp__tapper__meta` to update metadata without touching content. +- `mcp__tapper__edit` to write a node — `content` is the markdown body + and `meta` is the metadata document. They are separate fields, so + `content` must not carry a frontmatter block; send either one alone to + leave the other untouched, or both to replace the whole node. ## CLI and MCP share Hub concurrency rules @@ -69,7 +69,8 @@ another node first. If you are uncertain the removal is correct, defer. Snapshots do protect in-place edits. Take one before: - Any `mcp__tapper__edit` that rewrites more than a section. -- Any `mcp__tapper__meta` overwrite of existing tags or attributes. +- Any `mcp__tapper__edit` writing `meta`, which replaces the whole + metadata document and so overwrites existing tags and attributes. - `mcp__tapper__move` (the node survives the move, but a pre-move snapshot makes before/after diffs trivial). - Any multi-tool transformation where a mistake partway through would diff --git a/docs/ai-coding-agents/mcp-setup.md b/docs/ai-coding-agents/mcp-setup.md index 943d993d..e71793c8 100644 --- a/docs/ai-coding-agents/mcp-setup.md +++ b/docs/ai-coding-agents/mcp-setup.md @@ -106,31 +106,40 @@ explicitly configured root failed to initialize. | Tool | Description | | --- | --- | -| `create` | Atomically create 1–100 nodes from `nodes[]`; unique keys support forward/backward `{{node:key}}` body references | -| `edit` | Read with `cat`, then atomically replace 1–100 nodes from `edits[]`; every item requires that node's `expected_hash` | -| `meta` | Read `node_ids[]` without tokens, or read with `cat` and atomically replace metadata through `updates[]`; every update requires that node's `expected_hash` | +| `create` | Atomically create 1–100 nodes from `nodes[]`, each a markdown `content` document plus an optional YAML `meta` document; unique keys support forward/backward `{{node:key}}` content references | +| `edit` | Read with `cat`, then atomically replace `content`, `meta`, or both for 1–100 nodes from `nodes[]`; every item requires that node's `expected_hash`, and one hash covers content and metadata together | | `remove` | Read with `cat`, then atomically delete 1–100 `nodes[]`; every item requires its own `expected_hash` | | `move` | Read with `cat`, then move a node using its required `expected_hash` | | `keg_settings_edit` | Read the full document with `keg_settings`, then replace it using its required `expected_hash`; requires admin flight authority and editor/admin KEG access | -Mutation inputs are array-only and each array contains 1–100 items: +Mutation inputs are array-only, every batch tool takes its items under `nodes`, +and each array contains 1–100 items: ```json -{"nodes":[{"key":"plan","title":"Plan","body":"See [task](../{{node:task}})"}]} -{"edits":[{"node_id":"12","content":"# Revised","expected_hash":"...","snapshot_before":true}]} -{"node_ids":["12","13"]} -{"updates":[{"node_id":"12","content":"type: plan\n","expected_hash":"...","snapshot_before":true}]} +// create +{"nodes":[{"key":"plan","content":"# Plan\n\nSee [task](../{{node:task}})\n","meta":"type: plan\n"}]} +// edit — content only, metadata only, or both under one hash +{"nodes":[{"node_id":"12","content":"# Revised\n","expected_hash":"..."}]} +{"nodes":[{"node_id":"12","meta":"type: plan\n","expected_hash":"..."}]} +{"nodes":[{"node_id":"12","content":"# Revised\n","meta":"type: plan\n","expected_hash":"..."}]} +// remove {"nodes":[{"node_id":"12","expected_hash":"..."},{"node_id":"13","expected_hash":"..."}]} +// node_snapshot {"nodes":[{"node_id":"12","message":"reviewed"}]} ``` -The first and last shapes belong to `create` and `node_snapshot`; the middle -shapes cover `edit`, the two mutually exclusive `meta` modes, and `remove`. +Read metadata with `cat` and `meta_only`; take snapshots with `node_snapshot` +before a large or destructive edit. Mutation results preserve request order and report `node_id`, the resulting hash or snapshot revision, and advisory schema validation details when applicable. A failed batch returns no partial results and commits none of its changes. +Reads are self-contained: each `cat` row in `structuredContent.nodes[]` pairs +`node_id` and `hash` with that node's `content` and `meta`, matching the fields +`edit` accepts, so a read result can be modified and sent back without parsing +the human-readable rendering. + Every protected mutation names the read that supplies its token: `cat` for node edits, metadata updates, moves, and removals; `keg_settings` for settings; `schema_read` for schema edits/deletes; and `flight_show` for flight @@ -187,7 +196,6 @@ authenticated Hub identity; hosted MCP reports its single authenticated user. | Tool | Description | | --- | --- | -| `import_from_keg` | Import nodes from another keg | | `orient` | Return a read-only view of current instructions, selectable flights, and KEGs | | `session_refresh` | Retry activation after a broken explicit selection is repaired; zero arguments and no authority replacement once active | | `list_flights`, `flight_show` | Discover and inspect visible flights | @@ -196,7 +204,8 @@ authenticated Hub identity; hosted MCP reports its single authenticated user. MCP does not expose Tapper configuration, config templates, repository setup, archive import/export, raw auth status, license text, keg visibility, or namespace administration. Those remain external CLI, configuration, or Hub UI -operations. +operations. To put a node's content in another KEG, read it with `cat` and +`create` it there. The five batch mutation modes above intentionally use array-only inputs. Empty batches, batches over 100 items, duplicate keys/IDs, unknown create @@ -205,8 +214,6 @@ failure reject the entire call. Structured results preserve request order and include node IDs plus resulting hashes or snapshot revisions. The removed single-item fields are not accepted by the published MCP schemas. -`import_from_keg` requires editor identity and flight authority on the source -when `leave_stubs` is requested, because that option rewrites source nodes. Both transports also publish `tapper://orient` and the `tapper://node/{node_id}{?keg}` resource template. diff --git a/docs/query-expressions.md b/docs/query-expressions.md index 6640b3f2..8ad03218 100644 --- a/docs/query-expressions.md +++ b/docs/query-expressions.md @@ -9,7 +9,6 @@ expressions over tags and metadata attributes. - `tap tags --query EXPR` - `tap cat --query EXPR` - `tap rm --query EXPR` -- `tap import --query EXPR` ## Syntax diff --git a/integrations/content/agent-orient.md b/integrations/content/agent-orient.md index bb22f160..ab600efa 100644 --- a/integrations/content/agent-orient.md +++ b/integrations/content/agent-orient.md @@ -25,15 +25,20 @@ instructions as the authoritative context for the session. containing `README.md`, `meta.yaml`, and `stats.json`. These are tapper's internal storage format. Reading them bypasses the index; writing them bypasses locking and snapshot history. Always go through - `mcp__tapper__cat`, `mcp__tapper__edit`, `mcp__tapper__meta`, and related - tools. + `mcp__tapper__cat`, `mcp__tapper__edit`, and related tools. - **Treat the call-selected flight as MCP authority.** The root reference is pinned to the connection, but its manifest, transitive graph, and authorization are loaded before every authority-bearing call. Omit `flight` to use the root, or pass the root or one of the flattened descendants returned by orientation. A selected descendant contributes only its own instructions and authority; ancestor instructions and permission - caps are not inherited. `defaultKeg` does not grant authority. + caps are not inherited. No `keg` argument grants authority, and neither does + `defaultKeg`: naming a KEG chooses a target, and the flight decides whether + you may reach it. When orientation lists a KEG under "Reachable via + subflight", every call against it must carry that flight — reads included, so + `cat`, `links`, and `backlinks` need it just as much as `edit` does. + Omitting `flight` there returns `ORIENTATION_DENIED`, no matter what `keg` + says. - **Handle orientation failures explicitly.** `ORIENTATION_STALE` means authority raced between call resolution and Hub validation; `ORIENTATION_DENIED` means the selection is outside the accessible graph or lacks the diff --git a/integrations/content/linking.md b/integrations/content/linking.md index d042b6ac..45351cca 100644 --- a/integrations/content/linking.md +++ b/integrations/content/linking.md @@ -17,6 +17,12 @@ the same keg. A bare `keg:` reference in node prose is plain text: it does not create a graph link or backlink. Bare references remain valid as CLI arguments, configuration values, schema values, and tool parameters. +Linking across kegs is ordinary authoring, but *copying* nodes across them is +not an agent operation: no tool moves or duplicates nodes between kegs. Read +the source with `mcp__tapper__cat` and `mcp__tapper__create` the node in the +target, which also lets you adjust its links deliberately. Bulk transfer +between kegs is an operator task the user runs outside MCP. + ## Attachments A node's uploaded files and images live in two directories inside the node's diff --git a/integrations/content/snapshot-policy.md b/integrations/content/snapshot-policy.md index 340bfc1b..44f3e678 100644 --- a/integrations/content/snapshot-policy.md +++ b/integrations/content/snapshot-policy.md @@ -17,8 +17,8 @@ Snapshots do protect in-place edits. Take one before any of: - `mcp__tapper__edit` that rewrites more than a section, pipes in generated content, or replaces content the agent did not author. -- `mcp__tapper__meta` changes that overwrite existing tags, attributes, - or frontmatter. +- `mcp__tapper__edit` writing `meta`, which replaces the node's whole + metadata document and so overwrites existing tags and attributes. - `mcp__tapper__move` — while the node survives the move, a snapshot before the rename makes it easy to confirm the move did not lose content and to diff against the pre-move state. diff --git a/integrations/content/tool-inventory.md b/integrations/content/tool-inventory.md index 0af91853..1f122805 100644 --- a/integrations/content/tool-inventory.md +++ b/integrations/content/tool-inventory.md @@ -10,6 +10,27 @@ configuration, namespace/license discovery, `session_refresh`, `list_flights`, `flight_show`, and `keg_search` do not accept `flight`. MCP resources use root authority while rendering graph-wide discovery. +`flight` is an **operational** parameter, not a discovery-only one: `list`, +`cat`, `create`, `edit`, and `remove` all take it and all honour it. Pass the +**exact canonical name** orientation printed under "Selectable flights", +namespace sigil and `+` included: + +```json +{ "flight": "@admin/+mcp-smoke-readonly", "keg": "@admin/mcp-smoke-readonly", "limit": 10 } +``` + +A bare `mcp-smoke-readonly` or `+mcp-smoke-readonly` is not a canonical name. +Unqualified names resolve against the active KEG, so under a root whose cover is +empty there is nothing to resolve them against and the call fails +`ORIENTATION_DENIED` — which reads like a missing feature but is a name that did +not resolve. Re-read the orientation output and copy the name verbatim. + +`keg` never grants authority. It selects a target *within* the authority the +call already has; it cannot reach a KEG the selected flight does not cover. +Naming an uncovered KEG is `ORIENTATION_DENIED`, and that is the access control +working, not a bug. To widen what a call can reach, pass a `flight` that covers +it. + ## Orientation and management | Tool | Purpose | @@ -37,7 +58,7 @@ accessible transitive descendants. | `mcp__tapper__grep` | Regex search over node content. Supports `ignore_case`, `limit`, `max_lines`, and `id_only`. | | `mcp__tapper__tags` | List tags or filter nodes by a boolean expression over tags, attributes, and dot-prefix stats fields (for example `tapper and .created>2026-01-01`). | | `mcp__tapper__list` | List nodes in a keg with optional filters. | -| `mcp__tapper__cat` | Read one or more node bodies. Supports `meta_only`, `content_only`, `stats_only`, and `tag` expression selection as an alternative to explicit node IDs. | +| `mcp__tapper__cat` | Read one or more nodes. Each structured row pairs `node_id` and `hash` with that node's `content` and `meta`, so a read feeds straight into `edit`. Supports `meta_only`, `content_only`, `stats_only`, and `tag` expression selection as an alternative to explicit node IDs. | | `mcp__tapper__links` | Outbound links from a node. | | `mcp__tapper__backlinks` | Inbound links to a node. | | `mcp__tapper__list_indexes`, `mcp__tapper__index_cat` | Read generated index files (tag index, changelog, and others). | @@ -77,9 +98,8 @@ code; the index does the work in O(matches) rather than O(total). | Tool | Purpose | | ------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | -| `mcp__tapper__create` | Allocate a new numbered node. Accepts title, lead, tags, and attributes at creation time. | -| `mcp__tapper__edit` | Call `cat`, then atomically replace content for 1–100 nodes; every edit requires that node's returned hash. | -| `mcp__tapper__meta` | Read metadata without tokens, or call `cat` and atomically update 1–100 nodes; every update requires its hash. | +| `mcp__tapper__create` | Atomically create 1–100 nodes. Each is a markdown `content` document plus an optional YAML `meta` document; the title is the content's H1. Nodes in one batch reference each other with `{{node:KEY}}`. | +| `mcp__tapper__edit` | Call `cat`, then atomically replace `content`, `meta`, or both for 1–100 `nodes[]`; every item requires that node's returned hash, and one hash covers content and metadata together. | | `mcp__tapper__move` | Call `cat`, then relocate a node using its required returned hash. | | `mcp__tapper__remove` | Call `cat`, then atomically remove 1–100 `nodes[]`, each carrying its own required returned hash. | | `mcp__tapper__delete_file`, `mcp__tapper__delete_image` | Destructive attachment operations — see the Snapshots section below before calling. | @@ -92,3 +112,40 @@ Schema edits and deletes similarly require the hash from `schema_read`. Every conflict performs no operation: merge the change into returned current content or refetch with the corresponding read, then retry with the returned current hash. + +A hash covers exactly one write. Every successful write returns a new one and +invalidates the hash you sent, so a sequence like edit-then-delete needs a +fresh read between the two calls rather than a reused token. Node ids are +per-keg counters as well: node 4 in one keg is unrelated to node 4 in another. + +### Writing a node + +A node is two documents and nothing else: `content`, the markdown body whose H1 +is the title, and `meta`, the complete metadata document. Three placement rules +cover most first-attempt failures: + +- `schema` is a property of the item itself, a sibling of `meta` — never a key + inside the metadata. +- `meta` is a **YAML string**, not a JSON object. `"type: document\n"` is + right; `{"type": "document"}` is not. +- `content` must not open with a `---` frontmatter block. Metadata has one + home, and that is `meta`. + +Use `schema_list` to see the names a keg accepts, then: + +```json +{"nodes": [{"key": "a1", "content": "# Title\n\nBody", "meta": "type: document\n", "schema": "document"}]} +``` + +`edit` takes the same two documents per item plus that node's current hash from +`cat`, and either document may be omitted to leave it untouched: + +```json +{"nodes": [{"node_id": "12", "content": "# Revised\n\nBody", "expected_hash": "HASH_FROM_CAT"}]} +``` + +`remove` carries only ids and hashes: + +```json +{"nodes": [{"node_id": "12", "expected_hash": "HASH_FROM_CAT"}]} +``` diff --git a/integrations/rendered/claude/tapper/skills/tapper/SKILL.md b/integrations/rendered/claude/tapper/skills/tapper/SKILL.md index f2dd8403..1a00d570 100644 --- a/integrations/rendered/claude/tapper/skills/tapper/SKILL.md +++ b/integrations/rendered/claude/tapper/skills/tapper/SKILL.md @@ -30,15 +30,20 @@ instructions as the authoritative context for the session. containing `README.md`, `meta.yaml`, and `stats.json`. These are tapper's internal storage format. Reading them bypasses the index; writing them bypasses locking and snapshot history. Always go through - `mcp__tapper__cat`, `mcp__tapper__edit`, `mcp__tapper__meta`, and related - tools. + `mcp__tapper__cat`, `mcp__tapper__edit`, and related tools. - **Treat the call-selected flight as MCP authority.** The root reference is pinned to the connection, but its manifest, transitive graph, and authorization are loaded before every authority-bearing call. Omit `flight` to use the root, or pass the root or one of the flattened descendants returned by orientation. A selected descendant contributes only its own instructions and authority; ancestor instructions and permission - caps are not inherited. `defaultKeg` does not grant authority. + caps are not inherited. No `keg` argument grants authority, and neither does + `defaultKeg`: naming a KEG chooses a target, and the flight decides whether + you may reach it. When orientation lists a KEG under "Reachable via + subflight", every call against it must carry that flight — reads included, so + `cat`, `links`, and `backlinks` need it just as much as `edit` does. + Omitting `flight` there returns `ORIENTATION_DENIED`, no matter what `keg` + says. - **Handle orientation failures explicitly.** `ORIENTATION_STALE` means authority raced between call resolution and Hub validation; `ORIENTATION_DENIED` means the selection is outside the accessible graph or lacks the @@ -154,6 +159,27 @@ configuration, namespace/license discovery, `session_refresh`, `list_flights`, `flight_show`, and `keg_search` do not accept `flight`. MCP resources use root authority while rendering graph-wide discovery. +`flight` is an **operational** parameter, not a discovery-only one: `list`, +`cat`, `create`, `edit`, and `remove` all take it and all honour it. Pass the +**exact canonical name** orientation printed under "Selectable flights", +namespace sigil and `+` included: + +```json +{ "flight": "@admin/+mcp-smoke-readonly", "keg": "@admin/mcp-smoke-readonly", "limit": 10 } +``` + +A bare `mcp-smoke-readonly` or `+mcp-smoke-readonly` is not a canonical name. +Unqualified names resolve against the active KEG, so under a root whose cover is +empty there is nothing to resolve them against and the call fails +`ORIENTATION_DENIED` — which reads like a missing feature but is a name that did +not resolve. Re-read the orientation output and copy the name verbatim. + +`keg` never grants authority. It selects a target *within* the authority the +call already has; it cannot reach a KEG the selected flight does not cover. +Naming an uncovered KEG is `ORIENTATION_DENIED`, and that is the access control +working, not a bug. To widen what a call can reach, pass a `flight` that covers +it. + ## Orientation and management | Tool | Purpose | @@ -181,7 +207,7 @@ accessible transitive descendants. | `mcp__tapper__grep` | Regex search over node content. Supports `ignore_case`, `limit`, `max_lines`, and `id_only`. | | `mcp__tapper__tags` | List tags or filter nodes by a boolean expression over tags, attributes, and dot-prefix stats fields (for example `tapper and .created>2026-01-01`). | | `mcp__tapper__list` | List nodes in a keg with optional filters. | -| `mcp__tapper__cat` | Read one or more node bodies. Supports `meta_only`, `content_only`, `stats_only`, and `tag` expression selection as an alternative to explicit node IDs. | +| `mcp__tapper__cat` | Read one or more nodes. Each structured row pairs `node_id` and `hash` with that node's `content` and `meta`, so a read feeds straight into `edit`. Supports `meta_only`, `content_only`, `stats_only`, and `tag` expression selection as an alternative to explicit node IDs. | | `mcp__tapper__links` | Outbound links from a node. | | `mcp__tapper__backlinks` | Inbound links to a node. | | `mcp__tapper__list_indexes`, `mcp__tapper__index_cat` | Read generated index files (tag index, changelog, and others). | @@ -221,9 +247,8 @@ code; the index does the work in O(matches) rather than O(total). | Tool | Purpose | | ------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | -| `mcp__tapper__create` | Allocate a new numbered node. Accepts title, lead, tags, and attributes at creation time. | -| `mcp__tapper__edit` | Call `cat`, then atomically replace content for 1–100 nodes; every edit requires that node's returned hash. | -| `mcp__tapper__meta` | Read metadata without tokens, or call `cat` and atomically update 1–100 nodes; every update requires its hash. | +| `mcp__tapper__create` | Atomically create 1–100 nodes. Each is a markdown `content` document plus an optional YAML `meta` document; the title is the content's H1. Nodes in one batch reference each other with `{{node:KEY}}`. | +| `mcp__tapper__edit` | Call `cat`, then atomically replace `content`, `meta`, or both for 1–100 `nodes[]`; every item requires that node's returned hash, and one hash covers content and metadata together. | | `mcp__tapper__move` | Call `cat`, then relocate a node using its required returned hash. | | `mcp__tapper__remove` | Call `cat`, then atomically remove 1–100 `nodes[]`, each carrying its own required returned hash. | | `mcp__tapper__delete_file`, `mcp__tapper__delete_image` | Destructive attachment operations — see the Snapshots section below before calling. | @@ -237,6 +262,43 @@ conflict performs no operation: merge the change into returned current content or refetch with the corresponding read, then retry with the returned current hash. +A hash covers exactly one write. Every successful write returns a new one and +invalidates the hash you sent, so a sequence like edit-then-delete needs a +fresh read between the two calls rather than a reused token. Node ids are +per-keg counters as well: node 4 in one keg is unrelated to node 4 in another. + +### Writing a node + +A node is two documents and nothing else: `content`, the markdown body whose H1 +is the title, and `meta`, the complete metadata document. Three placement rules +cover most first-attempt failures: + +- `schema` is a property of the item itself, a sibling of `meta` — never a key + inside the metadata. +- `meta` is a **YAML string**, not a JSON object. `"type: document\n"` is + right; `{"type": "document"}` is not. +- `content` must not open with a `---` frontmatter block. Metadata has one + home, and that is `meta`. + +Use `schema_list` to see the names a keg accepts, then: + +```json +{"nodes": [{"key": "a1", "content": "# Title\n\nBody", "meta": "type: document\n", "schema": "document"}]} +``` + +`edit` takes the same two documents per item plus that node's current hash from +`cat`, and either document may be omitted to leave it untouched: + +```json +{"nodes": [{"node_id": "12", "content": "# Revised\n\nBody", "expected_hash": "HASH_FROM_CAT"}]} +``` + +`remove` carries only ids and hashes: + +```json +{"nodes": [{"node_id": "12", "expected_hash": "HASH_FROM_CAT"}]} +``` + ## Snapshots `mcp__tapper__node_snapshot` captures a node's current revision. Snapshots @@ -254,8 +316,8 @@ Snapshots do protect in-place edits. Take one before any of: - `mcp__tapper__edit` that rewrites more than a section, pipes in generated content, or replaces content the agent did not author. -- `mcp__tapper__meta` changes that overwrite existing tags, attributes, - or frontmatter. +- `mcp__tapper__edit` writing `meta`, which replaces the node's whole + metadata document and so overwrites existing tags and attributes. - `mcp__tapper__move` — while the node survives the move, a snapshot before the rename makes it easy to confirm the move did not lose content and to diff against the pre-move state. @@ -297,6 +359,12 @@ the same keg. A bare `keg:` reference in node prose is plain text: it does not create a graph link or backlink. Bare references remain valid as CLI arguments, configuration values, schema values, and tool parameters. +Linking across kegs is ordinary authoring, but *copying* nodes across them is +not an agent operation: no tool moves or duplicates nodes between kegs. Read +the source with `mcp__tapper__cat` and `mcp__tapper__create` the node in the +target, which also lets you adjust its links deliberately. Bulk transfer +between kegs is an operator task the user runs outside MCP. + ## Attachments A node's uploaded files and images live in two directories inside the node's diff --git a/integrations/rendered/codex/tapper/skills/tapper/SKILL.md b/integrations/rendered/codex/tapper/skills/tapper/SKILL.md index f2dd8403..1a00d570 100644 --- a/integrations/rendered/codex/tapper/skills/tapper/SKILL.md +++ b/integrations/rendered/codex/tapper/skills/tapper/SKILL.md @@ -30,15 +30,20 @@ instructions as the authoritative context for the session. containing `README.md`, `meta.yaml`, and `stats.json`. These are tapper's internal storage format. Reading them bypasses the index; writing them bypasses locking and snapshot history. Always go through - `mcp__tapper__cat`, `mcp__tapper__edit`, `mcp__tapper__meta`, and related - tools. + `mcp__tapper__cat`, `mcp__tapper__edit`, and related tools. - **Treat the call-selected flight as MCP authority.** The root reference is pinned to the connection, but its manifest, transitive graph, and authorization are loaded before every authority-bearing call. Omit `flight` to use the root, or pass the root or one of the flattened descendants returned by orientation. A selected descendant contributes only its own instructions and authority; ancestor instructions and permission - caps are not inherited. `defaultKeg` does not grant authority. + caps are not inherited. No `keg` argument grants authority, and neither does + `defaultKeg`: naming a KEG chooses a target, and the flight decides whether + you may reach it. When orientation lists a KEG under "Reachable via + subflight", every call against it must carry that flight — reads included, so + `cat`, `links`, and `backlinks` need it just as much as `edit` does. + Omitting `flight` there returns `ORIENTATION_DENIED`, no matter what `keg` + says. - **Handle orientation failures explicitly.** `ORIENTATION_STALE` means authority raced between call resolution and Hub validation; `ORIENTATION_DENIED` means the selection is outside the accessible graph or lacks the @@ -154,6 +159,27 @@ configuration, namespace/license discovery, `session_refresh`, `list_flights`, `flight_show`, and `keg_search` do not accept `flight`. MCP resources use root authority while rendering graph-wide discovery. +`flight` is an **operational** parameter, not a discovery-only one: `list`, +`cat`, `create`, `edit`, and `remove` all take it and all honour it. Pass the +**exact canonical name** orientation printed under "Selectable flights", +namespace sigil and `+` included: + +```json +{ "flight": "@admin/+mcp-smoke-readonly", "keg": "@admin/mcp-smoke-readonly", "limit": 10 } +``` + +A bare `mcp-smoke-readonly` or `+mcp-smoke-readonly` is not a canonical name. +Unqualified names resolve against the active KEG, so under a root whose cover is +empty there is nothing to resolve them against and the call fails +`ORIENTATION_DENIED` — which reads like a missing feature but is a name that did +not resolve. Re-read the orientation output and copy the name verbatim. + +`keg` never grants authority. It selects a target *within* the authority the +call already has; it cannot reach a KEG the selected flight does not cover. +Naming an uncovered KEG is `ORIENTATION_DENIED`, and that is the access control +working, not a bug. To widen what a call can reach, pass a `flight` that covers +it. + ## Orientation and management | Tool | Purpose | @@ -181,7 +207,7 @@ accessible transitive descendants. | `mcp__tapper__grep` | Regex search over node content. Supports `ignore_case`, `limit`, `max_lines`, and `id_only`. | | `mcp__tapper__tags` | List tags or filter nodes by a boolean expression over tags, attributes, and dot-prefix stats fields (for example `tapper and .created>2026-01-01`). | | `mcp__tapper__list` | List nodes in a keg with optional filters. | -| `mcp__tapper__cat` | Read one or more node bodies. Supports `meta_only`, `content_only`, `stats_only`, and `tag` expression selection as an alternative to explicit node IDs. | +| `mcp__tapper__cat` | Read one or more nodes. Each structured row pairs `node_id` and `hash` with that node's `content` and `meta`, so a read feeds straight into `edit`. Supports `meta_only`, `content_only`, `stats_only`, and `tag` expression selection as an alternative to explicit node IDs. | | `mcp__tapper__links` | Outbound links from a node. | | `mcp__tapper__backlinks` | Inbound links to a node. | | `mcp__tapper__list_indexes`, `mcp__tapper__index_cat` | Read generated index files (tag index, changelog, and others). | @@ -221,9 +247,8 @@ code; the index does the work in O(matches) rather than O(total). | Tool | Purpose | | ------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | -| `mcp__tapper__create` | Allocate a new numbered node. Accepts title, lead, tags, and attributes at creation time. | -| `mcp__tapper__edit` | Call `cat`, then atomically replace content for 1–100 nodes; every edit requires that node's returned hash. | -| `mcp__tapper__meta` | Read metadata without tokens, or call `cat` and atomically update 1–100 nodes; every update requires its hash. | +| `mcp__tapper__create` | Atomically create 1–100 nodes. Each is a markdown `content` document plus an optional YAML `meta` document; the title is the content's H1. Nodes in one batch reference each other with `{{node:KEY}}`. | +| `mcp__tapper__edit` | Call `cat`, then atomically replace `content`, `meta`, or both for 1–100 `nodes[]`; every item requires that node's returned hash, and one hash covers content and metadata together. | | `mcp__tapper__move` | Call `cat`, then relocate a node using its required returned hash. | | `mcp__tapper__remove` | Call `cat`, then atomically remove 1–100 `nodes[]`, each carrying its own required returned hash. | | `mcp__tapper__delete_file`, `mcp__tapper__delete_image` | Destructive attachment operations — see the Snapshots section below before calling. | @@ -237,6 +262,43 @@ conflict performs no operation: merge the change into returned current content or refetch with the corresponding read, then retry with the returned current hash. +A hash covers exactly one write. Every successful write returns a new one and +invalidates the hash you sent, so a sequence like edit-then-delete needs a +fresh read between the two calls rather than a reused token. Node ids are +per-keg counters as well: node 4 in one keg is unrelated to node 4 in another. + +### Writing a node + +A node is two documents and nothing else: `content`, the markdown body whose H1 +is the title, and `meta`, the complete metadata document. Three placement rules +cover most first-attempt failures: + +- `schema` is a property of the item itself, a sibling of `meta` — never a key + inside the metadata. +- `meta` is a **YAML string**, not a JSON object. `"type: document\n"` is + right; `{"type": "document"}` is not. +- `content` must not open with a `---` frontmatter block. Metadata has one + home, and that is `meta`. + +Use `schema_list` to see the names a keg accepts, then: + +```json +{"nodes": [{"key": "a1", "content": "# Title\n\nBody", "meta": "type: document\n", "schema": "document"}]} +``` + +`edit` takes the same two documents per item plus that node's current hash from +`cat`, and either document may be omitted to leave it untouched: + +```json +{"nodes": [{"node_id": "12", "content": "# Revised\n\nBody", "expected_hash": "HASH_FROM_CAT"}]} +``` + +`remove` carries only ids and hashes: + +```json +{"nodes": [{"node_id": "12", "expected_hash": "HASH_FROM_CAT"}]} +``` + ## Snapshots `mcp__tapper__node_snapshot` captures a node's current revision. Snapshots @@ -254,8 +316,8 @@ Snapshots do protect in-place edits. Take one before any of: - `mcp__tapper__edit` that rewrites more than a section, pipes in generated content, or replaces content the agent did not author. -- `mcp__tapper__meta` changes that overwrite existing tags, attributes, - or frontmatter. +- `mcp__tapper__edit` writing `meta`, which replaces the node's whole + metadata document and so overwrites existing tags and attributes. - `mcp__tapper__move` — while the node survives the move, a snapshot before the rename makes it easy to confirm the move did not lose content and to diff against the pre-move state. @@ -297,6 +359,12 @@ the same keg. A bare `keg:` reference in node prose is plain text: it does not create a graph link or backlink. Bare references remain valid as CLI arguments, configuration values, schema values, and tool parameters. +Linking across kegs is ordinary authoring, but *copying* nodes across them is +not an agent operation: no tool moves or duplicates nodes between kegs. Read +the source with `mcp__tapper__cat` and `mcp__tapper__create` the node in the +target, which also lets you adjust its links deliberately. Bulk transfer +between kegs is an operator task the user runs outside MCP. + ## Attachments A node's uploaded files and images live in two directories inside the node's diff --git a/pkg/cli/cmd_backlinks_test.go b/pkg/cli/cmd_backlinks_test.go index 06efd84b..86afbe1e 100644 --- a/pkg/cli/cmd_backlinks_test.go +++ b/pkg/cli/cmd_backlinks_test.go @@ -61,7 +61,7 @@ func TestBacklinksCommand_ListsBacklinkSources(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - targetCreate := NewProcess(t, false, "create", "--title", "Target").Run(sb.Context(), sb.Runtime()) + targetCreate := NewCreateProcess(t, false, "Target", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, targetCreate.Err) require.Equal(t, "1", strings.TrimSpace(string(targetCreate.Stdout))) @@ -96,7 +96,7 @@ func TestBacklinksCommand_OffsetSkipsResults(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - targetCreate := NewProcess(t, false, "create", "--title", "Target").Run(sb.Context(), sb.Runtime()) + targetCreate := NewCreateProcess(t, false, "Target", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, targetCreate.Err) createWithLinkToTarget(t, sb, "# Source A\n\nSee [target](../1).\n") @@ -124,11 +124,11 @@ func TestBacklinksCommand_MultipleNodeIDsMergesResults(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) // Create two target nodes. - target1 := NewProcess(t, false, "create", "--title", "Target1").Run(sb.Context(), sb.Runtime()) + target1 := NewCreateProcess(t, false, "Target1", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, target1.Err) require.Equal(t, "1", strings.TrimSpace(string(target1.Stdout))) - target2 := NewProcess(t, false, "create", "--title", "Target2").Run(sb.Context(), sb.Runtime()) + target2 := NewCreateProcess(t, false, "Target2", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, target2.Err) require.Equal(t, "2", strings.TrimSpace(string(target2.Stdout))) diff --git a/pkg/cli/cmd_cat_test.go b/pkg/cli/cmd_cat_test.go index 4c7f3a25..c7f0997f 100644 --- a/pkg/cli/cmd_cat_test.go +++ b/pkg/cli/cmd_cat_test.go @@ -362,7 +362,7 @@ func TestCatCommand_MultiNode_YAMLStream(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("joe", "~")) // Create a second node so we have two to cat. - createRes := NewProcess(t, false, "create", "--keg", "personal", "--title", "Second node").Run(sb.Context(), sb.Runtime()) + createRes := NewCreateProcess(t, false, "Second node", "", "--keg", "personal").Run(sb.Context(), sb.Runtime()) require.NoError(t, createRes.Err, "create should succeed") res := NewProcess(t, false, "cat", "0", "1", "--keg", "personal").Run(sb.Context(), sb.Runtime()) @@ -390,7 +390,7 @@ func TestCatCommand_MultiNode_ContentOnly(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("joe", "~")) // Create a second node so we have two to cat. - createRes := NewProcess(t, false, "create", "--keg", "personal", "--title", "Second node").Run(sb.Context(), sb.Runtime()) + createRes := NewCreateProcess(t, false, "Second node", "", "--keg", "personal").Run(sb.Context(), sb.Runtime()) require.NoError(t, createRes.Err, "create should succeed") res := NewProcess(t, false, "cat", "0", "1", "--keg", "personal", "--content-only").Run(sb.Context(), sb.Runtime()) @@ -412,7 +412,7 @@ func TestCatCommand_MultiNode_MetaOnly(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - createRes := NewProcess(t, false, "create", "--keg", "personal", "--title", "Second node").Run(sb.Context(), sb.Runtime()) + createRes := NewCreateProcess(t, false, "Second node", "", "--keg", "personal").Run(sb.Context(), sb.Runtime()) require.NoError(t, createRes.Err, "create should succeed") res := NewProcess(t, false, "cat", "0", "1", "--keg", "personal", "--meta-only").Run(sb.Context(), sb.Runtime()) @@ -431,7 +431,7 @@ func TestCatCommand_MultiNode_StatsOnly(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - createRes := NewProcess(t, false, "create", "--keg", "personal", "--title", "Second node").Run(sb.Context(), sb.Runtime()) + createRes := NewCreateProcess(t, false, "Second node", "", "--keg", "personal").Run(sb.Context(), sb.Runtime()) require.NoError(t, createRes.Err, "create should succeed") res := NewProcess(t, false, "cat", "0", "1", "--keg", "personal", "--stats-only").Run(sb.Context(), sb.Runtime()) @@ -571,7 +571,7 @@ func TestCatCommand_TTY_MultipleNodes_PrintsToStdout(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("joe", "~")) // Create a second node so we have two to cat. - createRes := NewProcess(t, false, "create", "--keg", "personal", "--title", "Second node"). + createRes := NewCreateProcess(t, false, "Second node", "", "--keg", "personal"). Run(sb.Context(), sb.Runtime()) require.NoError(t, createRes.Err, "create should succeed") diff --git a/pkg/cli/cmd_concurrent_test.go b/pkg/cli/cmd_concurrent_test.go index 5c1f22ab..1ae4dfee 100644 --- a/pkg/cli/cmd_concurrent_test.go +++ b/pkg/cli/cmd_concurrent_test.go @@ -29,7 +29,7 @@ func TestConcurrent_Creates_WithTitle(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - proc := NewProcess(t, false, "create", "--title", fmt.Sprintf("Node %d", idx)) + proc := NewCreateProcess(t, false, fmt.Sprintf("Node %d", idx), "") res := proc.Run(fx.Context(), fx.Runtime()) results[idx] = result{stdout: strings.TrimSpace(string(res.Stdout)), err: res.Err} }(i) @@ -95,7 +95,7 @@ func TestConcurrent_Edits_DifferentNodes(t *testing.T) { // Pre-create 5 nodes in the personal keg. nodeIDs := make([]string, N) for i := range N { - proc := NewProcess(t, false, "create", "--keg", "personal", "--title", fmt.Sprintf("Pre %d", i)) + proc := NewCreateProcess(t, false, fmt.Sprintf("Pre %d", i), "", "--keg", "personal") res := proc.Run(fx.Context(), fx.Runtime()) require.NoError(t, res.Err, "pre-create %d failed", i) nodeIDs[i] = strings.TrimSpace(string(res.Stdout)) @@ -146,7 +146,7 @@ func TestConcurrent_Creates_And_List(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - proc := NewProcess(t, false, "create", "--title", fmt.Sprintf("Created %d", idx)) + proc := NewCreateProcess(t, false, fmt.Sprintf("Created %d", idx), "") res := proc.Run(fx.Context(), fx.Runtime()) createErrs[idx] = res.Err }(i) @@ -187,7 +187,7 @@ func TestConcurrent_Creates_And_Cat(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - proc := NewProcess(t, false, "create", "--title", fmt.Sprintf("Created %d", idx)) + proc := NewCreateProcess(t, false, fmt.Sprintf("Created %d", idx), "") res := proc.Run(fx.Context(), fx.Runtime()) createErrs[idx] = res.Err }(i) @@ -226,7 +226,7 @@ func TestConcurrent_Creates_And_Edits(t *testing.T) { // Pre-create nodes for the editors to target. editTargets := make([]string, editors) for i := range editors { - proc := NewProcess(t, false, "create", "--keg", "personal", "--title", fmt.Sprintf("EditTarget %d", i)) + proc := NewCreateProcess(t, false, fmt.Sprintf("EditTarget %d", i), "", "--keg", "personal") res := proc.Run(fx.Context(), fx.Runtime()) require.NoError(t, res.Err, "pre-create edit target %d failed", i) editTargets[i] = strings.TrimSpace(string(res.Stdout)) @@ -245,7 +245,7 @@ func TestConcurrent_Creates_And_Edits(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - proc := NewProcess(t, false, "create", "--keg", "personal", "--title", fmt.Sprintf("Concurrent %d", idx)) + proc := NewCreateProcess(t, false, fmt.Sprintf("Concurrent %d", idx), "", "--keg", "personal") res := proc.Run(fx.Context(), fx.Runtime()) createResults[idx] = createResult{stdout: strings.TrimSpace(string(res.Stdout)), err: res.Err} }(i) @@ -301,7 +301,7 @@ func TestConcurrent_Creates_And_PipedEdits(t *testing.T) { // Pre-create nodes for the editors. editTargets := make([]string, editors) for i := range editors { - proc := NewProcess(t, false, "create", "--keg", "personal", "--title", fmt.Sprintf("PipedTarget %d", i)) + proc := NewCreateProcess(t, false, fmt.Sprintf("PipedTarget %d", i), "", "--keg", "personal") res := proc.Run(fx.Context(), fx.Runtime()) require.NoError(t, res.Err, "pre-create piped target %d failed", i) editTargets[i] = strings.TrimSpace(string(res.Stdout)) @@ -377,7 +377,7 @@ func TestConcurrent_Reindex_During_Creates(t *testing.T) { go func() { defer wg.Done() for i := range 5 { - proc := NewProcess(t, false, "create", "--title", fmt.Sprintf("During reindex %d", i)) + proc := NewCreateProcess(t, false, fmt.Sprintf("During reindex %d", i), "") res := proc.Run(fx.Context(), fx.Runtime()) if res.Err != nil { createErr = res.Err @@ -416,9 +416,7 @@ func TestConcurrent_Reads_And_Edits_IndexConsistency(t *testing.T) { // Pre-create N nodes, each with a unique tag. nodeIDs := make([]string, N) for i := range N { - proc := NewProcess(t, false, "create", - "--title", fmt.Sprintf("Original %d", i), - "--tags", fmt.Sprintf("pre%d", i)) + proc := NewCreateProcess(t, false, fmt.Sprintf("Original %d", i), fmt.Sprintf("tags:\n - pre%d\n", i)) res := proc.Run(fx.Context(), fx.Runtime()) require.NoError(t, res.Err, "pre-create %d failed", i) nodeIDs[i] = strings.TrimSpace(string(res.Stdout)) @@ -509,8 +507,7 @@ func TestConcurrent_Creates_And_Edits_IndexConsistency(t *testing.T) { // Pre-create nodes for the editors to target. editTargets := make([]string, editors) for i := range editors { - proc := NewProcess(t, false, "create", - "--title", fmt.Sprintf("EditMe %d", i)) + proc := NewCreateProcess(t, false, fmt.Sprintf("EditMe %d", i), "") res := proc.Run(fx.Context(), fx.Runtime()) require.NoError(t, res.Err, "pre-create edit target %d failed", i) editTargets[i] = strings.TrimSpace(string(res.Stdout)) @@ -529,9 +526,7 @@ func TestConcurrent_Creates_And_Edits_IndexConsistency(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - proc := NewProcess(t, false, "create", - "--title", fmt.Sprintf("New %d", idx), - "--tags", "freshly-created") + proc := NewCreateProcess(t, false, fmt.Sprintf("New %d", idx), "tags:\n - freshly-created\n") res := proc.Run(fx.Context(), fx.Runtime()) createResults[idx] = createResult{ stdout: strings.TrimSpace(string(res.Stdout)), diff --git a/pkg/cli/cmd_create.go b/pkg/cli/cmd_create.go index 6a804fad..b114ee26 100644 --- a/pkg/cli/cmd_create.go +++ b/pkg/cli/cmd_create.go @@ -11,8 +11,9 @@ import ( // // Usage examples: // -// Tap create --title "My note" --lead "one-line summary" -// Tap create --title "Note" --tags tag1 --tags tag2 --attrs foo=bar --attrs x=1 +// tap create +// tap create --schema task +// printf '---\ntype: task\n---\n# My note\n' | tap create func NewCreateCmd(deps *Deps) *cobra.Command { var opts tapper.CreateOptions @@ -26,12 +27,13 @@ If stdin is piped with non-empty content, it is used as the node body and no editor is launched. The content may optionally include YAML frontmatter; if present, the frontmatter is written to meta.yaml. -If no stdin and no flags are provided on a TTY, an editor is opened with a -pre-populated template. +Otherwise, on a TTY, an editor is opened on the new node. Everything about the +node is written there: the title is the H1, and metadata is the YAML +frontmatter above it. -If flags are provided without stdin, the node is created immediately from the -flag values without opening an editor. --schema selects the node schema and is -required when the keg is strict and the resolved human validation mode blocks.`, +--schema preselects the node type, which is prefilled as type: in the editor's +frontmatter and applied when you save. It is required when the keg is strict and +the resolved human validation mode blocks.`, RunE: func(cmd *cobra.Command, args []string) error { opts.Stream = deps.Runtime.Stream() applyKegTargetProfile(deps, &opts.KegTargetOptions) @@ -45,14 +47,7 @@ required when the keg is strict and the resolved human validation mode blocks.`, return nil }, } - cmd.Flags().StringVar(&opts.Title, "title", "", "title for the new node") cmd.Flags().StringVar(&opts.Schema, "schema", "", "schema to select for this write") - cmd.Flags().StringVar(&opts.Lead, "lead", "", "lead/short summary for the new node") - cmd.Flags().StringSliceVar(&opts.Tags, "tags", nil, "tags to apply to the node (repeatable)") - cmd.Flags().StringToStringVar( - &opts.Attrs, "attrs", nil, - "attributes as key=value pairs (repeatable)", - ) return cmd } diff --git a/pkg/cli/cmd_create_test.go b/pkg/cli/cmd_create_test.go index 4fce19e0..eca0465f 100644 --- a/pkg/cli/cmd_create_test.go +++ b/pkg/cli/cmd_create_test.go @@ -13,6 +13,7 @@ func TestCreate_Table(t *testing.T) { cases := []struct { name string args []string + stdin string exactOut string outRegex string wantReadmeNotEmpty bool @@ -22,7 +23,8 @@ func TestCreate_Table(t *testing.T) { }{ { name: "default_keg", - args: []string{"create", "--title", "Note", "--lead", "one-line"}, + args: []string{"create"}, + stdin: "# Note\n\none-line\n", exactOut: "1", readmeContains: []string{ "# Note", @@ -47,7 +49,8 @@ func TestCreate_Table(t *testing.T) { }, { name: "with_tags", - args: []string{"create", "--title", "Tagged", "--lead", "has tags", "--tags", "alpha", "--tags", "beta"}, + args: []string{"create"}, + stdin: "---\ntags:\n - alpha\n - beta\n---\n# Tagged\n\nhas tags\n", outRegex: `^\d+`, metaContains: []string{ "tags:", @@ -57,7 +60,8 @@ func TestCreate_Table(t *testing.T) { }, { name: "with_schema", - args: []string{"create", "--title", "Schema Note", "--schema", "note"}, + args: []string{"create", "--schema", "note"}, + stdin: "# Schema Note\n", outRegex: `^\d+`, metaContains: []string{ "type: note", @@ -70,6 +74,9 @@ func TestCreate_Table(t *testing.T) { // Set up a fresh fixture per case so repository state is isolated. fx := NewSandbox(t, testutils.WithFixture("testuser", "/home/testuser")) h := NewProcess(t, false, tc.args...) + if tc.stdin != "" { + h.SetStdin(strings.NewReader(tc.stdin)) + } // Capture the fixture time for timestamp assertions. now := fx.Now().Format(time.RFC3339) diff --git a/pkg/cli/cmd_edit_test.go b/pkg/cli/cmd_edit_test.go index 0c278814..f2166454 100644 --- a/pkg/cli/cmd_edit_test.go +++ b/pkg/cli/cmd_edit_test.go @@ -76,7 +76,7 @@ summary: from stdin func TestEdit_PipedSchemaSelectionPersistsType(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - created := NewProcess(t, false, "create", "--keg", "personal", "--title", "Editable").Run(sb.Context(), sb.Runtime()) + created := NewCreateProcess(t, false, "Editable", "", "--keg", "personal").Run(sb.Context(), sb.Runtime()) require.NoError(t, created.Err) res := NewProcess(t, false, "edit", "1", "--keg", "personal", "--schema", "task"). diff --git a/pkg/cli/cmd_import.go b/pkg/cli/cmd_import.go deleted file mode 100644 index b4a79722..00000000 --- a/pkg/cli/cmd_import.go +++ /dev/null @@ -1,103 +0,0 @@ -package cli - -import ( - "fmt" - "regexp" - - "github.com/jlrickert/tapper/pkg/tapper" - "github.com/spf13/cobra" -) - -// kegImportArgRefRE matches keg:ALIAS/N argument format. -var kegImportArgRefRE = regexp.MustCompile(`^keg:([a-zA-Z0-9][a-zA-Z0-9_-]*)/([0-9]+)$`) - -func NewImportCmd(deps *Deps) *cobra.Command { - var opts tapper.ImportFromKegOptions - var fromKeg string - - opts.SkipZeroNode = true - - cmd := &cobra.Command{ - Use: "import [NODE_ID | keg:ALIAS/NODE_ID]...", - Short: "import nodes from another keg", - Long: `Import nodes from a source keg into the target keg. - -Each imported node is assigned a fresh ID. Links in the copied content are -rewritten: - - ../N (imported) -> ../NEW_ID - ../N (not imported) -> keg:SOURCE/N - keg:TARGET/N -> ../N - keg:SOURCE/N (imported) -> ../NEW_ID - keg:SOURCE/N (other) -> unchanged - keg:OTHER/N -> unchanged - -Nodes may be specified as bare IDs with --from SOURCE, or as keg:ALIAS/NODE_ID -references. All must come from the same source keg.`, - RunE: func(cmd *cobra.Command, args []string) error { - // Extract source alias from keg:ALIAS/N args when --from is absent. - if fromKeg == "" { - for _, arg := range args { - if m := kegImportArgRefRE.FindStringSubmatch(arg); m != nil { - if fromKeg != "" && m[1] != fromKeg { - return fmt.Errorf("conflicting source keg aliases %q and %q in arguments", fromKeg, m[1]) - } - fromKeg = m[1] - } - } - } - if fromKeg == "" { - return fmt.Errorf("--from SOURCE is required (or use keg:ALIAS/NODE_ID references)") - } - - opts.Source.Keg = fromKeg - opts.NodeIDs = args - applyKegTargetProfile(deps, &opts.Target) - - imported, err := deps.Tap.ImportFromKeg(cmd.Context(), opts) - if err != nil { - return err - } - for _, node := range imported { - if _, err := fmt.Fprintf(cmd.OutOrStdout(), "%s -> %s\n", - node.SourceID.Path(), node.TargetID.Path()); err != nil { - return err - } - } - if len(imported) > 0 { - if _, err := fmt.Fprintf(cmd.OutOrStdout(), "\nimported %d node(s)\n", len(imported)); err != nil { - return err - } - } - return nil - }, - } - - cmd.Flags().StringVar(&fromKeg, "from", "", "source keg alias; required when using bare node IDs") - cmd.Flags().StringVar(&opts.TagQuery, "query", "", `boolean expression (see "tap docs query-expressions" for syntax)`) - cmd.Flags().BoolVar(&opts.LeaveStubs, "leave-stubs", false, "write forwarding stubs at source node locations after import") - cmd.Flags().BoolVar(&opts.SkipZeroNode, "skip-zero", true, "skip source node 0 (default true)") - - mustRegisterFlagCompletion(cmd, "from", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - // Kegs are no longer enumerable from config (the alias map is gone); the - // source keg must be typed as a reference (@ns/name or a path). - return nil, cobra.ShellCompDirectiveNoFileComp - }) - - cmd.ValidArgsFunction = func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - src, _ := cmd.Flags().GetString("from") - if src == "" || deps.Tap == nil { - return nil, cobra.ShellCompDirectiveNoFileComp - } - ids, err := deps.Tap.List(cmd.Context(), tapper.ListOptions{ - KegTargetOptions: tapper.KegTargetOptions{Keg: src}, - IdOnly: true, - }) - if err != nil { - return nil, cobra.ShellCompDirectiveError - } - return ids, cobra.ShellCompDirectiveNoFileComp - } - - return cmd -} diff --git a/pkg/cli/cmd_import_test.go b/pkg/cli/cmd_import_test.go deleted file mode 100644 index 5968135e..00000000 --- a/pkg/cli/cmd_import_test.go +++ /dev/null @@ -1,165 +0,0 @@ -package cli_test - -import ( - "strings" - "testing" - - testutils "github.com/jlrickert/cli-toolkit/sandbox" - "github.com/stretchr/testify/require" -) - -// TestImportCmd_BasicCopyWithLinkRewrite imports two named nodes from the -// "personal" keg into the "work" keg and verifies: -// - correct output lines and summary -// - link to an imported node stays relative (../NEW_ID) -// - link to a non-imported node is rewritten to keg:personal/N -func TestImportCmd_BasicCopyWithLinkRewrite(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - // personal node 1 = Personal Overview (links to ../2 and ../3) - // personal node 2 = Project Alpha (links to ../1 and ../3) - // work keg has only node 0, so next IDs are 1, 2. - res := NewProcess(t, false, "import", "--from", "personal", "1", "2", "--keg", "work"). - Run(sb.Context(), sb.Runtime()) - require.NoError(t, res.Err) - - out := string(res.Stdout) - // Each imported node emits "SRC -> DST" - require.Contains(t, out, "1 -> 1") - require.Contains(t, out, "2 -> 2") - require.Contains(t, out, "imported 2 node(s)") - - // Work node 1 (was personal/1): ../2 is imported → stays ../2; ../3 not imported → keg:personal/3 - node1 := fixtureContent(t, sb.Runtime(), "work", "1") - require.Contains(t, node1, "# Personal Overview") - require.Contains(t, node1, "../2", "link to imported node 2 should remain relative") - require.Contains(t, node1, "keg:personal/3", "link to non-imported node 3 should be cross-keg") - require.NotContains(t, node1, "../3", "bare ../3 must not remain") - - // Work node 2 (was personal/2): ../1 imported → ../1; ../3 not imported → keg:personal/3 - node2 := fixtureContent(t, sb.Runtime(), "work", "2") - require.Contains(t, node2, "# Project Alpha") - require.Contains(t, node2, "keg:personal/3") -} - -// TestImportCmd_KegRefArgFormat verifies that keg:ALIAS/NODE_ID positional -// arguments are accepted when --from is absent; the alias is extracted from -// the first argument. -func TestImportCmd_KegRefArgFormat(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - res := NewProcess(t, false, "import", "keg:personal/1", "keg:personal/2", "--keg", "work"). - Run(sb.Context(), sb.Runtime()) - require.NoError(t, res.Err) - require.Contains(t, string(res.Stdout), "imported 2 node(s)") -} - -// TestImportCmd_AllNodesSkipsZero imports all nodes from "personal" to "work" -// without specifying node IDs. Node 0 must be skipped by default. -func TestImportCmd_AllNodesSkipsZero(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - // personal has nodes 0, 1, 2, 3 — so 3 non-zero nodes should be imported. - res := NewProcess(t, false, "import", "--from", "personal", "--keg", "work"). - Run(sb.Context(), sb.Runtime()) - require.NoError(t, res.Err) - - out := string(res.Stdout) - require.Contains(t, out, "imported 3 node(s)") - - // Node 0 from personal must NOT be present in work (work already has its own 0). - // Work's node 0 content should be unchanged. - node0 := fixtureContent(t, sb.Runtime(), "work", "0") - require.Contains(t, node0, "Sorry, planned but not yet available") -} - -// TestImportCmd_SkipZeroFalseIncludesZero verifies that --skip-zero=false -// causes node 0 to be imported. -func TestImportCmd_SkipZeroFalseIncludesZero(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - res := NewProcess(t, false, "import", "--from", "personal", "--keg", "work", "--skip-zero=false"). - Run(sb.Context(), sb.Runtime()) - require.NoError(t, res.Err) - - out := string(res.Stdout) - // 4 nodes: 0, 1, 2, 3 - require.Contains(t, out, "imported 4 node(s)") -} - -// TestImportCmd_LeaveStubs verifies that --leave-stubs replaces each source -// node's README with a forwarding stub pointing to the new location. -func TestImportCmd_LeaveStubs(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - res := NewProcess(t, false, "import", "--from", "personal", "1", "--keg", "work", "--leave-stubs"). - Run(sb.Context(), sb.Runtime()) - require.NoError(t, res.Err) - - // personal/1/README.md should now be a stub. - stub := fixtureContent(t, sb.Runtime(), "personal", "1") - require.Contains(t, stub, "Personal Overview") - require.Contains(t, stub, "keg:work/") - require.Contains(t, stub, "Moved to") - // The stub should not contain the original body. - require.NotContains(t, stub, "An index of personal notes") -} - -// TestImportCmd_SelfImportError verifies that importing into the same keg as -// the source produces an error. -func TestImportCmd_SelfImportError(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - res := NewProcess(t, false, "import", "--from", "personal", "1", "--keg", "personal"). - Run(sb.Context(), sb.Runtime()) - require.Error(t, res.Err) - require.Contains(t, strings.ToLower(string(res.Stderr)), "same") -} - -// TestImportCmd_MissingFromError verifies that an error is returned when no -// --from flag and no keg:ALIAS/NODE_ID arguments are provided. -func TestImportCmd_MissingFromError(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - res := NewProcess(t, false, "import", "1", "2", "--keg", "work"). - Run(sb.Context(), sb.Runtime()) - require.Error(t, res.Err) - require.Contains(t, string(res.Stderr), "--from") -} - -// TestImportCmd_ConflictingAliasInArgsError verifies that using keg:ALIAS/N -// args with different aliases produces an error. -func TestImportCmd_ConflictingAliasInArgsError(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - res := NewProcess(t, false, "import", "keg:personal/1", "keg:work/1", "--keg", "example"). - Run(sb.Context(), sb.Runtime()) - require.Error(t, res.Err) -} - -// TestImportCmd_NodesNotInTarget verifies that after import the dex in the -// target keg lists the newly imported nodes. -func TestImportCmd_DexUpdatedAfterImport(t *testing.T) { - t.Parallel() - sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - - res := NewProcess(t, false, "import", "--from", "personal", "1", "2", "3", "--keg", "work"). - Run(sb.Context(), sb.Runtime()) - require.NoError(t, res.Err) - - // Use tap list to verify the work keg now lists the imported nodes. - res = NewProcess(t, false, "list", "--keg", "work").Run(sb.Context(), sb.Runtime()) - require.NoError(t, res.Err) - listOut := string(res.Stdout) - require.Contains(t, listOut, "Personal Overview") - require.Contains(t, listOut, "Project Alpha") - require.Contains(t, listOut, "Meeting Notes") -} diff --git a/pkg/cli/cmd_links_test.go b/pkg/cli/cmd_links_test.go index 8af27315..ca085609 100644 --- a/pkg/cli/cmd_links_test.go +++ b/pkg/cli/cmd_links_test.go @@ -62,11 +62,11 @@ func TestLinksCommand_ListsOutgoingLinks(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) // Create target nodes first (no links). - targetA := NewProcess(t, false, "create", "--title", "Target A").Run(sb.Context(), sb.Runtime()) + targetA := NewCreateProcess(t, false, "Target A", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, targetA.Err) require.Equal(t, "1", strings.TrimSpace(string(targetA.Stdout))) - targetB := NewProcess(t, false, "create", "--title", "Target B").Run(sb.Context(), sb.Runtime()) + targetB := NewCreateProcess(t, false, "Target B", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, targetB.Err) require.Equal(t, "2", strings.TrimSpace(string(targetB.Stdout))) @@ -104,7 +104,7 @@ func TestLinksCommand_OffsetSkipsResults(t *testing.T) { // Create 3 target nodes. for _, title := range []string{"Target A", "Target B", "Target C"} { - res := NewProcess(t, false, "create", "--title", title).Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, title, "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) } @@ -143,7 +143,7 @@ func TestLinksCommand_MultipleNodeIDsMergesResults(t *testing.T) { // Create target nodes. for _, title := range []string{"Target A", "Target B", "Target C"} { - res := NewProcess(t, false, "create", "--title", title).Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, title, "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) } diff --git a/pkg/cli/cmd_list_test.go b/pkg/cli/cmd_list_test.go index 27129f53..31901b2a 100644 --- a/pkg/cli/cmd_list_test.go +++ b/pkg/cli/cmd_list_test.go @@ -15,9 +15,9 @@ func TestListCommand_IdOnlyOutputsOnlyIDs(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "One").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "One", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) defaultRes := NewProcess(t, false, "list").Run(sb.Context(), sb.Runtime()) @@ -44,11 +44,11 @@ func TestListCommand_ReverseOrdering(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "One").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "One", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Three").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Three", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) normal := NewProcess(t, false, "list", "--id-only").Run(sb.Context(), sb.Runtime()) @@ -75,7 +75,7 @@ func TestListCommand_StaleIndexDoesNotCrash(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) // Create one node through the normal path so the dex has entries. - res := NewProcess(t, false, "create", "--title", "Indexed").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Indexed", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Write bare node directories directly on disk (content only, no dex update). @@ -110,15 +110,15 @@ func TestListCommand_SortUpdated(t *testing.T) { // Create nodes with advancing clock so they have different updated timestamps. sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "First").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "First", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Second").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Second", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Third").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Third", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Sort by updated: oldest first, newest last. @@ -139,15 +139,15 @@ func TestListCommand_SortUpdated_WithLimit(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "A").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "A", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "B").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "B", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "C").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "C", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Limit to first 2 by updated order (oldest first). @@ -171,7 +171,7 @@ func TestListCommand_FormatCreatedTimestamp(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "FormatTest").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "FormatTest", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Use %c to show created timestamp @@ -218,7 +218,7 @@ func TestListCommand_OffsetSkipsResults(t *testing.T) { // Create 3 nodes (plus zero node = 4 total). for _, title := range []string{"One", "Two", "Three"} { - res := NewProcess(t, false, "create", "--title", title).Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, title, "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) } @@ -242,7 +242,7 @@ func TestListCommand_OffsetWithLimit(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) for _, title := range []string{"A", "B", "C", "D", "E"} { - res := NewProcess(t, false, "create", "--title", title).Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, title, "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) } @@ -283,11 +283,11 @@ func TestListCommand_DotPrefixQuery_CreatedGT(t *testing.T) { // The zero node is created at the sandbox base time (2000-01-01 00:00:00 UTC // is the default for test sandboxes). sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "Early").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Early", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) sb.Advance(48 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Late").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Late", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // First, get the actual timestamps from the list output to compute a midpoint. @@ -336,11 +336,11 @@ func TestListCommand_DotPrefixQuery_CombinedWithAttribute(t *testing.T) { // The zero node exists from the fixture. Create an additional node and // query with a stats field combined with a tag. sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "Tagged", "--tags", "golang").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Tagged", "tags:\n - golang\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Untagged").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Untagged", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // ".created and golang" should return only the tagged node. @@ -360,11 +360,11 @@ func TestListCommand_DotPrefixQuery_BooleanCheck(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "A").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "A", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "B").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "B", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // ".created" without operator is a boolean check: all nodes with a @@ -391,7 +391,7 @@ func TestListCommand_AttrCompare_EntityNotEqual(t *testing.T) { // Create nodes with different entity values. sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "Plan node").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Plan node", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Set meta for node 1: entity=plan stdin := strings.NewReader("entity: plan\ntags:\n - golang\n") @@ -399,7 +399,7 @@ func TestListCommand_AttrCompare_EntityNotEqual(t *testing.T) { require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Task node").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Task node", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Set meta for node 2: entity=task stdin = strings.NewReader("entity: task\ntags:\n - golang\n") @@ -407,7 +407,7 @@ func TestListCommand_AttrCompare_EntityNotEqual(t *testing.T) { require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Concept node").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Concept node", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Set meta for node 3: entity=concept stdin = strings.NewReader("entity: concept\n") @@ -434,21 +434,21 @@ func TestListCommand_AttrCompare_NumericGte(t *testing.T) { // Create nodes with different numeric metadata values. sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "Low score").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Low score", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) stdin := strings.NewReader("score: 0.3\n") res = NewProcess(t, false, "meta", "1").RunWithIO(sb.Context(), sb.Runtime(), stdin) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Mid score").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Mid score", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) stdin = strings.NewReader("score: 0.5\n") res = NewProcess(t, false, "meta", "2").RunWithIO(sb.Context(), sb.Runtime(), stdin) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "High score").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "High score", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) stdin = strings.NewReader("score: 0.8\n") res = NewProcess(t, false, "meta", "3").RunWithIO(sb.Context(), sb.Runtime(), stdin) @@ -473,14 +473,14 @@ func TestListCommand_AttrCompare_BackwardCompat_EntityEquals(t *testing.T) { // Create a node with entity=plan. sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "Plan node").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Plan node", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) stdin := strings.NewReader("entity: plan\ntags:\n - golang\n") res = NewProcess(t, false, "meta", "1").RunWithIO(sb.Context(), sb.Runtime(), stdin) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Task node").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Task node", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) stdin = strings.NewReader("entity: task\n") res = NewProcess(t, false, "meta", "2").RunWithIO(sb.Context(), sb.Runtime(), stdin) @@ -504,14 +504,14 @@ func TestListCommand_AttrCompare_MixedWithDotPrefix(t *testing.T) { // Create nodes: one is entity=plan, one is entity=task. sb.Advance(1 * time.Hour) - res := NewProcess(t, false, "create", "--title", "Plan node").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Plan node", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) stdin := strings.NewReader("entity: plan\n") res = NewProcess(t, false, "meta", "1").RunWithIO(sb.Context(), sb.Runtime(), stdin) require.NoError(t, res.Err) sb.Advance(1 * time.Hour) - res = NewProcess(t, false, "create", "--title", "Task node").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Task node", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) stdin = strings.NewReader("entity: task\n") res = NewProcess(t, false, "meta", "2").RunWithIO(sb.Context(), sb.Runtime(), stdin) diff --git a/pkg/cli/cmd_meta_test.go b/pkg/cli/cmd_meta_test.go index e492aa55..36f51bb7 100644 --- a/pkg/cli/cmd_meta_test.go +++ b/pkg/cli/cmd_meta_test.go @@ -103,7 +103,7 @@ tags: func TestMetaCommand_ReplaceFromStdinPersistsSchemaSelection(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - created := NewProcess(t, false, "create", "--keg", "personal", "--title", "Metadata target").Run(sb.Context(), sb.Runtime()) + created := NewCreateProcess(t, false, "Metadata target", "", "--keg", "personal").Run(sb.Context(), sb.Runtime()) require.NoError(t, created.Err) stdin := strings.NewReader("summary: selected\n") diff --git a/pkg/cli/cmd_mv_test.go b/pkg/cli/cmd_mv_test.go index 36e3cf58..1e9e9cc6 100644 --- a/pkg/cli/cmd_mv_test.go +++ b/pkg/cli/cmd_mv_test.go @@ -12,9 +12,9 @@ func TestMoveCommand_RewritesLinks(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "One").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "One", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) fixtureSetContent(t, sb.Runtime(), "example", "1", "# One\n\nSee [two](../2).\nAlso ../2.\n") @@ -39,11 +39,11 @@ func TestMoveCommand_ErrorCases(t *testing.T) { require.Error(t, res.Err) require.Contains(t, string(res.Stderr), "node 999 not found") - res = NewProcess(t, false, "create", "--title", "One").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "One", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Three").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Three", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) res = NewProcess(t, false, "mv", "2", "3").Run(sb.Context(), sb.Runtime()) diff --git a/pkg/cli/cmd_rm_test.go b/pkg/cli/cmd_rm_test.go index dc0fe987..fd49b41b 100644 --- a/pkg/cli/cmd_rm_test.go +++ b/pkg/cli/cmd_rm_test.go @@ -12,7 +12,7 @@ func TestRemoveCommand_DeletesNode(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "Delete me").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Delete me", "").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) res = NewProcess(t, false, "rm", "1").Run(sb.Context(), sb.Runtime()) diff --git a/pkg/cli/cmd_root.go b/pkg/cli/cmd_root.go index 72f433e4..6656da03 100644 --- a/pkg/cli/cmd_root.go +++ b/pkg/cli/cmd_root.go @@ -294,7 +294,6 @@ func NewRootCmd(deps *Deps) *cobra.Command { NewGrepCmd(deps), NewHubCmd(deps), NewImageCmd(deps), - NewImportCmd(deps), NewIndexCmd(deps), NewInfoCmd(deps), NewLinksCmd(deps), diff --git a/pkg/cli/cmd_root_flags_test.go b/pkg/cli/cmd_root_flags_test.go index 364ac18d..7c80dab2 100644 --- a/pkg/cli/cmd_root_flags_test.go +++ b/pkg/cli/cmd_root_flags_test.go @@ -87,7 +87,7 @@ func TestTap_DirectCreateBypassesViewerFlightCap(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("joe", "~")) - res := NewProcess(t, false, "create", "--keg", "personal", "--flight", "+focused", "--title", "Allowed CLI Write").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Allowed CLI Write", "", "--keg", "personal", "--flight", "+focused").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) nodeID := strings.TrimSpace(string(res.Stdout)) require.NotEmpty(t, nodeID) diff --git a/pkg/cli/cmd_tags_completion_test.go b/pkg/cli/cmd_tags_completion_test.go index e0b575f4..d32f3a7d 100644 --- a/pkg/cli/cmd_tags_completion_test.go +++ b/pkg/cli/cmd_tags_completion_test.go @@ -11,11 +11,11 @@ func TestTagsCommand_CompletionSuggestsTags(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "One", "--tags", "zeta").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "One", "tags:\n - zeta\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two", "--tags", "alpha").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "tags:\n - alpha\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Three", "--tags", "beta").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Three", "tags:\n - beta\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) comp := NewCompletionProcess(t, false, 0, "tags", "").Run(sb.Context(), sb.Runtime()) @@ -29,11 +29,11 @@ func TestTagsCommand_CompletionFiltersByPrefix(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "One", "--tags", "alpha").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "One", "tags:\n - alpha\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two", "--tags", "alpine").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "tags:\n - alpine\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Three", "--tags", "beta").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Three", "tags:\n - beta\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) comp := NewCompletionProcess(t, false, 0, "tags", "al").Run(sb.Context(), sb.Runtime()) diff --git a/pkg/cli/cmd_tags_test.go b/pkg/cli/cmd_tags_test.go index dfd2e566..a82ce3d9 100644 --- a/pkg/cli/cmd_tags_test.go +++ b/pkg/cli/cmd_tags_test.go @@ -55,11 +55,11 @@ func TestTagsCommand_ListAllTagsSorted(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "One", "--tags", "zeta").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "One", "tags:\n - zeta\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two", "--tags", "alpha").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "tags:\n - alpha\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Three", "--tags", "beta").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Three", "tags:\n - beta\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) out := NewProcess(t, false, "tags").Run(sb.Context(), sb.Runtime()) @@ -75,11 +75,11 @@ func TestTagsCommand_ListNodesForTag(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "Alpha Node", "--tags", "fire").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Alpha Node", "tags:\n - fire\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) require.Equal(t, "1", strings.TrimSpace(string(res.Stdout))) - res = NewProcess(t, false, "create", "--title", "Beta Node", "--tags", "fire", "--tags", "earth").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Beta Node", "tags:\n - fire\n - earth\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) require.Equal(t, "2", strings.TrimSpace(string(res.Stdout))) @@ -100,15 +100,15 @@ func TestTagsCommand_TagExpression(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "Node AB", "--tags", "a", "--tags", "b").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "Node AB", "tags:\n - a\n - b\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) require.Equal(t, "1", strings.TrimSpace(string(res.Stdout))) - res = NewProcess(t, false, "create", "--title", "Node AC", "--tags", "a", "--tags", "c").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Node AC", "tags:\n - a\n - c\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) require.Equal(t, "2", strings.TrimSpace(string(res.Stdout))) - res = NewProcess(t, false, "create", "--title", "Node C", "--tags", "c").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Node C", "tags:\n - c\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) require.Equal(t, "3", strings.TrimSpace(string(res.Stdout))) @@ -139,7 +139,7 @@ func TestTagsCommand_OffsetWithTagFilter(t *testing.T) { sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) for _, title := range []string{"A", "B", "C"} { - res := NewProcess(t, false, "create", "--title", title, "--tags", "group").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, title, "tags:\n - group\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) } @@ -163,11 +163,11 @@ func TestTagsCommand_OffsetListAllTags(t *testing.T) { t.Parallel() sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) - res := NewProcess(t, false, "create", "--title", "One", "--tags", "alpha").Run(sb.Context(), sb.Runtime()) + res := NewCreateProcess(t, false, "One", "tags:\n - alpha\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Two", "--tags", "beta").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Two", "tags:\n - beta\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) - res = NewProcess(t, false, "create", "--title", "Three", "--tags", "gamma").Run(sb.Context(), sb.Runtime()) + res = NewCreateProcess(t, false, "Three", "tags:\n - gamma\n").Run(sb.Context(), sb.Runtime()) require.NoError(t, res.Err) // Offset 1 on tag list: skip "alpha", get "beta" and "gamma". diff --git a/pkg/cli/testhelpers_test.go b/pkg/cli/testhelpers_test.go index ac25c4a0..bc32d7fb 100644 --- a/pkg/cli/testhelpers_test.go +++ b/pkg/cli/testhelpers_test.go @@ -146,6 +146,23 @@ func NewProcess(t *testing.T, isTTY bool, args ...string) *tu.Process { }, isTTY) } +// NewCreateProcess builds a `tap create` process whose node content is piped +// on stdin. Content is the only way to give a new node a title now that +// --title, --lead, --tags and --attrs are gone, so tests that merely need a +// node with a known title go through here instead of repeating the heredoc. +// +// meta, when non-empty, is emitted as the content's YAML frontmatter — the +// documented CLI channel for metadata on a piped create. +func NewCreateProcess(t *testing.T, isTTY bool, title, meta string, extraArgs ...string) *tu.Process { + proc := NewProcess(t, isTTY, append([]string{"create"}, extraArgs...)...) + content := "# " + title + "\n" + if meta != "" { + content = "---\n" + meta + "---\n" + content + } + proc.SetStdin(strings.NewReader(content)) + return proc +} + func NewCompletionProcess(t *testing.T, isTTY bool, pos int, words ...string) *tu.Process { _ = pos var mu sync.Mutex diff --git a/pkg/keg/archive_test.go b/pkg/keg/archive_test.go index 827f278a..c3b7b65e 100644 --- a/pkg/keg/archive_test.go +++ b/pkg/keg/archive_test.go @@ -186,7 +186,7 @@ func TestArchiveExportUsesAssetsDirectoryAndIncludesConfigForFullBackup(t *testi src := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, src, ctx) - id, err := src.Create(ctx, &keg.CreateOptions{Title: "asset node", Body: []byte("# asset node\n")}) + id, err := src.Create(ctx, &keg.CreateOptions{Body: []byte("# asset node\n")}) require.NoError(t, err) require.NoError(t, src.WriteFile(ctx, id.ID, "doc.txt", []byte("doc bytes"))) require.NoError(t, src.WriteImage(ctx, id.ID, "diagram.png", tinyPNG(t))) @@ -240,7 +240,7 @@ func TestArchiveImportRestoresKegSettingsForFullBackup(t *testing.T) { src := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, src, ctx) - _, err := src.Create(ctx, &keg.CreateOptions{Title: "indexed", Body: []byte("# indexed\n"), Tags: []string{"restored"}}) + _, err := src.Create(ctx, &keg.CreateOptions{Body: []byte("# indexed\n"), Meta: []byte("tags:\n - restored\n")}) require.NoError(t, err) require.NoError(t, src.UpdateSettings(ctx, func(cfg *keg.Settings) { cfg.Title = "Restored Title" @@ -322,7 +322,7 @@ func TestArchiveImportRestoresKegSettingsFromLegacyConfigManifest(t *testing.T) src := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, src, ctx) - _, err := src.Create(ctx, &keg.CreateOptions{Title: "indexed", Body: []byte("# indexed\n"), Tags: []string{"restored"}}) + _, err := src.Create(ctx, &keg.CreateOptions{Body: []byte("# indexed\n"), Meta: []byte("tags:\n - restored\n")}) require.NoError(t, err) require.NoError(t, src.UpdateSettings(ctx, func(cfg *keg.Settings) { cfg.Title = "Legacy Title" @@ -374,7 +374,7 @@ func TestArchiveImportNodeSubsetDoesNotRestoreKegSettings(t *testing.T) { src := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, src, ctx) - id, err := src.Create(ctx, &keg.CreateOptions{Title: "partial", Body: []byte("# partial\n")}) + id, err := src.Create(ctx, &keg.CreateOptions{Body: []byte("# partial\n")}) require.NoError(t, err) require.NoError(t, src.UpdateSettings(ctx, func(cfg *keg.Settings) { cfg.Title = "Source Title" @@ -414,7 +414,7 @@ func TestArchiveExportNodeSubsetOmitsSchemas(t *testing.T) { initNonStrictTestKeg(t, src, ctx) require.NoError(t, src.CreateSchema(ctx, "task", archiveTaskSchema)) id, err := src.Create(ctx, &keg.CreateOptions{ - Body: []byte("---\ntype: task\n---\n# Partial\n"), + Body: []byte("# Partial\n"), Meta: []byte("type: task\n"), }) require.NoError(t, err) @@ -460,7 +460,7 @@ summary: Target decisions markZeroAsTask(t, src) require.NoError(t, src.CreateSchema(ctx, "task", archivedSchema)) _, err := src.Create(ctx, &keg.CreateOptions{ - Body: []byte("---\ntype: task\n---\n# Imported Task\n"), + Body: []byte("# Imported Task\n"), Meta: []byte("type: task\n"), }) require.NoError(t, err) archive := mustExportArchive(t, src, keg.ExportNodesOptions{}) @@ -500,7 +500,7 @@ markdown: markZeroAsTask(t, src) require.NoError(t, src.CreateSchema(ctx, "task", archiveTaskSchema)) id, err := src.Create(ctx, &keg.CreateOptions{ - Body: []byte("---\ntype: task\n---\n# Accepted By Archive Schema\n"), + Body: []byte("# Accepted By Archive Schema\n"), Meta: []byte("type: task\n"), }) require.NoError(t, err) archive := mustExportArchive(t, src, keg.ExportNodesOptions{}) @@ -587,7 +587,7 @@ func TestArchiveImportRejectsMalformedSchemaBeforeWritingNodes(t *testing.T) { markZeroAsTask(t, src) require.NoError(t, src.CreateSchema(ctx, "task", archiveTaskSchema)) id, err := src.Create(ctx, &keg.CreateOptions{ - Body: []byte("---\ntype: task\n---\n# Imported Task\n"), + Body: []byte("# Imported Task\n"), Meta: []byte("type: task\n"), }) require.NoError(t, err) archive := mustExportArchive(t, src, keg.ExportNodesOptions{}) @@ -614,7 +614,7 @@ func TestArchiveImportRejectsSchemasWhenTargetDoesNotSupportThemBeforeWritingNod markZeroAsTask(t, src) require.NoError(t, src.CreateSchema(ctx, "task", archiveTaskSchema)) id, err := src.Create(ctx, &keg.CreateOptions{ - Body: []byte("---\ntype: task\n---\n# Imported Task\n"), + Body: []byte("# Imported Task\n"), Meta: []byte("type: task\n"), }) require.NoError(t, err) archive := mustExportArchive(t, src, keg.ExportNodesOptions{}) @@ -636,7 +636,7 @@ func TestArchiveImportRejectsNestedAssetNameBeforeWritingNodes(t *testing.T) { src := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, src, ctx) - id, err := src.Create(ctx, &keg.CreateOptions{Title: "asset node", Body: []byte("# asset node\n")}) + id, err := src.Create(ctx, &keg.CreateOptions{Body: []byte("# asset node\n")}) require.NoError(t, err) require.NoError(t, src.WriteFile(ctx, id.ID, "doc.txt", []byte("doc bytes"))) archive := mustExportArchive(t, src, keg.ExportNodesOptions{WithAssets: true}) diff --git a/pkg/keg/asset_name_test.go b/pkg/keg/asset_name_test.go index e939afa7..5594208b 100644 --- a/pkg/keg/asset_name_test.go +++ b/pkg/keg/asset_name_test.go @@ -112,7 +112,7 @@ func TestImport_RejectsZipSlipArchive(t *testing.T) { // Build a legitimate archive (valid manifest/meta/stats) with one attachment. src := keg.NewLocalKeg(newTestMemoryRepo(rt), rt) initNonStrictTestKeg(t, src, ctx) - nid, err := src.Create(ctx, &keg.CreateOptions{Title: "x", Body: []byte("# x\n")}) + nid, err := src.Create(ctx, &keg.CreateOptions{Body: []byte("# x\n")}) require.NoError(t, err) require.NoError(t, src.WriteFile(ctx, nid.ID, "doc.txt", []byte("benign"))) rc, err := src.ExportNodes(ctx, keg.ExportNodesOptions{WithAssets: true}) diff --git a/pkg/keg/keg_aggregate.go b/pkg/keg/keg_aggregate.go index aeee70e4..4f6ec8e7 100644 --- a/pkg/keg/keg_aggregate.go +++ b/pkg/keg/keg_aggregate.go @@ -184,13 +184,14 @@ type CreateResult struct { const MaxMutationBatchSize = 100 type NodeCreate struct { - Key string `json:"key"` - Schema string `json:"schema,omitempty"` - Title string `json:"title,omitempty"` - Lead string `json:"lead,omitempty"` - Body []byte `json:"body,omitempty"` - Tags []string `json:"tags,omitempty"` - Attrs map[string]any `json:"attrs,omitempty"` + Key string `json:"key"` + Schema string `json:"schema,omitempty"` + // Body is the node's complete markdown content; its H1 is the title. Meta + // is the node's complete metadata document. These are the only two inputs + // a node is built from — there is deliberately no second, field-at-a-time + // way to write a title, lead, tags, or attributes. + Body []byte `json:"body,omitempty"` + Meta []byte `json:"meta,omitempty"` } type CreateNodeResult struct { diff --git a/pkg/keg/keg_aggregate_test.go b/pkg/keg/keg_aggregate_test.go index 82c55fab..e47c4775 100644 --- a/pkg/keg/keg_aggregate_test.go +++ b/pkg/keg/keg_aggregate_test.go @@ -17,9 +17,9 @@ func TestLocalKegAggregateOperations(t *testing.T) { ctx := fx.Context() k := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, k, ctx) - one, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# One\n\nlead\n"), Tags: []string{"alpha"}}) + one, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# One\n\nlead\n"), Meta: []byte("tags:\n - alpha\n")}) require.NoError(t, err) - two, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Two\n\n[One](../1)\n"), Tags: []string{"beta"}}) + two, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Two\n\n[One](../1)\n"), Meta: []byte("tags:\n - beta\n")}) require.NoError(t, err) listing, err := k.ListEntries(ctx, keg.ListEntriesOptions{Query: "alpha"}) @@ -90,7 +90,7 @@ func TestUpdateNodeRollsBackMemoryWritesOnFailure(t *testing.T) { repo := &failOnceContentRepo{Repository: base} k := keg.NewLocalKeg(repo, fx.Runtime()) initNonStrictTestKeg(t, k, ctx) - created, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Original\n\nbody\n"), Tags: []string{"original"}}) + created, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Original\n\nbody\n"), Meta: []byte("tags:\n - original\n")}) require.NoError(t, err) before, err := k.ReadNode(ctx, created.ID) require.NoError(t, err) @@ -156,7 +156,7 @@ func TestRemoteAggregateMethodsUseOneRequest(t *testing.T) { return err }}, {"create one", http.MethodPost, "/nodes", `[{"key":"node","id":1,"hash":"created"}]`, func(ctx context.Context, k *keg.RemoteKeg) error { - _, err := k.Create(ctx, &keg.CreateOptions{Title: "One"}) + _, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# One\n")}) return err }}, {"update", http.MethodPut, "/nodes", `[{"id":1,"hash":"updated"}]`, func(ctx context.Context, k *keg.RemoteKeg) error { diff --git a/pkg/keg/keg_batch.go b/pkg/keg/keg_batch.go index 4827af32..dde81d48 100644 --- a/pkg/keg/keg_batch.go +++ b/pkg/keg/keg_batch.go @@ -81,14 +81,16 @@ func (k *LocalKeg) createNodes(ctx context.Context, nodes []NodeCreate) ([]Creat if err != nil { return nil, &BatchMutationError{Index: i, Key: item.Key, NodeID: ids[i], Err: err} } - data, err := k.buildNodeData(ctx, &CreateOptions{Schema: item.Schema, Title: item.Title, Lead: item.Lead, Body: body, Tags: item.Tags, Attrs: item.Attrs}, now, fmt.Sprintf("NodeId %s", ids[i].Path())) + data, err := k.buildNodeData(ctx, &CreateOptions{Schema: item.Schema, Body: body, Meta: item.Meta}, now, fmt.Sprintf("NodeId %s", ids[i].Path())) if err != nil { return nil, &BatchMutationError{Index: i, Key: item.Key, NodeID: ids[i], Err: err} } data.ID = ids[i] + // Only meta can declare a type: buildNodeData rejects content that + // opens with a frontmatter block, so there is no second source to + // reconcile against the selected schema. validation, err := k.validateNodeWrite(ctx, schemaWriteCreate, ids[i], data, item.Schema, - schemaTypeCandidateFromAttrs("attributes", item.Attrs), - schemaTypeCandidateFromFrontmatter("frontmatter", data.Content)) + schemaTypeCandidateFromMeta("metadata", data.Meta)) if err != nil { return nil, &BatchMutationError{Index: i, Key: item.Key, NodeID: ids[i], Err: err} } @@ -190,6 +192,13 @@ func (k *LocalKeg) updateNodes(ctx context.Context, updates []NodeUpdateOptions) } contentBytes := existing.Content if opts.HasContent { + // Only caller-supplied content is held to the no-frontmatter rule. + // A node written before the rule existed may have a stored body + // that opens with `---`; checking contentBytes would make such a + // node permanently unupdatable. + if err := RejectFrontmatter(opts.Content); err != nil { + return nil, &BatchMutationError{Index: i, NodeID: opts.ID, Err: err} + } contentBytes = opts.Content } metaBytes := existing.Meta @@ -202,7 +211,7 @@ func (k *LocalKeg) updateNodes(ctx context.Context, updates []NodeUpdateOptions) } meta, err := ParseMeta(ctx, metaBytes) if err != nil { - return nil, &BatchMutationError{Index: i, NodeID: opts.ID, Err: fmt.Errorf("invalid metadata: %w", err)} + return nil, &BatchMutationError{Index: i, NodeID: opts.ID, Err: fmt.Errorf("invalid metadata: %s: %w", err, ErrInvalid)} } stats, err := cloneNodeStats(ctx, existing.Stats) if err != nil { @@ -216,10 +225,9 @@ func (k *LocalKeg) updateNodes(ctx context.Context, updates []NodeUpdateOptions) if err := data.updateMeta(ctx, k.Runtime, &now); err != nil { return nil, &BatchMutationError{Index: i, NodeID: opts.ID, Err: err} } - candidates := make([]schemaTypeCandidate, 0, 2) - if opts.HasContent { - candidates = append(candidates, schemaTypeCandidateFromFrontmatter("frontmatter", content)) - } + // Only meta can declare a type: RejectFrontmatter above guarantees + // submitted content carries none. + candidates := make([]schemaTypeCandidate, 0, 1) if opts.HasMeta { candidates = append(candidates, schemaTypeCandidateFromMeta("metadata", meta)) } diff --git a/pkg/keg/keg_batch_test.go b/pkg/keg/keg_batch_test.go index 0a6255cd..534e61d0 100644 --- a/pkg/keg/keg_batch_test.go +++ b/pkg/keg/keg_batch_test.go @@ -48,8 +48,8 @@ func newStrictBatchKeg(t *testing.T) (*keg.LocalKeg, context.Context) { func TestCreateNodesResolvesPlaceholdersAndPreservesOrder(t *testing.T) { k, ctx := newStrictBatchKeg(t) results, err := k.CreateNodes(ctx, []keg.NodeCreate{ - {Key: "first", Schema: "task", Body: []byte("# First\n\n[Second](../{{node:second}})\n"), Attrs: map[string]any{"type": "task"}}, - {Key: "second", Schema: "task", Body: []byte("# Second\n\n[First](../{{node:first}})\n"), Attrs: map[string]any{"type": "task"}}, + {Key: "first", Schema: "task", Body: []byte("# First\n\n[Second](../{{node:second}})\n"), Meta: []byte("type: task\n")}, + {Key: "second", Schema: "task", Body: []byte("# Second\n\n[First](../{{node:first}})\n"), Meta: []byte("type: task\n")}, }) require.NoError(t, err) require.Equal(t, []string{"first", "second"}, []string{results[0].Key, results[1].Key}) @@ -64,16 +64,95 @@ func TestCreateNodesResolvesPlaceholdersAndPreservesOrder(t *testing.T) { func TestCreateNodesPreflightFailureLeavesKegUnchanged(t *testing.T) { k, ctx := newStrictBatchKeg(t) - _, err := k.CreateNodes(ctx, []keg.NodeCreate{{Key: "one", Schema: "task", Body: []byte("# One\n"), Attrs: map[string]any{"type": "task"}}, {Key: "two", Schema: "task", Body: []byte("# Two\n\n[Missing](../{{node:nope}})\n"), Attrs: map[string]any{"type": "task"}}}) + _, err := k.CreateNodes(ctx, []keg.NodeCreate{{Key: "one", Schema: "task", Body: []byte("# One\n"), Meta: []byte("type: task\n")}, {Key: "two", Schema: "task", Body: []byte("# Two\n\n[Missing](../{{node:nope}})\n"), Meta: []byte("type: task\n")}}) require.Error(t, err) ids, err := k.ListNodes(ctx) require.NoError(t, err) require.Equal(t, []keg.NodeId{{ID: 0}}, ids) } +// A malformed metadata document is the caller's mistake, so it has to classify +// as ErrInvalid — a transport mapping it by error kind (the hub maps ErrInvalid +// to 400) otherwise answers a client typo with a server error. The same holds +// for content opening with a frontmatter block, which is rejected outright now +// that content and meta are separate inputs. +func TestMutationBatchesRejectMalformedMetaAndContentFrontmatter(t *testing.T) { + metas := []struct { + name string + meta string + }{ + {"sequence", "- one\n- two\n"}, + {"scalar", "just a string"}, + {"unterminated flow", "a: [1, 2\n"}, + {"tab indent", "a:\n\t- b\n"}, + } + for _, tc := range metas { + t.Run("create meta "+tc.name, func(t *testing.T) { + k, ctx := newStrictBatchKeg(t) + _, err := k.CreateNodes(ctx, []keg.NodeCreate{{ + Key: "n", Schema: "task", Body: []byte("# One\n"), Meta: []byte(tc.meta), + }}) + require.ErrorIs(t, err, keg.ErrInvalid) + // No id is consumed by a rejected create. + ids, err := k.ListNodes(ctx) + require.NoError(t, err) + require.Equal(t, []keg.NodeId{{ID: 0}}, ids) + }) + } + + t.Run("update meta", func(t *testing.T) { + k, ctx := newStrictBatchKeg(t) + created, err := k.CreateNodes(ctx, []keg.NodeCreate{{ + Key: "n", Schema: "task", Body: []byte("# One\n"), Meta: []byte("type: task\n"), + }}) + require.NoError(t, err) + _, err = k.UpdateNodes(ctx, []keg.NodeUpdateOptions{{ + ID: created[0].ID, Schema: "task", Meta: []byte("- one\n"), HasMeta: true, + ExpectedHash: created[0].Hash, + }}) + require.ErrorIs(t, err, keg.ErrInvalid) + }) + + t.Run("update content frontmatter", func(t *testing.T) { + k, ctx := newStrictBatchKeg(t) + created, err := k.CreateNodes(ctx, []keg.NodeCreate{{ + Key: "n", Schema: "task", Body: []byte("# One\n"), Meta: []byte("type: task\n"), + }}) + require.NoError(t, err) + _, err = k.UpdateNodes(ctx, []keg.NodeUpdateOptions{{ + ID: created[0].ID, Schema: "task", HasContent: true, + Content: []byte("---\ntype: task\n---\n# One\n"), + ExpectedHash: created[0].Hash, + }}) + require.ErrorIs(t, err, keg.ErrInvalid) + require.Contains(t, err.Error(), "must not start with a YAML frontmatter block") + }) + + // An update that does not carry content must not be judged on the stored + // body: a node written before the rule existed can legitimately have one + // that opens with `---`, and it still has to be editable. + t.Run("stored frontmatter stays updatable", func(t *testing.T) { + k, ctx := newStrictBatchKeg(t) + created, err := k.CreateNodes(ctx, []keg.NodeCreate{{ + Key: "n", Schema: "task", Body: []byte("# One\n"), Meta: []byte("type: task\n"), + }}) + require.NoError(t, err) + // Write the legacy shape past the create guard, the way a node + // predating this rule would already be stored. + require.NoError(t, k.Repo.WriteContent(ctx, created[0].ID, []byte("---\ntype: task\n---\n# One\n"))) + view, err := k.ReadNode(ctx, created[0].ID) + require.NoError(t, err) + _, err = k.UpdateNodes(ctx, []keg.NodeUpdateOptions{{ + ID: created[0].ID, Schema: "task", Meta: []byte("type: task\n"), HasMeta: true, + ExpectedHash: view.Hash(), + }}) + require.NoError(t, err) + }) +} + func TestUpdateNodesPreflightsHashesAndSnapshotsAtomically(t *testing.T) { k, ctx := newStrictBatchKeg(t) - created, err := k.CreateNodes(ctx, []keg.NodeCreate{{Key: "one", Schema: "task", Body: []byte("# One\n"), Attrs: map[string]any{"type": "task"}}, {Key: "two", Schema: "task", Body: []byte("# Two\n"), Attrs: map[string]any{"type": "task"}}}) + created, err := k.CreateNodes(ctx, []keg.NodeCreate{{Key: "one", Schema: "task", Body: []byte("# One\n"), Meta: []byte("type: task\n")}, {Key: "two", Schema: "task", Body: []byte("# Two\n"), Meta: []byte("type: task\n")}}) require.NoError(t, err) before, err := k.ReadNode(ctx, created[0].ID) require.NoError(t, err) @@ -178,7 +257,7 @@ func TestStrictPolicyUsesResolvedValidationMode(t *testing.T) { off := keg.WithValidationMode(ctx, keg.ValidationModeOff) _, err = k.Create(off, &keg.CreateOptions{Body: []byte("# Untyped\n")}) require.NoError(t, err) - _, err = k.Create(off, &keg.CreateOptions{Body: []byte("---\ntype: missing\n---\n# Unknown\n")}) + _, err = k.Create(off, &keg.CreateOptions{Body: []byte("# Unknown\n"), Meta: []byte("type: missing\n")}) require.NoError(t, err) _, err = k.Create(ctx, &keg.CreateOptions{Body: []byte("# Missing selection\n")}) require.ErrorIs(t, err, keg.ErrSchemaInvalid) diff --git a/pkg/keg/keg_concurrent_test.go b/pkg/keg/keg_concurrent_test.go index c2667510..974e96a1 100644 --- a/pkg/keg/keg_concurrent_test.go +++ b/pkg/keg/keg_concurrent_test.go @@ -32,9 +32,7 @@ func TestConcurrentCreate_UniqueIDs(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: fmt.Sprintf("Node %d", idx), - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# " + fmt.Sprintf("Node %d", idx) + "\n")}) ids[idx] = id.ID errs[idx] = err }(i) @@ -71,9 +69,7 @@ func TestConcurrentCreate_MemoryRepository(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: fmt.Sprintf("FsNode %d", idx), - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# " + fmt.Sprintf("FsNode %d", idx) + "\n")}) ids[idx] = id.ID errs[idx] = err }(i) @@ -104,9 +100,7 @@ func TestConcurrentSetContent_DifferentNodes(t *testing.T) { const N = 10 ids := make([]kegpkg.NodeId, N) for i := range N { - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: fmt.Sprintf("Node %d", i), - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# " + fmt.Sprintf("Node %d", i) + "\n")}) require.NoError(t, err) ids[i] = id.ID } @@ -146,7 +140,7 @@ func TestConcurrentSetContent_SameNode(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Shared"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Shared\n")}) require.NoError(t, err) const N = 5 @@ -182,7 +176,7 @@ func TestConcurrentSetMeta_SameNode(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Shared Meta"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Shared Meta\n")}) require.NoError(t, err) const N = 5 @@ -218,9 +212,7 @@ func TestConcurrentCreateAndEdit(t *testing.T) { const preCreated = 5 preIDs := make([]kegpkg.NodeId, preCreated) for i := range preCreated { - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: fmt.Sprintf("Pre %d", i), - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# " + fmt.Sprintf("Pre %d", i) + "\n")}) require.NoError(t, err) preIDs[i] = id.ID } @@ -236,9 +228,7 @@ func TestConcurrentCreateAndEdit(t *testing.T) { wg.Add(1) go func(idx int) { defer wg.Done() - _, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: fmt.Sprintf("Created %d", idx), - }) + _, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# " + fmt.Sprintf("Created %d", idx) + "\n")}) createErrs[idx] = err }(i) } @@ -284,12 +274,12 @@ func TestTwoKegInstances_DexNotOverwritten(t *testing.T) { k2 := kegpkg.NewLocalKeg(repo, f.Runtime()) // k1 creates node 1 - id1, err := k1.Create(f.Context(), &kegpkg.CreateOptions{Title: "From K1"}) + id1, err := k1.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# From K1\n")}) require.NoError(t, err) require.Equal(t, 1, id1.ID.ID) // k2 creates node 2 -- its dex should include node 1 from k1 - id2, err := k2.Create(f.Context(), &kegpkg.CreateOptions{Title: "From K2"}) + id2, err := k2.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# From K2\n")}) require.NoError(t, err) require.Equal(t, 2, id2.ID.ID) @@ -312,7 +302,7 @@ func TestConcurrentCrossLock_OnlyOneWins(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Lock Race"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Lock Race\n")}) require.NoError(t, err) const N = 10 @@ -351,7 +341,7 @@ func TestCrossLock_DoesNotBlockWithNodeLock(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Lock Independence"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Lock Independence\n")}) require.NoError(t, err) // Acquire cross-process lock. @@ -384,7 +374,7 @@ func TestConcurrentRemoveThenSetContent_RaceCondition(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Race Node"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Race Node\n")}) require.NoError(t, err) var wg sync.WaitGroup @@ -441,7 +431,7 @@ func TestConcurrentRemoveDuringSetContent_MemoryRepository(t *testing.T) { require.NoError(t, err) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "FsDoomed"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# FsDoomed\n")}) require.NoError(t, err) // Remove the node. @@ -470,10 +460,7 @@ func TestConcurrentRemoveDuringSetMeta_MemoryRepository(t *testing.T) { require.NoError(t, err) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: "FsMetaDoomed", - Tags: []string{"victim"}, - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# FsMetaDoomed\n"), Meta: []byte("tags:\n - victim\n")}) require.NoError(t, err) meta, err := k.GetMeta(f.Context(), id.ID) @@ -500,7 +487,7 @@ func TestConcurrentRemoveDuringTouch_MemoryRepository(t *testing.T) { require.NoError(t, err) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "FsTouchDoomed"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# FsTouchDoomed\n")}) require.NoError(t, err) require.NoError(t, errOnly(k.Remove(f.Context(), removeOptions(t, f.Context(), k, id.ID)))) @@ -524,7 +511,7 @@ func TestConcurrentRemoveDuringUpdateMeta_MemoryRepository(t *testing.T) { require.NoError(t, err) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "FsUpdateDoomed"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# FsUpdateDoomed\n")}) require.NoError(t, err) require.NoError(t, errOnly(k.Remove(f.Context(), removeOptions(t, f.Context(), k, id.ID)))) diff --git a/pkg/keg/keg_iface.go b/pkg/keg/keg_iface.go index 7155e59a..09cd33a4 100644 --- a/pkg/keg/keg_iface.go +++ b/pkg/keg/keg_iface.go @@ -77,10 +77,6 @@ type Keg interface { // unique and may be referenced from bodies as {{node:key}}. CreateNodes(ctx context.Context, nodes []NodeCreate) ([]CreateNodeResult, error) - // Next reserves and returns the next available node id. The reservation does - // not become a complete node until content is written. - Next(ctx context.Context) (NodeId, error) - // ListNodes returns all node ids present in the keg. ListNodes(ctx context.Context) ([]NodeId, error) diff --git a/pkg/keg/keg_iface_test.go b/pkg/keg/keg_iface_test.go index ad038bd9..864b5ce5 100644 --- a/pkg/keg/keg_iface_test.go +++ b/pkg/keg/keg_iface_test.go @@ -19,17 +19,9 @@ func newLiftedKeg(t *testing.T) (*sandbox.Sandbox, *kegpkg.LocalKeg) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - _, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: "Alpha node", - Body: []byte("# Alpha node\n\nAlpha body links to [beta](../2)"), - Tags: []string{"alpha", "shared"}, - }) - require.NoError(t, err) - _, err = k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: "Beta node", - Body: []byte("# Beta node\n\nBeta body mentions gamma rays"), - Tags: []string{"beta", "shared"}, - }) + _, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Alpha node\n\nAlpha body links to [beta](../2)"), Meta: []byte("tags:\n - alpha\n - shared\n")}) + require.NoError(t, err) + _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Beta node\n\nBeta body mentions gamma rays"), Meta: []byte("tags:\n - beta\n - shared\n")}) require.NoError(t, err) return f, k } diff --git a/pkg/keg/keg_integer_schema_test.go b/pkg/keg/keg_integer_schema_test.go index 6c85c87b..187e32f9 100644 --- a/pkg/keg/keg_integer_schema_test.go +++ b/pkg/keg/keg_integer_schema_test.go @@ -19,25 +19,31 @@ markdown: requireTitle: true ` -// TestCreate_IntegerFrontmatterSatisfiesSchema is the issue's reproduction, end -// to end through the real create path rather than at the metadata layer alone. -// Before the fix this failed with: +// TestCreate_IntegerMetaSatisfiesSchema is the issue's reproduction, end to end +// through the real create path rather than at the metadata layer alone. Before +// the fix this failed with: // // validating /properties/contract_count: type: 1 has type "string", want "integer" // // which was a closed loop: the node could not be created, and meta could not // repair it because meta validates the whole node including markdown sections. -func TestCreate_IntegerFrontmatterSatisfiesSchema(t *testing.T) { +// +// The issue reproduced through body frontmatter, which content no longer +// accepts; the metadata document is the path that carries typed scalars now, +// and it is the one that has to keep the integer an integer. +func TestCreate_IntegerMetaSatisfiesSchema(t *testing.T) { fx := NewSandbox(t) ctx := fx.Context() k := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) require.NoError(t, k.Init(ctx)) require.NoError(t, k.CreateSchema(ctx, "change", []byte(changeSchema))) - body := "---\ntype: change\ncontract_count: 1\n---\n\n# Widen the contract\n\nBody text.\n" - results, err := k.CreateNodes(ctx, []keg.NodeCreate{ - {Key: "n", Schema: "change", Body: []byte(body)}, - }) + results, err := k.CreateNodes(ctx, []keg.NodeCreate{{ + Key: "n", + Schema: "change", + Body: []byte("# Widen the contract\n\nBody text.\n"), + Meta: []byte("type: change\ncontract_count: 1\n"), + }}) require.NoError(t, err) require.Len(t, results, 1) if v := results[0].Validation; v != nil { @@ -51,9 +57,9 @@ func TestCreate_IntegerFrontmatterSatisfiesSchema(t *testing.T) { require.NotContains(t, meta.ToYAML(), `contract_count: "1"`) } -// TestUpdate_IntegerFrontmatterSurvivesEdit covers the edit half of the issue: -// a body carrying inline frontmatter runs through the same metadata path. -func TestUpdate_IntegerFrontmatterSurvivesEdit(t *testing.T) { +// TestUpdate_IntegerMetaSurvivesEdit covers the edit half of the issue: a +// replacement metadata document runs through the same path. +func TestUpdate_IntegerMetaSurvivesEdit(t *testing.T) { fx := NewSandbox(t) ctx := fx.Context() k := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) @@ -63,15 +69,16 @@ func TestUpdate_IntegerFrontmatterSurvivesEdit(t *testing.T) { results, err := k.CreateNodes(ctx, []keg.NodeCreate{{ Key: "n", Schema: "change", - Body: []byte("---\ntype: change\ncontract_count: 1\n---\n\n# Title\n\nBody.\n"), + Body: []byte("# Title\n\nBody.\n"), + Meta: []byte("type: change\ncontract_count: 1\n"), }}) require.NoError(t, err) _, err = k.UpdateNode(ctx, keg.NodeUpdateOptions{ - ID: results[0].ID, - Schema: "change", - Content: []byte("---\ntype: change\ncontract_count: 7\n---\n\n# Title\n\nEdited.\n"), - HasContent: true, + ID: results[0].ID, + Schema: "change", + Content: []byte("# Title\n\nEdited.\n"), HasContent: true, + Meta: []byte("type: change\ncontract_count: 7\n"), HasMeta: true, // Writes are guarded; carry the hash the create returned. ExpectedHash: results[0].Hash, }) diff --git a/pkg/keg/keg_listview_batch_test.go b/pkg/keg/keg_listview_batch_test.go index b0d7c3fb..2140102e 100644 --- a/pkg/keg/keg_listview_batch_test.go +++ b/pkg/keg/keg_listview_batch_test.go @@ -61,10 +61,7 @@ func seedNodes(t *testing.T, k *kegpkg.LocalKeg, n int) { t.Helper() ctx := context.Background() for i := range n { - _, err := k.Create(ctx, &kegpkg.CreateOptions{ - Title: fmt.Sprintf("Node %d", i), - Attrs: map[string]any{"type": fmt.Sprintf("kind%d", i%3)}, - }) + _, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# " + fmt.Sprintf("Node %d", i) + "\n"), Meta: []byte("type: " + fmt.Sprintf("kind%d", i%3) + "\n")}) require.NoError(t, err) } } diff --git a/pkg/keg/keg_local_create.go b/pkg/keg/keg_local_create.go index 20e86130..eeb1d8b0 100644 --- a/pkg/keg/keg_local_create.go +++ b/pkg/keg/keg_local_create.go @@ -1,9 +1,9 @@ package keg import ( + "bytes" "context" "fmt" - "strings" "time" "github.com/jlrickert/cli-toolkit/toolkit" @@ -81,37 +81,25 @@ func (k *LocalKeg) init(ctx context.Context) error { return nil } -// Next reserves and returns the next available node ID from the repository. -func (k *LocalKeg) Next(ctx context.Context) (NodeId, error) { - return withKegWriteValue(ctx, k, func(ctx context.Context) (NodeId, error) { - return k.Repo.Next(ctx) - }) -} - // CreateOptions specifies parameters for creating a new node type CreateOptions struct { // Schema is the explicitly selected schema for this write. Schema string - // Title is the human-readable title for the node - Title string - // Lead is a one-line summary - Lead string - // Tags are searchable labels for the node - Tags []string - // Body is the raw markdown content; if empty, default content is generated from Title/Lead + // Body is the raw markdown content; its H1 is the node's title. When + // empty, a placeholder heading is generated from the allocated node id. Body []byte - // Attrs are arbitrary key-value attributes attached to the node - Attrs map[string]any + // Meta is the node's complete metadata document. + Meta []byte } // Create creates a new node: allocates an ID, parses content, generates metadata, // and indexes the node in the dex. The node is immediately persisted to the repository. -// If Body is empty, default markdown content is generated from Title and Lead. +// If Body is empty, a placeholder heading is generated from the allocated node id. func (k *LocalKeg) Create(ctx context.Context, opts *CreateOptions) (CreateResult, error) { if opts == nil { opts = &CreateOptions{} } - results, err := k.CreateNodes(ctx, []NodeCreate{{Key: "node", Schema: opts.Schema, Title: opts.Title, Lead: opts.Lead, Body: opts.Body, Tags: opts.Tags, Attrs: opts.Attrs}}) + results, err := k.CreateNodes(ctx, []NodeCreate{{Key: "node", Schema: opts.Schema, Body: opts.Body, Meta: opts.Meta}}) if len(results) == 0 { return CreateResult{}, err } @@ -189,40 +177,53 @@ func (k *LocalKeg) buildNodeData(ctx context.Context, opts *CreateOptions, now t // buildCreateNodeData assembles the content/meta/stats for a new node from // opts. The node id is not part of the result (callers set it once known) -// except via fallbackHeading, the H1 used when opts carries neither a Body nor -// a Title — local creates pass "NodeId " there; remote creates, where the -// id is assigned by the hub, pass a generic heading. +// except via fallbackHeading, the H1 used when opts carries no Body — local +// creates pass "NodeId " there; remote creates, where the id is assigned +// by the hub, pass a generic heading. func buildCreateNodeData(ctx context.Context, rt *toolkit.Runtime, opts *CreateOptions, now time.Time, fallbackHeading string) (*NodeData, error) { - var rawContent []byte - if len(opts.Body) > 0 { - rawContent = opts.Body - } else { - b := strings.Builder{} - if opts.Title != "" { - b.WriteString(fmt.Sprintf("# %s\n", opts.Title)) - } else { - b.WriteString(fmt.Sprintf("# %s\n", fallbackHeading)) - } - if opts.Lead != "" { - b.WriteString(fmt.Sprintf("\n%s\n", opts.Lead)) - } - rawContent = []byte(b.String()) + rawContent := opts.Body + if len(rawContent) == 0 { + rawContent = []byte(fmt.Sprintf("# %s\n", fallbackHeading)) + } + if err := RejectFrontmatter(rawContent); err != nil { + return nil, err } content, err := ParseContent(rt, rawContent, MarkdownContentFilename) if err != nil { return nil, fmt.Errorf("invalid content: %w", err) } + m := NewMeta(ctx, now) - if len(opts.Attrs) > 0 { - m.SetAttrs(ctx, opts.Attrs) + if len(bytes.TrimSpace(opts.Meta)) > 0 { + m, err = ParseMeta(ctx, opts.Meta) + if err != nil { + return nil, fmt.Errorf("invalid metadata: %s: %w", err, ErrInvalid) + } } + stats := NewStats(now) - if len(opts.Tags) > 0 { - m.SetTags(opts.Tags) - } nodeData := &NodeData{Content: content, Meta: m, Stats: stats} _ = nodeData.updateMeta(ctx, rt, &now) nodeData.Stats.EnsureTimes(now) return nodeData, nil } + +// RejectFrontmatter refuses content that opens with a YAML frontmatter +// delimiter. A node is built from two separate inputs — content, the markdown +// body opening with its H1 title, and meta, the complete metadata document — so +// a frontmatter block is a second, silent way to write metadata. The failure +// mode is severe: a body that legitimately begins with a horizontal rule would +// either consume the following lines as metadata or fail deep in the parser +// with an unrelated message. +// +// This lives here rather than in the tool layer so every writer reaches the +// same rule: the REST handlers, the MCP tools, the web UI, and the tap CLI all +// pass through create and update below. +func RejectFrontmatter(content []byte) error { + trimmed := bytes.TrimPrefix(content, []byte("\xef\xbb\xbf")) + if !bytes.HasPrefix(trimmed, []byte("---\n")) && !bytes.HasPrefix(trimmed, []byte("---\r\n")) { + return nil + } + return fmt.Errorf("content must not start with a YAML frontmatter block; send metadata in the meta field instead: %w", ErrInvalid) +} diff --git a/pkg/keg/keg_remote.go b/pkg/keg/keg_remote.go index c16b6227..4298dfa7 100644 --- a/pkg/keg/keg_remote.go +++ b/pkg/keg/keg_remote.go @@ -427,24 +427,13 @@ func (k *RemoteKeg) Create(ctx context.Context, opts *CreateOptions) (CreateResu if opts == nil { opts = &CreateOptions{} } - results, err := k.CreateNodes(ctx, []NodeCreate{{Key: "node", Schema: opts.Schema, Title: opts.Title, Lead: opts.Lead, Body: opts.Body, Tags: opts.Tags, Attrs: opts.Attrs}}) + results, err := k.CreateNodes(ctx, []NodeCreate{{Key: "node", Schema: opts.Schema, Body: opts.Body, Meta: opts.Meta}}) if len(results) == 0 { return CreateResult{}, err } return CreateResult{ID: results[0].ID, Validation: results[0].Validation}, err } -// Next implements Keg via GET /nodes/next. -func (k *RemoteKeg) Next(ctx context.Context) (NodeId, error) { - var result struct { - ID int `json:"id"` - } - if err := k.getJSON(ctx, "/nodes/next", "Next", &result); err != nil { - return NodeId{}, err - } - return NodeId{ID: result.ID}, nil -} - // ListNodes implements Keg via GET /nodes. func (k *RemoteKeg) ListNodes(ctx context.Context) ([]NodeId, error) { var ids []int diff --git a/pkg/keg/keg_remote_aggregate.go b/pkg/keg/keg_remote_aggregate.go index c4a2e62d..1b1e3c37 100644 --- a/pkg/keg/keg_remote_aggregate.go +++ b/pkg/keg/keg_remote_aggregate.go @@ -209,17 +209,21 @@ func (k *RemoteKeg) UpdateNode(ctx context.Context, opts NodeUpdateOptions) (*No func (k *RemoteKeg) CreateNodes(ctx context.Context, nodes []NodeCreate) ([]CreateNodeResult, error) { type wireNode struct { - Key string `json:"key"` - Schema string `json:"schema,omitempty"` - Title string `json:"title,omitempty"` - Lead string `json:"lead,omitempty"` - Body string `json:"body,omitempty"` - Tags []string `json:"tags,omitempty"` - Attrs map[string]any `json:"attrs,omitempty"` + Key string `json:"key"` + Schema string `json:"schema,omitempty"` + Content string `json:"content,omitempty"` + Meta string `json:"meta,omitempty"` } wire := make([]wireNode, len(nodes)) for i, n := range nodes { - wire[i] = wireNode{n.Key, n.Schema, n.Title, n.Lead, string(n.Body), n.Tags, n.Attrs} + // Keyed, not positional: a positional literal is what let Meta be added + // to NodeCreate and go unsent here without anything failing to compile. + wire[i] = wireNode{ + Key: n.Key, + Schema: n.Schema, + Content: string(n.Body), + Meta: string(n.Meta), + } } var response []struct { Key string `json:"key"` diff --git a/pkg/keg/keg_remote_test.go b/pkg/keg/keg_remote_test.go index 9003e9b2..f20c510f 100644 --- a/pkg/keg/keg_remote_test.go +++ b/pkg/keg/keg_remote_test.go @@ -29,6 +29,9 @@ type mockOpsHub struct { token string requests atomic.Int64 srv *httptest.Server + // inspectCreate, when set, receives the raw POST /nodes body so a test can + // assert on what actually crossed the wire rather than only on the result. + inspectCreate func([]byte) } func (h *mockOpsHub) writeJSON(w http.ResponseWriter, status int, v any) { @@ -82,17 +85,9 @@ func newMockOpsHub(t *testing.T, f *sandbox.Sandbox, token string) *mockOpsHub { repo := newTestMemoryRepo(f.Runtime()) backing := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, backing, f.Context()) - _, err := backing.Create(f.Context(), &kegpkg.CreateOptions{ - Title: "Alpha node", - Body: []byte("# Alpha node\n\nAlpha body links to [beta](../2)"), - Tags: []string{"alpha", "shared"}, - }) + _, err := backing.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Alpha node\n\nAlpha body links to [beta](../2)"), Meta: []byte("tags:\n - alpha\n - shared\n")}) require.NoError(t, err) - _, err = backing.Create(f.Context(), &kegpkg.CreateOptions{ - Title: "Beta node", - Body: []byte("# Beta node\n\nBeta body mentions gamma rays"), - Tags: []string{"beta", "shared"}, - }) + _, err = backing.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Beta node\n\nBeta body mentions gamma rays"), Meta: []byte("tags:\n - beta\n - shared\n")}) require.NoError(t, err) h := &mockOpsHub{backing: backing, token: token} @@ -111,33 +106,30 @@ func newMockOpsHub(t *testing.T, f *sandbox.Sandbox, token string) *mockOpsHub { } h.writeJSON(w, http.StatusOK, out) }) - mux.HandleFunc("GET /nodes/next", func(w http.ResponseWriter, r *http.Request) { - id, err := backing.Next(r.Context()) - if err != nil { - h.kegError(w, err) + mux.HandleFunc("POST /nodes", func(w http.ResponseWriter, r *http.Request) { + raw, readErr := io.ReadAll(r.Body) + if readErr != nil { + h.writeError(w, http.StatusBadRequest, "unreadable body", "BAD_REQUEST") return } - h.writeJSON(w, http.StatusOK, map[string]int{"id": id.ID}) - }) - mux.HandleFunc("POST /nodes", func(w http.ResponseWriter, r *http.Request) { + if h.inspectCreate != nil { + h.inspectCreate(raw) + } var req struct { Nodes []struct { - Key string `json:"key"` - Schema string `json:"schema"` - Title string `json:"title"` - Lead string `json:"lead"` - Body string `json:"body"` - Tags []string `json:"tags"` - Attrs map[string]any `json:"attrs"` + Key string `json:"key"` + Schema string `json:"schema"` + Content string `json:"content"` + Meta string `json:"meta"` } `json:"nodes"` } - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err := json.Unmarshal(raw, &req); err != nil { h.writeError(w, http.StatusBadRequest, "invalid JSON body", "BAD_REQUEST") return } nodes := make([]kegpkg.NodeCreate, len(req.Nodes)) for i, item := range req.Nodes { - nodes[i] = kegpkg.NodeCreate{Key: item.Key, Schema: item.Schema, Title: item.Title, Lead: item.Lead, Body: []byte(item.Body), Tags: item.Tags, Attrs: item.Attrs} + nodes[i] = kegpkg.NodeCreate{Key: item.Key, Schema: item.Schema, Body: []byte(item.Content), Meta: []byte(item.Meta)} } results, err := backing.CreateNodes(r.Context(), nodes) if err != nil { @@ -629,11 +621,7 @@ func TestRemoteKegRoundTripBasics(t *testing.T) { ctx := f.Context() // Create with tags carries composed content + meta in one request. - id, err := rk.Create(ctx, &kegpkg.CreateOptions{ - Title: "Gamma node", - Lead: "A gamma lead", - Tags: []string{"gamma"}, - }) + id, err := rk.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Gamma node\n\nA gamma lead\n"), Meta: []byte("tags:\n - gamma\n")}) require.NoError(t, err) require.Equal(t, 3, id.ID.ID) @@ -696,13 +684,10 @@ func TestRemoteKegRoundTripBasics(t *testing.T) { require.NoError(t, err) require.False(t, exists) - // ListNodes and Next agree with the backing keg. + // ListNodes agrees with the backing keg. nodeIDs, err := rk.ListNodes(ctx) require.NoError(t, err) require.Len(t, nodeIDs, 4) - next, err := rk.Next(ctx) - require.NoError(t, err) - require.Equal(t, 4, next.ID) } func TestRemoteMutationBatchesPreserveOrderAndAtomicity(t *testing.T) { @@ -830,10 +815,7 @@ func TestRemoteKegImportRoundTrip(t *testing.T) { // with fresh ids. src := kegpkg.NewLocalKeg(newTestMemoryRepo(f.Runtime()), f.Runtime()) initNonStrictTestKeg(t, src, ctx) - srcID, err := src.Create(ctx, &kegpkg.CreateOptions{ - Title: "Imported node", - Body: []byte("# Imported node\n\ntravels by archive"), - }) + srcID, err := src.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Imported node\n\ntravels by archive")}) require.NoError(t, err) rc, err := src.ExportNodes(ctx, kegpkg.ExportNodesOptions{NodeIDs: []kegpkg.NodeId{srcID.ID}}) @@ -876,10 +858,7 @@ func TestRemoteKegSingleRoundTrip(t *testing.T) { return err }}, {"Create", func() error { - _, err := rk.Create(ctx, &kegpkg.CreateOptions{ - Title: "Budget node", - Tags: []string{"budget"}, - }) + _, err := rk.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Budget node\n"), Meta: []byte("tags:\n - budget\n")}) return err }}, {"Index", func() error { @@ -918,3 +897,46 @@ func TestRemoteKegErrorMapping(t *testing.T) { require.ErrorIs(t, rk.Init(ctx), kegpkg.ErrNotSupported) }) } + +// TestRemoteCreateCarriesMetaOverTheWire is a regression test for a create +// wire that silently dropped NodeCreate.Meta. Meta was added to the keg layer +// and to the MCP create tool, but RemoteKeg.CreateNodes built its wire struct +// positionally and never sent it, so metadata vanished on every hub-backed +// create while local creates kept working. Every other create test runs +// against a LocalKeg, so nothing crossed the transport that lost it. +// +// Assert on the decoded request body, not just the resulting node: the point +// is that meta is actually transmitted. +func TestRemoteCreateCarriesMetaOverTheWire(t *testing.T) { + t.Parallel() + f, hub, rk := newRemoteKegFixture(t) + ctx := f.Context() + + var sawMeta string + hub.inspectCreate = func(body []byte) { + var req struct { + Nodes []struct { + Meta string `json:"meta"` + } `json:"nodes"` + } + require.NoError(t, json.Unmarshal(body, &req)) + require.Len(t, req.Nodes, 1) + sawMeta = req.Nodes[0].Meta + } + + results, err := rk.CreateNodes(ctx, []kegpkg.NodeCreate{{ + Key: "node", + Body: []byte("# Wire Meta\n"), + Meta: []byte("tags:\n - over-the-wire\n"), + }}) + require.NoError(t, err) + require.Len(t, results, 1) + + require.Contains(t, sawMeta, "over-the-wire", "meta was not sent in the create request") + + view, err := rk.ReadNode(ctx, results[0].ID) + require.NoError(t, err) + meta, err := kegpkg.ParseMeta(ctx, view.Meta) + require.NoError(t, err) + require.Contains(t, meta.Tags(), "over-the-wire", "meta did not persist through a hub-backed create") +} diff --git a/pkg/keg/keg_snapshots_test.go b/pkg/keg/keg_snapshots_test.go index eaecf4b6..226a5451 100644 --- a/pkg/keg/keg_snapshots_test.go +++ b/pkg/keg/keg_snapshots_test.go @@ -18,10 +18,7 @@ func TestKegSnapshotsRestoreSkipsSchemaEnforcement(t *testing.T) { k := kegpkg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, k, ctx) - id, err := k.Create(kegpkg.WithValidationMode(ctx, kegpkg.ValidationModeOff), &kegpkg.CreateOptions{ - Body: []byte("# Historical Task\n"), - Attrs: map[string]any{"type": "task"}, - }) + id, err := k.Create(kegpkg.WithValidationMode(ctx, kegpkg.ValidationModeOff), &kegpkg.CreateOptions{Body: []byte("# Historical Task\n"), Meta: []byte("type: task\n")}) require.NoError(t, err) snap, err := k.AppendSnapshot(ctx, id.ID, "before schema") require.NoError(t, err) @@ -62,7 +59,7 @@ func TestKegSnapshots_ReturnErrNotSupportedWithoutSnapshotBackend(t *testing.T) initNonStrictTestKeg(t, k, fx.Context()) - id, err := k.Create(fx.Context(), &kegpkg.CreateOptions{Title: "Snapshot Target"}) + id, err := k.Create(fx.Context(), &kegpkg.CreateOptions{Body: []byte("# Snapshot Target\n")}) require.NoError(t, err) _, err = k.AppendSnapshot(fx.Context(), id.ID, "before unsupported") diff --git a/pkg/keg/keg_test.go b/pkg/keg/keg_test.go index 9464be5c..95b6c813 100644 --- a/pkg/keg/keg_test.go +++ b/pkg/keg/keg_test.go @@ -117,11 +117,7 @@ func TestCreateNodeWithMeta(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - opts := &kegpkg.CreateOptions{ - Title: "MyTitle", - Lead: "short lead", - Tags: []string{"TagA", "tag-a"}, - } + opts := &kegpkg.CreateOptions{Body: []byte("# MyTitle\n\nshort lead\n"), Meta: []byte("tags:\n - TagA\n - tag-a\n")} id, err := k.Create(f.Context(), opts) require.NoError(t, err) require.Equal(t, 1, id.ID.ID, "expected created node id to be 1") @@ -168,9 +164,13 @@ func TestCreateWithBody(t *testing.T) { require.Equal(t, "body paragraph", stats.Lead()) } -// New test: Body contains YAML frontmatter. Ensure content written equals the -// provided bytes and parsed meta reflects the markdown heading and lead. -func TestCreateWithBodyFrontmatter(t *testing.T) { +// A node is built from two separate inputs, so content that opens with a +// frontmatter block is rejected rather than silently folded into metadata — +// otherwise a body legitimately starting with a horizontal rule would have its +// following lines eaten as meta. The same declarations belong in Meta, and this +// pins that the two halves still produce the title, lead, tags, and attributes +// the merged form used to. +func TestCreateRejectsBodyFrontmatterAndAcceptsMeta(t *testing.T) { t.Parallel() f := NewSandbox(t) @@ -178,7 +178,7 @@ func TestCreateWithBodyFrontmatter(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - rawBody := []byte(`--- + _, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte(`--- tags: - fm foo: bar @@ -186,15 +186,21 @@ foo: bar # FMTitle fm lead paragraph -`) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: rawBody}) +`)}) + require.ErrorIs(t, err, kegpkg.ErrInvalid) + require.Contains(t, err.Error(), "must not start with a YAML frontmatter block") + + rawBody := []byte("# FMTitle\n\nfm lead paragraph\n") + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ + Body: rawBody, + Meta: []byte("tags:\n - fm\nfoo: bar\n"), + }) require.NoError(t, err) - require.Equal(t, 1, id.ID.ID, "expected created node id to be 1") + require.Equal(t, 1, id.ID.ID, "the rejected create must not consume an id") got, err := k.GetContent(f.Context(), id.ID) - content, _ := kegpkg.ParseContent(f.Runtime(), rawBody, kegpkg.FormatMarkdown) require.NoError(t, err) - require.Equal(t, content.Body, string(got)) + require.Equal(t, string(rawBody), string(got)) m, err := k.GetMeta(f.Context(), id.ID) require.NoError(t, err) @@ -222,7 +228,7 @@ func TestSetContentAndUpdate(t *testing.T) { _, err := k.Create(f.Context(), nil) require.NoError(t, err) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Initial"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Initial\n")}) require.NoError(t, err) // change content to include a new lead paragraph @@ -249,10 +255,7 @@ func TestCreateAndUpdateNodesWithMemoryRepository(t *testing.T) { initNonStrictTestKeg(t, k, f.Context()) // Create a new node with title and lead. - opts := &kegpkg.CreateOptions{ - Title: "FSNode", - Lead: "lead fs", - } + opts := &kegpkg.CreateOptions{Body: []byte("# FSNode\n\nlead fs\n")} id, err := k.Create(f.Context(), opts) require.NoError(t, err) require.Equal(t, 1, id.ID.ID, "expected created node id to be 1") @@ -304,21 +307,13 @@ func TestNodesWithTagsAndInterlinks(t *testing.T) { initNonStrictTestKeg(t, k, f.Context()) // Create node A with tags - optsA := &kegpkg.CreateOptions{ - Title: "NodeA", - Lead: "lead a", - Tags: []string{"Alpha", "Shared"}, - } + optsA := &kegpkg.CreateOptions{Body: []byte("# NodeA\n\nlead a\n"), Meta: []byte("tags:\n - Alpha\n - Shared\n")} idA, err := k.Create(f.Context(), optsA) require.NoError(t, err) require.Equal(t, 1, idA.ID.ID) // Create node B with tags - optsB := &kegpkg.CreateOptions{ - Title: "NodeB", - Lead: "lead b", - Tags: []string{"Beta", "Shared"}, - } + optsB := &kegpkg.CreateOptions{Body: []byte("# NodeB\n\nlead b\n"), Meta: []byte("tags:\n - Beta\n - Shared\n")} idB, err := k.Create(f.Context(), optsB) require.NoError(t, err) require.Equal(t, 2, idB.ID.ID) @@ -380,11 +375,11 @@ func TestMarkdownLinkCreatesBacklinkWhileBareKegProseDoesNot(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - one, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "One"}) + one, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# One\n")}) require.NoError(t, err) - two, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Two"}) + two, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Two\n")}) require.NoError(t, err) - three, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Three"}) + three, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Three\n")}) require.NoError(t, err) require.NoError(t, k.SetContent(f.Context(), one.ID, []byte( @@ -408,7 +403,7 @@ func TestIndex_PreservesUnknownConfigFields(t *testing.T) { require.NoError(t, err, "NewKegFromTarget failed") initNonStrictTestKeg(t, k, f.Context()) - _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Settings Field Preservation"}) + _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Settings Field Preservation\n")}) require.NoError(t, err) customSettings := []byte(`kegv: "2025-07" @@ -445,11 +440,11 @@ func TestMove_RewritesLinksAndUpdatesDex(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id1, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "One"}) + id1, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# One\n")}) require.NoError(t, err) require.Equal(t, 1, id1.ID.ID) - id2, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Two"}) + id2, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Two\n")}) require.NoError(t, err) require.Equal(t, 2, id2.ID.ID) @@ -494,11 +489,11 @@ func TestMove_DestinationExists(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - _, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "One"}) + _, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# One\n")}) require.NoError(t, err) - _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Two"}) + _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Two\n")}) require.NoError(t, err) - _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Three"}) + _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Three\n")}) require.NoError(t, err) _, err = k.Move(f.Context(), moveOptions(t, f.Context(), k, kegpkg.NodeId{ID: 2}, kegpkg.NodeId{ID: 3})) @@ -514,9 +509,9 @@ func TestRemove_DeletesNodeAndUpdatesDex(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id1, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "One"}) + id1, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# One\n")}) require.NoError(t, err) - id2, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Two"}) + id2, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Two\n")}) require.NoError(t, err) require.NoError(t, k.SetContent(f.Context(), id1.ID, []byte("# One\n\nSee [two](../2).\n"))) @@ -553,7 +548,7 @@ func TestSetContent_OnRemovedNode(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Doomed"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Doomed\n")}) require.NoError(t, err) require.NoError(t, errOnly(k.Remove(f.Context(), removeOptions(t, f.Context(), k, id.ID)))) @@ -589,9 +584,9 @@ func TestSetMeta_PreservesLinksInDex(t *testing.T) { initNonStrictTestKeg(t, k, f.Context()) // Create two nodes - id1, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Source"}) + id1, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Source\n")}) require.NoError(t, err) - id2, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Target"}) + id2, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Target\n")}) require.NoError(t, err) // Set content with a link from node 1 to node 2 @@ -661,7 +656,7 @@ func TestIndex_MalformedMetaNodeGetsIndexed(t *testing.T) { initNonStrictTestKeg(t, k, f.Context()) // Create a node normally first, then corrupt its meta. - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Good Node"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Good Node\n")}) require.NoError(t, err) // Overwrite meta with invalid YAML. @@ -723,10 +718,7 @@ func TestSetMeta_NoChangeSkipsDexAndConfig(t *testing.T) { require.NoError(t, err) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: "Meta NoOp", - Tags: []string{"test"}, - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Meta NoOp\n"), Meta: []byte("tags:\n - test\n")}) require.NoError(t, err) // Normalize on-disk meta format by doing one round-trip through @@ -766,10 +758,7 @@ func TestSetMeta_WithChangeUpdatesDexAndConfig(t *testing.T) { require.NoError(t, err) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Title: "Meta Change", - Tags: []string{"old-tag"}, - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Meta Change\n"), Meta: []byte("tags:\n - old-tag\n")}) require.NoError(t, err) // Record keg settings updated timestamp after create. @@ -849,7 +838,7 @@ func TestIndexRefreshesStatsForOutOfBandMetadataChange(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Out Of Band Meta"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Out Of Band Meta\n")}) require.NoError(t, err) initialStats, err := k.GetStats(f.Context(), id.ID) require.NoError(t, err) @@ -934,10 +923,7 @@ func TestEditNoChange_SimulatesSaveWithoutChanges(t *testing.T) { initNonStrictTestKeg(t, k, f.Context()) body := []byte("# Edit NoOp\n\nSome content.\n") - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{ - Body: body, - Tags: []string{"edit-test"}, - }) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: body, Meta: []byte("tags:\n - edit-test\n")}) require.NoError(t, err) // First round-trip normalizes the on-disk meta format from Create's @@ -986,7 +972,7 @@ func TestCreateAlwaysTriggersUpdate(t *testing.T) { f.Advance(5 * time.Minute) expectedUpdated := f.Now().Format(time.RFC3339) - _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Title: "New Node"}) + _, err = k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# New Node\n")}) require.NoError(t, err) cfg2, err := k.Settings(f.Context()) @@ -1005,14 +991,14 @@ func TestDexFresh_ReloadsForExternalRepoImplementations(t *testing.T) { k := kegpkg.NewLocalKeg(repo, f.Runtime()) initNonStrictTestKeg(t, k, f.Context()) - _, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Original Node"}) + _, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Original Node\n")}) require.NoError(t, err) dex1, err := k.Dex(f.Context()) require.NoError(t, err) externalKeg := kegpkg.NewLocalKeg(repo, f.Runtime()) - externalID, err := externalKeg.Create(f.Context(), &kegpkg.CreateOptions{Title: "External Node"}) + externalID, err := externalKeg.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# External Node\n")}) require.NoError(t, err) require.Nil(t, dex1.GetRef(f.Context(), externalID.ID), "primed dex should not mutate behind the caller") @@ -1051,7 +1037,7 @@ func TestSetContent_LocalNodeIDStaysBare(t *testing.T) { initNonStrictTestKeg(t, k, f.Context()) require.Equal(t, "example", k.Target().KegName, "KegName must be set to reproduce the bug") - id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Node 2"}) + id, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Node 2\n")}) require.NoError(t, err) // SetContent is the edit path that previously tainted the dex entry. @@ -1087,9 +1073,9 @@ func TestMove_LocalNodeIDStaysBare(t *testing.T) { require.NoError(t, err) initNonStrictTestKeg(t, k, f.Context()) - target, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Target"}) + target, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Target\n")}) require.NoError(t, err) - referrer, err := k.Create(f.Context(), &kegpkg.CreateOptions{Title: "Referrer"}) + referrer, err := k.Create(f.Context(), &kegpkg.CreateOptions{Body: []byte("# Referrer\n")}) require.NoError(t, err) // Referrer links to target via a canonical relative node link. diff --git a/pkg/keg/node_meta.go b/pkg/keg/node_meta.go index 7ce0cb20..20f2c156 100644 --- a/pkg/keg/node_meta.go +++ b/pkg/keg/node_meta.go @@ -64,6 +64,15 @@ func ParseMeta(ctx context.Context, raw []byte) (*NodeMeta, error) { return nil, fmt.Errorf("failed to parse meta yaml: %w", err) } + // A node's metadata is a mapping of keys. Name the shape that arrived + // instead of surfacing the decoder's complaint about the internal struct + // it failed to fill — this message reaches API clients verbatim. + if len(doc.Content) > 0 { + if kind := describeYAMLKind(doc.Content[0].Kind); kind != "" { + return nil, fmt.Errorf("metadata must be a YAML mapping of keys, got %s", kind) + } + } + var tmp metaYAML if len(doc.Content) > 0 { if err := doc.Content[0].Decode(&tmp); err != nil { @@ -80,6 +89,22 @@ func ParseMeta(ctx context.Context, raw []byte) (*NodeMeta, error) { return m, nil } +// describeYAMLKind names a root node kind that cannot serve as metadata, +// returning "" for a mapping (the only acceptable shape) and for kinds where +// naming it would not help the caller. +func describeYAMLKind(kind yaml.Kind) string { + switch kind { + case yaml.SequenceNode: + return "a sequence" + case yaml.ScalarNode: + return "a scalar" + case yaml.AliasNode: + return "an alias" + default: + return "" + } +} + // ToYAML serializes only manually edited metadata fields. func (m *NodeMeta) ToYAML() string { return m.ToYAMLWithStats(nil) @@ -230,6 +255,30 @@ func (m *NodeMeta) Get(key string) (string, bool) { return val.Value, true } +// Keys returns the top-level keys present in the metadata document, in +// document order. It exists so callers can detect two inputs writing the same +// key before either write happens; Get cannot serve that purpose because it +// reports only scalars and so silently misses nested keys. +func (m *NodeMeta) Keys() []string { + if m == nil || m.node == nil || len(m.node.Content) == 0 { + if m != nil && len(m.tags) > 0 { + return []string{"tags"} + } + return nil + } + root := m.node.Content[0] + if root == nil || root.Kind != yaml.MappingNode { + return nil + } + out := make([]string, 0, len(root.Content)/2) + for i := 0; i+1 < len(root.Content); i += 2 { + if k := root.Content[i]; k != nil && k.Kind == yaml.ScalarNode { + out = append(out, k.Value) + } + } + return out +} + // Set updates known NodeMeta keys (tags) and preserves unknown keys in // the yaml node when available. func (m *NodeMeta) Set(ctx context.Context, key string, val any) error { diff --git a/pkg/keg/precondition_test.go b/pkg/keg/precondition_test.go index 6a750331..7b5b4d33 100644 --- a/pkg/keg/precondition_test.go +++ b/pkg/keg/precondition_test.go @@ -22,7 +22,7 @@ func TestLocalKegNodePreconditionsProtectContentAndMetadataTogether(t *testing.T ctx := fx.Context() k := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, k, ctx) - created, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Original\n"), Tags: []string{"before"}}) + created, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Original\n"), Meta: []byte("tags:\n - before\n")}) require.NoError(t, err) original, err := k.ReadNode(ctx, created.ID) require.NoError(t, err) @@ -183,9 +183,9 @@ func TestLocalKegQueryRemovalPinsHashesInsideWriteBoundary(t *testing.T) { ctx := fx.Context() k := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) initNonStrictTestKeg(t, k, ctx) - one, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# One\n"), Tags: []string{"discard"}}) + one, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# One\n"), Meta: []byte("tags:\n - discard\n")}) require.NoError(t, err) - two, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Two\n"), Tags: []string{"keep"}}) + two, err := k.Create(ctx, &keg.CreateOptions{Body: []byte("# Two\n"), Meta: []byte("tags:\n - keep\n")}) require.NoError(t, err) result, err := k.RemoveNodes(ctx, keg.RemoveNodesOptions{Query: "discard"}) diff --git a/pkg/keg/schema_selection_test.go b/pkg/keg/schema_selection_test.go index cf738445..51e0e821 100644 --- a/pkg/keg/schema_selection_test.go +++ b/pkg/keg/schema_selection_test.go @@ -105,12 +105,18 @@ func TestExplicitSchemaSelectionPersistsReplacesAndRejectsConflicts(t *testing.T typeName, _ = meta.Get("type") require.Equal(t, "note", typeName) - _, err = k.Create(ctx, &keg.CreateOptions{ - Schema: "task", Attrs: map[string]any{"type": "task"}, - Body: []byte("---\ntype: note\n---\n# Conflict\n"), - }) + // Content is never a second place to declare a type: a body opening with + // frontmatter is rejected outright, before schema selection is considered. + // That makes it a malformed request (ErrInvalid), not a schema violation. + _, err = k.Create(ctx, &keg.CreateOptions{Schema: "task", Body: []byte("---\ntype: note\n---\n# Conflict\n"), Meta: []byte("type: task\n")}) + require.ErrorIs(t, err, keg.ErrInvalid) + require.Contains(t, err.Error(), "must not start with a YAML frontmatter block") + + // A schema selection disagreeing with the type declared in meta stays a + // schema error, and is still never resolved by precedence. + _, err = k.Create(ctx, &keg.CreateOptions{Schema: "task", Body: []byte("# Conflict\n"), Meta: []byte("type: note\n")}) require.ErrorIs(t, err, keg.ErrSchemaInvalid) - require.Contains(t, err.Error(), "conflicts with attributes type") + require.Contains(t, err.Error(), "conflicts with metadata type") _, err = k.Create(ctx, &keg.CreateOptions{Schema: " ", Body: []byte("# Whitespace\n")}) require.ErrorIs(t, err, keg.ErrSchemaInvalid) diff --git a/pkg/keg/schema_test.go b/pkg/keg/schema_test.go index d5c014f8..670ecb67 100644 --- a/pkg/keg/schema_test.go +++ b/pkg/keg/schema_test.go @@ -97,7 +97,7 @@ markdown: t.Fatalf("human-created invalid node validated true; result=%#v", result) } - id, err = k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("---\ntype: task\n---\n# Typed\n\n## Context\n")}) + id, err = k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Typed\n\n## Context\n"), Meta: []byte("type: task\n")}) if err != nil { t.Fatalf("typed Create: %v", err) } @@ -202,10 +202,7 @@ markdown: t.Fatalf("WriteSchema note: %v", err) } - evidenceID, err := k.Create(ctx, &kegpkg.CreateOptions{ - Title: "Evidence", - Attrs: map[string]any{"type": "evidence", "status": "ready"}, - }) + evidenceID, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Evidence\n"), Meta: []byte("type: evidence\nstatus: ready\n")}) if err != nil { t.Fatalf("Create evidence: %v", err) } @@ -217,10 +214,7 @@ markdown: }); err != nil { t.Fatalf("unsnapshotted evidence edit: %v", err) } - sourceID, err := k.Create(ctx, &kegpkg.CreateOptions{ - Body: []byte("# Source\n\n[Evidence](../" + evidenceID.ID.Path() + ")\n"), - Attrs: map[string]any{"type": "note"}, - }) + sourceID, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Source\n\n[Evidence](../" + evidenceID.ID.Path() + ")\n"), Meta: []byte("type: note\n")}) if err != nil { t.Fatalf("Create source: %v", err) } @@ -240,10 +234,7 @@ markdown: t.Fatalf("snapshot omega = %v, want %v", snapshotOmega, 2.0/3.0) } - reviewID, err := k.Create(ctx, &kegpkg.CreateOptions{ - Body: []byte("# Review\n\n[Source](../" + sourceID.ID.Path() + ")\n"), - Attrs: map[string]any{"type": "evidence", "certainty": 0.5}, - }) + reviewID, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Review\n\n[Source](../" + sourceID.ID.Path() + ")\n"), Meta: []byte("type: evidence\ncertainty: 0.5\n")}) if err != nil { t.Fatalf("Create backlink evidence: %v", err) } @@ -319,10 +310,7 @@ markdown: if err := k.CreateSchema(ctx, "note", schema); err != nil { t.Fatalf("WriteSchema note: %v", err) } - id, err := k.Create(ctx, &kegpkg.CreateOptions{ - Title: "Own Metadata", - Attrs: map[string]any{"type": "note", "status": "review"}, - }) + id, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Own Metadata\n"), Meta: []byte("type: note\nstatus: review\n")}) if err != nil { t.Fatalf("Create note: %v", err) } @@ -374,10 +362,7 @@ markdown: if err := k.CreateSchema(ctx, "note", schema); err != nil { t.Fatalf("WriteSchema note: %v", err) } - id, err := k.Create(ctx, &kegpkg.CreateOptions{ - Title: "Legacy Metadata", - Attrs: map[string]any{"type": "note", "status": "ready"}, - }) + id, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Legacy Metadata\n"), Meta: []byte("type: note\nstatus: ready\n")}) if err != nil { t.Fatalf("Create note: %v", err) } @@ -435,20 +420,14 @@ markdown: t.Fatalf("WriteSchema note: %v", err) } - evidenceID, err := k.Create(ctx, &kegpkg.CreateOptions{ - Title: "Evidence", - Attrs: map[string]any{"type": "evidence", "confidence": 1}, - }) + evidenceID, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Evidence\n"), Meta: []byte("type: evidence\nconfidence: 1\n")}) if err != nil { t.Fatalf("Create evidence: %v", err) } if _, err := k.AppendSnapshot(ctx, evidenceID.ID, "evidence"); err != nil { t.Fatalf("AppendSnapshot evidence: %v", err) } - noteID, err := k.Create(ctx, &kegpkg.CreateOptions{ - Body: []byte("# Source\n\n[Evidence](../" + evidenceID.ID.Path() + ")\n"), - Attrs: map[string]any{"type": "note", "status": "draft"}, - }) + noteID, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Source\n\n[Evidence](../" + evidenceID.ID.Path() + ")\n"), Meta: []byte("type: note\nstatus: draft\n")}) if err != nil { t.Fatalf("Create note: %v", err) } diff --git a/pkg/keg/snapshot_indexes_test.go b/pkg/keg/snapshot_indexes_test.go index fb19e71d..45572de8 100644 --- a/pkg/keg/snapshot_indexes_test.go +++ b/pkg/keg/snapshot_indexes_test.go @@ -24,7 +24,7 @@ func TestTimelineIndex_OrdersSnapshotRowsAndReplaysBacklinks(t *testing.T) { setSnapshotIndexClock(t, rt, t1) alphaResult, err := k.Create(ctx, &CreateOptions{ - Body: []byte("---\ntype: task\n---\n# Alpha\n\nSee [three](../3).\n"), + Body: []byte("# Alpha\n\nSee [three](../3).\n"), Meta: []byte("type: task\n"), }) require.NoError(t, err) alpha := alphaResult.ID @@ -148,7 +148,7 @@ markdown: t1 := time.Date(2026, 2, 26, 10, 0, 0, 0, time.UTC) setSnapshotIndexClock(t, rt, t1) - evidenceResult, err := k.Create(ctx, &CreateOptions{Title: "Evidence", Attrs: map[string]any{"type": "evidence", "status": "ready"}}) + evidenceResult, err := k.Create(ctx, &CreateOptions{Body: []byte("# Evidence\n"), Meta: []byte("type: evidence\nstatus: ready\n")}) require.NoError(t, err) evidence := evidenceResult.ID _, err = k.AppendSnapshot(ctx, evidence, "evidence") @@ -156,8 +156,8 @@ markdown: setSnapshotIndexClock(t, rt, t1.Add(time.Hour)) sourceResult, err := k.Create(ctx, &CreateOptions{ - Body: []byte("# Source\n\n[Evidence](../" + evidence.Path() + ")\n"), - Attrs: map[string]any{"type": "note"}, + Body: []byte("# Source\n\n[Evidence](../" + evidence.Path() + ")\n"), + Meta: []byte("type: note\n"), }) require.NoError(t, err) source := sourceResult.ID @@ -166,8 +166,8 @@ markdown: setSnapshotIndexClock(t, rt, t1.Add(2*time.Hour)) reviewResult, err := k.Create(ctx, &CreateOptions{ - Body: []byte("# Review\n\n[Source](../" + source.Path() + ")\n"), - Attrs: map[string]any{"type": "evidence", "certainty": 0.5}, + Body: []byte("# Review\n\n[Source](../" + source.Path() + ")\n"), + Meta: []byte("type: evidence\ncertainty: 0.5\n"), }) require.NoError(t, err) review := reviewResult.ID diff --git a/pkg/keg/snapshot_policy_internal_test.go b/pkg/keg/snapshot_policy_internal_test.go index 6170c226..f5ce433a 100644 --- a/pkg/keg/snapshot_policy_internal_test.go +++ b/pkg/keg/snapshot_policy_internal_test.go @@ -86,7 +86,7 @@ func TestSnapshotPolicy_BadLatestContentHashWithIdenticalContentDoesNotDuplicate fx, k, repo := newInternalSnapshotPolicyTestKeg(t) ctx := fx.Context() - id, err := k.Create(ctx, &CreateOptions{Title: "Legacy Hash Target"}) + id, err := k.Create(ctx, &CreateOptions{Body: []byte("# Legacy Hash Target\n")}) require.NoError(t, err) fx.Advance(2 * time.Hour) result, err := k.RunSnapshotPolicy(ctx) @@ -120,7 +120,7 @@ func TestSnapshotPolicy_BadLatestContentHashWithDifferentContentCreatesSnapshot( fx, k, repo := newInternalSnapshotPolicyTestKeg(t) ctx := fx.Context() - id, err := k.Create(ctx, &CreateOptions{Title: "Legacy Drift Target"}) + id, err := k.Create(ctx, &CreateOptions{Body: []byte("# Legacy Drift Target\n")}) require.NoError(t, err) fx.Advance(2 * time.Hour) result, err := k.RunSnapshotPolicy(ctx) diff --git a/pkg/keg/snapshot_policy_test.go b/pkg/keg/snapshot_policy_test.go index 46bcf583..c16816d3 100644 --- a/pkg/keg/snapshot_policy_test.go +++ b/pkg/keg/snapshot_policy_test.go @@ -15,7 +15,7 @@ func TestSnapshotPolicy_MissingSnapshotAfterIdle(t *testing.T) { fx, k := newSnapshotPolicyTestKeg(t) ctx := fx.Context() - id, err := k.Create(ctx, &kegpkg.CreateOptions{Title: "Policy Target"}) + id, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Policy Target\n")}) require.NoError(t, err) fx.Advance(59 * time.Minute) @@ -50,7 +50,7 @@ func TestSnapshotPolicy_OffModeSkipsSnapshots(t *testing.T) { require.NoError(t, k.UpdateSettings(ctx, func(cfg *kegpkg.Settings) { cfg.Snapshots = &kegpkg.SnapshotSettings{Mode: kegpkg.SnapshotModeOff} })) - _, err := k.Create(ctx, &kegpkg.CreateOptions{Title: "No Auto Snapshot"}) + _, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# No Auto Snapshot\n")}) require.NoError(t, err) fx.Advance(2 * time.Hour) @@ -64,7 +64,7 @@ func TestSnapshotPolicy_ContentDriftCreatesSnapshot(t *testing.T) { fx, k := newSnapshotPolicyTestKeg(t) ctx := fx.Context() - id, err := k.Create(ctx, &kegpkg.CreateOptions{Title: "Drift Target"}) + id, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Drift Target\n")}) require.NoError(t, err) fx.Advance(2 * time.Hour) result, err := k.RunSnapshotPolicy(ctx) @@ -84,7 +84,7 @@ func TestSnapshotPolicy_MetadataDriftDoesNotCreateSnapshotAndRemainsDirty(t *tes fx, k := newSnapshotPolicyTestKeg(t) ctx := fx.Context() - id, err := k.Create(ctx, &kegpkg.CreateOptions{Title: "Metadata Target"}) + id, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Metadata Target\n")}) require.NoError(t, err) fx.Advance(2 * time.Hour) result, err := k.RunSnapshotPolicy(ctx) @@ -117,7 +117,7 @@ func TestSnapshotPolicy_TouchDoesNotCreateSnapshot(t *testing.T) { fx, k := newSnapshotPolicyTestKeg(t) ctx := fx.Context() - id, err := k.Create(ctx, &kegpkg.CreateOptions{Title: "Touch Target"}) + id, err := k.Create(ctx, &kegpkg.CreateOptions{Body: []byte("# Touch Target\n")}) require.NoError(t, err) fx.Advance(2 * time.Hour) result, err := k.RunSnapshotPolicy(ctx) diff --git a/pkg/mcp/auth_error_internal_test.go b/pkg/mcp/auth_error_internal_test.go index 2ddbd7d7..aa82032f 100644 --- a/pkg/mcp/auth_error_internal_test.go +++ b/pkg/mcp/auth_error_internal_test.go @@ -97,12 +97,23 @@ func TestErrorResult_AuthFailuresLeakNoCredentials(t *testing.T) { } } -// TestErrorResult_NonAuthErrorsAreUnchanged confirms the new branch did not +// TestErrorResult_NonAuthErrorsAreUnchanged confirms the auth branch did not // swallow the precondition paths that sit next to it. +// +// A plain error used to return nil StructuredContent. It no longer does: every +// error now carries the recovery contract, and an unclassified one reports its +// outcome as unknown. See TestErrorResultsAlwaysCarryTheRecoveryContract. func TestErrorResult_NonAuthErrorsAreUnchanged(t *testing.T) { res := errorResult(errors.New("something ordinary went wrong")) - if res.StructuredContent != nil { - t.Fatalf("StructuredContent = %v, want nil for a plain error", res.StructuredContent) + plain, ok := res.StructuredContent.(map[string]any) + if !ok { + t.Fatalf("plain error lost its structured content") + } + if plain["code"] != keg.RemoteCodeInternal { + t.Fatalf("code = %v, want %v", plain["code"], keg.RemoteCodeInternal) + } + if plain["operationPerformed"] != nil { + t.Fatalf("operationPerformed = %v, want nil (unknown) for an unclassified error", plain["operationPerformed"]) } res = errorResult(fmt.Errorf("write: %w", keg.ErrPreconditionRequired)) diff --git a/pkg/mcp/error_contract_internal_test.go b/pkg/mcp/error_contract_internal_test.go new file mode 100644 index 00000000..d2f48c6a --- /dev/null +++ b/pkg/mcp/error_contract_internal_test.go @@ -0,0 +1,99 @@ +package mcp + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/jlrickert/tapper/pkg/keg" +) + +// TestErrorResultsAlwaysCarryTheRecoveryContract is the guard that keeps the +// error contract from decaying back into bare prose. +// +// An agent that gets only a sentence cannot tell whether its write landed, and +// in field testing that gap produced invented rules: a tester concluded `edit` +// required `schema` and that `expected_hash` validation was self-contradictory, +// because errors said what was wrong without saying where or what to do next. +// +// Every error a tool can return must therefore carry a code, a next step, and +// an explicit statement about whether state changed. +func TestErrorResultsAlwaysCarryTheRecoveryContract(t *testing.T) { + t.Parallel() + + sentinels := []struct { + name string + err error + }{ + {"not found", keg.ErrNotExist}, + {"already exists", keg.ErrExist}, + {"destination exists", keg.ErrDestinationExists}, + {"invalid", keg.ErrInvalid}, + {"schema invalid", keg.ErrSchemaInvalid}, + {"invalid image", keg.ErrInvalidImage}, + {"lock mismatch", keg.ErrLockTokenMismatch}, + {"not locked", keg.ErrNotLocked}, + {"lock", keg.ErrLock}, + {"lock timeout", keg.ErrLockTimeout}, + {"not supported", keg.ErrNotSupported}, + {"precondition required", keg.ErrPreconditionRequired}, + {"precondition conflict", &keg.PreconditionConflictError{Resource: "node 1", CurrentHash: "abc"}}, + {"unauthorized", keg.ErrUnauthorized}, + {"forbidden", keg.ErrForbidden}, + {"orientation denied", ErrOrientationDenied}, + {"orientation unavailable", ErrOrientationUnavailable}, + {"orientation root unavailable", ErrOrientationRootUnavailable}, + {"unclassified", fmt.Errorf("some backend blew up")}, + } + + for _, tc := range sentinels { + t.Run(tc.name, func(t *testing.T) { + res := errorResult(tc.err) + require.True(t, res.IsError, "result is not marked as an error") + require.NotNil(t, res.StructuredContent, "error carries no structured content") + + raw, err := json.Marshal(res.StructuredContent) + require.NoError(t, err) + var got map[string]any + require.NoError(t, json.Unmarshal(raw, &got)) + + code, _ := got["code"].(string) + require.NotEmpty(t, code, "error carries no code") + + action, _ := got["action"].(string) + require.NotEmpty(t, action, "error carries no action; an agent cannot know what to do next") + require.Greater(t, len(action), 25, "action %q is too terse to be actionable", action) + + performed, present := got["operationPerformed"] + require.True(t, present, "error does not say whether state changed") + if performed != nil { + require.IsType(t, false, performed, + "operationPerformed must be a bool or null (null meaning genuinely unknown)") + } + + // The action must tell the caller what to do, not restate the code. + require.NotEqual(t, strings.ToLower(code), strings.ToLower(action)) + }) + } +} + +// TestUnclassifiedErrorsDoNotClaimNothingHappened pins the one case where +// honesty beats a definite answer. An unrecognised failure may have been raised +// after a partial write, so reporting operationPerformed:false would be a guess +// — and the field is only useful if an agent can trust it. +func TestUnclassifiedErrorsDoNotClaimNothingHappened(t *testing.T) { + t.Parallel() + + raw, err := json.Marshal(errorResult(fmt.Errorf("backend exploded")).StructuredContent) + require.NoError(t, err) + var got map[string]any + require.NoError(t, json.Unmarshal(raw, &got)) + + performed, present := got["operationPerformed"] + require.True(t, present) + require.Nil(t, performed, "an unclassified failure must report unknown, not false") + require.Contains(t, got["action"], "cat", "the action must tell the agent how to establish current state") +} diff --git a/pkg/mcp/precondition_read_test.go b/pkg/mcp/precondition_read_test.go index b6af1a04..21d34e52 100644 --- a/pkg/mcp/precondition_read_test.go +++ b/pkg/mcp/precondition_read_test.go @@ -277,3 +277,94 @@ func TestPrecondition_ReadsExposeDocumentTokens(t *testing.T) { require.False(t, minimal.IsError) require.Nil(t, minimal.StructuredContent, "the minimal summary must not offer a write token") } + +// TestCat_StructuredRowsAreSelfContained pins tapper#93: one cat row must carry +// the node's document AND the hash a write echoes back. They used to live in +// separate response surfaces — hash in structuredContent, document only in the +// rendered text — so an agent doing read-modify-write had to parse output meant +// for humans, and a multi-node read had to correlate two lists by position. +// +// Content and meta are asserted separately because that is the shape `edit` +// accepts; a composed ---meta---body blob could not be sent back, since edit +// rejects frontmatter inside content. +func TestCat_StructuredRowsAreSelfContained(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + first := createNodeForTest(t, session, ctx, "# Alpha\n\nAlpha body.\n", "tags:\n - alpha\n") + second := createNodeForTest(t, session, ctx, "# Beta\n\nBeta body.\n", "tags:\n - beta\n") + + rows := catRows(t, session, ctx, map[string]any{"node_ids": []string{first, second}}) + require.Len(t, rows, 2) + require.Equal(t, []string{first, second}, []string{rows[0].NodeID, rows[1].NodeID}, + "rows must stay in request order so no positional correlation is needed") + for i, row := range rows { + require.NotEmpty(t, row.Hash, "row %d has no hash", i) + require.Contains(t, row.Content, "body.", "row %d has no content", i) + require.Contains(t, row.Meta, "tags:", "row %d has no meta", i) + } + require.Contains(t, rows[0].Content, "# Alpha") + require.Contains(t, rows[1].Meta, "beta") + + // meta_only: metadata only, and the hash still works for a write. + metaRows := catRows(t, session, ctx, map[string]any{"node_ids": []string{first}, "meta_only": true}) + require.Len(t, metaRows, 1) + require.Contains(t, metaRows[0].Meta, "alpha") + require.Empty(t, metaRows[0].Content, "meta_only must not return content") + + // content_only: the inverse. + contentRows := catRows(t, session, ctx, map[string]any{"node_ids": []string{first}, "content_only": true}) + require.Len(t, contentRows, 1) + require.Contains(t, contentRows[0].Content, "# Alpha") + require.Empty(t, contentRows[0].Meta, "content_only must not return meta") + + // stats_only is explicit too. + statsRows := catRows(t, session, ctx, map[string]any{"node_ids": []string{first}, "stats_only": true}) + require.Len(t, statsRows, 1) + require.NotEmpty(t, statsRows[0].Stats, "stats_only must return stats") + + // The whole point: a structured row round-trips into edit with no parsing. + editRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "edit", + Arguments: batchEditArgs(map[string]any{ + "node_id": metaRows[0].NodeID, + "meta": metaRows[0].Meta, + "expected_hash": metaRows[0].Hash, + }), + }) + require.NoError(t, err) + require.False(t, editRes.IsError, "structured meta did not round-trip: %s", extractText(t, editRes)) +} + +type catRow struct { + NodeID string `json:"node_id"` + Hash string `json:"hash"` + Content string `json:"content"` + Meta string `json:"meta"` + Stats string `json:"stats"` +} + +func catRows(t *testing.T, session *sdkmcp.ClientSession, ctx context.Context, args map[string]any) []catRow { + t.Helper() + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{Name: "cat", Arguments: args}) + require.NoError(t, err) + require.False(t, res.IsError, "cat returned error: %s", extractText(t, res)) + raw, err := json.Marshal(res.StructuredContent) + require.NoError(t, err) + var out struct { + Nodes []catRow `json:"nodes"` + } + require.NoError(t, json.Unmarshal(raw, &out)) + return out.Nodes +} + +func createNodeForTest(t *testing.T, session *sdkmcp.ClientSession, ctx context.Context, content, meta string) string { + t.Helper() + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "create", + Arguments: map[string]any{"nodes": []any{map[string]any{"key": "node", "content": content, "meta": meta}}}, + }) + require.NoError(t, err) + require.False(t, res.IsError, "create returned error: %s", extractText(t, res)) + return extractText(t, res) +} diff --git a/pkg/mcp/precondition_schema_test.go b/pkg/mcp/precondition_schema_test.go index f863d259..7746c621 100644 --- a/pkg/mcp/precondition_schema_test.go +++ b/pkg/mcp/precondition_schema_test.go @@ -85,17 +85,18 @@ func TestMCP_MutationSchemasRequireExpectedHashesAtResourceLocation(t *testing.T } for tool, array := range map[string]string{ - "edit": "edits", "meta": "updates", "remove": "nodes", + "edit": "nodes", "remove": "nodes", } { root, ok := schemas[tool] require.True(t, ok, "missing tool %q", tool) requireSchemaField(t, schemaArrayItem(t, root, array), "expected_hash", true) } - // Metadata reads use node_ids and never need a mutation token. Requiring - // expected_hash only inside updates keeps that read mode token-free. - meta := schemas["meta"] - requireSchemaField(t, meta, "expected_hash", false) + // create allocates ids, so there is no prior revision to guard and no + // expected_hash anywhere in its schema. + createItem := schemaArrayItem(t, schemas["create"], "nodes") + requireSchemaField(t, createItem, "expected_hash", false) + requireSchemaField(t, schemas["create"], "expected_hash", false) } func TestMCP_MutationDescriptionsTeachReadMergeRetryProtocol(t *testing.T) { @@ -105,7 +106,7 @@ func TestMCP_MutationDescriptionsTeachReadMergeRetryProtocol(t *testing.T) { require.NoError(t, err) wants := map[string]string{ - "edit": "cat", "meta": "cat", "remove": "cat", "move": "cat", + "edit": "cat", "remove": "cat", "move": "cat", "keg_settings_edit": "keg_settings", "schema_edit": "schema_read", "schema_delete": "schema_read", "flight_edit": "flight_show", "flight_delete": "flight_show", } @@ -124,3 +125,55 @@ func TestMCP_MutationDescriptionsTeachReadMergeRetryProtocol(t *testing.T) { require.True(t, seen[tool], "missing tool description for %q", tool) } } + +// TestMCP_AuthorityBearingToolsDescribeFlight pins that the injected flight +// property is also documented. schemaWithFlight adds `flight` to every +// authority-bearing tool's schema, but an agent reading descriptions rather +// than raw schemas saw no mention of it and reported the parameter as missing +// from keg_create. A property nothing describes reads as absent. +func TestMCP_AuthorityBearingToolsDescribeFlight(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + result, err := session.ListTools(ctx, nil) + require.NoError(t, err) + + checked := 0 + for _, tool := range result.Tools { + schema, err := json.Marshal(tool.InputSchema) + require.NoError(t, err) + var object struct { + Properties map[string]json.RawMessage `json:"properties"` + } + require.NoError(t, json.Unmarshal(schema, &object)) + if _, ok := object.Properties["flight"]; !ok { + continue + } + checked++ + require.Containsf(t, strings.ToLower(tool.Description), "flight", + "tool %q accepts a flight property but never mentions it in its description", tool.Name) + } + require.Greater(t, checked, 0, "no tool exposed a flight property; the injection may have broken") +} + +// TestMCP_WriteToolsStateTheContentContract keeps the two rules an agent +// previously had to discover by triggering them in the descriptions themselves. +func TestMCP_WriteToolsStateTheContentContract(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + result, err := session.ListTools(ctx, nil) + require.NoError(t, err) + + byName := map[string]string{} + for _, tool := range result.Tools { + byName[tool.Name] = strings.ToLower(tool.Description) + } + + for _, name := range []string{"create", "edit"} { + desc, ok := byName[name] + require.Truef(t, ok, "missing tool %q", name) + require.Containsf(t, desc, "frontmatter", + "%q does not say content must not begin with a frontmatter block", name) + } + require.Contains(t, byName["edit"], "content and metadata together", + "edit does not state that one hash covers both halves of a node") +} diff --git a/pkg/mcp/server.go b/pkg/mcp/server.go index 858e56b9..9080dff5 100644 --- a/pkg/mcp/server.go +++ b/pkg/mcp/server.go @@ -99,7 +99,6 @@ func NewServer(tap *tapper.Tap, version string, defaults KegDefaults, opts ...Se registerFileTools(srv, tap, defaults, opt.SharedFilesystem) registerDoctorTools(srv, tap, defaults) registerLockTools(srv, tap, defaults) - registerImportTools(srv, tap, defaults) registerFlightTools(srv, defaults, opt.FlightProvider) registerKegTools(srv, defaults, opt.KegProvider, opt.KegSearchProvider) registerResourceTools(srv, tap, defaults) @@ -235,14 +234,59 @@ func errorResult(err error) *sdkmcp.CallToolResult { IsError: true, } } + code, _ := keg.RemoteErrorCode(err) + action, performed := errorGuidance(code) return &sdkmcp.CallToolResult{ Content: []sdkmcp.Content{ - &sdkmcp.TextContent{Text: err.Error()}, + &sdkmcp.TextContent{Text: err.Error() + "\n\n" + action}, + }, + StructuredContent: map[string]any{ + "code": code, + "operationPerformed": performed, + "action": action, }, IsError: true, } } +// errorGuidance maps a wire code to the caller's next step and to whether the +// operation changed state. +// +// operationPerformed is deliberately three-valued. false means the request was +// refused before anything was written, which is knowable for every code below +// because each is raised during validation or precondition checking. nil — JSON +// null — means the outcome is genuinely unknown, which is the honest answer for +// an unclassified failure that may have been raised mid-write. Reporting false +// there would be a guess, and the whole point of this field is that an agent +// can trust it instead of reading the node back to find out. +// +// Actions are imperative and name the tool or field to change. They never +// restate the error text, which is already in the message. +func errorGuidance(code string) (action string, operationPerformed any) { + switch code { + case keg.RemoteCodeNotFound: + return "The target does not exist. Confirm the id with `list` or `grep`; create it with `create`.", false + case keg.RemoteCodeExist, keg.RemoteCodeDestExists: + return "Something already occupies that id or name. Choose another, or edit the existing node instead.", false + case keg.RemoteCodeSchemaInvalid: + return "The node does not satisfy its schema. Read the issues in this error, fix `content` or `meta`, and retry. `schema_read` shows the schema's requirements.", false + case keg.RemoteCodeInvalid, keg.RemoteCodeBadRequest: + return "The request is malformed. Fix the field named in the error and retry; do not retry the same arguments.", false + case keg.RemoteCodeInvalidImage: + return "The uploaded bytes are not a decodable image. Re-encode as PNG, JPEG, GIF, or WebP and retry.", false + case keg.RemoteCodeLockMismatch: + return "Another session holds this node's lock. Call `lock_status` to see the holder; wait, or use `lock_force_release` if you own the session that stranded it.", false + case keg.RemoteCodeNotLocked: + return "This operation needs a lock you do not hold. Call `lock_acquire` first.", false + case keg.RemoteCodeLock, keg.RemoteCodeLockTimeout: + return "The lock could not be acquired in time, usually because another write is in flight. Retry shortly; nothing was written.", false + case keg.RemoteCodeNotSupported: + return "This backend does not implement the operation. Do not retry — use a different tool or target a hub-backed keg.", false + default: + return "The outcome is unknown: this failure may have been raised after a partial write. Read the node with `cat` to establish current state before retrying, and do not blindly replay the mutation.", nil + } +} + // invocationLoggingMiddleware returns an MCP middleware that logs every // tools/call request with timing and client metadata. Timing is measured // via the provided Clock so that sandboxed tests can inject a fake clock diff --git a/pkg/mcp/server_test.go b/pkg/mcp/server_test.go index f73e52f2..8903c221 100644 --- a/pkg/mcp/server_test.go +++ b/pkg/mcp/server_test.go @@ -86,7 +86,7 @@ func newMemoryTap(t *testing.T, ctx context.Context, rt *toolkit.Runtime) *tappe } created, err := local.Create(ctx, &keg.CreateOptions{ Body: []byte("# Hello World\n\nA simple test node that links to [overview](../0).\n"), - Tags: []string{"test", "hello"}, + Meta: []byte("tags:\n - test\n - hello\n"), }) if err != nil { return nil, err @@ -230,10 +230,10 @@ func TestMCP_ToolsList(t *testing.T) { } for _, want := range []string{ "auth_info", "keg_list", "keg_search", "cat", "list", "grep", "tags", "backlinks", "links", "info", - "keg_settings", "keg_settings_edit", "stats", "create", "edit", "meta", "remove", "move", + "keg_settings", "keg_settings_edit", "stats", "create", "edit", "remove", "move", "index", "list_indexes", "index_cat", "doctor", "node_history", "node_snapshot", "node_snapshot_view", "node_restore", "list_files", "list_images", "delete_file", "delete_image", - "upload_file", "upload_image", "download_image", "orient", "session_refresh", "import_from_keg", + "upload_file", "upload_image", "download_image", "orient", "session_refresh", "lock_acquire", "lock_release", "lock_status", "lock_force_release", "list_flights", "flight_show", "flight_create", "flight_edit", "flight_delete", "schema_list", "schema_read", "schema_create", "schema_edit", "schema_delete", "validate", @@ -273,13 +273,13 @@ func TestMCP_CommonAgentSafeSurface(t *testing.T) { for _, want := range []string{ "cat", "list", "grep", "tags", "backlinks", "links", "info", "keg_settings", "keg_settings_edit", - "stats", "create", "edit", "meta", "remove", "move", "index", + "stats", "create", "edit", "remove", "move", "index", "list_indexes", "index_cat", "node_history", "node_snapshot", "node_snapshot_view", "node_restore", "orient", "session_refresh", "list_files", "list_images", "delete_file", "delete_image", "upload_file", "upload_image", "download_image", "schema_list", "schema_read", "schema_create", "schema_edit", - "schema_delete", "validate", "doctor", "import_from_keg", "keg_list", "keg_search", "auth_info", + "schema_delete", "validate", "doctor", "keg_list", "keg_search", "auth_info", "lock_acquire", "lock_release", "lock_status", "lock_force_release", "list_flights", "flight_show", "flight_create", "flight_edit", "flight_delete", } { @@ -293,6 +293,9 @@ func TestMCP_CommonAgentSafeSurface(t *testing.T) { "namespace_list", "namespace_members", "namespace_add_member", "namespace_set_role", "namespace_remove_member", "namespace_create", "flight_update", + // meta was folded into edit (writes) and cat meta_only (reads); it + // must not come back as a third way to touch node metadata. + "meta", } { require.Falsef(t, names[banned], "common surface must not expose %q", banned) } @@ -593,23 +596,31 @@ func TestMCP_CatError(t *testing.T) { // --- write tool tests --- +// batchCreateArgs builds a one-node create payload. It accepts the "title" and +// "lead" shorthands the create tool no longer has and folds them into the +// markdown content, so tests that only need *a node to exist* stay readable +// and do not each have to spell out a heading. func batchCreateArgs(item map[string]any) map[string]any { + title, hasTitle := item["title"].(string) + lead, hasLead := item["lead"].(string) + if hasTitle || hasLead { + delete(item, "title") + delete(item, "lead") + if !hasTitle { + title = "Node" + } + content := "# " + title + "\n" + if hasLead { + content += "\n" + lead + "\n" + } + item["content"] = content + } item["key"] = "node" return map[string]any{"nodes": []any{item}} } func batchEditArgs(item map[string]any) map[string]any { - return map[string]any{"edits": []any{item}} -} - -func batchMetaArgs(item map[string]any) map[string]any { - nodeID := item["node_id"] - delete(item, "node_id") - if _, writes := item["content"]; writes { - item["node_id"] = nodeID - return map[string]any{"updates": []any{item}} - } - return map[string]any{"node_ids": []any{nodeID}} + return map[string]any{"nodes": []any{item}} } func batchSnapshotArgs(item map[string]any) map[string]any { @@ -624,7 +635,6 @@ func TestMCPMutationSchemasRejectLegacySingleItemFields(t *testing.T) { }{ {"create", map[string]any{"title": "legacy"}}, {"edit", map[string]any{"node_id": "0", "content": "# legacy\n"}}, - {"meta", map[string]any{"node_id": "0"}}, {"remove", map[string]any{"node_ids": []string{"0"}, "expected_hash": "legacy"}}, {"node_snapshot", map[string]any{"node_id": "0"}}, } { @@ -637,11 +647,9 @@ func TestMCPMutationSchemasRejectLegacySingleItemFields(t *testing.T) { func TestMCPMutationSchemasRejectEmptyAndOversizedArrays(t *testing.T) { session, ctx := newTestSession(t) oversizedObjects := make([]any, 101) - oversizedIDs := make([]any, 101) oversizedRemovals := make([]any, 101) for i := range oversizedObjects { oversizedObjects[i] = map[string]any{"key": fmt.Sprintf("node-%d", i)} - oversizedIDs[i] = "0" oversizedRemovals[i] = map[string]any{"node_id": fmt.Sprintf("%d", i), "expected_hash": "hash"} } for _, tc := range []struct { @@ -650,12 +658,8 @@ func TestMCPMutationSchemasRejectEmptyAndOversizedArrays(t *testing.T) { }{ {"create empty", map[string]any{"nodes": []any{}}}, {"create oversized", map[string]any{"nodes": oversizedObjects}}, - {"edit empty", map[string]any{"edits": []any{}}}, - {"edit oversized", map[string]any{"edits": oversizedObjects}}, - {"meta read empty", map[string]any{"node_ids": []any{}}}, - {"meta read oversized", map[string]any{"node_ids": oversizedIDs}}, - {"meta update empty", map[string]any{"updates": []any{}}}, - {"meta update oversized", map[string]any{"updates": oversizedObjects}}, + {"edit empty", map[string]any{"nodes": []any{}}}, + {"edit oversized", map[string]any{"nodes": oversizedObjects}}, {"remove empty", map[string]any{"nodes": []any{}}}, {"remove oversized", map[string]any{"nodes": oversizedRemovals}}, {"snapshot empty", map[string]any{"nodes": []any{}}}, @@ -702,7 +706,7 @@ meta: invalid, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ Name: "create", - Arguments: map[string]any{"nodes": []any{map[string]any{"key": "invalid", "body": "# Missing type\n"}}}, + Arguments: map[string]any{"nodes": []any{map[string]any{"key": "invalid", "content": "# Missing type\n"}}}, }) require.NoError(t, err) require.True(t, invalid.IsError, "MCP write was reclassified as human and bypassed agent:block") @@ -711,7 +715,7 @@ meta: valid, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ Name: "create", Arguments: map[string]any{"nodes": []any{map[string]any{ - "key": "valid", "schema": "task", "body": "# Typed\n", "attrs": map[string]any{"type": "task"}, + "key": "valid", "schema": "task", "content": "# Typed\n", "meta": "type: task\n", }}}, }) require.NoError(t, err) @@ -727,7 +731,7 @@ func TestMCP_Create(t *testing.T) { Arguments: batchCreateArgs(map[string]any{ "title": "New Node", "lead": "A node created via MCP.", - "tags": []string{"mcp-test"}, + "meta": "tags:\n - mcp-test\n", }), }) require.NoError(t, err) @@ -750,13 +754,71 @@ func TestMCP_Create(t *testing.T) { require.Contains(t, readText, "A node created via MCP.") } +// TestMCP_CreateRejectsFrontmatterInContent keeps content and meta from being +// two ways to write the same node metadata. +func TestMCP_CreateRejectsFrontmatterInContent(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "create", + Arguments: map[string]any{"nodes": []any{map[string]any{ + "key": "node", + "content": "---\ntags:\n - sneaky\n---\n\n# Body\n", + }}}, + }) + require.NoError(t, err) + require.True(t, res.IsError, "create accepted frontmatter in content") + require.Contains(t, extractText(t, res), "meta field") +} + +// TestMCP_CreateRejectsSchemaConflictingWithMeta pins the promise made in the +// schema field's description: a type declared in meta is not overridden by a +// different schema selection, it is refused. +func TestMCP_CreateRejectsSchemaConflictingWithMeta(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "create", + Arguments: map[string]any{"nodes": []any{map[string]any{ + "key": "node", + "content": "# Conflicted\n", + "meta": "type: note\n", + "schema": "task", + }}}, + }) + require.NoError(t, err) + require.True(t, res.IsError, "create accepted a schema conflicting with meta.type") + require.Contains(t, extractText(t, res), "conflicts with") +} + +// TestMCP_CreateRejectsLegacyStructuredFields pins that title, lead, tags, and +// attrs are gone rather than silently ignored: each was a second way to write +// something content or meta already carries. +func TestMCP_CreateRejectsLegacyStructuredFields(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + for _, field := range []string{"title", "lead", "body", "tags", "attrs"} { + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "create", + Arguments: map[string]any{"nodes": []any{map[string]any{ + "key": "node", "content": "# Legacy\n", field: "x", + }}}, + }) + require.NoError(t, err) + require.Truef(t, res.IsError, "create accepted legacy field %q", field) + } +} + func TestMCP_CreateBatchReturnsOrderedStructuredResults(t *testing.T) { session, ctx := newTestSession(t) res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ Name: "create", Arguments: map[string]any{"nodes": []any{ - map[string]any{"key": "first", "body": "# First\n\n[Second](../{{node:second}})\n"}, - map[string]any{"key": "second", "body": "# Second\n\n[First](../{{node:first}})\n"}, + map[string]any{"key": "first", "content": "# First\n\n[Second](../{{node:second}})\n"}, + map[string]any{"key": "second", "content": "# Second\n\n[First](../{{node:first}})\n"}, }}, }) require.NoError(t, err) @@ -787,7 +849,7 @@ func TestMCP_CreateWithBody(t *testing.T) { res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ Name: "create", Arguments: batchCreateArgs(map[string]any{ - "body": body, + "content": body, }), }) require.NoError(t, err) @@ -849,28 +911,32 @@ func TestMCP_Edit(t *testing.T) { require.Contains(t, readText, "Edited via MCP.") } -func TestMCP_MetaRead(t *testing.T) { - t.Parallel() - session, ctx := newTestSession(t) - - // Node 0 has tags: [overview] +// metaOnlyText reads a node's metadata document through cat, which is the only +// metadata read now that the meta tool is gone. +func metaOnlyText(t *testing.T, session *sdkmcp.ClientSession, ctx context.Context, nodeID string) string { + t.Helper() res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ - Name: "meta", - Arguments: batchMetaArgs(map[string]any{ - "node_id": "0", - }), + Name: "cat", + Arguments: map[string]any{"node_ids": []string{nodeID}, "meta_only": true}, }) require.NoError(t, err) text := extractText(t, res) - require.False(t, res.IsError, "meta returned error: %s", text) - require.Contains(t, text, "overview") + require.False(t, res.IsError, "cat meta_only returned error: %s", text) + return text } -func TestMCP_MetaWrite(t *testing.T) { +func TestMCP_MetaReadThroughCat(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + // Node 0 has tags: [overview] + require.Contains(t, metaOnlyText(t, session, ctx, "0"), "overview") +} + +func TestMCP_EditWritesMeta(t *testing.T) { t.Parallel() session, ctx := newTestSession(t) - // Create a node. createRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ Name: "create", Arguments: batchCreateArgs(map[string]any{ @@ -881,29 +947,123 @@ func TestMCP_MetaWrite(t *testing.T) { nodeID := extractText(t, createRes) expectedHash := readNodeHash(t, session, ctx, nodeID) - // Write new metadata. writeRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ - Name: "meta", - Arguments: batchMetaArgs(map[string]any{ + Name: "edit", + Arguments: batchEditArgs(map[string]any{ "node_id": nodeID, - "content": "tags:\n - updated\n - mcp\n", + "meta": "tags:\n - updated\n - mcp\n", "expected_hash": expectedHash, }), }) require.NoError(t, err) - require.False(t, writeRes.IsError, "meta write returned error: %s", extractText(t, writeRes)) + require.False(t, writeRes.IsError, "edit returned error: %s", extractText(t, writeRes)) - // Read back. - readRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ - Name: "meta", - Arguments: batchMetaArgs(map[string]any{ - "node_id": nodeID, + readText := metaOnlyText(t, session, ctx, nodeID) + require.Contains(t, readText, "updated") + require.Contains(t, readText, "mcp") +} + +// TestMCP_EditWritesContentAndMetaTogether covers the case the old two-tool +// split could not express at all: both halves of a node replaced in one call, +// under the single hash that covers them both. +func TestMCP_EditWritesContentAndMetaTogether(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + createRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "create", + Arguments: batchCreateArgs(map[string]any{"title": "Before Both"}), + }) + require.NoError(t, err) + nodeID := extractText(t, createRes) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "edit", + Arguments: batchEditArgs(map[string]any{ + "node_id": nodeID, + "content": "# After Both\n\nRewritten body.\n", + "meta": "tags:\n - both\n", + "expected_hash": readNodeHash(t, session, ctx, nodeID), }), }) require.NoError(t, err) - readText := extractText(t, readRes) - require.Contains(t, readText, "updated") - require.Contains(t, readText, "mcp") + require.False(t, res.IsError, "edit returned error: %s", extractText(t, res)) + + require.Contains(t, metaOnlyText(t, session, ctx, nodeID), "both") + + contentRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "cat", + Arguments: map[string]any{"node_ids": []string{nodeID}, "content_only": true}, + }) + require.NoError(t, err) + require.Contains(t, extractText(t, contentRes), "# After Both") +} + +// TestMCP_EditRejectsFrontmatterInContent pins the footgun this refactor +// removed: content that opens with a frontmatter delimiter used to be parsed +// as metadata silently. +func TestMCP_EditRejectsFrontmatterInContent(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + createRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "create", + Arguments: batchCreateArgs(map[string]any{"title": "Frontmatter Subject"}), + }) + require.NoError(t, err) + nodeID := extractText(t, createRes) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "edit", + Arguments: batchEditArgs(map[string]any{ + "node_id": nodeID, + "content": "---\ntags:\n - sneaky\n---\n\n# Body\n", + "expected_hash": readNodeHash(t, session, ctx, nodeID), + }), + }) + require.NoError(t, err) + require.True(t, res.IsError, "edit accepted frontmatter in content") + require.Contains(t, extractText(t, res), "meta field") + + require.NotContains(t, metaOnlyText(t, session, ctx, nodeID), "sneaky") +} + +// TestMCP_EditRequiresContentOrMeta rejects an item that names a node and a +// hash but asks for no change. +func TestMCP_EditRequiresContentOrMeta(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "edit", + Arguments: batchEditArgs(map[string]any{ + "node_id": "0", + "expected_hash": readNodeHash(t, session, ctx, "0"), + }), + }) + require.NoError(t, err) + require.True(t, res.IsError, "edit accepted an item with neither content nor meta") + require.Contains(t, extractText(t, res), "content or meta is required") +} + +// TestMCP_EditRejectsSnapshotBefore pins the removal of the implicit snapshot +// flag: a stale caller must fail loudly rather than quietly lose its snapshot. +func TestMCP_EditRejectsSnapshotBefore(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ + Name: "edit", + Arguments: batchEditArgs(map[string]any{ + "node_id": "0", + "content": "# Zero\n", + "expected_hash": readNodeHash(t, session, ctx, "0"), + "snapshot_before": true, + }), + }) + require.NoError(t, err) + require.True(t, res.IsError, "edit accepted snapshot_before") + require.Contains(t, extractText(t, res), "snapshot_before") } func TestMCP_Remove(t *testing.T) { @@ -1453,85 +1613,6 @@ func TestMCP_RepoInitMissingAlias(t *testing.T) { require.True(t, res.IsError, "expected error for missing alias") } -// --- import tool tests --- - -func TestMCP_ToolsList_ExcludesArchiveImportAndKeepsKegImport(t *testing.T) { - t.Parallel() - session, ctx := newTestSession(t) - - res, err := session.ListTools(ctx, nil) - require.NoError(t, err) - - names := make([]string, len(res.Tools)) - for i, tool := range res.Tools { - names[i] = tool.Name - } - - require.Contains(t, names, "import_from_keg") - require.NotContains(t, names, "import") -} - -func TestMCP_ImportFromKeg(t *testing.T) { - t.Skip("legacy fixture setup depends on removed repo_init; provider parity covers import_from_keg") - t.Parallel() - session, ctx := newTestSession(t) - - // First, init a second keg to import from. - initRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ - Name: "repo_init", - Arguments: map[string]any{ - "keg": "source", - "user": true, - "title": "Source KEG", - }, - }) - require.NoError(t, err) - require.False(t, initRes.IsError, "repo_init returned error: %s", extractText(t, initRes)) - - // Create a node in the source keg. - createRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ - Name: "create", - Arguments: batchCreateArgs(map[string]any{ - "title": "Imported Node", - "lead": "This node will be imported.", - "keg": "source", - }), - }) - require.NoError(t, err) - require.False(t, createRes.IsError, "create returned error: %s", extractText(t, createRes)) - srcNodeID := extractText(t, createRes) - - // Import from source into personal (default). - importRes, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ - Name: "import_from_keg", - Arguments: map[string]any{ - "source_keg": "source", - "node_ids": []string{srcNodeID}, - "target_keg": "personal", - "skip_zero_node": true, - }, - }) - require.NoError(t, err) - text := extractText(t, importRes) - require.False(t, importRes.IsError, "import_from_keg returned error: %s", text) - require.Contains(t, text, "imported 1 node(s)") -} - -func TestMCP_ImportFromKegSameKegError(t *testing.T) { - t.Parallel() - session, ctx := newTestSession(t) - - res, err := session.CallTool(ctx, &sdkmcp.CallToolParams{ - Name: "import_from_keg", - Arguments: map[string]any{ - "source_keg": "personal", - "target_keg": "personal", - }, - }) - require.NoError(t, err) - require.True(t, res.IsError, "expected error importing from same keg") -} - // --- file transfer tool tests --- func TestMCP_ToolsList_IncludesFileTransferTools(t *testing.T) { @@ -2069,11 +2150,10 @@ func TestMCP_ToolAnnotations_AllPresent(t *testing.T) { // --- write non-destructive tools --- writeTools := []string{ - "create", "edit", "meta", + "create", "edit", "node_snapshot", "upload_file", "upload_image", "lock_acquire", "lock_release", - "import_from_keg", } for _, name := range writeTools { tool, ok := byName[name] diff --git a/pkg/mcp/session_flight.go b/pkg/mcp/session_flight.go index 4c545e2d..c17edccd 100644 --- a/pkg/mcp/session_flight.go +++ b/pkg/mcp/session_flight.go @@ -584,7 +584,12 @@ func (g *sessionFlightGate) middleware(next sdkmcp.MethodHandler) sdkmcp.MethodH } copyTool := *tool if authorityBearingTool(tool.Name) { + // Describe flight where the property is added, so the two + // can never drift. Agents read descriptions, not just + // schemas: an injected property nothing mentions reads as + // absent, and one has already been reported as missing. copyTool.InputSchema = schemaWithFlight(tool.InputSchema) + copyTool.Description = strings.TrimRight(copyTool.Description, " ") + flightParameterNote } copyResult.Tools = append(copyResult.Tools, ©Tool) } @@ -626,6 +631,12 @@ func (g *sessionFlightGate) middleware(next sdkmcp.MethodHandler) sdkmcp.MethodH } } if err != nil { + // A denial on a call that named no flight is reported in terms + // of what the caller actually did, not in terms of a "requested + // flight" they never requested. + if selected == "" && errors.Is(err, ErrOrientationDenied) { + return orientationFailureResultWithAction(err, bareCallDeniedAction), nil + } return orientationFailureResult(err), nil } ctx = context.WithValue(ctx, orientationContextKey{}, callOrientation) @@ -710,6 +721,11 @@ func extractFlightArgument(params *sdkmcp.CallToolParamsRaw) (string, error) { return strings.TrimSpace(selected), nil } +// flightParameterNote documents the flight property injected by +// schemaWithFlight. It is appended to every authority-bearing tool's +// description at the same point the property is added. +const flightParameterNote = " Accepts an optional flight: omit it to use this connection's pinned authority, or name a flight listed by orient to run this call under that flight's cover and capabilities." + func schemaWithFlight(schema any) any { raw, err := json.Marshal(schema) if err != nil { @@ -731,25 +747,53 @@ func schemaWithFlight(schema any) any { return object } +// bareCallDeniedAction answers the denial an agent actually hits most often: +// it passed `keg` but no `flight`, so the connection's pinned root answered and +// refused a KEG only a descendant covers. The default wording talks about "the +// selected flight", which reads as a wrong flight name and sends the agent +// looking through `list_flights` instead of at the selection it never made. +const bareCallDeniedAction = "This call named no flight, so it resolved against this connection's pinned root. " + + "A `keg` argument chooses a target; it never grants authority. Call `orient`: if the KEG appears under " + + "\"Reachable via subflight\", pass that flight as the `flight` argument on this call — reads included, " + + "`cat`, `links`, and `backlinks` among them. Nothing was written." + func orientationFailureResult(err error) *sdkmcp.CallToolResult { + return orientationFailureResultWithAction(err, "") +} + +// orientationFailureResultWithAction renders a failure with caller-supplied +// remediation text. An empty action keeps the wording chosen for the error class. +func orientationFailureResultWithAction(err error, action string) *sdkmcp.CallToolResult { if err == nil { err = ErrOrientationStale } code := "ORIENTATION_STALE" + override := action + action = "Authority changed between resolution and dispatch. Nothing was written. Review current authority with `orient`, then reissue the call yourself — mutations are never replayed automatically." switch { case errors.Is(err, ErrOrientationRootUnavailable): code = "ORIENTATION_ROOT_UNAVAILABLE" + action = "This session's pinned root flight is gone and cannot be replaced from inside MCP. Ask the user to repair or repin the flight outside MCP and start a new connection; `session_refresh` cannot recover this." case errors.Is(err, ErrOrientationUnavailable): code = "ORIENTATION_UNAVAILABLE" + action = "Authority could not be resolved right now; this is transient and nothing was written. Retry the same call shortly." case errors.Is(err, ErrOrientationDenied): code = "ORIENTATION_DENIED" + // Deliberately does not tell the agent to reorient. A denial is not + // disorientation: the next call resolves live authority on its own, and + // saying otherwise sends the agent after a remedy that cannot help. + action = "The selected flight does not permit this operation. Nothing was written, and neither retrying nor logging in again will change it. Pass a flight that covers this keg, or ask the user to widen the flight's cover or capabilities; `list_flights` shows what exists." + } + if override != "" { + action = override } return &sdkmcp.CallToolResult{ - Content: []sdkmcp.Content{&sdkmcp.TextContent{Text: err.Error()}}, + Content: []sdkmcp.Content{&sdkmcp.TextContent{Text: err.Error() + "\n\n" + action}}, StructuredContent: map[string]any{ "code": code, "reorientRequired": false, "operationPerformed": false, + "action": action, }, IsError: true, } diff --git a/pkg/mcp/tools_archive.go b/pkg/mcp/tools_archive.go deleted file mode 100644 index dae373df..00000000 --- a/pkg/mcp/tools_archive.go +++ /dev/null @@ -1,47 +0,0 @@ -package mcp - -import ( - "context" - "fmt" - - sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" - - "github.com/jlrickert/tapper/pkg/tapper" -) - -func registerArchiveTools(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { - registerExport(srv, tap, defaults) -} - -// --- export --- - -type exportInput struct { - Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - OutputPath string `json:"output_path" jsonschema:"filesystem path for the generated tar.gz archive"` - NodeIDs []string `json:"node_ids,omitempty" jsonschema:"node IDs to export (empty exports all nodes)"` - WithHistory bool `json:"with_history,omitempty" jsonschema:"include snapshot history in the archive"` -} - -func registerExport(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { - sdkmcp.AddTool(srv, &sdkmcp.Tool{ - Name: "export", - Description: "Export KEG nodes to a tar.gz archive file", - Annotations: &sdkmcp.ToolAnnotations{ - DestructiveHint: boolPtr(false), - OpenWorldHint: boolPtr(false), - }, - }, func(ctx context.Context, req *sdkmcp.CallToolRequest, in exportInput) (*sdkmcp.CallToolResult, any, error) { - opts := tapper.ExportOptions{ - KegTargetOptions: resolveKegTarget(ctx, in.Keg, defaults), - OutputPath: in.OutputPath, - NodeIDs: in.NodeIDs, - WithHistory: in.WithHistory, - } - - path, err := tap.Export(ctx, opts) - if err != nil { - return errorResult(err), nil, nil - } - return textResult(fmt.Sprintf("exported to %s", path)), nil, nil - }) -} diff --git a/pkg/mcp/tools_import.go b/pkg/mcp/tools_import.go deleted file mode 100644 index 6c89d3e7..00000000 --- a/pkg/mcp/tools_import.go +++ /dev/null @@ -1,60 +0,0 @@ -package mcp - -import ( - "context" - "fmt" - "strings" - - sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" - - "github.com/jlrickert/tapper/pkg/keg" - "github.com/jlrickert/tapper/pkg/tapper" -) - -func registerImportTools(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { - registerImportFromKeg(srv, tap, defaults) -} - -// --- import_from_keg --- - -type importFromKegInput struct { - SourceKeg string `json:"source_keg" jsonschema:"source keg alias to import nodes from"` - NodeIDs []string `json:"node_ids,omitempty" jsonschema:"source node IDs to import (empty imports all non-zero nodes)"` - TargetKeg string `json:"target_keg,omitempty" jsonschema:"target keg alias (uses default if empty)"` - TagQuery string `json:"tag_query,omitempty" jsonschema:"boolean tag expression to select additional source nodes"` - LeaveStubs bool `json:"leave_stubs,omitempty" jsonschema:"write forwarding stubs at source locations after import"` - SkipZeroNode bool `json:"skip_zero_node,omitempty" jsonschema:"skip importing the source keg zero node"` -} - -func registerImportFromKeg(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { - sdkmcp.AddTool(srv, &sdkmcp.Tool{ - Name: "import_from_keg", - Description: "Import nodes from one KEG into another, rewriting links", - Annotations: &sdkmcp.ToolAnnotations{ - DestructiveHint: boolPtr(false), - OpenWorldHint: boolPtr(false), - }, - }, func(ctx context.Context, req *sdkmcp.CallToolRequest, in importFromKegInput) (*sdkmcp.CallToolResult, any, error) { - ctx = keg.WithValidationActor(ctx, keg.ValidationActorAgent) - opts := tapper.ImportFromKegOptions{ - Source: resolveKegTarget(ctx, in.SourceKeg, defaults), - Target: resolveKegTarget(ctx, in.TargetKeg, defaults), - NodeIDs: in.NodeIDs, - TagQuery: in.TagQuery, - LeaveStubs: in.LeaveStubs, - SkipZeroNode: in.SkipZeroNode, - } - - imported, err := tap.ImportFromKeg(ctx, opts) - if err != nil { - return errorResult(err), nil, nil - } - - lines := make([]string, len(imported)) - for i, node := range imported { - lines[i] = fmt.Sprintf("%s -> %s", node.SourceID.Path(), node.TargetID.Path()) - } - summary := fmt.Sprintf("imported %d node(s)\n%s", len(imported), strings.Join(lines, "\n")) - return textResult(summary), nil, nil - }) -} diff --git a/pkg/mcp/tools_read.go b/pkg/mcp/tools_read.go index 8d18d24b..698f3f23 100644 --- a/pkg/mcp/tools_read.go +++ b/pkg/mcp/tools_read.go @@ -33,24 +33,30 @@ type catInput struct { Query string `json:"query,omitempty" jsonschema:"boolean expression to select nodes (alternative to node_ids)"` } -// nodeReadOutput carries the precondition token alongside each node a read -// returns. Writes require the caller to echo the hash it read, so every read -// that can precede a write has to hand it over; leaving it buried in the -// rendered text would force agents to parse output meant for humans. +// nodeReadOutput is one self-contained read result: the node's document +// alongside the precondition token a write must echo back. Splitting those +// across two response surfaces — the hash here, the document only in the +// rendered text — made a read-modify-write cycle require parsing output meant +// for humans, and made multi-node reads correlate rows by position. +// +// Content and Meta are populated to match the read mode and are exactly the +// fields `edit` accepts, so a row can be modified and sent straight back. type nodeReadOutput struct { NodeID string `json:"node_id"` Hash string `json:"hash"` Content string `json:"content,omitempty"` + Meta string `json:"meta,omitempty"` + Stats string `json:"stats,omitempty"` } -func nodeReadOutputs(views []keg.NodeView, withContent bool) []nodeReadOutput { +func nodeReadOutputs(ctx context.Context, views []keg.NodeView, opts tapper.CatOptions) []nodeReadOutput { out := make([]nodeReadOutput, 0, len(views)) for _, view := range views { - row := nodeReadOutput{NodeID: view.ID.Path(), Hash: view.Hash()} - if withContent { - row.Content = string(view.Content) - } - out = append(out, row) + content, meta, stats := tapper.CatViewDocument(ctx, view, opts) + out = append(out, nodeReadOutput{ + NodeID: view.ID.Path(), Hash: view.Hash(), + Content: content, Meta: meta, Stats: stats, + }) } return out } @@ -58,8 +64,10 @@ func nodeReadOutputs(views []keg.NodeView, withContent bool) []nodeReadOutput { func registerCat(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { sdkmcp.AddTool(srv, &sdkmcp.Tool{ Name: "cat", - Description: "Read the content of one or more KEG nodes. Each result carries the " + - "node's hash; pass it back as expected_hash when editing that node.", + Description: "Read one or more KEG nodes. The default returns metadata and content together; " + + "meta_only returns just the metadata document, which is how you read metadata before " + + "editing it. Each result carries the node's hash; pass it back as expected_hash when " + + "editing that node.", Annotations: &sdkmcp.ToolAnnotations{ ReadOnlyHint: true, OpenWorldHint: boolPtr(false), @@ -80,7 +88,7 @@ func registerCat(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { return errorResult(err), nil, nil } res := textResult(tapper.FormatCatViews(ctx, views, opts)) - res.StructuredContent = map[string]any{"nodes": nodeReadOutputs(views, false)} + res.StructuredContent = map[string]any{"nodes": nodeReadOutputs(ctx, views, opts)} return res, nil, nil }) } diff --git a/pkg/mcp/tools_write.go b/pkg/mcp/tools_write.go index e0acdc4d..36da5e61 100644 --- a/pkg/mcp/tools_write.go +++ b/pkg/mcp/tools_write.go @@ -33,7 +33,6 @@ func boundedMutationInputSchema[T any](arrayFields ...string) *jsonschema.Schema func registerWriteTools(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { registerCreate(srv, tap, defaults) registerEdit(srv, tap, defaults) - registerMeta(srv, tap, defaults) registerRemove(srv, tap, defaults) registerMove(srv, tap, defaults) registerKegSettingsEdit(srv, tap, defaults) @@ -78,16 +77,10 @@ type createInput struct { Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` } type createNodeInput struct { - Key string `json:"key"` - Schema string `json:"schema,omitempty" jsonschema:"schema selected for this write; required when strict policy and agent mode both block"` - Title string `json:"title,omitempty"` - Lead string `json:"lead,omitempty"` - Body string `json:"body,omitempty"` - Tags []string `json:"tags,omitempty"` - // map[string]any, not map[string]string: a string-typed map generates an - // `additionalProperties: {type: string}` schema, which cannot express an - // integer at all, so schema fields typed `integer` become unwritable. - Attrs map[string]any `json:"attrs,omitempty"` + Key string `json:"key" jsonschema:"caller-chosen label for this node within the batch, unique across the batch. Other nodes in the same batch link to it by writing {{node:KEY}} in their content; each placeholder is replaced with the id this node is assigned."` + Content string `json:"content" jsonschema:"the node's complete markdown body, opening with an H1 title. Must not start with a YAML frontmatter block: metadata goes in the meta field."` + Meta string `json:"meta,omitempty" jsonschema:"the node's complete metadata document as YAML, including keys such as type, tags, and any schema-defined attributes. Omit for no metadata."` + Schema string `json:"schema,omitempty" jsonschema:"schema selected for this write. Writes meta.type; a different type declared in meta is a hard error, not an override. Required when strict policy and agent mode both block."` } type createNodeOutput struct { @@ -107,8 +100,13 @@ func createNodeOutputs(results []keg.CreateNodeResult) []createNodeOutput { func registerCreate(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { sdkmcp.AddTool(srv, &sdkmcp.Tool{ - Name: "create", - Description: "Atomically create 1-100 KEG nodes with optional intra-batch references. Each optional schema selection is required when strict policy and the resolved agent mode both block.", + Name: "create", + Description: "Atomically create 1-100 KEG nodes. " + + "Each node is a markdown content document plus an optional YAML metadata document. " + + "content must not begin with a YAML frontmatter block — metadata goes in meta. " + + "A node's title is its content H1 and its lead is the first paragraph, so there are no separate title, lead, tags, or attrs fields. " + + "Nodes in one batch can reference each other's not-yet-assigned ids by writing {{node:KEY}} in content. " + + "A schema selection is required only when strict policy and the resolved agent mode both block.", InputSchema: boundedMutationInputSchema[createInput]("nodes"), Annotations: &sdkmcp.ToolAnnotations{ DestructiveHint: boolPtr(false), @@ -118,7 +116,7 @@ func registerCreate(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { ctx = keg.WithValidationActor(ctx, keg.ValidationActorAgent) nodes := make([]tapper.BatchCreateNode, len(in.Nodes)) for i, item := range in.Nodes { - nodes[i] = tapper.BatchCreateNode{Key: item.Key, Schema: item.Schema, Title: item.Title, Lead: item.Lead, Body: item.Body, Tags: item.Tags, Attrs: item.Attrs} + nodes[i] = tapper.BatchCreateNode{Key: item.Key, Schema: item.Schema, Content: item.Content, Meta: item.Meta} } results, err := tap.CreateBatch(ctx, tapper.BatchCreateOptions{KegTargetOptions: resolveKegTarget(ctx, in.Keg, defaults), Nodes: nodes}) if err != nil { @@ -137,15 +135,15 @@ func registerCreate(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { // --- edit --- type editInput struct { - Edits []editItemInput `json:"edits" jsonschema:"1-100 node replacements to apply atomically"` + Nodes []editItemInput `json:"nodes" jsonschema:"1-100 nodes to update atomically"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` } type editItemInput struct { - NodeID string `json:"node_id"` - Schema string `json:"schema,omitempty" jsonschema:"schema selected for this write; required when strict policy and agent mode both block"` - Content string `json:"content"` - ExpectedHash string `json:"expected_hash" jsonschema:"precondition token returned by cat"` - SnapshotBefore bool `json:"snapshot_before,omitempty"` + NodeID string `json:"node_id" jsonschema:"id of the node to update"` + Content *string `json:"content,omitempty" jsonschema:"replacement markdown body, replacing the node's content entirely. Must not start with a YAML frontmatter block: metadata goes in the meta field. Omit to leave content unchanged."` + Meta *string `json:"meta,omitempty" jsonschema:"replacement metadata document as YAML, replacing the node's metadata entirely. Omit to leave metadata unchanged."` + ExpectedHash string `json:"expected_hash" jsonschema:"precondition token returned by cat. One hash covers content and metadata together, so a call that changes only one of them still invalidates the other's hash."` + Schema string `json:"schema,omitempty" jsonschema:"schema selected for this write. Writes meta.type; a different type declared in meta is a hard error, not an override. Required when strict policy and agent mode both block."` } type nodeUpdateOutput struct { @@ -164,18 +162,32 @@ func nodeUpdateOutputs(results []keg.NodeUpdateResult) []nodeUpdateOutput { func registerEdit(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { sdkmcp.AddTool(srv, &sdkmcp.Tool{ - Name: "edit", - Description: "Call cat first for every node, then atomically replace the content of 1-100 nodes using each returned hash as that edit's expected_hash. Each optional schema selection is required when strict policy and the resolved agent mode both block. On conflict, merge into the returned current content (or refetch with cat) and retry with the returned current hash.", - InputSchema: boundedMutationInputSchema[editInput]("edits"), + Name: "edit", + Description: "Atomically replace the content and/or metadata of 1-100 nodes. " + + "Supply content, meta, or both for each node; at least one is required. " + + "content is the markdown body and must not begin with a YAML frontmatter block — metadata goes in meta. " + + "One expected_hash covers a node's content and metadata together, so changing either invalidates the hash for both. " + + "Call cat first for every node and pass each returned hash as that node's expected_hash; cat meta_only reads metadata. " + + "Take a snapshot with node_snapshot before a large or destructive edit. " + + "A schema selection is required only when strict policy and the resolved agent mode both block. " + + "On conflict, merge into the returned current content (or refetch with cat) and retry with the returned current hash.", + InputSchema: boundedMutationInputSchema[editInput]("nodes"), Annotations: &sdkmcp.ToolAnnotations{ DestructiveHint: boolPtr(false), OpenWorldHint: boolPtr(false), }, }, func(ctx context.Context, req *sdkmcp.CallToolRequest, in editInput) (*sdkmcp.CallToolResult, any, error) { ctx = keg.WithValidationActor(ctx, keg.ValidationActorAgent) - edits := make([]tapper.BatchEditItem, len(in.Edits)) - for i, item := range in.Edits { - edits[i] = tapper.BatchEditItem{NodeID: item.NodeID, Schema: item.Schema, Content: item.Content, ExpectedHash: item.ExpectedHash, SnapshotBefore: item.SnapshotBefore} + edits := make([]tapper.BatchEditItem, len(in.Nodes)) + for i, item := range in.Nodes { + edit := tapper.BatchEditItem{NodeID: item.NodeID, Schema: item.Schema, ExpectedHash: item.ExpectedHash} + if item.Content != nil { + edit.Content, edit.HasContent = *item.Content, true + } + if item.Meta != nil { + edit.Meta, edit.HasMeta = *item.Meta, true + } + edits[i] = edit } results, err := tap.EditBatch(ctx, tapper.BatchEditOptions{KegTargetOptions: resolveKegTarget(ctx, in.Keg, defaults), Edits: edits}) if err != nil { @@ -187,55 +199,6 @@ func registerEdit(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { }) } -// --- meta --- - -type metaInput struct { - NodeIDs []string `json:"node_ids,omitempty" jsonschema:"1-100 node IDs to read"` - Updates []metaUpdateInput `json:"updates,omitempty" jsonschema:"1-100 metadata replacements to apply atomically"` - Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` -} -type metaUpdateInput struct { - NodeID string `json:"node_id"` - Schema string `json:"schema,omitempty" jsonschema:"schema selected for this write; required when strict policy and agent mode both block"` - Content string `json:"content"` - ExpectedHash string `json:"expected_hash" jsonschema:"precondition token returned by cat"` - SnapshotBefore bool `json:"snapshot_before,omitempty"` -} - -func registerMeta(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { - sdkmcp.AddTool(srv, &sdkmcp.Tool{ - Name: "meta", - Description: "Read metadata for 1-100 nodes without a token, or call cat first and atomically replace metadata for 1-100 nodes using each returned hash as that update's expected_hash. Each optional schema selection on an update is required when strict policy and the resolved agent mode both block. On conflict, merge into the returned current metadata (or refetch with cat) and retry with the returned current hash.", - InputSchema: boundedMutationInputSchema[metaInput]("node_ids", "updates"), - Annotations: &sdkmcp.ToolAnnotations{ - DestructiveHint: boolPtr(false), - OpenWorldHint: boolPtr(false), - }, - }, func(ctx context.Context, req *sdkmcp.CallToolRequest, in metaInput) (*sdkmcp.CallToolResult, any, error) { - ctx = keg.WithValidationActor(ctx, keg.ValidationActorAgent) - updates := make([]tapper.BatchMetaUpdate, len(in.Updates)) - for i, item := range in.Updates { - updates[i] = tapper.BatchMetaUpdate{NodeID: item.NodeID, Schema: item.Schema, Content: item.Content, ExpectedHash: item.ExpectedHash, SnapshotBefore: item.SnapshotBefore} - } - reads, writes, err := tap.MetaBatch(ctx, tapper.BatchMetaOptions{KegTargetOptions: resolveKegTarget(ctx, in.Keg, defaults), NodeIDs: in.NodeIDs, Updates: updates}) - if err != nil { - return errorResult(err), nil, nil - } - if len(writes) > 0 { - res := textResult(fmt.Sprintf("updated metadata for %d node(s)", len(writes))) - res.StructuredContent = map[string]any{"results": nodeUpdateOutputs(writes)} - return res, nil, nil - } - message := fmt.Sprintf("read metadata for %d node(s)", len(reads)) - if len(reads) == 1 { - message = reads[0].Content - } - res := textResult(message) - res.StructuredContent = map[string]any{"results": reads} - return res, nil, nil - }) -} - // --- remove --- type removeInput struct { diff --git a/pkg/parity/parity_coverage_test.go b/pkg/parity/parity_coverage_test.go index 1d2f39e0..4a3c09c3 100644 --- a/pkg/parity/parity_coverage_test.go +++ b/pkg/parity/parity_coverage_test.go @@ -18,86 +18,83 @@ import ( // (e.g., internal helpers, config-only methods, or methods that don't have // a direct consumer surface). var tapMethodToSurfaces = map[string]struct { - CLI string // CLI command path (e.g., "list", "repo init", "index rebuild") - MCP string // MCP tool name (e.g., "list", "repo_init", "index") + CLI string // CLI command path (e.g., "list", "repo init", "index rebuild") + MCP []string // MCP tool names covering it (e.g., "list", "repo_init", "index") }{ // Read operations - "Cat": {CLI: "cat", MCP: "cat"}, - "List": {CLI: "list", MCP: "list"}, - "Grep": {CLI: "grep", MCP: "grep"}, - "Tags": {CLI: "tags", MCP: "tags"}, - "Backlinks": {CLI: "backlinks", MCP: "backlinks"}, - "Links": {CLI: "links", MCP: "links"}, - "Info": {CLI: "info", MCP: "info"}, - "KegSettings": {CLI: "keg settings", MCP: "keg_settings"}, - "KegSettingsEdit": {CLI: "keg settings edit", MCP: "keg_settings_edit"}, - "Stats": {CLI: "stats", MCP: "stats"}, - "ListIndexes": {CLI: "index list", MCP: "list_indexes"}, - "IndexCat": {CLI: "index get", MCP: "index_cat"}, - "Doctor": {CLI: "doctor", MCP: "doctor"}, + "Cat": {CLI: "cat", MCP: []string{"cat"}}, + "List": {CLI: "list", MCP: []string{"list"}}, + "Grep": {CLI: "grep", MCP: []string{"grep"}}, + "Tags": {CLI: "tags", MCP: []string{"tags"}}, + "Backlinks": {CLI: "backlinks", MCP: []string{"backlinks"}}, + "Links": {CLI: "links", MCP: []string{"links"}}, + "Info": {CLI: "info", MCP: []string{"info"}}, + "KegSettings": {CLI: "keg settings", MCP: []string{"keg_settings"}}, + "KegSettingsEdit": {CLI: "keg settings edit", MCP: []string{"keg_settings_edit"}}, + "Stats": {CLI: "stats", MCP: []string{"stats"}}, + "ListIndexes": {CLI: "index list", MCP: []string{"list_indexes"}}, + "IndexCat": {CLI: "index get", MCP: []string{"index_cat"}}, + "Doctor": {CLI: "doctor", MCP: []string{"doctor"}}, // Write operations - "Create": {CLI: "create", MCP: "create"}, - "Edit": {CLI: "edit", MCP: "edit"}, - "Meta": {CLI: "meta", MCP: "meta"}, - "Remove": {CLI: "rm", MCP: "remove"}, - "Move": {CLI: "mv", MCP: "move"}, + "Create": {CLI: "create", MCP: []string{"create"}}, + "Edit": {CLI: "edit", MCP: []string{"edit"}}, + // tap meta reads and writes; on MCP those halves live in different tools — + // cat meta_only reads, edit writes — so the metadata capability is present + // on both surfaces without a tool of its own. + "Meta": {CLI: "meta", MCP: []string{"cat", "edit"}}, + "Remove": {CLI: "rm", MCP: []string{"remove"}}, + "Move": {CLI: "mv", MCP: []string{"move"}}, // Index operations - "Index": {CLI: "index rebuild", MCP: "index"}, + "Index": {CLI: "index rebuild", MCP: []string{"index"}}, // Schema operations (type-based keg schemas). Full CRUD + validation is // exposed on both MCP surfaces; schema mutation resolves at editor role, // consistent with node writes. - "ListSchemas": {CLI: "schema list", MCP: "schema_list"}, - "ReadSchema": {CLI: "schema get", MCP: "schema_read"}, - "CreateSchema": {CLI: "schema create", MCP: "schema_create"}, - "EditSchema": {CLI: "schema edit", MCP: "schema_edit"}, - "DeleteSchema": {CLI: "schema rm", MCP: "schema_delete"}, - "Validate": {CLI: "validate", MCP: "validate"}, + "ListSchemas": {CLI: "schema list", MCP: []string{"schema_list"}}, + "ReadSchema": {CLI: "schema get", MCP: []string{"schema_read"}}, + "CreateSchema": {CLI: "schema create", MCP: []string{"schema_create"}}, + "EditSchema": {CLI: "schema edit", MCP: []string{"schema_edit"}}, + "DeleteSchema": {CLI: "schema rm", MCP: []string{"schema_delete"}}, + "Validate": {CLI: "validate", MCP: []string{"validate"}}, // Snapshot operations - "NodeSnapshot": {CLI: "snapshot create", MCP: "node_snapshot"}, - "NodeHistory": {CLI: "snapshot history", MCP: "node_history"}, - "NodeSnapshotView": {CLI: "snapshot view", MCP: "node_snapshot_view"}, - "NodeRestore": {CLI: "snapshot restore", MCP: "node_restore"}, + "NodeSnapshot": {CLI: "snapshot create", MCP: []string{"node_snapshot"}}, + "NodeHistory": {CLI: "snapshot history", MCP: []string{"node_history"}}, + "NodeSnapshotView": {CLI: "snapshot view", MCP: []string{"node_snapshot_view"}}, + "NodeRestore": {CLI: "snapshot restore", MCP: []string{"node_restore"}}, // File operations - "ListFiles": {CLI: "file ls", MCP: "list_files"}, - "ListImages": {CLI: "image ls", MCP: "list_images"}, - "DeleteFile": {CLI: "file rm", MCP: "delete_file"}, - "DeleteImage": {CLI: "image rm", MCP: "delete_image"}, - "UploadFile": {CLI: "file upload", MCP: "upload_file"}, - "DownloadFile": {CLI: "file download", MCP: "download_file"}, - "UploadImage": {CLI: "image upload", MCP: "upload_image"}, - "DownloadImage": {CLI: "image download", MCP: "download_image"}, + "ListFiles": {CLI: "file ls", MCP: []string{"list_files"}}, + "ListImages": {CLI: "image ls", MCP: []string{"list_images"}}, + "DeleteFile": {CLI: "file rm", MCP: []string{"delete_file"}}, + "DeleteImage": {CLI: "image rm", MCP: []string{"delete_image"}}, + "UploadFile": {CLI: "file upload", MCP: []string{"upload_file"}}, + "DownloadFile": {CLI: "file download", MCP: []string{"download_file"}}, + "UploadImage": {CLI: "image upload", MCP: []string{"upload_image"}}, + "DownloadImage": {CLI: "image download", MCP: []string{"download_image"}}, // Lock operations - "Lock": {CLI: "lock acquire", MCP: "lock_acquire"}, - "Unlock": {CLI: "lock release", MCP: "lock_release"}, - "LockStatus": {CLI: "lock status", MCP: "lock_status"}, - "ForceUnlock": {CLI: "lock force-release", MCP: "lock_force_release"}, - - // Repo management - // Note: "import" at top level is ImportFromKeg (live keg import). - // "archive import" is Import (archive import). Different commands, - // different Tap methods, same word. Only the live-keg one has an MCP peer. - "ImportFromKeg": {CLI: "import", MCP: "import_from_keg"}, + "Lock": {CLI: "lock acquire", MCP: []string{"lock_acquire"}}, + "Unlock": {CLI: "lock release", MCP: []string{"lock_release"}}, + "LockStatus": {CLI: "lock status", MCP: []string{"lock_status"}}, + "ForceUnlock": {CLI: "lock force-release", MCP: []string{"lock_force_release"}}, // Flights (keg restriction + agent instructions) - "ListFlights": {CLI: "flight list", MCP: "list_flights"}, - "GetFlight": {CLI: "flight show", MCP: "flight_show"}, - "CreateFlight": {CLI: "flight create", MCP: "flight_create"}, - "EditFlight": {CLI: "flight edit", MCP: "flight_edit"}, - "DeleteFlight": {CLI: "flight delete", MCP: "flight_delete"}, + "ListFlights": {CLI: "flight list", MCP: []string{"list_flights"}}, + "GetFlight": {CLI: "flight show", MCP: []string{"flight_show"}}, + "CreateFlight": {CLI: "flight create", MCP: []string{"flight_create"}}, + "EditFlight": {CLI: "flight edit", MCP: []string{"flight_edit"}}, + "DeleteFlight": {CLI: "flight delete", MCP: []string{"flight_delete"}}, // Keg discovery (hub-side). HubListKegs backs `tap keg list`; on MCP the // same listing is filtered through the session's active flight cover. - "HubListKegs": {CLI: "keg list", MCP: "keg_list"}, + "HubListKegs": {CLI: "keg list", MCP: []string{"keg_list"}}, // Agent orientation remains shared. Native plugin installation is an // intentionally CLI-only host operation (see tapMethodsExcluded). - "Orient": {CLI: "orient", MCP: "orient"}, + "Orient": {CLI: "orient", MCP: []string{"orient"}}, } // tapMethodsExcluded lists Tap methods that are intentionally excluded from @@ -106,7 +103,6 @@ var tapMethodToSurfaces = map[string]struct { var tapMethodsExcluded = map[string]string{ "CreateBatch": "MCP batch backing operation; CLI create remains a one-node command", "EditBatch": "MCP batch backing operation; CLI edit remains a one-node command", - "MetaBatch": "MCP batch backing operation; CLI meta remains a one-node command", "NodeSnapshotBatch": "MCP batch backing operation; CLI snapshot create remains a one-node command", "CatViews": "structured accessor behind Cat; MCP cat uses it to return per-node precondition hashes without re-reading", "NodeHash": "explicit CLI read-before-write helper; MCP reads return the same token in structured content", @@ -161,7 +157,7 @@ var tapMethodsExcluded = map[string]string{ "ConfigTemplate": "emits starter config files for a human to edit; CLI-only setup step", "InitKeg": "provisions a keg destination on local disk or a hub; CLI-only setup step", "Export": "writes a keg archive to the local filesystem; CLI-only bulk operation", - "Import": "reads a keg archive from the local filesystem; CLI-only bulk operation (import_from_keg covers the agent-safe node-level path)", + "Import": "reads a keg archive from the local filesystem; CLI-only bulk operation", "KegVisibility": "UI-only visibility management; MCP must not flip a keg between public and private", "NamespaceList": "namespace discovery folded into auth_info's identity payload; the standalone tool was tenant-administration shaped", "License": "prints bundled license text; CLI-only via `tap version --license`", @@ -202,9 +198,12 @@ func TestCoverage_AllTapMethodsHaveBothSurfaces(t *testing.T) { // Check each mapped method has both surfaces. for method, surfaces := range tapMethodToSurfaces { t.Run("surface/"+method, func(t *testing.T) { - // Check MCP tool exists. - require.Contains(t, mcpTools, surfaces.MCP, - "Tap.%s is mapped to MCP tool %q but tool is not registered", method, surfaces.MCP) + // Check every mapped MCP tool exists. + require.NotEmpty(t, surfaces.MCP, "Tap.%s maps to no MCP tool", method) + for _, tool := range surfaces.MCP { + require.Contains(t, mcpTools, tool, + "Tap.%s is mapped to MCP tool %q but tool is not registered", method, tool) + } // Check CLI command exists (just verify it's in the known set). require.Contains(t, cliCommands, surfaces.CLI, diff --git a/pkg/parity/parity_test.go b/pkg/parity/parity_test.go index 524dd10b..6efe31d1 100644 --- a/pkg/parity/parity_test.go +++ b/pkg/parity/parity_test.go @@ -194,7 +194,23 @@ func newParityKeg(t *testing.T, ctx context.Context, rt *toolkit.Runtime) keg.Ke } // runCLI executes a CLI command and returns stdout as a string. +// runCLICreate creates a node through the CLI by piping content on stdin, +// which is the only way to give a new node a title now that --title/--lead/ +// --tags/--attrs are gone. meta, when non-empty, rides as YAML frontmatter. +func (e *parityEnv) runCLICreate(title, meta string, args ...string) (string, error) { + e.t.Helper() + content := "# " + title + "\n" + if meta != "" { + content = "---\n" + meta + "---\n" + content + } + return e.runCLIWithStdin(content, append([]string{"create"}, args...)...) +} + func (e *parityEnv) runCLI(args ...string) (string, error) { + return e.runCLIWithStdin("", args...) +} + +func (e *parityEnv) runCLIWithStdin(stdin string, args ...string) (string, error) { e.t.Helper() proc := sandbox.NewProcess(func(ctx context.Context, rt *toolkit.Runtime) (int, error) { ctx = cli.WithTestDepsHook(ctx, func(deps *cli.Deps) { @@ -202,6 +218,9 @@ func (e *parityEnv) runCLI(args ...string) (string, error) { }) return cli.Run(ctx, rt, args) }, false) // isTTY=false to get stdout output, not editor + if stdin != "" { + proc.SetStdin(strings.NewReader(stdin)) + } result := proc.Run(e.ctx, e.sb.Runtime()) if result.Err != nil { return strings.TrimSpace(string(result.Stdout)), result.Err diff --git a/pkg/parity/parity_write_test.go b/pkg/parity/parity_write_test.go index 33360f4c..8a98d0af 100644 --- a/pkg/parity/parity_write_test.go +++ b/pkg/parity/parity_write_test.go @@ -20,7 +20,7 @@ func TestParity_WriteOperations(t *testing.T) { env := newParityEnv(t) // Create via CLI. - cliOut, err := env.runCLI("create", "--title", "CLI Node", "--tags", "parity-test") + cliOut, err := env.runCLICreate("CLI Node", "tags:\n - parity-test\n") require.NoError(t, err, "CLI create should succeed") cliNodeID := strings.TrimSpace(cliOut) require.NotEmpty(t, cliNodeID, "CLI should return a node ID") @@ -28,9 +28,9 @@ func TestParity_WriteOperations(t *testing.T) { // Create via MCP. mcpOut, err := env.runMCP("create", map[string]any{ "nodes": []any{map[string]any{ - "key": "node", - "title": "MCP Node", - "tags": []string{"parity-test"}, + "key": "node", + "content": "# MCP Node\n", + "meta": "tags:\n - parity-test\n", }}, }) require.NoError(t, err, "MCP create should succeed") @@ -82,12 +82,12 @@ func TestParity_WriteOperations(t *testing.T) { env := newParityEnv(t) // Create one node per surface. - cliOut, err := env.runCLI("create", "--title", "Listed CLI") + cliOut, err := env.runCLICreate("Listed CLI", "") require.NoError(t, err) cliID := strings.TrimSpace(cliOut) mcpOut, err := env.runMCP("create", map[string]any{ - "nodes": []any{map[string]any{"key": "node", "title": "Listed MCP"}}, + "nodes": []any{map[string]any{"key": "node", "content": "# Listed MCP\n"}}, }) require.NoError(t, err) mcpID := strings.TrimSpace(mcpOut) @@ -113,7 +113,7 @@ func TestParity_WriteOperations(t *testing.T) { // Create a node. out, err := env.runMCP("create", map[string]any{ - "nodes": []any{map[string]any{"key": "node", "title": "To Remove"}}, + "nodes": []any{map[string]any{"key": "node", "content": "# To Remove\n"}}, }) require.NoError(t, err) nodeID := strings.TrimSpace(out) @@ -140,7 +140,7 @@ func TestParity_WriteOperations(t *testing.T) { env := newParityEnv(t) // Create a node via CLI. - out, err := env.runCLI("create", "--title", "MCP Will Remove") + out, err := env.runCLICreate("MCP Will Remove", "") require.NoError(t, err) nodeID := strings.TrimSpace(out) @@ -175,7 +175,7 @@ func TestParity_WriteOperations(t *testing.T) { // Create a node. out, err := env.runMCP("create", map[string]any{ - "nodes": []any{map[string]any{"key": "node", "title": "Movable CLI"}}, + "nodes": []any{map[string]any{"key": "node", "content": "# Movable CLI\n"}}, }) require.NoError(t, err) srcID := strings.TrimSpace(out) @@ -211,7 +211,7 @@ func TestParity_WriteOperations(t *testing.T) { env := newParityEnv(t) // Create a node via CLI. - out, err := env.runCLI("create", "--title", "Movable MCP") + out, err := env.runCLICreate("Movable MCP", "") require.NoError(t, err) srcID := strings.TrimSpace(out) @@ -256,14 +256,14 @@ func TestParity_WriteOperations(t *testing.T) { // Create a node. out, err := env.runMCP("create", map[string]any{ - "nodes": []any{map[string]any{"key": "node", "title": "Before Edit"}}, + "nodes": []any{map[string]any{"key": "node", "content": "# Before Edit\n"}}, }) require.NoError(t, err) nodeID := strings.TrimSpace(out) // Edit via MCP. _, err = env.runMCP("edit", map[string]any{ - "edits": []any{map[string]any{ + "nodes": []any{map[string]any{ "node_id": nodeID, "content": "# After MCP Edit\n\nEdited content.\n", "expected_hash": env.nodeHash(nodeID), @@ -289,26 +289,26 @@ func TestParity_WriteOperations(t *testing.T) { require.Contains(t, mcpList, "After MCP Edit", "MCP list should show updated title") }) - t.Run("meta/mcp_meta_write_reflected_in_cli", func(t *testing.T) { + t.Run("meta/mcp_edit_meta_reflected_in_cli", func(t *testing.T) { t.Parallel() env := newParityEnv(t) // Create a node. out, err := env.runMCP("create", map[string]any{ - "nodes": []any{map[string]any{"key": "node", "title": "Meta Test"}}, + "nodes": []any{map[string]any{"key": "node", "content": "# Meta Test\n"}}, }) require.NoError(t, err) nodeID := strings.TrimSpace(out) // Write metadata via MCP. - _, err = env.runMCP("meta", map[string]any{ - "updates": []any{map[string]any{ + _, err = env.runMCP("edit", map[string]any{ + "nodes": []any{map[string]any{ "node_id": nodeID, - "content": "tags:\n - updated-meta\n - parity\n", + "meta": "tags:\n - updated-meta\n - parity\n", "expected_hash": env.nodeHash(nodeID), }}, }) - require.NoError(t, err, "MCP meta write should succeed") + require.NoError(t, err, "MCP metadata write should succeed") // CLI should see the updated metadata. cliMeta, err := env.runCLI("cat", nodeID, "--meta-only") @@ -339,40 +339,40 @@ func TestParity_WriteOperations(t *testing.T) { env := newParityEnv(t) out, err := env.runMCP("create", map[string]any{ - "nodes": []any{map[string]any{"key": "node", "title": "Id Strip Test"}}, + "nodes": []any{map[string]any{"key": "node", "content": "# Id Strip Test\n"}}, }) require.NoError(t, err) nodeID := strings.TrimSpace(out) - _, err = env.runMCP("meta", map[string]any{ - "updates": []any{map[string]any{ + _, err = env.runMCP("edit", map[string]any{ + "nodes": []any{map[string]any{ "node_id": nodeID, - "content": "id: \"" + nodeID + "\"\ntags:\n - round-trip\n", + "meta": "id: \"" + nodeID + "\"\ntags:\n - round-trip\n", "expected_hash": env.nodeHash(nodeID), }}, }) - require.NoError(t, err, "MCP meta write should succeed") + require.NoError(t, err, "MCP metadata write should succeed") first, err := env.runCLI("cat", nodeID, "--meta-only") require.NoError(t, err) require.NotContains(t, first, "id:", "id field must not be persisted to meta.yaml") require.Contains(t, first, "round-trip") - _, err = env.runMCP("meta", map[string]any{ - "updates": []any{map[string]any{ + _, err = env.runMCP("edit", map[string]any{ + "nodes": []any{map[string]any{ "node_id": nodeID, - "content": first, + "meta": first, "expected_hash": env.nodeHash(nodeID), }}, }) - require.NoError(t, err, "second MCP meta write should succeed") + require.NoError(t, err, "second MCP metadata write should succeed") second, err := env.runCLI("cat", nodeID, "--meta-only") require.NoError(t, err) require.Equal(t, first, second, "round-trip cat → meta write → cat must be byte-identical") }) - t.Run("meta/cli_and_mcp_read_same_metadata", func(t *testing.T) { + t.Run("meta/cli_meta_and_mcp_cat_read_same_metadata", func(t *testing.T) { t.Parallel() env := newParityEnv(t) @@ -380,8 +380,9 @@ func TestParity_WriteOperations(t *testing.T) { cliMeta, err := env.runCLI("meta", "0") require.NoError(t, err) - mcpMeta, err := env.runMCP("meta", map[string]any{ - "node_ids": []string{"0"}, + mcpMeta, err := env.runMCP("cat", map[string]any{ + "node_ids": []string{"0"}, + "meta_only": true, }) require.NoError(t, err) diff --git a/pkg/tapper/tap_batch.go b/pkg/tapper/tap_batch.go index a1b80228..f8cfe9c1 100644 --- a/pkg/tapper/tap_batch.go +++ b/pkg/tapper/tap_batch.go @@ -5,17 +5,13 @@ import ( "fmt" "github.com/jlrickert/tapper/pkg/keg" - "gopkg.in/yaml.v3" ) type BatchCreateNode struct { - Key string - Schema string - Title string - Lead string - Body string - Tags []string - Attrs map[string]any + Key string + Schema string + Content string + Meta string } type BatchCreateOptions struct { KegTargetOptions @@ -30,17 +26,24 @@ func (t *Tap) CreateBatch(ctx context.Context, opts BatchCreateOptions) ([]keg.C ctx = keg.WithDefaultValidationActor(ctx, keg.ValidationActorHuman) nodes := make([]keg.NodeCreate, len(opts.Nodes)) for i, item := range opts.Nodes { - nodes[i] = keg.NodeCreate{Key: item.Key, Schema: item.Schema, Title: item.Title, Lead: item.Lead, Body: []byte(item.Body), Tags: item.Tags, Attrs: item.Attrs} + // keg.RejectFrontmatter covers this too; naming the batch item here + // tells the caller which one to fix. + if err := keg.RejectFrontmatter([]byte(item.Content)); err != nil { + return nil, fmt.Errorf("create %d (key %q): %w", i, item.Key, err) + } + nodes[i] = keg.NodeCreate{Key: item.Key, Schema: item.Schema, Body: []byte(item.Content), Meta: []byte(item.Meta)} } return k.CreateNodes(ctx, nodes) } type BatchEditItem struct { - NodeID string - Schema string - Content string - ExpectedHash string - SnapshotBefore bool + NodeID string + Schema string + Content string + HasContent bool + Meta string + HasMeta bool + ExpectedHash string } type BatchEditOptions struct { KegTargetOptions @@ -59,103 +62,26 @@ func (t *Tap) EditBatch(ctx context.Context, opts BatchEditOptions) ([]keg.NodeU if err != nil { return nil, fmt.Errorf("edit %d node %q: %w", i, item.NodeID, err) } - hasMeta, rawMeta, body, err := splitEditNodeFile([]byte(item.Content)) - if err != nil { - return nil, fmt.Errorf("edit %d node %q: %w", i, item.NodeID, err) + if !item.HasContent && !item.HasMeta { + return nil, fmt.Errorf("edit %d node %q: content or meta is required: %w", i, item.NodeID, keg.ErrInvalid) } - updates[i] = keg.NodeUpdateOptions{ID: *id, Schema: item.Schema, Content: body, HasContent: true, ExpectedHash: item.ExpectedHash, SnapshotBefore: item.SnapshotBefore} - if hasMeta { - meta, err := keg.ParseMeta(ctx, rawMeta) - if err != nil { - return nil, fmt.Errorf("edit %d node %q metadata: %w", i, item.NodeID, err) + update := keg.NodeUpdateOptions{ID: *id, Schema: item.Schema, ExpectedHash: item.ExpectedHash} + if item.HasContent { + if err := keg.RejectFrontmatter([]byte(item.Content)); err != nil { + return nil, fmt.Errorf("edit %d node %q: %w", i, item.NodeID, err) } - updates[i].Meta, updates[i].HasMeta = []byte(meta.ToYAML()), true + update.Content, update.HasContent = []byte(item.Content), true } - } - return k.UpdateNodes(ctx, updates) -} - -type BatchMetaUpdate struct { - NodeID string - Schema string - Content string - ExpectedHash string - SnapshotBefore bool -} -type BatchMetaOptions struct { - KegTargetOptions - NodeIDs []string - Updates []BatchMetaUpdate -} -type BatchMetaResult struct { - NodeID string `json:"node_id"` - // Hash is the node's precondition token, echoed back as ExpectedHash on - // the next write to this node. It covers content and metadata together. - Hash string `json:"hash"` - Content string `json:"content"` -} - -func (t *Tap) MetaBatch(ctx context.Context, opts BatchMetaOptions) ([]BatchMetaResult, []keg.NodeUpdateResult, error) { - role := FlightRoleViewer - if len(opts.Updates) > 0 { - role = FlightRoleEditor - } - k, err := t.resolveKegForRole(ctx, opts.KegTargetOptions, role) - if err != nil { - return nil, nil, err - } - if len(opts.NodeIDs) > 0 && len(opts.Updates) > 0 { - return nil, nil, fmt.Errorf("node_ids and updates are mutually exclusive: %w", keg.ErrInvalid) - } - if len(opts.Updates) > 0 { - ctx = keg.WithDefaultValidationActor(ctx, keg.ValidationActorHuman) - updates := make([]keg.NodeUpdateOptions, len(opts.Updates)) - for i, item := range opts.Updates { - id, err := keg.ParseNode(item.NodeID) - if err != nil { - return nil, nil, fmt.Errorf("metadata update %d node %q: %w", i, item.NodeID, err) - } - var raw map[string]any - if err := yaml.Unmarshal([]byte(item.Content), &raw); err != nil { - return nil, nil, fmt.Errorf("metadata update %d node %q: %w", i, item.NodeID, err) - } - meta, err := keg.ParseMeta(ctx, []byte(item.Content)) + if item.HasMeta { + meta, err := keg.ParseMeta(ctx, []byte(item.Meta)) if err != nil { - return nil, nil, fmt.Errorf("metadata update %d node %q: %w", i, item.NodeID, err) + return nil, fmt.Errorf("edit %d node %q metadata: %w", i, item.NodeID, err) } - updates[i] = keg.NodeUpdateOptions{ID: *id, Schema: item.Schema, Meta: []byte(meta.ToYAML()), HasMeta: true, ExpectedHash: item.ExpectedHash, SnapshotBefore: item.SnapshotBefore} - } - results, err := k.UpdateNodes(ctx, updates) - return nil, results, err - } - if len(opts.NodeIDs) == 0 { - return nil, nil, fmt.Errorf("node_ids must contain at least one item: %w", keg.ErrInvalid) - } - if len(opts.NodeIDs) > keg.MaxMutationBatchSize { - return nil, nil, fmt.Errorf("node_ids exceeds maximum %d: %w", keg.MaxMutationBatchSize, keg.ErrInvalid) - } - ids := make([]keg.NodeId, len(opts.NodeIDs)) - seen := map[keg.NodeId]int{} - for i, raw := range opts.NodeIDs { - id, err := keg.ParseNode(raw) - if err != nil { - return nil, nil, fmt.Errorf("metadata read %d node %q: %w", i, raw, err) - } - ids[i] = *id - if first, ok := seen[*id]; ok { - return nil, nil, fmt.Errorf("metadata read %d node %q duplicates index %d: %w", i, raw, first, keg.ErrInvalid) + update.Meta, update.HasMeta = []byte(meta.ToYAML()), true } - seen[*id] = i + updates[i] = update } - views, err := k.ReadNodes(ctx, keg.ReadNodesOptions{NodeIDs: ids}) - if err != nil { - return nil, nil, err - } - out := make([]BatchMetaResult, len(views)) - for i, view := range views { - out[i] = BatchMetaResult{NodeID: view.ID.Path(), Hash: view.Hash(), Content: string(view.Meta)} - } - return out, nil, nil + return k.UpdateNodes(ctx, updates) } type BatchSnapshotItem struct { diff --git a/pkg/tapper/tap_cat.go b/pkg/tapper/tap_cat.go index 7cb7451a..05824b70 100644 --- a/pkg/tapper/tap_cat.go +++ b/pkg/tapper/tap_cat.go @@ -270,6 +270,27 @@ func (t *Tap) catSingleNode(ctx context.Context, k keg.Keg, nodeID string, opts return formatFrontmatter(ctx, meta, content), nil } +// CatViewDocument returns the machine-readable parts of one node view under +// opts, for callers handing structured data to a client instead of rendering it +// for a human. +// +// The fields deliberately mirror the write surface: content and meta are +// exactly what `edit` accepts, so a read result can be modified and sent back +// without parsing the rendered output. That is why this does not return the +// composed `---meta---body` document — `edit` rejects frontmatter inside +// content, so a composed blob could not be round-tripped. +func CatViewDocument(ctx context.Context, view keg.NodeView, opts CatOptions) (content, meta, stats string) { + switch { + case opts.ContentOnly: + return string(view.Content), "", "" + case opts.MetaOnly: + return "", normalizeMetaYAML(ctx, view.Meta), "" + case opts.StatsOnly: + return "", "", formatStatsOnlyYAML(ctx, view.Stats) + } + return string(view.Content), normalizeMetaYAML(ctx, view.Meta), "" +} + // formatFrontmatter renders meta as canonical YAML frontmatter ahead of the // content body. Raw repository bytes may be JSON (hub kegs store meta as // JSONB), so the meta always passes through normalizeMetaYAML. diff --git a/pkg/tapper/tap_create.go b/pkg/tapper/tap_create.go index bd1b37ec..0d1f63f5 100644 --- a/pkg/tapper/tap_create.go +++ b/pkg/tapper/tap_create.go @@ -4,21 +4,15 @@ import ( "context" "fmt" "io" - "strings" "github.com/jlrickert/cli-toolkit/toolkit" "github.com/jlrickert/tapper/pkg/keg" - "gopkg.in/yaml.v3" ) type CreateOptions struct { KegTargetOptions Schema string - Title string - Lead string - Tags []string - Attrs map[string]string Stream *toolkit.Stream } @@ -44,32 +38,30 @@ func (t *Tap) Create(ctx context.Context, opts CreateOptions) (keg.NodeId, error // on the already-persisted node. This avoids the double-allocation // bug where Next() was called for the editor scaffold and then // Create() called Next() again internally. - attrs := createAttrsFromStrings(opts.Attrs) - created, createErr := k.Create(ctx, &keg.CreateOptions{ - Schema: opts.Schema, - Title: opts.Title, - Lead: opts.Lead, - Tags: opts.Tags, - Attrs: attrs, - }) + // + // The scaffold is deliberately created WITHOUT opts.Schema. An empty + // node already typed `task` would fail that schema's required fields + // and, on a strict keg, be rejected before the editor ever opened. + // The schema instead rides editWithTempFileSchema, which prefills + // `type:` in the editor frontmatter and enforces the schema on save — + // where the user's content actually exists. + created, createErr := k.Create(ctx, &keg.CreateOptions{}) if createErr != nil { return keg.NodeId{}, fmt.Errorf("unable to create node: %w", createErr) } if editErr := t.editWithTempFileSchema(ctx, k, created.ID, opts.Schema); editErr != nil { return keg.NodeId{}, fmt.Errorf("unable to edit new node: %w", editErr) } - t.warnSchemaValidation(created.Validation, created.ID, opts.Stream) + // Re-validate from storage rather than reporting created.Validation, + // which describes the empty pre-edit scaffold: for a schema with + // required fields that result is always invalid, so reporting it warns + // about a node the user already filled in. This also still warns when + // the editor is closed without saving. + t.warnSchemaIssues(ctx, k, created.ID, opts.Stream) return created.ID, nil } - attrs := createAttrsFromStrings(opts.Attrs) - created, err := k.Create(ctx, &keg.CreateOptions{ - Schema: opts.Schema, - Title: opts.Title, - Lead: opts.Lead, - Tags: opts.Tags, - Attrs: attrs, - }) + created, err := k.Create(ctx, &keg.CreateOptions{Schema: opts.Schema}) if err != nil { return keg.NodeId{}, fmt.Errorf("unable to create node: %w", err) } @@ -77,57 +69,38 @@ func (t *Tap) Create(ctx context.Context, opts CreateOptions) (keg.NodeId, error return created.ID, nil } -func createAttrsFromStrings(attrs map[string]string) map[string]any { - out := make(map[string]any, len(attrs)) - for k, v := range attrs { - out[k] = v - } - return out -} - +// shouldUseLiveEditorOnCreate reports whether `tap create` should open an +// editor: an interactive terminal with nothing piped in. --schema no longer +// suppresses it — the schema is applied in the editor, not instead of it. func shouldUseLiveEditorOnCreate(opts CreateOptions) bool { if opts.Stream == nil { return false } - if opts.Stream.IsPiped || !opts.Stream.IsTTY { - return false - } - if strings.TrimSpace(opts.Schema) != "" || strings.TrimSpace(opts.Title) != "" || strings.TrimSpace(opts.Lead) != "" { - return false - } - if len(opts.Tags) > 0 || len(opts.Attrs) > 0 { - return false - } - return true + return !opts.Stream.IsPiped && opts.Stream.IsTTY } func (t *Tap) createNodeFromRaw(ctx context.Context, k keg.Keg, raw []byte, defaults CreateOptions) (keg.NodeId, error) { - createOpts := &keg.CreateOptions{ - Schema: defaults.Schema, - Title: defaults.Title, - Lead: defaults.Lead, - Tags: defaults.Tags, - Attrs: createAttrsFromStrings(defaults.Attrs), - } + createOpts := &keg.CreateOptions{Schema: defaults.Schema} - hasFrontmatter := false - var frontmatterRaw []byte if len(raw) > 0 { - originalRaw := raw - var err error - hasFrontmatter, frontmatterRaw, _, err = splitEditNodeFile(raw) + // The editor buffer presents metadata as frontmatter above the body + // because that reads well to a human, but content and meta are + // separate inputs to the keg, which rejects content opening with a + // `---` block. Split the buffer into the two fields — exactly what tap + // edit already does for UpdateNode — so create and edit accept the + // same thing. A type declared up there still reaches schema selection, + // now as metadata rather than as frontmatter. + hasFrontmatter, frontmatterRaw, bodyRaw, err := splitEditNodeFile(raw) if err != nil { return keg.NodeId{}, err } - createOpts.Body = originalRaw - } - if hasFrontmatter { - var attrs map[string]any - if err := yaml.Unmarshal(frontmatterRaw, &attrs); err != nil { - return keg.NodeId{}, fmt.Errorf("invalid frontmatter metadata: %w", err) + if hasFrontmatter { + if _, err := keg.ParseMeta(ctx, frontmatterRaw); err != nil { + return keg.NodeId{}, fmt.Errorf("invalid frontmatter metadata: %w", err) + } + createOpts.Meta = frontmatterRaw } - // Keep frontmatter in Body so the Keg layer can detect a conflicting - // type from --attrs or --schema before applying either source. + createOpts.Body = bodyRaw } created, err := k.Create(ctx, createOpts) diff --git a/pkg/tapper/tap_import.go b/pkg/tapper/tap_import.go deleted file mode 100644 index f45caa72..00000000 --- a/pkg/tapper/tap_import.go +++ /dev/null @@ -1,259 +0,0 @@ -package tapper - -import ( - "context" - "errors" - "fmt" - "regexp" - "strings" - - "github.com/jlrickert/tapper/pkg/keg" -) - -// ImportFromKegOptions controls how ImportFromKeg copies nodes from one live keg into another. -type ImportFromKegOptions struct { - // Source is the source keg to copy nodes from. - Source KegTargetOptions - // Target is the destination keg; defaults to the resolved default keg. - Target KegTargetOptions - // NodeIDs lists the source node IDs to import. Values may be bare integers - // ("5") or cross-keg references ("keg:pub/5"). All must resolve to Source. - // When empty and TagQuery is also empty, all non-zero nodes are imported. - NodeIDs []string - // TagQuery is a boolean tag expression (same syntax as tap tags EXPR) that - // selects additional source nodes; combined with NodeIDs as a union. - TagQuery string - // LeaveStubs writes a forwarding stub at each source node location after import. - LeaveStubs bool - // SkipZeroNode skips the source keg's node 0 (the index/root node). - SkipZeroNode bool -} - -// ImportedNode records the source → target ID mapping for one imported node. -type ImportedNode struct { - SourceID keg.NodeId - TargetID keg.NodeId -} - -// kegArgRefRE matches a bare keg:ALIAS/N argument (full string). -var kegArgRefRE = regexp.MustCompile(`^keg:([a-zA-Z0-9][a-zA-Z0-9_-]*)/([0-9]+)$`) - -// ImportFromKeg copies nodes from a source keg into the target keg. Each node -// is assigned a fresh ID via targetRepo.Next() and all links in the copied -// content are rewritten according to the six rules described in the plan. -func (t *Tap) ImportFromKeg(ctx context.Context, opts ImportFromKegOptions) ([]ImportedNode, error) { - // Extract the source alias from any keg:ALIAS/N positional args and - // validate consistency with opts.Source.Keg. - srcAlias, bareIDs, err := resolveImportSourceAlias(opts.NodeIDs, opts.Source.Keg) - if err != nil { - return nil, err - } - opts.Source.Keg = srcAlias - - sourceRole := FlightRoleViewer - if opts.LeaveStubs { - sourceRole = FlightRoleEditor - } - srcKeg, err := t.resolveKegForRole(ctx, opts.Source, sourceRole) - if err != nil { - return nil, fmt.Errorf("unable to open source keg: %w", err) - } - tgtKeg, err := t.resolveKegForRole(ctx, opts.Target, FlightRoleEditor) - if err != nil { - return nil, fmt.Errorf("unable to open target keg: %w", err) - } - if kegsAreSame(srcKeg, tgtKeg) { - return nil, fmt.Errorf("source and target keg are the same: %w", keg.ErrInvalid) - } - - tgtAlias := opts.Target.Keg - - // Parse bare node IDs. - srcIDs, err := parseImportNodeIDs(bareIDs) - if err != nil { - return nil, err - } - - // Stream source nodes through a keg-archive and land them on fresh target - // ids. The archive import rewrites links between imported nodes and - // retargets cross-keg references via the source/target aliases. - rc, err := srcKeg.ExportNodes(ctx, keg.ExportNodesOptions{ - NodeIDs: srcIDs, - Query: opts.TagQuery, - SkipZeroNode: opts.SkipZeroNode, - WithHistory: true, - HistoryIfSupported: true, - WithAssets: true, - Source: srcAlias, - }) - if err != nil { - return nil, fmt.Errorf("unable to export source nodes: %w", err) - } - defer rc.Close() - - imported, err := tgtKeg.ImportNodes(ctx, rc, keg.ImportNodesOptions{ - AssignNewIDs: true, - HistoryIfSupported: true, - SourceAlias: srcAlias, - TargetAlias: tgtAlias, - }) - if err != nil { - return nil, fmt.Errorf("unable to import nodes: %w", err) - } - - // Write forwarding stubs at source locations if requested. - if opts.LeaveStubs && tgtAlias != "" { - updates := make([]keg.NodeUpdateOptions, 0, len(imported)) - for _, node := range imported { - srcID, parseErr := keg.ParseNode(node.SourceID) - if parseErr != nil || srcID == nil { - continue - } - view, readErr := srcKeg.ReadNode(ctx, *srcID) - if readErr != nil { - return nil, fmt.Errorf("unable to read source node %s before writing forwarding stub: %w", srcID.Path(), readErr) - } - title := "" - if view.Stats != nil { - title = strings.TrimSpace(view.Stats.Title()) - } - if title == "" { - if parsed, parseContentErr := keg.ParseContent(t.Runtime, view.Content, keg.MarkdownContentFilename); parseContentErr == nil { - title = strings.TrimSpace(parsed.Title) - } - } - if title == "" { - title = srcID.Path() - } - target := "keg:" + tgtAlias - body := fmt.Sprintf("# %s\n\nMoved to [%s/%s](%s/%s).\n", title, target, node.ID.Path(), target, node.ID.Path()) - updates = append(updates, keg.NodeUpdateOptions{ID: *srcID, Content: []byte(body), HasContent: true, ExpectedHash: node.SourceHash}) - } - _, err := srcKeg.UpdateNodes(keg.WithValidationMode(ctx, keg.ValidationModeOff), updates) - if err != nil { - return nil, fmt.Errorf("unable to write forwarding stubs: %w", err) - } - } - - result := make([]ImportedNode, 0, len(imported)) - for _, node := range imported { - srcID, parseErr := keg.ParseNode(node.SourceID) - if parseErr == nil && srcID != nil { - result = append(result, ImportedNode{SourceID: *srcID, TargetID: node.ID}) - } - } - return result, nil -} - -// kegSupportsSnapshots probes snapshot support by listing revisions for the -// zero node and checking for ErrNotSupported. -func kegSupportsSnapshots(ctx context.Context, k keg.Keg) bool { - _, err := k.ListSnapshots(ctx, keg.NodeId{ID: 0}) - return !errors.Is(err, keg.ErrNotSupported) -} - -// resolveImportSourceAlias extracts the source keg alias from keg:ALIAS/N -// positional arguments, validates consistency with fromFlag, and returns the -// resolved alias and a slice of bare numeric ID strings. -func resolveImportSourceAlias(rawIDs []string, fromFlag string) (string, []string, error) { - bareIDs := make([]string, 0, len(rawIDs)) - found := "" - for _, raw := range rawIDs { - if m := kegArgRefRE.FindStringSubmatch(raw); m != nil { - alias, numStr := m[1], m[2] - if fromFlag != "" && alias != fromFlag { - return "", nil, fmt.Errorf("node reference %q has alias %q but --from is %q: %w", - raw, alias, fromFlag, keg.ErrInvalid) - } - if found != "" && alias != found { - return "", nil, fmt.Errorf("conflicting source keg aliases %q and %q in arguments: %w", - found, alias, keg.ErrInvalid) - } - found = alias - bareIDs = append(bareIDs, numStr) - } else { - bareIDs = append(bareIDs, raw) - } - } - alias := fromFlag - if alias == "" { - alias = found - } - return alias, bareIDs, nil -} - -// parseImportNodeIDs converts raw node ID strings to NodeId values. These ids -// are scoped to the import's source keg, which import selects through its own -// --from alias / "alias/id" syntax, so they are deliberately parsed as bare ids -// rather than routed through resolveNodeArg (whose keg: redirect would collide -// with import's own keg-selection mechanism). -func parseImportNodeIDs(rawIDs []string) ([]keg.NodeId, error) { - ids := make([]keg.NodeId, 0, len(rawIDs)) - for _, raw := range rawIDs { - id, err := parseNodeID(raw) - if err != nil { - return nil, fmt.Errorf("invalid node ID %q: %w", raw, err) - } - ids = append(ids, id) - } - return ids, nil -} - -// collectImportNodesByTag evaluates a boolean query expression (supporting both -// tag names and key=value attribute predicates) against the source keg's dex. -func collectImportNodesByTag(ctx context.Context, k keg.Keg, query string) ([]keg.NodeId, error) { - entries, err := k.Query(ctx, keg.QueryOptions{Expr: query}) - if err != nil { - return nil, fmt.Errorf("invalid query expression: %w", err) - } - ids := make([]keg.NodeId, 0, len(entries)) - seen := make(map[int]struct{}, len(entries)) - for _, entry := range entries { - n, parseErr := keg.ParseNode(entry.ID) - if parseErr != nil || n == nil { - continue - } - if _, ok := seen[n.ID]; ok { - continue - } - seen[n.ID] = struct{}{} - ids = append(ids, *n) - } - return ids, nil -} - -// unionImportNodeIDs merges two slices, deduplicating by numeric ID. -func unionImportNodeIDs(a, b []keg.NodeId) []keg.NodeId { - seen := make(map[string]struct{}, len(a)+len(b)) - out := make([]keg.NodeId, 0, len(a)+len(b)) - for _, id := range append(a, b...) { - key := id.Path() - if _, ok := seen[key]; !ok { - seen[key] = struct{}{} - out = append(out, id) - } - } - return out -} - -// filterZeroImportNode removes node 0 from the list. -func filterZeroImportNode(ids []keg.NodeId) []keg.NodeId { - out := make([]keg.NodeId, 0, len(ids)) - for _, id := range ids { - if id.ID != 0 || id.Code != "" { - out = append(out, id) - } - } - return out -} - -// kegsAreSame reports whether two kegs refer to the same underlying storage. -func kegsAreSame(a, b keg.Keg) bool { - if a == b { - return true - } - if a.Target() == nil || b.Target() == nil { - return false - } - return strings.EqualFold(a.Target().String(), b.Target().String()) -} diff --git a/pkg/tapper/tap_import_test.go b/pkg/tapper/tap_import_test.go deleted file mode 100644 index def90968..00000000 --- a/pkg/tapper/tap_import_test.go +++ /dev/null @@ -1,113 +0,0 @@ -package tapper - -import ( - "context" - "errors" - "testing" - - "github.com/jlrickert/tapper/pkg/keg" - "github.com/stretchr/testify/require" -) - -type updateRecordingKeg struct { - keg.Keg - batches [][]keg.NodeUpdateOptions - modes []keg.ValidationMode -} - -func (k *updateRecordingKeg) UpdateNodes(ctx context.Context, updates []keg.NodeUpdateOptions) ([]keg.NodeUpdateResult, error) { - k.batches = append(k.batches, append([]keg.NodeUpdateOptions(nil), updates...)) - k.modes = append(k.modes, keg.ValidationModeFromContext(ctx)) - return k.Keg.UpdateNodes(ctx, updates) -} - -func TestResolveImportSourceAlias_BareIDs(t *testing.T) { - t.Parallel() - alias, bareIDs, err := resolveImportSourceAlias([]string{"1", "2", "3"}, "mykeg") - require.NoError(t, err) - require.Equal(t, "mykeg", alias) - require.Equal(t, []string{"1", "2", "3"}, bareIDs) -} - -func TestImportFromKeg_LeaveStubsRequiresEditorOnSource(t *testing.T) { - t.Parallel() - for _, tc := range []struct { - name string - leaveStubs bool - want FlightRole - }{ - {name: "copy only", want: FlightRoleViewer}, - {name: "leave stubs", leaveStubs: true, want: FlightRoleEditor}, - } { - t.Run(tc.name, func(t *testing.T) { - var got FlightRole - tap := &Tap{KegResolver: func(context.Context, KegTargetOptions, FlightRole) (keg.Keg, error) { - return nil, errors.New("unexpected resolver path") - }} - tap.KegResolver = func(_ context.Context, _ KegTargetOptions, role FlightRole) (keg.Keg, error) { - got = role - return nil, errors.New("stop after role capture") - } - _, err := tap.ImportFromKeg(context.Background(), ImportFromKegOptions{ - Source: KegTargetOptions{Keg: "source"}, LeaveStubs: tc.leaveStubs, - }) - require.Error(t, err) - require.Equal(t, tc.want, got) - }) - } -} - -func TestResolveImportSourceAlias_KegRefArgs(t *testing.T) { - t.Parallel() - alias, bareIDs, err := resolveImportSourceAlias( - []string{"keg:pub/5", "keg:pub/7"}, "", - ) - require.NoError(t, err) - require.Equal(t, "pub", alias) - require.Equal(t, []string{"5", "7"}, bareIDs) -} - -func TestResolveImportSourceAlias_KegRefConflictsWithFrom(t *testing.T) { - t.Parallel() - _, _, err := resolveImportSourceAlias([]string{"keg:pub/1"}, "other") - require.Error(t, err) -} - -func TestResolveImportSourceAlias_ConflictingAliasesInArgs(t *testing.T) { - t.Parallel() - _, _, err := resolveImportSourceAlias( - []string{"keg:pub/1", "keg:priv/2"}, "", - ) - require.Error(t, err) -} - -func TestResolveImportSourceAlias_MixedBareAndKegRef(t *testing.T) { - t.Parallel() - // Mixing bare IDs with keg: refs is allowed; bare IDs are kept as-is. - alias, bareIDs, err := resolveImportSourceAlias( - []string{"3", "keg:pub/5"}, "", - ) - require.NoError(t, err) - require.Equal(t, "pub", alias) - require.Equal(t, []string{"3", "5"}, bareIDs) -} - -func TestFilterZeroImportNode(t *testing.T) { - t.Parallel() - ids := []keg.NodeId{{ID: 0}, {ID: 1}, {ID: 2}, {ID: 0, Code: "draft"}} - got := filterZeroImportNode(ids) - // Node 0 without code is filtered; Node 0 with code is kept. - require.Len(t, got, 3) - require.Equal(t, 1, got[0].ID) - require.Equal(t, 2, got[1].ID) - require.Equal(t, 0, got[2].ID) - require.Equal(t, "draft", got[2].Code) -} - -func TestUnionImportNodeIDs_Deduplication(t *testing.T) { - t.Parallel() - a := []keg.NodeId{{ID: 1}, {ID: 2}} - b := []keg.NodeId{{ID: 2}, {ID: 3}} - got := unionImportNodeIDs(a, b) - require.Len(t, got, 3) -} diff --git a/pkg/tapper/tap_orient.go b/pkg/tapper/tap_orient.go index b1bf0d35..75aadd6d 100644 --- a/pkg/tapper/tap_orient.go +++ b/pkg/tapper/tap_orient.go @@ -20,6 +20,8 @@ const orientRulesSummary = "Rules:\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" + + "- Every successful write returns a new hash and invalidates the one you were holding. Re-read with `cat`, `schema_read`, or `keg_settings` before each guarded write; a hash never covers two writes, so an edit followed by a delete needs two reads.\n" + + "- Node ids are per-keg counters. Node 4 in one keg has nothing to do with node 4 in another, ids are never reused after a removal, and a create takes the next free id rather than filling a gap.\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" + "- 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"