diff --git a/.agents/skills/.gitignore b/.agents/skills/.gitignore new file mode 100644 index 00000000..15ae2143 --- /dev/null +++ b/.agents/skills/.gitignore @@ -0,0 +1,4 @@ +# Skills symposium syncs in are generated; ignore them, keep hand-authored ones. +/* +!/.gitignore +!/authoring-rfds/ diff --git a/md/design/important-flows.md b/md/design/important-flows.md index 9c90b229..f08b5d42 100644 --- a/md/design/important-flows.md +++ b/md/design/important-flows.md @@ -6,15 +6,15 @@ This section describes the logic of each `cargo agents` command. A plugin loads a crate as a plugin by naming that crate in a `[[plugins]]` chained reference (`source.cargo = "..."`). When the owning plugin is active and the edge's predicates hold, sync resolves the crate. A single path handles every crate — a crate is always a first-class plugin, whether it describes itself with a `SYMPOSIUM.toml`, with `[package.metadata.symposium]`, with both, or with neither: -1. `skills_applicable_to` runs `expand_chained_plugins` over the active plugin's `plugin.chained` edges; each edge whose predicates hold (evaluated against the *owning* plugin's provenance) names a crate directly. -2. For that crate, `expand_chained_plugins` calls `CargoPm::load_plugin(name, workspace)`: +1. `resolve_applicable`'s `Resolver` runs `expand_chained` over the active plugin's `plugin.chained` edges; each edge whose predicates hold (evaluated against the *owning* plugin's provenance) names a crate directly. +2. For that crate, `Resolver::expand_chained` calls `CargoPm::load_plugin(name, workspace)`: - `CargoPm::fetch` resolves the source via `RustCrateFetch` (path overrides for local path deps, then the cargo registry cache, then crates.io). The fetched id carries the exact resolved version. - `plugins::load_crate_manifest` builds the plugin definition by layering three sources (merge order: crate defaults → `[package.metadata.symposium]` from `Cargo.toml` → `SYMPOSIUM.toml` file). Both manifest sources use the ordinary plugin-manifest schema and are parsed **leniently** (a malformed layer is logged and dropped). Validation runs under `ManifestOrigin::Crate` (name defaults to the crate, `depends-on` is waived, `[defaults]` accepted, default `skills/` group appended unless `[defaults] skills = false`). The result is a `ParsedPlugin` whose `canonical` id is the resolved crate. A crate with no manifest sources still yields one whose only content is that default `skills/` group. -3. Back in `expand_chained_plugins`, the crate plugin's own plugin-level predicates are honored (`applies`, which stamps its provenance — never a workspace member), its skill groups run through the ordinary `load_skills_for_group` pipeline — honoring named groups, group predicates, and `source.path`/`source.git`, with each discovered skill's origin hashed from its on-disk `SKILL.md` path — and **its own `[[plugins]]` edges are expanded in turn**. This is how a `[package.metadata.symposium]` redirect (now a `[[plugins]] source.cargo` chained reference to the target crate) is followed. A per-top-level-plugin `visited` set keyed on the normalized crate name collapses diamonds (a crate reached two ways loads once) and breaks cycles; `MAX_CHAIN_DEPTH` (10) is a backstop. The crate plugin's hooks/MCP/subcommands are parsed but not yet dispatched (a `warn_undispatched_crate_features` notice fires when present). +3. Back in `Resolver::expand_chained`, the crate plugin's own plugin-level predicates are honored (`applies`, which stamps its provenance — never a workspace member), its skill groups run through the ordinary `load_skills_for_group` pipeline — honoring named groups, group predicates, and `source.path`/`source.git`, with each discovered skill's origin hashed from its on-disk `SKILL.md` path — and **its own `[[plugins]]` edges are expanded in turn**. This is how a `[package.metadata.symposium]` redirect (now a `[[plugins]] source.cargo` chained reference to the target crate) is followed. A per-top-level-plugin `visited` set keyed on the normalized crate name collapses diamonds (a crate reached two ways loads once) and breaks cycles; `MAX_CHAIN_DEPTH` (10) is a backstop. The crate plugin's hooks/MCP/subcommands are parsed but not yet dispatched (a `warn_undispatched_crate_features` notice fires when present). A skill's install identity is the hash of its on-disk `SKILL.md` path, so a crate reached two ways dedupes to one install. The edge's version requirement is recorded but not yet enforced — the crate resolves against the workspace (pin / path override). -The key code paths are in `pm/cargo.rs` (`CargoPm::load_plugin`), `plugins.rs` (`load_crate_manifest`, `RawPluginManifest::merge`, `ManifestOrigin::Crate`, `ParsedPlugin::canonical`), `skills.rs` (`expand_chained_plugins`, `hash_origin_key`), `crate_metadata.rs` (`symposium_metadata`), and `crate_sources/mod.rs` (`RustCrateFetch`, `WorkspaceCrate`). +The key code paths are in `pm/cargo.rs` (`CargoPm::load_plugin`), `plugins.rs` (`load_crate_manifest`, `RawPluginManifest::merge`, `ManifestOrigin::Crate`, `ParsedPlugin::canonical`), `skills.rs` (`Resolver::expand_chained`, `hash_origin_key`), `crate_metadata.rs` (`symposium_metadata`), and `crate_sources/mod.rs` (`RustCrateFetch`, `WorkspaceCrate`). ## Help rendering diff --git a/md/design/module-structure.md b/md/design/module-structure.md index 6af82625..efe5cf53 100644 --- a/md/design/module-structure.md +++ b/md/design/module-structure.md @@ -8,6 +8,8 @@ Everything hangs off the `Symposium` struct, which wraps the parsed `Config` wit Defines the user-wide `Config` (stored at `~/.symposium/config.toml`) with `[[agent]]` entries, logging, plugin sources, defaults, and `auto-update` (off/warn/on, default on). User config is deserialized through `RawConfig` and validated into the runtime `Config`; runtime code does not deserialize `Config` directly. Provides `plugin_sources()` to resolve the effective list of plugin source directories. The `workspace_deps(cwd)` factory is the standard way to create a `WorkspaceDeps` — it wires in `cargo_override` and `cache_dir` so callers get both the `SYMPOSIUM_CARGO` override and cross-invocation disk caching. +`record_telemetry_batch(session_id, kinds)` is where `config.telemetry.enabled` is checked: it records a hook pass's events via [telemetry](./telemetry.md) when enabled and does nothing otherwise (`record_telemetry` is a one-event convenience wrapper over it). Callers therefore emit unconditionally. `roll_off_telemetry()` enforces retention, run once per session on `SessionStart`. It is deliberately *not* gated on `enabled`: retention is disk hygiene, so data recorded before a user opted out still ages out. Both are best-effort (a write failure never breaks a hook) and return nothing. + ### `agents.rs` — agent abstraction Centralizes agent-specific knowledge: hook registration file paths, skill installation directories, and hook registration logic for each supported agent (Claude Code, GitHub Copilot, Gemini CLI, Codex CLI, Kiro, OpenCode, Goose). Handles the differences between agents — e.g., Claude Code uses `.claude/skills/` and Kiro uses `.kiro/skills/`, while Copilot, Gemini, Codex, OpenCode, and Goose use the vendor-neutral `.agents/skills/`. OpenCode and Goose are skills-only agents (no hook registration). @@ -20,9 +22,9 @@ Implements `cargo agents init`. Prompts for agents (or accepts `--add-agent`/`-- Implements `cargo agents sync`. Scans workspace dependencies, finds applicable skills from plugin sources, and synchronizes them into each configured agent's skill directory. The core primitive is `sync_skill_dir(source_dir, dest_dir, project_root)`. It copies the entire source directory (not just `SKILL.md`) and is change-aware: it compares source and destination content, only performing the delete-and-recopy when files actually differ, so the disk shows no modifications when nothing changed. A configurable debounce (`sync-debounce-secs`, default 5s, keyed on the `.symposium` marker's mtime) skips even the comparison for recently-synced skills. On each sync, scans every agent's skills parent directory and reaps any marker-bearing subdirectory it didn't install this time, leaving user-managed skills (which lack the marker) untouched. Writes a `.gitignore` with `*` only into individual skill directories (not parent directories like `.claude/` or `.claude/skills/`). Also provides `register_hooks()` for use by `init`, which registers only symposium's own global hook handler — individual plugin hooks are never written into agent configs. -Two entry points: `sync(sym, cwd)` for standalone CLI use (creates its own `WorkspaceDeps`) and `sync_with_deps(sym, deps)` for the hook pipeline (shares the cached workspace resolution with other hook stages). +`sync(sym, deps, update)` runs the pass and returns a `SyncSummary`: the matched plugins and skills (each with the crates that activated it), plus `installed` (distinct skills this pass created or updated, counted once per skill regardless of how many agents it was written to, so a steady-state sync reports 0), `reaped` (stale directories removed, counted per directory), and the workspace `crate_count`. The hook pipeline emits telemetry from this; the standalone `cargo agents sync` path discards it. The summary is built from data `sync` already computes, so nothing extra is resolved when telemetry is off. -`sync` takes an `UpdateLevel` that it threads into skill resolution (`skills_applicable_to`), controlling how aggressively `source.git` skill groups are re-fetched. Callers choose: the auto-sync path passes `Check` on `SessionStart` (refresh) and `None` otherwise (debounced); the binary's global `--update` flag feeds manual `cargo agents sync`. +`sync` takes an `UpdateLevel` that it threads into skill resolution (`resolve_applicable`), controlling how aggressively `source.git` skill groups are re-fetched. Callers choose: the auto-sync path passes `Check` on `SessionStart` (refresh) and `None` otherwise (debounced); the binary's global `--update` flag feeds manual `cargo agents sync`. ### `plugins.rs` — plugin registry @@ -74,7 +76,11 @@ Each gated struct (plugin, skill group, skill, hook, MCP server, subcommand) sto Given a `PluginRegistry` and workspace dependencies, this module resolves skill group sources, discovers `SKILL.md` files, and evaluates dependency predicates at each level (plugin, group, skill) to determine which skills apply. Every `source` funnels through one seam: `resolve_group_dirs` turns a group into a list of `ResolvedSkillDir` (a base directory + report labels), then `collect_skills_from_dirs` scans each base for `SKILL.md` files. `PluginSource` has exactly two variants — `Path` (already on disk, relative to the plugin's source dir) and `Git` (fetched via the git cache); a source is required, so there is no "no source" state. -`[[plugins]]` chained references are how a crate becomes a plugin: after an active plugin's own skill groups, `skills_applicable_to` runs `expand_chained_plugins` over its `plugin.chained` edges. For each edge whose predicates hold it asks `CargoPm::load_plugin` for the crate, which always returns a first-class `ParsedPlugin` (built from `[package.metadata.symposium]` + `SYMPOSIUM.toml` + defaults). That plugin's own plugin-level predicates are honored, its skill groups run through the ordinary `load_skills_for_group` pipeline, and **its own `[[plugins]]` edges are expanded in turn** — a crate that names another crate (the reschema'd `[package.metadata.symposium]` redirect) is followed recursively. A per-top-level-plugin `visited` set (keyed on the normalized crate name via `canonical`) collapses diamonds and breaks cycles; `MAX_CHAIN_DEPTH` (10) is a backstop. The crate plugin's hooks/MCP/subcommands are parsed and carried but **not yet dispatched** (`warn_undispatched_crate_features` logs when present). Each discovered skill's install identity is the hash of its on-disk `SKILL.md` path (below), so a crate reached two ways dedupes to one install. +`resolve_applicable` is the entry point. It returns `ApplicableSkills { skills, plugins }`: the applicable skills (each `SkillWithGroupContext` carries its owning `plugin` and the `crates` that activated it) plus a `PluginActivation` per matched plugin (tracked separately, since a plugin can match yet vend no skill). The walk lives on a `Resolver` that bundles the shared inputs, the predicate cursor, and the two growing lists, so the recursive chained-plugin descent threads only an `inherited` crate set instead of a long argument list. `skills_applicable_to` is a test-only wrapper returning just the skills. + +A skill's (and plugin's) `crates` is a coarse, names-only attribution that feeds [telemetry](./telemetry.md): the dependency names the gate references (via `PredicateSet::collect_dep_names`) intersected with the workspace deps, then unioned down the plugin, group, and skill levels (`Resolver::matched_names`). It stands in for a retired predicate witness, and over-reports only a satisfied `any(...)`'s non-firing branch, which is acceptable for a names-only field. + +`[[plugins]]` chained references are how a crate becomes a plugin: after an active plugin's own skill groups, `Resolver::expand_chained` runs over its `plugin.chained` edges. For each edge whose predicates hold it asks `CargoPm::load_plugin` for the crate, which always returns a first-class `ParsedPlugin` (built from `[package.metadata.symposium]` + `SYMPOSIUM.toml` + defaults). That plugin's own plugin-level predicates are honored, its skill groups run through the ordinary `load_skills_for_group` pipeline, and **its own `[[plugins]]` edges are expanded in turn** — a crate that names another crate (the reschema'd `[package.metadata.symposium]` redirect) is followed recursively. A per-top-level-plugin `visited` set (keyed on the normalized crate name via `canonical`) collapses diamonds and breaks cycles; `MAX_CHAIN_DEPTH` (10) is a backstop. The crate plugin's hooks/MCP/subcommands are parsed and carried but **not yet dispatched** (`warn_undispatched_crate_features` logs when present). Each discovered skill's install identity is the hash of its on-disk `SKILL.md` path (below), so a crate reached two ways dedupes to one install. Each applicable skill carries an **origin hash** (a `String`) describing *where its bytes live*, used at sync time for dedup and install-path disambiguation. `skill_origin_hash` computes it as an 8-hex-char prefix of SHA-256 over the `SKILL.md`'s **canonical** on-disk path — nothing else. Identity is the file's location, not which plugin manifest pointed at it: two references that resolve to the same file (the same crate reached through two chained plugins, or a `source.path` group and the standalone walk landing on the same bundle) produce the same hash and dedupe; skills at different paths stay distinct. Canonicalizing inside the hash is what makes that hold across discovery paths — group discovery walks a canonicalized scan dir while `plugins.rs`'s standalone walk uses the configured source path verbatim, so on a platform whose temp prefix is a symlink (macOS `/var` → `/private/var`) the same file would otherwise hash two ways and install twice. @@ -102,13 +108,15 @@ Handles the hook pipeline: parse agent wire-format input → auto-sync → built Builtin dispatch currently only acts on `SessionStart`, where `handle_session_start` composes two independently-computed `additionalContext` fragments: a `discovery_hint` (suggests `cargo agents --help` when the workspace exposes applicable plugin subcommands, reusing `subcommand_dispatch::applicable_subcommands`) and an `update_nudge` (the throttled self-update warning); the discovery hint is not gated behind the update-check throttle. The plugin dispatch path matches plugin `Hook`s against the event, selects the best format for each plugin (native match > symposium > single-other-agent fallback), builds a `ResolvedHook` per match (looking up the named installations on the plugin), then for each `ResolvedHook`: acquires its `requirements` (best-effort), runs `install_commands` after the source step, picks a `Runnable` from (hook-or-install) `executable`/`script`, and spawns it (binary directly for `Exec`, via `sh ` for `Script`). Input is delivered in the selected format; output is converted back to the agent's wire format before returning. +Telemetry emission is centralized in `execute_hook`. `run_auto_sync` returns `Option` (`None` when auto-sync is off, debounced, or the sync errored) and `dispatch_plugin_hooks` returns a `DispatchOutcome`: the `Vec` it timed (each hook's run duration, exit code included) and a `Result` holding either the merged output or a blocking hook's stderr. After dispatch, the pure `telemetry_events` maps the wire input, the optional `SyncSummary`, and the hook invocations into the `EventKind`s for this pass, which `execute_hook` records through `sym.record_telemetry_batch` (the opt-in gate) in one batched write, so a pass opens the day's file once rather than reopening per event. Cadence: the wire funnel event (`session_start` / `user_prompt` / `tool_use` / `stop`) is emitted once per matching hook event (`PreToolUse` is silent, so a tool is counted once on `PostToolUse`); `sync_run` whenever the sync body ran; `plugin_activation` / `skill_activation` only on the `SessionStart` sync, so the activation snapshot is recorded once per session. `session_id` comes from the wire input and rides on the event envelope. When a plugin hook blocks (exit 2 / killed), dispatch still returns the invocations it collected, so `execute_hook` emits the pass's telemetry (the wire event, `sync_run`, and every hook that ran, including the blocking one) before propagating the block. On `SessionStart`, `execute_hook` also calls `sym.roll_off_telemetry()` once per session: the only place stale daily files past `RETENTION_DAYS` are deleted. + ### `state.rs` — persistent state Manages `state.toml` in the config directory. Deserializes through `RawState` and validates into the runtime `State`. Tracks the semver of the binary that last touched the directory (for future migration hooks) and the timestamp of the last update check (to throttle crates.io queries to once per 24 hours). `ensure_current()` is called on startup to silently stamp the current version. `should_check_for_update()` / `record_update_check()` gate the auto-update flow. ### `telemetry.rs` — opt-in usage telemetry -Implements the local, opt-in [telemetry](./telemetry.md) event log under `/telemetry/`, one JSONL file per UTC day. Off by default; gated by `[telemetry] enabled`. A `TelemetryEvent` is an `at` timestamp plus a kind-tagged `EventKind` (`session_start` / `user_prompt` / `tool_use`), serialized one per line. `record` / `record_kind` append an event; `roll_off` deletes files older than `RETENTION_DAYS` (30); `read_events` / `recent_events` read them back; `usage` + `status_text` back `telemetry status`; `recent_events` backs `telemetry show`. Events are anonymous by construction — no prompt text, command lines, or file paths. Every write path is best-effort — failures are logged and swallowed so a hook is never broken. The recording entry points are not yet called from the hook pipeline, so no events are produced today even when telemetry is enabled. +Implements the local, opt-in [telemetry](./telemetry.md) event log under `/telemetry/`, one JSONL file per UTC day. Off by default; gated by `[telemetry] enabled`. A `TelemetryEvent` is an `at` timestamp, an optional `session_id` (common to every kind, so it rides on the envelope rather than being repeated in each), plus a kind-tagged `EventKind` (`session_start` / `user_prompt` / `tool_use` / `plugin_activation` / `skill_activation` / `hook_invocation` / `sync_run` / `stop`), serialized one per line. `record` appends one event; `record_all` appends a whole hook pass with a single file open (the production path, used by the hook pipeline); `roll_off` deletes files older than `RETENTION_DAYS` (30), run once per session from the `SessionStart` hook via `Symposium::roll_off_telemetry`; `read_events` / `recent_events` read them back; `usage` + `status_text` back `telemetry status`; `recent_events` backs `telemetry show`. Events are anonymous by construction — no prompt text, command lines, or file paths. Every write path is best-effort — failures are logged and swallowed so a hook is never broken. The hook pipeline produces these events: `execute_hook` maps each pass to `EventKind`s and records them through `Symposium::record_telemetry_batch` (the opt-in gate), so events are written only when `[telemetry] enabled` is set. ### `report.rs` — structured report layer diff --git a/md/design/telemetry.md b/md/design/telemetry.md index 3e720a10..b8ff2a69 100644 --- a/md/design/telemetry.md +++ b/md/design/telemetry.md @@ -13,8 +13,10 @@ helping, while keeping the user in control of their data. Uploading is a separate, deliberate step. - **Anonymous by construction.** Events record counts and coarse metadata only — no prompt text, command lines, or file paths. -- **Extensible.** Events are JSON lines, so new event kinds and fields can be - added without breaking older readers. +- **Extensible.** Events are JSON lines and readers ignore unknown fields, so a + new field is backward compatible as long as it carries `#[serde(default)]`. A + new event *kind* read by an older binary deserializes to a catch-all `unknown` + variant, so the line is kept rather than dropped (its payload is not retained). - **Never breaks a hook.** Every recording path is best-effort — failures are logged and swallowed. @@ -28,16 +30,50 @@ When enabled, events are appended as **JSON lines** to per-day files under ``` Each line is one [`TelemetryEvent`](./module-structure.md): an `at` timestamp -plus a kind-tagged payload (`EventKind`), e.g. +and an optional `session_id` on the envelope, then a kind-tagged payload +(`EventKind`), e.g. ```json -{"at":"2026-06-23T17:58:13Z","kind":"session_start","session_id":"P1","agent":"claude","plugins":["tokio-plugin"]} -{"at":"2026-06-23T17:58:14Z","kind":"user_prompt","session_id":"P1"} -{"at":"2026-06-23T17:58:15Z","kind":"tool_use","session_id":"P1","tool":"Bash"} +{"at":"2026-07-09T17:58:13Z","session_id":"P1","kind":"session_start","agent":"claude","crate_count":42} +{"at":"2026-07-09T17:58:13Z","session_id":"P1","kind":"plugin_activation","plugin":"async-plugin","crates":["tokio"]} +{"at":"2026-07-09T17:58:13Z","session_id":"P1","kind":"skill_activation","skill":"async-patterns","plugin":"async-plugin","crates":["tokio"]} +{"at":"2026-07-09T17:58:13Z","session_id":"P1","kind":"skill_activation","skill":"rust-general","plugin":"core-plugin"} +{"at":"2026-07-09T17:58:13Z","session_id":"P1","kind":"sync_run","installed":2,"reaped":0,"plugins_matched":2} +{"at":"2026-07-09T17:58:14Z","session_id":"P1","kind":"user_prompt"} +{"at":"2026-07-09T17:58:15Z","session_id":"P1","kind":"tool_use","tool":"Bash"} +{"at":"2026-07-09T17:58:16Z","session_id":"P1","kind":"hook_invocation","hook":"format-check","plugin":"async-plugin","duration_ms":37,"exit_code":0} +{"at":"2026-07-09T17:58:40Z","session_id":"P1","kind":"stop"} ``` +The event kinds fall into three groups by what produces them: + +- **Session lifecycle**, keyed off the agent's hook events: `session_start` + (agent name, workspace crate count), `user_prompt`, `tool_use` (tool name + only), and `stop` (end of a turn). +- **Sync activity**, captured once per session when symposium syncs skills: + `plugin_activation` and `skill_activation` (which plugin or skill applied, and + the workspace crates that triggered it), plus `sync_run`. In `sync_run`, + `installed` counts the skills this pass created or updated, so a steady-state + sync reports 0; count `skill_activation` events for how many skills apply. + `reaped` counts stale directories removed, and `plugins_matched` the plugins + that applied. +- **Hook activity**: `hook_invocation` (which hook of which plugin ran, its + duration, and its exit code — `null` if the hook was killed by a signal). + +Not every agent produces every event. `stop` needs an agent that registers a Stop +hook, which today is Claude alone. Copilot supplies no `session_id`, so its +events cannot be grouped into a session. Goose and OpenCode register no hooks at +all (they are skills-only), so an opted-in user of those agents records nothing. +Any cross-agent comparison has a different denominator per agent. + +A `crates` list is the set of workspace crates that satisfied the activation's +predicates. It is omitted when the gate was a wildcard or a non-crate predicate +(`env`, `shell`, `path`), so a skill that applies for a non-crate reason records +no `crates` key. + Files older than `RETENTION_DAYS` (30) are rolled off — deleted on the next -`SessionStart`. +`SessionStart`, whether or not telemetry is currently enabled, so data recorded +before a user opted out still ages out. ## Configuration @@ -58,6 +94,18 @@ telemetry?") and can be toggled later with `cargo agents telemetry enable` / - `cargo agents telemetry show [--count N]` — print recent events for inspection (the data the user would share). +## Scope: symposium's own activity + +Every activation, sync, and hook event is computed from symposium's own plugin +registry (the configured plugin sources), not by scanning an agent's installed +skill directory. A skill or plugin that symposium did not install (a +hand-authored agent skill, or one from another source) is never in that +registry, so it produces no telemetry. A symposium skill gated on a non-crate +predicate is still recorded, since symposium activated it, with an empty +`crates` field. The `user_prompt` and `tool_use` events count session activity +as a whole but carry no skill or plugin name, so they never attribute unrelated +work to symposium. + ## What we deliberately do *not* record No prompt text, no shell command lines, no file paths. "How many tries until the diff --git a/src/cli.rs b/src/cli.rs index 1e3e58fc..cfb99efe 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -211,7 +211,9 @@ pub async fn run( init::init(sym, out, &opts).await } - Commands::Sync => sync::sync(sym, &mut sym.workspace_deps(cwd), update).await, + Commands::Sync => sync::sync(sym, &mut sym.workspace_deps(cwd), update) + .await + .map(drop), Commands::SelfUpdate => self_update::self_update(sym, out), diff --git a/src/config.rs b/src/config.rs index 84b74bb7..4cdbcfb1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,6 +4,11 @@ use std::fs; use std::path::{Path, PathBuf}; use tracing::Level; +use crate::telemetry; +use crate::telemetry::EventKind; +use crate::telemetry::RETENTION_DAYS; +use crate::telemetry::TelemetryEvent; + // --------------------------------------------------------------------------- // User configuration (~/.symposium/config.toml) // --------------------------------------------------------------------------- @@ -451,6 +456,37 @@ impl Symposium { &self.home_dir } + /// Record a hook pass's telemetry events, if the user opted in. + /// + /// The only opt-in gate, so callers emit unconditionally. Best-effort: a write failure must never break a hook + pub fn record_telemetry_batch(&self, session_id: Option, kinds: Vec) { + if !self.config.telemetry.enabled || kinds.is_empty() { + return; + } + + let events = kinds + .into_iter() + .map(|kind| TelemetryEvent::now(session_id.clone(), kind)) + .collect::>(); + + telemetry::record_all(self.config_dir(), &events); + } + + /// Record a single telemetry event. Convenience wrapper over + /// [`Self::record_telemetry_batch`]; same opt-in gate. + pub fn record_telemetry(&self, session_id: Option, kind: EventKind) { + self.record_telemetry_batch(session_id, vec![kind]); + } + + /// Delete telemetry older than [`RETENTION_DAYS`]. + /// Best-effort; call once per session (on `SessionStart`). + /// + /// Deliberately not gated on `telemetry.enabled`: retention is disk hygiene, + /// so date recorded before a user opted out must still age out. + pub fn roll_off_telemetry(&self) { + telemetry::roll_off(self.config_dir(), RETENTION_DAYS); + } + /// Returns the effective list of plugin sources, including built-in defaults. pub fn plugin_sources(&self) -> Vec { let mut sources = Vec::new(); @@ -730,6 +766,61 @@ mod tests { ); } + #[test] + fn record_telemetry_writes_nothing_when_disabled() { + let tmp = tempfile::tempdir().unwrap(); + let sym = Symposium::from_dir(tmp.path()); // telemetry off by default + assert!(!sym.config.telemetry.enabled); + + sym.record_telemetry(Some("s1".into()), telemetry::EventKind::UserPrompt); + + // The gate must produce no event log at all when the user hasn't opted in. + assert!(telemetry::read_events(sym.config_dir()).is_empty()); + } + + #[test] + fn record_telemetry_writes_when_enabled() { + let tmp = tempfile::tempdir().unwrap(); + let mut sym = Symposium::from_dir(tmp.path()); + sym.config.telemetry.enabled = true; + + sym.record_telemetry(Some("s1".into()), EventKind::UserPrompt); + + let events = telemetry::read_events(sym.config_dir()); + assert_eq!(events.len(), 1); + assert_eq!(events[0].kind_name(), "user_prompt"); + assert_eq!(events[0].session_id.as_deref(), Some("s1")); + } + + #[test] + fn record_telemetry_batch_writes_every_event_with_the_session_ids() { + let tmp = tempfile::tempdir().unwrap(); + let mut sym = Symposium::from_dir(tmp.path()); + sym.config.telemetry.enabled = true; + + sym.record_telemetry_batch( + Some("s1".to_string()), + vec![ + EventKind::SessionStart { + agent: "claude".to_string(), + crate_count: Some(2), + }, + EventKind::UserPrompt, + EventKind::Stop, + ], + ); + + let events = telemetry::read_events(sym.config_dir()); + assert_eq!(events.len(), 3); + assert_eq!(events[0].kind_name(), "session_start"); + assert_eq!(events[2].kind_name(), "stop"); + assert!( + events + .iter() + .all(|ev| ev.session_id.as_deref() == Some("s1")) + ) + } + #[test] fn parse_agents_syncing_disabled() { let config = parse_config(indoc! {" diff --git a/src/hook.rs b/src/hook.rs index 62605d3a..5209a9d6 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -2,16 +2,15 @@ use std::{ io::{Read, Write}, path::PathBuf, process::{Command, ExitCode, Stdio}, + time::Instant, }; use symposium_install::Runnable; -use crate::installation::{ - AcquiredInstallation, AcquiredRunnable, acquire_installation, refresh_installation_if_present, - resolve_runnable, -}; use crate::plugins::{HookFormat, Installation}; use crate::pm::PackageManager as _; +use crate::sync; +use crate::telemetry::EventKind; use crate::{ config::Symposium, hook_schema::{AgentHookInput, symposium}, @@ -22,6 +21,13 @@ use crate::{ hook_schema::symposium::{OutputEvent, SessionStartInput}, subcommand_dispatch::applicable_subcommands, }; +use crate::{ + installation::{ + AcquiredInstallation, AcquiredRunnable, acquire_installation, + refresh_installation_if_present, resolve_runnable, + }, + sync::sync, +}; use symposium_sdk::workspace::WorkspaceDeps; /// A hook prepared for dispatch — installation names looked up to concrete @@ -244,7 +250,7 @@ pub async fn execute_hook( // Auto-sync: install applicable skills into agent dirs (non-fatal). // SessionStart refreshes source caches and syncs unconditionally. let session_start = event == HookEvent::SessionStart; - run_auto_sync(sym, &mut deps, session_start).await; + let sync_summary = run_auto_sync(sym, &mut deps, session_start).await; // SessionStart (once per session) also refreshes every hook's already- // cached source, so later events dispatch fresh binaries without per- @@ -259,7 +265,10 @@ pub async fn execute_hook( let prior_output = builtin_agent_output.to_hook_output(); // Plugin dispatch with format routing - let final_output = dispatch_plugin_hooks( + let DispatchOutcome { + invocations, + result, + } = dispatch_plugin_hooks( sym, agent, event, @@ -268,8 +277,23 @@ pub async fn execute_hook( prior_output, &mut deps, ) - .await - .map_err(|stderr| { + .await; + + // Emit before propagating a block: the tool ran and the hooks that + // completed ran, so a blocked pass must not be a telemetry hole. + let events = telemetry_events(agent, &sym_input, sync_summary, invocations); + + if !events.is_empty() { + let session_id = sym_input.session_id().map(str::to_string); + sym.record_telemetry_batch(session_id, events); + } + + // Enforce telemetry retention once per session. + if session_start { + sym.roll_off_telemetry(); + } + + let final_output = result.map_err(|stderr| { anyhow::anyhow!("plugin blocked: {}", String::from_utf8_lossy(&stderr)) })?; @@ -282,6 +306,94 @@ pub async fn execute_hook( } } +/// Map a completed hook pass to its telemetry events. Pure: no I/O, no opt-in check so it is unit-testable. +/// +/// Consumes `sync_summary` and `hooks` so their strings move into the events rather than being cloned. +/// `session_id` is not produced here; it rides on the `TelemetryEvent` envelope, +/// so the caller pairs each returned kind with the wire input's session id. +fn telemetry_events( + agent: HookAgent, + sym_input: &symposium::InputEvent, + sync_summary: Option, + hooks: Vec, +) -> Vec { + let session_start = matches!(sym_input, symposium::InputEvent::SessionStart(_)); + + let mut events = Vec::new(); + if let Some(kind) = wire_event(agent, sym_input, sync_summary.as_ref()) { + events.push(kind); + } + + if let Some(summary) = sync_summary { + events.extend(sync_events(summary, session_start)); + } + + events.extend(hooks.into_iter().map(|hk| EventKind::HookInvocation { + hook: hk.hook, + plugin: hk.plugin, + duration_ms: hk.duration_ms, + exit_code: hk.exit_code, + })); + + events +} + +/// The wire-funnel event for this hook event, if any. `PreToolUse` maps to `None` so +/// a tool is counted once (on `PostToolUse`), not twice. +fn wire_event( + agent: HookAgent, + sym_input: &symposium::InputEvent, + sync_summary: Option<&sync::SyncSummary>, +) -> Option { + Some(match sym_input { + symposium::InputEvent::SessionStart(_) => EventKind::SessionStart { + agent: agent.as_str().to_string(), + crate_count: sync_summary.map(|ss| ss.crate_count), + }, + symposium::InputEvent::UserPromptSubmit(_) => EventKind::UserPrompt, + symposium::InputEvent::PostToolUse(ptui) => EventKind::ToolUse { + tool: ptui.tool_name.clone(), + }, + symposium::InputEvent::Stop(_) => EventKind::Stop, + _ => return None, + }) +} + +/// The sync-pass events: `sync_run` whenever the body ran, plus the plugin/skills activation +/// snapshot only on the SessionStart sync (recorded once per session). +fn sync_events(summary: sync::SyncSummary, session_start: bool) -> Vec { + let mut events = vec![EventKind::SyncRun { + installed: summary.installed, + reaped: summary.reaped, + plugins_matched: summary.plugins.len(), + }]; + + if session_start { + events.extend( + summary + .plugins + .into_iter() + .map(|pa| EventKind::PluginActivation { + plugin: pa.name, + crates: pa.crates, + }), + ); + + events.extend( + summary + .skills + .into_iter() + .map(|ska| EventKind::SkillActivation { + skill: ska.name, + plugin: ska.plugin, + crates: ska.crates, + }), + ); + } + + events +} + /// CLI entry point: read payload from stdin, dispatch, print output. pub async fn run(sym: &Symposium, agent: HookAgent, event: HookEvent) -> ExitCode { tracing::debug!("Running hook listener for agent {agent:?} and event {event:?}"); @@ -351,10 +463,14 @@ fn write_hook_trace(agent: HookAgent, event: HookEvent, input: &str, output: &[u /// every source cache (`UpdateLevel::Check`) and sync unconditionally, ignoring /// the `Cargo.lock` freshness gate — upstream skill changes land even when the /// workspace's dependencies are unchanged. -async fn run_auto_sync(sym: &Symposium, deps: &mut WorkspaceDeps, session_start: bool) { +async fn run_auto_sync( + sym: &Symposium, + deps: &mut WorkspaceDeps, + session_start: bool, +) -> Option { if !sym.config.auto_sync { tracing::debug!("auto-sync disabled, skipping"); - return; + return None; } let cwd = deps.cwd().to_path_buf(); @@ -370,7 +486,7 @@ async fn run_auto_sync(sym: &Symposium, deps: &mut WorkspaceDeps, session_start: let state = crate::workspace_state::WorkspaceState::load(sym, root); if state.sync_is_fresh(root) { tracing::debug!("auto-sync skipped: Cargo.lock unchanged since last sync"); - return; + return None; } } @@ -381,10 +497,13 @@ async fn run_auto_sync(sym: &Symposium, deps: &mut WorkspaceDeps, session_start: }; tracing::debug!("auto-sync running"); - if let Err(e) = crate::sync::sync(sym, deps, update).await { - tracing::warn!(error = %e, "auto-sync during hook failed (continuing)"); - return; - } + let summary = match sync(sym, deps, update).await { + Ok(summary) => summary, + Err(err) => { + tracing::warn!(error = %err, "auto-sync during hook failed (continuing)"); + return None; + } + }; // Record successful sync. If we didn't find the root earlier, // try again now (sync may have created Cargo.lock). @@ -395,6 +514,8 @@ async fn run_auto_sync(sym: &Symposium, deps: &mut WorkspaceDeps, session_start: state.workspace_root = Some(root.clone()); state.save(sym, root); } + + Some(summary) } /// Refresh the source cache for every hook the workspace could fire this @@ -555,13 +676,26 @@ pub enum PluginHookOutput { Failure(Vec), } +/// One plugin hook that ran during dispatch, with how long it took. +/// A plain record kept out of the telemetry types so dispatch stays decoupled from the +/// wire schema; `execute_hook` maps it to a `telemetry::EventKind`. +#[derive(Debug, Clone)] +pub struct HookInvocation { + pub plugin: String, + pub hook: String, + pub duration_ms: u64, + /// `None` when the hook was killed by a signal. + pub exit_code: Option, +} + /// Dispatch plugin hooks with format routing. /// /// Accumulates output as `serde_json::Value` in the host agent's wire format. /// When a plugin's format matches the host agent, input/output pass through directly. /// When formats differ, conversion goes through symposium canonical types. /// -/// Returns `Ok(json)` on success, `Err(stderr)` on exit code 2. +/// Dispatch stops at the first hook that blocks, but still reports the hooks +/// that already ran. pub async fn dispatch_plugin_hooks( sym: &Symposium, host_agent: HookAgent, @@ -570,7 +704,45 @@ pub async fn dispatch_plugin_hooks( original_input: &dyn AgentHookInput, prior_output: serde_json::Value, deps: &mut WorkspaceDeps, -) -> Result> { +) -> DispatchOutcome { + let hooks = select_dispatch_hooks(sym, host_agent, sym_input, deps); + + let mut output = prior_output; + let mut invocations = Vec::new(); + for hook in hooks { + match run_plugin_hook(sym, host_agent, event, sym_input, original_input, &hook).await { + HookRun::Ran { + invocation, + to_merge, + } => { + invocations.push(invocation); + if let Some(json) = to_merge { + merge(&mut output, json); + } + } + HookRun::Blocked { invocation, stderr } => { + invocations.push(invocation); + return DispatchOutcome { + invocations, + result: Err(stderr), + }; + } + HookRun::Skipped => {} + } + } + DispatchOutcome { + invocations, + result: Ok(output), + } +} + +/// Load the plugins and resolve which of their hooks apply to this event. +fn select_dispatch_hooks( + sym: &Symposium, + host_agent: HookAgent, + sym_input: &symposium::InputEvent, + deps: &mut WorkspaceDeps, +) -> Vec { let workspace = deps.load().cloned(); let plugins = crate::plugins::load_all_plugins(sym, workspace.as_deref()); @@ -583,132 +755,187 @@ pub async fn dispatch_plugin_hooks( Vec::new() }; let mut ctx = crate::predicate::PredicateContext::new(&dep_ids); - let hooks = dispatched_hooks_for_payload(&plugins, sym_input, host_agent, &mut ctx); + dispatched_hooks_for_payload(&plugins, sym_input, host_agent, &mut ctx) +} - let mut output = prior_output; +/// Outcome of running one plugin hook. +enum HookRun { + /// The hook ran; `to_merge` is its output to fold into the aggregate, if any. + Ran { + invocation: HookInvocation, + to_merge: Option, + }, + /// The hook ran and blocked the tool (exit 2 or killed). It still carries an + /// invocation: it ran, and a blocked pass must not be a telemetry hole. + Blocked { + invocation: HookInvocation, + stderr: Vec, + }, + /// The hook did not run (serialize / prepare / spawn / wait failure, already + /// logged), so there is nothing to record or merge. + Skipped, +} - for hook in hooks { - tracing::info!( - plugin = %hook.plugin_name, - hook = %hook.hook_name, - format = ?hook.format, - "running plugin hook" - ); +/// What one dispatch pass produced. +pub struct DispatchOutcome { + /// Every hook that ran, including one that blocked. Collected independently + /// of `result` so a blocked pass still reports the work that happened. + pub invocations: Vec, + /// The merged output, or the blocking hook's stderr (exit 2 / killed). + pub result: Result>, +} - // Determine stdin for the plugin based on its declared format. - // After format selection, the only two cases are: - // - native (matches host agent) → pass through original input - // - symposium → deliver canonical format - let hook_agent = hook.format.as_agent(); - let hook_input: &dyn AgentHookInput = if hook_agent == Some(host_agent) { - original_input - } else { - sym_input - }; - let stdin_str = match hook_input.to_string() { - Ok(s) => s, - Err(e) => { - tracing::error!(plugin = %hook.plugin_name, hook = %hook.hook_name, error = %e, "failed to serialize hook input"); - continue; - } - }; +/// Run one plugin hook end to end: route its stdin by declared format, spawn it, +/// time it, and interpret its output. +async fn run_plugin_hook( + sym: &Symposium, + host_agent: HookAgent, + event: HookEvent, + sym_input: &symposium::InputEvent, + original_input: &dyn AgentHookInput, + hook: &ResolvedHook, +) -> HookRun { + tracing::info!( + plugin = %hook.plugin_name, + hook = %hook.hook_name, + format = ?hook.format, + "running plugin hook" + ); + + // Route stdin by the hook's declared format: native (matches the host agent) + // gets the original input, otherwise the canonical symposium format. + let hook_agent = hook.format.as_agent(); + let hook_input: &dyn AgentHookInput = if hook_agent == Some(host_agent) { + original_input + } else { + sym_input + }; + let stdin_str = match hook_input.to_string() { + Ok(hook_output) => hook_output, + Err(err) => { + tracing::error!(plugin = %hook.plugin_name, hook = %hook.hook_name, error = %err, "failed to serialize hook input"); + return HookRun::Skipped; + } + }; - let spawn_res = match build_spawn_spec(sym, &hook).await { - Ok(spec) => spawn_from_spec(spec), - Err(e) => { - tracing::warn!(error = %e, "failed to prepare hook command"); - continue; - } - }; + let spawn_res = match build_spawn_spec(sym, hook).await { + Ok(spec) => spawn_from_spec(spec), + Err(e) => { + tracing::warn!(error = %e, "failed to prepare hook command"); + return HookRun::Skipped; + } + }; - match spawn_res { - Ok(mut child) => { - if let Some(mut stdin) = child.stdin.take() { - let _ = stdin.write_all(stdin_str.as_bytes()); - } + let mut child = match spawn_res { + Ok(child) => child, + Err(err) => { + tracing::debug!( + report = %crate::report::ReportEvent::HookDispatched { + plugin: hook.plugin_name.clone(), + hook: hook.hook_name.clone(), + exit_code: None, + error: Some(err.to_string()), + }, + ); + tracing::warn!(error = %err, "failed to spawn hook command"); + return HookRun::Skipped; + } + }; - let child_out = match child.wait_with_output() { - Ok(o) => o, - Err(e) => { - tracing::warn!(error = %e, "failed waiting for hook process"); - continue; - } - }; + let start = Instant::now(); + if let Some(mut stdin) = child.stdin.take() { + let _ = stdin.write_all(stdin_str.as_bytes()); + } - tracing::trace!(?child_out, "hook finished"); + let child_out = match child.wait_with_output() { + Ok(output) => output, + Err(err) => { + tracing::warn!(error = %err, "failed waiting for hook process"); + return HookRun::Skipped; + } + }; - let exit_code = child_out.status.code(); - tracing::debug!( - report = %crate::report::ReportEvent::HookDispatched { - plugin: hook.plugin_name.clone(), - hook: hook.hook_name.clone(), - exit_code, - error: None, - }, - ); - match exit_code { - None | Some(2) => return Err(child_out.stderr), - Some(0) if child_out.stdout.is_empty() => continue, - Some(0) => { - // Parse output and convert to host agent format. - // Two cases: native (same as host) or symposium. - let host_handler = host_agent.event(event); - let Some(host_h) = host_handler else { continue }; - - let host_json = if hook_agent == Some(host_agent) { - // Native format — parse as host agent output - match host_h.parse_output(&child_out.stdout) { - Ok(o) => o.to_hook_output(), - Err(e) => { - tracing::warn!(error = %e, "failed to parse hook output"); - continue; - } - } + tracing::trace!(?child_out, "hook finished"); + + let invocation = HookInvocation { + plugin: hook.plugin_name.clone(), + hook: hook.hook_name.clone(), + duration_ms: start.elapsed().as_millis() as u64, + exit_code: child_out.status.code(), + }; + tracing::debug!( + report = %crate::report::ReportEvent::HookDispatched { + plugin: hook.plugin_name.clone(), + hook: hook.hook_name.clone(), + exit_code: child_out.status.code(), + error: None, + }, + ); + + match interpret_hook_output(host_agent, event, hook_agent, child_out) { + Ok(to_merge) => HookRun::Ran { + invocation, + to_merge, + }, + Err(stderr) => HookRun::Blocked { invocation, stderr }, + } +} + +/// Interpret a finished hook's exit code and stdout into the JSON to merge. +/// `Err(stderr)` signals a block (exit 2 or killed); `Ok(None)` means the hook +/// produced nothing to merge. +fn interpret_hook_output( + host_agent: HookAgent, + event: HookEvent, + hook_agent: Option, + child_out: std::process::Output, +) -> Result, Vec> { + match child_out.status.code() { + None | Some(2) => Err(child_out.stderr), + Some(0) if child_out.stdout.is_empty() => Ok(None), + Some(0) => { + // Parse the hook's stdout and convert it to host-agent format. Two + // cases: native (same as host) or the canonical symposium format. + let Some(host_h) = host_agent.event(event) else { + return Ok(None); + }; + + let host_json = if hook_agent == Some(host_agent) { + match host_h.parse_output(&child_out.stdout) { + Ok(output) => output.to_hook_output(), + Err(err) => { + tracing::warn!(error = %err, "failed to parse hook output"); + return Ok(None); + } + } + } else { + match serde_json::from_slice::(&child_out.stdout) { + Ok(value) => { + if let Ok(sym_out) = + serde_json::from_value::(value.clone()) + { + host_h.translate_output(&sym_out).to_hook_output() } else { - // Symposium format — parse and convert to host agent - match serde_json::from_slice::(&child_out.stdout) { - Ok(v) => { - if let Ok(sym_out) = - serde_json::from_value::(v.clone()) - { - let host_out = host_h.translate_output(&sym_out); - host_out.to_hook_output() - } else { - v - } - } - Err(e) => { - tracing::warn!(error = %e, "failed to parse hook output"); - continue; - } - } - }; - - merge(&mut output, host_json); + value + } } - Some(code) => { - tracing::warn!( - exit_code = code, - "plugin hook exited with non-zero (continuing)" - ); + Err(err) => { + tracing::warn!(error = %err, "failed to parse hook output"); + return Ok(None); } } - } - Err(e) => { - tracing::debug!( - report = %crate::report::ReportEvent::HookDispatched { - plugin: hook.plugin_name.clone(), - hook: hook.hook_name.clone(), - exit_code: None, - error: Some(e.to_string()), - }, - ); - tracing::warn!(error = %e, "failed to spawn hook command"); - } + }; + + Ok(Some(host_json)) + } + Some(code) => { + tracing::warn!( + exit_code = code, + "plugin hook exited with non-zero (continuing)" + ); + Ok(None) } } - - Ok(output) } /// Recursively merge two JSON objects, with `b` taking precedence over `a`. @@ -856,6 +1083,191 @@ mod tests { use super::*; + // --- telemetry_events mapping --- + + #[test] + fn telemetry_events_session_start_emits_funnel_and_activations() { + let input = symposium::InputEvent::SessionStart(symposium::SessionStartInput::new( + Some("s1".into()), + None, + )); + let summary = sync::SyncSummary { + plugins: vec![crate::skills::PluginActivation { + name: "example-plugin".into(), + crates: vec!["acme-core".into()], + }], + skills: vec![crate::skills::SkillActivation { + name: "example-skill".into(), + plugin: Some("example-plugin".into()), + crates: vec!["acme-core".into()], + }], + installed: 2, + reaped: 1, + crate_count: 5, + }; + let events = telemetry_events(HookAgent::Claude, &input, Some(summary), vec![]); + + assert_eq!(events.len(), 4); + match &events[0] { + EventKind::SessionStart { agent, crate_count } => { + assert_eq!(agent, "claude"); + assert_eq!(*crate_count, Some(5)); + } + other => panic!("expected session_start, got {other:?}"), + } + assert!(matches!( + events[1], + EventKind::SyncRun { + installed: 2, + reaped: 1, + plugins_matched: 1 + } + )); + assert!(matches!(events[2], EventKind::PluginActivation { .. })); + assert!(matches!(events[3], EventKind::SkillActivation { .. })); + } + + #[test] + fn telemetry_events_non_session_sync_records_sync_run_but_no_activations() { + let input = symposium::InputEvent::PostToolUse(symposium::PostToolUseInput::new( + "Bash".into(), + serde_json::Value::Null, + serde_json::Value::Null, + Some("s1".into()), + None, + )); + let summary = sync::SyncSummary { + plugins: vec![crate::skills::PluginActivation { + name: "example-plugin".into(), + crates: vec![], + }], + skills: vec![], + installed: 0, + reaped: 0, + crate_count: 3, + }; + let events = telemetry_events(HookAgent::Claude, &input, Some(summary), vec![]); + + // tool_use + sync_run, but the activation snapshot is SessionStart-only. + assert_eq!(events.len(), 2); + assert!(matches!(events[0], EventKind::ToolUse { .. })); + assert!(matches!(events[1], EventKind::SyncRun { .. })); + assert!(!events.iter().any(|e| matches!( + e, + EventKind::PluginActivation { .. } | EventKind::SkillActivation { .. } + ))); + } + + #[test] + fn telemetry_events_pre_tool_use_is_silent() { + let input = symposium::InputEvent::PreToolUse(symposium::PreToolUseInput::new( + "Bash".into(), + serde_json::Value::Null, + Some("s1".into()), + None, + )); + let events = telemetry_events(HookAgent::Claude, &input, None, vec![]); + assert!(events.is_empty()); + } + + #[test] + fn telemetry_events_pre_tool_use_still_records_hook_invocations() { + // "PreToolUse is silent" means only the wire funnel event; a hook that + // ran on a PreToolUse pass must still be counted. + let input = symposium::InputEvent::PreToolUse(symposium::PreToolUseInput::new( + "Bash".into(), + serde_json::Value::Null, + Some("s1".into()), + None, + )); + let hooks = vec![HookInvocation { + plugin: "example-plugin".into(), + hook: "format-check".into(), + duration_ms: 3, + exit_code: Some(0), + }]; + let events = telemetry_events(HookAgent::Claude, &input, None, hooks); + + assert_eq!(events.len(), 1); + assert!(matches!( + events[0], + EventKind::HookInvocation { duration_ms: 3, .. } + )); + } + + #[test] + fn telemetry_events_maps_hook_invocations() { + let input = symposium::InputEvent::PostToolUse(symposium::PostToolUseInput::new( + "Edit".into(), + serde_json::Value::Null, + serde_json::Value::Null, + None, + None, + )); + let hooks = vec![HookInvocation { + plugin: "example-plugin".into(), + hook: "format-check".into(), + duration_ms: 7, + exit_code: Some(1), + }]; + let events = telemetry_events(HookAgent::Claude, &input, None, hooks); + + assert_eq!(events.len(), 2); + match &events[0] { + EventKind::ToolUse { tool } => assert_eq!(tool, "Edit"), + other => panic!("expected tool_use, got {other:?}"), + } + assert!(matches!( + events[1], + EventKind::HookInvocation { duration_ms: 7, .. } + )); + } + + // --- interpret_hook_output: exit-code handling --- + + /// A finished-process `Output` with a chosen exit code and stderr, built + /// without spawning so the block path is testable on every OS. + /// + /// Unix `ExitStatus::from_raw` wants the raw `waitpid` status, which carries + /// the exit code in the second byte, so the code is shifted left by 8. + #[cfg(unix)] + fn output_with_code(code: i32, stderr: &[u8]) -> std::process::Output { + use std::os::unix::process::ExitStatusExt; + std::process::Output { + status: std::process::ExitStatus::from_raw(code << 8), + stdout: Vec::new(), + stderr: stderr.to_vec(), + } + } + + #[cfg(windows)] + fn output_with_code(code: i32, stderr: &[u8]) -> std::process::Output { + use std::os::windows::process::ExitStatusExt; + std::process::Output { + status: std::process::ExitStatus::from_raw(code as u32), + stdout: Vec::new(), + stderr: stderr.to_vec(), + } + } + + #[test] + fn interpret_hook_output_blocks_on_exit_2() { + // Exit 2 is the block signal: dispatch surfaces the hook's stderr as the + // deny reason (Err) rather than merging any output. + let out = output_with_code(2, b"denied by policy"); + let result = interpret_hook_output(HookAgent::Claude, HookEvent::PreToolUse, None, out); + assert_eq!(result, Err(b"denied by policy".to_vec())); + } + + #[test] + fn interpret_hook_output_does_not_block_on_other_nonzero() { + // Only exit 2 blocks. Any other non-zero code is a soft failure that + // continues with nothing to merge. + let out = output_with_code(1, b"just a warning"); + let result = interpret_hook_output(HookAgent::Claude, HookEvent::PreToolUse, None, out); + assert_eq!(result, Ok(None)); + } + #[test] fn env_safe_sanitizes_punctuation() { assert_eq!(env_safe("rtk"), "rtk"); diff --git a/src/plugins.rs b/src/plugins.rs index bff6a952..3e7a85e4 100644 --- a/src/plugins.rs +++ b/src/plugins.rs @@ -422,7 +422,7 @@ pub struct ParsedPlugin { /// name (registry) or `"local"` (workspace). /// /// Used only to key chained-plugin cycle/diamond detection on the normalized - /// crate name (see `skills::expand_chained_plugins`). It does *not* affect + /// crate name (see `skills::Resolver::expand_chained`). It does *not* affect /// skill identity — that is the `SKILL.md` path hash. /// /// FIXME: the registry/workspace placeholder `pm` tags (`"user-plugins"`, @@ -472,7 +472,7 @@ pub struct Plugin { pub custom_predicates: Vec, /// Chained plugin references (`[[plugins]]`): whenever this plugin is /// active and any per-edge predicates hold, the referenced plugin loads - /// too. Expanded during skill resolution by `skills::expand_chained_plugins`. + /// too. Expanded during skill resolution by `skills::Resolver::expand_chained`. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub chained: Vec, } diff --git a/src/skills.rs b/src/skills.rs index c98792fe..3402e981 100644 --- a/src/skills.rs +++ b/src/skills.rs @@ -3,17 +3,19 @@ //! Skills follow the [agentskills.io](https://agentskills.io/specification.md) format //! and live inside plugin directories under `skills/*/SKILL.md`. -use std::collections::BTreeMap; +#[cfg(test)] +use std::collections::HashMap; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use symposium_install::UpdateLevel; +use symposium_sdk::workspace::WorkspaceCrate; use crate::config::Symposium; use crate::plugins::{ParsedPlugin, PluginRegistry, PluginSource, SkillGroup}; use crate::pm::PackageManager as _; use crate::predicate::{self, PredicateContext, PredicateSet}; - fn source_display(source: &PluginSource) -> String { match source { PluginSource::Path(p) => format!("path:{}", p.display()), @@ -88,41 +90,155 @@ pub(crate) fn skill_origin_hash(skill_md: &Path) -> String { /// /// The plugin-, group-, and skill-level predicate sets are all evaluated during /// collection; only skills whose every level holds end up here. +#[derive(Debug)] pub struct SkillWithGroupContext { pub skill: Skill, /// The hash of where the skill was discovered. Drives install-path disambiguation /// and dedup at sync time. pub origin_hash: String, + /// `None` for a standalone SKILL.md that no plugin vends. + pub plugin: Option, + /// Unioned across the plugin, group, and skill predicate levels; empty for wildcard / non-crate gates. + pub crates: Vec, } -/// Resolve all applicable skills from the registry. -/// -/// Resolve all skills applicable to the given crates. -/// -/// `for_crates` is the set of crate name/version pairs to match against. -/// For `crate --list`, this is the full workspace deps. -/// For `crate `, this is a single-element slice with the resolved crate. -pub async fn skills_applicable_to( +impl SkillWithGroupContext { + /// Project to the lean [`SkillActivation`] kept in a sync summary. + pub fn activation(&self) -> SkillActivation { + SkillActivation { + name: self.skill.name().to_string(), + plugin: self.plugin.clone(), + crates: self.crates.clone(), + } + } +} + +/// A plugin that matched the workspace during resolution. +#[derive(Debug, Clone)] +pub struct PluginActivation { + pub name: String, + pub crates: Vec, +} + +/// A matched skill projected for a sync summary: name, owning plugin, and the +/// crates that activated it. +#[derive(Debug, Clone)] +pub struct SkillActivation { + pub name: String, + /// `None` for a standalone SKILL.md that no plugin vends. + pub plugin: Option, + pub crates: Vec, +} + +/// The applicable skills plus the plugins that matched. Plugins are tracked separately +/// because one can match the workspace yet vend no applicable skill, and that match +/// still needs recording. +#[derive(Debug)] +pub struct ApplicableSkills { + pub skills: Vec, + pub plugins: Vec, +} + +/// Resolve every skill applicable to `workspace_crates`, together with the +/// plugins that matched (a plugin can match yet vend no skill). +pub async fn resolve_applicable( sym: &Symposium, registry: &PluginRegistry, - workspace_crates: &[symposium_sdk::workspace::WorkspaceCrate], + workspace_crates: &[WorkspaceCrate], custom_predicate_entries: std::collections::HashMap, update: UpdateLevel, -) -> Vec { - let mut results = Vec::new(); - +) -> ApplicableSkills { let for_crates = crate::pm::CargoPm.list_deps(workspace_crates); - let mut ctx = PredicateContext::with_custom_predicates(&for_crates, custom_predicate_entries); + let dep_names = for_crates.iter().map(|id| id.name.as_str()).collect(); + let mut resolver = Resolver { + sym, + workspace_crates, + update, + ctx: PredicateContext::with_custom_predicates(&for_crates, custom_predicate_entries), + dep_names, + skills: Vec::new(), + plugins: Vec::new(), + }; + resolver.resolve(registry).await; + ApplicableSkills { + skills: resolver.skills, + plugins: dedup_plugins(resolver.plugins), + } +} + +/// Collapse plugin activations to one per name. A crate chained from two +/// top-level plugins is walked once per owner (`visited` is per-plugin), so its +/// activation is recorded each time; merge those into a single record whose +/// crates are the union. +fn dedup_plugins(plugins: Vec) -> Vec { + let mut merged: BTreeMap> = BTreeMap::new(); + for activation in plugins { + merged + .entry(activation.name) + .or_default() + .extend(activation.crates); + } + merged + .into_iter() + .map(|(name, crates)| PluginActivation { + name, + crates: crates.into_iter().collect(), + }) + .collect() +} - // Skills from plugin manifests. We iterate these separately - // because we lazily load skill groups, so there - // is extra logic. - for parsed in ®istry.plugins { +/// One `resolve_applicable` pass: the shared inputs and predicate cursor plus +/// the growing activation lists. Bundling them keeps the recursive +/// chained-plugin walk from threading a long argument list. +struct Resolver<'a> { + sym: &'a Symposium, + workspace_crates: &'a [WorkspaceCrate], + update: UpdateLevel, + ctx: PredicateContext<'a>, + /// Workspace dependency names, deduped up front so `matched_names` is a set + /// lookup rather than a scan of `ctx.deps` per referenced crate. + dep_names: HashSet<&'a str>, + skills: Vec, + plugins: Vec, +} + +impl Resolver<'_> { + /// Walk the registry: each plugin's own groups and chained crates, then the + /// standalone skills that no plugin vends. + async fn resolve(&mut self, registry: &PluginRegistry) { + for parsed in ®istry.plugins { + self.visit_plugin(parsed).await; + } + + if !registry.standalone_skills.is_empty() { + tracing::debug!( + report = %crate::report::ReportEvent::PluginConsidered { + plugin: "(standalone skills)".into(), + matched: true, + reason: None, + }, + ); + } + // Standalone skills have no defining plugin, so they never count as + // workspace members: clear any stamp left by the plugin walk. + self.ctx.set_workspace_member(false); + for entry in ®istry.standalone_skills { + self.collect_skill( + entry.skill.clone(), + entry.origin_hash.clone(), + None, + &BTreeSet::new(), + ); + } + } + + /// Gate a top-level plugin; if it holds, record it and collect its own skill + /// groups and `[[plugins]]` chained references. + async fn visit_plugin(&mut self, parsed: &ParsedPlugin) { let plugin = &parsed.plugin; - // Plugin-level predicates gate everything below. Evaluated before - // group fetching to avoid wasted work. Goes through the ParsedPlugin - // so the plugin's provenance is stamped for `workspace-member()`. - if !parsed.applies(&mut ctx) { + // `applies` stamps the plugin's provenance for `workspace-member()`; the + // stamp carries to the groups and skills gated below. + if !parsed.applies(&mut self.ctx) { tracing::debug!( report = %crate::report::ReportEvent::PluginConsidered { plugin: plugin.name.clone(), @@ -130,9 +246,8 @@ pub async fn skills_applicable_to( reason: Some("plugin-level predicates not satisfied".into()), }, ); - continue; + return; } - tracing::debug!( report = %crate::report::ReportEvent::PluginConsidered { plugin: plugin.name.clone(), @@ -141,64 +256,173 @@ pub async fn skills_applicable_to( }, ); - for group in &plugin.skills { - let skills = load_skills_for_group(sym, parsed, group, &mut ctx, update).await; + let plugin_crates = self.matched_names(&plugin.predicates); + self.plugins.push(PluginActivation { + name: plugin.name.clone(), + crates: plugin_crates.iter().cloned().collect(), + }); + self.collect_groups(parsed, &plugin_crates).await; + + let mut visited = HashSet::new(); + self.expand_chained(parsed, &plugin_crates, &mut visited, 0) + .await; + } + + /// Load and collect the skills of `parsed`'s own groups, attributing each to + /// `parsed` with the plugin-and-group crates. + async fn collect_groups(&mut self, parsed: &ParsedPlugin, plugin_crates: &BTreeSet) { + for group in &parsed.plugin.skills { + let skills = + load_skills_for_group(self.sym, parsed, group, &mut self.ctx, self.update).await; + let mut inherited = plugin_crates.clone(); + inherited.extend(self.matched_names(&group.predicates)); for (skill, origin_hash) in skills { - collect_skill_applicable_to( - skill, - origin_hash, - &plugin.name, - &mut ctx, - &mut results, - ); + self.collect_skill(skill, origin_hash, Some(&parsed.plugin.name), &inherited); } } + } - // `[[plugins]]` chained references: whenever this plugin is active and - // an edge's own predicates hold, the referenced crate is loaded as a - // first-class plugin and its skills contributed. Expansion recurses - // into the loaded crate's own chained edges — a crate that names - // another crate (the reschema'd `[package.metadata.symposium]` - // redirect) is followed transitively — with per-plugin cycle detection. - let mut visited = std::collections::HashSet::new(); - expand_chained_plugins( - sym, - parsed, - workspace_crates, - &mut ctx, - update, - &mut visited, - 0, - &mut results, - ) - .await; + /// Expand `owner`'s chained references, recursively. For each edge whose + /// predicates hold, the named crate loads as a first-class plugin (its own + /// gate honored), its skills are collected, and its own chained edges are + /// expanded in turn. `visited` (normalized crate names) collapses diamonds + /// and breaks cycles; `depth`/[`MAX_CHAIN_DEPTH`] is a backstop. + async fn expand_chained( + &mut self, + owner: &ParsedPlugin, + inherited: &BTreeSet, + visited: &mut HashSet, + depth: usize, + ) { + if depth >= MAX_CHAIN_DEPTH { + tracing::warn!( + plugin = %owner.plugin.name, + "chained plugin expansion exceeded depth limit ({MAX_CHAIN_DEPTH}); stopping" + ); + return; + } + + for chained in &owner.plugin.chained { + // Edge predicates evaluate against the owning plugin's provenance; + // the crate plugin restamps its own below, so reset before the gate. + self.ctx.set_workspace_member(owner.workspace_member); + if !chained.predicates.evaluate(&mut self.ctx) { + continue; + } + + let Some(crate_plugin) = crate::pm::CargoPm + .load_plugin(&chained.name, self.workspace_crates) + .await + else { + continue; + }; + + // Cycle / diamond detection on the resolved crate identity, + // normalized so hyphen/underscore spellings collapse. + let key = crate::crate_sources::normalize_crate_name(&crate_plugin.canonical.name); + if !visited.insert(key) { + tracing::debug!( + crate_name = %chained.name, + "chained plugin already loaded on this chain; skipping (cycle or diamond)" + ); + continue; + } + + // Honor the crate plugin's own gate (which stamps its provenance: + // never a workspace member) before touching it. + if !crate_plugin.applies(&mut self.ctx) { + continue; + } + warn_undispatched_crate_features(&crate_plugin); + + // The crate plugin's crates: the owner chain, the edge, and its gate. + let mut plugin_crates = inherited.clone(); + plugin_crates.extend(self.matched_names(&chained.predicates)); + plugin_crates.extend(self.matched_names(&crate_plugin.plugin.predicates)); + self.plugins.push(PluginActivation { + name: crate_plugin.plugin.name.clone(), + crates: plugin_crates.iter().cloned().collect(), + }); + + self.collect_groups(&crate_plugin, &plugin_crates).await; + + Box::pin(self.expand_chained(&crate_plugin, &plugin_crates, visited, depth + 1)).await; + } } - // Standalone skills already carry their own origin hash (computed - // from the SKILL.md's on-disk path, like every other skill). - if !registry.standalone_skills.is_empty() { + /// Evaluate a skill's own gate and, if it holds, record it with its owning + /// plugin and the union of the inherited and skill-level crates. + fn collect_skill( + &mut self, + skill: Skill, + origin_hash: String, + plugin: Option<&str>, + inherited: &BTreeSet, + ) { + // `SkillConsidered` has no pluginless variant; standalone skills log here. + let plugin_label = plugin.unwrap_or("(standalone skills)"); + if !skill.predicates.evaluate(&mut self.ctx) { + tracing::debug!( + report = %crate::report::ReportEvent::SkillConsidered { + skill: skill.name().to_string(), + plugin: plugin_label.to_string(), + matched: false, + reason: Some("skill-level predicates not satisfied".into()), + }, + ); + return; + } tracing::debug!( - report = %crate::report::ReportEvent::PluginConsidered { - plugin: "(standalone skills)".into(), + report = %crate::report::ReportEvent::SkillConsidered { + skill: skill.name().to_string(), + plugin: plugin_label.to_string(), matched: true, reason: None, }, ); + + let mut crates = inherited.clone(); + crates.extend(self.matched_names(&skill.predicates)); + self.skills.push(SkillWithGroupContext { + skill, + origin_hash, + plugin: plugin.map(str::to_string), + crates: crates.into_iter().collect(), + }); } - // Standalone skills have no defining plugin; they never count as - // workspace members (clear any stamp left by the plugin loop). - ctx.set_workspace_member(false); - for entry in ®istry.standalone_skills { - collect_skill_applicable_to( - entry.skill.clone(), - entry.origin_hash.clone(), - "(standalone skills)", - &mut ctx, - &mut results, - ); + + /// Workspace crates a gate names *and* the workspace has: its referenced + /// dependency names (via [`PredicateSet::collect_dep_names`]) intersected + /// with the live deps. The coarse alternative to a predicate witness: it + /// over-reports only a satisfied `any(...)`'s non-firing branch, acceptable + /// for this names-only, privacy-bounded attribution. + fn matched_names(&self, gate: &PredicateSet) -> BTreeSet { + let mut names = BTreeSet::new(); + gate.collect_dep_names(&mut names); + names.retain(|name| self.dep_names.contains(name.as_str())); + names } +} - results +/// Skills-only view of [`resolve_applicable`], used by tests that don't need +/// the matched-plugin list. +#[cfg(test)] +pub async fn skills_applicable_to( + sym: &Symposium, + registry: &PluginRegistry, + workspace_crates: &[WorkspaceCrate], + custom_predicate_entries: HashMap, + update: UpdateLevel, +) -> Vec { + resolve_applicable( + sym, + registry, + workspace_crates, + custom_predicate_entries, + update, + ) + .await + .skills } /// Discover and load skills for a group, applying pre-fetch filtering. @@ -266,7 +490,7 @@ async fn load_skills_for_group( /// skills discovered inside it. Both `source` variants reduce to this: `Path` is /// already on disk; `Git` is fetched via the git cache. (A crate is not a group /// source — it becomes a plugin through a `[[plugins]]` chained reference; see -/// [`expand_chained_plugins`].) +/// [`Resolver::expand_chained`].) struct ResolvedSkillDir { dir: PathBuf, /// `SkillSourceSearched` report `plugin` label. @@ -355,102 +579,6 @@ fn warn_undispatched_crate_features(parsed: &ParsedPlugin) { } } -/// Expand an active plugin's `[[plugins]]` chained references, recursively. -/// -/// For each edge whose predicates hold (evaluated against the *owning* plugin's -/// provenance), the referenced crate is loaded as a first-class plugin via -/// [`CargoPm::load_plugin`], its own plugin-level predicates are honored, and -/// its skills are contributed with crate-origin identity. The loaded -/// crate's own chained edges are then expanded in turn — this is how a crate -/// that names another crate (a reschema'd `[package.metadata.symposium]` -/// redirect) is followed. -/// -/// `visited` holds the normalized crate names already loaded on this owning -/// plugin's chain; it collapses diamonds (a crate reached two ways loads once) -/// and breaks cycles. It is scoped per top-level plugin — cross-plugin dedup -/// stays the sync layer's job (via the origin hash). `depth`/[`MAX_CHAIN_DEPTH`] -/// is a backstop. -#[allow(clippy::too_many_arguments)] -async fn expand_chained_plugins( - sym: &Symposium, - owner: &ParsedPlugin, - workspace_crates: &[symposium_sdk::workspace::WorkspaceCrate], - ctx: &mut PredicateContext<'_>, - update: UpdateLevel, - visited: &mut std::collections::HashSet, - depth: usize, - results: &mut Vec, -) { - if depth >= MAX_CHAIN_DEPTH { - tracing::warn!( - plugin = %owner.plugin.name, - "chained plugin expansion exceeded depth limit ({MAX_CHAIN_DEPTH}); stopping" - ); - return; - } - - for chained in &owner.plugin.chained { - // Edge predicates evaluate against the owning plugin's provenance; the - // crate plugin's own `applies` (below) restamps its own — never a - // workspace member — so reset before each edge's gate. - ctx.set_workspace_member(owner.workspace_member); - if !chained.predicates.evaluate(ctx) { - continue; - } - - let Some(crate_plugin) = crate::pm::CargoPm - .load_plugin(&chained.name, workspace_crates) - .await - else { - continue; - }; - - // Cycle / diamond detection on the resolved crate identity, normalized - // so hyphen/underscore spellings of one crate collapse. - let key = crate::crate_sources::normalize_crate_name(&crate_plugin.canonical.name); - if !visited.insert(key) { - tracing::debug!( - crate_name = %chained.name, - "chained plugin already loaded on this chain; skipping (cycle or diamond)" - ); - continue; - } - - // Honor the crate plugin's own plugin-level predicates (which stamp its - // provenance: never a workspace member) before doing anything with it — - // an inactive crate plugin shouldn't warn about undispatched features. - if !crate_plugin.applies(ctx) { - continue; - } - warn_undispatched_crate_features(&crate_plugin); - - for group in &crate_plugin.plugin.skills { - let skills = load_skills_for_group(sym, &crate_plugin, group, ctx, update).await; - for (skill, origin_hash) in skills { - collect_skill_applicable_to( - skill, - origin_hash, - &crate_plugin.plugin.name, - ctx, - results, - ); - } - } - - Box::pin(expand_chained_plugins( - sym, - &crate_plugin, - workspace_crates, - ctx, - update, - visited, - depth + 1, - results, - )) - .await; - } -} - /// Discover skills in each resolved base dir and stamp origins. The single /// path all group sources funnel through, replacing the former per-source /// `load_*_skills` functions. @@ -703,40 +831,6 @@ fn load_skill( Ok(skill) } -/// Evaluate the skill-level predicate set and collect the skill if it holds. -/// -/// Plugin- and group-level predicates have already been evaluated by callers as -/// a pre-filter, so only the skill-level set is checked here. -fn collect_skill_applicable_to( - skill: Skill, - origin_hash: String, - plugin_name: &str, - ctx: &mut PredicateContext, - results: &mut Vec, -) { - if !skill.predicates.evaluate(ctx) { - tracing::debug!( - report = %crate::report::ReportEvent::SkillConsidered { - skill: skill.name().to_string(), - plugin: plugin_name.to_string(), - matched: false, - reason: Some("skill-level predicates not satisfied".into()), - }, - ); - return; - } - - tracing::debug!( - report = %crate::report::ReportEvent::SkillConsidered { - skill: skill.name().to_string(), - plugin: plugin_name.to_string(), - matched: true, - reason: None, - }, - ); - results.push(SkillWithGroupContext { skill, origin_hash }); -} - /// Raw frontmatter fields extracted from a SKILL.md file. /// `depends-on` is comma-separated on a single line. #[derive(Debug)] @@ -1426,6 +1520,119 @@ mod tests { assert_eq!(skills[0].skill.name(), "serde-basics"); } + #[tokio::test] + async fn resolve_applicable_attributes_crates_to_plugin_and_skill() { + use crate::plugins::{ParsedPlugin, Plugin, PluginRegistry, PluginSource, SkillGroup}; + use std::fs; + use tempfile::TempDir; + + let tmp = TempDir::new().unwrap(); + let sym = crate::config::Symposium::from_dir(tmp.path()); + + // A wildcard skill: it names no dependency of its own, so it can only + // "ride in" on the plugin's `depends-on(acme-core)` gate. + let skill_dir = tmp.path().join("acme-skill"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + indoc! {" + --- + name: acme-basics + description: Basic acme usage + depends-on: '*' + --- + + Use the derive macros. + "}, + ) + .unwrap(); + + // Plugin gated on a concrete crate; the group is a wildcard. + let plugin = Plugin { + name: "acme-plugin".to_string(), + predicates: pred_set("acme-core"), + hooks: vec![], + skills: vec![SkillGroup { + predicates: pred_set("*"), + source: PluginSource::Path(skill_dir.to_path_buf()), + workspace_member: false, + }], + mcp_servers: vec![], + installations: Vec::new(), + subcommands: BTreeMap::new(), + custom_predicates: vec![], + chained: vec![], + }; + + let registry = PluginRegistry { + plugins: vec![ParsedPlugin { + canonical: PackageId::new("test", &plugin.name, ANY_VERSION), + path: tmp.path().join("plugin.toml"), + plugin, + source_dir: tmp.path().to_path_buf(), + workspace_member: false, + }], + standalone_skills: vec![], + warnings: vec![], + custom_predicates: crate::plugins::CustomPredicateRegistry::default(), + }; + + let workspace_crates = vec![symposium_sdk::workspace::WorkspaceCrate::new( + "acme-core".to_string(), + semver::Version::new(1, 0, 0), + None, + )]; + + let applicable = resolve_applicable( + &sym, + ®istry, + &workspace_crates, + std::collections::HashMap::new(), + UpdateLevel::None, + ) + .await; + + // The plugin matched, attributed to the crate that satisfied its gate. + assert_eq!(applicable.plugins.len(), 1); + assert_eq!(applicable.plugins[0].name, "acme-plugin"); + assert_eq!(applicable.plugins[0].crates, vec!["acme-core".to_string()]); + + // The wildcard skill rode in on the plugin: its own gate names no + // dependency, so the crate union is exactly the plugin's `[acme-core]`. + assert_eq!(applicable.skills.len(), 1); + let skill = &applicable.skills[0]; + assert_eq!(skill.skill.name(), "acme-basics"); + assert_eq!(skill.plugin.as_deref(), Some("acme-plugin")); + assert_eq!(skill.crates, vec!["acme-core".to_string()]); + } + + #[test] + fn dedup_plugins_merges_by_name_unioning_crates() { + // Same plugin recorded twice (e.g. a crate chained from two owners), + // plus a distinct crate-less one. + let plugins = vec![ + PluginActivation { + name: "shared".into(), + crates: vec!["a".into()], + }, + PluginActivation { + name: "shared".into(), + crates: vec!["b".into(), "a".into()], + }, + PluginActivation { + name: "solo".into(), + crates: vec![], + }, + ]; + let merged = dedup_plugins(plugins); + assert_eq!(merged.len(), 2); + // Sorted by name; crates unioned and sorted. + assert_eq!(merged[0].name, "shared"); + assert_eq!(merged[0].crates, vec!["a".to_string(), "b".to_string()]); + assert_eq!(merged[1].name, "solo"); + assert!(merged[1].crates.is_empty()); + } + #[tokio::test] async fn predicate_failure_filters_skill() { use crate::plugins::{ParsedPlugin, Plugin, PluginRegistry, PluginSource, SkillGroup}; diff --git a/src/sync.rs b/src/sync.rs index 628fefd2..65d8f373 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -270,9 +270,28 @@ async fn resolve_custom_predicate_entries( entries } +/// Returned so the hook pipeline can emit telemetry from it; the standalone +/// `cargo agents sync` command discards it. +#[derive(Debug)] +pub struct SyncSummary { + pub plugins: Vec, + pub skills: Vec, + /// Distinct skills created or updated this pass, counted once per skill + /// regardless of agent count. A delta, so a steady-state sync reports 0; + /// count `skills` for how many apply. + pub installed: usize, + /// Stale skill directories removed, counted per directory. + pub reaped: usize, + pub crate_count: usize, +} + /// Run the full sync: discover applicable skills, install into agent dirs, /// clean up stale installations. -pub async fn sync(sym: &Symposium, deps: &mut WorkspaceDeps, update: UpdateLevel) -> Result<()> { +pub async fn sync( + sym: &Symposium, + deps: &mut WorkspaceDeps, + update: UpdateLevel, +) -> Result { let out = &Output::quiet(); let loaded = deps .load() @@ -305,7 +324,8 @@ pub async fn sync(sym: &Symposium, deps: &mut WorkspaceDeps, update: UpdateLevel // Find all applicable skills let applicable = - skills::skills_applicable_to(sym, ®istry, &workspace, custom_entries, update).await; + skills::resolve_applicable(sym, ®istry, &workspace, custom_entries, update).await; + let plugin_activations = applicable.plugins; // Dedup by `(skill_name, origin_hash)`: two crate origins with the same // (name, version, skill-path-within-crate) collapse (the same skill bytes @@ -319,10 +339,12 @@ pub async fn sync(sym: &Symposium, deps: &mut WorkspaceDeps, update: UpdateLevel let mut name_counts: std::collections::BTreeMap = std::collections::BTreeMap::new(); - for entry in &applicable { + let mut skill_activations: Vec = Vec::new(); + for entry in &applicable.skills { let name = entry.skill.name().to_string(); if seen.insert((name.clone(), entry.origin_hash.clone())) { *name_counts.entry(name.clone()).or_default() += 1; + skill_activations.push(entry.activation()); to_install.push((name, entry.origin_hash.clone(), &entry.skill.path)); } } @@ -363,12 +385,22 @@ pub async fn sync(sym: &Symposium, deps: &mut WorkspaceDeps, update: UpdateLevel message: "no agents configured, run `cargo agents init` to add one".into(), }, ); - return Ok(()); + return Ok(SyncSummary { + plugins: plugin_activations, + skills: skill_activations, + installed: 0, + reaped: 0, + crate_count: workspace.len(), + }); } // Track every skill directory we (re)install during this sync. Anything // we find later that has the marker file but isn't in this set is stale. let mut installed_dirs: BTreeSet = BTreeSet::new(); + // Separate from `installed_dirs`, which also holds unchanged dirs (so + // stale-cleanup keeps them) and is per-agent rather than per-skill. + let mut changed_skills: BTreeSet = BTreeSet::new(); + let mut reaped_count = 0usize; for agent_name in &agent_names { let agent = Agent::from_config_name(agent_name)?; @@ -444,6 +476,7 @@ pub async fn sync(sym: &Symposium, deps: &mut WorkspaceDeps, update: UpdateLevel match sync_skill_dir(source_dir, &dest_dir, &project_root, debounce) { Ok(true) => { installed_dirs.insert(dest_dir.clone()); + changed_skills.insert(dir_name.clone()); tracing::info!( report = %crate::report::ReportEvent::SkillInstalled { skill: dir_name.clone(), @@ -490,6 +523,7 @@ pub async fn sync(sym: &Symposium, deps: &mut WorkspaceDeps, update: UpdateLevel } match fs::remove_dir_all(&path) { Ok(()) => { + reaped_count += 1; tracing::info!( report = %crate::report::ReportEvent::SkillRemoved { path: display_path(&path), @@ -523,7 +557,13 @@ pub async fn sync(sym: &Symposium, deps: &mut WorkspaceDeps, update: UpdateLevel ); } - Ok(()) + Ok(SyncSummary { + plugins: plugin_activations, + skills: skill_activations, + installed: changed_skills.len(), + reaped: reaped_count, + crate_count: workspace.len(), + }) } /// Register global hooks for all configured agents. diff --git a/src/telemetry.rs b/src/telemetry.rs index 8bfe1fc3..0ba2eacd 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -26,6 +26,10 @@ pub const RETENTION_DAYS: i64 = 30; pub struct TelemetryEvent { /// When the event occurred (UTC). pub at: DateTime, + /// The session this event belongs to. Common to every kind, so it rides on + /// the envelope; `None` when the agent supplies no id (e.g. Copilot). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub session_id: Option, /// The kind-tagged payload. Flattened so a line reads /// `{"at": "...", "kind": "tool_use", ...}`. #[serde(flatten)] @@ -36,36 +40,71 @@ pub struct TelemetryEvent { /// /// Anonymous by construction: no prompt text, command lines, or file paths are /// recorded — only counts and coarse metadata. +/// #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "snake_case")] pub enum EventKind { /// An agent session began. SessionStart { - #[serde(default, skip_serializing_if = "Option::is_none")] - session_id: Option, agent: String, - /// Plugins applicable to the workspace at session start. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - plugins: Vec, - }, - /// The user submitted a prompt. - UserPrompt { + /// `None` when no sync ran (auto-sync off): we record the absence + /// rather than guess a count. #[serde(default, skip_serializing_if = "Option::is_none")] - session_id: Option, + crate_count: Option, }, + /// The user submitted a prompt. + UserPrompt, /// The agent invoked a tool (named, but with no arguments captured). - ToolUse { + ToolUse { tool: String }, + /// A plugin applied to the workspace. + PluginActivation { + plugin: String, + /// Only the crates that satisfied the plugin's predicates, so this is + /// empty for wildcard / env / shell gates. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + crates: Vec, + }, + /// A skill applied to the workspace. + SkillActivation { + skill: String, + /// `None` for a standalone SKILL.md that no plugin vends. #[serde(default, skip_serializing_if = "Option::is_none")] - session_id: Option, - tool: String, + plugin: Option, + /// Unioned across the plugin, group, and skill predicate levels; empty + /// for wildcard gates. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + crates: Vec, + }, + /// A plugin hook ran in response to an event. + HookInvocation { + hook: String, + plugin: String, + duration_ms: u64, + /// `None` when the hook was killed by a signal. `default` so lines + /// written before this field existed still deserialize. + #[serde(default)] + exit_code: Option, + }, + /// A sync pass over the workspace skills. + SyncRun { + installed: usize, + reaped: usize, + plugins_matched: usize, }, + /// The agent finished a turn (end of response). + Stop, + /// A kind this (older) reader does not recognize. `#[serde(other)]` keeps the + /// line instead of dropping it, but cannot retain the payload. + #[serde(other)] + Unknown, } impl TelemetryEvent { /// Build an event stamped at the current time. - pub fn now(kind: EventKind) -> Self { + pub fn now(session_id: Option, kind: EventKind) -> Self { Self { at: Utc::now(), + session_id, kind, } } @@ -74,8 +113,14 @@ impl TelemetryEvent { pub fn kind_name(&self) -> &'static str { match self.kind { EventKind::SessionStart { .. } => "session_start", - EventKind::UserPrompt { .. } => "user_prompt", + EventKind::UserPrompt => "user_prompt", EventKind::ToolUse { .. } => "tool_use", + EventKind::PluginActivation { .. } => "plugin_activation", + EventKind::SkillActivation { .. } => "skill_activation", + EventKind::HookInvocation { .. } => "hook_invocation", + EventKind::SyncRun { .. } => "sync_run", + EventKind::Stop => "stop", + EventKind::Unknown => "unknown", } } } @@ -91,37 +136,51 @@ fn file_for(dir: &Path, at: DateTime) -> PathBuf { /// Append one event to today's log file. Best-effort. pub fn record(config_dir: &Path, event: &TelemetryEvent) { + record_all(config_dir, std::slice::from_ref(event)); +} + +/// Append a batch of events, opening each day's file once rather than per event. +/// Best-effort. +/// +/// One `write_all` per event keeps a single line the unit of append, so +/// concurrent writers cannot interleave a partial line. +pub fn record_all(config_dir: &Path, events: &[TelemetryEvent]) { + if events.is_empty() { + return; + } let dir = telemetry_dir(config_dir); if let Err(e) = fs::create_dir_all(&dir) { tracing::debug!(error = %e, "telemetry: failed to create telemetry dir"); return; } - let line = match serde_json::to_string(event) { - Ok(mut s) => { - s.push('\n'); - s - } - Err(e) => { - tracing::debug!(error = %e, "telemetry: failed to serialize event"); - return; - } - }; - let path = file_for(&dir, event.at); - match OpenOptions::new().create(true).append(true).open(&path) { - Ok(mut f) => { - if let Err(e) = f.write_all(line.as_bytes()) { - tracing::debug!(error = %e, "telemetry: failed to append event"); + + // Events arrive in chronological order, so same-day events are contiguous: + // open each day's file once and append that day's lines to it. + for day in events.chunk_by(|prev, next| prev.at.date_naive() == next.at.date_naive()) { + let path = file_for(&dir, day[0].at); + let mut file = match OpenOptions::new().create(true).append(true).open(&path) { + Ok(file) => file, + Err(err) => { + tracing::debug!(error = %err, "telemetry: failed to open event file"); + continue; + } + }; + for event in day { + let mut line = match serde_json::to_string(event) { + Ok(line) => line, + Err(err) => { + tracing::debug!(error = %err, "telemetry: failed to serialize event"); + continue; + } + }; + line.push('\n'); + if let Err(err) = file.write_all(line.as_bytes()) { + tracing::debug!(error = %err, "telemetry: failed to append event"); } } - Err(e) => tracing::debug!(error = %e, "telemetry: failed to open event file"), } } -/// Convenience: stamp `kind` with the current time and record it. -pub fn record_kind(config_dir: &Path, kind: EventKind) { - record(config_dir, &TelemetryEvent::now(kind)); -} - /// Parse the date out of an `events-YYYY-MM-DD.jsonl` filename. fn file_date(path: &Path) -> Option { let stem = path.file_name()?.to_str()?; @@ -173,8 +232,11 @@ pub fn read_events(config_dir: &Path) -> Vec { if line.trim().is_empty() { continue; } - if let Ok(event) = serde_json::from_str::(line) { - events.push(event); + match serde_json::from_str::(line) { + Ok(event) => events.push(event), + Err(e) => { + tracing::debug!(error = %e, "telemetry: skipping unparseable event line") + } } } } @@ -276,42 +338,139 @@ mod tests { at: DateTime::parse_from_rfc3339("2026-06-23T10:00:00Z") .unwrap() .with_timezone(&Utc), + session_id: Some("s1".into()), kind: EventKind::ToolUse { - session_id: Some("s1".into()), tool: "Bash".into(), }, }; let line = serde_json::to_string(&event).unwrap(); assert!(line.contains(r#""kind":"tool_use""#), "line = {line}"); assert!(line.contains(r#""tool":"Bash""#)); + assert!(line.contains(r#""session_id":"s1""#), "line = {line}"); let back: TelemetryEvent = serde_json::from_str(&line).unwrap(); + assert_eq!(back.session_id.as_deref(), Some("s1")); assert_eq!(back.kind_name(), "tool_use"); } + #[test] + fn skill_activation_round_trips_with_witness_crates() { + let event = TelemetryEvent { + at: DateTime::parse_from_rfc3339("2026-07-09T10:00:00Z") + .unwrap() + .with_timezone(&Utc), + session_id: Some("s1".into()), + kind: EventKind::SkillActivation { + skill: "example-skill".into(), + plugin: Some("example-plugin".into()), + crates: vec!["acme-core".into(), "acme-io".into()], + }, + }; + let line = serde_json::to_string(&event).unwrap(); + assert!( + line.contains(r#""kind":"skill_activation""#), + "line = {line}" + ); + assert!(line.contains(r#""crates":["acme-core","acme-io"]"#)); + let back: TelemetryEvent = serde_json::from_str(&line).unwrap(); + assert_eq!(back.kind_name(), "skill_activation"); + } + + #[test] + fn every_kind_round_trips_with_matching_tag() { + let kinds = [ + EventKind::SessionStart { + agent: "claude".into(), + crate_count: Some(1), + }, + EventKind::SessionStart { + agent: "copilot".into(), + crate_count: None, + }, + EventKind::UserPrompt, + EventKind::ToolUse { + tool: "Bash".into(), + }, + EventKind::PluginActivation { + plugin: "example-plugin".into(), + crates: vec!["acme-core".into()], + }, + EventKind::SkillActivation { + skill: "example-skill".into(), + plugin: None, + crates: vec![], + }, + EventKind::HookInvocation { + hook: "format-check".into(), + plugin: "example-plugin".into(), + duration_ms: 5, + exit_code: Some(0), + }, + EventKind::SyncRun { + installed: 1, + reaped: 0, + plugins_matched: 2, + }, + EventKind::Stop, + ]; + for kind in kinds { + let event = TelemetryEvent::now(Some("s1".into()), kind); + let value = serde_json::to_value(&event).unwrap(); + assert_eq!( + value["kind"].as_str(), + Some(event.kind_name()), + "kind_name() drifted from the serde tag: {value}" + ); + let line = serde_json::to_string(&event).unwrap(); + let back: TelemetryEvent = serde_json::from_str(&line).unwrap(); + assert_eq!(back.kind_name(), event.kind_name(), "line = {line}"); + assert_eq!(back.session_id.as_deref(), Some("s1"), "line = {line}"); + } + } + + #[test] + fn empty_witness_crates_are_omitted() { + // A wildcard skill (empty witness) should serialize without a `crates` + // key at all, not as `"crates":[]`. + let event = TelemetryEvent::now( + None, + EventKind::SkillActivation { + skill: "wildcard-skill".into(), + plugin: None, + crates: vec![], + }, + ); + let line = serde_json::to_string(&event).unwrap(); + assert!(!line.contains("crates"), "line = {line}"); + assert!(!line.contains("plugin"), "line = {line}"); + assert!(!line.contains("session_id"), "line = {line}"); + } + #[test] fn records_append_and_read_back() { let tmp = tempfile::tempdir().unwrap(); let dir = tmp.path(); - record_kind( + record( dir, - EventKind::SessionStart { - session_id: Some("s1".into()), - agent: "claude".into(), - plugins: vec!["tokio-plugin".into()], - }, + &TelemetryEvent::now( + Some("s1".into()), + EventKind::SessionStart { + agent: "claude".into(), + crate_count: Some(3), + }, + ), ); - record_kind( + record( dir, - EventKind::UserPrompt { - session_id: Some("s1".into()), - }, + &TelemetryEvent::now(Some("s1".into()), EventKind::UserPrompt), ); - record_kind( + record( dir, - EventKind::ToolUse { - session_id: Some("s1".into()), - tool: "Bash".into(), - }, + &TelemetryEvent::now( + Some("s1".into()), + EventKind::ToolUse { + tool: "Bash".into(), + }, + ), ); let events = read_events(dir); @@ -325,6 +484,66 @@ mod tests { assert!(u.bytes > 0); } + #[test] + fn record_all_appends_batch_to_one_file() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + let events = vec![ + TelemetryEvent::now( + Some("s1".into()), + EventKind::SessionStart { + agent: "claude".into(), + crate_count: Some(1), + }, + ), + TelemetryEvent::now(Some("s1".into()), EventKind::UserPrompt), + TelemetryEvent::now(Some("s1".into()), EventKind::Stop), + ]; + record_all(dir, &events); + + let back = read_events(dir); + assert_eq!(back.len(), 3); + assert_eq!(back[0].kind_name(), "session_start"); + assert_eq!(back[2].kind_name(), "stop"); + assert_eq!(usage(dir).files, 1); + } + + #[test] + fn read_events_skips_unparseable_lines() { + let tmp = tempfile::tempdir().unwrap(); + let dir = telemetry_dir(tmp.path()); + fs::create_dir_all(&dir).unwrap(); + let file = file_for(&dir, Utc::now()); + // A good line, a corrupt one, and a blank one: only the good line reads. + fs::write( + &file, + "{\"at\":\"2026-07-09T10:00:00Z\",\"kind\":\"user_prompt\"}\nnot json\n\n", + ) + .unwrap(); + + let events = read_events(tmp.path()); + assert_eq!(events.len(), 1); + assert_eq!(events[0].kind_name(), "user_prompt"); + } + + #[test] + fn unknown_kind_is_preserved_not_dropped() { + // A kind from a newer writer must survive an older reader, not vanish. + let tmp = tempfile::tempdir().unwrap(); + let dir = telemetry_dir(tmp.path()); + fs::create_dir_all(&dir).unwrap(); + let file = file_for(&dir, Utc::now()); + fs::write( + &file, + "{\"at\":\"2026-07-09T10:00:00Z\",\"kind\":\"from_the_future\",\"rating\":5}\n", + ) + .unwrap(); + + let events = read_events(tmp.path()); + assert_eq!(events.len(), 1); + assert_eq!(events[0].kind_name(), "unknown"); + } + #[test] fn roll_off_removes_old_files_only() { let tmp = tempfile::tempdir().unwrap(); @@ -350,7 +569,7 @@ mod tests { assert!(empty.contains("Telemetry: disabled")); assert!(empty.contains("nothing yet")); - record_kind(dir, EventKind::UserPrompt { session_id: None }); + record(dir, &TelemetryEvent::now(None, EventKind::UserPrompt)); let filled = status_text(dir, true); assert!(filled.contains("Telemetry: enabled")); assert!(filled.contains("1 event(s)")); diff --git a/tests/fixtures/telemetry-disabled/dot-symposium/config.toml b/tests/fixtures/telemetry-disabled/dot-symposium/config.toml new file mode 100644 index 00000000..91768b15 --- /dev/null +++ b/tests/fixtures/telemetry-disabled/dot-symposium/config.toml @@ -0,0 +1,7 @@ +hook-scope = "project" +# Keep the run offline and deterministic: no crates.io update check on SessionStart. +auto-update = "off" + +[defaults] +symposium-recommendations = false +user-plugins = true diff --git a/tests/fixtures/telemetry-disabled/dot-symposium/plugins/skills-plugin/SYMPOSIUM.toml b/tests/fixtures/telemetry-disabled/dot-symposium/plugins/skills-plugin/SYMPOSIUM.toml new file mode 100644 index 00000000..e483328b --- /dev/null +++ b/tests/fixtures/telemetry-disabled/dot-symposium/plugins/skills-plugin/SYMPOSIUM.toml @@ -0,0 +1,5 @@ +name = "skills-plugin" +depends-on = ["*"] + +[[skills]] +source.path = "." diff --git a/tests/fixtures/telemetry-disabled/dot-symposium/plugins/skills-plugin/example-skill/SKILL.md b/tests/fixtures/telemetry-disabled/dot-symposium/plugins/skills-plugin/example-skill/SKILL.md new file mode 100644 index 00000000..5e19c254 --- /dev/null +++ b/tests/fixtures/telemetry-disabled/dot-symposium/plugins/skills-plugin/example-skill/SKILL.md @@ -0,0 +1,6 @@ +--- +name: example-skill +description: Guidance that applies to all workspaces +--- + +This skill applies to every workspace. diff --git a/tests/fixtures/telemetry-emit/dot-symposium/config.toml b/tests/fixtures/telemetry-emit/dot-symposium/config.toml new file mode 100644 index 00000000..5cdd6449 --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/config.toml @@ -0,0 +1,10 @@ +hook-scope = "project" +# Keep the run offline and deterministic: no crates.io update check on SessionStart. +auto-update = "off" + +[telemetry] +enabled = true + +[defaults] +symposium-recommendations = false +user-plugins = true diff --git a/tests/fixtures/telemetry-emit/dot-symposium/plugins/crate-gated-plugin/SYMPOSIUM.toml b/tests/fixtures/telemetry-emit/dot-symposium/plugins/crate-gated-plugin/SYMPOSIUM.toml new file mode 100644 index 00000000..a36cdca5 --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/plugins/crate-gated-plugin/SYMPOSIUM.toml @@ -0,0 +1,5 @@ +name = "crate-gated-plugin" +depends-on = ["tokio"] + +[[skills]] +source.path = "." diff --git a/tests/fixtures/telemetry-emit/dot-symposium/plugins/crate-gated-plugin/crate-gated-skill/SKILL.md b/tests/fixtures/telemetry-emit/dot-symposium/plugins/crate-gated-plugin/crate-gated-skill/SKILL.md new file mode 100644 index 00000000..8ba2f22d --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/plugins/crate-gated-plugin/crate-gated-skill/SKILL.md @@ -0,0 +1,6 @@ +--- +name: crate-gated-skill +description: Guidance for workspaces that depend on a specific crate +--- + +This skill applies only when the gating crate is a dependency. diff --git a/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/SYMPOSIUM.toml b/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/SYMPOSIUM.toml new file mode 100644 index 00000000..0ff0379e --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/SYMPOSIUM.toml @@ -0,0 +1,17 @@ +name = "hooks-plugin" +depends-on = ["*"] + +[[hooks]] +name = "example-hook" +event = "PreToolUse" +matcher = "Bash" +format = "claude" +command = { script = "$TEST_DIR/dot-symposium/plugins/hooks-plugin/scripts/probe.sh" } + +# Exit 2 blocks the tool. Matches "Grep" so only the block test triggers it. +[[hooks]] +name = "blocking-hook" +event = "PreToolUse" +matcher = "Grep" +format = "claude" +command = { script = "$TEST_DIR/dot-symposium/plugins/hooks-plugin/scripts/block.sh" } diff --git a/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/scripts/block.sh b/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/scripts/block.sh new file mode 100644 index 00000000..a94258af --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/scripts/block.sh @@ -0,0 +1,3 @@ +#!/bin/sh +echo "denied by policy" >&2 +exit 2 diff --git a/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/scripts/probe.sh b/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/scripts/probe.sh new file mode 100644 index 00000000..49d29fb4 --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/plugins/hooks-plugin/scripts/probe.sh @@ -0,0 +1,2 @@ +#!/bin/sh +echo '{"PreToolUse":{"additionalContext":"example-hook-fired"}}' diff --git a/tests/fixtures/telemetry-emit/dot-symposium/plugins/skills-plugin/SYMPOSIUM.toml b/tests/fixtures/telemetry-emit/dot-symposium/plugins/skills-plugin/SYMPOSIUM.toml new file mode 100644 index 00000000..e483328b --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/plugins/skills-plugin/SYMPOSIUM.toml @@ -0,0 +1,5 @@ +name = "skills-plugin" +depends-on = ["*"] + +[[skills]] +source.path = "." diff --git a/tests/fixtures/telemetry-emit/dot-symposium/plugins/skills-plugin/example-skill/SKILL.md b/tests/fixtures/telemetry-emit/dot-symposium/plugins/skills-plugin/example-skill/SKILL.md new file mode 100644 index 00000000..5e19c254 --- /dev/null +++ b/tests/fixtures/telemetry-emit/dot-symposium/plugins/skills-plugin/example-skill/SKILL.md @@ -0,0 +1,6 @@ +--- +name: example-skill +description: Guidance that applies to all workspaces +--- + +This skill applies to every workspace. diff --git a/tests/fixtures/telemetry/expected-events.jsonl b/tests/fixtures/telemetry/expected-events.jsonl new file mode 100644 index 00000000..1ec3d2f9 --- /dev/null +++ b/tests/fixtures/telemetry/expected-events.jsonl @@ -0,0 +1,8 @@ +{"at":"2026-07-15T10:00:00Z","session_id":"test-session-id","kind":"session_start","agent":"claude","crate_count":2} +{"at":"2026-07-15T10:00:01Z","session_id":"test-session-id","kind":"sync_run","installed":1,"reaped":0,"plugins_matched":1} +{"at":"2026-07-15T10:00:02Z","session_id":"test-session-id","kind":"plugin_activation","plugin":"skills-plugin","crates":["acme-core"]} +{"at":"2026-07-15T10:00:03Z","session_id":"test-session-id","kind":"skill_activation","skill":"example-skill","plugin":"skills-plugin","crates":["acme-core"]} +{"at":"2026-07-15T10:00:04Z","session_id":"test-session-id","kind":"user_prompt"} +{"at":"2026-07-15T10:00:05Z","session_id":"test-session-id","kind":"tool_use","tool":"Bash"} +{"at":"2026-07-15T10:00:06Z","session_id":"test-session-id","kind":"hook_invocation","hook":"example-hook","plugin":"hooks-plugin","duration_ms":4,"exit_code":0} +{"at":"2026-07-15T10:00:07Z","session_id":"test-session-id","kind":"stop"} diff --git a/tests/telemetry_emit.rs b/tests/telemetry_emit.rs new file mode 100644 index 00000000..95b9929f --- /dev/null +++ b/tests/telemetry_emit.rs @@ -0,0 +1,411 @@ +//! End-to-end telemetry emission: drives the real hook pipeline and asserts the +//! events land on disk, gated by the opt-in `[telemetry] enabled` flag. +//! +//! Only the sync-snapshot test overlays `workspace0`: `sync` needs a real Cargo +//! workspace (it errors "not in a Rust workspace" otherwise), and resolving its +//! deps costs a `cargo metadata`, so the other tests skip it. +//! +//! `stop` cannot be driven here: the harness `HookStep` has no `Stop` variant. + +use chrono::{DateTime, Utc}; +use serde_json::json; +use symposium::hook_schema::HookAgent; +use symposium::telemetry::{self, EventKind, TelemetryEvent}; +use symposium_testlib::{HookStep, TestMode, with_fixture}; + +/// Across one session's wire events, the funnel is recorded once each and a tool +/// is counted exactly once — on `PostToolUse`, since `PreToolUse` is silent. +#[tokio::test(flavor = "multi_thread")] +async fn wire_funnel_records_each_event_once() { + with_fixture( + TestMode::SimulationOnly, + &["telemetry-emit"], + async |mut ctx| { + ctx.prompt_or_hook( + "ignored", + &[ + HookStep::SessionStart, + HookStep::UserPromptSubmit { + prompt: "hello".to_string(), + }, + // `Read` matches no hook, so nothing spawns `sh` (Windows). + HookStep::PreToolUse { + tool_name: "Read".to_string(), + tool_input: json!({"file_path": "/tmp/x"}), + }, + HookStep::PostToolUse { + tool_name: "Read".to_string(), + tool_input: json!({"file_path": "/tmp/x"}), + tool_response: json!({"ok": true}), + }, + ], + HookAgent::Claude, + ) + .await?; + + let events = telemetry::read_events(ctx.sym.config_dir()); + let kinds: Vec<&str> = events.iter().map(|event| event.kind_name()).collect(); + + assert!(kinds.contains(&"session_start"), "kinds = {kinds:?}"); + assert!(kinds.contains(&"user_prompt"), "kinds = {kinds:?}"); + + let tools: Vec<&str> = events + .iter() + .filter_map(|event| match &event.kind { + EventKind::ToolUse { tool } => Some(tool.as_str()), + _ => None, + }) + .collect(); + assert_eq!( + tools, + ["Read"], + "tool counted once on PostToolUse (PreToolUse silent), kinds = {kinds:?}", + ); + assert!( + !kinds.contains(&"hook_invocation"), + "the Read steps match no hook, kinds = {kinds:?}", + ); + assert!( + events + .iter() + .all(|event| event.session_id.as_deref() == Some("test-session-id")), + "every event carries the wire session id", + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + +/// The `SessionStart` sync snapshot: `sync_run` plus the plugin / skill +/// activations, for both a wildcard gate (no witness crates) and a crate gate. +#[tokio::test(flavor = "multi_thread")] +async fn session_start_records_sync_snapshot() { + with_fixture( + TestMode::SimulationOnly, + &["telemetry-emit", "workspace0"], + async |mut ctx| { + ctx.prompt_or_hook("ignored", &[HookStep::SessionStart], HookAgent::Claude) + .await?; + + let events = telemetry::read_events(ctx.sym.config_dir()); + let kinds: Vec<&str> = events.iter().map(|event| event.kind_name()).collect(); + + let (agent, crate_count) = events + .iter() + .find_map(|event| match &event.kind { + EventKind::SessionStart { agent, crate_count } => { + Some((agent.as_str(), *crate_count)) + } + _ => None, + }) + .unwrap_or_else(|| panic!("no session_start, kinds = {kinds:?}")); + + assert_eq!(agent, "claude"); + assert!( + crate_count.is_some_and(|count| count >= 1), + "crate_count should reflect the fixture workspace, got {crate_count:?}", + ); + assert!(kinds.contains(&"sync_run"), "kinds = {kinds:?}"); + + assert!( + events.iter().any(|event| matches!( + &event.kind, + EventKind::PluginActivation { plugin, crates } + if plugin == "skills-plugin" && crates.is_empty() + )), + "expected wildcard skills-plugin activation, kinds = {kinds:?}", + ); + assert!( + events.iter().any(|event| matches!( + &event.kind, + EventKind::SkillActivation { skill, crates, .. } + if skill == "example-skill" && crates.is_empty() + )), + "expected wildcard example-skill activation, kinds = {kinds:?}", + ); + + assert!( + events.iter().any(|event| matches!( + &event.kind, + EventKind::PluginActivation { plugin, crates } + if plugin == "crate-gated-plugin" && crates.iter().any(|crate_name| crate_name == "tokio") + )), + "expected crate-gated-plugin activation with tokio witness, kinds = {kinds:?}", + ); + assert!( + events.iter().any(|event| matches!( + &event.kind, + EventKind::SkillActivation { skill, crates, .. } + if skill == "crate-gated-skill" && crates.iter().any(|crate_name| crate_name == "tokio") + )), + "expected crate-gated-skill activation with tokio witness, kinds = {kinds:?}", + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + +/// A plugin hook that runs is timed and recorded as a `hook_invocation`. +/// +/// Unix-only: a script hook runs as `sh `, and MSYS `sh` on Windows cannot +/// execute a Windows absolute path, so nothing spawns. CI is Linux + macOS. +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread")] +async fn plugin_hook_records_hook_invocation() { + with_fixture( + TestMode::SimulationOnly, + &["telemetry-emit"], + async |mut ctx| { + ctx.prompt_or_hook( + "ignored", + &[HookStep::PreToolUse { + tool_name: "Bash".to_string(), + tool_input: json!({"command": "ls"}), + }], + HookAgent::Claude, + ) + .await?; + + let events = telemetry::read_events(ctx.sym.config_dir()); + assert!( + events.iter().any(|event| matches!( + &event.kind, + EventKind::HookInvocation { plugin, .. } if plugin == "hooks-plugin" + )), + "expected hooks-plugin invocation, kinds = {:?}", + events + .iter() + .map(|event| event.kind_name()) + .collect::>(), + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + +/// A hook that blocks the tool (exit 2) is still recorded. The block must not be +/// a telemetry hole, though it still propagates as an error. +/// +/// Unix-only for the same reason as `plugin_hook_records_hook_invocation`. +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread")] +async fn blocked_hook_is_still_recorded() { + with_fixture( + TestMode::SimulationOnly, + &["telemetry-emit"], + async |mut ctx| { + let result = ctx + .prompt_or_hook( + "ignored", + &[HookStep::PreToolUse { + tool_name: "Grep".to_string(), + tool_input: json!({"pattern": "x"}), + }], + HookAgent::Claude, + ) + .await; + assert!(result.is_err(), "exit 2 must propagate as a block"); + + let events = telemetry::read_events(ctx.sym.config_dir()); + assert!( + events.iter().any(|event| matches!( + &event.kind, + EventKind::HookInvocation { plugin, exit_code, .. } + if plugin == "hooks-plugin" && *exit_code == Some(2) + )), + "blocked hook should be recorded with exit_code 2, kinds = {:?}", + events + .iter() + .map(|event| event.kind_name()) + .collect::>(), + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + +/// With telemetry disabled (the default), a `SessionStart` pass that would +/// otherwise emit `session_start` + `sync_run` + activations writes nothing. +#[tokio::test(flavor = "multi_thread")] +async fn disabled_gate_records_nothing() { + with_fixture( + TestMode::SimulationOnly, + &["telemetry-disabled"], + async |mut ctx| { + ctx.prompt_or_hook("ignored", &[HookStep::SessionStart], HookAgent::Claude) + .await?; + + let events = telemetry::read_events(ctx.sym.config_dir()); + assert!( + events.is_empty(), + "gate off must write nothing, got: {:?}", + events + .iter() + .map(|event| event.kind_name()) + .collect::>(), + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + +/// `SessionStart` is the only path that runs `roll_off`, so this guards against +/// retention silently never running. +#[tokio::test(flavor = "multi_thread")] +async fn session_start_rolls_off_stale_files() { + with_fixture( + TestMode::SimulationOnly, + &["telemetry-emit"], + async |mut ctx| { + let dir = telemetry::telemetry_dir(ctx.sym.config_dir()); + std::fs::create_dir_all(&dir).unwrap(); + let stale = dir.join("events-2000-01-01.jsonl"); + std::fs::write(&stale, "{}\n").unwrap(); + + ctx.prompt_or_hook("ignored", &[HookStep::SessionStart], HookAgent::Claude) + .await?; + + assert!( + !stale.exists(), + "SessionStart should roll off files past the retention window", + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + +/// Retention is disk hygiene, not collection: data recorded before a user opted +/// out must still age out, so `roll_off` runs even with telemetry disabled. +#[tokio::test(flavor = "multi_thread")] +async fn rolls_off_stale_files_even_when_disabled() { + with_fixture( + TestMode::SimulationOnly, + &["telemetry-disabled"], + async |mut ctx| { + let dir = telemetry::telemetry_dir(ctx.sym.config_dir()); + std::fs::create_dir_all(&dir).unwrap(); + let stale = dir.join("events-2000-01-01.jsonl"); + std::fs::write(&stale, "{}\n").unwrap(); + + ctx.prompt_or_hook("ignored", &[HookStep::SessionStart], HookAgent::Claude) + .await?; + + assert!( + !stale.exists(), + "opting out must not strand old data forever" + ); + Ok(()) + }, + ) + .await + .unwrap(); +} + +/// One event of every kind through the real `record` path, locked against a +/// committed golden sample. This is what covers `stop` and `hook_invocation`, +/// which the pipeline tests cannot drive portably. +/// +/// Compared as parsed JSON so serde key ordering cannot make it flaky. +#[test] +fn synthetic_events_match_golden_jsonl() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + + for event in synthetic_session() { + telemetry::record(dir, &event); + } + + // Every synthetic event shares one UTC day, so they land in one file. + let file = telemetry::telemetry_dir(dir).join("events-2026-07-15.jsonl"); + let got = std::fs::read_to_string(&file).expect("event file written"); + let golden = include_str!("fixtures/telemetry/expected-events.jsonl"); + + assert_eq!( + parse_jsonl(&got), + parse_jsonl(golden), + "on-disk JSONL drifted from golden. produced:\n{got}", + ); +} + +/// One `TelemetryEvent` of every kind, stamped one second apart on 2026-07-15. +fn synthetic_session() -> Vec { + let sid = || Some("test-session-id".to_string()); + let at = |secs: u32| -> DateTime { + DateTime::parse_from_rfc3339(&format!("2026-07-15T10:00:0{secs}Z")) + .unwrap() + .with_timezone(&Utc) + }; + let ev = |secs: u32, kind: EventKind| TelemetryEvent { + at: at(secs), + session_id: sid(), + kind, + }; + vec![ + ev( + 0, + EventKind::SessionStart { + agent: "claude".into(), + crate_count: Some(2), + }, + ), + ev( + 1, + EventKind::SyncRun { + installed: 1, + reaped: 0, + plugins_matched: 1, + }, + ), + ev( + 2, + EventKind::PluginActivation { + plugin: "skills-plugin".into(), + crates: vec!["acme-core".into()], + }, + ), + ev( + 3, + EventKind::SkillActivation { + skill: "example-skill".into(), + plugin: Some("skills-plugin".into()), + crates: vec!["acme-core".into()], + }, + ), + ev(4, EventKind::UserPrompt), + ev( + 5, + EventKind::ToolUse { + tool: "Bash".into(), + }, + ), + ev( + 6, + EventKind::HookInvocation { + hook: "example-hook".into(), + plugin: "hooks-plugin".into(), + duration_ms: 4, + exit_code: Some(0), + }, + ), + ev(7, EventKind::Stop), + ] +} + +fn parse_jsonl(s: &str) -> Vec { + s.lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| serde_json::from_str(line).expect("valid JSON line")) + .collect() +}