From c6d3de095822be291a8ca2b58a1a263bde5db9d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:02:13 +0200 Subject: [PATCH 1/5] fix(skill): harden inventory diagnostics --- pnpm-lock.yaml | 60 +++---- src/commands/skill.rs | 147 +++++++++++++----- tests/contracts/test_skill_list_output.rs | 41 ++++- .../docs/src/content/docs/guides/skills.mdx | 2 +- 4 files changed, 177 insertions(+), 73 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1d196b33..97594606 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -25,23 +25,23 @@ importers: version: 7.0.2 optionalDependencies: '@dallay/agentsync-darwin-arm64': - specifier: 1.52.2 - version: 1.52.2 + specifier: 1.52.3 + version: 1.52.3 '@dallay/agentsync-darwin-x64': - specifier: 1.52.2 - version: 1.52.2 + specifier: 1.52.3 + version: 1.52.3 '@dallay/agentsync-linux-arm64': - specifier: 1.52.2 - version: 1.52.2 + specifier: 1.52.3 + version: 1.52.3 '@dallay/agentsync-linux-x64': - specifier: 1.52.2 - version: 1.52.2 + specifier: 1.52.3 + version: 1.52.3 '@dallay/agentsync-windows-arm64': - specifier: 1.52.2 - version: 1.52.2 + specifier: 1.52.3 + version: 1.52.3 '@dallay/agentsync-windows-x64': - specifier: 1.52.2 - version: 1.52.2 + specifier: 1.52.3 + version: 1.52.3 website/docs: dependencies: @@ -335,43 +335,43 @@ packages: resolution: {integrity: sha512-kzyuwOAQnXJNLS9PSyrk0CWk35nWJW/zl/6KvnTBMFK65gm7U1/Z5BqjxeapjZCIhQcM/DsrEmcbRwDyXyXK4A==} engines: {node: '>=14'} - '@dallay/agentsync-darwin-arm64@1.52.2': - resolution: {integrity: sha512-CispJLqnatiE9K/CNZR7Uf6yuGGqJa1De9g8XnQJ0o0fwYWp3DOXWzXFcX8m10ondnyUQqR3bl8ic8tlsNeefw==} + '@dallay/agentsync-darwin-arm64@1.52.3': + resolution: {integrity: sha512-eqO9+yc6JI3+9F9upv41DmbeFOCIftMF4dDCaA39E8DB+3PuhV3VWm9yZjr9GUED9TMtz4koTgwamY39RNFMGw==} engines: {node: '>=18'} cpu: [arm64] os: [darwin] hasBin: true - '@dallay/agentsync-darwin-x64@1.52.2': - resolution: {integrity: sha512-Bq5x9AQSMl1NriMNuA2bZldUoF3mDrYgfrwbn5+4xWG1uQtvuWZar6V0iEcgZ5eS5GQaNWGEGeWeuXZq0cVdAQ==} + '@dallay/agentsync-darwin-x64@1.52.3': + resolution: {integrity: sha512-Y1i/VBFK9dmQvl+iN7Qt1CAcc2TkSV4PpU7PD94CiaYoGbddQ+ZOoq5TagNPkQjRw5IhUQXFEMuOxvA2qYFn/w==} engines: {node: '>=18'} cpu: [x64] os: [darwin] hasBin: true - '@dallay/agentsync-linux-arm64@1.52.2': - resolution: {integrity: sha512-Zbi3/S4zewAAdIw43zPq5h9ctPQ6+VoOa6H7lOdHGcdxie45i/E1g5/nop52QeGujl5z6rPzFGR0uRTH0Bqs3Q==} + '@dallay/agentsync-linux-arm64@1.52.3': + resolution: {integrity: sha512-q4gES2T/fPn9ltHmJVqnV8XHHAbEUIgTzpA/bwnwXC3dtqLaF0n8QiEBtxbt+uYqZpG7IvjgonCoaZGMNvjHfQ==} engines: {node: '>=18'} cpu: [arm64] os: [linux] hasBin: true - '@dallay/agentsync-linux-x64@1.52.2': - resolution: {integrity: sha512-TA9UM6OkvaqkmJhBLm6d1DhNsMuatNoejNF/L/EspT6c5A2Fd/P9pU7zWAZpBPps04UgFk+XKQS5p57fi0Y1ng==} + '@dallay/agentsync-linux-x64@1.52.3': + resolution: {integrity: sha512-/+E/DmAIXzMtIltNjxjLmTlosa3MPprCZ/YcYfBS3kw6pyzzFcUbX/InqCby1IhtpNIedQaI365IK4bLNlLH+A==} engines: {node: '>=18'} cpu: [x64] os: [linux] hasBin: true - '@dallay/agentsync-windows-arm64@1.52.2': - resolution: {integrity: sha512-ZLyrlb+EdJZZZp5f/dV84ut1ah2ZPesThybHsebf+NMiuTmzYq8H+WWF0tMqZbbaQhTyfBj8XKgOgQK1Tab7xw==} + '@dallay/agentsync-windows-arm64@1.52.3': + resolution: {integrity: sha512-cj/PlImrwP0Xa6KuQkcEC/DjI/nX+56l8vv/EHEWS67S975fgov0tBQZYzW/Q/owEPEKazY5zEqOMRASSsYQJQ==} engines: {node: '>=18'} cpu: [arm64] os: [win32] hasBin: true - '@dallay/agentsync-windows-x64@1.52.2': - resolution: {integrity: sha512-SoJ//1oPBjsOHikvRqQt1wAnCERpJkT9nHD5Ml6vVu7FXzBml0fjShYRHKnG/aqFY0mJLCn67yC7TJ6EKxbq8w==} + '@dallay/agentsync-windows-x64@1.52.3': + resolution: {integrity: sha512-yZorZEaA4ozouUnl0mHAmxMw0SR1KytWHjfdpRY/6aZgRBWJ3rcVzcxBAql03RyfBARhGlqexC/dR7OPJAnVpg==} engines: {node: '>=18'} cpu: [x64] os: [win32] @@ -2499,22 +2499,22 @@ snapshots: '@ctrl/tinycolor@4.2.0': {} - '@dallay/agentsync-darwin-arm64@1.52.2': + '@dallay/agentsync-darwin-arm64@1.52.3': optional: true - '@dallay/agentsync-darwin-x64@1.52.2': + '@dallay/agentsync-darwin-x64@1.52.3': optional: true - '@dallay/agentsync-linux-arm64@1.52.2': + '@dallay/agentsync-linux-arm64@1.52.3': optional: true - '@dallay/agentsync-linux-x64@1.52.2': + '@dallay/agentsync-linux-x64@1.52.3': optional: true - '@dallay/agentsync-windows-arm64@1.52.2': + '@dallay/agentsync-windows-arm64@1.52.3': optional: true - '@dallay/agentsync-windows-x64@1.52.2': + '@dallay/agentsync-windows-x64@1.52.3': optional: true '@emnapi/core@1.11.1': diff --git a/src/commands/skill.rs b/src/commands/skill.rs index cbaea070..1181ead1 100644 --- a/src/commands/skill.rs +++ b/src/commands/skill.rs @@ -777,6 +777,14 @@ fn run_update_inner(args: SkillUpdateArgs, project_root: PathBuf) -> Result<()> } } +#[derive(Debug, thiserror::Error)] +enum SkillInventoryError { + #[error("installed skill registry is invalid: {0:#}")] + Registry(#[source] anyhow::Error), + #[error("skill inventory could not be scanned: {0:#}")] + Scan(#[source] anyhow::Error), +} + #[derive(Debug, serde::Serialize)] struct SkillInventory { schema_version: u32, @@ -794,23 +802,67 @@ struct InventorySkill { state: String, } -fn skill_inventory(project_root: &Path) -> Result { +fn inventory_source_kind(entry: Option<®istry::SkillEntry>) -> &'static str { + let Some(entry) = entry else { + return "manual/untracked"; + }; + if entry + .plugin_owners + .as_ref() + .is_some_and(|owners| !owners.is_empty()) + || entry.plugin.is_some() + || entry.marketplace.is_some() + { + "plugin" + } else if entry.provider.is_some() || entry.source.is_some() { + "direct" + } else { + "unknown" + } +} + +fn skill_inventory( + project_root: &Path, +) -> std::result::Result { let skills_root = project_root.join(".agents/skills"); let registry_path = skills_root.join("registry.json"); let registry_data = if registry_path.exists() { - Some( - registry::read_registry(®istry_path) - .context("installed skill registry is invalid")?, - ) + Some(registry::read_registry(®istry_path).map_err(SkillInventoryError::Registry)?) } else { None }; let entries = registry_data.as_ref().and_then(|reg| reg.skills.as_ref()); + let skills_root_metadata = match std::fs::symlink_metadata(&skills_root) { + Ok(metadata) => Some(metadata), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + Err(error) => { + return Err(SkillInventoryError::Scan( + anyhow::Error::new(error).context(format!( + "failed to inspect skills directory {}", + skills_root.display() + )), + )); + } + }; + if skills_root_metadata.as_ref().is_some_and(|metadata| { + !metadata.is_dir() && !(metadata.file_type().is_symlink() && skills_root.is_dir()) + }) { + return Err(SkillInventoryError::Scan(anyhow::anyhow!( + "skills path is not a directory: {}", + skills_root.display() + ))); + } let mut found = std::collections::BTreeMap::::new(); let mut manifest_ids = std::collections::BTreeMap::>::new(); let mut diagnostics = Vec::new(); - if skills_root.is_dir() { - let mut items = std::fs::read_dir(&skills_root)?.collect::>>()?; + if skills_root_metadata.is_some() { + let directory = std::fs::read_dir(&skills_root) + .with_context(|| format!("failed to read skills directory {}", skills_root.display())) + .map_err(SkillInventoryError::Scan)?; + let mut items = directory + .collect::>>() + .context("failed to collect skill directory entries") + .map_err(SkillInventoryError::Scan)?; items.sort_by_key(|item| item.file_name()); for item in items { let name = item.file_name().to_string_lossy().into_owned(); @@ -819,7 +871,9 @@ fn skill_inventory(project_root: &Path) -> Result { } let path = item.path(); let relative = format!(".agents/skills/{name}"); - let metadata = std::fs::symlink_metadata(&path)?; + let metadata = std::fs::symlink_metadata(&path) + .with_context(|| format!("failed to inspect skill entry {}", path.display())) + .map_err(SkillInventoryError::Scan)?; let is_link = metadata.file_type().is_symlink(); if is_link && !path.exists() { diagnostics.push(format!("broken symlink: {relative}")); @@ -836,6 +890,9 @@ fn skill_inventory(project_root: &Path) -> Result { ); continue; } + if is_link { + continue; + } if !path.is_dir() { continue; } @@ -843,21 +900,7 @@ fn skill_inventory(project_root: &Path) -> Result { let manifest_exists = manifest_path.is_file(); let parsed = agentsync::skills::manifest::parse_skill_manifest(&manifest_path); let entry = entries.and_then(|all| all.get(&name)); - let source_kind = if entry.is_none() { - "manual/untracked" - } else if entry.is_some_and(|e| { - e.plugin_owners - .as_ref() - .is_some_and(|owners| !owners.is_empty()) - || e.plugin.is_some() - || e.marketplace.is_some() - }) { - "plugin" - } else if entry.is_some_and(|e| e.provider.is_some() || e.source.is_some()) { - "direct" - } else { - "unknown" - }; + let source_kind = inventory_source_kind(entry); let state = if parsed.is_err() { if manifest_exists { diagnostics.push(format!("malformed SKILL.md: {relative}")); @@ -891,7 +934,7 @@ fn skill_inventory(project_root: &Path) -> Result { .filter(|source| { source.starts_with("https://") || source.starts_with("git@") }) - .map(str::to_owned), + .map(agentsync::logging::redact_url), state: state.into(), }, ); @@ -916,16 +959,7 @@ fn skill_inventory(project_root: &Path) -> Result { InventorySkill { id: id.clone(), path: format!(".agents/skills/{id}"), - source_kind: if entry.plugin_owners.as_ref().is_some_and(|o| !o.is_empty()) - || entry.plugin.is_some() - { - "plugin" - } else if entry.provider.is_some() || entry.source.is_some() { - "direct" - } else { - "unknown" - } - .into(), + source_kind: inventory_source_kind(Some(entry)).into(), version: entry.version.clone(), provenance: entry .source @@ -933,7 +967,7 @@ fn skill_inventory(project_root: &Path) -> Result { .filter(|source| { source.starts_with("https://") || source.starts_with("git@") }) - .map(str::to_owned), + .map(agentsync::logging::redact_url), state: "missing".into(), }, ); @@ -969,18 +1003,26 @@ pub fn run_skill(cmd: SkillCommand, project_root: PathBuf) -> Result<()> { Ok(inventory) => inventory, Err(error) => { if json { + let diagnostic = match &error { + SkillInventoryError::Registry(_) => serde_json::json!({ + "code": "registry_invalid", + "message": "installed skill registry could not be read or parsed" + }), + SkillInventoryError::Scan(_) => serde_json::json!({ + "code": "inventory_unreadable", + "message": "skill inventory could not be scanned" + }), + }; println!( "{}", serde_json::json!({ "schema_version": 1, "skills": [], - "diagnostics": ["registry_invalid: installed skill registry could not be read or parsed"] + "diagnostics": [diagnostic] }) ); } - return Err(anyhow::anyhow!( - "installed skill registry is invalid: {error:#}" - )); + return Err(error.into()); } }; if json { @@ -1858,6 +1900,15 @@ mod tests { assert_eq!(inventory.skills[0].source_kind, "direct"); } + #[test] + fn inventory_classifies_registry_only_marketplace_entries_as_plugin() { + let entry: registry::SkillEntry = serde_json::from_value(serde_json::json!({ + "marketplace": "team-marketplace" + })) + .unwrap(); + assert_eq!(inventory_source_kind(Some(&entry)), "plugin"); + } + #[test] fn inventory_rejects_corrupted_registry() { let root = tempfile::TempDir::new().unwrap(); @@ -1891,6 +1942,24 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn inventory_skips_live_symlinked_skill_directories() { + use std::os::unix::fs::symlink; + + let root = tempfile::TempDir::new().unwrap(); + let target = root.path().join("external-skill"); + std::fs::create_dir_all(&target).unwrap(); + std::fs::write(target.join("SKILL.md"), "---\nname: external-skill\n---\n").unwrap(); + let skills = root.path().join(".agents/skills"); + std::fs::create_dir_all(&skills).unwrap(); + symlink(&target, skills.join("linked-skill")).unwrap(); + + let inventory = skill_inventory(root.path()).unwrap(); + assert!(inventory.skills.is_empty()); + assert!(inventory.diagnostics.is_empty()); + } + #[cfg(unix)] #[test] fn inventory_reports_broken_symlink() { diff --git a/tests/contracts/test_skill_list_output.rs b/tests/contracts/test_skill_list_output.rs index fe42e128..9370fe2b 100644 --- a/tests/contracts/test_skill_list_output.rs +++ b/tests/contracts/test_skill_list_output.rs @@ -44,7 +44,7 @@ fn skill_list_json_mixed_project_contract() { write_skill(&project, "malformed-skill", "no frontmatter"); std::fs::write( skills.join("registry.json"), - r#"{"schemaVersion":1,"skills":{"direct-skill":{"name":"direct-skill","version":"1.2.3","description":null,"provider":"source","source":"https://example.test/direct","installedAt":null,"files":null,"manifestHash":null},"plugin-skill":{"name":"plugin-skill","version":null,"description":null,"provider":null,"source":null,"installedAt":null,"files":null,"manifestHash":null,"pluginOwners":[{"marketplace":"m","plugin":"p","revision":"r"}]},"missing-skill":{"name":"missing-skill","version":null,"description":null,"provider":null,"source":null,"installedAt":null,"files":null,"manifestHash":null}}}"#, + r#"{"schemaVersion":1,"skills":{"direct-skill":{"name":"direct-skill","version":"1.2.3","description":null,"provider":"source","source":"https://user:secret@example.test/direct?token=abc","installedAt":null,"files":null,"manifestHash":null},"plugin-skill":{"name":"plugin-skill","version":null,"description":null,"provider":null,"source":null,"installedAt":null,"files":null,"manifestHash":null,"pluginOwners":[{"marketplace":"m","plugin":"p","revision":"r"}]},"missing-skill":{"name":"missing-skill","version":null,"description":null,"provider":null,"source":"https://user:secret@example.test/missing?token=abc","installedAt":null,"files":null,"manifestHash":null}}}"#, ) .unwrap(); write_skill(&project, "plugin-skill", "---\nname: plugin-skill\n---\n"); @@ -76,12 +76,27 @@ fn skill_list_json_mixed_project_contract() { assert_eq!(entries[0]["source_kind"], "direct"); assert_eq!(entries[0]["version"], "1.2.3"); assert_eq!(entries[0]["provenance"], "https://example.test/direct"); + assert!( + !entries[0]["provenance"] + .as_str() + .unwrap() + .contains("secret") + ); + assert!(!entries[0]["provenance"].as_str().unwrap().contains("token")); assert_eq!(entries[0]["state"], "installed"); assert_eq!(entries[1]["state"], "malformed"); assert_eq!(entries[1]["version"], serde_json::Value::Null); assert_eq!(entries[2]["source_kind"], "manual/untracked"); assert_eq!(entries[2]["provenance"], serde_json::Value::Null); assert_eq!(entries[3]["state"], "missing"); + assert_eq!(entries[3]["provenance"], "https://example.test/missing"); + assert!( + !entries[3]["provenance"] + .as_str() + .unwrap() + .contains("secret") + ); + assert!(!entries[3]["provenance"].as_str().unwrap().contains("token")); assert_eq!(entries[4]["source_kind"], "plugin"); assert!(!value["diagnostics"].as_array().unwrap().is_empty()); } @@ -113,6 +128,25 @@ fn skill_list_json_is_deterministic_and_read_only() { assert_eq!(before, after); } +#[test] +fn skill_list_json_scan_failure_has_distinct_diagnostic_and_preserves_error() { + let project = TempDir::new().unwrap(); + std::fs::create_dir_all(project.path().join(".agents")).unwrap(); + std::fs::write(project.path().join(".agents/skills"), "not a directory").unwrap(); + + let output = Command::new(binary()) + .args(["skill", "--project-root"]) + .arg(project.path()) + .args(["list", "--json"]) + .output() + .expect("run agentsync skill list"); + + assert!(!output.status.success()); + let value: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(value["diagnostics"][0]["code"], "inventory_unreadable"); + assert!(String::from_utf8_lossy(&output.stderr).contains("not a directory")); +} + #[test] fn skill_list_json_corrupt_registry_reports_redacted_diagnostic_and_fails() { let project = TempDir::new().unwrap(); @@ -131,9 +165,10 @@ fn skill_list_json_corrupt_registry_reports_redacted_diagnostic_and_fails() { let value: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); assert_eq!(value["schema_version"], 1); assert_eq!(value["skills"], serde_json::json!([])); + assert_eq!(value["diagnostics"][0]["code"], "registry_invalid"); assert_eq!( - value["diagnostics"][0], - "registry_invalid: installed skill registry could not be read or parsed" + value["diagnostics"][0]["message"], + "installed skill registry could not be read or parsed" ); assert!(!String::from_utf8_lossy(&output.stdout).contains("not json")); } diff --git a/website/docs/src/content/docs/guides/skills.mdx b/website/docs/src/content/docs/guides/skills.mdx index db70d17b..5aedab1b 100644 --- a/website/docs/src/content/docs/guides/skills.mdx +++ b/website/docs/src/content/docs/guides/skills.mdx @@ -260,7 +260,7 @@ Example `skill list --json` response: } ``` -`version` and `provenance` are `null` when the project registry cannot verify them. A skill directory that is not registered is reported as `manual/untracked`, and its `state` reflects the on-disk files: `installed`, `missing_manifest`, `malformed`, `duplicate_id`, or `broken_symlink`. A registry entry without files appears with `state: "missing"`. A corrupted `registry.json` fails the command with exit code `1` instead of returning an untrustworthy inventory. +`version` comes from the on-disk manifest first and falls back to the project registry; it is `null` only when neither provides a version. `provenance` is `null` unless the registry `source` uses `https://` or `git@`; accepted values have credentials and query tokens redacted. A skill directory that is not registered is reported as `manual/untracked`, and its `state` reflects the on-disk files: `installed`, `missing_manifest`, `malformed`, `duplicate_id`, or `broken_symlink`. A registry entry without files appears with `state: "missing"`. A corrupted `registry.json` fails the command with a `registry_invalid` diagnostic and exit code `1` instead of returning an untrustworthy inventory. Filesystem scan failures use `inventory_unreadable` and also return exit code `1`. Flags: - `--source` — optional for `skill install` and `skill update`. Use a local path, archive URL, or provider-specific identifier. From 4615ca58147e3e51f2c9cc381f1209c0a48adda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:02:25 +0200 Subject: [PATCH 2/5] feat: add read-only agent configuration inspect --- .../specs/resolved-agent-configuration/adr.md | 76 ++ src/commands/inspect.rs | 105 +++ src/commands/mod.rs | 1 + src/commands/status.rs | 449 +--------- src/inspect.rs | 821 ++++++++++++++++++ src/lib.rs | 2 + src/main.rs | 18 + src/plugins.rs | 65 ++ src/status.rs | 495 +++++++++++ tests/all_tests.rs | 2 + tests/contracts/test_inspect_output.rs | 290 +++++++ .../docs/src/content/docs/guides/inspect.mdx | 40 + .../docs/src/content/docs/reference/cli.mdx | 46 +- .../content/docs/reference/inspect-output.mdx | 58 ++ 14 files changed, 2024 insertions(+), 444 deletions(-) create mode 100644 .agents/sdd/specs/resolved-agent-configuration/adr.md create mode 100644 src/commands/inspect.rs create mode 100644 src/inspect.rs create mode 100644 src/status.rs create mode 100644 tests/contracts/test_inspect_output.rs create mode 100644 website/docs/src/content/docs/guides/inspect.mdx create mode 100644 website/docs/src/content/docs/reference/inspect-output.mdx diff --git a/.agents/sdd/specs/resolved-agent-configuration/adr.md b/.agents/sdd/specs/resolved-agent-configuration/adr.md new file mode 100644 index 00000000..56a85136 --- /dev/null +++ b/.agents/sdd/specs/resolved-agent-configuration/adr.md @@ -0,0 +1,76 @@ +# ADR: AgentSync-managed resolved configuration and inspection + +**Status:** Provisional — implemented for DALLAY-696; pending ratification through DALLAY-708. +**Date:** 2026-10-23 + +## Context + +AgentSync parses project declarations, projects targets, materializes symlinks, composes MCP server +configuration, and tracks plugin contributions. These stages are distinct. A configuration report +must not conflate declaration with planned output or observed filesystem state. Vendor runtime +configuration can also include global, user, organization, or session overlays that AgentSync does +not own. + +## Decision + +`ResolvedConfiguration` is a deterministic projection of **AgentSync-managed** configuration for +one configured assistant. It is not an authoritative provider-runtime snapshot. + +The JSON v1 envelope contains `schema_version`, `scope`, `agent`, `items`, `mcp`, `diagnostics`, and +`scope_notice`. It has no timestamp. Collections use stable ordering. Project-relative paths use `/` +separators. Each managed item carries declaring target, sync mode, source, destination, origin, +owner, provider-support status, projected state, observed state, and child observations where the +sync mode expands entries. + +Item states have these meanings: + +- `configured`: intent is declared, but a source is missing. +- `projected`: source exists; the destination is not observed as the planned output. +- `observed`: materialized output matches the projection. +- `conflicted`: observed destination differs from the projection or a declaration collision exists. +- `unsupported`: AgentSync has no projection for the capability. This does not assert that the + provider itself lacks that capability. + +`provider_support: "unknown"` is used for instruction, skill, and command targets unless a future +capability adapter can establish more. The inspector does not invent native support semantics. + +### MCP precedence + +1. Explicit `[mcp_servers.*]` declarations form the base set. +2. Plugin MCP contributions are namespaced as `plugin/{marketplace}/{plugin}/{server}` and included + only when listed in `plugins.allowed_mcp`. +3. A plugin contribution with a name already declared explicitly is accepted by `apply` only when + the server configurations are equal; a differing configuration is an error. +4. Disabled MCP servers are not generated. +5. The configured merge strategy determines how existing destination entries are merged or + overwritten. The report reads existing server names to explain destination interactions but never + emits unmanaged vendor values. + +### Read-only and offline boundary + +Inspection may read project config, target paths, MCP destination files, plugin lockfiles, and plugin +snapshots already present locally. It MUST NOT write files, restore or download snapshots, resolve +marketplace references, expand environment variables, launch provider CLIs, or start MCP servers. +An unavailable plugin snapshot produces an explicit diagnostic. + +### Redaction + +Environment and header values are never serialized, whether literal or `${ENV}` references. The +report contains key names and classifications only. Sensitive command arguments, URL userinfo, and +sensitive query values are redacted. Existing unmanaged MCP configuration contributes server names +only. The report does not read arbitrary instruction or skill file contents and is not a general +secret scanner. + +### Exit behavior + +Default mode returns `0` when it produces a report, including reports with diagnostics. `--strict` +returns `1` when any diagnostic has error severity. Clap usage errors retain exit code `2`; config +and runtime failures are non-zero. + +## Consequences + +- The model can be reused by later project profile and capability-adapter work. +- `status --json` remains unchanged; it consumes the same extracted materialization reader. +- Provider-native runtime settings remain explicitly unknown outside AgentSync's managed boundary. +- The exact state vocabulary, redaction matrix, and envelope remain provisional until DALLAY-708 and + DALLAY-709 are ratified. diff --git a/src/commands/inspect.rs b/src/commands/inspect.rs new file mode 100644 index 00000000..19db8f81 --- /dev/null +++ b/src/commands/inspect.rs @@ -0,0 +1,105 @@ +use agentsync::config::Config; +use agentsync::inspect::{DiagnosticSeverity, ResolvedConfiguration, resolve_configuration}; +use anyhow::Result; +use clap::Args; +use std::path::PathBuf; + +#[derive(Args, Debug)] +pub struct InspectArgs { + /// Assistant identifier from agentsync.toml (for example: opencode or codex). + #[arg(long)] + pub agent: String, + /// Output machine-readable JSON. + #[arg(long)] + pub json: bool, + /// Exit with code 1 when the report contains an error-severity diagnostic. + #[arg(long)] + pub strict: bool, +} + +pub fn run_inspect(args: InspectArgs, project_root: PathBuf) -> Result<()> { + let config_path = Config::find_config(&project_root)?; + let config = Config::load(&config_path)?; + let report = resolve_configuration(&config, &config_path, &args.agent)?; + if args.json { + println!("{}", serde_json::to_string(&report)?); + } else { + print_human(&report); + } + if args.strict + && report + .diagnostics + .iter() + .any(|diagnostic| diagnostic.severity == DiagnosticSeverity::Error) + { + anyhow::bail!("inspect found error-severity diagnostics"); + } + Ok(()) +} + +fn print_human(report: &ResolvedConfiguration) { + println!("AgentSync-managed configuration for {}", report.agent); + println!("Scope: {}", report.scope); + for item in &report.items { + println!("\n{} [{}]", item.kind, item.state.state_name()); + println!(" source: {}", item.source); + println!(" target: {} ({})", item.target, item.sync_mode); + println!(" destination: {}", item.destination); + println!(" owner: {} ({})", item.owner, item.origin); + println!(" provider support: {}", item.provider_support); + for child in &item.children { + println!( + " child: {} -> {} [{}]", + child.source, + child.destination, + child.state.state_name() + ); + } + } + println!( + "\nMCP: {} (enabled: {}, merge: {})", + report.mcp.capability, report.mcp.enabled, report.mcp.merge_strategy + ); + if let Some(destination) = &report.mcp.destination { + println!(" destination: {destination}"); + } + for server in &report.mcp.servers { + println!( + " {} [{}; {}; {}]", + server.name, + server.origin, + server.state.state_name(), + if server.disabled { + "disabled" + } else if server.included { + "included" + } else { + "not included" + } + ); + } + for diagnostic in &report.diagnostics { + println!("\n{:?}: {}", diagnostic.code, diagnostic.message); + } + println!("\n{}", report.scope_notice); +} + +trait StateName { + fn state_name(&self) -> &'static str; +} +impl StateName for agentsync::inspect::ItemState { + fn state_name(&self) -> &'static str { + match self { + Self::Configured => "configured", + Self::Projected => "projected", + Self::Observed => "observed", + Self::Unsupported => "unsupported", + Self::Conflicted => "conflicted", + } + } +} +impl StateName for agentsync::inspect::McpServerReport { + fn state_name(&self) -> &'static str { + self.state.state_name() + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 4912d49d..c963b6b0 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -2,6 +2,7 @@ pub mod dev_bench; pub mod doctor; #[cfg(test)] mod doctor_tests; +pub mod inspect; pub mod plugin; pub mod skill; pub mod status; diff --git a/src/commands/status.rs b/src/commands/status.rs index bd31ae8d..7136518b 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -1,13 +1,14 @@ use crate::output::{HumanFormatter, LabelKind, OutputMode}; -use agentsync::config::{SyncType, TargetConfig}; -use agentsync::skills_layout::detect_skills_mode_mismatch; -use agentsync::{Linker, linker::SymlinkContentsChildExpectation}; +use agentsync::Linker; use anyhow::Result; use clap::Args; -use serde::Serialize; -use std::collections::HashMap; use std::path::{Path, PathBuf}; +pub use agentsync::status::{ + DestinationKind, StatusEntry, StatusIssue, StatusIssueKind, collect_status_entries, + collect_status_hints, entry_is_problematic, +}; + /// Arguments for the status command #[derive(Args, Debug)] pub struct StatusArgs { @@ -16,133 +17,6 @@ pub struct StatusArgs { pub json: bool, } -#[derive(Serialize, Debug, Clone, Copy, PartialEq, Eq)] -#[serde(rename_all = "kebab-case")] -pub enum DestinationKind { - Missing, - Symlink, - Directory, - File, - Other, -} - -#[derive(Serialize, Debug, Clone, Copy, PartialEq, Eq)] -#[serde(rename_all = "kebab-case")] -pub enum StatusIssueKind { - MissingDestination, - InvalidDestinationType, - MissingExpectedChild, - ChildNotSymlink, - IncorrectLinkTarget, - MissingExpectedSource, -} - -#[derive(Serialize, Debug, Clone, PartialEq, Eq)] -pub struct StatusIssue { - pub kind: StatusIssueKind, - pub path: String, - pub expected: Option, - pub actual: Option, -} - -#[derive(Serialize, Debug, Clone, PartialEq, Eq)] -pub struct StatusChildEntry { - pub path: String, - pub exists: bool, - pub is_symlink: bool, - pub points_to: Option, - pub expected_source: String, -} - -#[derive(Serialize, Debug, Clone)] -pub struct StatusEntry { - pub destination: String, - pub sync_type: String, - pub destination_kind: DestinationKind, - pub exists: bool, - pub is_symlink: bool, - pub points_to: Option, - pub expected_source: Option, - pub issues: Vec, - #[serde(skip_serializing_if = "Option::is_none")] - pub managed_children: Option>, -} - -pub fn collect_status_entries(linker: &Linker, config_path: &Path) -> Result> { - let mut entries = Vec::new(); - let source_dir = linker.config().source_dir(config_path); - - for (agent_name, agent) in &linker.config().agents { - if !agent.enabled { - continue; - } - - for (target_name, target) in &agent.targets { - if target.sync_type == SyncType::ModuleMap { - for mapping in &target.mappings { - let filename = - agentsync::config::resolve_module_map_filename(mapping, agent_name); - let destination = linker - .project_root() - .join(&mapping.destination) - .join(&filename); - let source_path = source_dir.join(&mapping.source); - let expected_source = linker.expected_source_path(&source_path, target); - - entries.push(validate_symlink_entry( - destination, - expected_source, - SyncType::ModuleMap, - )); - } - continue; - } - - let destination = linker.project_root().join(&target.destination); - // NestedGlob resolves `source` relative to the project root (matching apply and - // doctor); all other sync types resolve it relative to source_dir. - let source_path = if target.sync_type == SyncType::NestedGlob { - linker.project_root().join(&target.source) - } else { - source_dir.join(&target.source) - }; - - let entry = match target.sync_type { - SyncType::Symlink => validate_symlink_entry( - destination, - linker.expected_source_path(&source_path, target), - SyncType::Symlink, - ), - SyncType::SymlinkContents => validate_symlink_contents_entry( - linker, - destination, - &source_path, - target, - agent_name, - detect_skills_mode_mismatch( - linker.project_root(), - &source_path, - agent_name, - target_name, - target, - ) - .is_some(), - )?, - SyncType::NestedGlob => validate_symlink_entry( - destination, - linker.expected_source_path(&source_path, target), - SyncType::NestedGlob, - ), - SyncType::ModuleMap => unreachable!(), - }; - - entries.push(entry); - } - } - - Ok(entries) -} - pub fn run_status(json: bool, project_root: PathBuf) -> Result<()> { let config_path = agentsync::config::Config::find_config(&project_root)?; let config = agentsync::config::Config::load(&config_path)?; @@ -191,43 +65,6 @@ pub fn run_status(json: bool, project_root: PathBuf) -> Result<()> { Ok(()) } -pub fn collect_status_hints(linker: &Linker, config_path: &Path) -> HashMap { - let mut hints = HashMap::new(); - let source_dir = linker.config().source_dir(config_path); - - for (agent_name, agent) in &linker.config().agents { - if !agent.enabled { - continue; - } - - for (target_name, target) in &agent.targets { - let expected_source = source_dir.join(&target.source); - if let Some(mismatch) = detect_skills_mode_mismatch( - linker.project_root(), - &expected_source, - agent_name, - target_name, - target, - ) { - hints.insert( - linker - .project_root() - .join(&target.destination) - .display() - .to_string(), - mismatch.status_hint(), - ); - } - } - } - - hints -} - -pub fn entry_is_problematic(e: &StatusEntry) -> bool { - !e.issues.is_empty() -} - pub(crate) fn render_status_entry(entry: &StatusEntry, formatter: &HumanFormatter) -> Vec { if entry.issues.is_empty() { return render_ok_line(entry, formatter); @@ -404,277 +241,3 @@ pub(crate) fn render_status_summary(problems: usize, formatter: &HumanFormatter) ) } } - -fn validate_symlink_entry( - destination: PathBuf, - expected_source: Option, - sync_type: SyncType, -) -> StatusEntry { - let metadata = std::fs::symlink_metadata(&destination).ok(); - let destination_kind = destination_kind(metadata.as_ref()); - let exists = metadata.is_some(); - let is_symlink = destination_kind == DestinationKind::Symlink; - let points_to = read_link_target(&destination, is_symlink); - let expected_source_string = expected_source.as_ref().map(|path| display_path(path)); - let mut issues = Vec::new(); - - match expected_source.as_ref() { - Some(expected) => { - if !exists { - issues.push(StatusIssue { - kind: StatusIssueKind::MissingDestination, - path: display_path(&destination), - expected: Some(display_path(expected)), - actual: None, - }); - } else if !is_symlink { - issues.push(StatusIssue { - kind: StatusIssueKind::InvalidDestinationType, - path: display_path(&destination), - expected: Some("symlink".to_string()), - actual: Some(destination_kind_label(destination_kind).to_string()), - }); - } else if let Some(actual) = points_to.as_ref() { - if !paths_match(&destination, Path::new(actual), expected) { - issues.push(StatusIssue { - kind: StatusIssueKind::IncorrectLinkTarget, - path: display_path(&destination), - expected: Some(display_path(expected)), - actual: Some(actual.clone()), - }); - } - } else { - issues.push(StatusIssue { - kind: StatusIssueKind::IncorrectLinkTarget, - path: display_path(&destination), - expected: Some(display_path(expected)), - actual: None, - }); - } - } - None => { - issues.push(StatusIssue { - kind: StatusIssueKind::MissingExpectedSource, - path: display_path(&destination), - expected: None, - actual: points_to.clone(), - }); - } - } - - StatusEntry { - destination: display_path(&destination), - sync_type: sync_type_label(sync_type).to_string(), - destination_kind, - exists, - is_symlink, - points_to, - expected_source: expected_source_string, - issues, - managed_children: None, - } -} - -fn collect_child_issues(child_status: &StatusChildEntry, issues: &mut Vec) { - if !child_status.exists { - issues.push(StatusIssue { - kind: StatusIssueKind::MissingExpectedChild, - path: child_status.path.clone(), - expected: Some(child_status.expected_source.clone()), - actual: None, - }); - } else if !child_status.is_symlink { - issues.push(StatusIssue { - kind: StatusIssueKind::ChildNotSymlink, - path: child_status.path.clone(), - expected: Some(child_status.expected_source.clone()), - actual: None, - }); - } else if let Some(actual) = child_status.points_to.as_ref() - && !paths_match( - Path::new(&child_status.path), - Path::new(actual), - Path::new(&child_status.expected_source), - ) - { - issues.push(StatusIssue { - kind: StatusIssueKind::IncorrectLinkTarget, - path: child_status.path.clone(), - expected: Some(child_status.expected_source.clone()), - actual: Some(actual.clone()), - }); - } -} - -fn validate_children_directory( - destination: &Path, - destination_kind: DestinationKind, - exists: bool, - children: Vec, - issues: &mut Vec, - managed_children: &mut Vec, -) { - if !exists { - issues.push(StatusIssue { - kind: StatusIssueKind::MissingDestination, - path: display_path(destination), - expected: Some("directory".to_string()), - actual: None, - }); - } else if destination_kind != DestinationKind::Directory { - issues.push(StatusIssue { - kind: StatusIssueKind::InvalidDestinationType, - path: display_path(destination), - expected: Some("directory".to_string()), - actual: Some(destination_kind_label(destination_kind).to_string()), - }); - } else { - for child in children { - let child_status = validate_symlink_contents_child(destination, child); - collect_child_issues(&child_status, issues); - managed_children.push(child_status); - } - } -} - -fn validate_symlink_contents_entry( - linker: &Linker, - destination: PathBuf, - source_path: &Path, - target: &TargetConfig, - agent_name: &str, - allow_skills_symlink_hint: bool, -) -> Result { - let metadata = std::fs::symlink_metadata(&destination).ok(); - let destination_kind = destination_kind(metadata.as_ref()); - let exists = metadata.is_some(); - let is_symlink = destination_kind == DestinationKind::Symlink; - let points_to = read_link_target(&destination, is_symlink); - let child_expectations = - linker.symlink_contents_expected_children(source_path, target, agent_name)?; - let mut issues = Vec::new(); - let mut managed_children = Vec::new(); - - if let Some(children) = child_expectations { - if allow_skills_symlink_hint && is_symlink { - return Ok(StatusEntry { - destination: display_path(&destination), - sync_type: sync_type_label(SyncType::SymlinkContents).to_string(), - destination_kind, - exists, - is_symlink, - points_to, - expected_source: None, - issues, - managed_children: Some(managed_children), - }); - } - - validate_children_directory( - &destination, - destination_kind, - exists, - children, - &mut issues, - &mut managed_children, - ); - } else { - issues.push(StatusIssue { - kind: StatusIssueKind::MissingExpectedSource, - path: display_path(source_path), - expected: None, - actual: Some(display_path(&destination)), - }); - } - - Ok(StatusEntry { - destination: display_path(&destination), - sync_type: sync_type_label(SyncType::SymlinkContents).to_string(), - destination_kind, - exists, - is_symlink, - points_to, - expected_source: None, - issues, - managed_children: Some(managed_children), - }) -} - -fn validate_symlink_contents_child( - destination: &Path, - expectation: SymlinkContentsChildExpectation, -) -> StatusChildEntry { - let child_path = destination.join(&expectation.name); - let metadata = std::fs::symlink_metadata(&child_path).ok(); - let exists = metadata.is_some(); - let is_symlink = metadata - .as_ref() - .map(|metadata| metadata.file_type().is_symlink()) - .unwrap_or(false); - - StatusChildEntry { - path: display_path(&child_path), - exists, - is_symlink, - points_to: read_link_target(&child_path, is_symlink), - expected_source: display_path(&expectation.expected_source_path), - } -} - -fn read_link_target(path: &Path, is_symlink: bool) -> Option { - if !is_symlink { - return None; - } - - std::fs::read_link(path) - .ok() - .map(|link| link.display().to_string()) -} - -fn display_path(path: &Path) -> String { - path.display().to_string() -} - -fn destination_kind(metadata: Option<&std::fs::Metadata>) -> DestinationKind { - match metadata { - None => DestinationKind::Missing, - Some(metadata) if metadata.file_type().is_symlink() => DestinationKind::Symlink, - Some(metadata) if metadata.is_dir() => DestinationKind::Directory, - Some(metadata) if metadata.is_file() => DestinationKind::File, - Some(_) => DestinationKind::Other, - } -} - -fn destination_kind_label(kind: DestinationKind) -> &'static str { - match kind { - DestinationKind::Missing => "missing", - DestinationKind::Symlink => "symlink", - DestinationKind::Directory => "directory", - DestinationKind::File => "file", - DestinationKind::Other => "other", - } -} - -fn sync_type_label(sync_type: SyncType) -> &'static str { - match sync_type { - SyncType::Symlink => "symlink", - SyncType::SymlinkContents => "symlink-contents", - SyncType::NestedGlob => "nested-glob", - SyncType::ModuleMap => "module-map", - } -} - -fn paths_match(destination: &Path, points_to: &Path, expected: &Path) -> bool { - let actual_path = if points_to.is_absolute() { - points_to.to_path_buf() - } else { - destination - .parent() - .unwrap_or_else(|| Path::new(".")) - .join(points_to) - }; - - let expected_canon = std::fs::canonicalize(expected).unwrap_or_else(|_| expected.to_path_buf()); - let actual_canon = std::fs::canonicalize(&actual_path).unwrap_or(actual_path); - expected_canon == actual_canon -} diff --git a/src/inspect.rs b/src/inspect.rs new file mode 100644 index 00000000..683c19c2 --- /dev/null +++ b/src/inspect.rs @@ -0,0 +1,821 @@ +//! Deterministic projection of configuration owned by AgentSync. +//! +//! This is not a snapshot of provider runtime state. Global, organization, +//! account, session, and other vendor-managed overlays are outside this model. + +use crate::Linker; +use crate::config::{Config, McpServerConfig, SyncType}; +use crate::mcp::McpAgent; +use crate::plugins::PluginLock; +use crate::status::{ + AgentStatusEntry, DestinationKind, StatusIssueKind, collect_agent_status_entries, +}; +use anyhow::Result; +use serde::Serialize; +use std::collections::BTreeMap; +use std::path::{Component, Path}; + +pub const INSPECT_SCHEMA_VERSION: u32 = 1; +pub const INSPECT_SCOPE: &str = "agentsync-managed"; +pub const SCOPE_NOTICE: &str = "This report describes only AgentSync-managed configuration. It is not a claim about full native runtime-effective settings; global, user, organization, and session overlays outside AgentSync's control are unknown."; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum ItemState { + Configured, + Projected, + Observed, + Unsupported, + Conflicted, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum DiagnosticSeverity { + Error, + Warning, + Info, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum DiagnosticCode { + AgentNotConfigured, + MissingSource, + DriftedSymlink, + BrokenSymlink, + DestinationCollision, + McpNameCollision, + UnsupportedTarget, + MissingPluginSnapshot, + DisabledMcpServer, + PendingPluginApproval, + UnsupportedPluginComponent, + ExistingMcpServer, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct Diagnostic { + pub code: DiagnosticCode, + pub severity: DiagnosticSeverity, + pub target: Option, + pub path: Option, + pub message: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ObservedState { + pub exists: bool, + pub kind: String, + pub points_to: Option, + pub matches_projection: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ManagedChild { + pub source: String, + pub destination: String, + pub state: ItemState, + pub observed: ObservedState, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ManagedItem { + pub kind: String, + pub target: String, + pub sync_mode: String, + pub source: String, + pub destination: String, + pub origin: String, + pub owner: String, + /// `unknown` means the provider-native semantics are not asserted here. + pub provider_support: String, + pub state: ItemState, + pub observed: ObservedState, + pub children: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct McpServerReport { + pub name: String, + pub origin: String, + pub owner: Option, + pub disabled: bool, + pub included: bool, + pub state: ItemState, + pub transport: Option, + pub command: Option, + pub args: Vec, + pub url: Option, + /// Environment variable names and value classifications only. Values are never emitted. + pub environment: BTreeMap, + /// Header names and value classifications only. Values are never emitted. + pub headers: BTreeMap, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct McpReport { + pub capability: String, + pub enabled: bool, + pub merge_strategy: String, + pub destination: Option, + pub servers: Vec, + pub existing_server_names: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ResolvedConfiguration { + pub schema_version: u32, + pub scope: String, + pub agent: String, + pub items: Vec, + pub mcp: McpReport, + pub diagnostics: Vec, + pub scope_notice: String, +} + +/// Resolve one configured assistant without writing files, expanding environment variables, +/// downloading plugin snapshots, or invoking child processes. +pub fn resolve_configuration( + config: &Config, + config_path: &Path, + agent_id: &str, +) -> Result { + let requested = agent_id.trim().to_ascii_lowercase(); + let agent_name = config + .agents + .keys() + .find(|name| name.to_ascii_lowercase() == requested) + .cloned() + .unwrap_or_else(|| requested.clone()); + let project_root = Config::project_root(config_path); + let linker = Linker::new(config.clone(), config_path.to_path_buf()); + let mut diagnostics = Vec::new(); + let Some(agent) = config.agents.get(&agent_name) else { + diagnostics.push(Diagnostic { code: DiagnosticCode::AgentNotConfigured, severity: DiagnosticSeverity::Warning, target: None, path: None, message: "Agent is not configured in agentsync.toml; AgentSync-managed configuration is unknown.".into() }); + return Ok(ResolvedConfiguration { + schema_version: INSPECT_SCHEMA_VERSION, + scope: INSPECT_SCOPE.into(), + agent: agent_name, + items: vec![], + mcp: empty_mcp_report(), + diagnostics, + scope_notice: SCOPE_NOTICE.into(), + }); + }; + + let status = collect_agent_status_entries(&linker, config_path)?; + let source_root = config.source_dir(config_path); + let mut items = Vec::new(); + let mut destinations = BTreeMap::::new(); + if agent.enabled { + for (target_name, target) in &agent.targets { + let kind = classify_target(target_name); + let source = if target.sync_type == SyncType::NestedGlob { + project_root.join(&target.source) + } else { + source_root.join(&target.source) + }; + let destination = project_root.join(&target.destination); + let portable_source = portable_path(&project_root, &source); + let portable_destination = portable_path(&project_root, &destination); + if let Some(previous) = + destinations.insert(portable_destination.clone(), target_name.clone()) + { + diagnostics.push(Diagnostic { + code: DiagnosticCode::DestinationCollision, + severity: DiagnosticSeverity::Error, + target: Some(target_name.clone()), + path: Some(portable_destination.clone()), + message: format!( + "Targets {previous} and {target_name} project to the same destination." + ), + }); + } + let matched = status + .iter() + .filter(|entry| entry.agent == agent_name && entry.target == *target_name) + .collect::>(); + let observed = observed_for(&matched); + let source_exists = source.exists(); + let destination_occupied = target.sync_type == SyncType::Symlink + && std::fs::symlink_metadata(&destination).is_ok(); + let mut state = if !source_exists { + ItemState::Configured + } else if observed.matches_projection == Some(true) { + ItemState::Observed + } else if observed.exists || destination_occupied { + ItemState::Conflicted + } else { + ItemState::Projected + }; + if !source_exists { + diagnostics.push(Diagnostic { + code: DiagnosticCode::MissingSource, + severity: DiagnosticSeverity::Error, + target: Some(target_name.clone()), + path: Some(portable_source.clone()), + message: "Configured source does not exist.".into(), + }); + } + for entry in &matched { + diagnostics.extend(status_diagnostics(entry, target_name, &project_root)); + if entry.entry.destination_kind == DestinationKind::Symlink + && entry.entry.points_to.as_deref().is_some_and(|target| { + let target = Path::new(target); + let resolved = if target.is_absolute() { + target.to_path_buf() + } else { + Path::new(&entry.entry.destination) + .parent() + .unwrap_or(Path::new(".")) + .join(target) + }; + !resolved.exists() + }) + { + diagnostics.push(Diagnostic { + code: DiagnosticCode::BrokenSymlink, + severity: DiagnosticSeverity::Error, + target: Some(target_name.clone()), + path: Some(portable_destination.clone()), + message: "Destination is a broken or unreadable symlink.".into(), + }); + state = ItemState::Conflicted; + } + } + let mut children = Vec::new(); + if let Some(entry) = matched + .iter() + .find(|entry| entry.entry.managed_children.is_some()) + && let Some(entry) = entry.entry.managed_children.as_ref() + { + for child in entry { + let child_source = + portable_path(&project_root, Path::new(&child.expected_source)); + let child_destination = portable_path(&project_root, Path::new(&child.path)); + let child_matches = child.exists + && child.is_symlink + && child.points_to.as_deref().is_some_and(|actual| { + same_link_target( + Path::new(&child.path), + Path::new(actual), + Path::new(&child.expected_source), + ) + }); + let child_state = if !child.exists { + ItemState::Projected + } else if child_matches { + ItemState::Observed + } else { + ItemState::Conflicted + }; + children.push(ManagedChild { + source: child_source, + destination: child_destination, + state: child_state, + observed: ObservedState { + exists: child.exists, + kind: if child.is_symlink { + "symlink" + } else if child.exists { + "other" + } else { + "missing" + } + .into(), + points_to: child + .points_to + .as_deref() + .map(|path| portable_path(&project_root, Path::new(path))), + matches_projection: Some(child_matches), + }, + }); + } + } + items.push(ManagedItem { + kind, + target: target_name.clone(), + sync_mode: sync_type_name(target.sync_type).into(), + source: portable_source, + destination: portable_destination, + origin: "config".into(), + owner: "agentsync".into(), + provider_support: "unknown".into(), + state, + observed, + children, + }); + } + } + items.sort_by(|a, b| { + (&a.kind, &a.target, &a.source, &a.destination).cmp(&( + &b.kind, + &b.target, + &b.source, + &b.destination, + )) + }); + + let mcp = resolve_mcp( + config, + config_path, + &agent_name, + &project_root, + &mut diagnostics, + )?; + diagnostics.sort_by(|a, b| { + (&a.code, &a.target, &a.path, &a.message).cmp(&(&b.code, &b.target, &b.path, &b.message)) + }); + Ok(ResolvedConfiguration { + schema_version: INSPECT_SCHEMA_VERSION, + scope: INSPECT_SCOPE.into(), + agent: agent_name, + items, + mcp, + diagnostics, + scope_notice: SCOPE_NOTICE.into(), + }) +} + +fn empty_mcp_report() -> McpReport { + McpReport { + capability: "not-configured".into(), + enabled: false, + merge_strategy: "merge".into(), + destination: None, + servers: vec![], + existing_server_names: vec![], + } +} + +fn resolve_mcp( + config: &Config, + config_path: &Path, + agent_id: &str, + root: &Path, + diagnostics: &mut Vec, +) -> Result { + let Some(agent) = McpAgent::from_id(agent_id) else { + if config.mcp.enabled + && (!config.mcp_servers.is_empty() || !config.plugins.selections.is_empty()) + { + diagnostics.push(Diagnostic { code: DiagnosticCode::UnsupportedTarget, severity: DiagnosticSeverity::Warning, target: Some("mcp".into()), path: None, message: "This assistant has no AgentSync MCP formatter; configured MCP servers are not projected for it.".into() }); + } + return Ok(McpReport { + capability: if config.mcp.enabled + && (!config.mcp_servers.is_empty() || !config.plugins.selections.is_empty()) + { + "unsupported" + } else { + "not-configured" + } + .into(), + enabled: config.mcp.enabled, + merge_strategy: match config.mcp.merge_strategy { + crate::config::McpMergeStrategy::Merge => "merge", + crate::config::McpMergeStrategy::Overwrite => "overwrite", + } + .into(), + destination: None, + servers: vec![], + existing_server_names: vec![], + }); + }; + let Some(path) = agent.resolved_config_path(root) else { + return Ok(empty_mcp_report()); + }; + let dest = portable_path(root, &path); + let existing_content = std::fs::read_to_string(&path).ok(); + let existing_servers = if let Some(content) = existing_content.as_deref() { + agent + .formatter() + .parse_existing(content) + .unwrap_or_default() + } else { + BTreeMap::new() + }; + let existing_names = existing_servers.keys().cloned().collect::>(); + let mut servers = Vec::new(); + if !config.mcp.enabled + && (!config.mcp_servers.is_empty() || !config.plugins.selections.is_empty()) + { + diagnostics.push(Diagnostic { + code: DiagnosticCode::DisabledMcpServer, + severity: DiagnosticSeverity::Info, + target: Some("mcp".into()), + path: Some(dest.clone()), + message: "MCP generation is disabled by [mcp].enabled; configured declarations are not projected.".into(), + }); + } + if config.mcp.enabled { + for (name, server) in &config.mcp_servers { + let existing_equal = existing_servers + .get(name) + .is_some_and(|value| existing_server_matches(value, server)); + let mut server_report = + mcp_server_report(name, server, "config", None, existing_equal, root, &dest); + server_report.state = if server.disabled { + ItemState::Configured + } else if existing_equal { + ItemState::Observed + } else { + ItemState::Projected + }; + servers.push(server_report); + if existing_servers.contains_key(name) && !existing_equal && !server.disabled { + diagnostics.push(Diagnostic { + code: DiagnosticCode::ExistingMcpServer, + severity: DiagnosticSeverity::Warning, + target: Some("mcp".into()), + path: Some(dest.clone()), + message: format!("Managed server {name} replaces a differing destination entry under the current merge rules."), + }); + } + if server.disabled { + diagnostics.push(Diagnostic { + code: DiagnosticCode::DisabledMcpServer, + severity: DiagnosticSeverity::Info, + target: Some("mcp".into()), + path: Some(dest.clone()), + message: format!( + "Explicit MCP server {name} is disabled and is not generated." + ), + }); + } + } + let plugins = &config.plugins; + if plugins.enabled && !plugins.selections.is_empty() { + let manager = crate::plugins::PluginManager::new( + root.to_path_buf(), + config_path.to_path_buf(), + plugins.clone(), + ); + match manager.inspect_status_report_read_only() { + Ok(report) => { + for item in report.servers { + let approved = + matches!(item.approval, crate::plugins::PluginMcpApproval::Allowed); + let collision = config + .mcp_servers + .get(&item.name) + .is_some_and(|explicit| explicit != &item.server); + if collision { + diagnostics.push(Diagnostic { code: DiagnosticCode::McpNameCollision, severity: DiagnosticSeverity::Error, target: Some("mcp".into()), path: Some(dest.clone()), message: format!("Explicit MCP declaration and plugin contribution collide at {}; apply rejects differing definitions.", item.name) }); + } + let existing_equal = existing_servers + .get(&item.name) + .is_some_and(|value| existing_server_matches(value, &item.server)); + let mut server_report = mcp_server_report( + &item.name, + &item.server, + "plugin", + Some(plugin_owner(&item.name)), + existing_equal, + root, + &dest, + ); + server_report.included = approved && !collision && !item.server.disabled; + server_report.state = if item.server.disabled || !approved { + ItemState::Configured + } else if existing_equal { + ItemState::Observed + } else { + ItemState::Projected + }; + servers.push(server_report); + if !approved { + diagnostics.push(Diagnostic { code: DiagnosticCode::PendingPluginApproval, severity: DiagnosticSeverity::Warning, target: Some("mcp".into()), path: Some(dest.clone()), message: format!("Plugin MCP server {} is not approved by plugins.allowed_mcp and is not included.", item.name) }); + } + } + } + Err(error) => { + let lock_path = manager.lock_path()?; + let lock = PluginLock::load(&lock_path).ok(); + let missing_snapshot = lock.as_ref().is_some_and(|lock| { + lock.plugins.values().any(|plugin| { + plugin.source.kind == crate::plugins::LockedSourceKind::Git + }) + }); + diagnostics.push(Diagnostic { code: if missing_snapshot { DiagnosticCode::MissingPluginSnapshot } else { DiagnosticCode::UnsupportedPluginComponent }, severity: DiagnosticSeverity::Warning, target: Some("plugins".into()), path: Some(portable_path(root, &lock_path)), message: if missing_snapshot { "A required local plugin snapshot is unavailable; plugin MCP contributions are unknown.".into() } else { format!("Plugin contributions could not be resolved offline: {}", error.to_string().split(':').next().unwrap_or("plugin metadata error")) } }); + } + } + } + } + servers.sort_by(|a, b| a.name.cmp(&b.name)); + let existing_unmanaged = existing_names + .into_iter() + .filter(|name| { + config.mcp.merge_strategy == crate::config::McpMergeStrategy::Merge + && !servers.iter().any(|server| server.name == *name) + }) + .collect::>(); + for name in &existing_unmanaged { + diagnostics.push(Diagnostic { code: DiagnosticCode::ExistingMcpServer, severity: DiagnosticSeverity::Info, target: Some("mcp".into()), path: Some(dest.clone()), message: format!("Existing destination contains unmanaged MCP server {name}; merge preserves it unless an AgentSync server with the same name replaces it." ) }); + } + if !config.mcp.enabled { + servers.clear(); + } + Ok(McpReport { + capability: if config.mcp.enabled + && (!config.mcp_servers.is_empty() || !config.plugins.selections.is_empty()) + { + "managed" + } else { + "not-configured" + } + .into(), + enabled: config.mcp.enabled, + merge_strategy: match config.mcp.merge_strategy { + crate::config::McpMergeStrategy::Merge => "merge", + crate::config::McpMergeStrategy::Overwrite => "overwrite", + } + .into(), + destination: Some(dest), + servers, + existing_server_names: existing_unmanaged, + }) +} + +fn existing_server_matches(value: &serde_json::Value, server: &McpServerConfig) -> bool { + let Some(object) = value.as_object() else { + return false; + }; + let field_matches = |key: &str, expected: Option| match expected { + Some(expected) => object.get(key) == Some(&expected), + None => object.get(key).is_none_or(serde_json::Value::is_null), + }; + field_matches( + "command", + server.command.clone().map(serde_json::Value::String), + ) && field_matches("args", Some(serde_json::json!(server.args))) + && field_matches("url", server.url.clone().map(serde_json::Value::String)) + && field_matches("env", Some(serde_json::json!(server.env))) + && field_matches("headers", Some(serde_json::json!(server.headers))) + && field_matches( + "type", + server.transport_type.clone().map(serde_json::Value::String), + ) +} + +fn mcp_server_report( + name: &str, + server: &McpServerConfig, + origin: &str, + owner: Option, + exists: bool, + root: &Path, + destination: &str, +) -> McpServerReport { + let mut report = McpServerReport { + name: name.into(), + origin: origin.into(), + owner, + disabled: server.disabled, + included: !server.disabled, + state: if server.disabled { + ItemState::Configured + } else if exists { + ItemState::Observed + } else { + ItemState::Projected + }, + transport: server.transport_type.clone(), + command: server.command.as_deref().map(|command| { + redact_args(&[command.to_string()]) + .into_iter() + .next() + .unwrap_or_default() + }), + args: redact_args(&server.args), + url: server.url.as_deref().map(redact_url), + environment: classify_values(&server.env), + headers: classify_values(&server.headers), + }; + if exists && !server.disabled { + report.state = ItemState::Observed; + } + let _ = (root, destination); + report +} + +fn classify_values(values: &BTreeMap) -> BTreeMap { + values + .iter() + .map(|(key, value)| { + ( + key.clone(), + if is_environment_reference(value) { + "environment-reference".into() + } else if is_sensitive_key(key) { + "redacted-literal".into() + } else { + "literal-redacted".into() + }, + ) + }) + .collect() +} +fn is_sensitive_key(key: &str) -> bool { + let key = key.to_ascii_lowercase().replace(['-', '.'], "_"); + [ + "token", + "secret", + "password", + "passwd", + "credential", + "api_key", + "apikey", + "auth", + "cookie", + "credential", + "private_key", + "client_secret", + ] + .iter() + .any(|part| key.contains(part)) +} +fn is_environment_reference(value: &str) -> bool { + let value = value.trim(); + value.starts_with("${") + && value.ends_with('}') + && value.len() > 3 + && value[2..value.len() - 1] + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_') +} +fn redact_inline_secret(value: &str) -> String { + if let Some((key, _)) = value.split_once('=') + && is_sensitive_key(key.trim().trim_start_matches('-')) + { + return format!("{key}="); + } + if let Some((key, _)) = value.split_once(':') + && is_sensitive_key(key.trim().trim_start_matches('-')) + { + return format!("{key}: "); + } + value.to_string() +} + +fn redact_args(args: &[String]) -> Vec { + let mut redact_next = false; + args.iter() + .map(|arg| { + if redact_next { + redact_next = false; + return "".into(); + } + let flag = arg.trim_start_matches('-'); + if flag.starts_with("--") || arg.starts_with('-') { + let (key, value) = flag.split_once('=').unwrap_or((flag, "")); + if is_sensitive_key(key) && !value.is_empty() { + return format!("--{key}="); + } + if let Some((key, _)) = flag.split_once(':') + && is_sensitive_key(key) + { + return format!("--{key}:"); + } + redact_next = is_sensitive_key(key) && value.is_empty(); + return arg.clone(); + } + if is_environment_reference(arg) { + return "".into(); + } + redact_next = false; + redact_inline_secret(arg) + }) + .collect() +} +fn redact_url(value: &str) -> String { + let (scheme, rest) = value.split_once("://").unwrap_or(("", value)); + let (authority, path_query) = rest.split_once('/').unwrap_or((rest, "")); + let safe_authority = if let Some((_, host)) = authority.rsplit_once('@') { + format!("@{host}") + } else { + authority.to_string() + }; + let base_value = if scheme.is_empty() { + format!("{safe_authority}/{path_query}") + } else { + format!("{scheme}://{safe_authority}/{path_query}") + }; + if let Some((base, query)) = base_value.split_once('?') { + let parts = query + .split('&') + .map(|part| { + let (key, val) = part.split_once('=').unwrap_or((part, "")); + if is_sensitive_key(key) { + format!("{key}=") + } else if is_environment_reference(val) { + format!("{key}=") + } else { + part.into() + } + }) + .collect::>(); + format!("{base}?{}", parts.join("&")) + } else { + base_value + } +} +fn plugin_owner(name: &str) -> String { + let parts = name.split('/').collect::>(); + if parts.len() >= 4 { + format!("{}/{}", parts[1], parts[2]) + } else { + "plugin".into() + } +} + +fn status_diagnostics(entry: &AgentStatusEntry, target: &str, root: &Path) -> Vec { + entry + .entry + .issues + .iter() + .map(|issue| { + let code = match issue.kind { + StatusIssueKind::IncorrectLinkTarget => DiagnosticCode::DriftedSymlink, + StatusIssueKind::MissingExpectedSource => DiagnosticCode::MissingSource, + _ => DiagnosticCode::DriftedSymlink, + }; + Diagnostic { + code, + severity: DiagnosticSeverity::Warning, + target: Some(target.into()), + path: Some(portable_path(root, Path::new(&issue.path))), + message: format!( + "Materialized destination differs from the configured projection ({:?}).", + issue.kind + ), + } + }) + .collect() +} +fn observed_for(entries: &[&AgentStatusEntry]) -> ObservedState { + let Some(entry) = entries + .iter() + .find(|entry| { + entry.entry.sync_type != "module-map" || entry.entry.managed_children.is_some() + }) + .or_else(|| entries.first()) + else { + return ObservedState { + exists: false, + kind: "missing".into(), + points_to: None, + matches_projection: None, + }; + }; + let e = &entry.entry; + let matches = entries + .iter() + .all(|entry| entry.entry.issues.is_empty() && entry.entry.exists); + ObservedState { + exists: entries.iter().any(|entry| entry.entry.exists), + kind: format!("{:?}", e.destination_kind).to_ascii_lowercase(), + points_to: e.points_to.clone(), + matches_projection: Some(matches), + } +} +fn classify_target(name: &str) -> String { + match name.to_ascii_lowercase().as_str() { + "instructions" | "agents" | "agent" => "instructions", + "skill" | "skills" => "skills", + "command" | "commands" => "commands", + _ => "other", + } + .into() +} +fn sync_type_name(sync_type: SyncType) -> &'static str { + match sync_type { + SyncType::Symlink => "symlink", + SyncType::SymlinkContents => "symlink-contents", + SyncType::NestedGlob => "nested-glob", + SyncType::ModuleMap => "module-map", + } +} +fn same_link_target(destination: &Path, actual: &Path, expected: &Path) -> bool { + let actual = if actual.is_absolute() { + actual.to_path_buf() + } else { + destination.parent().unwrap_or(Path::new(".")).join(actual) + }; + std::fs::canonicalize(&actual).unwrap_or(actual) + == std::fs::canonicalize(expected).unwrap_or_else(|_| expected.to_path_buf()) +} +fn portable_path(root: &Path, path: &Path) -> String { + let value = path.strip_prefix(root).unwrap_or(path); + value + .components() + .map(|component| match component { + Component::Normal(part) => part.to_string_lossy().to_string(), + Component::CurDir => ".".into(), + Component::ParentDir => "..".into(), + Component::RootDir => "/".into(), + Component::Prefix(prefix) => prefix.as_os_str().to_string_lossy().to_string(), + }) + .collect::>() + .join("/") +} diff --git a/src/lib.rs b/src/lib.rs index 6b85179f..36cdc099 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,6 +8,7 @@ pub(crate) mod agent_ids; pub mod config; pub mod gitignore; pub mod init; +pub mod inspect; pub mod linker; pub mod logging; pub mod mcp; @@ -16,6 +17,7 @@ pub(crate) mod mcp_ownership; pub mod plugins; pub mod skills; pub mod skills_layout; +pub mod status; pub mod update_check; pub use config::Config; diff --git a/src/main.rs b/src/main.rs index 447b8ae1..6cfaa733 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,6 +18,7 @@ mod commands; mod output; use commands::dev_bench::{DevBenchArgs, run_dev_bench}; use commands::doctor::run_doctor; +use commands::inspect::{InspectArgs, run_inspect}; use commands::plugin::{PluginCommand, run_plugin}; use commands::skill::{SkillCommand, run_skill}; use commands::status::{StatusArgs, run_status}; @@ -39,6 +40,9 @@ fn should_spawn_update_check(command: &Commands) -> bool { { return false; } + if matches!(command, Commands::Inspect { .. }) { + return false; + } true } @@ -130,6 +134,14 @@ enum Commands { #[arg(short, long)] project_root: Option, }, + /// Inspect resolved AgentSync-managed configuration for one assistant. + Inspect { + #[command(flatten)] + args: InspectArgs, + /// Project root (defaults to CWD) + #[arg(short, long)] + project_root: Option, + }, /// Show status of managed symlinks Status { #[command(flatten)] @@ -265,6 +277,12 @@ fn run() -> Result<()> { .context("failed to create runtime for plugin command")?; runtime.block_on(run_plugin(cmd, root)) }), + Commands::Inspect { args, project_root } => run_in_root_span("inspect", || { + let project_root = + current_project_root(project_root, || env::current_dir().map_err(Into::into))?; + run_inspect(args, project_root)?; + Ok(()) + }), Commands::Status { args, project_root } => run_in_root_span("status", || { let project_root = current_project_root(project_root, || env::current_dir().map_err(Into::into))?; diff --git a/src/plugins.rs b/src/plugins.rs index 91e7607b..9473dfcb 100644 --- a/src/plugins.rs +++ b/src/plugins.rs @@ -346,6 +346,71 @@ impl PluginManager { }) } + /// Read plugin state without restoring snapshots or writing files. + /// + /// Unlike `status_report`, this method never materializes a local plugin source. Git snapshots + /// must already exist in the local cache. This is intended for offline inspection. + pub(crate) fn inspect_status_report_read_only(&self) -> Result { + self.config.validate()?; + let mut report = PluginStatusReport { + status: "ok".to_string(), + ..PluginStatusReport::default() + }; + if !self.config.enabled || self.config.selections.is_empty() { + ensure!( + self.config.allowed_mcp.is_empty(), + "plugin MCP approvals are stale: no selected plugin can provide {}", + self.config.allowed_mcp.join(", ") + ); + return Ok(report); + } + let lock = PluginLock::load(&self.lock_path()?)?; + let selections = selected_lock_entries(&self.config, &lock)?; + validate_allowed_mcp_against_lock(&self.config.allowed_mcp, &selections)?; + let allowed = self.config.allowed_mcp.iter().collect::>(); + for locked in selections.values() { + let root = match locked.source.kind { + LockedSourceKind::Local => { + let location = Path::new(&locked.source.location); + ensure!( + !is_absolute_path(&locked.source.location), + "locked local plugin source must be relative" + ); + self.config_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(location) + } + LockedSourceKind::Git => self.git_source_cache_path(&locked.source)?, + }; + ensure!( + root.is_dir(), + "plugin snapshot unavailable: {}", + root.display() + ); + let discovered = discover_plugin(&root, &locked.marketplace, &locked.plugin)?; + ensure_snapshot_discovery_matches_lock(&discovered, locked)?; + report.skills += discovered.skills.len(); + for (name, server) in + discovered.namespaced_mcp_servers(&locked.marketplace, &locked.plugin)? + { + report.servers.push(PluginMcpStatus { + approval: if allowed.contains(&name) { + PluginMcpApproval::Allowed + } else { + PluginMcpApproval::Pending + }, + name, + server, + }); + } + } + report + .servers + .sort_by(|left, right| left.name.cmp(&right.name)); + Ok(report) + } + pub fn status_report(&self) -> Result { self.config.validate()?; let mut report = PluginStatusReport { diff --git a/src/status.rs b/src/status.rs new file mode 100644 index 00000000..17752b1d --- /dev/null +++ b/src/status.rs @@ -0,0 +1,495 @@ +//! Read-only inspection of materialized AgentSync destinations. +//! +//! This module owns the filesystem probes that back both `agentsync status` and +//! `agentsync inspect`. It lives in the library so that other commands (and future +//! stories such as profiles and capability adapters) can reuse the same reader +//! without duplicating drift detection. +//! +//! The types and functions here describe what AgentSync **materialized on disk**. +//! They never describe what a provider's runtime resolves to. + +use crate::Linker; +use crate::config::{SyncType, TargetConfig}; +use crate::linker::SymlinkContentsChildExpectation; +use crate::skills_layout::detect_skills_mode_mismatch; +use anyhow::Result; +use serde::Serialize; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +#[derive(Serialize, Debug, Clone, Copy, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub enum DestinationKind { + Missing, + Symlink, + Directory, + File, + Other, +} + +#[derive(Serialize, Debug, Clone, Copy, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub enum StatusIssueKind { + MissingDestination, + InvalidDestinationType, + MissingExpectedChild, + ChildNotSymlink, + IncorrectLinkTarget, + MissingExpectedSource, +} + +#[derive(Serialize, Debug, Clone, PartialEq, Eq)] +pub struct StatusIssue { + pub kind: StatusIssueKind, + pub path: String, + pub expected: Option, + pub actual: Option, +} + +#[derive(Serialize, Debug, Clone, PartialEq, Eq)] +pub struct StatusChildEntry { + pub path: String, + pub exists: bool, + pub is_symlink: bool, + pub points_to: Option, + pub expected_source: String, +} + +#[derive(Serialize, Debug, Clone)] +pub struct StatusEntry { + pub destination: String, + pub sync_type: String, + pub destination_kind: DestinationKind, + pub exists: bool, + pub is_symlink: bool, + pub points_to: Option, + pub expected_source: Option, + pub issues: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub managed_children: Option>, +} + +/// A status entry together with the agent and target that declared it. +/// +/// `collect_status_entries` flattens this shape for the `status` command. The +/// inspector needs the provenance keys, so the reader keeps them addressable. +#[derive(Debug, Clone)] +pub struct AgentStatusEntry { + pub agent: String, + pub target: String, + pub entry: StatusEntry, +} + +/// Collect the materialized destination entries for every enabled agent. +/// +/// The order is deterministic: agents and targets are visited in `BTreeMap` +/// order, so the output never depends on hashing or filesystem order. +pub fn collect_status_entries(linker: &Linker, config_path: &Path) -> Result> { + Ok(collect_agent_status_entries(linker, config_path)? + .into_iter() + .map(|entry| entry.entry) + .collect()) +} + +/// Collect the materialized destination entries keyed by declaring agent and target. +pub fn collect_agent_status_entries( + linker: &Linker, + config_path: &Path, +) -> Result> { + let mut entries = Vec::new(); + let source_dir = linker.config().source_dir(config_path); + + for (agent_name, agent) in &linker.config().agents { + if !agent.enabled { + continue; + } + + for (target_name, target) in &agent.targets { + if target.sync_type == SyncType::ModuleMap { + for mapping in &target.mappings { + let filename = crate::config::resolve_module_map_filename(mapping, agent_name); + let destination = linker + .project_root() + .join(&mapping.destination) + .join(&filename); + let source_path = source_dir.join(&mapping.source); + let expected_source = linker.expected_source_path(&source_path, target); + + entries.push(AgentStatusEntry { + agent: agent_name.clone(), + target: target_name.clone(), + entry: validate_symlink_entry( + destination, + expected_source, + SyncType::ModuleMap, + ), + }); + } + continue; + } + + let destination = linker.project_root().join(&target.destination); + // NestedGlob resolves `source` relative to the project root (matching apply and + // doctor); all other sync types resolve it relative to source_dir. + let source_path = if target.sync_type == SyncType::NestedGlob { + linker.project_root().join(&target.source) + } else { + source_dir.join(&target.source) + }; + + let entry = match target.sync_type { + SyncType::Symlink => validate_symlink_entry( + destination, + linker.expected_source_path(&source_path, target), + SyncType::Symlink, + ), + SyncType::SymlinkContents => validate_symlink_contents_entry( + linker, + destination, + &source_path, + target, + agent_name, + detect_skills_mode_mismatch( + linker.project_root(), + &source_path, + agent_name, + target_name, + target, + ) + .is_some(), + )?, + SyncType::NestedGlob => validate_symlink_entry( + destination, + linker.expected_source_path(&source_path, target), + SyncType::NestedGlob, + ), + SyncType::ModuleMap => unreachable!(), + }; + + entries.push(AgentStatusEntry { + agent: agent_name.clone(), + target: target_name.clone(), + entry, + }); + } + } + + Ok(entries) +} + +pub fn collect_status_hints(linker: &Linker, config_path: &Path) -> HashMap { + let mut hints = HashMap::new(); + let source_dir = linker.config().source_dir(config_path); + + for (agent_name, agent) in &linker.config().agents { + if !agent.enabled { + continue; + } + + for (target_name, target) in &agent.targets { + let expected_source = source_dir.join(&target.source); + if let Some(mismatch) = detect_skills_mode_mismatch( + linker.project_root(), + &expected_source, + agent_name, + target_name, + target, + ) { + hints.insert( + linker + .project_root() + .join(&target.destination) + .display() + .to_string(), + mismatch.status_hint(), + ); + } + } + } + + hints +} + +pub fn entry_is_problematic(entry: &StatusEntry) -> bool { + !entry.issues.is_empty() +} + +fn validate_symlink_entry( + destination: PathBuf, + expected_source: Option, + sync_type: SyncType, +) -> StatusEntry { + let metadata = std::fs::symlink_metadata(&destination).ok(); + let destination_kind = destination_kind(metadata.as_ref()); + let exists = metadata.is_some(); + let is_symlink = destination_kind == DestinationKind::Symlink; + let points_to = read_link_target(&destination, is_symlink); + let expected_source_string = expected_source.as_ref().map(|path| display_path(path)); + let mut issues = Vec::new(); + + match expected_source.as_ref() { + Some(expected) => { + if !exists { + issues.push(StatusIssue { + kind: StatusIssueKind::MissingDestination, + path: display_path(&destination), + expected: Some(display_path(expected)), + actual: None, + }); + } else if !is_symlink { + issues.push(StatusIssue { + kind: StatusIssueKind::InvalidDestinationType, + path: display_path(&destination), + expected: Some("symlink".to_string()), + actual: Some(destination_kind_label(destination_kind).to_string()), + }); + } else if let Some(actual) = points_to.as_ref() { + if !paths_match(&destination, Path::new(actual), expected) { + issues.push(StatusIssue { + kind: StatusIssueKind::IncorrectLinkTarget, + path: display_path(&destination), + expected: Some(display_path(expected)), + actual: Some(actual.clone()), + }); + } + } else { + issues.push(StatusIssue { + kind: StatusIssueKind::IncorrectLinkTarget, + path: display_path(&destination), + expected: Some(display_path(expected)), + actual: None, + }); + } + } + None => { + issues.push(StatusIssue { + kind: StatusIssueKind::MissingExpectedSource, + path: display_path(&destination), + expected: None, + actual: points_to.clone(), + }); + } + } + + StatusEntry { + destination: display_path(&destination), + sync_type: sync_type_label(sync_type).to_string(), + destination_kind, + exists, + is_symlink, + points_to, + expected_source: expected_source_string, + issues, + managed_children: None, + } +} + +fn validate_symlink_contents_child( + destination: &Path, + child: SymlinkContentsChildExpectation, +) -> StatusChildEntry { + let child_path = destination.join(&child.name); + let metadata = std::fs::symlink_metadata(&child_path).ok(); + let kind = destination_kind(metadata.as_ref()); + let exists = metadata.is_some(); + let is_symlink = kind == DestinationKind::Symlink; + let points_to = read_link_target(&child_path, is_symlink); + + StatusChildEntry { + path: display_path(&child_path), + exists, + is_symlink, + points_to, + expected_source: display_path(&child.expected_source_path), + } +} + +fn collect_child_issues(child_status: &StatusChildEntry, issues: &mut Vec) { + if !child_status.exists { + issues.push(StatusIssue { + kind: StatusIssueKind::MissingExpectedChild, + path: child_status.path.clone(), + expected: Some(child_status.expected_source.clone()), + actual: None, + }); + } else if !child_status.is_symlink { + issues.push(StatusIssue { + kind: StatusIssueKind::ChildNotSymlink, + path: child_status.path.clone(), + expected: Some(child_status.expected_source.clone()), + actual: None, + }); + } else if let Some(actual) = child_status.points_to.as_ref() + && !paths_match( + Path::new(&child_status.path), + Path::new(actual), + Path::new(&child_status.expected_source), + ) + { + issues.push(StatusIssue { + kind: StatusIssueKind::IncorrectLinkTarget, + path: child_status.path.clone(), + expected: Some(child_status.expected_source.clone()), + actual: Some(actual.clone()), + }); + } +} + +fn validate_children_directory( + destination: &Path, + destination_kind: DestinationKind, + exists: bool, + children: Vec, + issues: &mut Vec, + managed_children: &mut Vec, +) { + if !exists { + issues.push(StatusIssue { + kind: StatusIssueKind::MissingDestination, + path: display_path(destination), + expected: Some("directory".to_string()), + actual: None, + }); + } else if destination_kind != DestinationKind::Directory { + issues.push(StatusIssue { + kind: StatusIssueKind::InvalidDestinationType, + path: display_path(destination), + expected: Some("directory".to_string()), + actual: Some(destination_kind_label(destination_kind).to_string()), + }); + } else { + for child in children { + let child_status = validate_symlink_contents_child(destination, child); + collect_child_issues(&child_status, issues); + managed_children.push(child_status); + } + } +} + +fn validate_symlink_contents_entry( + linker: &Linker, + destination: PathBuf, + source_path: &Path, + target: &TargetConfig, + agent_name: &str, + allow_skills_symlink_hint: bool, +) -> Result { + let metadata = std::fs::symlink_metadata(&destination).ok(); + let destination_kind = destination_kind(metadata.as_ref()); + let exists = metadata.is_some(); + let is_symlink = destination_kind == DestinationKind::Symlink; + let points_to = read_link_target(&destination, is_symlink); + let child_expectations = + linker.symlink_contents_expected_children(source_path, target, agent_name)?; + let mut issues = Vec::new(); + let mut managed_children = Vec::new(); + + if let Some(children) = child_expectations { + if allow_skills_symlink_hint && is_symlink { + return Ok(StatusEntry { + destination: display_path(&destination), + sync_type: sync_type_label(SyncType::SymlinkContents).to_string(), + destination_kind, + exists, + is_symlink, + points_to, + expected_source: None, + issues, + managed_children: Some(managed_children), + }); + } + + validate_children_directory( + &destination, + destination_kind, + exists, + children, + &mut issues, + &mut managed_children, + ); + } else { + issues.push(StatusIssue { + kind: StatusIssueKind::MissingExpectedSource, + path: display_path(source_path), + expected: None, + actual: Some(display_path(&destination)), + }); + } + + Ok(StatusEntry { + destination: display_path(&destination), + sync_type: sync_type_label(SyncType::SymlinkContents).to_string(), + destination_kind, + exists, + is_symlink, + points_to, + expected_source: None, + issues, + managed_children: Some(managed_children), + }) +} + +fn destination_kind(metadata: Option<&std::fs::Metadata>) -> DestinationKind { + match metadata { + None => DestinationKind::Missing, + Some(meta) => { + let file_type = meta.file_type(); + if file_type.is_symlink() { + DestinationKind::Symlink + } else if file_type.is_dir() { + DestinationKind::Directory + } else if file_type.is_file() { + DestinationKind::File + } else { + DestinationKind::Other + } + } + } +} + +fn read_link_target(path: &Path, is_symlink: bool) -> Option { + if !is_symlink { + return None; + } + std::fs::read_link(path) + .ok() + .map(|target| display_path(&target)) +} + +fn display_path(path: &Path) -> String { + path.to_string_lossy().to_string() +} + +fn paths_match(destination: &Path, points_to: &Path, expected: &Path) -> bool { + let actual_path = if points_to.is_absolute() { + points_to.to_path_buf() + } else { + destination + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(points_to) + }; + + let expected_canon = std::fs::canonicalize(expected).unwrap_or_else(|_| expected.to_path_buf()); + let actual_canon = std::fs::canonicalize(&actual_path).unwrap_or(actual_path); + expected_canon == actual_canon +} + +fn sync_type_label(sync_type: SyncType) -> &'static str { + match sync_type { + SyncType::Symlink => "symlink", + SyncType::SymlinkContents => "symlink-contents", + SyncType::NestedGlob => "nested-glob", + SyncType::ModuleMap => "module-map", + } +} + +fn destination_kind_label(kind: DestinationKind) -> &'static str { + match kind { + DestinationKind::Missing => "missing", + DestinationKind::Symlink => "symlink", + DestinationKind::Directory => "directory", + DestinationKind::File => "file", + DestinationKind::Other => "other", + } +} diff --git a/tests/all_tests.rs b/tests/all_tests.rs index 9a64dbb6..f9ccf4e8 100644 --- a/tests/all_tests.rs +++ b/tests/all_tests.rs @@ -1,3 +1,5 @@ +#[path = "contracts/test_inspect_output.rs"] +mod inspect_output_contract; mod integration; mod real_world_skills; mod security_repro; diff --git a/tests/contracts/test_inspect_output.rs b/tests/contracts/test_inspect_output.rs new file mode 100644 index 00000000..bb1ad85a --- /dev/null +++ b/tests/contracts/test_inspect_output.rs @@ -0,0 +1,290 @@ +use serde_json::Value; +use std::process::Command; +use tempfile::TempDir; + +fn binary() -> &'static str { + env!("CARGO_BIN_EXE_agentsync") +} + +fn project() -> TempDir { + let project = TempDir::new().unwrap(); + std::fs::create_dir_all(project.path().join(".agents/skills/demo")).unwrap(); + std::fs::create_dir_all(project.path().join(".agents/commands")).unwrap(); + std::fs::write( + project.path().join(".agents/AGENTS.md"), + "fixture instructions\n", + ) + .unwrap(); + std::fs::write( + project.path().join(".agents/skills/demo/SKILL.md"), + "fixture skill\n", + ) + .unwrap(); + std::fs::write( + project.path().join(".agents/commands/review.md"), + "fixture command\n", + ) + .unwrap(); + std::fs::write( + project.path().join(".agents/agentsync.toml"), + r#" +source_dir = "." +[mcp] +enabled = true +merge_strategy = "merge" +[mcp_servers.fixture] +command = "fixture-mcp" +args = ["--token", "CANARY_ARG_SECRET", "--api-key=CANARY_INLINE_SECRET", "${MCP_TOKEN}"] +url = "https://user:CANARY_USERINFO_SECRET@example.invalid/path?token=CANARY_URL_SECRET" +[mcp_servers.fixture.env] +API_TOKEN = "CANARY_ENV_SECRET" +LOG_LEVEL = "debug" +[agents.opencode] +enabled = true +[agents.opencode.targets.instructions] +source = "AGENTS.md" +destination = "AGENTS.md" +type = "symlink" +[agents.opencode.targets.skills] +source = "skills" +destination = ".opencode/skills" +type = "symlink-contents" +[agents.opencode.targets.commands] +source = "commands" +destination = ".opencode/command" +type = "symlink-contents" +[agents.codex] +enabled = true +[agents.codex.targets.instructions] +source = "AGENTS.md" +destination = "AGENTS.md" +type = "symlink" +[agents.codex.targets.skills] +source = "skills" +destination = ".codex/skills" +type = "symlink-contents" +[agents.codex.targets.commands] +source = "commands" +destination = ".codex/commands" +type = "symlink-contents" +"#, + ) + .unwrap(); + project +} + +fn inspect(project: &TempDir, agent: &str, strict: bool) -> std::process::Output { + let mut command = Command::new(binary()); + command.args(["inspect", "--agent", agent, "--json", "--project-root"]); + command.arg(project.path()); + if strict { + command.arg("--strict"); + } + command.output().unwrap() +} + +#[test] +fn inspect_json_is_stable_read_only_and_secret_safe() { + let project = project(); + let before = std::fs::read_dir(project.path()).unwrap().count(); + let first = inspect(&project, "opencode", false); + let second = inspect(&project, "opencode", false); + assert!( + first.status.success(), + "{}", + String::from_utf8_lossy(&first.stderr) + ); + assert_eq!(first.stdout, second.stdout); + assert!(first.stderr.is_empty()); + let value: Value = serde_json::from_slice(&first.stdout).unwrap(); + assert_eq!(value["schema_version"], 1); + assert_eq!(value["scope"], "agentsync-managed"); + assert!( + value["scope_notice"] + .as_str() + .unwrap() + .contains("not a claim") + ); + assert!( + value["items"] + .as_array() + .unwrap() + .iter() + .any(|item| item["kind"] == "instructions") + ); + assert!( + value["items"] + .as_array() + .unwrap() + .iter() + .any(|item| item["kind"] == "skills") + ); + assert!( + value["items"] + .as_array() + .unwrap() + .iter() + .any(|item| item["kind"] == "commands") + ); + assert!( + String::from_utf8_lossy(&first.stdout) + .find("CANARY_ENV_SECRET") + .is_none() + ); + assert!( + String::from_utf8_lossy(&first.stdout) + .find("CANARY_ARG_SECRET") + .is_none() + ); + assert!( + String::from_utf8_lossy(&first.stdout) + .find("CANARY_INLINE_SECRET") + .is_none() + ); + assert!(!String::from_utf8_lossy(&first.stdout).contains("${MCP_TOKEN}")); + assert!(!String::from_utf8_lossy(&first.stdout).contains("CANARY_USERINFO_SECRET")); + assert!(!String::from_utf8_lossy(&first.stdout).contains("CANARY_URL_SECRET")); + assert_eq!(std::fs::read_dir(project.path()).unwrap().count(), before); + assert!(!project.path().join("AGENTS.md").exists()); + assert!(!project.path().join(".opencode").exists()); +} + +#[test] +fn inspect_codex_projects_canonical_sources_and_destinations() { + let project = project(); + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let instructions = value["items"] + .as_array() + .unwrap() + .iter() + .find(|item| item["kind"] == "instructions") + .unwrap(); + assert_eq!(instructions["source"], ".agents/AGENTS.md"); + assert_eq!(instructions["destination"], "AGENTS.md"); + assert_eq!(instructions["sync_mode"], "symlink"); + assert_eq!(value["mcp"]["capability"], "managed"); + assert_eq!(value["mcp"]["servers"][0]["name"], "fixture"); +} + +#[test] +fn inspect_default_is_report_only_and_strict_fails_for_missing_source() { + let project = project(); + std::fs::remove_file(project.path().join(".agents/AGENTS.md")).unwrap(); + let default = inspect(&project, "codex", false); + assert!(default.status.success()); + let value: Value = serde_json::from_slice(&default.stdout).unwrap(); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| diagnostic["code"] == "missing-source") + ); + let strict = inspect(&project, "codex", true); + assert_eq!(strict.status.code(), Some(1)); +} + +#[test] +fn inspect_broken_symlink_has_distinct_typed_diagnostic() { + let project = project(); + let path = project.path().join("AGENTS.md"); + #[cfg(unix)] + std::os::unix::fs::symlink("missing-source.md", &path).unwrap(); + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| diagnostic["code"] == "broken-symlink") + ); +} + +#[test] +fn inspect_unknown_agent_reports_unknown_managed_scope_without_failure() { + let project = project(); + let output = inspect(&project, "unconfigured", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(value["items"], serde_json::json!([])); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| diagnostic["code"] == "agent-not-configured") + ); +} + +#[test] +fn inspect_reports_overwrite_without_claiming_unmanaged_servers_survive() { + let project = project(); + let config_path = project.path().join(".agents/agentsync.toml"); + let mut config = std::fs::read_to_string(&config_path).unwrap(); + config = config.replace( + "merge_strategy = \"merge\"", + "merge_strategy = \"overwrite\"", + ); + std::fs::write(&config_path, config).unwrap(); + std::fs::create_dir_all(project.path().join(".codex")).unwrap(); + std::fs::write( + project.path().join(".codex/config.toml"), + "[mcp_servers.local-only]\ncommand = 'local'\n", + ) + .unwrap(); + + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(value["mcp"]["merge_strategy"], "overwrite"); + assert!( + value["mcp"]["existing_server_names"] + .as_array() + .unwrap() + .is_empty() + ); +} + +#[test] +fn inspect_disabled_mcp_does_not_claim_servers_are_projected() { + let project = project(); + let config_path = project.path().join(".agents/agentsync.toml"); + let config = std::fs::read_to_string(&config_path).unwrap().replace( + "enabled = true\nmerge_strategy", + "enabled = false\nmerge_strategy", + ); + std::fs::write(config_path, config).unwrap(); + + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!(value["mcp"]["servers"].as_array().unwrap().is_empty()); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { diagnostic["code"] == "disabled-mcp-server" }) + ); +} + +#[test] +fn inspect_redacts_sensitive_assignment_variants() { + let project = project(); + let config_path = project.path().join(".agents/agentsync.toml"); + let config = std::fs::read_to_string(&config_path).unwrap().replace( + "\"${MCP_TOKEN}\"]", + "\"--authorization:CANARY_AUTH_SECRET\", \"--client-secret=CANARY_CLIENT_SECRET\", \"${MCP_TOKEN}\"]", + ); + std::fs::write(config_path, config).unwrap(); + + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(!stdout.contains("CANARY_AUTH_SECRET")); + assert!(!stdout.contains("CANARY_CLIENT_SECRET")); +} diff --git a/website/docs/src/content/docs/guides/inspect.mdx b/website/docs/src/content/docs/guides/inspect.mdx new file mode 100644 index 00000000..91c7c614 --- /dev/null +++ b/website/docs/src/content/docs/guides/inspect.mdx @@ -0,0 +1,40 @@ +--- +title: Inspect AgentSync-Managed Configuration +description: Review one assistant's declared sources, planned targets, observed drift, and MCP inputs. +--- + +# Inspect AgentSync-managed configuration + +Use `inspect` to explain what AgentSync intends to manage for one assistant before applying changes. + +```bash +agentsync inspect --agent opencode +agentsync inspect --agent codex --json +``` + +The report connects each configured target to its source and destination. It also reports the observed +filesystem state, such as a missing source, a missing destination, a broken link, or a drifted link. +For skills and commands configured as `symlink-contents`, child links are listed individually. + +The command is read-only and offline. It does not repair drift, write files, download plugins, expand +environment variables, invoke assistant CLIs, or start MCP servers. A normal invocation exits `0` +when it can produce a report. Add `--strict` to return `1` if an error-severity diagnostic exists. +This lets teams review reports without turning every warning into a CI failure. + +For example, use strict JSON in a check: + +```bash +agentsync inspect --agent opencode --json --strict > inspect.json +``` + +MCP precedence follows the current `apply` rules: explicit `[mcp_servers.*]` declarations form the +base set; plugin contributions are namespaced and require approval in `plugins.allowed_mcp`. A +conflicting explicit/plugin name with a different definition is rejected by `apply`. Existing +unmanaged server names are reported without exposing their configuration values. Merge behavior +preserves or replaces destination entries according to `mcp.merge_strategy`. + +`inspect` reports only AgentSync-managed configuration. It does not claim that provider-native, +global, user, organization, or session settings are absent. `provider_support: "unknown"` means +AgentSync is not asserting that the assistant natively supports the semantic capability. See the +inspect output reference for the JSON schema and diagnostic +codes. diff --git a/website/docs/src/content/docs/reference/cli.mdx b/website/docs/src/content/docs/reference/cli.mdx index 082f636e..0113cc25 100644 --- a/website/docs/src/content/docs/reference/cli.mdx +++ b/website/docs/src/content/docs/reference/cli.mdx @@ -329,7 +329,7 @@ agentsync skill list agentsync skill list --json ``` -`--json` writes a stable JSON object to stdout with `schema_version: 1`, `skills`, and `diagnostics`. Each skill reports `id`, `path`, `source_kind` (`direct`, `plugin`, `manual/untracked`, or `unknown`), nullable `version` and `provenance`, and `state` (`installed`, `missing`, `missing_manifest`, `malformed`, `duplicate_id`, or `broken_symlink`). Diagnostics are separate from inventory entries in the JSON response. A malformed or missing individual skill does not fail the command. A present but unreadable or invalid `registry.json` returns exit code `1`; with `--json`, stdout contains the same top-level JSON shape and a redacted `registry_invalid` diagnostic, while the error details go to stderr. An absent registry is recoverable and unregistered skill directories are listed as `manual/untracked`. An empty or absent skills directory returns exit code `0` and a successful empty list. Human-readable warnings are written to stderr. Duplicate manifest IDs receive the `duplicate_id` state and a diagnostic; broken symlinks receive the `broken_symlink` state. A skill directory without `SKILL.md` receives the `missing_manifest` state. +`--json` writes a stable JSON object to stdout with `schema_version: 1`, `skills`, and `diagnostics`. Each skill reports `id`, `path`, `source_kind` (`direct`, `plugin`, `manual/untracked`, or `unknown`), nullable `version` and `provenance`, and `state` (`installed`, `missing`, `missing_manifest`, `malformed`, `duplicate_id`, or `broken_symlink`). Diagnostics are separate from inventory entries in the JSON response. A malformed or missing individual skill does not fail the command. A present but unreadable or invalid `registry.json` returns exit code `1`; with `--json`, stdout contains the same top-level JSON shape and a redacted `registry_invalid` diagnostic, while the error details go to stderr. Filesystem scan failures use a distinct `inventory_unreadable` diagnostic and also return exit code `1`. An absent registry is recoverable and unregistered skill directories are listed as `manual/untracked`. An empty or absent skills directory returns exit code `0` and a successful empty list. Human-readable warnings are written to stderr. Duplicate manifest IDs receive the `duplicate_id` state and a diagnostic; broken symlinks receive the `broken_symlink` state. A skill directory without `SKILL.md` receives the `missing_manifest` state. #### `registry` @@ -505,6 +505,50 @@ Shows help information for the CLI or a specific command. --- +### `inspect` + +Inspect the deterministic projection of configuration AgentSync manages for one assistant. This is a +read-only, offline report. It does not apply changes, download plugin snapshots, expand environment +variables, invoke provider CLIs, or start MCP servers. + +```bash +agentsync inspect --agent [--json] [--strict] [--project-root ] +``` + +**Flags**: +- `--agent `: Required. Selects one configured assistant, such as `opencode` or `codex`. +- `--json`: Emit compact, deterministic JSON v1 on stdout. +- `--strict`: Exit with code `1` if the report contains an error-severity diagnostic. +- `-p, --project-root `: Project root; defaults to the current directory. + +By default, exit code `0` means the report was produced, even when it contains diagnostics. Strict +mode is suitable for CI. Configuration or runtime errors exit non-zero; invalid CLI arguments use +Clap's usage-error exit code. + +The report separates declared configuration, planned destinations, observed filesystem state, and +unsupported capabilities. It identifies source, target, sync mode, owner, and provenance. It does not +claim to describe complete native runtime-effective settings. Global, user, organization, and session +overlays outside AgentSync's control are unknown, not absent. + +MCP reports show explicit declarations, approved or pending plugin contributions, disabled servers, +and existing destination server names. Explicit project MCP declarations are the base input; +plugin contributions require approval in `plugins.allowed_mcp`. A differing explicit/plugin name +collision is rejected by `apply`. Existing destination entries are preserved or replaced according +to the configured merge strategy. Existing unmanaged server values are never emitted. + +Environment and header values are never included in the report. The output includes variable/header +names and a value classification only. Do not treat the report as a secret scanner for arbitrary +instruction or skill content. + +Examples: +- `agentsync inspect --agent opencode` +- `agentsync inspect --agent codex --json` +- `agentsync inspect --agent codex --json --strict` + +The JSON contract is documented in Inspect output. + +--- + ### `status` Verify the status of AgentSync-managed targets and report drift. diff --git a/website/docs/src/content/docs/reference/inspect-output.mdx b/website/docs/src/content/docs/reference/inspect-output.mdx new file mode 100644 index 00000000..f17ba5c7 --- /dev/null +++ b/website/docs/src/content/docs/reference/inspect-output.mdx @@ -0,0 +1,58 @@ +--- +title: Inspect Output +description: Stable JSON v1 contract for agentsync inspect. +--- + +# `agentsync inspect --json` output + +`inspect --json` writes one compact JSON object to stdout. The report is deterministic for the same +configuration and filesystem tree. It contains no timestamp. + +## Envelope + +- `schema_version`: JSON contract version (`1`). +- `scope`: always `agentsync-managed`. +- `agent`: selected configured assistant identifier (lowercase for an unconfigured identifier). +- `items`: configured instruction, skill, command, and other targets. +- `mcp`: AgentSync-managed MCP declarations, plugin contributions, and existing destination names. +- `diagnostics`: sorted typed findings. +- `scope_notice`: explicit boundary from provider runtime-effective configuration. + +Each item reports `kind`, `target`, `sync_mode`, `source`, `destination`, `origin`, `owner`, +`provider_support`, `state`, `observed`, and optional `children`. Paths inside the project root are +relative and use `/` separators. Symlink `points_to` values are the observed link text and therefore +may be relative to the destination. + +Item states are: + +- `configured`: declared intent exists but its source is missing. +- `projected`: source exists but the destination is not materialized. +- `observed`: the materialized destination matches the projection. +- `conflicted`: the materialized destination differs from the projection, or a collision exists. +- `unsupported`: AgentSync cannot project the requested capability for the selected assistant. + +`provider_support: "unknown"` means the report does not assert that the provider natively supports +the semantic capability. This is deliberate; `inspect` does not implement cross-provider adapters. + +## Diagnostics + +Diagnostics contain `code`, `severity`, `target`, `path`, and a concise `message`. Codes include +`agent-not-configured`, `missing-source`, `drifted-symlink`, `broken-symlink`, +`destination-collision`, `mcp-name-collision`, `unsupported-target`, `missing-plugin-snapshot`, +`disabled-mcp-server`, `pending-plugin-approval`, `unsupported-plugin-component`, and +`existing-mcp-server`. + +Default exit behavior is report-oriented: exit `0` if a report was produced. With `--strict`, any +error-severity diagnostic returns exit `1`. Invalid CLI arguments use Clap's exit `2`. + +## Redaction and limits + +Environment and header values are never included. Only keys and value classifications are reported. +Sensitive argument values and sensitive URL query values are redacted. Existing unmanaged MCP +configuration is parsed only to obtain server names; its values are not included. `inspect` does not +read arbitrary instruction/skill contents into the JSON report, but this is not a general secret scan. + +The report describes AgentSync-owned configuration only. Native global, user, organization, and +session settings outside AgentSync's control are unknown. The command does not invoke provider CLIs, +start MCP servers, perform network requests, expand environment variables, download snapshots, or +write files. From e0c034fdfcdd77e4ae51ae5d5940945a7a071b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:11:59 +0200 Subject: [PATCH 3/5] fix: address inspect review findings --- src/commands/inspect.rs | 9 +- src/commands/skill.rs | 21 +- src/inspect.rs | 335 ++++++++++++------ src/plugins.rs | 10 +- src/status.rs | 4 +- tests/contracts/test_inspect_output.rs | 98 ++++- tests/contracts/test_skill_list_output.rs | 10 +- .../content/docs/reference/inspect-output.mdx | 5 +- 8 files changed, 347 insertions(+), 145 deletions(-) diff --git a/src/commands/inspect.rs b/src/commands/inspect.rs index 19db8f81..7a189ca2 100644 --- a/src/commands/inspect.rs +++ b/src/commands/inspect.rs @@ -79,7 +79,14 @@ fn print_human(report: &ResolvedConfiguration) { ); } for diagnostic in &report.diagnostics { - println!("\n{:?}: {}", diagnostic.code, diagnostic.message); + println!( + "\n{:?} [{:?}] target={} path={}: {}", + diagnostic.code, + diagnostic.severity, + diagnostic.target.as_deref().unwrap_or("-"), + diagnostic.path.as_deref().unwrap_or("-"), + diagnostic.message + ); } println!("\n{}", report.scope_notice); } diff --git a/src/commands/skill.rs b/src/commands/skill.rs index 1181ead1..71dc117c 100644 --- a/src/commands/skill.rs +++ b/src/commands/skill.rs @@ -1003,22 +1003,23 @@ pub fn run_skill(cmd: SkillCommand, project_root: PathBuf) -> Result<()> { Ok(inventory) => inventory, Err(error) => { if json { - let diagnostic = match &error { - SkillInventoryError::Registry(_) => serde_json::json!({ - "code": "registry_invalid", - "message": "installed skill registry could not be read or parsed" - }), - SkillInventoryError::Scan(_) => serde_json::json!({ - "code": "inventory_unreadable", - "message": "skill inventory could not be scanned" - }), + let (code, message) = match &error { + SkillInventoryError::Registry(_) => ( + "registry_invalid", + "installed skill registry could not be read or parsed", + ), + SkillInventoryError::Scan(_) => ( + "inventory_unreadable", + "skill inventory could not be scanned", + ), }; println!( "{}", serde_json::json!({ "schema_version": 1, "skills": [], - "diagnostics": [diagnostic] + "diagnostic_code": code, + "diagnostics": [message] }) ); } diff --git a/src/inspect.rs b/src/inspect.rs index 683c19c2..90b0a9dd 100644 --- a/src/inspect.rs +++ b/src/inspect.rs @@ -8,7 +8,8 @@ use crate::config::{Config, McpServerConfig, SyncType}; use crate::mcp::McpAgent; use crate::plugins::PluginLock; use crate::status::{ - AgentStatusEntry, DestinationKind, StatusIssueKind, collect_agent_status_entries, + AgentStatusEntry, DestinationKind, StatusIssueKind, collect_agent_status_entries, paths_match, + sync_type_label, }; use anyhow::Result; use serde::Serialize; @@ -52,6 +53,7 @@ pub enum DiagnosticCode { PendingPluginApproval, UnsupportedPluginComponent, ExistingMcpServer, + DisabledAgent, } #[derive(Debug, Clone, PartialEq, Eq, Serialize)] @@ -168,38 +170,72 @@ pub fn resolve_configuration( let source_root = config.source_dir(config_path); let mut items = Vec::new(); let mut destinations = BTreeMap::::new(); + if !agent.enabled { + diagnostics.push(Diagnostic { + code: DiagnosticCode::DisabledAgent, + severity: DiagnosticSeverity::Info, + target: None, + path: None, + message: "Agent is disabled in agentsync.toml; its targets are not projected.".into(), + }); + } if agent.enabled { for (target_name, target) in &agent.targets { let kind = classify_target(target_name); + let is_module_map = target.sync_type == SyncType::ModuleMap; let source = if target.sync_type == SyncType::NestedGlob { project_root.join(&target.source) } else { source_root.join(&target.source) }; - let destination = project_root.join(&target.destination); - let portable_source = portable_path(&project_root, &source); - let portable_destination = portable_path(&project_root, &destination); - if let Some(previous) = - destinations.insert(portable_destination.clone(), target_name.clone()) - { - diagnostics.push(Diagnostic { - code: DiagnosticCode::DestinationCollision, - severity: DiagnosticSeverity::Error, - target: Some(target_name.clone()), - path: Some(portable_destination.clone()), - message: format!( - "Targets {previous} and {target_name} project to the same destination." - ), - }); + let mappings = if is_module_map { + target + .mappings + .iter() + .map(|mapping| { + ( + source_root.join(&mapping.source), + project_root.join(&mapping.destination).join( + crate::config::resolve_module_map_filename(mapping, &agent_name), + ), + ) + }) + .collect::>() + } else { + vec![(source.clone(), project_root.join(&target.destination))] + }; + let (projection_source, projection_destination) = mappings + .first() + .cloned() + .unwrap_or_else(|| (source.clone(), project_root.join(&target.destination))); + let portable_source = portable_path(&project_root, &projection_source); + let portable_destination = portable_path(&project_root, &projection_destination); + for (_, mapped_destination) in &mappings { + let mapped_destination = portable_path(&project_root, mapped_destination); + if let Some(previous) = + destinations.insert(mapped_destination.clone(), target_name.clone()) + { + diagnostics.push(Diagnostic { + code: DiagnosticCode::DestinationCollision, + severity: DiagnosticSeverity::Error, + target: Some(target_name.clone()), + path: Some(mapped_destination), + message: format!( + "Targets {previous} and {target_name} project to the same destination." + ), + }); + } } let matched = status .iter() .filter(|entry| entry.agent == agent_name && entry.target == *target_name) .collect::>(); let observed = observed_for(&matched); - let source_exists = source.exists(); + let source_exists = mappings + .iter() + .all(|(mapped_source, _)| mapped_source.exists()); let destination_occupied = target.sync_type == SyncType::Symlink - && std::fs::symlink_metadata(&destination).is_ok(); + && std::fs::symlink_metadata(&projection_destination).is_ok(); let mut state = if !source_exists { ItemState::Configured } else if observed.matches_projection == Some(true) { @@ -210,16 +246,42 @@ pub fn resolve_configuration( ItemState::Projected }; if !source_exists { + let missing_source = mappings + .iter() + .find(|(mapped_source, _)| !mapped_source.exists()) + .map(|(mapped_source, _)| portable_path(&project_root, mapped_source)) + .unwrap_or_else(|| portable_source.clone()); diagnostics.push(Diagnostic { code: DiagnosticCode::MissingSource, severity: DiagnosticSeverity::Error, target: Some(target_name.clone()), - path: Some(portable_source.clone()), + path: Some(missing_source), message: "Configured source does not exist.".into(), }); } - for entry in &matched { - diagnostics.extend(status_diagnostics(entry, target_name, &project_root)); + for (index, entry) in matched.iter().enumerate() { + let entry_state = if is_module_map { + let source_exists = mappings + .get(index) + .is_some_and(|(source, _)| source.exists()); + if !source_exists { + ItemState::Configured + } else if entry.entry.issues.is_empty() && entry.entry.exists { + ItemState::Observed + } else if entry.entry.exists { + ItemState::Conflicted + } else { + ItemState::Projected + } + } else { + state + }; + diagnostics.extend(status_diagnostics( + entry, + target_name, + &project_root, + entry_state, + )); if entry.entry.destination_kind == DestinationKind::Symlink && entry.entry.points_to.as_deref().is_some_and(|target| { let target = Path::new(target); @@ -245,7 +307,38 @@ pub fn resolve_configuration( } } let mut children = Vec::new(); - if let Some(entry) = matched + if is_module_map { + for ((mapped_source, mapped_destination), entry) in + mappings.iter().zip(matched.iter()) + { + let exists = mapped_source.exists(); + let matches = entry.entry.issues.is_empty() && entry.entry.exists; + children.push(ManagedChild { + source: portable_path(&project_root, mapped_source), + destination: portable_path(&project_root, mapped_destination), + state: if !exists { + ItemState::Configured + } else if matches { + ItemState::Observed + } else if entry.entry.exists { + ItemState::Conflicted + } else { + ItemState::Projected + }, + observed: ObservedState { + exists: entry.entry.exists, + kind: format!("{:?}", entry.entry.destination_kind) + .to_ascii_lowercase(), + points_to: entry + .entry + .points_to + .as_ref() + .map(|path| portable_path(&project_root, Path::new(path))), + matches_projection: Some(matches), + }, + }); + } + } else if let Some(entry) = matched .iter() .find(|entry| entry.entry.managed_children.is_some()) && let Some(entry) = entry.entry.managed_children.as_ref() @@ -257,7 +350,7 @@ pub fn resolve_configuration( let child_matches = child.exists && child.is_symlink && child.points_to.as_deref().is_some_and(|actual| { - same_link_target( + paths_match( Path::new(&child.path), Path::new(actual), Path::new(&child.expected_source), @@ -296,7 +389,7 @@ pub fn resolve_configuration( items.push(ManagedItem { kind, target: target_name.clone(), - sync_mode: sync_type_name(target.sync_type).into(), + sync_mode: sync_type_label(target.sync_type).into(), source: portable_source, destination: portable_destination, origin: "config".into(), @@ -396,6 +489,14 @@ fn resolve_mcp( BTreeMap::new() }; let existing_names = existing_servers.keys().cloned().collect::>(); + let projected = config + .mcp_servers + .iter() + .map(|(name, server)| (name.as_str(), server)) + .collect::>(); + let formatter = agent.formatter(); + let formatted = formatter.format_to_string(&projected)?; + let formatted_servers = formatter.parse_existing(&formatted).unwrap_or_default(); let mut servers = Vec::new(); if !config.mcp.enabled && (!config.mcp_servers.is_empty() || !config.plugins.selections.is_empty()) @@ -410,9 +511,12 @@ fn resolve_mcp( } if config.mcp.enabled { for (name, server) in &config.mcp_servers { - let existing_equal = existing_servers - .get(name) - .is_some_and(|value| existing_server_matches(value, server)); + let existing_equal = match (existing_servers.get(name), formatted_servers.get(name)) { + (Some(existing), Some(expected)) => { + normalized_mcp_value(existing) == normalized_mcp_value(expected) + } + _ => false, + }; let mut server_report = mcp_server_report(name, server, "config", None, existing_equal, root, &dest); server_report.state = if server.disabled { @@ -463,9 +567,17 @@ fn resolve_mcp( if collision { diagnostics.push(Diagnostic { code: DiagnosticCode::McpNameCollision, severity: DiagnosticSeverity::Error, target: Some("mcp".into()), path: Some(dest.clone()), message: format!("Explicit MCP declaration and plugin contribution collide at {}; apply rejects differing definitions.", item.name) }); } + let plugin_expected = formatter + .format_to_string(&BTreeMap::from([(item.name.as_str(), &item.server)])) + .ok() + .and_then(|formatted| formatter.parse_existing(&formatted).ok()) + .and_then(|servers| servers.get(&item.name).cloned()); let existing_equal = existing_servers .get(&item.name) - .is_some_and(|value| existing_server_matches(value, &item.server)); + .zip(plugin_expected.as_ref()) + .is_some_and(|(existing, expected)| { + normalized_mcp_value(existing) == normalized_mcp_value(expected) + }); let mut server_report = mcp_server_report( &item.name, &item.server, @@ -537,25 +649,25 @@ fn resolve_mcp( }) } -fn existing_server_matches(value: &serde_json::Value, server: &McpServerConfig) -> bool { - let Some(object) = value.as_object() else { - return false; - }; - let field_matches = |key: &str, expected: Option| match expected { - Some(expected) => object.get(key) == Some(&expected), - None => object.get(key).is_none_or(serde_json::Value::is_null), - }; - field_matches( - "command", - server.command.clone().map(serde_json::Value::String), - ) && field_matches("args", Some(serde_json::json!(server.args))) - && field_matches("url", server.url.clone().map(serde_json::Value::String)) - && field_matches("env", Some(serde_json::json!(server.env))) - && field_matches("headers", Some(serde_json::json!(server.headers))) - && field_matches( - "type", - server.transport_type.clone().map(serde_json::Value::String), - ) +fn normalized_mcp_value(value: &serde_json::Value) -> serde_json::Value { + match value { + serde_json::Value::Object(object) => { + let normalized = object + .iter() + .filter(|(_, value)| { + !value.is_null() + && !matches!(value, serde_json::Value::Array(values) if values.is_empty()) + && !matches!(value, serde_json::Value::Object(values) if values.is_empty()) + }) + .map(|(key, value)| (key.clone(), normalized_mcp_value(value))) + .collect::>(); + serde_json::Value::Object(normalized) + } + serde_json::Value::Array(values) => { + serde_json::Value::Array(values.iter().map(normalized_mcp_value).collect()) + } + _ => value.clone(), + } } fn mcp_server_report( @@ -605,7 +717,7 @@ fn classify_values(values: &BTreeMap) -> BTreeMap) -> BTreeMap bool { - let key = key.to_ascii_lowercase().replace(['-', '.'], "_"); - [ - "token", - "secret", - "password", - "passwd", - "credential", - "api_key", - "apikey", - "auth", - "cookie", - "credential", - "private_key", - "client_secret", - ] - .iter() - .any(|part| key.contains(part)) -} -fn is_environment_reference(value: &str) -> bool { - let value = value.trim(); - value.starts_with("${") - && value.ends_with('}') - && value.len() > 3 - && value[2..value.len() - 1] - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_') + crate::plugins::is_sensitive_mcp_key(key) } fn redact_inline_secret(value: &str) -> String { if let Some((key, _)) = value.split_once('=') @@ -680,26 +767,42 @@ fn redact_args(args: &[String]) -> Vec { redact_next = is_sensitive_key(key) && value.is_empty(); return arg.clone(); } - if is_environment_reference(arg) { - return "".into(); - } redact_next = false; - redact_inline_secret(arg) + let value = if arg.contains("://") { + redact_url(arg) + } else { + arg.clone() + }; + if crate::plugins::is_environment_reference(&value) { + "".into() + } else { + redact_inline_secret(&value) + } }) .collect() } fn redact_url(value: &str) -> String { let (scheme, rest) = value.split_once("://").unwrap_or(("", value)); - let (authority, path_query) = rest.split_once('/').unwrap_or((rest, "")); + let authority_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); + let (authority, path_query) = rest.split_at(authority_end); + let safe_authority = if let Some((_, host)) = authority.rsplit_once('@') { format!("@{host}") + } else if authority.starts_with('[') { + authority.to_string() + } else if let Some((host, port)) = authority.rsplit_once(':') { + if is_sensitive_key(host) { + format!(":{port}") + } else { + authority.to_string() + } } else { authority.to_string() }; let base_value = if scheme.is_empty() { - format!("{safe_authority}/{path_query}") + format!("{safe_authority}{path_query}") } else { - format!("{scheme}://{safe_authority}/{path_query}") + format!("{scheme}://{safe_authority}{path_query}") }; if let Some((base, query)) = base_value.split_once('?') { let parts = query @@ -708,7 +811,7 @@ fn redact_url(value: &str) -> String { let (key, val) = part.split_once('=').unwrap_or((part, "")); if is_sensitive_key(key) { format!("{key}=") - } else if is_environment_reference(val) { + } else if crate::plugins::is_environment_reference(val) { format!("{key}=") } else { part.into() @@ -729,11 +832,23 @@ fn plugin_owner(name: &str) -> String { } } -fn status_diagnostics(entry: &AgentStatusEntry, target: &str, root: &Path) -> Vec { +fn status_diagnostics( + entry: &AgentStatusEntry, + target: &str, + root: &Path, + state: ItemState, +) -> Vec { entry .entry .issues .iter() + .filter(|issue| { + !(state == ItemState::Projected + && matches!( + issue.kind, + StatusIssueKind::MissingExpectedSource | StatusIssueKind::MissingDestination + )) + }) .map(|issue| { let code = match issue.kind { StatusIssueKind::IncorrectLinkTarget => DiagnosticCode::DriftedSymlink, @@ -788,34 +903,32 @@ fn classify_target(name: &str) -> String { } .into() } -fn sync_type_name(sync_type: SyncType) -> &'static str { - match sync_type { - SyncType::Symlink => "symlink", - SyncType::SymlinkContents => "symlink-contents", - SyncType::NestedGlob => "nested-glob", - SyncType::ModuleMap => "module-map", - } -} -fn same_link_target(destination: &Path, actual: &Path, expected: &Path) -> bool { - let actual = if actual.is_absolute() { - actual.to_path_buf() - } else { - destination.parent().unwrap_or(Path::new(".")).join(actual) - }; - std::fs::canonicalize(&actual).unwrap_or(actual) - == std::fs::canonicalize(expected).unwrap_or_else(|_| expected.to_path_buf()) -} fn portable_path(root: &Path, path: &Path) -> String { let value = path.strip_prefix(root).unwrap_or(path); - value - .components() - .map(|component| match component { - Component::Normal(part) => part.to_string_lossy().to_string(), - Component::CurDir => ".".into(), - Component::ParentDir => "..".into(), - Component::RootDir => "/".into(), - Component::Prefix(prefix) => prefix.as_os_str().to_string_lossy().to_string(), - }) - .collect::>() - .join("/") + let mut result = String::new(); + for component in value.components() { + match component { + Component::Normal(part) => { + if !result.is_empty() && !result.ends_with('/') { + result.push('/'); + } + result.push_str(&part.to_string_lossy()); + } + Component::CurDir => { + if !result.is_empty() && !result.ends_with('/') { + result.push('/'); + } + result.push('.'); + } + Component::ParentDir => { + if !result.is_empty() && !result.ends_with('/') { + result.push('/'); + } + result.push_str(".."); + } + Component::RootDir => result.push('/'), + Component::Prefix(prefix) => result.push_str(&prefix.as_os_str().to_string_lossy()), + } + } + result } diff --git a/src/plugins.rs b/src/plugins.rs index 9473dfcb..597815cc 100644 --- a/src/plugins.rs +++ b/src/plugins.rs @@ -381,7 +381,11 @@ impl PluginManager { .unwrap_or_else(|| Path::new(".")) .join(location) } - LockedSourceKind::Git => self.git_source_cache_path(&locked.source)?, + LockedSourceKind::Git => { + let root = self.git_source_cache_path(&locked.source)?; + ensure_git_snapshot_dir(&root)?; + root + } }; ensure!( root.is_dir(), @@ -1768,7 +1772,7 @@ fn validate_plugin_mcp_server(name: &str, server: &McpServerConfig) -> Result<() Ok(()) } -fn is_sensitive_mcp_key(key: &str) -> bool { +pub(crate) fn is_sensitive_mcp_key(key: &str) -> bool { let normalized = key.to_ascii_lowercase().replace(['-', '.'], "_"); [ "token", @@ -1785,7 +1789,7 @@ fn is_sensitive_mcp_key(key: &str) -> bool { .any(|marker| normalized.contains(marker)) } -fn is_environment_reference(value: &str) -> bool { +pub(crate) fn is_environment_reference(value: &str) -> bool { let value = value.trim(); let valid_name = |name: &str| { !name.is_empty() diff --git a/src/status.rs b/src/status.rs index 17752b1d..910b521a 100644 --- a/src/status.rs +++ b/src/status.rs @@ -460,7 +460,7 @@ fn display_path(path: &Path) -> String { path.to_string_lossy().to_string() } -fn paths_match(destination: &Path, points_to: &Path, expected: &Path) -> bool { +pub(crate) fn paths_match(destination: &Path, points_to: &Path, expected: &Path) -> bool { let actual_path = if points_to.is_absolute() { points_to.to_path_buf() } else { @@ -475,7 +475,7 @@ fn paths_match(destination: &Path, points_to: &Path, expected: &Path) -> bool { expected_canon == actual_canon } -fn sync_type_label(sync_type: SyncType) -> &'static str { +pub(crate) fn sync_type_label(sync_type: SyncType) -> &'static str { match sync_type { SyncType::Symlink => "symlink", SyncType::SymlinkContents => "symlink-contents", diff --git a/tests/contracts/test_inspect_output.rs b/tests/contracts/test_inspect_output.rs index bb1ad85a..5274c369 100644 --- a/tests/contracts/test_inspect_output.rs +++ b/tests/contracts/test_inspect_output.rs @@ -75,8 +75,16 @@ type = "symlink-contents" fn inspect(project: &TempDir, agent: &str, strict: bool) -> std::process::Output { let mut command = Command::new(binary()); - command.args(["inspect", "--agent", agent, "--json", "--project-root"]); + command.args([ + "inspect", + "--agent", + if agent == "human" { "opencode" } else { agent }, + "--project-root", + ]); command.arg(project.path()); + if agent != "human" { + command.arg("--json"); + } if strict { command.arg("--strict"); } @@ -144,6 +152,8 @@ fn inspect_json_is_stable_read_only_and_secret_safe() { assert!(!String::from_utf8_lossy(&first.stdout).contains("${MCP_TOKEN}")); assert!(!String::from_utf8_lossy(&first.stdout).contains("CANARY_USERINFO_SECRET")); assert!(!String::from_utf8_lossy(&first.stdout).contains("CANARY_URL_SECRET")); + let url = value["mcp"]["servers"][0]["url"].as_str().unwrap(); + assert!(url.contains("example.invalid/path"), "redacted URL: {url}"); assert_eq!(std::fs::read_dir(project.path()).unwrap().count(), before); assert!(!project.path().join("AGENTS.md").exists()); assert!(!project.path().join(".opencode").exists()); @@ -168,6 +178,66 @@ fn inspect_codex_projects_canonical_sources_and_destinations() { assert_eq!(value["mcp"]["servers"][0]["name"], "fixture"); } +#[test] +fn inspect_module_map_projects_each_mapping_as_child() { + let project = TempDir::new().unwrap(); + let agents = project.path().join(".agents/source"); + std::fs::create_dir_all(&agents).unwrap(); + std::fs::write(agents.join("first.md"), "first\n").unwrap(); + std::fs::write(agents.join("second.md"), "second\n").unwrap(); + std::fs::write( + project.path().join(".agents/agentsync.toml"), + r#" +source_dir = "source" +[agents.codex] +enabled = true +[agents.codex.targets.modules] +source = "placeholder-source" +destination = "placeholder-destination" +type = "module-map" +[[agents.codex.targets.modules.mappings]] +source = "first.md" +destination = "src/one" +[[agents.codex.targets.modules.mappings]] +source = "second.md" +destination = "src/two" +"#, + ) + .unwrap(); + let apply = Command::new(binary()) + .current_dir(project.path()) + .arg("apply") + .output() + .unwrap(); + assert!( + apply.status.success(), + "{}", + String::from_utf8_lossy(&apply.stderr) + ); + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let item = value["items"].as_array().unwrap().first().unwrap(); + let children = item["children"].as_array().unwrap(); + assert_eq!(children.len(), 2); + assert_eq!(children[0]["source"], ".agents/source/first.md"); + assert_eq!(children[0]["destination"], "src/one/AGENTS.md"); + assert_eq!(children[1]["source"], ".agents/source/second.md"); + assert_eq!(children[1]["destination"], "src/two/AGENTS.md"); +} + +#[test] +fn inspect_human_diagnostics_show_code_severity_target_and_path() { + let project = project(); + std::fs::remove_file(project.path().join(".agents/AGENTS.md")).unwrap(); + let output = inspect(&project, "human", false); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("MissingSource [Error]")); + assert!(stdout.contains("target=instructions")); + assert!(stdout.contains("path=.agents/AGENTS.md")); +} + #[test] fn inspect_default_is_report_only_and_strict_fails_for_missing_source() { let project = project(); @@ -189,19 +259,21 @@ fn inspect_default_is_report_only_and_strict_fails_for_missing_source() { #[test] fn inspect_broken_symlink_has_distinct_typed_diagnostic() { let project = project(); - let path = project.path().join("AGENTS.md"); #[cfg(unix)] - std::os::unix::fs::symlink("missing-source.md", &path).unwrap(); - let output = inspect(&project, "codex", false); - assert!(output.status.success()); - let value: Value = serde_json::from_slice(&output.stdout).unwrap(); - assert!( - value["diagnostics"] - .as_array() - .unwrap() - .iter() - .any(|diagnostic| diagnostic["code"] == "broken-symlink") - ); + { + let path = project.path().join("AGENTS.md"); + std::os::unix::fs::symlink("missing-source.md", &path).unwrap(); + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| diagnostic["code"] == "broken-symlink") + ); + } } #[test] diff --git a/tests/contracts/test_skill_list_output.rs b/tests/contracts/test_skill_list_output.rs index 9370fe2b..2f81f10d 100644 --- a/tests/contracts/test_skill_list_output.rs +++ b/tests/contracts/test_skill_list_output.rs @@ -143,7 +143,11 @@ fn skill_list_json_scan_failure_has_distinct_diagnostic_and_preserves_error() { assert!(!output.status.success()); let value: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); - assert_eq!(value["diagnostics"][0]["code"], "inventory_unreadable"); + assert_eq!(value["diagnostic_code"], "inventory_unreadable"); + assert_eq!( + value["diagnostics"][0], + "skill inventory could not be scanned" + ); assert!(String::from_utf8_lossy(&output.stderr).contains("not a directory")); } @@ -165,9 +169,9 @@ fn skill_list_json_corrupt_registry_reports_redacted_diagnostic_and_fails() { let value: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); assert_eq!(value["schema_version"], 1); assert_eq!(value["skills"], serde_json::json!([])); - assert_eq!(value["diagnostics"][0]["code"], "registry_invalid"); + assert_eq!(value["diagnostic_code"], "registry_invalid"); assert_eq!( - value["diagnostics"][0]["message"], + value["diagnostics"][0], "installed skill registry could not be read or parsed" ); assert!(!String::from_utf8_lossy(&output.stdout).contains("not json")); diff --git a/website/docs/src/content/docs/reference/inspect-output.mdx b/website/docs/src/content/docs/reference/inspect-output.mdx index f17ba5c7..ef1e02b0 100644 --- a/website/docs/src/content/docs/reference/inspect-output.mdx +++ b/website/docs/src/content/docs/reference/inspect-output.mdx @@ -19,7 +19,8 @@ configuration and filesystem tree. It contains no timestamp. - `scope_notice`: explicit boundary from provider runtime-effective configuration. Each item reports `kind`, `target`, `sync_mode`, `source`, `destination`, `origin`, `owner`, -`provider_support`, `state`, `observed`, and optional `children`. Paths inside the project root are +`provider_support`, `state`, `observed`, and the always-present `children` array (empty when no child +projection applies). For `module-map`, each mapping is represented as a child. Paths inside the project root are relative and use `/` separators. Symlink `points_to` values are the observed link text and therefore may be relative to the destination. @@ -29,7 +30,7 @@ Item states are: - `projected`: source exists but the destination is not materialized. - `observed`: the materialized destination matches the projection. - `conflicted`: the materialized destination differs from the projection, or a collision exists. -- `unsupported`: AgentSync cannot project the requested capability for the selected assistant. +- `unsupported`: reserved for a future capability-specific projection; current v1 reports do not assign this state to targets or items. `provider_support: "unknown"` means the report does not assert that the provider natively supports the semantic capability. This is deliberate; `inspect` does not implement cross-provider adapters. From 6d49afc1f252548b30468997e25c1f09e971ddb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yuniel=20Acosta=20P=C3=A9rez?= <33158051+yacosta738@users.noreply.github.com> Date: Fri, 23 Oct 2026 20:33:35 +0200 Subject: [PATCH 4/5] fix: resolve PR 706 review findings --- Cargo.lock | 75 ++- Cargo.toml | 2 +- src/commands/skill.rs | 279 ++++++---- src/inspect.rs | 631 ++++++++++++---------- src/skills/detect.rs | 3 +- src/skills/manifest.rs | 5 +- tests/contracts/test_inspect_output.rs | 274 +++++++++- tests/contracts/test_skill_list_output.rs | 97 ++++ tests/unit/manifest.rs | 14 + tests/unit/mod.rs | 3 + 10 files changed, 959 insertions(+), 424 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c50fbfd5..2945a971 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -47,8 +47,8 @@ dependencies = [ "rustix", "semver", "serde", + "serde-saphyr", "serde_json", - "serde_yaml", "sha2", "tar", "tempfile", @@ -96,6 +96,17 @@ dependencies = [ "libc", ] +[[package]] +name = "annotate-snippets" +version = "0.12.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2a5b0a5ab8561c603f16c3b78c24a386b1f70e7fc1b1b1e8c15738828e4a176" +dependencies = [ + "anstyle", + "memchr", + "unicode-width", +] + [[package]] name = "anstream" version = "1.0.0" @@ -161,6 +172,12 @@ dependencies = [ "num-traits", ] +[[package]] +name = "arraydeque" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d902e3d592a523def97af8f317b08ce16b7ab854c1985a0c671e6f15cebc236" + [[package]] name = "async-compression" version = "0.4.43" @@ -608,9 +625,9 @@ dependencies = [ [[package]] name = "ctutils" -version = "0.4.2" +version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e" +checksum = "12218662ccac37d9714f2191252553d67585b97df8c839cbe49e0bde278dc865" dependencies = [ "cmov", ] @@ -776,6 +793,15 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "encoding_rs_io" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fba3fe847045ecff794b9c138293a80db914678c453ad63fbf0c6a9eb6e00b22" +dependencies = [ + "encoding_rs", +] + [[package]] name = "equivalent" version = "1.0.2" @@ -963,6 +989,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "granit-parser" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e20f99e46474f56bd905c56e817ebddcf377a611f94c53ac4649e4d3fa3c0cd0" +dependencies = [ + "arraydeque", + "smallvec", +] + [[package]] name = "h2" version = "0.4.16" @@ -2272,6 +2308,20 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-saphyr" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8050abb251097357e24aff63ba2c52a6309ecb7d23a5474023df960a02694d8" +dependencies = [ + "annotate-snippets", + "encoding_rs_io", + "granit-parser", + "num-traits", + "serde_core", + "smallvec", +] + [[package]] name = "serde_core" version = "1.0.229" @@ -2316,19 +2366,6 @@ dependencies = [ "serde_core", ] -[[package]] -name = "serde_yaml" -version = "0.9.34+deprecated" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" -dependencies = [ - "indexmap", - "itoa", - "ryu", - "serde", - "unsafe-libyaml", -] - [[package]] name = "sha1" version = "0.11.0" @@ -2928,12 +2965,6 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" -[[package]] -name = "unsafe-libyaml" -version = "0.2.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" - [[package]] name = "untrusted" version = "0.9.0" diff --git a/Cargo.toml b/Cargo.toml index 6011ca63..a8f83d53 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,7 @@ serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", features = ["preserve_order"] } toml = "1.0" toml_edit = "0.25.0" -serde_yaml = "0.9" +serde-saphyr = { version = "1", default-features = false, features = ["deserialize"] } regex = "1" chrono = { version = "0.4", features = ["serde"] } zip = "9" diff --git a/src/commands/skill.rs b/src/commands/skill.rs index 71dc117c..22bb3e44 100644 --- a/src/commands/skill.rs +++ b/src/commands/skill.rs @@ -832,17 +832,21 @@ fn skill_inventory( None }; let entries = registry_data.as_ref().and_then(|reg| reg.skills.as_ref()); - let skills_root_metadata = match std::fs::symlink_metadata(&skills_root) { + let skills_root_metadata = match std::fs::symlink_metadata(&skills_root).with_context(|| { + format!( + "failed to inspect skills directory {}", + skills_root.display() + ) + }) { Ok(metadata) => Some(metadata), - Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, - Err(error) => { - return Err(SkillInventoryError::Scan( - anyhow::Error::new(error).context(format!( - "failed to inspect skills directory {}", - skills_root.display() - )), - )); + Err(error) + if error + .downcast_ref::() + .is_some_and(|error| error.kind() == std::io::ErrorKind::NotFound) => + { + None } + Err(error) => return Err(SkillInventoryError::Scan(error)), }; if skills_root_metadata.as_ref().is_some_and(|metadata| { !metadata.is_dir() && !(metadata.file_type().is_symlink() && skills_root.is_dir()) @@ -856,90 +860,120 @@ fn skill_inventory( let mut manifest_ids = std::collections::BTreeMap::>::new(); let mut diagnostics = Vec::new(); if skills_root_metadata.is_some() { - let directory = std::fs::read_dir(&skills_root) - .with_context(|| format!("failed to read skills directory {}", skills_root.display())) - .map_err(SkillInventoryError::Scan)?; - let mut items = directory - .collect::>>() - .context("failed to collect skill directory entries") - .map_err(SkillInventoryError::Scan)?; - items.sort_by_key(|item| item.file_name()); + let items = read_skill_directory(&skills_root)?; + let mut context = InventoryEntryContext { + entries, + found: &mut found, + manifest_ids: &mut manifest_ids, + diagnostics: &mut diagnostics, + }; for item in items { - let name = item.file_name().to_string_lossy().into_owned(); - if name == "registry.json" { - continue; - } - let path = item.path(); - let relative = format!(".agents/skills/{name}"); - let metadata = std::fs::symlink_metadata(&path) - .with_context(|| format!("failed to inspect skill entry {}", path.display())) - .map_err(SkillInventoryError::Scan)?; - let is_link = metadata.file_type().is_symlink(); - if is_link && !path.exists() { - diagnostics.push(format!("broken symlink: {relative}")); - found.insert( - name.clone(), - InventorySkill { - id: name, - path: relative, - source_kind: "unknown".into(), - version: None, - provenance: None, - state: "broken_symlink".into(), - }, - ); - continue; - } - if is_link { - continue; - } - if !path.is_dir() { - continue; - } - let manifest_path = path.join("SKILL.md"); - let manifest_exists = manifest_path.is_file(); - let parsed = agentsync::skills::manifest::parse_skill_manifest(&manifest_path); - let entry = entries.and_then(|all| all.get(&name)); - let source_kind = inventory_source_kind(entry); - let state = if parsed.is_err() { - if manifest_exists { - diagnostics.push(format!("malformed SKILL.md: {relative}")); - "malformed" - } else { - diagnostics.push(format!("missing SKILL.md: {relative}")); - "missing_manifest" - } - } else { - "installed" - }; - let manifest = parsed.ok(); - if let Some(manifest) = &manifest { - manifest_ids - .entry(manifest.name.clone()) - .or_default() - .push(name.clone()); - } - found.insert( - name.clone(), - InventorySkill { - id: name, - path: relative, - source_kind: source_kind.into(), - version: manifest - .as_ref() - .and_then(|m| m.version.clone()) - .or_else(|| entry.and_then(|e| e.version.clone())), - provenance: entry - .and_then(|e| e.source.as_deref()) - .filter(|source| { - source.starts_with("https://") || source.starts_with("git@") - }) - .map(agentsync::logging::redact_url), - state: state.into(), - }, - ); + add_inventory_entry(item, &mut context)?; } } + mark_duplicate_inventory_ids(&manifest_ids, &mut found, &mut diagnostics); + add_missing_registry_entries(entries, &mut found, &mut diagnostics); + Ok(SkillInventory { + schema_version: 1, + skills: found.into_values().collect(), + diagnostics, + }) +} + +struct InventoryEntryContext<'a> { + entries: Option<&'a std::collections::BTreeMap>, + found: &'a mut std::collections::BTreeMap, + manifest_ids: &'a mut std::collections::BTreeMap>, + diagnostics: &'a mut Vec, +} + +fn add_inventory_entry( + item: std::fs::DirEntry, + context: &mut InventoryEntryContext<'_>, +) -> std::result::Result<(), SkillInventoryError> { + let name = item.file_name().to_string_lossy().into_owned(); + if name == "registry.json" { + return Ok(()); + } + let path = item.path(); + let relative = format!(".agents/skills/{name}"); + let metadata = std::fs::symlink_metadata(&path) + .with_context(|| format!("failed to inspect skill entry {}", path.display())) + .map_err(SkillInventoryError::Scan)?; + let is_link = metadata.file_type().is_symlink(); + if is_link && !path.exists() { + context + .diagnostics + .push(format!("broken symlink: {relative}")); + context.found.insert( + name.clone(), + InventorySkill { + id: name, + path: relative, + source_kind: "unknown".into(), + version: None, + provenance: None, + state: "broken_symlink".into(), + }, + ); + return Ok(()); + } + if is_link || !path.is_dir() { + return Ok(()); + } + + let manifest_path = path.join("SKILL.md"); + let manifest_exists = manifest_path.is_file(); + let parsed = agentsync::skills::manifest::parse_skill_manifest(&manifest_path); + let entry = context.entries.and_then(|all| all.get(&name)); + let state = if parsed.is_err() { + if manifest_exists { + context + .diagnostics + .push(format!("malformed SKILL.md: {relative}")); + "malformed" + } else { + context + .diagnostics + .push(format!("missing SKILL.md: {relative}")); + "missing_manifest" + } + } else { + "installed" + }; + let manifest = parsed.ok(); + if let Some(manifest) = &manifest { + context + .manifest_ids + .entry(manifest.name.clone()) + .or_default() + .push(name.clone()); + } + context.found.insert( + name.clone(), + InventorySkill { + id: name, + path: relative, + source_kind: inventory_source_kind(entry).into(), + version: manifest + .as_ref() + .and_then(|manifest| manifest.version.clone()) + .or_else(|| entry.and_then(|entry| entry.version.clone())), + provenance: entry + .and_then(|entry| entry.source.as_deref()) + .filter(|source| source.starts_with("https://") || source.starts_with("git@")) + .map(agentsync::logging::redact_url), + state: state.into(), + }, + ); + Ok(()) +} + +fn mark_duplicate_inventory_ids( + manifest_ids: &std::collections::BTreeMap>, + found: &mut std::collections::BTreeMap, + diagnostics: &mut Vec, +) { for (manifest_id, directory_ids) in manifest_ids.iter().filter(|(_, ids)| ids.len() > 1) { diagnostics.push(format!( "duplicate skill ID '{manifest_id}' in directories: {}", @@ -951,35 +985,52 @@ fn skill_inventory( } } } +} + +fn add_missing_registry_entries( + entries: Option<&std::collections::BTreeMap>, + found: &mut std::collections::BTreeMap, + diagnostics: &mut Vec, +) { if let Some(entries) = entries { for (id, entry) in entries { - if !found.contains_key(id) { - found.insert( - id.clone(), - InventorySkill { - id: id.clone(), - path: format!(".agents/skills/{id}"), - source_kind: inventory_source_kind(Some(entry)).into(), - version: entry.version.clone(), - provenance: entry - .source - .as_deref() - .filter(|source| { - source.starts_with("https://") || source.starts_with("git@") - }) - .map(agentsync::logging::redact_url), - state: "missing".into(), - }, - ); - diagnostics.push(format!("registry entry has no skill files: {id}")); + if found.contains_key(id) { + continue; } + found.insert( + id.clone(), + InventorySkill { + id: id.clone(), + path: format!(".agents/skills/{id}"), + source_kind: inventory_source_kind(Some(entry)).into(), + version: entry.version.clone(), + provenance: entry + .source + .as_deref() + .filter(|source| { + source.starts_with("https://") || source.starts_with("git@") + }) + .map(agentsync::logging::redact_url), + state: "missing".into(), + }, + ); + diagnostics.push(format!("registry entry has no skill files: {id}")); } } - Ok(SkillInventory { - schema_version: 1, - skills: found.into_values().collect(), - diagnostics, - }) +} + +fn read_skill_directory( + skills_root: &Path, +) -> std::result::Result, SkillInventoryError> { + let directory = std::fs::read_dir(skills_root) + .with_context(|| format!("failed to read skills directory {}", skills_root.display())) + .map_err(SkillInventoryError::Scan)?; + let mut items = directory + .collect::>>() + .context("failed to collect skill directory entries") + .map_err(SkillInventoryError::Scan)?; + items.sort_by_key(|item| item.file_name()); + Ok(items) } fn skill_label_kind(state: &str) -> LabelKind { diff --git a/src/inspect.rs b/src/inspect.rs index 90b0a9dd..d5680545 100644 --- a/src/inspect.rs +++ b/src/inspect.rs @@ -180,225 +180,16 @@ pub fn resolve_configuration( }); } if agent.enabled { + let mut context = TargetResolutionContext { + agent_name: &agent_name, + project_root: &project_root, + source_root: &source_root, + status: &status, + destinations: &mut destinations, + diagnostics: &mut diagnostics, + }; for (target_name, target) in &agent.targets { - let kind = classify_target(target_name); - let is_module_map = target.sync_type == SyncType::ModuleMap; - let source = if target.sync_type == SyncType::NestedGlob { - project_root.join(&target.source) - } else { - source_root.join(&target.source) - }; - let mappings = if is_module_map { - target - .mappings - .iter() - .map(|mapping| { - ( - source_root.join(&mapping.source), - project_root.join(&mapping.destination).join( - crate::config::resolve_module_map_filename(mapping, &agent_name), - ), - ) - }) - .collect::>() - } else { - vec![(source.clone(), project_root.join(&target.destination))] - }; - let (projection_source, projection_destination) = mappings - .first() - .cloned() - .unwrap_or_else(|| (source.clone(), project_root.join(&target.destination))); - let portable_source = portable_path(&project_root, &projection_source); - let portable_destination = portable_path(&project_root, &projection_destination); - for (_, mapped_destination) in &mappings { - let mapped_destination = portable_path(&project_root, mapped_destination); - if let Some(previous) = - destinations.insert(mapped_destination.clone(), target_name.clone()) - { - diagnostics.push(Diagnostic { - code: DiagnosticCode::DestinationCollision, - severity: DiagnosticSeverity::Error, - target: Some(target_name.clone()), - path: Some(mapped_destination), - message: format!( - "Targets {previous} and {target_name} project to the same destination." - ), - }); - } - } - let matched = status - .iter() - .filter(|entry| entry.agent == agent_name && entry.target == *target_name) - .collect::>(); - let observed = observed_for(&matched); - let source_exists = mappings - .iter() - .all(|(mapped_source, _)| mapped_source.exists()); - let destination_occupied = target.sync_type == SyncType::Symlink - && std::fs::symlink_metadata(&projection_destination).is_ok(); - let mut state = if !source_exists { - ItemState::Configured - } else if observed.matches_projection == Some(true) { - ItemState::Observed - } else if observed.exists || destination_occupied { - ItemState::Conflicted - } else { - ItemState::Projected - }; - if !source_exists { - let missing_source = mappings - .iter() - .find(|(mapped_source, _)| !mapped_source.exists()) - .map(|(mapped_source, _)| portable_path(&project_root, mapped_source)) - .unwrap_or_else(|| portable_source.clone()); - diagnostics.push(Diagnostic { - code: DiagnosticCode::MissingSource, - severity: DiagnosticSeverity::Error, - target: Some(target_name.clone()), - path: Some(missing_source), - message: "Configured source does not exist.".into(), - }); - } - for (index, entry) in matched.iter().enumerate() { - let entry_state = if is_module_map { - let source_exists = mappings - .get(index) - .is_some_and(|(source, _)| source.exists()); - if !source_exists { - ItemState::Configured - } else if entry.entry.issues.is_empty() && entry.entry.exists { - ItemState::Observed - } else if entry.entry.exists { - ItemState::Conflicted - } else { - ItemState::Projected - } - } else { - state - }; - diagnostics.extend(status_diagnostics( - entry, - target_name, - &project_root, - entry_state, - )); - if entry.entry.destination_kind == DestinationKind::Symlink - && entry.entry.points_to.as_deref().is_some_and(|target| { - let target = Path::new(target); - let resolved = if target.is_absolute() { - target.to_path_buf() - } else { - Path::new(&entry.entry.destination) - .parent() - .unwrap_or(Path::new(".")) - .join(target) - }; - !resolved.exists() - }) - { - diagnostics.push(Diagnostic { - code: DiagnosticCode::BrokenSymlink, - severity: DiagnosticSeverity::Error, - target: Some(target_name.clone()), - path: Some(portable_destination.clone()), - message: "Destination is a broken or unreadable symlink.".into(), - }); - state = ItemState::Conflicted; - } - } - let mut children = Vec::new(); - if is_module_map { - for ((mapped_source, mapped_destination), entry) in - mappings.iter().zip(matched.iter()) - { - let exists = mapped_source.exists(); - let matches = entry.entry.issues.is_empty() && entry.entry.exists; - children.push(ManagedChild { - source: portable_path(&project_root, mapped_source), - destination: portable_path(&project_root, mapped_destination), - state: if !exists { - ItemState::Configured - } else if matches { - ItemState::Observed - } else if entry.entry.exists { - ItemState::Conflicted - } else { - ItemState::Projected - }, - observed: ObservedState { - exists: entry.entry.exists, - kind: format!("{:?}", entry.entry.destination_kind) - .to_ascii_lowercase(), - points_to: entry - .entry - .points_to - .as_ref() - .map(|path| portable_path(&project_root, Path::new(path))), - matches_projection: Some(matches), - }, - }); - } - } else if let Some(entry) = matched - .iter() - .find(|entry| entry.entry.managed_children.is_some()) - && let Some(entry) = entry.entry.managed_children.as_ref() - { - for child in entry { - let child_source = - portable_path(&project_root, Path::new(&child.expected_source)); - let child_destination = portable_path(&project_root, Path::new(&child.path)); - let child_matches = child.exists - && child.is_symlink - && child.points_to.as_deref().is_some_and(|actual| { - paths_match( - Path::new(&child.path), - Path::new(actual), - Path::new(&child.expected_source), - ) - }); - let child_state = if !child.exists { - ItemState::Projected - } else if child_matches { - ItemState::Observed - } else { - ItemState::Conflicted - }; - children.push(ManagedChild { - source: child_source, - destination: child_destination, - state: child_state, - observed: ObservedState { - exists: child.exists, - kind: if child.is_symlink { - "symlink" - } else if child.exists { - "other" - } else { - "missing" - } - .into(), - points_to: child - .points_to - .as_deref() - .map(|path| portable_path(&project_root, Path::new(path))), - matches_projection: Some(child_matches), - }, - }); - } - } - items.push(ManagedItem { - kind, - target: target_name.clone(), - sync_mode: sync_type_label(target.sync_type).into(), - source: portable_source, - destination: portable_destination, - origin: "config".into(), - owner: "agentsync".into(), - provider_support: "unknown".into(), - state, - observed, - children, - }); + items.push(resolve_target(target_name, target, &mut context)); } } items.sort_by(|a, b| { @@ -431,6 +222,241 @@ pub fn resolve_configuration( }) } +struct TargetResolutionContext<'a> { + agent_name: &'a str, + project_root: &'a Path, + source_root: &'a Path, + status: &'a [AgentStatusEntry], + destinations: &'a mut BTreeMap, + diagnostics: &'a mut Vec, +} + +fn resolve_target( + target_name: &String, + target: &crate::config::TargetConfig, + context: &mut TargetResolutionContext<'_>, +) -> ManagedItem { + let agent_name = context.agent_name; + let project_root = context.project_root; + let source_root = context.source_root; + let status = context.status; + let destinations = &mut *context.destinations; + let diagnostics = &mut *context.diagnostics; + let kind = classify_target(target_name); + let is_module_map = target.sync_type == SyncType::ModuleMap; + let source = if target.sync_type == SyncType::NestedGlob { + project_root.join(&target.source) + } else { + source_root.join(&target.source) + }; + let mappings = if is_module_map { + target + .mappings + .iter() + .map(|mapping| { + ( + source_root.join(&mapping.source), + project_root.join(&mapping.destination).join( + crate::config::resolve_module_map_filename(mapping, agent_name), + ), + ) + }) + .collect::>() + } else { + vec![(source.clone(), project_root.join(&target.destination))] + }; + let (projection_source, projection_destination) = mappings + .first() + .cloned() + .unwrap_or_else(|| (source.clone(), project_root.join(&target.destination))); + let portable_source = portable_path(project_root, &projection_source); + let portable_destination = portable_path(project_root, &projection_destination); + for (_, mapped_destination) in &mappings { + let mapped_destination = portable_path(project_root, mapped_destination); + if let Some(previous) = destinations.insert(mapped_destination.clone(), target_name.clone()) + { + diagnostics.push(Diagnostic { + code: DiagnosticCode::DestinationCollision, + severity: DiagnosticSeverity::Error, + target: Some(target_name.clone()), + path: Some(mapped_destination), + message: format!( + "Targets {previous} and {target_name} project to the same destination." + ), + }); + } + } + let matched = status + .iter() + .filter(|entry| entry.agent == agent_name && entry.target == *target_name) + .collect::>(); + let observed = observed_for(&matched); + let source_exists = mappings + .iter() + .all(|(mapped_source, _)| mapped_source.exists()); + let destination_occupied = target.sync_type == SyncType::Symlink + && std::fs::symlink_metadata(&projection_destination).is_ok(); + let mut state = if !source_exists { + ItemState::Configured + } else if observed.matches_projection == Some(true) { + ItemState::Observed + } else if observed.exists || destination_occupied { + ItemState::Conflicted + } else { + ItemState::Projected + }; + if !source_exists { + let missing_source = mappings + .iter() + .find(|(mapped_source, _)| !mapped_source.exists()) + .map(|(mapped_source, _)| portable_path(project_root, mapped_source)) + .unwrap_or_else(|| portable_source.clone()); + diagnostics.push(Diagnostic { + code: DiagnosticCode::MissingSource, + severity: DiagnosticSeverity::Error, + target: Some(target_name.clone()), + path: Some(missing_source), + message: "Configured source does not exist.".into(), + }); + } + for (index, entry) in matched.iter().enumerate() { + let entry_state = if is_module_map { + let source_exists = mappings + .get(index) + .is_some_and(|(source, _)| source.exists()); + if !source_exists { + ItemState::Configured + } else if entry.entry.issues.is_empty() && entry.entry.exists { + ItemState::Observed + } else if entry.entry.exists { + ItemState::Conflicted + } else { + ItemState::Projected + } + } else { + state + }; + diagnostics.extend(status_diagnostics( + entry, + target_name, + project_root, + entry_state, + )); + if entry.entry.destination_kind == DestinationKind::Symlink + && entry.entry.points_to.as_deref().is_some_and(|target| { + let target = Path::new(target); + let resolved = if target.is_absolute() { + target.to_path_buf() + } else { + Path::new(&entry.entry.destination) + .parent() + .unwrap_or(Path::new(".")) + .join(target) + }; + !resolved.exists() + }) + { + diagnostics.push(Diagnostic { + code: DiagnosticCode::BrokenSymlink, + severity: DiagnosticSeverity::Error, + target: Some(target_name.clone()), + path: Some(portable_destination.clone()), + message: "Destination is a broken or unreadable symlink.".into(), + }); + state = ItemState::Conflicted; + } + } + let mut children = Vec::new(); + if is_module_map { + for ((mapped_source, mapped_destination), entry) in mappings.iter().zip(matched.iter()) { + let exists = mapped_source.exists(); + let matches = entry.entry.issues.is_empty() && entry.entry.exists; + children.push(ManagedChild { + source: portable_path(project_root, mapped_source), + destination: portable_path(project_root, mapped_destination), + state: if !exists { + ItemState::Configured + } else if matches { + ItemState::Observed + } else if entry.entry.exists { + ItemState::Conflicted + } else { + ItemState::Projected + }, + observed: ObservedState { + exists: entry.entry.exists, + kind: format!("{:?}", entry.entry.destination_kind).to_ascii_lowercase(), + points_to: entry + .entry + .points_to + .as_ref() + .map(|path| portable_path(project_root, Path::new(path))), + matches_projection: Some(matches), + }, + }); + } + } else if let Some(entry) = matched + .iter() + .find(|entry| entry.entry.managed_children.is_some()) + && let Some(entry) = entry.entry.managed_children.as_ref() + { + for child in entry { + let child_source = portable_path(project_root, Path::new(&child.expected_source)); + let child_destination = portable_path(project_root, Path::new(&child.path)); + let child_matches = child.exists + && child.is_symlink + && child.points_to.as_deref().is_some_and(|actual| { + paths_match( + Path::new(&child.path), + Path::new(actual), + Path::new(&child.expected_source), + ) + }); + let child_state = if !child.exists { + ItemState::Projected + } else if child_matches { + ItemState::Observed + } else { + ItemState::Conflicted + }; + children.push(ManagedChild { + source: child_source, + destination: child_destination, + state: child_state, + observed: ObservedState { + exists: child.exists, + kind: if child.is_symlink { + "symlink" + } else if child.exists { + "other" + } else { + "missing" + } + .into(), + points_to: child + .points_to + .as_deref() + .map(|path| portable_path(project_root, Path::new(path))), + matches_projection: Some(child_matches), + }, + }); + } + } + ManagedItem { + kind, + target: target_name.clone(), + sync_mode: sync_type_label(target.sync_type).into(), + source: portable_source, + destination: portable_destination, + origin: "config".into(), + owner: "agentsync".into(), + provider_support: "unknown".into(), + state, + observed, + children, + } +} + fn empty_mcp_report() -> McpReport { McpReport { capability: "not-configured".into(), @@ -548,71 +574,18 @@ fn resolve_mcp( }); } } - let plugins = &config.plugins; - if plugins.enabled && !plugins.selections.is_empty() { - let manager = crate::plugins::PluginManager::new( - root.to_path_buf(), - config_path.to_path_buf(), - plugins.clone(), - ); - match manager.inspect_status_report_read_only() { - Ok(report) => { - for item in report.servers { - let approved = - matches!(item.approval, crate::plugins::PluginMcpApproval::Allowed); - let collision = config - .mcp_servers - .get(&item.name) - .is_some_and(|explicit| explicit != &item.server); - if collision { - diagnostics.push(Diagnostic { code: DiagnosticCode::McpNameCollision, severity: DiagnosticSeverity::Error, target: Some("mcp".into()), path: Some(dest.clone()), message: format!("Explicit MCP declaration and plugin contribution collide at {}; apply rejects differing definitions.", item.name) }); - } - let plugin_expected = formatter - .format_to_string(&BTreeMap::from([(item.name.as_str(), &item.server)])) - .ok() - .and_then(|formatted| formatter.parse_existing(&formatted).ok()) - .and_then(|servers| servers.get(&item.name).cloned()); - let existing_equal = existing_servers - .get(&item.name) - .zip(plugin_expected.as_ref()) - .is_some_and(|(existing, expected)| { - normalized_mcp_value(existing) == normalized_mcp_value(expected) - }); - let mut server_report = mcp_server_report( - &item.name, - &item.server, - "plugin", - Some(plugin_owner(&item.name)), - existing_equal, - root, - &dest, - ); - server_report.included = approved && !collision && !item.server.disabled; - server_report.state = if item.server.disabled || !approved { - ItemState::Configured - } else if existing_equal { - ItemState::Observed - } else { - ItemState::Projected - }; - servers.push(server_report); - if !approved { - diagnostics.push(Diagnostic { code: DiagnosticCode::PendingPluginApproval, severity: DiagnosticSeverity::Warning, target: Some("mcp".into()), path: Some(dest.clone()), message: format!("Plugin MCP server {} is not approved by plugins.allowed_mcp and is not included.", item.name) }); - } - } - } - Err(error) => { - let lock_path = manager.lock_path()?; - let lock = PluginLock::load(&lock_path).ok(); - let missing_snapshot = lock.as_ref().is_some_and(|lock| { - lock.plugins.values().any(|plugin| { - plugin.source.kind == crate::plugins::LockedSourceKind::Git - }) - }); - diagnostics.push(Diagnostic { code: if missing_snapshot { DiagnosticCode::MissingPluginSnapshot } else { DiagnosticCode::UnsupportedPluginComponent }, severity: DiagnosticSeverity::Warning, target: Some("plugins".into()), path: Some(portable_path(root, &lock_path)), message: if missing_snapshot { "A required local plugin snapshot is unavailable; plugin MCP contributions are unknown.".into() } else { format!("Plugin contributions could not be resolved offline: {}", error.to_string().split(':').next().unwrap_or("plugin metadata error")) } }); - } - } - } + append_plugin_mcp_servers( + PluginMcpContext { + config, + config_path, + agent, + root, + destination: &dest, + existing_servers: &existing_servers, + }, + &mut servers, + diagnostics, + )?; } servers.sort_by(|a, b| a.name.cmp(&b.name)); let existing_unmanaged = existing_names @@ -649,6 +622,98 @@ fn resolve_mcp( }) } +struct PluginMcpContext<'a> { + config: &'a Config, + config_path: &'a Path, + agent: McpAgent, + root: &'a Path, + destination: &'a str, + existing_servers: &'a BTreeMap, +} + +fn append_plugin_mcp_servers( + context: PluginMcpContext<'_>, + servers: &mut Vec, + diagnostics: &mut Vec, +) -> Result<()> { + let PluginMcpContext { + config, + config_path, + agent, + root, + destination, + existing_servers, + } = context; + let dest = destination; + let plugins = &config.plugins; + if !plugins.enabled || plugins.selections.is_empty() { + return Ok(()); + } + let manager = crate::plugins::PluginManager::new( + root.to_path_buf(), + config_path.to_path_buf(), + plugins.clone(), + ); + let formatter = agent.formatter(); + match manager.inspect_status_report_read_only() { + Ok(report) => { + for item in report.servers { + let approved = matches!(item.approval, crate::plugins::PluginMcpApproval::Allowed); + let collision = config + .mcp_servers + .get(&item.name) + .is_some_and(|explicit| explicit != &item.server); + if collision { + diagnostics.push(Diagnostic { code: DiagnosticCode::McpNameCollision, severity: DiagnosticSeverity::Error, target: Some("mcp".into()), path: Some(dest.into()), message: format!("Explicit MCP declaration and plugin contribution collide at {}; apply rejects differing definitions.", item.name) }); + } + let plugin_expected = formatter + .format_to_string(&BTreeMap::from([(item.name.as_str(), &item.server)])) + .ok() + .and_then(|formatted| formatter.parse_existing(&formatted).ok()) + .and_then(|servers| servers.get(&item.name).cloned()); + let existing_equal = existing_servers + .get(&item.name) + .zip(plugin_expected.as_ref()) + .is_some_and(|(existing, expected)| { + normalized_mcp_value(existing) == normalized_mcp_value(expected) + }); + let mut server_report = mcp_server_report( + &item.name, + &item.server, + "plugin", + Some(plugin_owner(&item.name)), + existing_equal, + root, + dest, + ); + server_report.included = approved && !collision && !item.server.disabled; + server_report.state = if item.server.disabled || !approved { + ItemState::Configured + } else if existing_equal { + ItemState::Observed + } else { + ItemState::Projected + }; + servers.push(server_report); + if !approved { + diagnostics.push(Diagnostic { code: DiagnosticCode::PendingPluginApproval, severity: DiagnosticSeverity::Warning, target: Some("mcp".into()), path: Some(dest.into()), message: format!("Plugin MCP server {} is not approved by plugins.allowed_mcp and is not included.", item.name) }); + } + } + } + Err(error) => { + let lock_path = manager.lock_path()?; + let lock = PluginLock::load(&lock_path).ok(); + let missing_snapshot = lock.as_ref().is_some_and(|lock| { + lock.plugins + .values() + .any(|plugin| plugin.source.kind == crate::plugins::LockedSourceKind::Git) + }); + diagnostics.push(Diagnostic { code: if missing_snapshot { DiagnosticCode::MissingPluginSnapshot } else { DiagnosticCode::UnsupportedPluginComponent }, severity: DiagnosticSeverity::Warning, target: Some("plugins".into()), path: Some(portable_path(root, &lock_path)), message: if missing_snapshot { "A required local plugin snapshot is unavailable; plugin MCP contributions are unknown.".into() } else { format!("Plugin contributions could not be resolved offline: {}", error.to_string().split(':').next().unwrap_or("plugin metadata error")) } }); + } + } + Ok(()) +} + fn normalized_mcp_value(value: &serde_json::Value) -> serde_json::Value { match value { serde_json::Value::Object(object) => { diff --git a/src/skills/detect.rs b/src/skills/detect.rs index ebf43656..8a5c3e4f 100644 --- a/src/skills/detect.rs +++ b/src/skills/detect.rs @@ -983,7 +983,8 @@ fn parse_pnpm_workspace_yaml(content: &str) -> Vec { packages: Vec, } - serde_yaml::from_str::(content) + let options = serde_saphyr::options! { strict_booleans: true }; + serde_saphyr::from_str_with_options::(content, options) .map(|ws| ws.packages) .unwrap_or_default() } diff --git a/src/skills/manifest.rs b/src/skills/manifest.rs index 3fd5db67..4c32cec0 100644 --- a/src/skills/manifest.rs +++ b/src/skills/manifest.rs @@ -40,8 +40,9 @@ pub fn parse_skill_manifest(path: &Path) -> Result TempDir { + let project = project(); + let fixture = + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/plugin-marketplace"); + copy_tree(&fixture, &project.path().join("marketplace")); + let config_path = project.path().join(".agents/agentsync.toml"); + let mut config = std::fs::read_to_string(&config_path).unwrap(); + config.push_str( + r#" +[plugins] +enabled = true +lockfile = "plugins.lock.toml" +allowed_mcp = ["plugin/internal/engineering/safe-fixture"] + +[plugins.marketplaces.internal] +source = "../marketplace" +reference = "main" + +[[plugins.selections]] +marketplace = "internal" +plugin = "engineering" +"#, + ); + std::fs::write(config_path, config).unwrap(); + let add = Command::new(binary()) + .args(["plugin", "--project-root"]) + .arg(project.path()) + .args(["add", "internal/engineering"]) + .output() + .unwrap(); + assert!( + add.status.success(), + "plugin add failed: {}", + String::from_utf8_lossy(&add.stderr) + ); + project +} + +fn copy_tree(source: &std::path::Path, target: &std::path::Path) { + std::fs::create_dir_all(target).unwrap(); + for entry in std::fs::read_dir(source).unwrap() { + let entry = entry.unwrap(); + let source_path = entry.path(); + let target_path = target.join(entry.file_name()); + if source_path.is_dir() { + copy_tree(&source_path, &target_path); + } else { + std::fs::create_dir_all(target_path.parent().unwrap()).unwrap(); + std::fs::copy(source_path, target_path).unwrap(); + } + } +} + fn inspect(project: &TempDir, agent: &str, strict: bool) -> std::process::Output { let mut command = Command::new(binary()); command.args([ @@ -91,6 +144,176 @@ fn inspect(project: &TempDir, agent: &str, strict: bool) -> std::process::Output command.output().unwrap() } +#[test] +fn inspect_projects_approved_plugin_mcp_and_reports_pending_plugin_mcp() { + let project = inspect_plugin_project(); + + let config_path = project.path().join(".agents/agentsync.toml"); + let config = std::fs::read_to_string(&config_path).unwrap().replace( + "allowed_mcp = [\"plugin/internal/engineering/safe-fixture\"]", + "allowed_mcp = []", + ); + std::fs::write(config_path, config).unwrap(); + + let output = inspect(&project, "opencode", false); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let servers = value["mcp"]["servers"].as_array().unwrap(); + let plugin_server = servers + .iter() + .find(|server| server["name"] == "plugin/internal/engineering/safe-fixture") + .unwrap(); + assert_eq!(plugin_server["origin"], "plugin"); + assert_eq!(plugin_server["state"], "configured"); + assert_eq!(plugin_server["included"], false); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { + diagnostic["code"] == "pending-plugin-approval" + && diagnostic["severity"] == "warning" + }) + ); + + let config_path = project.path().join(".agents/agentsync.toml"); + let config = std::fs::read_to_string(&config_path).unwrap().replace( + "allowed_mcp = []", + "allowed_mcp = [\"plugin/internal/engineering/safe-fixture\"]", + ); + std::fs::write(config_path, config).unwrap(); + let output = inspect(&project, "opencode", false); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let plugin_server = value["mcp"]["servers"] + .as_array() + .unwrap() + .iter() + .find(|server| server["name"] == "plugin/internal/engineering/safe-fixture") + .unwrap(); + assert_eq!(plugin_server["state"], "projected"); + assert_eq!(plugin_server["included"], true); +} + +#[test] +fn inspect_reports_plugin_mcp_collision_with_explicit_server() { + let project = inspect_plugin_project(); + let config_path = project.path().join(".agents/agentsync.toml"); + let mut config = std::fs::read_to_string(&config_path).unwrap(); + config.push_str( + r#" +[mcp_servers."plugin/internal/engineering/safe-fixture"] +command = "/bin/true" +"#, + ); + std::fs::write(config_path, config).unwrap(); + + let output = inspect(&project, "opencode", false); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let plugin_server = value["mcp"]["servers"] + .as_array() + .unwrap() + .iter() + .find(|server| { + server["name"] == "plugin/internal/engineering/safe-fixture" + && server["origin"] == "plugin" + }) + .unwrap(); + assert_eq!(plugin_server["included"], false); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { + diagnostic["code"] == "mcp-name-collision" && diagnostic["severity"] == "error" + }) + ); +} + +#[test] +fn inspect_reports_plugin_error_when_lockfile_is_missing() { + let project = inspect_plugin_project(); + std::fs::remove_file(project.path().join(".agents/plugins.lock.toml")).unwrap(); + + let output = inspect(&project, "opencode", false); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { + diagnostic["code"] == "unsupported-plugin-component" + && diagnostic["severity"] == "warning" + }) + ); +} + +#[test] +fn inspect_reports_missing_plugin_snapshot_from_git_lock_entry() { + let project = inspect_plugin_project(); + let lock_path = project.path().join(".agents/plugins.lock.toml"); + let lock = std::fs::read_to_string(&lock_path).unwrap(); + let lock = lock + .lines() + .map(|line| { + if line.trim() == "kind = \"local\"" { + "kind = \"git\"".to_string() + } else if line.trim() == "location = \"../marketplace\"" { + "location = \"https://example.invalid/marketplace.git\"".to_string() + } else if line.trim_start().starts_with("revision = \"local:") + || line + .trim_start() + .starts_with("resolved_revision = \"local:") + { + format!( + "{}\"0123456789abcdef0123456789abcdef01234567\"", + line.split_once("= ").unwrap().0.to_owned() + "= " + ) + } else { + line.to_string() + } + }) + .collect::>() + .join("\n"); + std::fs::write(lock_path, lock).unwrap(); + + let output = inspect(&project, "opencode", false); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { + diagnostic["code"] == "missing-plugin-snapshot" + && diagnostic["severity"] == "warning" + }), + "diagnostics: {}", + value["diagnostics"] + ); +} + #[test] fn inspect_json_is_stable_read_only_and_secret_safe() { let project = project(); @@ -321,6 +544,55 @@ fn inspect_reports_overwrite_without_claiming_unmanaged_servers_survive() { ); } +#[test] +fn inspect_malformed_existing_mcp_config_falls_back_to_projection() { + let project = project(); + std::fs::create_dir_all(project.path().join(".codex")).unwrap(); + std::fs::write( + project.path().join(".codex/config.toml"), + "mcp_servers = [\n", + ) + .unwrap(); + + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(value["mcp"]["servers"][0]["name"], "fixture"); + assert_eq!(value["mcp"]["servers"][0]["state"], "projected"); + assert!( + value["mcp"]["existing_server_names"] + .as_array() + .unwrap() + .is_empty() + ); +} + +#[test] +fn inspect_disabled_server_is_configured_and_not_included() { + let project = project(); + let config_path = project.path().join(".agents/agentsync.toml"); + let config = std::fs::read_to_string(&config_path).unwrap().replace( + "command = \"fixture-mcp\"", + "command = \"fixture-mcp\"\ndisabled = true", + ); + std::fs::write(config_path, config).unwrap(); + + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let server = &value["mcp"]["servers"][0]; + assert_eq!(server["disabled"], true); + assert_eq!(server["included"], false); + assert_eq!(server["state"], "configured"); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| diagnostic["code"] == "disabled-mcp-server") + ); +} + #[test] fn inspect_disabled_mcp_does_not_claim_servers_are_projected() { let project = project(); diff --git a/tests/contracts/test_skill_list_output.rs b/tests/contracts/test_skill_list_output.rs index 2f81f10d..76438a76 100644 --- a/tests/contracts/test_skill_list_output.rs +++ b/tests/contracts/test_skill_list_output.rs @@ -5,6 +5,45 @@ fn binary() -> &'static str { env!("CARGO_BIN_EXE_agentsync") } +#[cfg(unix)] +#[test] +fn skill_list_json_reports_missing_manifest_and_broken_symlink() { + let project = TempDir::new().unwrap(); + let skills = project.path().join(".agents/skills"); + std::fs::create_dir_all(skills.join("missing-manifest")).unwrap(); + std::os::unix::fs::symlink("does-not-exist", skills.join("broken-link")).unwrap(); + + let output = Command::new(binary()) + .args(["skill", "--project-root"]) + .arg(project.path()) + .args(["list", "--json"]) + .output() + .expect("run agentsync skill list"); + + assert!(output.status.success()); + let value: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(value["skills"][0]["id"], "broken-link"); + assert_eq!(value["skills"][0]["state"], "broken_symlink"); + assert_eq!(value["skills"][1]["id"], "missing-manifest"); + assert_eq!(value["skills"][1]["state"], "missing_manifest"); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { diagnostic == "broken symlink: .agents/skills/broken-link" }) + ); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { + diagnostic == "missing SKILL.md: .agents/skills/missing-manifest" + }) + ); +} + #[test] fn skill_list_json_empty_contract() { let project = TempDir::new().unwrap(); @@ -128,6 +167,33 @@ fn skill_list_json_is_deterministic_and_read_only() { assert_eq!(before, after); } +#[test] +fn skill_list_json_reports_malformed_manifest_state_and_diagnostic() { + let project = TempDir::new().unwrap(); + let skill = project.path().join(".agents/skills/malformed"); + std::fs::create_dir_all(&skill).unwrap(); + std::fs::write(skill.join("SKILL.md"), "---\nname: invalid name\n---\n").unwrap(); + + let output = Command::new(binary()) + .args(["skill", "--project-root"]) + .arg(project.path()) + .args(["list", "--json"]) + .output() + .expect("run agentsync skill list"); + + assert!(output.status.success()); + let value: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(value["skills"][0]["id"], "malformed"); + assert_eq!(value["skills"][0]["state"], "malformed"); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { diagnostic == "malformed SKILL.md: .agents/skills/malformed" }) + ); +} + #[test] fn skill_list_json_scan_failure_has_distinct_diagnostic_and_preserves_error() { let project = TempDir::new().unwrap(); @@ -176,3 +242,34 @@ fn skill_list_json_corrupt_registry_reports_redacted_diagnostic_and_fails() { ); assert!(!String::from_utf8_lossy(&output.stdout).contains("not json")); } + +#[test] +fn skill_list_json_reports_duplicate_manifest_ids() { + let project = TempDir::new().unwrap(); + write_skill(&project, "first-copy", "---\nname: shared-skill\n---\n"); + write_skill(&project, "second-copy", "---\nname: shared-skill\n---\n"); + + let output = Command::new(binary()) + .args(["skill", "--project-root"]) + .arg(project.path()) + .args(["list", "--json"]) + .output() + .expect("run agentsync skill list"); + + assert!(output.status.success()); + let value: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(value["skills"][0]["id"], "first-copy"); + assert_eq!(value["skills"][0]["state"], "duplicate_id"); + assert_eq!(value["skills"][1]["id"], "second-copy"); + assert_eq!(value["skills"][1]["state"], "duplicate_id"); + assert!( + value["diagnostics"] + .as_array() + .unwrap() + .iter() + .any(|diagnostic| { + diagnostic + == "duplicate skill ID 'shared-skill' in directories: first-copy, second-copy" + }) + ); +} diff --git a/tests/unit/manifest.rs b/tests/unit/manifest.rs index bdd41eb2..2be81f61 100644 --- a/tests/unit/manifest.rs +++ b/tests/unit/manifest.rs @@ -12,6 +12,20 @@ fn parse_valid_manifest() { assert_eq!(manifest.name, "sample-skill"); } +#[test] +fn parse_manifest_preserves_boolean_like_description_as_string() { + let td = TempDir::new().unwrap(); + let path = td.path().join("SKILL.md"); + fs::write( + &path, + "---\nname: sample-skill\ndescription: yes\n---\n# Body", + ) + .unwrap(); + + let manifest = agentsync::skills::manifest::parse_skill_manifest(&path).unwrap(); + assert_eq!(manifest.description.as_deref(), Some("yes")); +} + #[test] fn reject_invalid_name() { let td = TempDir::new().unwrap(); diff --git a/tests/unit/mod.rs b/tests/unit/mod.rs index 617bf43e..8604a8d3 100644 --- a/tests/unit/mod.rs +++ b/tests/unit/mod.rs @@ -1,6 +1,9 @@ #[cfg(test)] mod install; +#[cfg(test)] +mod manifest; + #[cfg(test)] mod provider; From b741021127242ba7f5636f4dcfe2b525e031a422 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:16:06 +0000 Subject: [PATCH 5/5] fix: correct inspect plugin error classification, MCP collision states, and broken symlink paths --- src/inspect.rs | 28 ++-- src/plugins.rs | 43 ++++-- tests/contracts/test_inspect_output.rs | 175 +++++++++++++++++++++---- 3 files changed, 200 insertions(+), 46 deletions(-) diff --git a/src/inspect.rs b/src/inspect.rs index d5680545..353db75f 100644 --- a/src/inspect.rs +++ b/src/inspect.rs @@ -6,7 +6,7 @@ use crate::Linker; use crate::config::{Config, McpServerConfig, SyncType}; use crate::mcp::McpAgent; -use crate::plugins::PluginLock; +use crate::plugins::PluginInspectError; use crate::status::{ AgentStatusEntry, DestinationKind, StatusIssueKind, collect_agent_status_entries, paths_match, sync_type_label, @@ -360,7 +360,10 @@ fn resolve_target( code: DiagnosticCode::BrokenSymlink, severity: DiagnosticSeverity::Error, target: Some(target_name.clone()), - path: Some(portable_destination.clone()), + path: Some(portable_path( + project_root, + Path::new(&entry.entry.destination), + )), message: "Destination is a broken or unreadable symlink.".into(), }); state = ItemState::Conflicted; @@ -687,7 +690,9 @@ fn append_plugin_mcp_servers( dest, ); server_report.included = approved && !collision && !item.server.disabled; - server_report.state = if item.server.disabled || !approved { + server_report.state = if collision { + ItemState::Conflicted + } else if item.server.disabled || !approved { ItemState::Configured } else if existing_equal { ItemState::Observed @@ -701,14 +706,15 @@ fn append_plugin_mcp_servers( } } Err(error) => { - let lock_path = manager.lock_path()?; - let lock = PluginLock::load(&lock_path).ok(); - let missing_snapshot = lock.as_ref().is_some_and(|lock| { - lock.plugins - .values() - .any(|plugin| plugin.source.kind == crate::plugins::LockedSourceKind::Git) - }); - diagnostics.push(Diagnostic { code: if missing_snapshot { DiagnosticCode::MissingPluginSnapshot } else { DiagnosticCode::UnsupportedPluginComponent }, severity: DiagnosticSeverity::Warning, target: Some("plugins".into()), path: Some(portable_path(root, &lock_path)), message: if missing_snapshot { "A required local plugin snapshot is unavailable; plugin MCP contributions are unknown.".into() } else { format!("Plugin contributions could not be resolved offline: {}", error.to_string().split(':').next().unwrap_or("plugin metadata error")) } }); + let missing_snapshot = matches!( + error.downcast_ref::(), + Some(PluginInspectError::MissingSnapshot) + ); + let path = manager + .lock_path() + .ok() + .map(|path| portable_path(root, &path)); + diagnostics.push(Diagnostic { code: if missing_snapshot { DiagnosticCode::MissingPluginSnapshot } else { DiagnosticCode::UnsupportedPluginComponent }, severity: DiagnosticSeverity::Warning, target: Some("plugins".into()), path, message: if missing_snapshot { "A required local plugin snapshot is unavailable; plugin MCP contributions are unknown.".into() } else { format!("Plugin contributions could not be resolved offline: {}", error.to_string().split(':').next().unwrap_or("plugin metadata error")) } }); } } Ok(()) diff --git a/src/plugins.rs b/src/plugins.rs index 597815cc..b48d266a 100644 --- a/src/plugins.rs +++ b/src/plugins.rs @@ -302,6 +302,12 @@ enum GitRestoreAction { Skipped, } +#[derive(Debug, thiserror::Error)] +pub(crate) enum PluginInspectError { + #[error("plugin snapshot unavailable")] + MissingSnapshot, +} + /// Repository-owned plugin operations. #[derive(Clone)] pub struct PluginManager { @@ -364,7 +370,10 @@ impl PluginManager { ); return Ok(report); } - let lock = PluginLock::load(&self.lock_path()?)?; + let lock_path = self + .lock_path() + .context("failed to resolve plugin lockfile path")?; + let lock = PluginLock::load(&lock_path)?; let selections = selected_lock_entries(&self.config, &lock)?; validate_allowed_mcp_against_lock(&self.config.allowed_mcp, &selections)?; let allowed = self.config.allowed_mcp.iter().collect::>(); @@ -381,15 +390,33 @@ impl PluginManager { .unwrap_or_else(|| Path::new(".")) .join(location) } - LockedSourceKind::Git => { - let root = self.git_source_cache_path(&locked.source)?; - ensure_git_snapshot_dir(&root)?; - root - } + LockedSourceKind::Git => self.git_source_cache_path(&locked.source)?, }; + let metadata = match locked.source.kind { + LockedSourceKind::Local => fs::metadata(&root), + LockedSourceKind::Git => fs::symlink_metadata(&root), + } + .map_err(|error| { + let missing = error.kind() == std::io::ErrorKind::NotFound; + let error = anyhow::Error::new(error); + if missing { + error.context(PluginInspectError::MissingSnapshot) + } else { + error + } + }) + .with_context(|| { + format!( + "failed to read plugin snapshot metadata: {}", + root.display() + ) + })?; + if locked.source.kind == LockedSourceKind::Git { + ensure_git_snapshot_dir(&root)?; + } ensure!( - root.is_dir(), - "plugin snapshot unavailable: {}", + metadata.is_dir(), + "plugin snapshot is not a directory: {}", root.display() ); let discovered = discover_plugin(&root, &locked.marketplace, &locked.plugin)?; diff --git a/tests/contracts/test_inspect_output.rs b/tests/contracts/test_inspect_output.rs index f8ba2392..0e8f7c65 100644 --- a/tests/contracts/test_inspect_output.rs +++ b/tests/contracts/test_inspect_output.rs @@ -1,4 +1,5 @@ use serde_json::Value; +use sha2::{Digest, Sha256}; use std::process::Command; use tempfile::TempDir; @@ -111,6 +112,30 @@ plugin = "engineering" project } +fn inspect_git_plugin_project() -> (TempDir, std::path::PathBuf) { + let project = inspect_plugin_project(); + let lock_path = project.path().join(".agents/plugins.lock.toml"); + let mut lock = agentsync::plugins::PluginLock::load(&lock_path).unwrap(); + let location = "https://example.invalid/marketplace.git"; + let revision = "0123456789abcdef0123456789abcdef01234567"; + for plugin in lock.plugins.values_mut() { + plugin.source.kind = agentsync::plugins::LockedSourceKind::Git; + plugin.source.location = location.into(); + plugin.source.revision = revision.into(); + plugin.provenance.resolved_revision = revision.into(); + } + lock.save_atomic(&lock_path).unwrap(); + let digest = Sha256::digest(location.as_bytes()) + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + let snapshot = project + .path() + .join(".agents/.agentsync-plugin-sources") + .join(format!("{}-{revision}", &digest[..16])); + (project, snapshot) +} + fn copy_tree(source: &std::path::Path, target: &std::path::Path) { std::fs::create_dir_all(target).unwrap(); for entry in std::fs::read_dir(source).unwrap() { @@ -210,7 +235,7 @@ fn inspect_reports_plugin_mcp_collision_with_explicit_server() { command = "/bin/true" "#, ); - std::fs::write(config_path, config).unwrap(); + std::fs::write(&config_path, &config).unwrap(); let output = inspect(&project, "opencode", false); assert!( @@ -229,6 +254,7 @@ command = "/bin/true" }) .unwrap(); assert_eq!(plugin_server["included"], false); + assert_eq!(plugin_server["state"], "conflicted"); assert!( value["diagnostics"] .as_array() @@ -238,6 +264,23 @@ command = "/bin/true" diagnostic["code"] == "mcp-name-collision" && diagnostic["severity"] == "error" }) ); + + let config = config.replace( + "allowed_mcp = [\"plugin/internal/engineering/safe-fixture\"]", + "allowed_mcp = []", + ); + std::fs::write(config_path, config).unwrap(); + let output = inspect(&project, "opencode", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let plugin_server = value["mcp"]["servers"] + .as_array() + .unwrap() + .iter() + .find(|server| server["origin"] == "plugin") + .unwrap(); + assert_eq!(plugin_server["included"], false); + assert_eq!(plugin_server["state"], "conflicted"); } #[test] @@ -266,32 +309,8 @@ fn inspect_reports_plugin_error_when_lockfile_is_missing() { #[test] fn inspect_reports_missing_plugin_snapshot_from_git_lock_entry() { - let project = inspect_plugin_project(); - let lock_path = project.path().join(".agents/plugins.lock.toml"); - let lock = std::fs::read_to_string(&lock_path).unwrap(); - let lock = lock - .lines() - .map(|line| { - if line.trim() == "kind = \"local\"" { - "kind = \"git\"".to_string() - } else if line.trim() == "location = \"../marketplace\"" { - "location = \"https://example.invalid/marketplace.git\"".to_string() - } else if line.trim_start().starts_with("revision = \"local:") - || line - .trim_start() - .starts_with("resolved_revision = \"local:") - { - format!( - "{}\"0123456789abcdef0123456789abcdef01234567\"", - line.split_once("= ").unwrap().0.to_owned() + "= " - ) - } else { - line.to_string() - } - }) - .collect::>() - .join("\n"); - std::fs::write(lock_path, lock).unwrap(); + let (project, snapshot) = inspect_git_plugin_project(); + assert!(!snapshot.exists()); let output = inspect(&project, "opencode", false); assert!( @@ -314,6 +333,94 @@ fn inspect_reports_missing_plugin_snapshot_from_git_lock_entry() { ); } +#[test] +fn inspect_classifies_plugin_failures_by_the_actual_error() { + let (project, snapshot) = inspect_git_plugin_project(); + let fixture = project.path().join("marketplace"); + copy_tree(&fixture, &snapshot); + let before = std::fs::read(project.path().join(".agents/plugins.lock.toml")).unwrap(); + let manifest = snapshot.join(".agents/plugins/marketplace.json"); + assert!(manifest.is_file()); + std::fs::write(&manifest, "invalid json").unwrap(); + + let output = inspect(&project, "opencode", false); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let diagnostics = value["diagnostics"].as_array().unwrap(); + assert!( + diagnostics + .iter() + .any(|d| d["code"] == "unsupported-plugin-component") + ); + assert!( + !diagnostics + .iter() + .any(|d| d["code"] == "missing-plugin-snapshot") + ); + assert_eq!(std::fs::read(manifest).unwrap(), b"invalid json"); + assert_eq!( + std::fs::read(project.path().join(".agents/plugins.lock.toml")).unwrap(), + before + ); + assert!(!project.path().join(".agents/plugins").exists()); +} + +#[test] +fn inspect_reports_stale_approvals_before_missing_git_snapshot() { + let (project, snapshot) = inspect_git_plugin_project(); + let config_path = project.path().join(".agents/agentsync.toml"); + let config = std::fs::read_to_string(&config_path).unwrap().replace( + "plugin/internal/engineering/safe-fixture", + "plugin/internal/engineering/unknown", + ); + std::fs::write(config_path, config).unwrap(); + let output = inspect(&project, "opencode", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let diagnostics = value["diagnostics"].as_array().unwrap(); + assert!( + diagnostics + .iter() + .any(|d| d["code"] == "unsupported-plugin-component") + ); + assert!( + !diagnostics + .iter() + .any(|d| d["code"] == "missing-plugin-snapshot") + ); + assert!(!snapshot.exists()); +} + +#[test] +fn inspect_reports_invalid_plugin_lock_path_without_aborting() { + let project = inspect_plugin_project(); + let config_path = project.path().join(".agents/agentsync.toml"); + let config = std::fs::read_to_string(&config_path).unwrap().replace( + "lockfile = \"plugins.lock.toml\"", + "lockfile = \"../plugins.lock.toml\"", + ); + std::fs::write(config_path, config).unwrap(); + let output = inspect(&project, "opencode", false); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let diagnostic = value["diagnostics"] + .as_array() + .unwrap() + .iter() + .find(|d| d["code"] == "unsupported-plugin-component") + .unwrap(); + assert!(diagnostic["path"].is_null()); + assert!(!project.path().join("plugins.lock.toml").exists()); +} + #[test] fn inspect_json_is_stable_read_only_and_secret_safe() { let project = project(); @@ -447,6 +554,20 @@ destination = "src/two" assert_eq!(children[0]["destination"], "src/one/AGENTS.md"); assert_eq!(children[1]["source"], ".agents/source/second.md"); assert_eq!(children[1]["destination"], "src/two/AGENTS.md"); + + std::fs::remove_file(agents.join("first.md")).unwrap(); + std::fs::remove_file(agents.join("second.md")).unwrap(); + let output = inspect(&project, "codex", false); + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).unwrap(); + let broken_paths = value["diagnostics"] + .as_array() + .unwrap() + .iter() + .filter(|d| d["code"] == "broken-symlink") + .map(|d| d["path"].as_str().unwrap()) + .collect::>(); + assert_eq!(broken_paths, ["src/one/AGENTS.md", "src/two/AGENTS.md"]); } #[test]