diff --git a/AGENTS.md b/AGENTS.md index 3ebac79..dd083c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -269,7 +269,31 @@ Two readers exist: `claude_code` and `codex`. Adding a third means adding a `Rea - **`plugins/engram/` is the single source of truth.** `install.rs` embeds the command bodies with `include_str!`, so the plugin directory and the installed files cannot drift and the compiler enforces the files exist. Exactly two substitutions, via `str::replace`, no template engine: `{{DB}}` and `{{HARNESS}}`. - **`{{DB}}` is load-bearing.** The path is discovered from the harness's *own* MCP registration (`harness::registered_db`) — on a typical host all writable harnesses point at one shared store (here `~/.local/share/engram/engram.db`) — but see the drift note below: what they registered *yesterday* is not necessarily what a previously-generated command still pins. A generated command that omitted `--db` would fall back to clap's relative `engram.db` default and quietly write to a different store than the agents read. Config formats are scanned narrowly rather than deserialized: JSON (`mcpServers`, or Opencode's `mcp`), **JSONC** (comment-stripped by a string-aware pass — a `//` inside `"https://…"` must survive), and TOML (line-scanned, so engram needs no TOML dependency). Engram **reads** JSONC and never rewrites it; a serde round-trip would delete the user's comments. -- **7 of 9 harnesses can host something; only 3 host a *command*.** Claude Code, **OpenClaude**, and Opencode have writable command dirs. **Codex 0.149 removed `~/.codex/prompts/`** — the binary contains no such string — and moved to skills at `~/.codex/skills//SKILL.md`, discovered automatically with nothing to register; engram writes there now. It wrote prompt files nobody read for a release, which is exactly what a harness table drifting from reality looks like. **Antigravity has no slash-command directory at all** — its extension surface is skills, packaged in plugins, and `agy plugin validate` reports a plugin's `commands/` as "2 processed (converted to skills)", so a command there is a skill either way. Engram writes it a plugin (`~/.gemini/config/plugins/engram/`: `plugin.json` + one `skills/engram-/SKILL.md` per command). **Kimi** and **Qwen** take skills in their own config roots (`~/.kimi-code/skills`, `~/.qwen/skills`) — Qwen's was verified against its own bundled `docs/features/skills.md`, having previously been dismissed as "format unverified". Goose and Copilot CLI have nothing engram can write and each says so **in its own words** — one shared sentence described none of them accurately. +- **9 of 11 harnesses can host something.** Claude Code, **OpenClaude**, and Opencode have writable command dirs. **Codex 0.149 removed `~/.codex/prompts/`** — the binary contains no such string — and moved to skills at `~/.codex/skills//SKILL.md`, discovered automatically with nothing to register; engram writes there now. It wrote prompt files nobody read for a release, which is exactly what a harness table drifting from reality looks like. **Antigravity has no slash-command directory at all** — its extension surface is skills, packaged in plugins, and `agy plugin validate` reports a plugin's `commands/` as "2 processed (converted to skills)", so a command there is a skill either way. Engram writes it a plugin (`~/.gemini/config/plugins/engram/`: `plugin.json` + one `skills/engram-/SKILL.md` per command). **Kimi** and **Qwen** take skills in their own config roots (`~/.kimi-code/skills`, `~/.qwen/skills`) — Qwen's was verified against its own bundled `docs/features/skills.md`, having previously been dismissed as "format unverified". Goose and Copilot CLI have nothing engram can write and each says so **in its own words** — one shared sentence described none of them accurately. +- **A skill description is YAML, so engram quotes it.** `description: Save this + conversation: capture the transcript ...` emitted bare is invalid YAML + (`mapping values are not allowed in this context`) and the **whole skill + silently fails to load** — Antigravity offered two of engram's three commands + for exactly this reason, with no error anywhere. `yaml_quote` emits a + double-quoted scalar, the form that survives colons, `#`, and the apostrophe + in "engram's" alike. +- **VS Code and Cursor, both first-party-verified.** VS Code takes a reusable + prompt file, `~/.config/Code/User/prompts/engram-.prompt.md` — Microsoft + documents the `.prompt.md` extension and VS Code itself creates the profile + folder — and its `mcp.json` keys servers under `servers`, not `mcpServers`, + which `json_engram_args` now accepts. Cursor takes a skill at + `~/.cursor/skills//SKILL.md`: both that path and `SKILL.md` appear + inside the cursor-agent binary. `~/.cursor/skills-cursor` is the vendor's own + bundle and is *not* a user surface — Grok's compatibility scanner filters + those same vendor defaults out. +- **A shared skills directory duplicates commands, and engram says so.** + `also_scans` records the directories a harness *loads* from but engram never + writes. On a machine where `~/.claude/skills` and `~/.codex/skills` both + resolve to one library (a normal way to keep a single skill set), the + commands engram writes for Codex are also loaded by Claude Code, which then + offers every engram command twice. Engram cannot fix this by writing + differently — both targets are correct for their own harness — so `install` + reports the overlap and names the harness whose commands are being doubled. - **Grok needs no harness entry, and adding one would duplicate its commands.** Grok reads other vendors' directories on purpose: `[compat.claude]` in `~/.grok/config.toml` defaults every cell to `true`, so `~/.claude/commands/` diff --git a/CHANGELOG.md b/CHANGELOG.md index 2292ca2..f0bbd64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ follows [Keep a Changelog](https://keepachangelog.com/); versions follow ### Fixed +- **A skill description containing a colon silently broke the whole skill.** + `description: Save this conversation: capture ...` is invalid YAML, so + Antigravity loaded two of engram's three commands and reported nothing. + Descriptions are now emitted as quoted scalars. + - **An unwritable target no longer aborts the whole install.** A read-only command or skills directory is now reported per file with its reason and the run continues; previously the `EROFS` propagated and every harness after the @@ -32,6 +37,13 @@ follows [Keep a Changelog](https://keepachangelog.com/); versions follow ### Added +- **VS Code** and **Cursor** are supported harnesses (the tenth and eleventh). + VS Code gets `~/.config/Code/User/prompts/engram-.prompt.md`; Cursor + gets `~/.cursor/skills/engram-/SKILL.md`. VS Code's `mcp.json` keys its + servers under `servers`, which the MCP scanner now reads. +- `install` warns when two harnesses share a command or skills directory, so a + duplicated slash command is explained rather than left to be noticed. + - **Kimi** is a supported harness (the ninth). Skills go to `~/.kimi-code/skills/engram-/SKILL.md`; the MCP registration is read from `~/.kimi-code/mcp.json`. Its transcripts (`~/.kimi/sessions// diff --git a/src/harness.rs b/src/harness.rs index 41275db..f492d21 100644 --- a/src/harness.rs +++ b/src/harness.rs @@ -38,6 +38,13 @@ pub enum Harness { Codex, Opencode, Kimi, + /// Renamed on both derives for the same reason as `OpenClaude`: + /// kebab-casing gives `vs-code`, but the editor is written `vscode` and + /// that is what [`HarnessSpec::name`] carries. + #[serde(rename = "vscode")] + #[clap(name = "vscode")] + VsCode, + Cursor, Antigravity, Goose, CopilotCli, @@ -124,6 +131,16 @@ pub struct HarnessSpec { /// can host one, and claiming otherwise would make `install` look broken /// on the rest. pub command_surface: CommandSurface, + /// Extra home-relative directories this harness *loads* commands or skills + /// from, which engram never writes to. + /// + /// Engram writes one target per harness, but several harnesses read more + /// than one directory, and on a machine where those directories are + /// symlinked together a command engram wrote for harness A shows up a + /// second time in harness B. Recording what a harness reads is what lets + /// `install` say so instead of leaving the user to notice the duplicate in + /// their own slash-command list. + pub also_scans: &'static [&'static str], /// Where this harness registers MCP servers, when engram knows. Read to /// discover which database the user already shares between harnesses. pub mcp_config: Option, @@ -138,7 +155,8 @@ pub struct HarnessSpec { /// database the user actually shares between harnesses rather than guessing. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum McpConfigSource { - /// A JSON file with an `mcpServers` (or `mcp`) object at the top level. + /// A JSON file with an `mcpServers` (or `mcp`, or VS Code's `servers`) + /// object at the top level. Json(&'static str), /// The same, but JSON **with comments** — Opencode's `opencode.jsonc`. /// Comments are stripped before parsing; the file itself is never @@ -162,11 +180,13 @@ pub const ALL: &[HarnessSpec] = &[ probe: &[".claude", ".claude.json"], sessions_dir: Some(".claude/projects"), transcript: TranscriptSupport::Reader(ReaderKind::ClaudeCode), + // Claude Code loads skills as well as commands. command_surface: CommandSurface::Markdown { dir: ".claude/commands", file: "engram-{name}.md", frontmatter: true, }, + also_scans: &[".claude/skills"], mcp_config: Some(McpConfigSource::Json(".claude.json")), hooks_config: Some(".claude/settings.json"), }, @@ -188,6 +208,7 @@ pub const ALL: &[HarnessSpec] = &[ file: "engram-{name}.md", frontmatter: true, }, + also_scans: &[".openclaude/skills"], mcp_config: Some(McpConfigSource::Json(".openclaude.json")), // The fork has a `hooks` key, but its shape is unverified against a // real run; `install --hooks` stays Claude-Code-only until it is. @@ -205,6 +226,7 @@ pub const ALL: &[HarnessSpec] = &[ command_surface: CommandSurface::Skill { dir: ".codex/skills", }, + also_scans: &[], mcp_config: Some(McpConfigSource::Toml(".codex/config.toml")), hooks_config: None, }, @@ -221,6 +243,7 @@ pub const ALL: &[HarnessSpec] = &[ file: "engram-{name}.md", frontmatter: true, }, + also_scans: &[], mcp_config: Some(McpConfigSource::Jsonc(".config/opencode/opencode.jsonc")), hooks_config: None, }, @@ -241,9 +264,54 @@ pub const ALL: &[HarnessSpec] = &[ command_surface: CommandSurface::Skill { dir: ".kimi-code/skills", }, + also_scans: &[], mcp_config: Some(McpConfigSource::Json(".kimi-code/mcp.json")), hooks_config: None, }, + HarnessSpec { + id: Harness::VsCode, + name: "vscode", + probe: &[".config/Code"], + // Chat history lives in workspaceStorage as editor state, not as a + // transcript engram can read turn by turn. + sessions_dir: None, + transcript: TranscriptSupport::Unsupported { + detail: "vs code keeps chat in workspaceStorage as editor state, not a \ + per-session transcript file", + }, + // Reusable prompt files: `.prompt.md` in the profile's `prompts` + // folder, invoked with `/`. The extension and the `description` + // frontmatter field are Microsoft-documented; the directory is created + // by VS Code itself. + command_surface: CommandSurface::Markdown { + dir: ".config/Code/User/prompts", + file: "engram-{name}.prompt.md", + frontmatter: true, + }, + also_scans: &[], + mcp_config: Some(McpConfigSource::Json(".config/Code/User/mcp.json")), + hooks_config: None, + }, + HarnessSpec { + id: Harness::Cursor, + name: "cursor", + probe: &[".cursor"], + sessions_dir: Some(".cursor/chats"), + transcript: TranscriptSupport::NotImplemented { + detail: "cursor stores chats under ~/.cursor/chats in a format engram has not \ + surveyed", + }, + // `.cursor/skills` and `SKILL.md` both appear in the cursor-agent + // binary. `.cursor/skills-cursor` is the vendor's own bundle and is + // not a user surface --- Grok's compatibility scanner filters those + // same vendor defaults out. + command_surface: CommandSurface::Skill { + dir: ".cursor/skills", + }, + also_scans: &[".cursor/commands"], + mcp_config: Some(McpConfigSource::Json(".cursor/mcp.json")), + hooks_config: None, + }, HarnessSpec { id: Harness::Antigravity, name: "antigravity", @@ -255,6 +323,7 @@ pub const ALL: &[HarnessSpec] = &[ command_surface: CommandSurface::Plugin { dir: ".gemini/config/plugins", }, + also_scans: &[], mcp_config: Some(McpConfigSource::Json(".gemini/antigravity/mcp_config.json")), hooks_config: None, }, @@ -269,6 +338,7 @@ pub const ALL: &[HarnessSpec] = &[ command_surface: CommandSurface::None { detail: "goose has no user command directory engram has surveyed", }, + also_scans: &[], mcp_config: None, hooks_config: None, }, @@ -285,6 +355,7 @@ pub const ALL: &[HarnessSpec] = &[ them from a marketplace, a GitHub repository, or a git URL --- \ there is no user-writable directory engram can drop a command into", }, + also_scans: &[], mcp_config: Some(McpConfigSource::Json(".copilot/mcp-config.json")), hooks_config: None, }, @@ -302,6 +373,7 @@ pub const ALL: &[HarnessSpec] = &[ command_surface: CommandSurface::Skill { dir: ".qwen/skills", }, + also_scans: &[], mcp_config: Some(McpConfigSource::Json(".qwen/settings.json")), hooks_config: None, }, @@ -500,7 +572,9 @@ fn json_engram_args(text: &str) -> Option> { // `mcpServers` is the common key; Opencode uses `mcp`. let servers = value .get("mcpServers") - .or_else(|| value.get("mcp"))? + .or_else(|| value.get("mcp")) + // VS Code's `~/.config/Code/User/mcp.json` uses `servers`. + .or_else(|| value.get("servers"))? .as_object()?; let entry = servers.get("engram")?; // `command` may be a bare string with `args` alongside, or (Opencode) an diff --git a/src/install.rs b/src/install.rs index 7710432..7aeaf96 100644 --- a/src/install.rs +++ b/src/install.rs @@ -227,7 +227,33 @@ fn render_skill(name: &str, template_body: &str, spec: &HarnessSpec, db: &str) - let body = strip_frontmatter(template_body) .replace("{{DB}}", db) .replace("{{HARNESS}}", spec.name); - format!("---\nname: engram-{name}\ndescription: {description}\n---\n{BANNER}\n{body}") + format!( + "---\nname: engram-{name}\ndescription: {}\n---\n{BANNER}\n{body}", + yaml_quote(&description) + ) +} + +/// Renders a string as a YAML double-quoted scalar. +/// +/// A skill description is a sentence engram does not control, and one of them +/// is "Save this conversation: capture the transcript ...". Emitted bare, the +/// second colon makes the line `mapping values are not allowed in this +/// context` and the *whole skill silently fails to load* — Antigravity showed +/// two of engram's three commands for exactly this reason. Double quotes are +/// the form that survives colons, `#`, leading `%`/`@`, and the apostrophe in +/// "engram's" alike; only `\` and `"` need escaping inside them. +fn yaml_quote(value: &str) -> String { + let mut out = String::with_capacity(value.len() + 2); + out.push('"'); + for c in value.chars() { + match c { + '\\' => out.push_str("\\\\"), + '"' => out.push_str("\\\""), + _ => out.push(c), + } + } + out.push('"'); + out } /// Reads one scalar field out of a leading YAML block. Deliberately not a YAML @@ -288,6 +314,68 @@ fn is_nix_managed(path: &std::path::Path) -> bool { .any(|target| target.starts_with("/nix/store")) } +/// The canonical form of a path whose leaf may not exist yet. +/// +/// `canonicalize` fails on a missing leaf, so this walks up to the deepest +/// existing ancestor, canonicalizes that, and re-appends the rest. Without it +/// a target directory engram is about to create compares unequal to the very +/// same directory reached through a symlink. +fn canonical_key(path: &std::path::Path) -> PathBuf { + let mut tail = Vec::new(); + let mut cursor = path; + loop { + if let Ok(real) = std::fs::canonicalize(cursor) { + let mut out = real; + for part in tail.iter().rev() { + out.push(part); + } + return out; + } + match (cursor.file_name(), cursor.parent()) { + (Some(name), Some(parent)) => { + tail.push(name.to_owned()); + cursor = parent; + } + _ => return path.to_path_buf(), + } + } +} + +/// Warns when another harness's engram commands are visible from this one. +/// +/// Engram writes one directory per harness, but a harness may *read* several, +/// and those directories are routinely symlinked together — `~/.claude/skills` +/// and `~/.codex/skills` both resolving to a shared `~/.agents/skills` is a +/// normal way to keep one skill library. The consequence is that a command +/// engram wrote for Codex is also loaded by Claude Code, which then lists every +/// engram command twice. Engram cannot fix that by writing differently: both +/// targets are correct for their own harness. It can say so. +fn overlap_warning(spec: &HarnessSpec, targets: &[(&'static str, PathBuf)]) -> Option { + let mine: Vec = spec + .also_scans + .iter() + .filter_map(|d| harness::in_home(d)) + .map(|d| canonical_key(&d)) + .collect(); + let clashes: Vec<&str> = targets + .iter() + .filter(|(name, dir)| *name != spec.name && mine.iter().any(|m| m == dir)) + .map(|(name, _)| *name) + .collect(); + if clashes.is_empty() { + return None; + } + Some(format!( + "{} also loads commands from a directory engram writes for {}, so every engram \ + command will appear twice in {}. Both writes are correct for their own harness — \ + the duplication comes from those directories being the same one. Point them at \ + separate directories to remove it.", + spec.name, + clashes.join(", "), + spec.name + )) +} + /// True when this write failed only because the target is not writable. /// /// A read-only target is a fact about the user's machine, not an engram error: @@ -320,6 +408,14 @@ pub fn install( let mut installed = 0usize; let mut skipped = 0usize; + // Every detected harness's canonical write directory, so a harness that + // *reads* one of them can say whose commands it is about to show twice. + let written_dirs: Vec<(&'static str, PathBuf)> = targets + .iter() + .filter(|spec| harness::describe(spec).present) + .filter_map(|spec| harness::commands_dir(spec).map(|d| (spec.name, canonical_key(&d)))) + .collect(); + for spec in targets { let detected = harness::describe(spec); let Some(dir) = harness::commands_dir(spec) else { @@ -349,6 +445,7 @@ pub fn install( let (db, db_origin) = resolve_db(spec, db_override); + let overlap = overlap_warning(spec, &written_dirs); let nix_warning = is_nix_managed(&dir).then(|| { format!( "{} resolves into the Nix store; anything written here will be replaced by the \ @@ -449,6 +546,16 @@ pub fn install( } (None, nix) => nix, }; + // The overlap note is independent of both: a directory can be shared + // whether or not it is in the store or was re-pinned. + let warning = match (warning, overlap) { + (Some(mut w), Some(o)) => { + w.push(' '); + w.push_str(&o); + Some(w) + } + (w, o) => w.or(o), + }; // Hooks are opt-in twice over: this flag, and the fact that a hook // only ever runs `ingest`. diff --git a/tests/cli.rs b/tests/cli.rs index 818fdc5..601e4f4 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -2095,7 +2095,7 @@ fn install_list_reports_every_harness_and_writes_nothing() { let data = parse_single_line_json(&assert.get_output().stdout)["data"].clone(); let harnesses = data["harnesses"].as_array().expect("harnesses"); - assert_eq!(harnesses.len(), 9, "every known harness must be reported"); + assert_eq!(harnesses.len(), 11, "every known harness must be reported"); let claude = harnesses .iter() @@ -3380,3 +3380,117 @@ fn install_writes_kimi_and_qwen_skills() { assert!(text.contains(&format!("--harness {harness}")), "{harness}"); } } + +/// A skill description containing a colon must survive as valid YAML. +/// +/// `Save this conversation: capture the transcript ...` emitted bare makes the +/// frontmatter `mapping values are not allowed in this context`, and the whole +/// skill silently fails to load — Antigravity offered two of engram's three +/// commands for exactly this reason, with no error anywhere. +#[test] +fn install_quotes_skill_descriptions_containing_a_colon() { + let tmp = TempDir::new().expect("tempdir"); + let db = tmp.path().join("test.db"); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).expect("create fake home"); + pretend_installed(&home, ".qwen"); + + install(&db, &home, &["--db-path", "/shared/engram.db"]) + .assert() + .success(); + + let text = std::fs::read_to_string(home.join(".qwen/skills/engram-save-chat/SKILL.md")) + .expect("skill written"); + let description = text + .lines() + .find(|l| l.starts_with("description:")) + .expect("a description line"); + assert!( + description.starts_with("description: \"") && description.ends_with('"'), + "description must be a quoted scalar: {description}" + ); + // The colon that broke it is still present — quoted, not stripped. + assert!(description.contains("conversation:"), "{description}"); +} + +/// VS Code takes a `.prompt.md` reusable prompt; Cursor takes a skill. +/// +/// Both are first-party-verified: Microsoft documents the `.prompt.md` +/// extension and VS Code itself creates the profile `prompts` folder, while +/// `.cursor/skills` and `SKILL.md` both appear inside the cursor-agent binary. +#[test] +fn install_writes_vscode_prompts_and_cursor_skills() { + let tmp = TempDir::new().expect("tempdir"); + let db = tmp.path().join("test.db"); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).expect("create fake home"); + pretend_installed(&home, ".config/Code"); + pretend_installed(&home, ".cursor"); + + install(&db, &home, &["--db-path", "/shared/engram.db"]) + .assert() + .success(); + + let vscode = + std::fs::read_to_string(home.join(".config/Code/User/prompts/engram-save-chat.prompt.md")) + .expect("vscode prompt written"); + assert!(vscode.contains("--db /shared/engram.db"), "{vscode}"); + assert!(vscode.contains("--harness vscode"), "{vscode}"); + + let cursor = std::fs::read_to_string(home.join(".cursor/skills/engram-save-chat/SKILL.md")) + .expect("cursor skill written"); + assert!(cursor.starts_with("---\n"), "{cursor}"); + assert!(cursor.contains("name: engram-save-chat"), "{cursor}"); + assert!(cursor.contains("--harness cursor"), "{cursor}"); +} + +/// A shared skills directory makes one harness list another's commands. +/// +/// `~/.claude/skills` and `~/.codex/skills` both pointing at one library is a +/// normal way to keep a single set of skills, and the consequence is that the +/// commands engram writes for Codex are also loaded by Claude Code, which then +/// offers every engram command twice. Engram cannot fix it by writing +/// differently — both targets are right for their own harness — so it says so. +#[test] +fn install_warns_when_two_harnesses_share_a_skills_directory() { + let tmp = TempDir::new().expect("tempdir"); + let db = tmp.path().join("test.db"); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).expect("create fake home"); + pretend_installed(&home, ".claude"); + pretend_installed(&home, ".codex"); + + // One shared library, reached from both harnesses. + let shared = home.join(".agents/skills"); + std::fs::create_dir_all(&shared).expect("shared skills dir"); + std::os::unix::fs::symlink(&shared, home.join(".codex/skills")).expect("codex link"); + std::os::unix::fs::symlink(&shared, home.join(".claude/skills")).expect("claude link"); + + let assert = install(&db, &home, &["--db-path", "/shared/engram.db"]) + .assert() + .success(); + let data = parse_single_line_json(&assert.get_output().stdout)["data"].clone(); + + let claude = data["harnesses"] + .as_array() + .expect("harnesses") + .iter() + .find(|h| h["harness"] == "claude-code") + .expect("claude-code reported") + .clone(); + let warning = claude["warning"] + .as_str() + .expect("claude-code must warn about the shared directory"); + assert!(warning.contains("codex"), "{warning}"); + assert!(warning.contains("twice"), "{warning}"); + + // Codex itself has nothing to warn about: it reads only what it was given. + let codex = data["harnesses"] + .as_array() + .expect("harnesses") + .iter() + .find(|h| h["harness"] == "codex") + .expect("codex reported") + .clone(); + assert!(codex["warning"].is_null(), "codex: {:?}", codex["warning"]); +}