From 9863c7a5f3355c04a3447232f56098c382855d83 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 14:34:26 -0600 Subject: [PATCH 01/18] docs(plans): add macros-as-custom-commands design (gate-approved) Design doc for macros as first-class custom commands: top-level .name invocation, description/isolated fields, enabled_macros scoping, workspace-local macros, completion integration. Gatekeeper: sealed (3 findings fixed). Oracle plan-review: approved. --- plans/custom-commands-design.md | 505 ++++++++++++++++++++++++++++++++ 1 file changed, 505 insertions(+) create mode 100644 plans/custom-commands-design.md diff --git a/plans/custom-commands-design.md b/plans/custom-commands-design.md new file mode 100644 index 0000000..c5a8054 --- /dev/null +++ b/plans/custom-commands-design.md @@ -0,0 +1,505 @@ +# Design: Macros as First-Class Custom Commands + +Status: DRAFT v2 — grounded in code (src/config/macros.rs, src/repl/mod.rs, +src/config/install_remote.rs, src/config/paths.rs). Supersedes v1, which +proposed a new markdown "commands" artifact before discovering macros already +cover ~80% of the feature. + +## 0. Decision record + +- **Naming: keep "macros"** (user decision). No rename, no new artifact type, + no new install filter — `--install-from --filter macros` already + exists in both the CLI enum (`InstallFilter`, config/mod.rs:403) and the + REPL parser (`install_remote_from_repl_args`). (Note: the flag may be + renamed to `--install` per plans/bundle-manifest-design.md §5; the filter + is unaffected.) Docs will state plainly: *macros are + coyote's custom commands* (README + config examples + `.help`). + "Macro" is also the more accurate term: these are replayable scripts of + REPL commands with variables, not just prompt templates. +- **Keep the `.macro` subcommand** alongside top-level invocation (user + decision): it hosts the interactive creator (which top-level must never + trigger), it is the escape hatch for macros shadowed by built-ins (incl. + future built-ins landing on existing macro names), and removing it breaks + muscle memory and macros whose steps invoke `.macro`. Cost of keeping: ~0. +- **Rejected**: separate markdown command files (redundant — a single-step + macro with one `rest` variable IS a prompt command, and macros additionally + do multi-step, role switching, and `` .file `cmd` `` shell embedding); + rename to "commands" (churn, dir migration, and the less accurate word); + an `alias` field (complexity without payoff — the file name is the command + name, matching the existing naming structure); command-side agent/role + binding (inverts contexts-curate-artifacts; lets installed bundles mutate + existing contexts). +- **Bundle manifest / provenance tracking is NOT in this plan.** It is an + orthogonal, separate design (see §10); this feature neither depends on nor + blocks it. + +## 1. What exists today (verified) + +- `Macro { variables: Vec, steps: + Vec }`, YAML at `macros_dir()/.yaml` (global only; no + workspace dir, unlike skills). +- `.macro [args]` executes; nonexistent name + no args opens the + interactive macro creator (`ctx.new_macro`). +- `macro_execute` forks a fresh `RequestContext` from the current role + (inherits role's model/temperature/enabled_tools/enabled_mcp_servers), + copies `last_message` in (discontinuous), sets `macro_flag`, runs each + `{{var}}`-interpolated step via `run_repl_command`. **No state flows back**: + the macro's exchanges are not recorded in the active session and do not + update the caller's `last_message`. +- Auto-generated usage strings; positional vars with defaults + rest-capture. +- `.list macros`, `.delete macro`, built-ins embedded from `assets/macros/`. +- Unknown dot-command → `unknown_command()` at repl/mod.rs:1521: + `Error: Unknown command. Type ".help" for additional help.` + +## 2. Gaps this design closes + +1. Top-level invocation: `.review-work args`, not just `.macro review-work args`. +2. Discoverability: `description` field → listings, completion, `.help`. +3. Context scoping: `enabled_macros` on role / agent (non-graph) / session / + global config, mirroring `enabled_skills`. +4. Runtime toggles: `.macro enable|disable `, shorthand for + `.set enabled_macros` (see §6). +5. Conversation-integrated macros: `isolated: false`. +6. Workspace-local macros: `.coyote/macros/` shadowing global, mirroring + workspace skills/MCP conventions (see §5). + +## 3. New Macro fields + +```yaml +description: Review WIP against a base branch # optional; shown in listings/completion/.help +isolated: false # optional; default TRUE (current behavior) +variables: + - name: base + default: main + - name: instructions + rest: true + default: "" +steps: + - "Review the diff against {{base}}. {{instructions}}" +``` + +Both new fields are `#[serde(default)]`-style optional → every existing +macro file remains valid; default `isolated: true` preserves current behavior +exactly. + +### `isolated` semantics + +- `true` (default, today's behavior): forked context as described in §1. + Right for utility macros (e.g. generate-commit-message) whose chatter + should not pollute the session. +- `false`: steps run via `run_repl_command(ctx, ...)` against the **live** + context — exactly as if the user had typed each step at the prompt + themselves. Prompts continue the actual conversation: recorded in the + active session, `last_message` updated, agent context preserved. + Consequence to document loudly: mutating steps (`.role x`, `.model y`) + **persist after the macro ends** — that is the meaning of non-isolation, + not a bug. +- **VERIFIED: the session-recording chain has no macro_flag check anywhere** + (`after_chat_completion` request_context.rs:1528-1544 → `save_message` + :1464-1474 → `Session::add_message` session.rs:724-763; disk save deferred + to `Session::exit` :626-631). Today's "macros aren't recorded" behavior + comes ENTIRELY from the fork (`session: None` in the fresh context), not + from the flag — so non-isolated execution records normally with zero + changes to the recording path. +- Footnotes to today's isolation (documented, not changed): (a) the forked + ctx still appends exchanges to the flat `messages.md` file via the + `save_message` fallthrough (request_context.rs:1518-1526) — only *session* + recording is skipped; (b) Arc-backed state copied into the fork + (supervisor, inbox, escalation_queue) is shared, so mutations through those + handles already reach the parent. +- Both modes: `macro_flag` is set for the duration (temporarily on the live + ctx for non-isolated, restored on exit **including the error path** — RAII + guard) so the existing forbidden-op guards (repl/mod.rs:890, 979, 1310) + apply, and nested `.macro`/top-level macro invocation inside a macro is + rejected in non-isolated mode (isolated mode already recurses safely via + `#[async_recursion]`; keep as-is). + Precise predicate (oracle note): reject when `macro_flag` is set AND the + CURRENT execution mode is non-isolated. An **isolated** macro's step + invoking a **non-isolated** macro runs it inline on the FORKED ctx + (harmless — the fork has no session); pin this behavior in the step-5 + test matrix. + Implementation footguns (oracle): (a) the RAII guard cannot hold + `&mut ctx.macro_flag` while `&mut ctx` is passed to `run_repl_command` — + wrap the whole `&mut RequestContext` (Drop restores prior mode) or use + save/restore around a closure; (b) prefer a COMPANION FIELD for the mode + over changing `macro_flag` to `Option` (the enum touches all 13 + sites incl. both fork-propagation sites; the companion field is the + smaller diff); (c) a separate `macro_execute_inline` fn creates a new + type-level recursion cycle with `run_repl_command` and needs its own + `#[async_recursion]`/boxing — a branch inside the already-boxed + `macro_execute` is free. + Semantics caveat to document (§8): steps are FAIL-FAST — a mid-macro error + aborts remaining steps while completed steps' mutations persist (slightly + stronger than "as if typed", where a human would continue past errors); + and a `.exit` step's exit signal is swallowed inside macros today (bool + discarded at macros.rs:67) — unchanged, but say so. +- **VERIFIED guard inventory** (13 `macro_flag` sites: 4 decl/init, 2 + propagation — `fork_for_branch` request_context.rs:276, `new_for_child` + :315 — and 7 behavioral). Under non-isolated execution: + - Keep as-is (desirable in a macro even on the live ctx): `.update` bail + (repl/mod.rs:890), `.edit` bail (:979 — no `$EDITOR` mid-macro), + blank-line suppression (:1310, cosmetic), `new_role` prompt→bail + (request_context.rs:2259), `new_macro` prompt→bail (:2360). + - Non-issue: `apply_prelude` skip (:3966) — on a live REPL ctx the prelude + already ran and `state()` is non-empty. + - **RULED (user, 2026-08-20): condition on isolation.** `use_agent` + (:3743-3749) suppresses the agent's default `agent_session` when + `macro_flag` is set; under `isolated: false` the suppression is LIFTED — + a non-isolated `.agent foo` step inherits foo's default session exactly + as if typed. Mechanism: the RAII guard records the mode (companion field + or `Option` replacing the bare bool) so use_agent can + distinguish isolated from non-isolated; isolated mode keeps today's + suppression verbatim. + - Nested-macro *execution* is currently unguarded (`new_macro` only blocks + the interactive creator) — the non-isolated nesting rejection is NEW + code, not a reuse of an existing check. + +## 4. Top-level invocation + +- Dispatch order in the REPL command match: **built-ins first**, then — where + `unknown_command()` fires today — look up visible macros by name (file + stem). Hit → execute exactly as `.macro ` would (honoring + `isolated`). Miss → existing `unknown_command()` error, verbatim, unchanged. + **VERIFIED insertion point**: the top-level catch-all `_ => + unknown_command()?` at repl/mod.rs:1297. The other `unknown_command()` + call sites (:630, :740, :763, :1236, :1259) are sub-argument mismatches + inside known commands and must NOT dispatch macros. +- `.macro` subcommand is kept verbatim for back-compat, including the + interactive-creation flow (creation stays ONLY under `.macro`; a top-level + typo like `.hi` must error, never open the creator). +- Both entry points (`.name` and `.macro name`) go through the same §5 + visibility check — otherwise `.macro` trivially bypasses `enabled_macros`. +- Collision rules: + - Macro name colliding with a **built-in** command: built-in always + wins; macro still invokable via `.macro `; flagged + `shadowed (built-in)` in `.list macros`. Built-in list sourced from the + existing `ReplCommand` registry, not a hardcoded copy. **VERIFIED shape**: + `static REPL_COMMANDS: LazyLock<[ReplCommand; 60]>` (repl/mod.rs:56-330), + `ReplCommand { name: &'static str, description, state: AssertState }` + (:554-571), with a test hardcoding the count (:1732). Macros must NOT be + added to this array (static, `&'static str`, fixed count). +- Tab completion: on `.`, visible macros appear alongside the usual + built-ins, with their `description` shown when available — joining the + built-in completer — as a SEPARATE dynamic source queried at + completion time: the completer clones REPL_COMMANDS at construction + (completer.rs:93-98) but already holds `Arc>` + (completer.rs:84), so it can query visible macros live. + **Shadowed macros are excluded from completion entirely** (RULED): a macro + whose name collides with a built-in is neither dispatchable via `.` + (built-in always wins) nor listed in `.` completions — it surfaces + only in `.list macros` as `shadowed (built-in)` and stays invokable via + `.macro `. +- **`.macro ` argument completion (RULED)**: upgraded to match `.` + presentation — macro names WITH descriptions when available. VERIFIED + today: request_context.rs:3004 completes `.macro` args via + `map_completion_values(paths::list_macros())` (names only, no + descriptions), while the plumbing already supports described suggestions + (`repl_complete` returns `(String, Option)`; `.model`/`.agent` + arms use it, rendered at completer.rs:58-60). Change the `.macro` arm to + the same resolver source as top-level completion. Differences from + `.`: shadowed macros ARE listed here (`.macro` is their escape + hatch), and the `enable`/`disable` subcommands appear alongside macro + names. Second-arg completion: `.macro enable ` / `.macro disable + ` complete toggle-eligible macro names. +- Completion entries for macros carry the same `AssertState` stance as the + `.macro` built-in (the completer filters on `cmd.is_valid(state)`, + completer.rs:46). +- `enable`/`disable` are sub-args of the existing `.macro` entry, NOT new + `REPL_COMMANDS` array entries (the count-asserting test at repl/mod.rs:1732 + stays untouched). + `.help` gains a "custom commands (macros)" section and + a line stating macros = custom commands. + +## 5. Scoping: `enabled_macros` + +New optional field, mirroring `enabled_skills` semantics **verbatim** — same +parser (`parse_string_or_array`: YAML list or comma-separated string), same +null/absent/empty behavior. **VERIFIED semantics** (resolver: +`SkillPolicy::effective_with`, skill_policy.rs:40-132; precedence :78-82; +regression test :388-404 pinning CHANGELOG:320): + +**Scope of "mirror" (explicit):** the mirroring covers ONLY the allowlist +resolution semantics (None/empty/populated meanings, first-`Some`-wins +precedence) and the config plumbing (where the field lives, how each level +parses it). It does NOT copy any LLM-facing skills machinery: skills feed +instruction injection and tool-scope refresh into the model payload — +macros have no analog of any of that. The LLM never learns macros exist or +ran (an isolated macro's exchanges arrive as ordinary messages on a fork; a +non-isolated macro's steps are indistinguishable from typed input). +`enabled_macros` gets its own small resolver over the discovered-files set — +it is NOT wired into `SkillPolicy`, prompt building, or context startup +(see "Lazy resolution" below). + +- `None`/absent = "no opinion" → fall through to the next level; all-`None` + → everything visible. +- `Some([])` (empty list, incl. empty string via the parsers) = **explicit + ZERO** — nothing enabled. Empty ≠ all; that was the regression. +- Populated = exactly those names. **Deliberate DIVERGENCE from skills on + unknown names**: skills hard-bail the whole resolution + (skill_policy.rs:94-103 — there is no warn path). For macros, hard + validation happens ONLY at `.set enabled_macros` time (the + request_context.rs:2709-2727 pattern: bail on a name that exists in + neither workspace nor global macros dir); CONFIG-FILE lists are validated + gracefully at resolution time — warn + `missing` row in `.list macros`, + never a bail (a stale name in a role file must not brick that role). + There is no `visible_macros` concept in v1. +- Precedence: `.or_else()` chain — session → agent → role → global, **first + `Some` wins outright**, no merging. Matches the ruling below. +- Plumbing gotcha: the global level is TWO structs (`Config` mod.rs:222 AND + `AppConfig` app_config.rs:44/:212) plus an env-override arm + (app_config.rs:532); role parses via frontmatter `parse_string_or_array` + (role.rs:131), session via plain serde — three parse paths to mirror. + +- Global config (`config.example.yaml`, alongside `enabled_skills`): default + when no role/agent/session is active. Absent/null = all macros visible. +- Role (`config.role.example.md` frontmatter), agent config + (`config.agent.example.yaml`), session config: allowlist for that context. +- **Graph-based agents — CORRECTED BY VERIFICATION, then RULED (user, + 2026-08-20): silently ignored, option (a).** + The prior ruling ("graph configs reject the field") is not implementable + as specified: coyote uses `deny_unknown_fields` nowhere except + mcp/mod.rs:79, so unknown fields in graph.yaml are silently ignored — and + `enabled_skills` is in fact SUPPORTED at graph level (`AgentConfig:: + from_graph` copies it at agent.rs:811; graph/llm.rs:195-226 swaps per-node + values with save/restore; validator enforces node⊆graph at + graph/validator.rs:1156,:1235). Ruling: `enabled_macros` is simply omitted + from the `Graph` struct — graph.yaml ignores it like any other unknown + field (zero code, consistent behavior); the omission is documented in the + wiki/agent docs. Full skills-style graph support was rejected as + meaningless (graph nodes never dispatch REPL commands); a bespoke + validator warning was considered and declined. +- Precedence: most-specific active context that defines the field wins — + session > agent > role > global. No merging/intersection. +- Allowlist entries are macro **names** (the file stem — the same identifier + used for invocation). + +### Workspace-local macros (RULED: in scope) + +Workspace macro definitions are allowed, mirroring the existing +workspace-artifact conventions (VERIFIED mechanics): + +- Discovery: `.coyote/macros/*.yaml` under `workspace_config_dir()` + (paths.rs:198-207 — CWD only, no ancestor walk-up, dir name overridable + via `COYOTE_WORKSPACE_CONFIG_DIR`), exactly like workspace skills + (`workspace_skills_dir()`, paths.rs:209-215). +- Collision rule: **workspace shadows global by name** — same as workspace + skills (`list_skills` iterates [workspace, global] with shadowing, + paths.rs:478-501; `has_skill` checks workspace first, :503-505) and + workspace MCP (HashMap insert, workspace wins, mcp/mod.rs:239). +- Opt-out mirrors MCP: new `--no-workspace-macros` CLI flag + + `no_workspace_macros` config key (default false), modeled on + `--no-workspace-mcp` (cli/mod.rs:96-98 → main.rs:222-223, + app_config.rs:98). +- Trust model: no confirmation prompt — consistent with workspace MCP and + skills, which load with no gate (mcp/mod.rs:225-268 merely eprintlns); + macros are strictly lower-risk since they run only on explicit user + invocation, never automatically. `.list macros` gains a source column + (`workspace` | `global`) so provenance is always visible. +- `enabled_macros` existence validation consults workspace-then-global (the + `has_skill` pattern). +- The interactive creator (`.macro `) continues to write to the + GLOBAL macros dir; workspace macros are authored by hand / committed to + the repo. Remote installs (`--filter macros`) also target global only — + bundles never write into a workspace. +- Agent plumbing (VERIFIED): `AgentConfig` agent.rs:694-764 (getter/setter + pattern :399-412), reached lazily from `RequestContext.agent` + (request_context.rs:147, set in `use_agent` :3670+). Policy is resolved + lazily at enforcement sites from `ctx.role/agent/session` — no + activation-time snapshot exists, which confirms the lazy-resolution ruling + fits the existing architecture exactly. +- Session-level note: `enabled_skills` has NO REPL setter today (session file + serde only). Runtime toggles do NOT touch session state — `.macro + enable|disable` edits the global-level in-memory list via + `update_app_config` (see §6), reusing the `.set` machinery. +- Unknown names in a config-file list: warning + `missing` row in `.list + macros` (per the divergence ruling above — resolution never bails). + +### Lazy resolution + +`enabled_macros` never touches the LLM payload (macros are invisible to the +model), so the visible set is computed on demand — at top-level dispatch +fallback, completion, `.list macros`, `.macro enable|disable` — from: +discovered files x active context's allowlist x runtime toggles. Zero +context-startup work; no new startup-ordering surface. Rescan both macro +dirs (workspace + global) on each resolution — two `read_dir`s; matches +`paths::list_macros()` cost today. + +## 6. REPL management surface + +- `.list macros` (enriched): name, description, isolated?, state: + `enabled` | `disabled (runtime)` | `locked` | `missing` | `shadowed + (built-in)` | `invalid`. `locked` names the restricting config (e.g. + `agent:oracle enabled_macros`). Plus a source column + (`workspace` | `global`). +- **`.macro enable ` / `.macro disable `** (RULED — replaces the + earlier `.command` proposal; user prefers no separate command at the cost + of reserving two names): shorthand for editing `.set enabled_macros`. + Consequently **`enable` and `disable` are reserved macro names**: the + creator rejects them, discovery marks such a file `invalid` (with a + warning), and remote installs warn. +- **`.set enabled_macros `** — new key in the `.set` match + (`RequestContext::update`, request_context.rs:2644-2929), mirroring + `enabled_skills` (:2706-2730) verbatim: `csv_to_vec` parsing (space-free, + comma-separated), `null` clears back to None, per-name existence + validation (workspace-then-global), then + `update_app_config(|app| app.enabled_macros = ...)`. +- **Mechanism makes the no-override rule structural** (VERIFIED): `.set` + writes the in-memory GLOBAL AppConfig level — the LOWEST-precedence rung + of the session > agent > role > global chain — in-memory only, never + persisted (`update_app_config` clone-and-swaps the Arc, + request_context.rs:347-354). So `.macro enable|disable` physically cannot + override a role/agent/session `enabled_macros` allowlist. When such an + allowlist is active, the toggle ERRORS (naming the owning config) instead + of silently writing a shadowed value. +- Toggle semantics over the global-level list: `disable X` with list=None + (all visible) materializes the list as all-discovered-minus-X; `enable X` + appends if absent (None → already visible → no-op notice); `disable X` + removes (absent → no-op notice). Lifetime: process runtime, like every + other `.set` key. +- Drive-by fix bundled here: `enabled_skills` is missing from the `.set` + key completion list (request_context.rs:3018-3047) although its setter + works — add both `enabled_skills` and `enabled_macros` to completion. + +States per macro per context: +1. permitted + enabled (default within allowlist) +2. permitted + disabled (runtime toggle) +3. locked (outside a ROLE/AGENT/SESSION allowlist) — not enableable from the + REPL; the error says to edit `enabled_macros` in the owning config. No + REPL override, ever: those configs stay the single source of truth. + +Boundary rule: `locked` applies ONLY to role/agent/session allowlists. A +global-level exclusion (whether from config.yaml or a prior toggle — they +occupy the same in-memory list and are indistinguishable) is always +toggleable via `.macro enable` and displays as `disabled (runtime)`. + +### Error handling matrix + +| Action | Condition | Behavior | +|---|---|---| +| `.name` | no built-in, no macro | existing error verbatim: `Error: Unknown command. Type ".help" for additional help.` | +| `.name` / `.macro name` | locked | error naming the restricting context/config | +| `.name` / `.macro name` | runtime-disabled | error: re-enable with `.macro enable ` | +| `.macro enable X` | locked (more-specific allowlist active) | error: "restricted by ; edit `enabled_macros` there" | +| `.macro enable X` | unknown | existing unknown-style error | +| `.macro enable X` | already enabled | no-op notice | +| `.macro create/creator` | name is `enable` or `disable` | error: reserved name | +| discovery | file named `enable`/`disable`.yaml | `invalid` in `.list macros` + warning | +| config `enabled_macros` | unknown name | warning + `missing` in `.list macros` | +| discovery | name shadows built-in | works via `.macro` only; `shadowed` in `.list macros` | +| discovery | workspace + global same name | workspace shadows global; both visible in `.list macros` source column | +| non-isolated macro | step invokes another macro | error: nested macros not allowed in non-isolated mode | + +## 7. Install & distribution + +Unchanged: `--install-from --filter macros` (CLI + REPL; flag rename to +`--install` tracked in plans/bundle-manifest-design.md §5). Installing macros +never modifies any `enabled_macros` list — a context with an allowlist is +unaffected by new installs until the user edits it (the security property +motivating context-side scoping). Existing overwrite/skip behavior applies; +install-time warning if an installed macro's name shadows a built-in. + +## 8. Docs + +- README + `.help`: "macros are coyote's custom commands" framing; top-level + invocation; `isolated` semantics with the role-switch-persists warning. +- `config.example.yaml`, `config.role.example.md`, + `config.agent.example.yaml`: `enabled_macros` entries mirroring the + existing `enabled_skills` doc comments; `no_workspace_macros` entry + alongside the existing `no_workspace_mcp` one (config.example.yaml:140-145). +- New `macro.example.yaml` (or extend existing docs) showing all fields incl. + description/isolated. +- Workspace macros: document `.coyote/macros/` alongside the existing + workspace skills/MCP conventions. + +## 9. Implementation sketch + +1. **Macro struct**: add `description`, `isolated` (serde defaults); + deser tests for back-compat with field-less YAML. +2. **`enabled_macros` field**: global AppConfig + role + agent (non-graph) + + session structs (both `Config` AND `AppConfig` at the global level + env + arm), via `parse_string_or_array` like role.rs:131; graph.yaml silently + ignores it (field omitted from the Graph struct, per §5 ruling); + precedence resolver + tests (incl. the empty-list-means-zero regression + case). +3. **Resolver**: discovered x allowlist x runtime toggles (global-level + in-memory) → + visible set + per-macro state; table-driven tests over the §6 matrix. + OWNS the two-dir discovery (`workspace_macros_dir()` + shadowing, the + has_skill/list_skills pattern) so steps 4/5/6 are genuinely independent. +4. **REPL dispatch**: macro fallback immediately before `unknown_command()`; + `.macro enable|disable` + `.set enabled_macros` key (+ completion + drive-by); enriched `.list macros`; dynamic macro completion with + shadowed-name exclusion; `.macro ` arg completion upgraded to + descriptions + subcommands (request_context.rs:3004); `.help`. Enforce + visibility on the `.macro` path too. +5. **Non-isolated execution**: `macro_execute_inline(ctx, ...)` variant (or + branch) running steps on the live ctx with RAII macro_flag guard + + nested-macro rejection; tests for flag restore on error, plus two + mock-free session tests (oracle note — no mock-client harness needed): + (i) the non-isolated path passes the LIVE ctx (session `Some`) into + `run_repl_command`, not a fork; (ii) a mutating step (`.model x`) + persists on the live ctx after the macro returns. Also pin + isolated→non-isolated nesting (§3). +6. **Workspace macros (flag + docs only; discovery lives in step 3)**: + `--no-workspace-macros` flag + `no_workspace_macros` config key, source + column in `.list macros`. +7. **Docs** (§8). + +Order: 1 → 2 → 3 → {4, 5, 6 in parallel} → 7. + +## 10. Follow-ups (out of scope) + +- Persisted toggles (surviving restart) — `.set` state is process-lifetime + by design; persistence would be new machinery for all `.set` keys, not + just macros. +- Bundle manifest & provenance layer — **separate design doc**, now written: + `plans/bundle-manifest-design.md` (per-file provenance recording at install + time, `--list-bundles` / `--update-bundle` / `--uninstall`, optional + author-shipped `coyote-bundle.yaml` manifest). Once it exists, `.list + macros` gains a "source bundle" column for free. This plan neither depends + on nor blocks it. +- "Did you mean" suggestions on unknown command (only if built-ins get it too). + +## 11. VERIFY before task materialization + +All items resolved 2026-08-20 (findings folded into §3/§4/§5 above): + +- [x] `enabled_skills` semantics: None=fall-through, empty=ZERO, populated= + exact+hard-bail validation; first-`Some`-wins precedence + (`SkillPolicy::effective_with`). See §5. +- [x] Agent config plumbing: lazy resolution from `ctx.agent`, no snapshot; + new field = AgentConfig field + accessor + resolver read. See §5. +- [x] `ReplCommand` registry: static fixed array, count-asserting test; + macros go in a separate dynamic completer source. See §4. +- [x] `macro_flag` guards: 13 sites inventoried; none suppress session + recording; one OPEN DECISION (`use_agent` agent_session suppression) + + nested-rejection is new code. See §3. +- [x] Session recording: `after_chat_completion` → `save_message` → + `Session::add_message`, no macro conditions; isolation lives in the + fork's `session: None`. See §3. + +Both open decisions ruled by the user 2026-08-20: §3 `use_agent` suppression +is conditioned on isolation (lifted for isolated:false); §5 graph.yaml +silently ignores `enabled_macros`. No open questions remain. + +Tooling warning for implementers: `fs_grep`/plain grep tools silently skip +src/config/request_context.rs (file-size exclusion) — audit that file with +ast_grep or targeted reads only. + +### Second verification round (2026-08-20, after user review) + +- [x] `.set` mechanics: fixed hand-written key match in + `RequestContext::update` (request_context.rs:2644-2929); + `enabled_skills` IS settable (:2706-2730) and always writes the + in-memory global AppConfig via `update_app_config` (:347-354) — + lowest precedence, never persisted, no role/agent/session setter. + Values: `csv_to_vec` comma lists (space-free; whitespace rejected for + all but two keys, :2654-2662), `null` clears. `enabled_skills` missing + from `.set` completion (:3018-3047) — oversight, fixed as drive-by. +- [x] Workspace discovery precedents: MCP = CWD-only probe + `.coyote/mcp.json` → `.coyote/.mcp.json` → `.mcp.json` + (paths.rs:217-230), workspace wins collisions via HashMap insert + (mcp/mod.rs:239), gated by `no_workspace_mcp` only, no trust prompt; + skills = `.coyote/skills/` shadowing global by name + (paths.rs:478-505); memory walks ancestors but MCP/skills do NOT. + Workspace macros copy the skills model + an MCP-style opt-out flag. From e8b55bba15fd46870ef7e9bcf4d14fe57c0678aa Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 14:45:13 -0600 Subject: [PATCH 02/18] feat: add description and isolated fields to Macro struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit description (optional, default None) will surface in listings and completion; isolated (default true) preserves today's forked-context execution behavior exactly. Both fields use plain serde defaults so every existing macro YAML deserializes unchanged, and unknown fields in newer files remain tolerated by older binaries. Adds Serialize to Macro/MacroVariable (None description skipped) and back-compat, round-trip, and embedded-asset deserialization tests. Per plans/custom-commands-design.md §3 / §9 step 1. --- src/config/macros.rs | 89 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 3 deletions(-) diff --git a/src/config/macros.rs b/src/config/macros.rs index 179e52f..3a1ceed 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -5,7 +5,7 @@ use crate::utils::{AbortSignal, multiline_text}; use anyhow::{Context, Result, anyhow}; use indexmap::IndexMap; use rust_embed::Embed; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::fs::{File, read_to_string}; use std::io::Write; use std::sync::Arc; @@ -69,8 +69,12 @@ pub async fn macro_execute( Ok(()) } -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct Macro { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, + #[serde(default = "default_true")] + pub isolated: bool, #[serde(default)] pub variables: Vec, pub steps: Vec, @@ -162,14 +166,19 @@ impl Macro { } } -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct MacroVariable { pub name: String, #[serde(default)] pub rest: bool, + #[serde(skip_serializing_if = "Option::is_none")] pub default: Option, } +fn default_true() -> bool { + true +} + #[cfg(test)] mod tests { use super::*; @@ -184,6 +193,8 @@ mod tests { fn macro_with_vars(vars: Vec) -> Macro { Macro { + description: None, + isolated: true, variables: vars, steps: vec![], } @@ -370,4 +381,76 @@ steps: assert!(m.variables.is_empty()); assert_eq!(m.steps.len(), 1); } + + #[test] + fn deserialize_macro_without_new_fields_uses_defaults() { + let yaml = r#" +steps: + - ".help" +"#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert!(m.description.is_none()); + assert!(m.isolated); + } + + #[test] + fn deserialize_macro_with_description_and_isolated() { + let yaml = r#" +description: "Review WIP against a base branch" +isolated: false +steps: + - "Review the diff against {{base}}" +variables: + - name: base + default: main +"#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert_eq!( + m.description.as_deref(), + Some("Review WIP against a base branch") + ); + assert!(!m.isolated); + assert_eq!(m.variables.len(), 1); + } + + #[test] + fn round_trip_preserves_new_fields() { + let original = Macro { + description: Some("does a thing".to_string()), + isolated: false, + variables: vec![var("target", false, Some("all"))], + steps: vec!["build {{target}}".to_string()], + }; + let yaml = serde_yaml::to_string(&original).unwrap(); + let back: Macro = serde_yaml::from_str(&yaml).unwrap(); + assert_eq!(back.description.as_deref(), Some("does a thing")); + assert!(!back.isolated); + assert_eq!(back.variables.len(), 1); + assert_eq!(back.variables[0].name, "target"); + assert_eq!(back.variables[0].default.as_deref(), Some("all")); + assert_eq!(back.steps, original.steps); + } + + #[test] + fn round_trip_defaults_survive() { + let original = macro_with_vars(vec![]); + let yaml = serde_yaml::to_string(&original).unwrap(); + assert!(!yaml.contains("description")); + let back: Macro = serde_yaml::from_str(&yaml).unwrap(); + assert!(back.description.is_none()); + assert!(back.isolated); + } + + #[test] + fn embedded_macro_assets_deserialize_with_defaults() { + for file in MacroAssets::iter() { + let embedded = MacroAssets::get(&file).unwrap(); + let content = std::str::from_utf8(&embedded.data).unwrap(); + let m: Macro = serde_yaml::from_str(content) + .unwrap_or_else(|e| panic!("asset '{}' failed to deserialize: {e}", file.as_ref())); + assert!(m.description.is_none(), "asset '{}'", file.as_ref()); + assert!(m.isolated, "asset '{}'", file.as_ref()); + assert!(!m.steps.is_empty(), "asset '{}'", file.as_ref()); + } + } } From f39381aa9d022b6b9ce8e23753c63c1e63ccbc89 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 15:14:20 -0600 Subject: [PATCH 03/18] feat: add enabled_macros config field at global, role, agent, and session levels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the enabled_skills plumbing per plans/custom-commands-design.md §5: - global: Config + AppConfig structs, from_config copy, and the COYOTE_ENABLED_MACROS env-override arm (csv_to_vec parsing) - role: frontmatter via parse_string_or_array (list or csv string), plus the export() mirror so Role::save round-trips the field - agent (non-graph): plain serde on AgentConfig; graph.yaml silently ignores the key (pinned by test, no field on Graph by design) - session: plain serde with csv-or-vec deserializer Empty list/string deserializes to Some([]) (explicit zero), distinct from absent/null (None) — pinned by tests at every level, including the env arm (serial-fenced against the from_config tests, which read the process env via load_envs). --- src/config/agent.rs | 26 +++++++++++++++ src/config/app_config.rs | 72 ++++++++++++++++++++++++++++++++++++++++ src/config/mod.rs | 39 ++++++++++++++++++++++ src/config/role.rs | 60 +++++++++++++++++++++++++++++++++ src/config/session.rs | 45 +++++++++++++++++++++++++ src/graph/types.rs | 8 +++++ 6 files changed, 250 insertions(+) diff --git a/src/config/agent.rs b/src/config/agent.rs index 95467ce..2a9f1ba 100644 --- a/src/config/agent.rs +++ b/src/config/agent.rs @@ -744,6 +744,8 @@ pub struct AgentConfig { #[serde(skip_serializing_if = "Option::is_none")] pub enabled_skills: Option>, #[serde(skip_serializing_if = "Option::is_none")] + pub enabled_macros: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub continuation_prompt: Option, #[serde(default)] pub instructions: String, @@ -1225,6 +1227,30 @@ variables: assert!(config.top_p.is_none()); } + #[test] + fn agent_config_enabled_macros_absent_is_none() { + let yaml = "name: minimal\ninstructions: hi\n"; + let config: AgentConfig = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!(config.enabled_macros, None); + } + + #[test] + fn agent_config_enabled_macros_empty_list_is_some_empty() { + let yaml = "name: minimal\ninstructions: hi\nenabled_macros: []\n"; + let config: AgentConfig = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!(config.enabled_macros, Some(vec![])); + } + + #[test] + fn agent_config_enabled_macros_list() { + let yaml = "name: minimal\ninstructions: hi\nenabled_macros:\n - a\n"; + let config: AgentConfig = serde_yaml::from_str(yaml).unwrap(); + + assert_eq!(config.enabled_macros, Some(vec!["a".to_string()])); + } + #[test] fn agent_config_with_model() { let yaml = diff --git a/src/config/app_config.rs b/src/config/app_config.rs index be85041..51a1855 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -43,6 +43,8 @@ pub struct AppConfig { #[serde(default, deserialize_with = "super::deserialize_csv_or_vec")] pub enabled_skills: Option>, pub visible_skills: Option>, + #[serde(default, deserialize_with = "super::deserialize_csv_or_vec")] + pub enabled_macros: Option>, pub mcp_server_support: bool, pub mapping_mcp_servers: IndexMap, @@ -127,6 +129,7 @@ impl Default for AppConfig { skills_enabled: true, enabled_skills: None, visible_skills: None, + enabled_macros: None, mcp_server_support: true, mapping_mcp_servers: Default::default(), @@ -211,6 +214,7 @@ impl AppConfig { skills_enabled: config.skills_enabled, enabled_skills: config.enabled_skills, visible_skills: config.visible_skills, + enabled_macros: config.enabled_macros, mcp_server_support: config.mcp_server_support, mapping_mcp_servers: config.mapping_mcp_servers, @@ -533,6 +537,10 @@ impl AppConfig { self.enabled_skills = v.map(|raw| super::csv_to_vec(&raw)); } + if let Some(v) = super::read_env_value::(&get_env_name("enabled_macros")) { + self.enabled_macros = v.map(|raw| super::csv_to_vec(&raw)); + } + if let Some(Some(v)) = super::read_env_bool(&get_env_name("mcp_server_support")) { self.mcp_server_support = v; } @@ -769,6 +777,70 @@ mod tests { ); } + #[test] + #[serial_test::serial] + fn from_config_copies_enabled_macros() { + let cfg = Config { + model_id: "provider:test".to_string(), + enabled_macros: Some(vec!["a".to_string()]), + ..Config::default() + }; + + let app = AppConfig::from_config(cfg).unwrap(); + + assert_eq!(app.enabled_macros, Some(vec!["a".to_string()])); + } + + #[test] + #[serial_test::serial] + fn from_config_preserves_explicit_empty_enabled_macros() { + let cfg = Config { + model_id: "provider:test".to_string(), + enabled_macros: Some(vec![]), + ..Config::default() + }; + + let app = AppConfig::from_config(cfg).unwrap(); + + assert_eq!(app.enabled_macros, Some(vec![])); + } + + #[test] + #[serial_test::serial] + fn load_envs_overrides_enabled_macros() { + let env_name = get_env_name("enabled_macros"); + let prev = std::env::var_os(&env_name); + + let mut app = AppConfig::default(); + + unsafe { std::env::set_var(&env_name, "a,b") }; + app.load_envs(); + assert_eq!( + app.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + + unsafe { std::env::set_var(&env_name, "") }; + app.load_envs(); + assert_eq!(app.enabled_macros, Some(vec![])); + + unsafe { std::env::set_var(&env_name, "null") }; + app.load_envs(); + assert_eq!(app.enabled_macros, None); + + unsafe { std::env::remove_var(&env_name) }; + app.enabled_macros = Some(vec!["keep".to_string()]); + app.load_envs(); + assert_eq!(app.enabled_macros, Some(vec!["keep".to_string()])); + + unsafe { + match prev { + Some(v) => std::env::set_var(&env_name, v), + None => std::env::remove_var(&env_name), + } + } + } + #[test] fn editor_returns_configured_value() { let configured = cached_editor() diff --git a/src/config/mod.rs b/src/config/mod.rs index e92f5b6..ad25b6e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -221,6 +221,8 @@ pub struct Config { #[serde(default, deserialize_with = "deserialize_csv_or_vec")] pub enabled_skills: Option>, pub visible_skills: Option>, + #[serde(default, deserialize_with = "deserialize_csv_or_vec")] + pub enabled_macros: Option>, pub mcp_server_support: bool, pub mapping_mcp_servers: IndexMap, @@ -303,6 +305,7 @@ impl Default for Config { skills_enabled: true, enabled_skills: None, visible_skills: None, + enabled_macros: None, mcp_server_support: true, mapping_mcp_servers: Default::default(), @@ -1124,6 +1127,42 @@ clients: assert!(cfg.enabled_mcp_servers.is_none()); } + #[test] + fn config_enabled_macros_absent_is_none() { + let cfg: Config = serde_yaml::from_str("model: provider:test").unwrap(); + assert_eq!(cfg.enabled_macros, None); + } + + #[test] + fn config_enabled_macros_empty_string_is_some_empty() { + let cfg: Config = serde_yaml::from_str("enabled_macros: \"\"").unwrap(); + assert_eq!(cfg.enabled_macros, Some(vec![])); + } + + #[test] + fn config_enabled_macros_csv_string() { + let cfg: Config = serde_yaml::from_str("enabled_macros: \"a, b\"").unwrap(); + assert_eq!( + cfg.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn config_enabled_macros_list() { + let cfg: Config = serde_yaml::from_str("enabled_macros:\n - a\n - b").unwrap(); + assert_eq!( + cfg.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn config_enabled_macros_null_is_none() { + let cfg: Config = serde_yaml::from_str("enabled_macros: null").unwrap(); + assert_eq!(cfg.enabled_macros, None); + } + #[test] fn assert_state_pass_always_true() { let pass = AssertState::pass(); diff --git a/src/config/role.rs b/src/config/role.rs index cfb6104..69281f8 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -75,6 +75,12 @@ pub struct Role { deserialize_with = "super::deserialize_csv_or_vec" )] enabled_skills: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "super::deserialize_csv_or_vec" + )] + enabled_macros: Option>, #[serde(skip_serializing_if = "Option::is_none")] auto_continue: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -129,6 +135,7 @@ impl Role { } "skills_enabled" => role.skills_enabled = value.as_bool(), "enabled_skills" => role.enabled_skills = parse_string_or_array(value), + "enabled_macros" => role.enabled_macros = parse_string_or_array(value), "auto_continue" => role.auto_continue = value.as_bool(), "max_auto_continues" => { role.max_auto_continues = value.as_u64().map(|v| v as usize) @@ -196,6 +203,10 @@ impl Role { let inline = serde_json::to_string(enabled_skills).unwrap_or_else(|_| "[]".to_string()); metadata.push(format!("enabled_skills: {inline}")); } + if let Some(enabled_macros) = &self.enabled_macros { + let inline = serde_json::to_string(enabled_macros).unwrap_or_else(|_| "[]".to_string()); + metadata.push(format!("enabled_macros: {inline}")); + } if let Some(auto_continue) = self.auto_continue { metadata.push(format!("auto_continue: {auto_continue}")); } @@ -588,6 +599,55 @@ mod tests { assert_eq!(role.enabled_mcp_servers(), None); } + #[test] + fn role_new_enabled_macros_absent_is_none() { + let role = Role::new("test", "---\ntemperature: 0.5\n---\nPrompt"); + assert_eq!(role.enabled_macros, None); + } + + #[test] + fn role_new_enabled_macros_empty_string_is_some_empty() { + let role = Role::new("test", "---\nenabled_macros: \"\"\n---\nPrompt"); + assert_eq!(role.enabled_macros, Some(vec![])); + } + + #[test] + fn role_new_enabled_macros_csv_string() { + let role = Role::new("test", "---\nenabled_macros: a, b\n---\nPrompt"); + assert_eq!( + role.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn role_new_enabled_macros_list() { + let role = Role::new("test", "---\nenabled_macros: [a, b]\n---\nPrompt"); + assert_eq!( + role.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn role_new_enabled_macros_null_is_none() { + let role = Role::new("test", "---\nenabled_macros: null\n---\nPrompt"); + assert_eq!(role.enabled_macros, None); + } + + #[test] + fn role_export_includes_enabled_macros() { + let role = Role::new("test", "---\nenabled_macros: [a]\n---\nPrompt"); + let exported = role.export(); + assert!(exported.contains("enabled_macros: [\"a\"]")); + } + + #[test] + fn role_export_omits_enabled_macros_when_none() { + let role = Role::new("test", "Just a prompt"); + assert!(!role.export().contains("enabled_macros")); + } + #[test] fn role_builtin_shell_loads() { let role = Role::builtin("shell").unwrap(); diff --git a/src/config/session.rs b/src/config/session.rs index 0b593b7..cd7f04e 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -46,6 +46,12 @@ pub struct Session { deserialize_with = "super::deserialize_csv_or_vec" )] enabled_skills: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "super::deserialize_csv_or_vec" + )] + enabled_macros: Option>, #[serde(skip_serializing_if = "Option::is_none")] save_session: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -931,6 +937,45 @@ mod tests { assert!(!session.dirty()); } + #[test] + fn session_enabled_macros_absent_is_none() { + let session: Session = serde_yaml::from_str("model: provider:test\nmessages: []").unwrap(); + assert_eq!(session.enabled_macros, None); + } + + #[test] + fn session_enabled_macros_empty_list_is_some_empty() { + let session: Session = + serde_yaml::from_str("model: provider:test\nenabled_macros: []\nmessages: []").unwrap(); + assert_eq!(session.enabled_macros, Some(vec![])); + } + + #[test] + fn session_enabled_macros_empty_string_is_some_empty() { + let session: Session = + serde_yaml::from_str("model: provider:test\nenabled_macros: \"\"\nmessages: []") + .unwrap(); + assert_eq!(session.enabled_macros, Some(vec![])); + } + + #[test] + fn session_enabled_macros_csv_string() { + let session: Session = + serde_yaml::from_str("model: provider:test\nenabled_macros: \"a,b\"\nmessages: []") + .unwrap(); + assert_eq!( + session.enabled_macros, + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + + #[test] + fn session_serialize_omits_enabled_macros_when_none() { + let session = Session::default(); + let yaml = serde_yaml::to_string(&session).unwrap(); + assert!(!yaml.contains("enabled_macros")); + } + #[test] fn session_new_from_ctx_captures_save_session() { let app_config = Arc::new(AppConfig::default()); diff --git a/src/graph/types.rs b/src/graph/types.rs index fcd3292..8e13538 100644 --- a/src/graph/types.rs +++ b/src/graph/types.rs @@ -588,6 +588,14 @@ nodes: )); } + #[test] + fn graph_silently_ignores_enabled_macros_key() { + let yaml = "name: g\nenabled_macros: [\"x\"]\nstart: x\nnodes:\n x:\n id: x\n type: end\n output: ok\n"; + let graph: Graph = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(graph.name, "g"); + assert_eq!(graph.start, "x"); + } + #[test] fn graph_settings_have_sensible_defaults() { let yaml = "name: g\nstart: x\nnodes:\n x:\n id: x\n type: end\n output: ok\n"; From e8ddb615184ebe9d7ef6160aa9b9ec279805a0e8 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 15:28:50 -0600 Subject: [PATCH 04/18] feat: add lazy macro resolver with two-dir discovery and per-macro states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds src/config/macro_policy.rs: MacroPolicy::effective computes the visible macro set on demand from the discovered definition files, the four-level enabled_macros allowlists, and the built-in command names. - Discovery scans workspace (.coyote/macros/) then global macros dirs on every resolution; workspace shadows global by name, and the shadowed global entry is retained and flagged so both stay listable (plan custom-commands-design.md §5). Workspace scanning is gated on a bool parameter so the future --no-workspace-macros flag wires in one line. - Allowlist precedence is session > agent > role > global, first Some wins, no merging; None falls through, an empty list is an explicit zero, all-None enables everything (mirrors SkillPolicy). - Per-macro states per plan §6: enabled, disabled (runtime, global-level exclusions only), locked (role/agent/session exclusions, recording the owning level), missing (unknown allowlist names warn instead of bailing — deliberate divergence from skills), shadowed (built-in name collisions), and invalid (parse failures and the reserved names enable/disable). Invalid beats allowlist exclusion beats shadowing. - Adds enabled_macros() accessors on Role, Session, and Agent alongside their enabled_skills() counterparts, plus paths::workspace_macros_dir. - 37 tests: state matrix, pairwise precedence, explicit-zero pinned at every level, workspace shadowing, reserved names, builtin collisions, missing rows, invalid YAML, and env-gated discovery (#[serial]). --- src/config/agent.rs | 4 + src/config/macro_policy.rs | 929 +++++++++++++++++++++++++++++++++++++ src/config/mod.rs | 6 + src/config/paths.rs | 4 + src/config/role.rs | 4 + src/config/session.rs | 4 + 6 files changed, 951 insertions(+) create mode 100644 src/config/macro_policy.rs diff --git a/src/config/agent.rs b/src/config/agent.rs index 2a9f1ba..dce814c 100644 --- a/src/config/agent.rs +++ b/src/config/agent.rs @@ -400,6 +400,10 @@ impl Agent { self.config.enabled_skills.as_deref() } + pub fn enabled_macros(&self) -> Option<&[String]> { + self.config.enabled_macros.as_deref() + } + pub fn memory(&self) -> Option { self.config.memory } diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs new file mode 100644 index 0000000..bee02e6 --- /dev/null +++ b/src/config/macro_policy.rs @@ -0,0 +1,929 @@ +use super::agent::Agent; +use super::app_config::AppConfig; +use super::macros::Macro; +use super::paths; +use super::role::Role; +use super::session::Session; + +use log::warn; +use std::collections::HashSet; +use std::fmt; +use std::fs::{read_dir, read_to_string}; +use std::path::PathBuf; + +/// Names that cannot be used as macro names because they are reserved for +/// `.macro enable ` / `.macro disable `. +pub const RESERVED_MACRO_NAMES: [&str; 2] = ["enable", "disable"]; + +/// Where a macro definition file was discovered. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MacroSource { + Workspace, + Global, +} + +impl fmt::Display for MacroSource { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MacroSource::Workspace => write!(f, "workspace"), + MacroSource::Global => write!(f, "global"), + } + } +} + +/// The configuration level whose `enabled_macros` allowlist won the +/// first-`Some`-wins precedence chain (session > agent > role > global). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MacroAllowlistLevel { + Session, + Agent, + Role, + Global, +} + +impl fmt::Display for MacroAllowlistLevel { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MacroAllowlistLevel::Session => write!(f, "session"), + MacroAllowlistLevel::Agent => write!(f, "agent"), + MacroAllowlistLevel::Role => write!(f, "role"), + MacroAllowlistLevel::Global => write!(f, "global"), + } + } +} + +/// The effective state of a macro within the active context. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MacroState { + /// Visible and invocable. + Enabled, + /// Excluded by the GLOBAL-level `enabled_macros` list (config file or + /// runtime toggle — the two are indistinguishable by design). Can be + /// re-enabled at runtime with `.macro enable `. + DisabledRuntime, + /// Excluded by a role/agent/session `enabled_macros` allowlist. Not + /// enableable from the REPL; the owning config is the source of truth. + /// `level` is never `Global` — a global exclusion is `DisabledRuntime`. + Locked { level: MacroAllowlistLevel }, + /// Named by the effective allowlist, but no such macro is installed. + Missing, + /// The macro name collides with a built-in REPL command; invocable only + /// via `.macro `, never as a top-level `.name` command. + ShadowedBuiltin, + /// The definition file failed to parse, or the name is reserved. + Invalid { reason: String }, +} + +impl MacroState { + #[allow(dead_code)] + pub fn is_invocable(&self) -> bool { + matches!(self, MacroState::Enabled | MacroState::ShadowedBuiltin) + } +} + +/// A macro definition file found on disk, before allowlist resolution. +#[derive(Debug, Clone)] +pub struct DiscoveredMacro { + pub name: String, + pub source: MacroSource, + /// The parsed definition, or the parse failure reason. + pub definition: Result, + /// True for a GLOBAL entry whose name is shadowed by a workspace entry. + pub shadowed_by_workspace: bool, +} + +/// One row of the resolved macro set. Missing allowlist entries produce rows +/// with `source: None`. +#[derive(Debug, Clone)] +#[allow(dead_code)] +pub struct ResolvedMacro { + pub name: String, + pub source: Option, + pub description: Option, + pub isolated: Option, + /// True for a global entry shadowed by a workspace entry of the same + /// name; the row is kept for display but is never the invocation target. + pub shadowed_by_workspace: bool, + pub state: MacroState, +} + +/// The visible macro set for the active context, resolved lazily on demand +/// from the discovered definition files and the effective `enabled_macros` +/// allowlist. +#[derive(Debug)] +pub struct MacroPolicy { + pub macros: Vec, +} + +impl MacroPolicy { + #[allow(dead_code)] + pub fn effective( + global: &AppConfig, + role: Option<&Role>, + agent: Option<&Agent>, + session: Option<&Session>, + builtin_commands: &[&str], + no_workspace_macros: bool, + ) -> Self { + Self::effective_with( + discover_macros(no_workspace_macros), + session.and_then(|s| s.enabled_macros()), + agent.and_then(|a| a.enabled_macros()), + role.and_then(|r| r.enabled_macros()), + global.enabled_macros.as_deref(), + builtin_commands, + ) + } + + fn effective_with( + discovered: Vec, + session_list: Option<&[String]>, + agent_list: Option<&[String]>, + role_list: Option<&[String]>, + global_list: Option<&[String]>, + builtin_commands: &[&str], + ) -> Self { + let allowlist = session_list + .map(|list| (MacroAllowlistLevel::Session, list)) + .or_else(|| agent_list.map(|list| (MacroAllowlistLevel::Agent, list))) + .or_else(|| role_list.map(|list| (MacroAllowlistLevel::Role, list))) + .or_else(|| global_list.map(|list| (MacroAllowlistLevel::Global, list))); + + let mut macros: Vec = discovered + .into_iter() + .map(|discovered_macro| { + let state = resolve_state(&discovered_macro, allowlist, builtin_commands); + let (description, isolated) = match &discovered_macro.definition { + Ok(value) => (value.description.clone(), Some(value.isolated)), + Err(_) => (None, None), + }; + ResolvedMacro { + name: discovered_macro.name, + source: Some(discovered_macro.source), + description, + isolated, + shadowed_by_workspace: discovered_macro.shadowed_by_workspace, + state, + } + }) + .collect(); + + if let Some((_, list)) = allowlist { + let known: HashSet<&str> = macros.iter().map(|m| m.name.as_str()).collect(); + let mut missing: Vec = vec![]; + for name in list { + if !known.contains(name.as_str()) && !missing.iter().any(|m| &m.name == name) { + warn!("enabled_macros references macro '{name}' which is not installed"); + missing.push(ResolvedMacro { + name: name.clone(), + source: None, + description: None, + isolated: None, + shadowed_by_workspace: false, + state: MacroState::Missing, + }); + } + } + macros.extend(missing); + } + + macros.sort_by(|a, b| { + a.name + .cmp(&b.name) + .then_with(|| source_rank(a.source).cmp(&source_rank(b.source))) + }); + + Self { macros } + } + + /// The invocation target for `name`: the workspace entry when one shadows + /// a global entry, otherwise the single discovered entry. Missing rows + /// are never returned. + #[allow(dead_code)] + pub fn find(&self, name: &str) -> Option<&ResolvedMacro> { + self.macros + .iter() + .find(|m| m.name == name && m.source.is_some() && !m.shadowed_by_workspace) + } +} + +fn source_rank(source: Option) -> u8 { + match source { + Some(MacroSource::Workspace) => 0, + Some(MacroSource::Global) => 1, + None => 2, + } +} + +fn resolve_state( + discovered: &DiscoveredMacro, + allowlist: Option<(MacroAllowlistLevel, &[String])>, + builtin_commands: &[&str], +) -> MacroState { + if RESERVED_MACRO_NAMES.contains(&discovered.name.as_str()) { + warn!( + "Ignoring macro '{}': the name is reserved for '.macro {}'", + discovered.name, discovered.name + ); + return MacroState::Invalid { + reason: format!("'{}' is a reserved macro name", discovered.name), + }; + } + + if let Err(reason) = &discovered.definition { + return MacroState::Invalid { + reason: reason.clone(), + }; + } + + if let Some((level, list)) = allowlist + && !list.iter().any(|name| name == &discovered.name) + { + return match level { + MacroAllowlistLevel::Global => MacroState::DisabledRuntime, + level => MacroState::Locked { level }, + }; + } + + if builtin_commands.contains(&discovered.name.as_str()) { + return MacroState::ShadowedBuiltin; + } + + MacroState::Enabled +} + +/// Rescans the workspace and global macro directories. Workspace entries +/// shadow global entries of the same name; the shadowed global entry is kept +/// and flagged so both remain listable. +pub fn discover_macros(no_workspace_macros: bool) -> Vec { + let mut dirs = vec![]; + if !no_workspace_macros { + dirs.push((MacroSource::Workspace, paths::workspace_macros_dir())); + } + dirs.push((MacroSource::Global, paths::macros_dir())); + discover_macros_in(&dirs) +} + +fn discover_macros_in(dirs: &[(MacroSource, PathBuf)]) -> Vec { + let mut seen: HashSet = HashSet::new(); + let mut output = vec![]; + + for (source, dir) in dirs { + let Ok(rd) = read_dir(dir) else { + continue; + }; + let mut entries: Vec<_> = rd.flatten().collect(); + entries.sort_by_key(|entry| entry.file_name()); + for entry in entries { + let is_file = entry + .file_type() + .map(|file_type| file_type.is_file()) + .unwrap_or(false); + if !is_file { + continue; + } + let Some(name) = entry + .file_name() + .to_str() + .and_then(|v| v.strip_suffix(".yaml")) + .map(str::to_string) + else { + continue; + }; + if name.is_empty() { + continue; + } + let definition = read_to_string(entry.path()) + .map_err(|err| err.to_string()) + .and_then(|content| { + serde_yaml::from_str::(&content).map_err(|err| err.to_string()) + }); + let shadowed_by_workspace = !seen.insert(name.clone()); + output.push(DiscoveredMacro { + name, + source: *source, + definition, + shadowed_by_workspace, + }); + } + } + + output +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::utils::get_env_name; + use serial_test::serial; + use std::path::Path; + use std::{env, fs, time}; + + fn valid_macro() -> Macro { + Macro { + description: Some("a test macro".to_string()), + isolated: true, + variables: vec![], + steps: vec![".help".to_string()], + } + } + + fn disc(name: &str, source: MacroSource) -> DiscoveredMacro { + DiscoveredMacro { + name: name.to_string(), + source, + definition: Ok(valid_macro()), + shadowed_by_workspace: false, + } + } + + fn disc_invalid(name: &str, reason: &str) -> DiscoveredMacro { + DiscoveredMacro { + name: name.to_string(), + source: MacroSource::Global, + definition: Err(reason.to_string()), + shadowed_by_workspace: false, + } + } + + fn globals(names: &[&str]) -> Vec { + names.iter().map(|n| disc(n, MacroSource::Global)).collect() + } + + fn list(names: &[&str]) -> Vec { + names.iter().map(|s| s.to_string()).collect() + } + + fn resolve( + discovered: Vec, + session: Option<&[String]>, + agent: Option<&[String]>, + role: Option<&[String]>, + global: Option<&[String]>, + ) -> MacroPolicy { + MacroPolicy::effective_with(discovered, session, agent, role, global, &[]) + } + + fn state_of<'a>(policy: &'a MacroPolicy, name: &str) -> &'a MacroState { + &policy + .macros + .iter() + .find(|m| m.name == name) + .unwrap_or_else(|| panic!("no row for macro '{name}'")) + .state + } + + #[test] + fn all_none_enables_everything() { + let policy = resolve(globals(&["a", "b"]), None, None, None, None); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "b"), &MacroState::Enabled); + } + + #[test] + fn global_empty_list_disables_all_as_runtime() { + let l = list(&[]); + let policy = resolve(globals(&["a", "b"]), None, None, None, Some(&l)); + assert_eq!(state_of(&policy, "a"), &MacroState::DisabledRuntime); + assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); + } + + #[test] + fn global_populated_partitions_enabled_and_disabled_runtime() { + let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), None, None, None, Some(&l)); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); + } + + #[test] + fn role_populated_locks_excluded_at_role_level() { + let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), None, None, Some(&l), None); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + } + + #[test] + fn agent_populated_locks_excluded_at_agent_level() { + let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), None, Some(&l), None, None); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Agent + } + ); + } + + #[test] + fn session_populated_locks_excluded_at_session_level() { + let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), Some(&l), None, None, None); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn role_empty_list_locks_everything_at_role_level() { + let l = list(&[]); + let policy = resolve(globals(&["a"]), None, None, Some(&l), None); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + } + + #[test] + fn agent_empty_list_locks_everything_at_agent_level() { + let l = list(&[]); + let policy = resolve(globals(&["a"]), None, Some(&l), None, None); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Agent + } + ); + } + + #[test] + fn session_empty_list_locks_everything_at_session_level() { + let l = list(&[]); + let policy = resolve(globals(&["a"]), Some(&l), None, None, None); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn session_wins_over_agent() { + let session = list(&["a"]); + let agent = list(&["b"]); + let policy = resolve( + globals(&["a", "b"]), + Some(&session), + Some(&agent), + None, + None, + ); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn agent_wins_over_role() { + let agent = list(&["a"]); + let role = list(&["b"]); + let policy = resolve(globals(&["a", "b"]), None, Some(&agent), Some(&role), None); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Agent + } + ); + } + + #[test] + fn role_wins_over_global() { + let role = list(&["a"]); + let global = list(&["b"]); + let policy = resolve(globals(&["a", "b"]), None, None, Some(&role), Some(&global)); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + } + + #[test] + fn empty_list_at_session_beats_populated_global() { + let session = list(&[]); + let global = list(&["a"]); + let policy = resolve(globals(&["a"]), Some(&session), None, None, Some(&global)); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn unknown_allowlist_name_yields_missing_row_without_error() { + let l = list(&["a", "ghost"]); + let policy = resolve(globals(&["a"]), None, None, None, Some(&l)); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "ghost"), &MacroState::Missing); + let ghost = policy.macros.iter().find(|m| m.name == "ghost").unwrap(); + assert_eq!(ghost.source, None); + } + + #[test] + fn missing_row_deduplicated_for_repeated_allowlist_names() { + let l = list(&["ghost", "ghost"]); + let policy = resolve(vec![], None, None, None, Some(&l)); + assert_eq!(policy.macros.len(), 1); + assert_eq!(state_of(&policy, "ghost"), &MacroState::Missing); + } + + #[test] + fn no_missing_rows_without_an_allowlist() { + let policy = resolve(globals(&["a"]), None, None, None, None); + assert_eq!(policy.macros.len(), 1); + } + + #[test] + fn builtin_name_collision_is_shadowed() { + let policy = + MacroPolicy::effective_with(globals(&["help", "a"]), None, None, None, None, &["help"]); + assert_eq!(state_of(&policy, "help"), &MacroState::ShadowedBuiltin); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + } + + #[test] + fn locked_wins_over_shadowed_builtin() { + let l = list(&["a"]); + let policy = MacroPolicy::effective_with( + globals(&["help", "a"]), + Some(&l), + None, + None, + None, + &["help"], + ); + assert_eq!( + state_of(&policy, "help"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + } + + #[test] + fn allowlisted_builtin_collision_stays_shadowed() { + let l = list(&["help"]); + let policy = + MacroPolicy::effective_with(globals(&["help"]), None, None, None, Some(&l), &["help"]); + assert_eq!(state_of(&policy, "help"), &MacroState::ShadowedBuiltin); + } + + #[test] + fn reserved_names_are_invalid() { + let policy = resolve(globals(&["enable", "disable"]), None, None, None, None); + for name in RESERVED_MACRO_NAMES { + assert_eq!( + state_of(&policy, name), + &MacroState::Invalid { + reason: format!("'{name}' is a reserved macro name") + } + ); + } + } + + #[test] + fn reserved_name_invalid_even_when_allowlisted() { + let l = list(&["enable"]); + let policy = resolve(globals(&["enable"]), Some(&l), None, None, None); + assert_eq!( + state_of(&policy, "enable"), + &MacroState::Invalid { + reason: "'enable' is a reserved macro name".to_string() + } + ); + } + + #[test] + fn reserved_name_invalid_wins_over_builtin_collision() { + let policy = + MacroPolicy::effective_with(globals(&["enable"]), None, None, None, None, &["enable"]); + assert_eq!( + state_of(&policy, "enable"), + &MacroState::Invalid { + reason: "'enable' is a reserved macro name".to_string() + } + ); + } + + #[test] + fn parse_failure_is_invalid() { + let policy = resolve(vec![disc_invalid("bad", "boom")], None, None, None, None); + assert_eq!( + state_of(&policy, "bad"), + &MacroState::Invalid { + reason: "boom".to_string() + } + ); + let bad = policy.macros.iter().find(|m| m.name == "bad").unwrap(); + assert_eq!(bad.description, None); + assert_eq!(bad.isolated, None); + } + + #[test] + fn invalid_wins_over_allowlist_exclusion() { + let l = list(&["other"]); + let policy = resolve( + vec![disc_invalid("bad", "boom")], + Some(&l), + None, + None, + None, + ); + assert_eq!( + state_of(&policy, "bad"), + &MacroState::Invalid { + reason: "boom".to_string() + } + ); + } + + #[test] + fn workspace_shadowing_keeps_both_rows_and_find_returns_workspace() { + let discovered = vec![ + disc("a", MacroSource::Workspace), + DiscoveredMacro { + shadowed_by_workspace: true, + ..disc("a", MacroSource::Global) + }, + ]; + let policy = resolve(discovered, None, None, None, None); + assert_eq!(policy.macros.len(), 2); + assert_eq!(policy.macros[0].source, Some(MacroSource::Workspace)); + assert!(!policy.macros[0].shadowed_by_workspace); + assert_eq!(policy.macros[1].source, Some(MacroSource::Global)); + assert!(policy.macros[1].shadowed_by_workspace); + let found = policy.find("a").unwrap(); + assert_eq!(found.source, Some(MacroSource::Workspace)); + } + + #[test] + fn find_skips_missing_rows() { + let l = list(&["ghost"]); + let policy = resolve(vec![], None, None, None, Some(&l)); + assert!(policy.find("ghost").is_none()); + } + + #[test] + fn rows_are_sorted_by_name() { + let policy = resolve(globals(&["c", "a", "b"]), None, None, None, None); + let names: Vec<&str> = policy.macros.iter().map(|m| m.name.as_str()).collect(); + assert_eq!(names, vec!["a", "b", "c"]); + } + + #[test] + fn resolved_rows_carry_description_and_isolated() { + let policy = resolve(globals(&["a"]), None, None, None, None); + let row = policy.macros.first().unwrap(); + assert_eq!(row.description.as_deref(), Some("a test macro")); + assert_eq!(row.isolated, Some(true)); + } + + #[test] + fn is_invocable_only_for_enabled_and_shadowed() { + assert!(MacroState::Enabled.is_invocable()); + assert!(MacroState::ShadowedBuiltin.is_invocable()); + assert!(!MacroState::DisabledRuntime.is_invocable()); + assert!( + !MacroState::Locked { + level: MacroAllowlistLevel::Role + } + .is_invocable() + ); + assert!(!MacroState::Missing.is_invocable()); + assert!( + !MacroState::Invalid { + reason: "x".to_string() + } + .is_invocable() + ); + } + + #[test] + fn level_and_source_display() { + assert_eq!(MacroAllowlistLevel::Session.to_string(), "session"); + assert_eq!(MacroAllowlistLevel::Agent.to_string(), "agent"); + assert_eq!(MacroAllowlistLevel::Role.to_string(), "role"); + assert_eq!(MacroAllowlistLevel::Global.to_string(), "global"); + assert_eq!(MacroSource::Workspace.to_string(), "workspace"); + assert_eq!(MacroSource::Global.to_string(), "global"); + } + + fn with_macro_dirs(f: F) { + let unique = time::SystemTime::now() + .duration_since(time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let root = env::temp_dir().join(format!("coyote-macro-policy-test-{unique}")); + let workspace = root.join("workspace-macros"); + let global = root.join("global-macros"); + fs::create_dir_all(&workspace).unwrap(); + fs::create_dir_all(&global).unwrap(); + f(&workspace, &global); + let _ = fs::remove_dir_all(&root); + } + + fn write_macro(dir: &Path, name: &str, content: &str) { + fs::write(dir.join(format!("{name}.yaml")), content).unwrap(); + } + + const VALID_YAML: &str = "steps:\n - \".help\"\n"; + + #[test] + fn discovery_scans_workspace_then_global_with_shadowing() { + with_macro_dirs(|workspace, global| { + write_macro(workspace, "both", VALID_YAML); + write_macro(workspace, "ws-only", VALID_YAML); + write_macro(global, "both", VALID_YAML); + write_macro(global, "global-only", VALID_YAML); + + let discovered = discover_macros_in(&[ + (MacroSource::Workspace, workspace.to_path_buf()), + (MacroSource::Global, global.to_path_buf()), + ]); + + assert_eq!(discovered.len(), 4); + let both_ws = discovered + .iter() + .find(|d| d.name == "both" && d.source == MacroSource::Workspace) + .unwrap(); + assert!(!both_ws.shadowed_by_workspace); + let both_global = discovered + .iter() + .find(|d| d.name == "both" && d.source == MacroSource::Global) + .unwrap(); + assert!(both_global.shadowed_by_workspace); + let global_only = discovered.iter().find(|d| d.name == "global-only").unwrap(); + assert!(!global_only.shadowed_by_workspace); + }); + } + + #[test] + fn discovery_ignores_non_yaml_files_and_directories() { + with_macro_dirs(|_, global| { + write_macro(global, "good", VALID_YAML); + fs::write(global.join("notes.txt"), "not a macro").unwrap(); + fs::write(global.join(".yaml"), VALID_YAML).unwrap(); + fs::create_dir_all(global.join("subdir.yaml")).unwrap(); + + let discovered = discover_macros_in(&[(MacroSource::Global, global.to_path_buf())]); + + assert_eq!(discovered.len(), 1); + assert_eq!(discovered[0].name, "good"); + }); + } + + #[test] + fn discovery_records_parse_failures() { + with_macro_dirs(|_, global| { + write_macro(global, "broken", "steps: {not valid"); + + let discovered = discover_macros_in(&[(MacroSource::Global, global.to_path_buf())]); + + assert_eq!(discovered.len(), 1); + assert!(discovered[0].definition.is_err()); + }); + } + + #[test] + fn discovery_of_nonexistent_dirs_is_empty() { + let discovered = discover_macros_in(&[( + MacroSource::Global, + PathBuf::from("/nonexistent/coyote-macro-policy-test"), + )]); + assert!(discovered.is_empty()); + } + + fn with_macro_dir_envs(workspace: &Path, global: &Path, f: F) { + let ws_env = get_env_name("workspace_config_dir"); + let global_env = get_env_name("macros_dir"); + let prev_ws = env::var_os(&ws_env); + let prev_global = env::var_os(&global_env); + unsafe { + env::set_var(&ws_env, workspace); + env::set_var(&global_env, global); + } + f(); + unsafe { + match prev_ws { + Some(v) => env::set_var(&ws_env, v), + None => env::remove_var(&ws_env), + } + match prev_global { + Some(v) => env::set_var(&global_env, v), + None => env::remove_var(&global_env), + } + } + } + + #[test] + #[serial] + fn discover_macros_honors_no_workspace_macros() { + with_macro_dirs(|workspace_root, global| { + let macros_subdir = workspace_root.join("macros"); + fs::create_dir_all(¯os_subdir).unwrap(); + write_macro(¯os_subdir, "ws-macro", VALID_YAML); + write_macro(global, "global-macro", VALID_YAML); + + with_macro_dir_envs(workspace_root, global, || { + let with_workspace = discover_macros(false); + let names: Vec<&str> = with_workspace.iter().map(|d| d.name.as_str()).collect(); + assert!(names.contains(&"ws-macro")); + assert!(names.contains(&"global-macro")); + + let without_workspace = discover_macros(true); + let names: Vec<&str> = without_workspace.iter().map(|d| d.name.as_str()).collect(); + assert!(!names.contains(&"ws-macro")); + assert!(names.contains(&"global-macro")); + }); + }); + } + + #[test] + #[serial] + fn effective_resolves_role_session_and_global_levels() { + with_macro_dirs(|workspace_root, global_dir| { + write_macro(global_dir, "a", VALID_YAML); + write_macro(global_dir, "b", VALID_YAML); + + with_macro_dir_envs(workspace_root, global_dir, || { + let global = AppConfig { + enabled_macros: Some(vec!["a".to_string()]), + ..AppConfig::default() + }; + + let policy = MacroPolicy::effective(&global, None, None, None, &[], false); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); + + let role = Role::new("test", "---\nenabled_macros: b\n---\nbody"); + let policy = MacroPolicy::effective(&global, Some(&role), None, None, &[], false); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + assert_eq!(state_of(&policy, "b"), &MacroState::Enabled); + + let session: Session = serde_yaml::from_str( + "model: provider:test\nenabled_macros: \"a\"\nmessages: []", + ) + .unwrap(); + let policy = + MacroPolicy::effective(&global, Some(&role), None, Some(&session), &[], false); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); + assert_eq!( + state_of(&policy, "b"), + &MacroState::Locked { + level: MacroAllowlistLevel::Session + } + ); + }); + }); + } + + #[test] + #[serial] + fn effective_pins_empty_string_role_allowlist_as_explicit_zero() { + with_macro_dirs(|workspace_root, global_dir| { + write_macro(global_dir, "a", VALID_YAML); + + with_macro_dir_envs(workspace_root, global_dir, || { + let global = AppConfig { + enabled_macros: Some(vec!["a".to_string()]), + ..AppConfig::default() + }; + let role = Role::new("test", "---\nenabled_macros: \"\"\n---\nbody"); + + let policy = MacroPolicy::effective(&global, Some(&role), None, None, &[], false); + assert_eq!( + state_of(&policy, "a"), + &MacroState::Locked { + level: MacroAllowlistLevel::Role + } + ); + }); + }); + } +} diff --git a/src/config/mod.rs b/src/config/mod.rs index ad25b6e..bb0c408 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -4,6 +4,7 @@ mod app_state; mod input; mod install_remote; pub(crate) mod instructions; +mod macro_policy; mod macros; mod mcp_factory; pub(crate) mod memory; @@ -31,6 +32,11 @@ pub use self::app_state::AppState; pub use self::input::Input; pub use self::install_remote::{install_remote, install_remote_from_repl_args}; #[allow(unused_imports)] +pub use self::macro_policy::{ + DiscoveredMacro, MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, + RESERVED_MACRO_NAMES, ResolvedMacro, discover_macros, +}; +#[allow(unused_imports)] pub use self::request_context::{RenderMode, RequestContext, should_inject_skill_instructions}; pub use self::role::{ CODE_ROLE, CREATE_TITLE_ROLE, EXPLAIN_SHELL_ROLE, Role, RoleLike, SHELL_ROLE, diff --git a/src/config/paths.rs b/src/config/paths.rs index 5beeeb7..10fe403 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -214,6 +214,10 @@ pub fn workspace_skill_file(name: &str) -> PathBuf { workspace_skills_dir().join(name).join("SKILL.md") } +pub fn workspace_macros_dir() -> PathBuf { + workspace_config_dir().join(MACROS_DIR_NAME) +} + pub fn workspace_mcp_config_file() -> Option { workspace_mcp_config_file_in(&env::current_dir().unwrap_or_default()) } diff --git a/src/config/role.rs b/src/config/role.rs index 69281f8..213f848 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -368,6 +368,10 @@ impl Role { self.enabled_skills.as_deref() } + pub fn enabled_macros(&self) -> Option<&[String]> { + self.enabled_macros.as_deref() + } + pub fn append_to_prompt(&mut self, text: &str) { self.prompt.push_str(text); } diff --git a/src/config/session.rs b/src/config/session.rs index cd7f04e..59d334f 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -113,6 +113,10 @@ impl Session { self.enabled_skills.as_deref() } + pub fn enabled_macros(&self) -> Option<&[String]> { + self.enabled_macros.as_deref() + } + pub fn set_skills_enabled(&mut self, value: Option) { if self.skills_enabled != value { self.skills_enabled = value; From e94bd450cd69333d5a3ad13d696f053e0176bc16 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 16:07:34 -0600 Subject: [PATCH 05/18] feat: surface macros as first-class custom commands in the REPL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the invocation and management surfaces from plans/custom-commands-design.md §4 and §6: - Top-level dispatch: an enabled macro now runs as ". [args]" from the command catch-all; runtime-disabled macros point at ".macro enable ", locked macros name the owning config, and unknown commands keep the existing error verbatim - .macro enable|disable : runtime toggles over the in-memory global-level enabled_macros list (disable with no list materializes all-active-minus-name); toggles error when a role/agent/session allowlist owns the field - .set enabled_macros with workspace-then-global existence validation; .set key completion gains enabled_macros and the previously missing enabled_skills - Dynamic completion: enabled macros (with descriptions) join built-ins on "." without touching the static command registry; ".macro " lists invocable macros (incl. built-in-shadowed ones) plus the enable/disable subcommands; second-arg completion offers toggle-eligible names - .list macros: enriched table (name, source, isolated, state, description) covering every resolver state incl. missing and shadowed rows; .help gains a custom-commands section - Session info/render and sysinfo display enabled_macros; Macro::load resolves workspace-then-global; enable/disable rejected as macro names in the creator --- src/config/macro_policy.rs | 4 - src/config/macros.rs | 7 +- src/config/mod.rs | 4 +- src/config/paths.rs | 5 - src/config/request_context.rs | 445 +++++++++++++++++++++++++++++++--- src/config/session.rs | 7 + src/repl/completer.rs | 19 +- src/repl/mod.rs | 99 +++++++- 8 files changed, 537 insertions(+), 53 deletions(-) diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs index bee02e6..93f4c10 100644 --- a/src/config/macro_policy.rs +++ b/src/config/macro_policy.rs @@ -75,7 +75,6 @@ pub enum MacroState { } impl MacroState { - #[allow(dead_code)] pub fn is_invocable(&self) -> bool { matches!(self, MacroState::Enabled | MacroState::ShadowedBuiltin) } @@ -95,7 +94,6 @@ pub struct DiscoveredMacro { /// One row of the resolved macro set. Missing allowlist entries produce rows /// with `source: None`. #[derive(Debug, Clone)] -#[allow(dead_code)] pub struct ResolvedMacro { pub name: String, pub source: Option, @@ -116,7 +114,6 @@ pub struct MacroPolicy { } impl MacroPolicy { - #[allow(dead_code)] pub fn effective( global: &AppConfig, role: Option<&Role>, @@ -199,7 +196,6 @@ impl MacroPolicy { /// The invocation target for `name`: the workspace entry when one shadows /// a global entry, otherwise the single discovered entry. Missing rows /// are never returned. - #[allow(dead_code)] pub fn find(&self, name: &str) -> Option<&ResolvedMacro> { self.macros .iter() diff --git a/src/config/macros.rs b/src/config/macros.rs index 3a1ceed..bedceb9 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -82,7 +82,12 @@ pub struct Macro { impl Macro { pub fn load(name: &str) -> Result { - let path = paths::macro_file(name); + let workspace_path = paths::workspace_macros_dir().join(format!("{name}.yaml")); + let path = if workspace_path.exists() { + workspace_path + } else { + paths::macro_file(name) + }; let err = || format!("Failed to load macro '{name}' at '{}'", path.display()); let content = read_to_string(&path).with_context(err)?; let value: Macro = serde_yaml::from_str(&content).with_context(err)?; diff --git a/src/config/mod.rs b/src/config/mod.rs index bb0c408..f36cc99 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -31,10 +31,8 @@ pub use self::app_config::AppConfig; pub use self::app_state::AppState; pub use self::input::Input; pub use self::install_remote::{install_remote, install_remote_from_repl_args}; -#[allow(unused_imports)] pub use self::macro_policy::{ - DiscoveredMacro, MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, - RESERVED_MACRO_NAMES, ResolvedMacro, discover_macros, + MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, ResolvedMacro, }; #[allow(unused_imports)] pub use self::request_context::{RenderMode, RequestContext, should_inject_skill_instructions}; diff --git a/src/config/paths.rs b/src/config/paths.rs index 10fe403..6a0ecfe 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -474,11 +474,6 @@ pub fn list_macros() -> Vec { list_file_names(macros_dir(), ".yaml") } -pub fn has_macro(name: &str) -> bool { - let names = list_macros(); - names.contains(&name.to_string()) -} - pub fn list_skills() -> Vec { let mut names = Vec::new(); let mut seen = HashSet::new(); diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 606cc48..d75bcca 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -7,7 +7,8 @@ use super::todo::TodoList; use super::tool_scope::{McpRuntime, ToolScope}; use super::{ AGENTS_DIR_NAME, Agent, AgentVariables, AppConfig, AppState, AssetCategory, CREATE_TITLE_ROLE, - Input, InstallFilter, LEFT_PROMPT, LastMessage, MESSAGES_FILE_NAME, RIGHT_PROMPT, Role, + Input, InstallFilter, LEFT_PROMPT, LastMessage, MESSAGES_FILE_NAME, MacroAllowlistLevel, + MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, RIGHT_PROMPT, ResolvedMacro, Role, RoleLike, SESSIONS_DIR_NAME, SUMMARIZATION_PROMPT, SUMMARY_CONTEXT_PROMPT, StateFlags, TEMP_ROLE_NAME, TEMP_SESSION_NAME, WorkingMode, ensure_parent_exists, list_agents_with_descriptions, memory, paths, @@ -123,6 +124,104 @@ fn complete_skills_with_descriptions(names: Vec) -> Vec<(String, Option< .collect() } +/// Keys offered by `.set ` completion. `reasoning_effort` is appended at +/// completion time only when the current model supports reasoning levels. +const SET_COMPLETION_KEYS: [&str; 26] = [ + "auto_continue", + "continuation_prompt", + "temperature", + "top_p", + "enabled_macros", + "enabled_skills", + "enabled_tools", + "enabled_mcp_servers", + "inject_todo_instructions", + "inject_skill_instructions", + "skill_instructions", + "max_auto_continues", + "memory", + "save_session", + "compression_threshold", + "rag_reranker_model", + "rag_top_k", + "max_output_tokens", + "dry_run", + "function_calling_support", + "mcp_server_support", + "skills_enabled", + "stream", + "save", + "highlight", + "raw_markdown", +]; + +/// The new global-level `enabled_macros` list after toggling `name`, or +/// `None` when the toggle is a no-op (already in the requested state). +/// Disabling with no current list (all macros visible) materializes the list +/// as every active macro name minus `name`. +fn toggled_enabled_macros( + current: Option<&[String]>, + all_active: &[String], + name: &str, + enable: bool, +) -> Option> { + match (current, enable) { + (None, true) => None, + (Some(list), true) => { + if list.iter().any(|v| v == name) { + None + } else { + let mut list = list.to_vec(); + list.push(name.to_string()); + Some(list) + } + } + (None, false) => Some( + all_active + .iter() + .filter(|v| v.as_str() != name) + .cloned() + .collect(), + ), + (Some(list), false) => { + if list.iter().any(|v| v == name) { + Some( + list.iter() + .filter(|v| v.as_str() != name) + .cloned() + .collect(), + ) + } else { + None + } + } + } +} + +/// The `.list macros` state column for a resolved row. +fn macro_state_display( + row: &ResolvedMacro, + lock_owner: impl Fn(MacroAllowlistLevel) -> String, +) -> String { + match &row.state { + MacroState::Enabled => "enabled".to_string(), + MacroState::DisabledRuntime => "disabled (runtime)".to_string(), + MacroState::Locked { level } => format!("locked ({} enabled_macros)", lock_owner(*level)), + MacroState::Missing => "missing".to_string(), + MacroState::ShadowedBuiltin => "shadowed (built-in)".to_string(), + MacroState::Invalid { reason } => format!("invalid ({reason})"), + } +} + +/// The `.list macros` source column: where the definition file lives, or `-` +/// for allowlist entries with no installed file. +fn macro_source_display(source: Option) -> String { + match source { + Some(source) => source.to_string(), + None => "-".to_string(), + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum RenderMode { #[default] @@ -1577,6 +1676,10 @@ impl RequestContext { "enabled_skills", super::format_option_value(&role.enabled_skills().map(|v| v.join(","))), ), + ( + "enabled_macros", + super::format_option_value(&role.enabled_macros().map(|v| v.join(","))), + ), ( "max_output_tokens", role.model() @@ -2357,6 +2460,9 @@ impl RequestContext { } pub fn new_macro(&self, app: &AppConfig, name: &str) -> Result<()> { + if RESERVED_MACRO_NAMES.contains(&name) { + bail!("'{name}' is a reserved macro name"); + } if self.macro_flag { bail!("No macro"); } @@ -2374,12 +2480,150 @@ impl RequestContext { Ok(()) } + /// The resolved macro set for the active context (workspace + global + /// discovery, effective `enabled_macros` allowlist, built-in shadowing). + pub fn macro_policy(&self) -> MacroPolicy { + MacroPolicy::effective( + &self.app.config, + self.role.as_ref(), + self.agent.as_ref(), + self.session.as_ref(), + &crate::repl::builtin_command_names(), + false, + ) + } + + /// A human-readable name for the config level whose `enabled_macros` + /// allowlist restricts a macro, e.g. `agent:oracle` or `role:coder`. + pub fn macro_lock_owner(&self, level: MacroAllowlistLevel) -> String { + let name = match level { + MacroAllowlistLevel::Session => self.session.as_ref().map(|s| s.name()), + MacroAllowlistLevel::Agent => self.agent.as_ref().map(|a| a.name()), + MacroAllowlistLevel::Role => self.role.as_ref().map(|r| r.name()), + MacroAllowlistLevel::Global => return "global config".to_string(), + }; + match name { + Some(name) => format!("{level}:{name}"), + None => level.to_string(), + } + } + + /// Enables or disables a macro by editing the in-memory global-level + /// `enabled_macros` list. Errors when a role/agent/session allowlist is + /// active, since a global-level write would be silently shadowed. + pub fn macro_toggle(&mut self, name: &str, enable: bool) -> Result<()> { + let restricting_level = if self + .session + .as_ref() + .and_then(|s| s.enabled_macros()) + .is_some() + { + Some(MacroAllowlistLevel::Session) + } else if self + .agent + .as_ref() + .and_then(|a| a.enabled_macros()) + .is_some() + { + Some(MacroAllowlistLevel::Agent) + } else if self + .role + .as_ref() + .and_then(|r| r.enabled_macros()) + .is_some() + { + Some(MacroAllowlistLevel::Role) + } else { + None + }; + if let Some(level) = restricting_level { + bail!( + "Macro toggles are restricted by {} enabled_macros; edit enabled_macros there", + self.macro_lock_owner(level) + ); + } + + let policy = self.macro_policy(); + match policy.find(name).map(|row| &row.state) { + None => bail!("Unknown macro '{name}'"), + Some(MacroState::Invalid { reason }) => bail!("Macro '{name}' is invalid: {reason}"), + Some(_) => {} + } + let all_active: Vec = policy + .macros + .iter() + .filter(|row| { + row.source.is_some() + && !row.shadowed_by_workspace + && !matches!(row.state, MacroState::Missing | MacroState::Invalid { .. }) + }) + .map(|row| row.name.clone()) + .collect(); + + let action = if enable { "enabled" } else { "disabled" }; + match toggled_enabled_macros( + self.app.config.enabled_macros.as_deref(), + &all_active, + name, + enable, + ) { + Some(list) => { + self.update_app_config(|app| app.enabled_macros = Some(list)); + println!("Macro '{name}' {action}"); + } + None => println!("Macro '{name}' is already {action}"), + } + Ok(()) + } + + /// The macros offered by top-level `.` completion: enabled rows + /// only. Macros shadowed by a built-in command never appear here; they + /// stay reachable via `.macro `. + pub fn visible_macro_completions(&self) -> Vec<(String, Option)> { + self.macro_policy() + .macros + .into_iter() + .filter(|row| row.state == MacroState::Enabled && !row.shadowed_by_workspace) + .map(|row| (row.name, row.description)) + .collect() + } + pub fn list_assets(&self, kind: &str) -> Result<()> { match kind { "roles" => print_asset_names("roles", &paths::list_roles(true)), "sessions" => print_asset_names("sessions", &self.list_sessions()), "rags" => print_asset_names("RAGs", &paths::list_rags()), - "macros" => print_asset_names("macros", &paths::list_macros()), + "macros" => { + let policy = self.macro_policy(); + if policy.macros.is_empty() { + println!("No macros found."); + return Ok(()); + } + + println!("Macros:"); + let header = format!( + " {:<24} {:<10} {:<9} {:<40} {}", + "name", "source", "isolated", "state", "description" + ); + println!("{header}"); + for row in &policy.macros { + let source = macro_source_display(row.source); + let isolated = match row.isolated { + Some(true) => "yes", + Some(false) => "no", + None => "-", + }; + let state = macro_state_display(row, |level| self.macro_lock_owner(level)); + let description = row.description.as_deref().unwrap_or_default(); + let line = format!( + " {:<24} {:<10} {:<9} {:<40} {}", + row.name, source, isolated, state, description + ); + println!("{}", line.trim_end()); + } + + Ok(()) + } "agents" => { let entries = list_agents_with_descriptions(); if entries.is_empty() { @@ -2728,6 +2972,23 @@ impl RequestContext { } self.update_app_config(|app| app.enabled_skills = parsed.clone()); } + "enabled_macros" => { + let raw: Option = super::parse_value(value)?; + let parsed: Option> = raw.map(|s| super::csv_to_vec(&s)); + if let Some(names) = parsed.as_ref() { + let policy = self.macro_policy(); + for name in names { + if !policy + .macros + .iter() + .any(|m| m.source.is_some() && &m.name == name) + { + bail!("macro '{name}' is not installed"); + } + } + } + self.update_app_config(|app| app.enabled_macros = parsed.clone()); + } "skills_enabled" => { let value: Option = super::parse_value(value)?; if let Some(session) = self.session.as_mut() { @@ -3001,7 +3262,28 @@ impl RequestContext { values.push("remote".to_string()); super::map_completion_values(values) } - ".macro" => super::map_completion_values(paths::list_macros()), + ".macro" => { + let policy = self.macro_policy(); + let mut values: Vec<(String, Option)> = policy + .macros + .iter() + .filter(|row| { + row.source.is_some() + && !row.shadowed_by_workspace + && row.state.is_invocable() + }) + .map(|row| (row.name.clone(), row.description.clone())) + .collect(); + values.push(( + "enable ".to_string(), + Some("Re-enable a runtime-disabled macro".to_string()), + )); + values.push(( + "disable ".to_string(), + Some("Disable a macro for the rest of this process".to_string()), + )); + values + } ".reasoning" => { let levels = self.current_model().reasoning_levels(); levels.iter().map(|v| (v.clone(), None)).collect() @@ -3016,32 +3298,7 @@ impl RequestContext { None => vec![], }, ".set" => { - let mut values = vec![ - "auto_continue", - "continuation_prompt", - "temperature", - "top_p", - "enabled_tools", - "enabled_mcp_servers", - "inject_todo_instructions", - "inject_skill_instructions", - "skill_instructions", - "max_auto_continues", - "memory", - "save_session", - "compression_threshold", - "rag_reranker_model", - "rag_top_k", - "max_output_tokens", - "dry_run", - "function_calling_support", - "mcp_server_support", - "skills_enabled", - "stream", - "save", - "highlight", - "raw_markdown", - ]; + let mut values = SET_COMPLETION_KEYS.to_vec(); if !self.current_model().reasoning_levels().is_empty() { values.push("reasoning_effort"); } @@ -3185,6 +3442,25 @@ impl RequestContext { .collect() }; values = super::map_completion_values(candidates); + } else if cmd == ".macro" + && (args.first() == Some(&"enable") || args.first() == Some(&"disable")) + && args.len() == 2 + { + let enable = args.first() == Some(&"enable"); + values = self + .macro_policy() + .macros + .into_iter() + .filter(|row| row.source.is_some() && !row.shadowed_by_workspace) + .filter(|row| { + if enable { + row.state == MacroState::DisabledRuntime + } else { + row.state.is_invocable() + } + }) + .map(|row| (row.name, row.description)) + .collect(); } else if (cmd == ".edit" && args.first() == Some(&"skill") && args.len() == 2) || (cmd == ".skill" && args.first() == Some(&"load") && args.len() == 2) { @@ -6463,4 +6739,115 @@ mod tests { "install_mcp_config must add new bundled servers" ); } + + fn strings(names: &[&str]) -> Vec { + names.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn toggled_enabled_macros_covers_all_transitions() { + let all_active = strings(&["a", "b", "c"]); + type ToggleCase = (Option>, &'static str, bool, Option>); + let cases: Vec = vec![ + (None, "a", true, None), + (Some(strings(&["a"])), "a", true, None), + (Some(strings(&["a"])), "b", true, Some(strings(&["a", "b"]))), + (None, "b", false, Some(strings(&["a", "c"]))), + ( + Some(strings(&["a", "b"])), + "b", + false, + Some(strings(&["a"])), + ), + (Some(strings(&["a"])), "b", false, None), + ]; + for (current, name, enable, expected) in cases { + let result = toggled_enabled_macros(current.as_deref(), &all_active, name, enable); + assert_eq!( + result, expected, + "current={current:?} name={name} enable={enable}" + ); + } + } + + fn resolved(state: MacroState) -> ResolvedMacro { + ResolvedMacro { + name: "m".to_string(), + source: Some(MacroSource::Global), + description: None, + isolated: None, + shadowed_by_workspace: false, + state, + } + } + + #[test] + fn macro_state_display_covers_all_states() { + let owner = |level: MacroAllowlistLevel| format!("{level}:test"); + let cases = vec![ + (MacroState::Enabled, "enabled"), + (MacroState::DisabledRuntime, "disabled (runtime)"), + ( + MacroState::Locked { + level: MacroAllowlistLevel::Agent, + }, + "locked (agent:test enabled_macros)", + ), + (MacroState::Missing, "missing"), + (MacroState::ShadowedBuiltin, "shadowed (built-in)"), + ( + MacroState::Invalid { + reason: "boom".to_string(), + }, + "invalid (boom)", + ), + ]; + for (state, expected) in cases { + assert_eq!(macro_state_display(&resolved(state), owner), expected); + } + } + + #[test] + fn macro_source_display_names_source_or_dash() { + assert_eq!( + macro_source_display(Some(MacroSource::Workspace)), + "workspace" + ); + assert_eq!(macro_source_display(Some(MacroSource::Global)), "global"); + assert_eq!(macro_source_display(None), "-"); + } + + #[test] + fn set_completion_keys_include_enabled_skills_and_macros() { + assert!(SET_COMPLETION_KEYS.contains(&"enabled_skills")); + assert!(SET_COMPLETION_KEYS.contains(&"enabled_macros")); + } + + #[test] + fn new_macro_rejects_reserved_names() { + let ctx = create_test_ctx(); + let app = ctx.app.config.clone(); + for name in RESERVED_MACRO_NAMES { + let err = ctx.new_macro(&app, name).unwrap_err(); + assert_eq!( + err.to_string(), + format!("'{name}' is a reserved macro name") + ); + } + } + + #[test] + fn macro_lock_owner_names_the_owning_config() { + let mut ctx = create_test_ctx(); + assert_eq!(ctx.macro_lock_owner(MacroAllowlistLevel::Role), "role"); + ctx.role = Some(Role::new("coder", "prompt")); + assert_eq!( + ctx.macro_lock_owner(MacroAllowlistLevel::Role), + "role:coder" + ); + assert_eq!( + ctx.macro_lock_owner(MacroAllowlistLevel::Global), + "global config" + ); + } } diff --git a/src/config/session.rs b/src/config/session.rs index 59d334f..728b100 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -246,6 +246,9 @@ impl Session { if let Some(enabled_skills) = self.enabled_skills() { data["enabled_skills"] = json!(enabled_skills); } + if let Some(enabled_macros) = self.enabled_macros() { + data["enabled_macros"] = json!(enabled_macros); + } if let Some(save_session) = self.save_session() { data["save_session"] = save_session.into(); } @@ -325,6 +328,10 @@ impl Session { items.push(("enabled_skills", enabled_skills.join(","))); } + if let Some(enabled_macros) = self.enabled_macros() { + items.push(("enabled_macros", enabled_macros.join(","))); + } + if let Some(save_session) = self.save_session() { items.push(("save_session", save_session.to_string())); } diff --git a/src/repl/completer.rs b/src/repl/completer.rs index 880c789..17b4825 100644 --- a/src/repl/completer.rs +++ b/src/repl/completer.rs @@ -74,7 +74,24 @@ impl Completer for ReplCompleter { format!("{name} ") }; create_suggestion(&name, description, span) - })) + })); + + let macros: Vec<(String, Option)> = ctx + .visible_macro_completions() + .into_iter() + .map(|(name, description)| (format!(".{name}"), description)) + .filter(|(name, _)| { + command_filter.len() == 1 || name.starts_with(&command_filter[..2]) + }) + .collect(); + let macros = fuzzy_filter(macros, |(name, _)| name.as_str(), &command_filter); + suggestions.extend(macros.iter().map(|(name, description)| { + create_suggestion( + &format!("{name} "), + description.as_deref().unwrap_or_default(), + span, + ) + })); } suggestions } diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 47e95c0..21bd991 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -12,8 +12,8 @@ use crate::client::{ oauth, }; use crate::config::{ - AgentVariables, AppConfig, AssertState, Input, LastMessage, RequestContext, StateFlags, - macro_execute, + AgentVariables, AppConfig, AssertState, Input, LastMessage, MacroState, RequestContext, + StateFlags, macro_execute, }; use crate::config::{AssetCategory, paths}; use crate::function::supervisor::{GuardrailAction, check_pending_agents_guardrail}; @@ -1073,15 +1073,44 @@ pub async fn run_repl_command( } }, ".macro" => match split_first_arg(args) { - Some((name, extra)) => { - let app = Arc::clone(&ctx.app.config); - if !paths::has_macro(name) && extra.is_none() { - ctx.new_macro(app.as_ref(), name)?; - } else { - macro_execute(ctx, name, extra, abort_signal.clone()).await?; + Some((sub @ ("enable" | "disable"), rest)) => { + match rest.and_then(|v| v.split_whitespace().next()) { + Some(name) => ctx.macro_toggle(name, sub == "enable")?, + None => println!("Usage: .macro {sub} "), } } - None => println!("Usage: .macro ..."), + Some((name, extra)) => { + let policy = ctx.macro_policy(); + match policy.find(name).map(|row| &row.state) { + Some(state) if state.is_invocable() => { + macro_execute(ctx, name, extra, abort_signal.clone()).await?; + } + Some(MacroState::DisabledRuntime) => bail!( + r#"Macro '{name}' is disabled. Re-enable it with ".macro enable {name}""# + ), + Some(MacroState::Locked { level }) => bail!( + "Macro '{name}' is restricted by {} enabled_macros", + ctx.macro_lock_owner(*level) + ), + Some(MacroState::Invalid { reason }) => { + bail!("Macro '{name}' is invalid: {reason}") + } + Some(_) | None => { + if extra.is_none() { + let app = Arc::clone(&ctx.app.config); + ctx.new_macro(app.as_ref(), name)?; + } else { + macro_execute(ctx, name, extra, abort_signal.clone()).await?; + } + } + } + } + None => println!( + r#"Usage: + .macro [text]... # Execute a macro + .macro enable # Re-enable a runtime-disabled macro + .macro disable # Disable a macro for the rest of this process"# + ), }, ".file" => match args { Some(args) => { @@ -1294,7 +1323,23 @@ pub async fn run_repl_command( println!("Usage: .vault [name]") } }, - _ => unknown_command()?, + _ => { + let name = cmd.strip_prefix('.').unwrap_or(cmd); + let policy = ctx.macro_policy(); + match policy.find(name).map(|row| &row.state) { + Some(MacroState::Enabled) => { + macro_execute(ctx, name, args, abort_signal.clone()).await?; + } + Some(MacroState::DisabledRuntime) => bail!( + r#"Macro '{name}' is disabled. Re-enable it with ".macro enable {name}""# + ), + Some(MacroState::Locked { level }) => bail!( + "Macro '{name}' is restricted by {} enabled_macros", + ctx.macro_lock_owner(*level) + ), + _ => unknown_command()?, + } + } }, None => { if let Some(cmd) = try_extract_shell_command(line) { @@ -1522,6 +1567,20 @@ fn unknown_command() -> Result<()> { bail!(r#"Unknown command. Type ".help" for additional help."#); } +/// The name of every built-in REPL command (first word, without the leading +/// dot), sorted and deduplicated. Macros with one of these names are shadowed +/// by the built-in and stay reachable only via `.macro `. +pub fn builtin_command_names() -> Vec<&'static str> { + let mut names: Vec<&'static str> = REPL_COMMANDS + .iter() + .filter_map(|cmd| cmd.name.split_whitespace().next()) + .filter_map(|name| name.strip_prefix('.')) + .collect(); + names.sort_unstable(); + names.dedup(); + names +} + fn dump_repl_help() { let head = REPL_COMMANDS .iter() @@ -1532,6 +1591,10 @@ fn dump_repl_help() { r###"{head} {:<24} Run an arbitrary shell command (stdout/stderr stream to your terminal; Ctrl+C interrupts) +Custom commands (macros): macros are coyote's custom commands. An enabled +macro runs top-level as . [args...], equivalent to ".macro ". +List them with ".list macros"; toggle them with ".macro enable|disable ". + Type ::: to start multi-line editing, type ::: to finish it. Press Ctrl+O to open an editor for editing the input buffer. Press Ctrl+C to cancel the response, Ctrl+D to exit the REPL."###, @@ -1732,6 +1795,22 @@ mod tests { assert_eq!(REPL_COMMANDS.len(), 60); } + #[test] + fn builtin_command_names_are_sorted_deduped_first_words_without_dots() { + let names = builtin_command_names(); + assert!(!names.is_empty()); + for name in &names { + assert!(!name.starts_with('.'), "'{name}' should not keep the dot"); + assert!(!name.contains(' '), "'{name}' should be a single word"); + } + assert!( + names.windows(2).all(|w| w[0] < w[1]), + "names should be sorted and deduplicated: {names:?}" + ); + assert!(names.contains(&"help")); + assert!(names.contains(&"macro")); + } + #[test] fn repl_commands_all_start_with_dot() { for cmd in REPL_COMMANDS.iter() { From 5478c5a239f485e3a476ed5e5bc110edf89655e1 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 16:28:06 -0600 Subject: [PATCH 06/18] feat: execute non-isolated macros on the live REPL context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the macro isolated field live (design §3, §9 step 5): isolated: false now runs the interpolated steps via run_repl_command on the live RequestContext — session-recorded, conversation-visible, with mutating steps persisting by design — while isolated: true keeps the forked execution byte-for-byte unchanged. - Add macro_non_isolated companion field beside macro_flag; both fork-propagation sites mirror it verbatim. - RAII MacroModeGuard wraps the whole &mut RequestContext (DerefMut passthrough) and restores flag+mode on every exit path, including a failing step; steps remain fail-fast. - Reject nested macro invocation when the current mode is non-isolated ("nested macros not allowed in non-isolated mode"); an isolated macro's step may still run a non-isolated macro inline on its fork. - use_agent now suppresses the agent's default session only for isolated macros; a non-isolated .agent step engages it as if typed. --- src/config/macros.rs | 349 +++++++++++++++++++++++++++++++++- src/config/request_context.rs | 88 ++++++++- 2 files changed, 435 insertions(+), 2 deletions(-) diff --git a/src/config/macros.rs b/src/config/macros.rs index bedceb9..ea981fc 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -2,12 +2,13 @@ use crate::config::paths; use crate::config::{RequestContext, RoleLike, ensure_parent_exists}; use crate::repl::{run_repl_command, split_args_text}; use crate::utils::{AbortSignal, multiline_text}; -use anyhow::{Context, Result, anyhow}; +use anyhow::{Context, Result, anyhow, bail}; use indexmap::IndexMap; use rust_embed::Embed; use serde::{Deserialize, Serialize}; use std::fs::{File, read_to_string}; use std::io::Write; +use std::ops::{Deref, DerefMut}; use std::sync::Arc; #[derive(Embed)] @@ -21,6 +22,9 @@ pub async fn macro_execute( args: Option<&str>, abort_signal: AbortSignal, ) -> Result<()> { + if ctx.in_non_isolated_macro() { + bail!("nested macros not allowed in non-isolated mode"); + } let macro_value = Macro::load(name)?; let (mut new_args, text) = split_args_text(args.unwrap_or_default(), cfg!(windows)); if !text.is_empty() { @@ -29,6 +33,17 @@ pub async fn macro_execute( let variables = macro_value .resolve_variables(&new_args) .map_err(|err| anyhow!("{err}. Usage: {}", macro_value.usage(name)))?; + + if !macro_value.isolated { + let mut live = MacroModeGuard::new(ctx); + for step in ¯o_value.steps { + let command = Macro::interpolate_command(step, &variables); + println!(">> {}", multiline_text(&command)); + run_repl_command(&mut live, abort_signal.clone(), &command).await?; + } + return Ok(()); + } + let role = ctx.extract_role(ctx.app.config.as_ref())?; let mut app_config = (*ctx.app.config).clone(); app_config.temperature = role.temperature(); @@ -69,6 +84,50 @@ pub async fn macro_execute( Ok(()) } +/// Marks a live context as executing a non-isolated macro for the duration of +/// its steps. Restores the previous flag and mode on drop, so every exit path +/// — including a failing step — leaves the context as it found it. +struct MacroModeGuard<'a> { + ctx: &'a mut RequestContext, + prev_flag: bool, + prev_non_isolated: bool, +} + +impl<'a> MacroModeGuard<'a> { + fn new(ctx: &'a mut RequestContext) -> Self { + let prev_flag = ctx.macro_flag; + let prev_non_isolated = ctx.macro_non_isolated; + ctx.macro_flag = true; + ctx.macro_non_isolated = true; + Self { + ctx, + prev_flag, + prev_non_isolated, + } + } +} + +impl Deref for MacroModeGuard<'_> { + type Target = RequestContext; + + fn deref(&self) -> &Self::Target { + self.ctx + } +} + +impl DerefMut for MacroModeGuard<'_> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.ctx + } +} + +impl Drop for MacroModeGuard<'_> { + fn drop(&mut self) { + self.ctx.macro_flag = self.prev_flag; + self.ctx.macro_non_isolated = self.prev_non_isolated; + } +} + #[derive(Debug, Clone, Deserialize, Serialize)] pub struct Macro { #[serde(default, skip_serializing_if = "Option::is_none")] @@ -187,6 +246,90 @@ fn default_true() -> bool { #[cfg(test)] mod tests { use super::*; + use crate::config::{AppState, Session, WorkingMode}; + use crate::utils::{create_abort_signal, get_env_name}; + use serial_test::serial; + use std::env; + use std::fs::{create_dir_all, remove_dir_all, write}; + use std::future::Future; + use std::path::PathBuf; + use std::time::{SystemTime, UNIX_EPOCH}; + + struct TestConfigDirGuard { + key: String, + previous: Option, + path: PathBuf, + } + + impl TestConfigDirGuard { + fn new() -> Self { + let key = get_env_name("config_dir"); + let previous = env::var_os(&key); + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let path = env::temp_dir().join(format!("coyote-macros-tests-{unique}")); + create_dir_all(&path).unwrap(); + unsafe { + env::set_var(&key, &path); + } + Self { + key, + previous, + path, + } + } + } + + impl Drop for TestConfigDirGuard { + fn drop(&mut self) { + if let Some(previous) = &self.previous { + unsafe { + env::set_var(&self.key, previous); + } + } else { + unsafe { + env::remove_var(&self.key); + } + } + let _ = remove_dir_all(&self.path); + } + } + + fn test_ctx() -> RequestContext { + RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd) + } + + fn write_macro_file(name: &str, content: &str) { + let path = paths::macros_dir().join(format!("{name}.yaml")); + ensure_parent_exists(&path).unwrap(); + write(&path, content).unwrap(); + } + + /// Drives a macro-execution future to completion on a thread with extra + /// stack headroom: nested `run_repl_command` poll frames are deep in + /// debug builds and overflow the 2 MiB default test-thread stack. + fn run_async(f: F) -> F::Output + where + F: Future + Send, + F::Output: Send, + { + std::thread::scope(|scope| { + std::thread::Builder::new() + .stack_size(8 * 1024 * 1024) + .spawn_scoped(scope, || { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(f) + }) + .unwrap() + .join() + .unwrap() + }) + } fn var(name: &str, rest: bool, default: Option<&str>) -> MacroVariable { MacroVariable { @@ -458,4 +601,208 @@ variables: assert!(!m.steps.is_empty(), "asset '{}'", file.as_ref()); } } + + #[test] + #[serial] + fn non_isolated_steps_run_on_live_ctx_and_mutations_persist() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "live-macro", + "isolated: false\nsteps:\n - \".set temperature 0.42\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + run_async(macro_execute( + &mut ctx, + "live-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert!(ctx.session.is_some(), "live session must survive the macro"); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + Some(0.42), + "the step must mutate the live context's session, not a fork" + ); + assert!(!ctx.macro_flag, "flag must be restored after success"); + assert!( + !ctx.macro_non_isolated, + "mode must be restored after success" + ); + } + + #[test] + #[serial] + fn non_isolated_step_failure_aborts_and_restores_flag_and_mode() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "fail-macro", + "isolated: false\nsteps:\n - \".set temperature 0.9\"\n - \".update\"\n - \".set temperature 0.1\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + let result = run_async(macro_execute( + &mut ctx, + "fail-macro", + None, + create_abort_signal(), + )); + + assert!(result.is_err(), "a failing step must abort the macro"); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + Some(0.9), + "completed steps' mutations persist; steps after the failure never run" + ); + assert!(!ctx.macro_flag, "flag must be restored on the error path"); + assert!( + !ctx.macro_non_isolated, + "mode must be restored on the error path" + ); + } + + #[test] + fn nested_macro_rejected_when_non_isolated_mode_active() { + let mut ctx = test_ctx(); + ctx.macro_flag = true; + ctx.macro_non_isolated = true; + + let result = run_async(macro_execute( + &mut ctx, + "anything", + None, + create_abort_signal(), + )); + + let err = result.unwrap_err().to_string(); + assert!( + err.contains("nested macros not allowed in non-isolated mode"), + "{err}" + ); + } + + #[test] + #[serial] + fn non_isolated_macro_step_invoking_macro_is_rejected() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "outer-macro", + "isolated: false\nsteps:\n - \".inner-macro\"\n", + ); + write_macro_file( + "inner-macro", + "isolated: false\nsteps:\n - \".set temperature 0.5\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + let result = run_async(macro_execute( + &mut ctx, + "outer-macro", + None, + create_abort_signal(), + )); + + let err = result.unwrap_err().to_string(); + assert!( + err.contains("nested macros not allowed in non-isolated mode"), + "{err}" + ); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + None, + "the nested macro's steps must not run" + ); + assert!(!ctx.macro_flag); + assert!(!ctx.macro_non_isolated); + } + + #[test] + #[serial] + fn isolated_macro_step_runs_non_isolated_macro_inline_on_fork() { + let _guard = TestConfigDirGuard::new(); + write_macro_file("iso-outer-macro", "steps:\n - \".inner-macro\"\n"); + write_macro_file( + "inner-macro", + "isolated: false\nsteps:\n - \".set temperature 0.33\"\n", + ); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + + run_async(macro_execute( + &mut ctx, + "iso-outer-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + None, + "the inline run happens on the fork, never on the live context" + ); + assert!(!ctx.macro_flag); + assert!(!ctx.macro_non_isolated); + } + + #[test] + #[serial] + fn isolated_macro_still_forks_and_leaves_live_ctx_untouched() { + let _guard = TestConfigDirGuard::new(); + write_macro_file("iso-macro", "steps:\n - \".set temperature 0.77\"\n"); + let mut ctx = test_ctx(); + ctx.session = Some(Session::default()); + let app_before = Arc::clone(&ctx.app.config); + + run_async(macro_execute( + &mut ctx, + "iso-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert!(ctx.session.is_some()); + assert_eq!( + ctx.session.as_ref().unwrap().temperature(), + None, + "an isolated macro's mutations must stay on the fork" + ); + assert!( + Arc::ptr_eq(&ctx.app.config, &app_before), + "isolated execution must not swap the live app config" + ); + assert!(!ctx.macro_flag); + } + + #[test] + #[serial] + fn guard_restores_prior_flag_values_after_inline_run() { + let _guard = TestConfigDirGuard::new(); + write_macro_file( + "inner-macro", + "isolated: false\nsteps:\n - \".set temperature 0.11\"\n", + ); + let mut ctx = test_ctx(); + ctx.macro_flag = true; + + run_async(macro_execute( + &mut ctx, + "inner-macro", + None, + create_abort_signal(), + )) + .unwrap(); + + assert!( + ctx.macro_flag, + "a pre-existing flag must be restored, not cleared" + ); + assert!(!ctx.macro_non_isolated); + } } diff --git a/src/config/request_context.rs b/src/config/request_context.rs index d75bcca..67b6ffa 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -233,6 +233,10 @@ pub struct RequestContext { pub app: Arc, pub macro_flag: bool, + /// Companion to `macro_flag`: true while a non-isolated macro is running + /// its steps directly on this live context. Isolated macros execute on a + /// forked context and leave this false. + pub macro_non_isolated: bool, pub info_flag: bool, pub working_mode: WorkingMode, @@ -270,6 +274,7 @@ impl RequestContext { Self { app, macro_flag: false, + macro_non_isolated: false, info_flag: false, working_mode, model: Default::default(), @@ -324,6 +329,7 @@ impl RequestContext { Ok(Self { app, macro_flag: false, + macro_non_isolated: false, info_flag, working_mode, model, @@ -373,6 +379,7 @@ impl RequestContext { Self { app: Arc::clone(&self.app), macro_flag: self.macro_flag, + macro_non_isolated: self.macro_non_isolated, info_flag: self.info_flag, working_mode: self.working_mode, model: self.model.clone(), @@ -412,6 +419,7 @@ impl RequestContext { Self { app, macro_flag: parent.macro_flag, + macro_non_isolated: parent.macro_non_isolated, info_flag: parent.info_flag, working_mode: WorkingMode::Cmd, model: parent.model.clone(), @@ -2480,6 +2488,14 @@ impl RequestContext { Ok(()) } + /// Whether a non-isolated macro is currently running its steps on this + /// context. Macro invocations are rejected in this mode: the nested + /// macro's steps would interleave with the outer macro's on the live + /// session. + pub fn in_non_isolated_macro(&self) -> bool { + self.macro_flag && self.macro_non_isolated + } + /// The resolved macro set for the active context (workspace + global /// discovery, effective `enabled_macros` allowlist, built-in shadowing). pub fn macro_policy(&self) -> MacroPolicy { @@ -4016,8 +4032,11 @@ impl RequestContext { // Graph agents manage their own state; never engage a session, // not even an inherited app-level `agent_session` default. + // Isolated macros suppress an inherited default too — their forked + // context has no session to return to. A non-isolated macro's `.agent` + // step engages it exactly as if the user had typed the command. let session_name = session_name.map(|v| v.to_string()).or_else(|| { - if self.macro_flag || is_graph_agent { + if (self.macro_flag && !self.macro_non_isolated) || is_graph_agent { None } else { agent.agent_session().map(|v| v.to_string()) @@ -6566,6 +6585,73 @@ mod tests { ); } + #[test] + #[serial] + fn use_agent_suppresses_inherited_session_in_isolated_macro() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + ctx.macro_flag = true; + ctx.update_app_config(|app| app.agent_session = Some("inherited".to_string())); + + let app = ctx.app.config.clone(); + let agent_name = format!( + "test_agent_{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let agent_dir = paths::agent_data_dir(&agent_name); + create_dir_all(&agent_dir).unwrap(); + write( + agent_dir.join("config.yaml"), + format!("name: {agent_name}\ninstructions: hi\n"), + ) + .unwrap(); + + let abort = utils::create_abort_signal(); + run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap(); + + assert!( + ctx.session.is_none(), + "an isolated macro must keep suppressing the agent's default session" + ); + } + + #[test] + #[serial] + fn use_agent_engages_inherited_session_in_non_isolated_macro() { + let _guard = TestConfigDirGuard::new(); + let mut ctx = create_test_ctx(); + ctx.macro_flag = true; + ctx.macro_non_isolated = true; + ctx.update_app_config(|app| app.agent_session = Some("inherited".to_string())); + + let app = ctx.app.config.clone(); + let agent_name = format!( + "test_agent_{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let agent_dir = paths::agent_data_dir(&agent_name); + create_dir_all(&agent_dir).unwrap(); + write( + agent_dir.join("config.yaml"), + format!("name: {agent_name}\ninstructions: hi\n"), + ) + .unwrap(); + + let abort = utils::create_abort_signal(); + run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap(); + + assert!( + ctx.session.is_some(), + "a non-isolated macro's agent step must engage the default session as if typed" + ); + } + fn first_file(dir: &Path) -> Option { for entry in read_dir(dir).ok()?.flatten() { let path = entry.path(); From 4323d4823c21c91526f13b988f9a1b2c17e007a4 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 16:38:00 -0600 Subject: [PATCH 07/18] feat: add --no-workspace-macros opt-out for workspace macro loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors --no-workspace-mcp exactly: a CLI-only flag backed by an AppConfig field (default false) that disables .coyote/macros in both the resolved macro policy and Macro::load's workspace-then-global preference, so the two always agree (custom-commands design §5). --- src/cli/mod.rs | 3 ++ src/config/app_config.rs | 3 ++ src/config/macro_policy.rs | 33 ++++++++++++++++ src/config/macros.rs | 71 +++++++++++++++++++++++++++++++++-- src/config/request_context.rs | 2 +- src/main.rs | 3 ++ 6 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 9f86023..9c4394d 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -96,6 +96,9 @@ pub struct Cli { /// Disable loading workspace MCP servers from .coyote/mcp.json, .coyote/.mcp.json, or .mcp.json #[arg(long)] pub no_workspace_mcp: bool, + /// Disable loading workspace macros from .coyote/macros + #[arg(long)] + pub no_workspace_macros: bool, /// Disable memory for this invocation #[arg(long)] pub no_memory: bool, diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 51a1855..5dd6b2d 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -98,6 +98,7 @@ pub struct AppConfig { pub user_agent: Option, pub save_shell_history: bool, pub no_workspace_mcp: bool, + pub no_workspace_macros: bool, pub sync_models_url: Option, pub clients: Vec, @@ -181,6 +182,7 @@ impl Default for AppConfig { user_agent: None, save_shell_history: true, no_workspace_mcp: false, + no_workspace_macros: false, sync_models_url: None, clients: vec![], @@ -266,6 +268,7 @@ impl AppConfig { user_agent: config.user_agent, save_shell_history: config.save_shell_history, no_workspace_mcp: false, + no_workspace_macros: false, sync_models_url: config.sync_models_url, clients: config.clients, diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs index 93f4c10..8dba677 100644 --- a/src/config/macro_policy.rs +++ b/src/config/macro_policy.rs @@ -855,6 +855,39 @@ mod tests { }); } + #[test] + #[serial] + fn effective_honors_no_workspace_macros() { + with_macro_dirs(|workspace_root, global| { + let macros_subdir = workspace_root.join("macros"); + fs::create_dir_all(¯os_subdir).unwrap(); + write_macro(¯os_subdir, "shared", VALID_YAML); + write_macro(¯os_subdir, "ws-only", VALID_YAML); + write_macro(global, "shared", VALID_YAML); + write_macro(global, "global-only", VALID_YAML); + + with_macro_dir_envs(workspace_root, global, || { + let config = AppConfig::default(); + + let policy = MacroPolicy::effective(&config, None, None, None, &[], false); + assert_eq!( + policy.find("shared").unwrap().source, + Some(MacroSource::Workspace) + ); + assert!(policy.find("ws-only").is_some()); + assert!(policy.find("global-only").is_some()); + + let policy = MacroPolicy::effective(&config, None, None, None, &[], true); + assert_eq!( + policy.find("shared").unwrap().source, + Some(MacroSource::Global) + ); + assert!(policy.find("ws-only").is_none()); + assert!(policy.find("global-only").is_some()); + }); + }); + } + #[test] #[serial] fn effective_resolves_role_session_and_global_levels() { diff --git a/src/config/macros.rs b/src/config/macros.rs index ea981fc..667a020 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -25,7 +25,7 @@ pub async fn macro_execute( if ctx.in_non_isolated_macro() { bail!("nested macros not allowed in non-isolated mode"); } - let macro_value = Macro::load(name)?; + let macro_value = Macro::load(name, ctx.app.config.no_workspace_macros)?; let (mut new_args, text) = split_args_text(args.unwrap_or_default(), cfg!(windows)); if !text.is_empty() { new_args.push(text.to_string()); @@ -140,9 +140,9 @@ pub struct Macro { } impl Macro { - pub fn load(name: &str) -> Result { + pub fn load(name: &str, no_workspace_macros: bool) -> Result { let workspace_path = paths::workspace_macros_dir().join(format!("{name}.yaml")); - let path = if workspace_path.exists() { + let path = if !no_workspace_macros && workspace_path.exists() { workspace_path } else { paths::macro_file(name) @@ -307,6 +307,71 @@ mod tests { write(&path, content).unwrap(); } + /// Sets up a temp workspace macros dir and a temp global macros dir, each + /// containing a `shared` macro whose `description` names its source, and + /// points the workspace/global dir env overrides at them for `f`. + fn with_macro_load_envs(f: F) { + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let root = env::temp_dir().join(format!("coyote-macro-load-tests-{unique}")); + let workspace_root = root.join("workspace"); + let workspace_macros = workspace_root.join("macros"); + let global = root.join("global"); + create_dir_all(&workspace_macros).unwrap(); + create_dir_all(&global).unwrap(); + write( + workspace_macros.join("shared.yaml"), + "description: workspace\nsteps:\n - \".help\"\n", + ) + .unwrap(); + write( + global.join("shared.yaml"), + "description: global\nsteps:\n - \".help\"\n", + ) + .unwrap(); + + let ws_env = get_env_name("workspace_config_dir"); + let global_env = get_env_name("macros_dir"); + let prev_ws = env::var_os(&ws_env); + let prev_global = env::var_os(&global_env); + unsafe { + env::set_var(&ws_env, &workspace_root); + env::set_var(&global_env, &global); + } + f(); + unsafe { + match prev_ws { + Some(v) => env::set_var(&ws_env, v), + None => env::remove_var(&ws_env), + } + match prev_global { + Some(v) => env::set_var(&global_env, v), + None => env::remove_var(&global_env), + } + } + let _ = remove_dir_all(&root); + } + + #[test] + #[serial] + fn load_prefers_workspace_over_global_by_default() { + with_macro_load_envs(|| { + let loaded = Macro::load("shared", false).unwrap(); + assert_eq!(loaded.description.as_deref(), Some("workspace")); + }); + } + + #[test] + #[serial] + fn load_skips_workspace_when_no_workspace_macros() { + with_macro_load_envs(|| { + let loaded = Macro::load("shared", true).unwrap(); + assert_eq!(loaded.description.as_deref(), Some("global")); + }); + } + /// Drives a macro-execution future to completion on a thread with extra /// stack headroom: nested `run_repl_command` poll frames are deep in /// debug builds and overflow the 2 MiB default test-thread stack. diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 67b6ffa..4762e9c 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -2505,7 +2505,7 @@ impl RequestContext { self.agent.as_ref(), self.session.as_ref(), &crate::repl::builtin_command_names(), - false, + self.app.config.no_workspace_macros, ) } diff --git a/src/main.rs b/src/main.rs index 937d255..68be6c7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -222,6 +222,9 @@ async fn main() -> Result<()> { if cli.no_workspace_mcp { app_config.no_workspace_mcp = true; } + if cli.no_workspace_macros { + app_config.no_workspace_macros = true; + } let app_config: Arc = Arc::new(app_config); let app_state: Arc = Arc::new( AppState::init( From 125360033d20039f8ea4c537020ff45f1641b782 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 16:40:56 -0600 Subject: [PATCH 08/18] docs(plans): record implementation-verified correction from T6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no_workspace_mcp (and therefore no_workspace_macros, per the exact-mirror ruling) is CLI-flag-only: AppConfig field exists but there is no Config-struct key and no env arm, so a config.yaml entry is non-functional. Pre-existing issue: config.example.yaml:140 documents the dead no_workspace_mcp key — recorded as follow-up, out of scope. --- plans/custom-commands-design.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/plans/custom-commands-design.md b/plans/custom-commands-design.md index c5a8054..afd4f1f 100644 --- a/plans/custom-commands-design.md +++ b/plans/custom-commands-design.md @@ -286,10 +286,14 @@ workspace-artifact conventions (VERIFIED mechanics): skills (`list_skills` iterates [workspace, global] with shadowing, paths.rs:478-501; `has_skill` checks workspace first, :503-505) and workspace MCP (HashMap insert, workspace wins, mcp/mod.rs:239). -- Opt-out mirrors MCP: new `--no-workspace-macros` CLI flag + - `no_workspace_macros` config key (default false), modeled on - `--no-workspace-mcp` (cli/mod.rs:96-98 → main.rs:222-223, - app_config.rs:98). +- Opt-out mirrors MCP: new `--no-workspace-macros` CLI flag, modeled on + `--no-workspace-mcp` (cli/mod.rs:96-98 → main.rs:222-223). + IMPLEMENTATION-VERIFIED CORRECTION (T6): `no_workspace_mcp` is + CLI-flag-only — AppConfig field exists but there is NO Config-struct key + and NO env arm, so a config.yaml entry is non-functional. The exact-mirror + ruling therefore makes `no_workspace_macros` CLI-flag-only too. + (Pre-existing bug, out of scope: config.example.yaml:140 documents + `no_workspace_mcp:` as a yaml key even though it does nothing — follow-up.) - Trust model: no confirmation prompt — consistent with workspace MCP and skills, which load with no gate (mcp/mod.rs:225-268 merely eprintlns); macros are strictly lower-risk since they run only on explicit user @@ -404,8 +408,9 @@ install-time warning if an installed macro's name shadows a built-in. invocation; `isolated` semantics with the role-switch-persists warning. - `config.example.yaml`, `config.role.example.md`, `config.agent.example.yaml`: `enabled_macros` entries mirroring the - existing `enabled_skills` doc comments; `no_workspace_macros` entry - alongside the existing `no_workspace_mcp` one (config.example.yaml:140-145). + existing `enabled_skills` doc comments. Do NOT add a `no_workspace_macros` + yaml entry — the flag is CLI-only (see §5 correction); the existing + `no_workspace_mcp` yaml line documents a dead key (pre-existing, follow-up). - New `macro.example.yaml` (or extend existing docs) showing all fields incl. description/isolated. - Workspace macros: document `.coyote/macros/` alongside the existing From 91328ca7e16c17e382eae25256e61202375dbdc9 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 20 Aug 2026 16:54:19 -0600 Subject: [PATCH 09/18] docs: document macros as first-class custom commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers top-level .name invocation, the new description/isolated macro fields (with the non-isolation persistence, fail-fast, nested-macro, and .exit caveats), workspace .coyote/macros/ + --no-workspace-macros, enabled_macros scoping at global/role/agent/session levels, and .macro enable|disable across the README and every example config. graph.example.yaml gains a note that enabled_macros is ignored in graph configs. CHANGELOG intentionally untouched: it is generated by commitizen at release time from the conventional commit subjects. Implements plans/custom-commands-design.md §8. --- README.md | 5 ++++- config.agent.example.yaml | 2 ++ config.example.yaml | 15 +++++++++++++++ config.macro.example.yaml | 10 ++++++++++ config.role.example.md | 3 +++ graph.example.yaml | 3 +++ 6 files changed, 37 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4143a81..b9b00df 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,10 @@ Coming from [AIChat](https://github.com/sigoden/aichat)? Follow the [migration g * [Create Custom Bash Tools](https://github.com/Dark-Alex-17/coyote/wiki/Custom-Bash-Tools) * [Bash Prompt Utilities](https://github.com/Dark-Alex-17/coyote/wiki/Bash-Prompt-Helpers) * [First-Class MCP Server Support](https://github.com/Dark-Alex-17/coyote/wiki/MCP-Servers): Easily connect and interact with MCP servers for advanced functionality. -* [Macros](https://github.com/Dark-Alex-17/coyote/wiki/Macros): Automate repetitive tasks and workflows with Coyote "scripts" (macros). +* [Macros](https://github.com/Dark-Alex-17/coyote/wiki/Macros): Automate repetitive tasks and workflows with Coyote "scripts" (macros). Macros are Coyote's custom commands: invoke any macro directly by name (e.g. `.review main`), with tab-completion, right alongside the built-in REPL commands. + * Give a macro a `description` (shown in `.list macros` and completions) and set `isolated: false` to run its steps on the live session, exactly as if you typed them. Note that non-isolated steps are recorded in the session, and mutating steps (`.role`, `.model`, ...) persist after the macro ends — by design. Steps are fail-fast: an error aborts the remaining steps, but completed steps' effects remain. A non-isolated macro step cannot invoke another macro, and a `.exit` step never exits the REPL. + * Commit project-specific macros to `.coyote/macros/` in your repo — they shadow same-named global macros (opt out with `--no-workspace-macros`). + * Scope which macros are invocable with `enabled_macros` in the global config, a role, an agent, or a session (most specific wins; an empty list disables all macros), and toggle at runtime with `.macro enable|disable `. * [RAG](https://github.com/Dark-Alex-17/coyote/wiki/RAG): Retrieval-Augmented Generation for enhanced information retrieval and generation. * [Sessions](https://github.com/Dark-Alex-17/coyote/wiki/Sessions): Manage and persist conversational contexts and settings across multiple interactions. * [Memory](https://github.com/Dark-Alex-17/coyote/wiki/Memory): Persistent file-based memory that survives across sessions. Bootstrap with `coyote --init-memory [global|workspace]`. diff --git a/config.agent.example.yaml b/config.agent.example.yaml index d7c6d5a..c4562df 100644 --- a/config.agent.example.yaml +++ b/config.agent.example.yaml @@ -60,6 +60,8 @@ enabled_skills: # Optional list of skills available when this a inject_skill_instructions: true # Inject a short hint pointing the model at `skill__list` when skills are enabled # (default: true). Suppressed automatically when no skills are available. skill_instructions: null # Custom text for the skill hint (optional; uses built-in default if null) +enabled_macros: # Optional list of macros invocable when this agent is active in the REPL. + - generate-commit-message # An empty list disables all macros. Omit to inherit the role/global default. memory: null # Per-agent memory override (default: inherit). Set to `false` to disable memory # for this agent regardless of workspace/global presence. See the Memory wiki page. diff --git a/config.example.yaml b/config.example.yaml index c93a509..247333a 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -169,6 +169,21 @@ inject_skill_instructions: true # Inject a short hint pointing the model at `s # effective enabled skill set is non-empty (default: true). skill_instructions: null # Custom text used for the skill hint when injected. If null, uses built-in default. +# ---- Macros ---- +# Macros are Coyote's custom commands: named sequences of REPL commands and prompts, invoked directly by name +# (a macro file named `review.yaml` runs as `.review [args]`; built-in commands always win a name collision). +# Workspace-local macros in `.coyote/macros/` shadow same-named global macros (skip them with --no-workspace-macros). +# See the [Macros documentation](https://github.com/Dark-Alex-17/coyote/wiki/Macros) for more details. +enabled_macros: null # Which macros are invocable by default (no role/agent/session active). null = all visible. + # An empty list means NO macros are invocable. Accepts either a YAML list or a + # comma-separated string. Roles, agents, and sessions may define their own + # `enabled_macros`; the most specific active one wins (session > agent > role > global). + # Example (list form): + # enabled_macros: + # - generate-commit-message + # Example (comma-separated form): + # enabled_macros: generate-commit-message,review + # ---- Auto-Continue (Todo System) ---- # The auto-continue system provides built-in task tracking for improved reliability. # When enabled, the model can create todo lists and the system will automatically diff --git a/config.macro.example.yaml b/config.macro.example.yaml index ba83d07..341e8b4 100644 --- a/config.macro.example.yaml +++ b/config.macro.example.yaml @@ -1,3 +1,13 @@ +description: Demonstrates every macro field # Optional; shown in `.list macros` and in `.` tab-completion. +isolated: true # Optional; 'true' by default. When true, steps run in a forked, + # throwaway context: the exchange and any `.role`/`.model` switches + # vanish when the macro ends. When false, steps run on the LIVE + # session exactly as if you typed them: prompts are recorded, and + # mutating steps (e.g. `.role`, `.model`) PERSIST after the macro + # finishes -- by design. Steps are fail-fast in both modes: an error + # aborts the remaining steps, but completed steps' effects remain. + # A non-isolated macro step cannot invoke another macro, and a + # `.exit` step never exits the REPL. variables: # A list of positional variables that the macro uses - name: positional_1 # The name of the positional variable. default: null # Since no default value is provided, this argument is required; 'null' by default diff --git a/config.role.example.md b/config.role.example.md index ff66eaa..3fabd63 100644 --- a/config.role.example.md +++ b/config.role.example.md @@ -24,6 +24,9 @@ enabled_skills: # Skills available when this role is activ inject_skill_instructions: true # Inject a short hint pointing the model at `skill__list` when skills are enabled # (default: true). Suppressed automatically when no skills are available. skill_instructions: null # Custom text for the skill hint (optional; uses built-in default if null) +enabled_macros: # Macros invocable when this role is active. Accepts a YAML list (preferred) + - generate-commit-message # or a comma-separated string (e.g. `enabled_macros: generate-commit-message,review`). + # An empty list disables all macros. Omit to inherit the global default. memory: null # Per-role memory override (default: inherit). Set to `false` to disable memory # when this role is active. See the Memory wiki page. diff --git a/graph.example.yaml b/graph.example.yaml index f1ea9ae..ece3cda 100644 --- a/graph.example.yaml +++ b/graph.example.yaml @@ -68,6 +68,9 @@ enabled_skills: inject_skill_instructions: true # Inject a hint pointing the model at `skill__list`. Defaults to true; suppressed # automatically when no skills are available. skill_instructions: null # Custom text for the skill hint (optional; uses the built-in default if omitted). +# Note: `enabled_macros` is NOT a graph setting. Graph nodes never dispatch REPL +# commands, so the field is silently ignored in graph configs; scope macros via +# the global config, roles, agents, or sessions instead. conversation_starters: # Suggested prompts surfaced in the UI - "Research the current state of WebAssembly outside the browser" From f61a8f7afdd3ced23edee90496085f17de487cc1 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 10:43:42 -0600 Subject: [PATCH 10/18] style: updated styles across macro implementation --- graph.example.yaml | 3 -- src/config/macro_policy.rs | 84 ++++++++++++++++++++++------------- src/config/macros.rs | 45 +++++++++++++++++-- src/config/mod.rs | 12 +++++ src/config/request_context.rs | 31 ++----------- src/config/role.rs | 29 ++++++++++++ src/config/session.rs | 15 +++++++ src/graph/types.rs | 2 + src/repl/mod.rs | 5 +-- 9 files changed, 158 insertions(+), 68 deletions(-) diff --git a/graph.example.yaml b/graph.example.yaml index ece3cda..f1ea9ae 100644 --- a/graph.example.yaml +++ b/graph.example.yaml @@ -68,9 +68,6 @@ enabled_skills: inject_skill_instructions: true # Inject a hint pointing the model at `skill__list`. Defaults to true; suppressed # automatically when no skills are available. skill_instructions: null # Custom text for the skill hint (optional; uses the built-in default if omitted). -# Note: `enabled_macros` is NOT a graph setting. Graph nodes never dispatch REPL -# commands, so the field is silently ignored in graph configs; scope macros via -# the global config, roles, agents, or sessions instead. conversation_starters: # Suggested prompts surfaced in the UI - "Research the current state of WebAssembly outside the browser" diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs index 8dba677..6636778 100644 --- a/src/config/macro_policy.rs +++ b/src/config/macro_policy.rs @@ -11,11 +11,8 @@ use std::fmt; use std::fs::{read_dir, read_to_string}; use std::path::PathBuf; -/// Names that cannot be used as macro names because they are reserved for -/// `.macro enable ` / `.macro disable `. pub const RESERVED_MACRO_NAMES: [&str; 2] = ["enable", "disable"]; -/// Where a macro definition file was discovered. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MacroSource { Workspace, @@ -52,25 +49,13 @@ impl fmt::Display for MacroAllowlistLevel { } } -/// The effective state of a macro within the active context. #[derive(Debug, Clone, PartialEq, Eq)] pub enum MacroState { - /// Visible and invocable. Enabled, - /// Excluded by the GLOBAL-level `enabled_macros` list (config file or - /// runtime toggle — the two are indistinguishable by design). Can be - /// re-enabled at runtime with `.macro enable `. DisabledRuntime, - /// Excluded by a role/agent/session `enabled_macros` allowlist. Not - /// enableable from the REPL; the owning config is the source of truth. - /// `level` is never `Global` — a global exclusion is `DisabledRuntime`. Locked { level: MacroAllowlistLevel }, - /// Named by the effective allowlist, but no such macro is installed. Missing, - /// The macro name collides with a built-in REPL command; invocable only - /// via `.macro `, never as a top-level `.name` command. ShadowedBuiltin, - /// The definition file failed to parse, or the name is reserved. Invalid { reason: String }, } @@ -80,34 +65,24 @@ impl MacroState { } } -/// A macro definition file found on disk, before allowlist resolution. #[derive(Debug, Clone)] pub struct DiscoveredMacro { pub name: String, pub source: MacroSource, - /// The parsed definition, or the parse failure reason. pub definition: Result, - /// True for a GLOBAL entry whose name is shadowed by a workspace entry. pub shadowed_by_workspace: bool, } -/// One row of the resolved macro set. Missing allowlist entries produce rows -/// with `source: None`. #[derive(Debug, Clone)] pub struct ResolvedMacro { pub name: String, pub source: Option, pub description: Option, pub isolated: Option, - /// True for a global entry shadowed by a workspace entry of the same - /// name; the row is kept for display but is never the invocation target. pub shadowed_by_workspace: bool, pub state: MacroState, } -/// The visible macro set for the active context, resolved lazily on demand -/// from the discovered definition files and the effective `enabled_macros` -/// allowlist. #[derive(Debug)] pub struct MacroPolicy { pub macros: Vec, @@ -181,6 +156,7 @@ impl MacroPolicy { }); } } + macros.extend(missing); } @@ -193,9 +169,6 @@ impl MacroPolicy { Self { macros } } - /// The invocation target for `name`: the workspace entry when one shadows - /// a global entry, otherwise the single discovered entry. Missing rows - /// are never returned. pub fn find(&self, name: &str) -> Option<&ResolvedMacro> { self.macros .iter() @@ -221,6 +194,7 @@ fn resolve_state( "Ignoring macro '{}': the name is reserved for '.macro {}'", discovered.name, discovered.name ); + return MacroState::Invalid { reason: format!("'{}' is a reserved macro name", discovered.name), }; @@ -248,14 +222,12 @@ fn resolve_state( MacroState::Enabled } -/// Rescans the workspace and global macro directories. Workspace entries -/// shadow global entries of the same name; the shadowed global entry is kept -/// and flagged so both remain listable. pub fn discover_macros(no_workspace_macros: bool) -> Vec { let mut dirs = vec![]; if !no_workspace_macros { dirs.push((MacroSource::Workspace, paths::workspace_macros_dir())); } + dirs.push((MacroSource::Global, paths::macros_dir())); discover_macros_in(&dirs) } @@ -270,14 +242,17 @@ fn discover_macros_in(dirs: &[(MacroSource, PathBuf)]) -> Vec { }; let mut entries: Vec<_> = rd.flatten().collect(); entries.sort_by_key(|entry| entry.file_name()); + for entry in entries { let is_file = entry .file_type() .map(|file_type| file_type.is_file()) .unwrap_or(false); + if !is_file { continue; } + let Some(name) = entry .file_name() .to_str() @@ -286,9 +261,11 @@ fn discover_macros_in(dirs: &[(MacroSource, PathBuf)]) -> Vec { else { continue; }; + if name.is_empty() { continue; } + let definition = read_to_string(entry.path()) .map_err(|err| err.to_string()) .and_then(|content| { @@ -372,6 +349,7 @@ mod tests { #[test] fn all_none_enables_everything() { let policy = resolve(globals(&["a", "b"]), None, None, None, None); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!(state_of(&policy, "b"), &MacroState::Enabled); } @@ -380,6 +358,7 @@ mod tests { fn global_empty_list_disables_all_as_runtime() { let l = list(&[]); let policy = resolve(globals(&["a", "b"]), None, None, None, Some(&l)); + assert_eq!(state_of(&policy, "a"), &MacroState::DisabledRuntime); assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); } @@ -388,6 +367,7 @@ mod tests { fn global_populated_partitions_enabled_and_disabled_runtime() { let l = list(&["a"]); let policy = resolve(globals(&["a", "b"]), None, None, None, Some(&l)); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!(state_of(&policy, "b"), &MacroState::DisabledRuntime); } @@ -395,7 +375,9 @@ mod tests { #[test] fn role_populated_locks_excluded_at_role_level() { let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), None, None, Some(&l), None); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!( state_of(&policy, "b"), @@ -408,7 +390,9 @@ mod tests { #[test] fn agent_populated_locks_excluded_at_agent_level() { let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), None, Some(&l), None, None); + assert_eq!( state_of(&policy, "b"), &MacroState::Locked { @@ -420,7 +404,9 @@ mod tests { #[test] fn session_populated_locks_excluded_at_session_level() { let l = list(&["a"]); + let policy = resolve(globals(&["a", "b"]), Some(&l), None, None, None); + assert_eq!( state_of(&policy, "b"), &MacroState::Locked { @@ -432,7 +418,9 @@ mod tests { #[test] fn role_empty_list_locks_everything_at_role_level() { let l = list(&[]); + let policy = resolve(globals(&["a"]), None, None, Some(&l), None); + assert_eq!( state_of(&policy, "a"), &MacroState::Locked { @@ -444,7 +432,9 @@ mod tests { #[test] fn agent_empty_list_locks_everything_at_agent_level() { let l = list(&[]); + let policy = resolve(globals(&["a"]), None, Some(&l), None, None); + assert_eq!( state_of(&policy, "a"), &MacroState::Locked { @@ -456,7 +446,9 @@ mod tests { #[test] fn session_empty_list_locks_everything_at_session_level() { let l = list(&[]); + let policy = resolve(globals(&["a"]), Some(&l), None, None, None); + assert_eq!( state_of(&policy, "a"), &MacroState::Locked { @@ -469,6 +461,7 @@ mod tests { fn session_wins_over_agent() { let session = list(&["a"]); let agent = list(&["b"]); + let policy = resolve( globals(&["a", "b"]), Some(&session), @@ -476,6 +469,7 @@ mod tests { None, None, ); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!( state_of(&policy, "b"), @@ -489,7 +483,9 @@ mod tests { fn agent_wins_over_role() { let agent = list(&["a"]); let role = list(&["b"]); + let policy = resolve(globals(&["a", "b"]), None, Some(&agent), Some(&role), None); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!( state_of(&policy, "b"), @@ -503,7 +499,9 @@ mod tests { fn role_wins_over_global() { let role = list(&["a"]); let global = list(&["b"]); + let policy = resolve(globals(&["a", "b"]), None, None, Some(&role), Some(&global)); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!( state_of(&policy, "b"), @@ -517,7 +515,9 @@ mod tests { fn empty_list_at_session_beats_populated_global() { let session = list(&[]); let global = list(&["a"]); + let policy = resolve(globals(&["a"]), Some(&session), None, None, Some(&global)); + assert_eq!( state_of(&policy, "a"), &MacroState::Locked { @@ -529,7 +529,9 @@ mod tests { #[test] fn unknown_allowlist_name_yields_missing_row_without_error() { let l = list(&["a", "ghost"]); + let policy = resolve(globals(&["a"]), None, None, None, Some(&l)); + assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); assert_eq!(state_of(&policy, "ghost"), &MacroState::Missing); let ghost = policy.macros.iter().find(|m| m.name == "ghost").unwrap(); @@ -539,7 +541,9 @@ mod tests { #[test] fn missing_row_deduplicated_for_repeated_allowlist_names() { let l = list(&["ghost", "ghost"]); + let policy = resolve(vec![], None, None, None, Some(&l)); + assert_eq!(policy.macros.len(), 1); assert_eq!(state_of(&policy, "ghost"), &MacroState::Missing); } @@ -547,6 +551,7 @@ mod tests { #[test] fn no_missing_rows_without_an_allowlist() { let policy = resolve(globals(&["a"]), None, None, None, None); + assert_eq!(policy.macros.len(), 1); } @@ -554,6 +559,7 @@ mod tests { fn builtin_name_collision_is_shadowed() { let policy = MacroPolicy::effective_with(globals(&["help", "a"]), None, None, None, None, &["help"]); + assert_eq!(state_of(&policy, "help"), &MacroState::ShadowedBuiltin); assert_eq!(state_of(&policy, "a"), &MacroState::Enabled); } @@ -561,6 +567,7 @@ mod tests { #[test] fn locked_wins_over_shadowed_builtin() { let l = list(&["a"]); + let policy = MacroPolicy::effective_with( globals(&["help", "a"]), Some(&l), @@ -569,6 +576,7 @@ mod tests { None, &["help"], ); + assert_eq!( state_of(&policy, "help"), &MacroState::Locked { @@ -580,8 +588,10 @@ mod tests { #[test] fn allowlisted_builtin_collision_stays_shadowed() { let l = list(&["help"]); + let policy = MacroPolicy::effective_with(globals(&["help"]), None, None, None, Some(&l), &["help"]); + assert_eq!(state_of(&policy, "help"), &MacroState::ShadowedBuiltin); } @@ -601,7 +611,9 @@ mod tests { #[test] fn reserved_name_invalid_even_when_allowlisted() { let l = list(&["enable"]); + let policy = resolve(globals(&["enable"]), Some(&l), None, None, None); + assert_eq!( state_of(&policy, "enable"), &MacroState::Invalid { @@ -614,6 +626,7 @@ mod tests { fn reserved_name_invalid_wins_over_builtin_collision() { let policy = MacroPolicy::effective_with(globals(&["enable"]), None, None, None, None, &["enable"]); + assert_eq!( state_of(&policy, "enable"), &MacroState::Invalid { @@ -625,6 +638,7 @@ mod tests { #[test] fn parse_failure_is_invalid() { let policy = resolve(vec![disc_invalid("bad", "boom")], None, None, None, None); + assert_eq!( state_of(&policy, "bad"), &MacroState::Invalid { @@ -639,6 +653,7 @@ mod tests { #[test] fn invalid_wins_over_allowlist_exclusion() { let l = list(&["other"]); + let policy = resolve( vec![disc_invalid("bad", "boom")], Some(&l), @@ -646,6 +661,7 @@ mod tests { None, None, ); + assert_eq!( state_of(&policy, "bad"), &MacroState::Invalid { @@ -663,7 +679,9 @@ mod tests { ..disc("a", MacroSource::Global) }, ]; + let policy = resolve(discovered, None, None, None, None); + assert_eq!(policy.macros.len(), 2); assert_eq!(policy.macros[0].source, Some(MacroSource::Workspace)); assert!(!policy.macros[0].shadowed_by_workspace); @@ -676,13 +694,16 @@ mod tests { #[test] fn find_skips_missing_rows() { let l = list(&["ghost"]); + let policy = resolve(vec![], None, None, None, Some(&l)); + assert!(policy.find("ghost").is_none()); } #[test] fn rows_are_sorted_by_name() { let policy = resolve(globals(&["c", "a", "b"]), None, None, None, None); + let names: Vec<&str> = policy.macros.iter().map(|m| m.name.as_str()).collect(); assert_eq!(names, vec!["a", "b", "c"]); } @@ -690,6 +711,7 @@ mod tests { #[test] fn resolved_rows_carry_description_and_isolated() { let policy = resolve(globals(&["a"]), None, None, None, None); + let row = policy.macros.first().unwrap(); assert_eq!(row.description.as_deref(), Some("a test macro")); assert_eq!(row.isolated, Some(true)); diff --git a/src/config/macros.rs b/src/config/macros.rs index 667a020..93d9806 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -41,6 +41,7 @@ pub async fn macro_execute( println!(">> {}", multiline_text(&command)); run_repl_command(&mut live, abort_signal.clone(), &command).await?; } + return Ok(()); } @@ -84,9 +85,6 @@ pub async fn macro_execute( Ok(()) } -/// Marks a live context as executing a non-isolated macro for the duration of -/// its steps. Restores the previous flag and mode on drop, so every exit path -/// — including a failing step — leaves the context as it found it. struct MacroModeGuard<'a> { ctx: &'a mut RequestContext, prev_flag: bool, @@ -223,9 +221,11 @@ impl Macro { pub fn interpolate_command(command: &str, variables: &IndexMap) -> String { let mut output = command.to_string(); + for (key, value) in variables { output = output.replace(&format!("{{{{{key}}}}}"), value); } + output } } @@ -416,21 +416,27 @@ mod tests { #[test] fn resolve_no_variables() { let m = macro_with_vars(vec![]); + let result = m.resolve_variables(&[]).unwrap(); + assert!(result.is_empty()); } #[test] fn resolve_required_variable_provided() { let m = macro_with_vars(vec![var("name", false, None)]); + let result = m.resolve_variables(&["Alice".into()]).unwrap(); + assert_eq!(result["name"], "Alice"); } #[test] fn resolve_required_variable_missing_errors() { let m = macro_with_vars(vec![var("name", false, None)]); + let result = m.resolve_variables(&[]); + assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("name")); } @@ -438,23 +444,29 @@ mod tests { #[test] fn resolve_default_variable_uses_default() { let m = macro_with_vars(vec![var("color", false, Some("blue"))]); + let result = m.resolve_variables(&[]).unwrap(); + assert_eq!(result["color"], "blue"); } #[test] fn resolve_default_variable_overridden() { let m = macro_with_vars(vec![var("color", false, Some("blue"))]); + let result = m.resolve_variables(&["red".into()]).unwrap(); + assert_eq!(result["color"], "red"); } #[test] fn resolve_rest_variable_captures_all_remaining() { let m = macro_with_vars(vec![var("first", false, None), var("rest", true, None)]); + let result = m .resolve_variables(&["a".into(), "b".into(), "c".into()]) .unwrap(); + assert_eq!(result["first"], "a"); assert_eq!(result["rest"], "b c"); } @@ -462,7 +474,9 @@ mod tests { #[test] fn resolve_rest_variable_with_default() { let m = macro_with_vars(vec![var("args", true, Some("default text"))]); + let result = m.resolve_variables(&[]).unwrap(); + assert_eq!(result["args"], "default text"); } @@ -473,7 +487,9 @@ mod tests { var("b", false, None), var("c", false, Some("default_c")), ]); + let result = m.resolve_variables(&["x".into(), "y".into()]).unwrap(); + assert_eq!(result["a"], "x"); assert_eq!(result["b"], "y"); assert_eq!(result["c"], "default_c"); @@ -482,30 +498,35 @@ mod tests { #[test] fn usage_no_variables() { let m = macro_with_vars(vec![]); + assert_eq!(m.usage("my-macro"), "my-macro"); } #[test] fn usage_required_variable() { let m = macro_with_vars(vec![var("name", false, None)]); + assert_eq!(m.usage("greet"), "greet "); } #[test] fn usage_optional_variable() { let m = macro_with_vars(vec![var("color", false, Some("blue"))]); + assert_eq!(m.usage("paint"), "paint [color]"); } #[test] fn usage_rest_variable() { let m = macro_with_vars(vec![var("args", true, None)]); + assert_eq!(m.usage("run"), "run ..."); } #[test] fn usage_rest_with_default() { let m = macro_with_vars(vec![var("args", true, Some("default"))]); + assert_eq!(m.usage("run"), "run [args]..."); } @@ -515,6 +536,7 @@ mod tests { var("target", false, None), var("flags", true, Some("")), ]); + assert_eq!(m.usage("build"), "build [flags]..."); } @@ -522,6 +544,7 @@ mod tests { fn interpolate_replaces_variables() { let vars = IndexMap::from([("name".to_string(), "world".to_string())]); let result = Macro::interpolate_command("hello {{name}}", &vars); + assert_eq!(result, "hello world"); } @@ -532,6 +555,7 @@ mod tests { ("b".to_string(), "2".to_string()), ]); let result = Macro::interpolate_command("{{a}} + {{b}}", &vars); + assert_eq!(result, "1 + 2"); } @@ -539,6 +563,7 @@ mod tests { fn interpolate_no_variables_passthrough() { let vars = IndexMap::new(); let result = Macro::interpolate_command("no vars here", &vars); + assert_eq!(result, "no vars here"); } @@ -546,6 +571,7 @@ mod tests { fn interpolate_variable_not_found_left_as_is() { let vars = IndexMap::new(); let result = Macro::interpolate_command("hello {{missing}}", &vars); + assert_eq!(result, "hello {{missing}}"); } @@ -578,7 +604,9 @@ variables: rest: true default: "none" "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(m.variables[0].default, Some("fast".to_string())); assert!(m.variables[1].rest); assert_eq!(m.variables[1].default, Some("none".to_string())); @@ -590,7 +618,9 @@ variables: steps: - ".help" "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert!(m.variables.is_empty()); assert_eq!(m.steps.len(), 1); } @@ -601,7 +631,9 @@ steps: steps: - ".help" "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert!(m.description.is_none()); assert!(m.isolated); } @@ -617,7 +649,9 @@ variables: - name: base default: main "#; + let m: Macro = serde_yaml::from_str(yaml).unwrap(); + assert_eq!( m.description.as_deref(), Some("Review WIP against a base branch") @@ -634,8 +668,10 @@ variables: variables: vec![var("target", false, Some("all"))], steps: vec!["build {{target}}".to_string()], }; + let yaml = serde_yaml::to_string(&original).unwrap(); let back: Macro = serde_yaml::from_str(&yaml).unwrap(); + assert_eq!(back.description.as_deref(), Some("does a thing")); assert!(!back.isolated); assert_eq!(back.variables.len(), 1); @@ -648,6 +684,7 @@ variables: fn round_trip_defaults_survive() { let original = macro_with_vars(vec![]); let yaml = serde_yaml::to_string(&original).unwrap(); + assert!(!yaml.contains("description")); let back: Macro = serde_yaml::from_str(&yaml).unwrap(); assert!(back.description.is_none()); @@ -659,8 +696,10 @@ variables: for file in MacroAssets::iter() { let embedded = MacroAssets::get(&file).unwrap(); let content = std::str::from_utf8(&embedded.data).unwrap(); + let m: Macro = serde_yaml::from_str(content) .unwrap_or_else(|e| panic!("asset '{}' failed to deserialize: {e}", file.as_ref())); + assert!(m.description.is_none(), "asset '{}'", file.as_ref()); assert!(m.isolated, "asset '{}'", file.as_ref()); assert!(!m.steps.is_empty(), "asset '{}'", file.as_ref()); diff --git a/src/config/mod.rs b/src/config/mod.rs index f36cc99..ea90e76 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1140,12 +1140,14 @@ clients: #[test] fn config_enabled_macros_empty_string_is_some_empty() { let cfg: Config = serde_yaml::from_str("enabled_macros: \"\"").unwrap(); + assert_eq!(cfg.enabled_macros, Some(vec![])); } #[test] fn config_enabled_macros_csv_string() { let cfg: Config = serde_yaml::from_str("enabled_macros: \"a, b\"").unwrap(); + assert_eq!( cfg.enabled_macros, Some(vec!["a".to_string(), "b".to_string()]) @@ -1155,6 +1157,7 @@ clients: #[test] fn config_enabled_macros_list() { let cfg: Config = serde_yaml::from_str("enabled_macros:\n - a\n - b").unwrap(); + assert_eq!( cfg.enabled_macros, Some(vec!["a".to_string(), "b".to_string()]) @@ -1164,12 +1167,14 @@ clients: #[test] fn config_enabled_macros_null_is_none() { let cfg: Config = serde_yaml::from_str("enabled_macros: null").unwrap(); + assert_eq!(cfg.enabled_macros, None); } #[test] fn assert_state_pass_always_true() { let pass = AssertState::pass(); + assert!(pass.assert(StateFlags::empty())); assert!(pass.assert(StateFlags::ROLE)); assert!(pass.assert(StateFlags::SESSION | StateFlags::AGENT)); @@ -1179,6 +1184,7 @@ clients: #[test] fn assert_state_bare_only_empty() { let bare = AssertState::bare(); + assert!(bare.assert(StateFlags::empty())); assert!(!bare.assert(StateFlags::ROLE)); assert!(!bare.assert(StateFlags::SESSION)); @@ -1187,6 +1193,7 @@ clients: #[test] fn assert_state_true_requires_flag_present() { let state = AssertState::True(StateFlags::ROLE); + assert!(state.assert(StateFlags::ROLE)); assert!(state.assert(StateFlags::ROLE | StateFlags::SESSION)); assert!(!state.assert(StateFlags::empty())); @@ -1196,6 +1203,7 @@ clients: #[test] fn assert_state_true_with_multiple_flags_any_match() { let state = AssertState::True(StateFlags::SESSION_EMPTY | StateFlags::SESSION); + assert!(state.assert(StateFlags::SESSION_EMPTY)); assert!(state.assert(StateFlags::SESSION)); assert!(state.assert(StateFlags::SESSION | StateFlags::ROLE)); @@ -1206,6 +1214,7 @@ clients: #[test] fn assert_state_false_requires_flag_absent() { let state = AssertState::False(StateFlags::AGENT); + assert!(state.assert(StateFlags::empty())); assert!(state.assert(StateFlags::ROLE)); assert!(!state.assert(StateFlags::AGENT)); @@ -1215,6 +1224,7 @@ clients: #[test] fn assert_state_false_with_multiple_flags() { let state = AssertState::False(StateFlags::SESSION | StateFlags::AGENT); + assert!(state.assert(StateFlags::empty())); assert!(state.assert(StateFlags::ROLE)); assert!(!state.assert(StateFlags::SESSION)); @@ -1225,6 +1235,7 @@ clients: #[test] fn assert_state_truefalse_requires_true_present_and_false_absent() { let state = AssertState::TrueFalse(StateFlags::ROLE, StateFlags::SESSION); + assert!(state.assert(StateFlags::ROLE)); assert!(state.assert(StateFlags::ROLE | StateFlags::RAG)); assert!(!state.assert(StateFlags::empty())); @@ -1235,6 +1246,7 @@ clients: #[test] fn assert_state_equal_exact_match() { let state = AssertState::Equal(StateFlags::ROLE | StateFlags::SESSION); + assert!(state.assert(StateFlags::ROLE | StateFlags::SESSION)); assert!(!state.assert(StateFlags::ROLE)); assert!(!state.assert(StateFlags::SESSION)); diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 4762e9c..be8daf2 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -124,8 +124,6 @@ fn complete_skills_with_descriptions(names: Vec) -> Vec<(String, Option< .collect() } -/// Keys offered by `.set ` completion. `reasoning_effort` is appended at -/// completion time only when the current model supports reasoning levels. const SET_COMPLETION_KEYS: [&str; 26] = [ "auto_continue", "continuation_prompt", @@ -155,10 +153,6 @@ const SET_COMPLETION_KEYS: [&str; 26] = [ "raw_markdown", ]; -/// The new global-level `enabled_macros` list after toggling `name`, or -/// `None` when the toggle is a no-op (already in the requested state). -/// Disabling with no current list (all macros visible) materializes the list -/// as every active macro name minus `name`. fn toggled_enabled_macros( current: Option<&[String]>, all_active: &[String], @@ -198,7 +192,6 @@ fn toggled_enabled_macros( } } -/// The `.list macros` state column for a resolved row. fn macro_state_display( row: &ResolvedMacro, lock_owner: impl Fn(MacroAllowlistLevel) -> String, @@ -213,8 +206,6 @@ fn macro_state_display( } } -/// The `.list macros` source column: where the definition file lives, or `-` -/// for allowlist entries with no installed file. fn macro_source_display(source: Option) -> String { match source { Some(source) => source.to_string(), @@ -233,9 +224,6 @@ pub struct RequestContext { pub app: Arc, pub macro_flag: bool, - /// Companion to `macro_flag`: true while a non-isolated macro is running - /// its steps directly on this live context. Isolated macros execute on a - /// forked context and leave this false. pub macro_non_isolated: bool, pub info_flag: bool, pub working_mode: WorkingMode, @@ -2488,16 +2476,10 @@ impl RequestContext { Ok(()) } - /// Whether a non-isolated macro is currently running its steps on this - /// context. Macro invocations are rejected in this mode: the nested - /// macro's steps would interleave with the outer macro's on the live - /// session. pub fn in_non_isolated_macro(&self) -> bool { self.macro_flag && self.macro_non_isolated } - /// The resolved macro set for the active context (workspace + global - /// discovery, effective `enabled_macros` allowlist, built-in shadowing). pub fn macro_policy(&self) -> MacroPolicy { MacroPolicy::effective( &self.app.config, @@ -2509,8 +2491,6 @@ impl RequestContext { ) } - /// A human-readable name for the config level whose `enabled_macros` - /// allowlist restricts a macro, e.g. `agent:oracle` or `role:coder`. pub fn macro_lock_owner(&self, level: MacroAllowlistLevel) -> String { let name = match level { MacroAllowlistLevel::Session => self.session.as_ref().map(|s| s.name()), @@ -2524,9 +2504,6 @@ impl RequestContext { } } - /// Enables or disables a macro by editing the in-memory global-level - /// `enabled_macros` list. Errors when a role/agent/session allowlist is - /// active, since a global-level write would be silently shadowed. pub fn macro_toggle(&mut self, name: &str, enable: bool) -> Result<()> { let restricting_level = if self .session @@ -2577,6 +2554,7 @@ impl RequestContext { .collect(); let action = if enable { "enabled" } else { "disabled" }; + match toggled_enabled_macros( self.app.config.enabled_macros.as_deref(), &all_active, @@ -2589,12 +2567,10 @@ impl RequestContext { } None => println!("Macro '{name}' is already {action}"), } + Ok(()) } - /// The macros offered by top-level `.` completion: enabled rows - /// only. Macros shadowed by a built-in command never appear here; they - /// stay reachable via `.macro `. pub fn visible_macro_completions(&self) -> Vec<(String, Option)> { self.macro_policy() .macros @@ -2622,6 +2598,7 @@ impl RequestContext { "name", "source", "isolated", "state", "description" ); println!("{header}"); + for row in &policy.macros { let source = macro_source_display(row.source); let isolated = match row.isolated { @@ -4032,7 +4009,7 @@ impl RequestContext { // Graph agents manage their own state; never engage a session, // not even an inherited app-level `agent_session` default. - // Isolated macros suppress an inherited default too — their forked + // Isolated macros suppress an inherited default too: their forked // context has no session to return to. A non-isolated macro's `.agent` // step engages it exactly as if the user had typed the command. let session_name = session_name.map(|v| v.to_string()).or_else(|| { diff --git a/src/config/role.rs b/src/config/role.rs index 213f848..db82655 100644 --- a/src/config/role.rs +++ b/src/config/role.rs @@ -558,6 +558,7 @@ mod tests { #[test] fn role_new_parses_prompt() { let role = Role::new("test", "You are a helpful assistant"); + assert_eq!(role.name(), "test"); assert_eq!(role.prompt(), "You are a helpful assistant"); } @@ -566,7 +567,9 @@ mod tests { fn role_new_parses_metadata() { let content = "---\nmodel: openai:gpt-4\ntemperature: 0.7\ntop_p: 0.9\n---\nYou are helpful"; + let role = Role::new("test", content); + assert_eq!(role.model_id(), Some("openai:gpt-4")); assert_eq!(role.temperature(), Some(0.7)); assert_eq!(role.top_p(), Some(0.9)); @@ -576,7 +579,9 @@ mod tests { #[test] fn role_new_parses_enabled_tools() { let content = "---\nenabled_tools: tool1,tool2\n---\nPrompt"; + let role = Role::new("test", content); + assert_eq!( role.enabled_tools(), Some(vec!["tool1".to_string(), "tool2".to_string()]) @@ -586,7 +591,9 @@ mod tests { #[test] fn role_new_parses_enabled_mcp_servers() { let content = "---\nenabled_mcp_servers: github,jira\n---\nPrompt"; + let role = Role::new("test", content); + assert_eq!( role.enabled_mcp_servers(), Some(vec!["github".to_string(), "jira".to_string()]) @@ -596,6 +603,7 @@ mod tests { #[test] fn role_new_no_metadata_has_none_fields() { let role = Role::new("test", "Just a prompt"); + assert_eq!(role.model_id(), None); assert_eq!(role.temperature(), None); assert_eq!(role.top_p(), None); @@ -606,18 +614,21 @@ mod tests { #[test] fn role_new_enabled_macros_absent_is_none() { let role = Role::new("test", "---\ntemperature: 0.5\n---\nPrompt"); + assert_eq!(role.enabled_macros, None); } #[test] fn role_new_enabled_macros_empty_string_is_some_empty() { let role = Role::new("test", "---\nenabled_macros: \"\"\n---\nPrompt"); + assert_eq!(role.enabled_macros, Some(vec![])); } #[test] fn role_new_enabled_macros_csv_string() { let role = Role::new("test", "---\nenabled_macros: a, b\n---\nPrompt"); + assert_eq!( role.enabled_macros, Some(vec!["a".to_string(), "b".to_string()]) @@ -627,6 +638,7 @@ mod tests { #[test] fn role_new_enabled_macros_list() { let role = Role::new("test", "---\nenabled_macros: [a, b]\n---\nPrompt"); + assert_eq!( role.enabled_macros, Some(vec!["a".to_string(), "b".to_string()]) @@ -636,25 +648,30 @@ mod tests { #[test] fn role_new_enabled_macros_null_is_none() { let role = Role::new("test", "---\nenabled_macros: null\n---\nPrompt"); + assert_eq!(role.enabled_macros, None); } #[test] fn role_export_includes_enabled_macros() { let role = Role::new("test", "---\nenabled_macros: [a]\n---\nPrompt"); + let exported = role.export(); + assert!(exported.contains("enabled_macros: [\"a\"]")); } #[test] fn role_export_omits_enabled_macros_when_none() { let role = Role::new("test", "Just a prompt"); + assert!(!role.export().contains("enabled_macros")); } #[test] fn role_builtin_shell_loads() { let role = Role::builtin("shell").unwrap(); + assert_eq!(role.name(), "shell"); assert!(!role.prompt().is_empty()); } @@ -662,6 +679,7 @@ mod tests { #[test] fn role_builtin_code_loads() { let role = Role::builtin("code").unwrap(); + assert_eq!(role.name(), "code"); assert!(!role.prompt().is_empty()); } @@ -669,12 +687,14 @@ mod tests { #[test] fn role_builtin_nonexistent_errors() { let result = Role::builtin("nonexistent_role_xyz"); + assert!(result.is_err()); } #[test] fn role_default_has_empty_fields() { let role = Role::default(); + assert_eq!(role.name(), ""); assert_eq!(role.prompt(), ""); assert_eq!(role.model_id(), None); @@ -684,14 +704,18 @@ mod tests { fn role_set_model_updates_model() { let mut role = Role::new("test", "prompt"); let model = Model::default(); + role.set_model(model.clone()); + assert_eq!(role.model().id(), model.id()); } #[test] fn role_set_temperature_works() { let mut role = Role::new("test", "prompt"); + role.set_temperature(Some(0.5)); + assert_eq!(role.temperature(), Some(0.5)); } @@ -699,7 +723,9 @@ mod tests { fn role_export_includes_metadata() { let content = "---\ntemperature: 0.8\n---\nMy prompt"; let role = Role::new("test", content); + let exported = role.export(); + assert!(exported.contains("temperature")); assert!(exported.contains("My prompt")); } @@ -713,6 +739,7 @@ Input 1 ### OUTPUT: Output 1 "#; + assert_eq!( parse_structure_prompt(prompt), ("System message", vec![("Input 1", "Output 1")]) @@ -727,6 +754,7 @@ Input 1 ### OUTPUT: Output 1 "#; + assert_eq!( parse_structure_prompt(prompt), ("", vec![("Input 1", "Output 1")]) @@ -740,6 +768,7 @@ System message ### INPUT: Input 1 "#; + assert_eq!(parse_structure_prompt(prompt), (prompt, vec![])); } } diff --git a/src/config/session.rs b/src/config/session.rs index 728b100..1e3b042 100644 --- a/src/config/session.rs +++ b/src/config/session.rs @@ -942,6 +942,7 @@ mod tests { #[test] fn session_default_is_empty() { let session = Session::default(); + assert!(session.is_empty()); assert_eq!(session.name(), ""); assert_eq!(session.role_name(), None); @@ -951,6 +952,7 @@ mod tests { #[test] fn session_enabled_macros_absent_is_none() { let session: Session = serde_yaml::from_str("model: provider:test\nmessages: []").unwrap(); + assert_eq!(session.enabled_macros, None); } @@ -958,6 +960,7 @@ mod tests { fn session_enabled_macros_empty_list_is_some_empty() { let session: Session = serde_yaml::from_str("model: provider:test\nenabled_macros: []\nmessages: []").unwrap(); + assert_eq!(session.enabled_macros, Some(vec![])); } @@ -966,6 +969,7 @@ mod tests { let session: Session = serde_yaml::from_str("model: provider:test\nenabled_macros: \"\"\nmessages: []") .unwrap(); + assert_eq!(session.enabled_macros, Some(vec![])); } @@ -974,6 +978,7 @@ mod tests { let session: Session = serde_yaml::from_str("model: provider:test\nenabled_macros: \"a,b\"\nmessages: []") .unwrap(); + assert_eq!( session.enabled_macros, Some(vec!["a".to_string(), "b".to_string()]) @@ -984,6 +989,7 @@ mod tests { fn session_serialize_omits_enabled_macros_when_none() { let session = Session::default(); let yaml = serde_yaml::to_string(&session).unwrap(); + assert!(!yaml.contains("enabled_macros")); } @@ -1001,6 +1007,7 @@ mod tests { functions: Functions::default(), }); let ctx = RequestContext::new(app_state, WorkingMode::Cmd); + let session = Session::new_from_ctx(&ctx, &app_config, "test-session").unwrap(); assert_eq!(session.name(), "test-session"); @@ -1040,25 +1047,30 @@ mod tests { #[test] fn session_guard_empty_passes_when_empty() { let session = Session::default(); + assert!(session.guard_empty().is_ok()); } #[test] fn session_needs_compression_threshold() { let session = Session::default(); + assert!(!session.needs_compression(4000)); } #[test] fn session_needs_compression_returns_false_when_compressing() { let mut session = Session::default(); + session.set_compressing(true); + assert!(!session.needs_compression(0)); } #[test] fn session_needs_compression_returns_false_when_threshold_zero() { let session = Session::default(); + assert!(!session.needs_compression(0)); } @@ -1130,13 +1142,16 @@ mod tests { #[test] fn session_need_autoname_default_false() { let session = Session::default(); + assert!(!session.need_autoname()); } #[test] fn session_set_autonaming_doesnt_panic_without_autoname() { let mut session = Session::default(); + session.set_autonaming(true); + assert!(!session.need_autoname()); } diff --git a/src/graph/types.rs b/src/graph/types.rs index 8e13538..18bdf5d 100644 --- a/src/graph/types.rs +++ b/src/graph/types.rs @@ -591,7 +591,9 @@ nodes: #[test] fn graph_silently_ignores_enabled_macros_key() { let yaml = "name: g\nenabled_macros: [\"x\"]\nstart: x\nnodes:\n x:\n id: x\n type: end\n output: ok\n"; + let graph: Graph = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(graph.name, "g"); assert_eq!(graph.start, "x"); } diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 21bd991..77e9233 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -1086,7 +1086,7 @@ pub async fn run_repl_command( macro_execute(ctx, name, extra, abort_signal.clone()).await?; } Some(MacroState::DisabledRuntime) => bail!( - r#"Macro '{name}' is disabled. Re-enable it with ".macro enable {name}""# + r#"Macro '{name}' is disabled. Enable it with ".macro enable {name}""# ), Some(MacroState::Locked { level }) => bail!( "Macro '{name}' is restricted by {} enabled_macros", @@ -1567,9 +1567,6 @@ fn unknown_command() -> Result<()> { bail!(r#"Unknown command. Type ".help" for additional help."#); } -/// The name of every built-in REPL command (first word, without the leading -/// dot), sorted and deduplicated. Macros with one of these names are shadowed -/// by the built-in and stay reachable only via `.macro `. pub fn builtin_command_names() -> Vec<&'static str> { let mut names: Vec<&'static str> = REPL_COMMANDS .iter() From fba040c668d775d7a79a413541312d22864b0a69 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 10:48:34 -0600 Subject: [PATCH 11/18] fix: render .list macros as a comfy-table instead of fixed-width columns Hand-rolled {:<24} padding broke alignment as soon as a macro name exceeded the column width. Reuse the comfy-table UTF8_FULL preset with dynamic content arrangement, matching the markdown renderer's tables. --- src/config/request_context.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/config/request_context.rs b/src/config/request_context.rs index be8daf2..7f19ad1 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -33,6 +33,7 @@ use crate::utils::{ AbortSignal, abortable_run_with_spinner, edit_file, fuzzy_filter, get_env_name, list_file_names, now, render_prompt, temp_file, }; +use comfy_table::{ContentArrangement, Table, presets::UTF8_FULL}; use super::instructions; use super::memory::{ @@ -2592,12 +2593,10 @@ impl RequestContext { return Ok(()); } - println!("Macros:"); - let header = format!( - " {:<24} {:<10} {:<9} {:<40} {}", - "name", "source", "isolated", "state", "description" - ); - println!("{header}"); + let mut table = Table::new(); + table.load_preset(UTF8_FULL); + table.set_content_arrangement(ContentArrangement::Dynamic); + table.set_header(vec!["name", "source", "isolated", "state", "description"]); for row in &policy.macros { let source = macro_source_display(row.source); @@ -2608,13 +2607,17 @@ impl RequestContext { }; let state = macro_state_display(row, |level| self.macro_lock_owner(level)); let description = row.description.as_deref().unwrap_or_default(); - let line = format!( - " {:<24} {:<10} {:<9} {:<40} {}", - row.name, source, isolated, state, description - ); - println!("{}", line.trim_end()); + table.add_row(vec![ + row.name.as_str(), + &source, + isolated, + &state, + description, + ]); } + println!("Macros:"); + println!("{table}"); Ok(()) } "agents" => { From e1b55628880235fba4f3fab71c1edc0d67719825 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 10:54:28 -0600 Subject: [PATCH 12/18] refactor: render .list agents and .list skills as comfy-tables Long agent/skill descriptions wrapped badly in the bullet-list format. Extract a shared asset_table helper (UTF8_FULL + dynamic arrangement, same style as the markdown renderer and .list macros) and use it for the agents, skills, and macros listings. The skills loaded marker keeps its color; comfy-table's custom_styling feature accounts for ANSI sequences in column widths. --- src/config/request_context.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 7f19ad1..24597b6 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -112,6 +112,14 @@ fn print_asset_names(kind: &str, names: &[String]) -> Result<()> { Ok(()) } +fn asset_table(header: &[&str]) -> Table { + let mut table = Table::new(); + table.load_preset(UTF8_FULL); + table.set_content_arrangement(ContentArrangement::Dynamic); + table.set_header(header.to_vec()); + table +} + fn complete_skills_with_descriptions(names: Vec) -> Vec<(String, Option)> { names .into_iter() @@ -2593,10 +2601,8 @@ impl RequestContext { return Ok(()); } - let mut table = Table::new(); - table.load_preset(UTF8_FULL); - table.set_content_arrangement(ContentArrangement::Dynamic); - table.set_header(vec!["name", "source", "isolated", "state", "description"]); + let mut table = + asset_table(&["name", "source", "isolated", "state", "description"]); for row in &policy.macros { let source = macro_source_display(row.source); @@ -2627,15 +2633,13 @@ impl RequestContext { return Ok(()); } - println!("Agents:"); + let mut table = asset_table(&["name", "description"]); for (name, description) in entries { - if description.is_empty() { - println!(" • {name}"); - } else { - println!(" • {name} — {description}"); - } + table.add_row(vec![name, description]); } + println!("Agents:"); + println!("{table}"); Ok(()) } "skills" => { @@ -2681,16 +2685,18 @@ impl RequestContext { return Ok(()); } - println!("Skills:"); + let mut table = asset_table(&["loaded", "name", "description"]); for (name, description, loaded) in entries { let marker = if loaded { "✓".green().bold().to_string() } else { "✗".red().bold().to_string() }; - println!(" {marker} {name} — {description}"); + table.add_row(vec![marker, name, description]); } + println!("Skills:"); + println!("{table}"); Ok(()) } "tools" => { From 95dd31e24beb55f45baa798c2766c36ac0672afd Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 11:10:50 -0600 Subject: [PATCH 13/18] docs: cleaned up docs --- plans/custom-commands-design.md | 510 -------------------------------- 1 file changed, 510 deletions(-) delete mode 100644 plans/custom-commands-design.md diff --git a/plans/custom-commands-design.md b/plans/custom-commands-design.md deleted file mode 100644 index afd4f1f..0000000 --- a/plans/custom-commands-design.md +++ /dev/null @@ -1,510 +0,0 @@ -# Design: Macros as First-Class Custom Commands - -Status: DRAFT v2 — grounded in code (src/config/macros.rs, src/repl/mod.rs, -src/config/install_remote.rs, src/config/paths.rs). Supersedes v1, which -proposed a new markdown "commands" artifact before discovering macros already -cover ~80% of the feature. - -## 0. Decision record - -- **Naming: keep "macros"** (user decision). No rename, no new artifact type, - no new install filter — `--install-from --filter macros` already - exists in both the CLI enum (`InstallFilter`, config/mod.rs:403) and the - REPL parser (`install_remote_from_repl_args`). (Note: the flag may be - renamed to `--install` per plans/bundle-manifest-design.md §5; the filter - is unaffected.) Docs will state plainly: *macros are - coyote's custom commands* (README + config examples + `.help`). - "Macro" is also the more accurate term: these are replayable scripts of - REPL commands with variables, not just prompt templates. -- **Keep the `.macro` subcommand** alongside top-level invocation (user - decision): it hosts the interactive creator (which top-level must never - trigger), it is the escape hatch for macros shadowed by built-ins (incl. - future built-ins landing on existing macro names), and removing it breaks - muscle memory and macros whose steps invoke `.macro`. Cost of keeping: ~0. -- **Rejected**: separate markdown command files (redundant — a single-step - macro with one `rest` variable IS a prompt command, and macros additionally - do multi-step, role switching, and `` .file `cmd` `` shell embedding); - rename to "commands" (churn, dir migration, and the less accurate word); - an `alias` field (complexity without payoff — the file name is the command - name, matching the existing naming structure); command-side agent/role - binding (inverts contexts-curate-artifacts; lets installed bundles mutate - existing contexts). -- **Bundle manifest / provenance tracking is NOT in this plan.** It is an - orthogonal, separate design (see §10); this feature neither depends on nor - blocks it. - -## 1. What exists today (verified) - -- `Macro { variables: Vec, steps: - Vec }`, YAML at `macros_dir()/.yaml` (global only; no - workspace dir, unlike skills). -- `.macro [args]` executes; nonexistent name + no args opens the - interactive macro creator (`ctx.new_macro`). -- `macro_execute` forks a fresh `RequestContext` from the current role - (inherits role's model/temperature/enabled_tools/enabled_mcp_servers), - copies `last_message` in (discontinuous), sets `macro_flag`, runs each - `{{var}}`-interpolated step via `run_repl_command`. **No state flows back**: - the macro's exchanges are not recorded in the active session and do not - update the caller's `last_message`. -- Auto-generated usage strings; positional vars with defaults + rest-capture. -- `.list macros`, `.delete macro`, built-ins embedded from `assets/macros/`. -- Unknown dot-command → `unknown_command()` at repl/mod.rs:1521: - `Error: Unknown command. Type ".help" for additional help.` - -## 2. Gaps this design closes - -1. Top-level invocation: `.review-work args`, not just `.macro review-work args`. -2. Discoverability: `description` field → listings, completion, `.help`. -3. Context scoping: `enabled_macros` on role / agent (non-graph) / session / - global config, mirroring `enabled_skills`. -4. Runtime toggles: `.macro enable|disable `, shorthand for - `.set enabled_macros` (see §6). -5. Conversation-integrated macros: `isolated: false`. -6. Workspace-local macros: `.coyote/macros/` shadowing global, mirroring - workspace skills/MCP conventions (see §5). - -## 3. New Macro fields - -```yaml -description: Review WIP against a base branch # optional; shown in listings/completion/.help -isolated: false # optional; default TRUE (current behavior) -variables: - - name: base - default: main - - name: instructions - rest: true - default: "" -steps: - - "Review the diff against {{base}}. {{instructions}}" -``` - -Both new fields are `#[serde(default)]`-style optional → every existing -macro file remains valid; default `isolated: true` preserves current behavior -exactly. - -### `isolated` semantics - -- `true` (default, today's behavior): forked context as described in §1. - Right for utility macros (e.g. generate-commit-message) whose chatter - should not pollute the session. -- `false`: steps run via `run_repl_command(ctx, ...)` against the **live** - context — exactly as if the user had typed each step at the prompt - themselves. Prompts continue the actual conversation: recorded in the - active session, `last_message` updated, agent context preserved. - Consequence to document loudly: mutating steps (`.role x`, `.model y`) - **persist after the macro ends** — that is the meaning of non-isolation, - not a bug. -- **VERIFIED: the session-recording chain has no macro_flag check anywhere** - (`after_chat_completion` request_context.rs:1528-1544 → `save_message` - :1464-1474 → `Session::add_message` session.rs:724-763; disk save deferred - to `Session::exit` :626-631). Today's "macros aren't recorded" behavior - comes ENTIRELY from the fork (`session: None` in the fresh context), not - from the flag — so non-isolated execution records normally with zero - changes to the recording path. -- Footnotes to today's isolation (documented, not changed): (a) the forked - ctx still appends exchanges to the flat `messages.md` file via the - `save_message` fallthrough (request_context.rs:1518-1526) — only *session* - recording is skipped; (b) Arc-backed state copied into the fork - (supervisor, inbox, escalation_queue) is shared, so mutations through those - handles already reach the parent. -- Both modes: `macro_flag` is set for the duration (temporarily on the live - ctx for non-isolated, restored on exit **including the error path** — RAII - guard) so the existing forbidden-op guards (repl/mod.rs:890, 979, 1310) - apply, and nested `.macro`/top-level macro invocation inside a macro is - rejected in non-isolated mode (isolated mode already recurses safely via - `#[async_recursion]`; keep as-is). - Precise predicate (oracle note): reject when `macro_flag` is set AND the - CURRENT execution mode is non-isolated. An **isolated** macro's step - invoking a **non-isolated** macro runs it inline on the FORKED ctx - (harmless — the fork has no session); pin this behavior in the step-5 - test matrix. - Implementation footguns (oracle): (a) the RAII guard cannot hold - `&mut ctx.macro_flag` while `&mut ctx` is passed to `run_repl_command` — - wrap the whole `&mut RequestContext` (Drop restores prior mode) or use - save/restore around a closure; (b) prefer a COMPANION FIELD for the mode - over changing `macro_flag` to `Option` (the enum touches all 13 - sites incl. both fork-propagation sites; the companion field is the - smaller diff); (c) a separate `macro_execute_inline` fn creates a new - type-level recursion cycle with `run_repl_command` and needs its own - `#[async_recursion]`/boxing — a branch inside the already-boxed - `macro_execute` is free. - Semantics caveat to document (§8): steps are FAIL-FAST — a mid-macro error - aborts remaining steps while completed steps' mutations persist (slightly - stronger than "as if typed", where a human would continue past errors); - and a `.exit` step's exit signal is swallowed inside macros today (bool - discarded at macros.rs:67) — unchanged, but say so. -- **VERIFIED guard inventory** (13 `macro_flag` sites: 4 decl/init, 2 - propagation — `fork_for_branch` request_context.rs:276, `new_for_child` - :315 — and 7 behavioral). Under non-isolated execution: - - Keep as-is (desirable in a macro even on the live ctx): `.update` bail - (repl/mod.rs:890), `.edit` bail (:979 — no `$EDITOR` mid-macro), - blank-line suppression (:1310, cosmetic), `new_role` prompt→bail - (request_context.rs:2259), `new_macro` prompt→bail (:2360). - - Non-issue: `apply_prelude` skip (:3966) — on a live REPL ctx the prelude - already ran and `state()` is non-empty. - - **RULED (user, 2026-08-20): condition on isolation.** `use_agent` - (:3743-3749) suppresses the agent's default `agent_session` when - `macro_flag` is set; under `isolated: false` the suppression is LIFTED — - a non-isolated `.agent foo` step inherits foo's default session exactly - as if typed. Mechanism: the RAII guard records the mode (companion field - or `Option` replacing the bare bool) so use_agent can - distinguish isolated from non-isolated; isolated mode keeps today's - suppression verbatim. - - Nested-macro *execution* is currently unguarded (`new_macro` only blocks - the interactive creator) — the non-isolated nesting rejection is NEW - code, not a reuse of an existing check. - -## 4. Top-level invocation - -- Dispatch order in the REPL command match: **built-ins first**, then — where - `unknown_command()` fires today — look up visible macros by name (file - stem). Hit → execute exactly as `.macro ` would (honoring - `isolated`). Miss → existing `unknown_command()` error, verbatim, unchanged. - **VERIFIED insertion point**: the top-level catch-all `_ => - unknown_command()?` at repl/mod.rs:1297. The other `unknown_command()` - call sites (:630, :740, :763, :1236, :1259) are sub-argument mismatches - inside known commands and must NOT dispatch macros. -- `.macro` subcommand is kept verbatim for back-compat, including the - interactive-creation flow (creation stays ONLY under `.macro`; a top-level - typo like `.hi` must error, never open the creator). -- Both entry points (`.name` and `.macro name`) go through the same §5 - visibility check — otherwise `.macro` trivially bypasses `enabled_macros`. -- Collision rules: - - Macro name colliding with a **built-in** command: built-in always - wins; macro still invokable via `.macro `; flagged - `shadowed (built-in)` in `.list macros`. Built-in list sourced from the - existing `ReplCommand` registry, not a hardcoded copy. **VERIFIED shape**: - `static REPL_COMMANDS: LazyLock<[ReplCommand; 60]>` (repl/mod.rs:56-330), - `ReplCommand { name: &'static str, description, state: AssertState }` - (:554-571), with a test hardcoding the count (:1732). Macros must NOT be - added to this array (static, `&'static str`, fixed count). -- Tab completion: on `.`, visible macros appear alongside the usual - built-ins, with their `description` shown when available — joining the - built-in completer — as a SEPARATE dynamic source queried at - completion time: the completer clones REPL_COMMANDS at construction - (completer.rs:93-98) but already holds `Arc>` - (completer.rs:84), so it can query visible macros live. - **Shadowed macros are excluded from completion entirely** (RULED): a macro - whose name collides with a built-in is neither dispatchable via `.` - (built-in always wins) nor listed in `.` completions — it surfaces - only in `.list macros` as `shadowed (built-in)` and stays invokable via - `.macro `. -- **`.macro ` argument completion (RULED)**: upgraded to match `.` - presentation — macro names WITH descriptions when available. VERIFIED - today: request_context.rs:3004 completes `.macro` args via - `map_completion_values(paths::list_macros())` (names only, no - descriptions), while the plumbing already supports described suggestions - (`repl_complete` returns `(String, Option)`; `.model`/`.agent` - arms use it, rendered at completer.rs:58-60). Change the `.macro` arm to - the same resolver source as top-level completion. Differences from - `.`: shadowed macros ARE listed here (`.macro` is their escape - hatch), and the `enable`/`disable` subcommands appear alongside macro - names. Second-arg completion: `.macro enable ` / `.macro disable - ` complete toggle-eligible macro names. -- Completion entries for macros carry the same `AssertState` stance as the - `.macro` built-in (the completer filters on `cmd.is_valid(state)`, - completer.rs:46). -- `enable`/`disable` are sub-args of the existing `.macro` entry, NOT new - `REPL_COMMANDS` array entries (the count-asserting test at repl/mod.rs:1732 - stays untouched). - `.help` gains a "custom commands (macros)" section and - a line stating macros = custom commands. - -## 5. Scoping: `enabled_macros` - -New optional field, mirroring `enabled_skills` semantics **verbatim** — same -parser (`parse_string_or_array`: YAML list or comma-separated string), same -null/absent/empty behavior. **VERIFIED semantics** (resolver: -`SkillPolicy::effective_with`, skill_policy.rs:40-132; precedence :78-82; -regression test :388-404 pinning CHANGELOG:320): - -**Scope of "mirror" (explicit):** the mirroring covers ONLY the allowlist -resolution semantics (None/empty/populated meanings, first-`Some`-wins -precedence) and the config plumbing (where the field lives, how each level -parses it). It does NOT copy any LLM-facing skills machinery: skills feed -instruction injection and tool-scope refresh into the model payload — -macros have no analog of any of that. The LLM never learns macros exist or -ran (an isolated macro's exchanges arrive as ordinary messages on a fork; a -non-isolated macro's steps are indistinguishable from typed input). -`enabled_macros` gets its own small resolver over the discovered-files set — -it is NOT wired into `SkillPolicy`, prompt building, or context startup -(see "Lazy resolution" below). - -- `None`/absent = "no opinion" → fall through to the next level; all-`None` - → everything visible. -- `Some([])` (empty list, incl. empty string via the parsers) = **explicit - ZERO** — nothing enabled. Empty ≠ all; that was the regression. -- Populated = exactly those names. **Deliberate DIVERGENCE from skills on - unknown names**: skills hard-bail the whole resolution - (skill_policy.rs:94-103 — there is no warn path). For macros, hard - validation happens ONLY at `.set enabled_macros` time (the - request_context.rs:2709-2727 pattern: bail on a name that exists in - neither workspace nor global macros dir); CONFIG-FILE lists are validated - gracefully at resolution time — warn + `missing` row in `.list macros`, - never a bail (a stale name in a role file must not brick that role). - There is no `visible_macros` concept in v1. -- Precedence: `.or_else()` chain — session → agent → role → global, **first - `Some` wins outright**, no merging. Matches the ruling below. -- Plumbing gotcha: the global level is TWO structs (`Config` mod.rs:222 AND - `AppConfig` app_config.rs:44/:212) plus an env-override arm - (app_config.rs:532); role parses via frontmatter `parse_string_or_array` - (role.rs:131), session via plain serde — three parse paths to mirror. - -- Global config (`config.example.yaml`, alongside `enabled_skills`): default - when no role/agent/session is active. Absent/null = all macros visible. -- Role (`config.role.example.md` frontmatter), agent config - (`config.agent.example.yaml`), session config: allowlist for that context. -- **Graph-based agents — CORRECTED BY VERIFICATION, then RULED (user, - 2026-08-20): silently ignored, option (a).** - The prior ruling ("graph configs reject the field") is not implementable - as specified: coyote uses `deny_unknown_fields` nowhere except - mcp/mod.rs:79, so unknown fields in graph.yaml are silently ignored — and - `enabled_skills` is in fact SUPPORTED at graph level (`AgentConfig:: - from_graph` copies it at agent.rs:811; graph/llm.rs:195-226 swaps per-node - values with save/restore; validator enforces node⊆graph at - graph/validator.rs:1156,:1235). Ruling: `enabled_macros` is simply omitted - from the `Graph` struct — graph.yaml ignores it like any other unknown - field (zero code, consistent behavior); the omission is documented in the - wiki/agent docs. Full skills-style graph support was rejected as - meaningless (graph nodes never dispatch REPL commands); a bespoke - validator warning was considered and declined. -- Precedence: most-specific active context that defines the field wins — - session > agent > role > global. No merging/intersection. -- Allowlist entries are macro **names** (the file stem — the same identifier - used for invocation). - -### Workspace-local macros (RULED: in scope) - -Workspace macro definitions are allowed, mirroring the existing -workspace-artifact conventions (VERIFIED mechanics): - -- Discovery: `.coyote/macros/*.yaml` under `workspace_config_dir()` - (paths.rs:198-207 — CWD only, no ancestor walk-up, dir name overridable - via `COYOTE_WORKSPACE_CONFIG_DIR`), exactly like workspace skills - (`workspace_skills_dir()`, paths.rs:209-215). -- Collision rule: **workspace shadows global by name** — same as workspace - skills (`list_skills` iterates [workspace, global] with shadowing, - paths.rs:478-501; `has_skill` checks workspace first, :503-505) and - workspace MCP (HashMap insert, workspace wins, mcp/mod.rs:239). -- Opt-out mirrors MCP: new `--no-workspace-macros` CLI flag, modeled on - `--no-workspace-mcp` (cli/mod.rs:96-98 → main.rs:222-223). - IMPLEMENTATION-VERIFIED CORRECTION (T6): `no_workspace_mcp` is - CLI-flag-only — AppConfig field exists but there is NO Config-struct key - and NO env arm, so a config.yaml entry is non-functional. The exact-mirror - ruling therefore makes `no_workspace_macros` CLI-flag-only too. - (Pre-existing bug, out of scope: config.example.yaml:140 documents - `no_workspace_mcp:` as a yaml key even though it does nothing — follow-up.) -- Trust model: no confirmation prompt — consistent with workspace MCP and - skills, which load with no gate (mcp/mod.rs:225-268 merely eprintlns); - macros are strictly lower-risk since they run only on explicit user - invocation, never automatically. `.list macros` gains a source column - (`workspace` | `global`) so provenance is always visible. -- `enabled_macros` existence validation consults workspace-then-global (the - `has_skill` pattern). -- The interactive creator (`.macro `) continues to write to the - GLOBAL macros dir; workspace macros are authored by hand / committed to - the repo. Remote installs (`--filter macros`) also target global only — - bundles never write into a workspace. -- Agent plumbing (VERIFIED): `AgentConfig` agent.rs:694-764 (getter/setter - pattern :399-412), reached lazily from `RequestContext.agent` - (request_context.rs:147, set in `use_agent` :3670+). Policy is resolved - lazily at enforcement sites from `ctx.role/agent/session` — no - activation-time snapshot exists, which confirms the lazy-resolution ruling - fits the existing architecture exactly. -- Session-level note: `enabled_skills` has NO REPL setter today (session file - serde only). Runtime toggles do NOT touch session state — `.macro - enable|disable` edits the global-level in-memory list via - `update_app_config` (see §6), reusing the `.set` machinery. -- Unknown names in a config-file list: warning + `missing` row in `.list - macros` (per the divergence ruling above — resolution never bails). - -### Lazy resolution - -`enabled_macros` never touches the LLM payload (macros are invisible to the -model), so the visible set is computed on demand — at top-level dispatch -fallback, completion, `.list macros`, `.macro enable|disable` — from: -discovered files x active context's allowlist x runtime toggles. Zero -context-startup work; no new startup-ordering surface. Rescan both macro -dirs (workspace + global) on each resolution — two `read_dir`s; matches -`paths::list_macros()` cost today. - -## 6. REPL management surface - -- `.list macros` (enriched): name, description, isolated?, state: - `enabled` | `disabled (runtime)` | `locked` | `missing` | `shadowed - (built-in)` | `invalid`. `locked` names the restricting config (e.g. - `agent:oracle enabled_macros`). Plus a source column - (`workspace` | `global`). -- **`.macro enable ` / `.macro disable `** (RULED — replaces the - earlier `.command` proposal; user prefers no separate command at the cost - of reserving two names): shorthand for editing `.set enabled_macros`. - Consequently **`enable` and `disable` are reserved macro names**: the - creator rejects them, discovery marks such a file `invalid` (with a - warning), and remote installs warn. -- **`.set enabled_macros `** — new key in the `.set` match - (`RequestContext::update`, request_context.rs:2644-2929), mirroring - `enabled_skills` (:2706-2730) verbatim: `csv_to_vec` parsing (space-free, - comma-separated), `null` clears back to None, per-name existence - validation (workspace-then-global), then - `update_app_config(|app| app.enabled_macros = ...)`. -- **Mechanism makes the no-override rule structural** (VERIFIED): `.set` - writes the in-memory GLOBAL AppConfig level — the LOWEST-precedence rung - of the session > agent > role > global chain — in-memory only, never - persisted (`update_app_config` clone-and-swaps the Arc, - request_context.rs:347-354). So `.macro enable|disable` physically cannot - override a role/agent/session `enabled_macros` allowlist. When such an - allowlist is active, the toggle ERRORS (naming the owning config) instead - of silently writing a shadowed value. -- Toggle semantics over the global-level list: `disable X` with list=None - (all visible) materializes the list as all-discovered-minus-X; `enable X` - appends if absent (None → already visible → no-op notice); `disable X` - removes (absent → no-op notice). Lifetime: process runtime, like every - other `.set` key. -- Drive-by fix bundled here: `enabled_skills` is missing from the `.set` - key completion list (request_context.rs:3018-3047) although its setter - works — add both `enabled_skills` and `enabled_macros` to completion. - -States per macro per context: -1. permitted + enabled (default within allowlist) -2. permitted + disabled (runtime toggle) -3. locked (outside a ROLE/AGENT/SESSION allowlist) — not enableable from the - REPL; the error says to edit `enabled_macros` in the owning config. No - REPL override, ever: those configs stay the single source of truth. - -Boundary rule: `locked` applies ONLY to role/agent/session allowlists. A -global-level exclusion (whether from config.yaml or a prior toggle — they -occupy the same in-memory list and are indistinguishable) is always -toggleable via `.macro enable` and displays as `disabled (runtime)`. - -### Error handling matrix - -| Action | Condition | Behavior | -|---|---|---| -| `.name` | no built-in, no macro | existing error verbatim: `Error: Unknown command. Type ".help" for additional help.` | -| `.name` / `.macro name` | locked | error naming the restricting context/config | -| `.name` / `.macro name` | runtime-disabled | error: re-enable with `.macro enable ` | -| `.macro enable X` | locked (more-specific allowlist active) | error: "restricted by ; edit `enabled_macros` there" | -| `.macro enable X` | unknown | existing unknown-style error | -| `.macro enable X` | already enabled | no-op notice | -| `.macro create/creator` | name is `enable` or `disable` | error: reserved name | -| discovery | file named `enable`/`disable`.yaml | `invalid` in `.list macros` + warning | -| config `enabled_macros` | unknown name | warning + `missing` in `.list macros` | -| discovery | name shadows built-in | works via `.macro` only; `shadowed` in `.list macros` | -| discovery | workspace + global same name | workspace shadows global; both visible in `.list macros` source column | -| non-isolated macro | step invokes another macro | error: nested macros not allowed in non-isolated mode | - -## 7. Install & distribution - -Unchanged: `--install-from --filter macros` (CLI + REPL; flag rename to -`--install` tracked in plans/bundle-manifest-design.md §5). Installing macros -never modifies any `enabled_macros` list — a context with an allowlist is -unaffected by new installs until the user edits it (the security property -motivating context-side scoping). Existing overwrite/skip behavior applies; -install-time warning if an installed macro's name shadows a built-in. - -## 8. Docs - -- README + `.help`: "macros are coyote's custom commands" framing; top-level - invocation; `isolated` semantics with the role-switch-persists warning. -- `config.example.yaml`, `config.role.example.md`, - `config.agent.example.yaml`: `enabled_macros` entries mirroring the - existing `enabled_skills` doc comments. Do NOT add a `no_workspace_macros` - yaml entry — the flag is CLI-only (see §5 correction); the existing - `no_workspace_mcp` yaml line documents a dead key (pre-existing, follow-up). -- New `macro.example.yaml` (or extend existing docs) showing all fields incl. - description/isolated. -- Workspace macros: document `.coyote/macros/` alongside the existing - workspace skills/MCP conventions. - -## 9. Implementation sketch - -1. **Macro struct**: add `description`, `isolated` (serde defaults); - deser tests for back-compat with field-less YAML. -2. **`enabled_macros` field**: global AppConfig + role + agent (non-graph) + - session structs (both `Config` AND `AppConfig` at the global level + env - arm), via `parse_string_or_array` like role.rs:131; graph.yaml silently - ignores it (field omitted from the Graph struct, per §5 ruling); - precedence resolver + tests (incl. the empty-list-means-zero regression - case). -3. **Resolver**: discovered x allowlist x runtime toggles (global-level - in-memory) → - visible set + per-macro state; table-driven tests over the §6 matrix. - OWNS the two-dir discovery (`workspace_macros_dir()` + shadowing, the - has_skill/list_skills pattern) so steps 4/5/6 are genuinely independent. -4. **REPL dispatch**: macro fallback immediately before `unknown_command()`; - `.macro enable|disable` + `.set enabled_macros` key (+ completion - drive-by); enriched `.list macros`; dynamic macro completion with - shadowed-name exclusion; `.macro ` arg completion upgraded to - descriptions + subcommands (request_context.rs:3004); `.help`. Enforce - visibility on the `.macro` path too. -5. **Non-isolated execution**: `macro_execute_inline(ctx, ...)` variant (or - branch) running steps on the live ctx with RAII macro_flag guard + - nested-macro rejection; tests for flag restore on error, plus two - mock-free session tests (oracle note — no mock-client harness needed): - (i) the non-isolated path passes the LIVE ctx (session `Some`) into - `run_repl_command`, not a fork; (ii) a mutating step (`.model x`) - persists on the live ctx after the macro returns. Also pin - isolated→non-isolated nesting (§3). -6. **Workspace macros (flag + docs only; discovery lives in step 3)**: - `--no-workspace-macros` flag + `no_workspace_macros` config key, source - column in `.list macros`. -7. **Docs** (§8). - -Order: 1 → 2 → 3 → {4, 5, 6 in parallel} → 7. - -## 10. Follow-ups (out of scope) - -- Persisted toggles (surviving restart) — `.set` state is process-lifetime - by design; persistence would be new machinery for all `.set` keys, not - just macros. -- Bundle manifest & provenance layer — **separate design doc**, now written: - `plans/bundle-manifest-design.md` (per-file provenance recording at install - time, `--list-bundles` / `--update-bundle` / `--uninstall`, optional - author-shipped `coyote-bundle.yaml` manifest). Once it exists, `.list - macros` gains a "source bundle" column for free. This plan neither depends - on nor blocks it. -- "Did you mean" suggestions on unknown command (only if built-ins get it too). - -## 11. VERIFY before task materialization - -All items resolved 2026-08-20 (findings folded into §3/§4/§5 above): - -- [x] `enabled_skills` semantics: None=fall-through, empty=ZERO, populated= - exact+hard-bail validation; first-`Some`-wins precedence - (`SkillPolicy::effective_with`). See §5. -- [x] Agent config plumbing: lazy resolution from `ctx.agent`, no snapshot; - new field = AgentConfig field + accessor + resolver read. See §5. -- [x] `ReplCommand` registry: static fixed array, count-asserting test; - macros go in a separate dynamic completer source. See §4. -- [x] `macro_flag` guards: 13 sites inventoried; none suppress session - recording; one OPEN DECISION (`use_agent` agent_session suppression) - + nested-rejection is new code. See §3. -- [x] Session recording: `after_chat_completion` → `save_message` → - `Session::add_message`, no macro conditions; isolation lives in the - fork's `session: None`. See §3. - -Both open decisions ruled by the user 2026-08-20: §3 `use_agent` suppression -is conditioned on isolation (lifted for isolated:false); §5 graph.yaml -silently ignores `enabled_macros`. No open questions remain. - -Tooling warning for implementers: `fs_grep`/plain grep tools silently skip -src/config/request_context.rs (file-size exclusion) — audit that file with -ast_grep or targeted reads only. - -### Second verification round (2026-08-20, after user review) - -- [x] `.set` mechanics: fixed hand-written key match in - `RequestContext::update` (request_context.rs:2644-2929); - `enabled_skills` IS settable (:2706-2730) and always writes the - in-memory global AppConfig via `update_app_config` (:347-354) — - lowest precedence, never persisted, no role/agent/session setter. - Values: `csv_to_vec` comma lists (space-free; whitespace rejected for - all but two keys, :2654-2662), `null` clears. `enabled_skills` missing - from `.set` completion (:3018-3047) — oversight, fixed as drive-by. -- [x] Workspace discovery precedents: MCP = CWD-only probe - `.coyote/mcp.json` → `.coyote/.mcp.json` → `.mcp.json` - (paths.rs:217-230), workspace wins collisions via HashMap insert - (mcp/mod.rs:239), gated by `no_workspace_mcp` only, no trust prompt; - skills = `.coyote/skills/` shadowing global by name - (paths.rs:478-505); memory walks ancestors but MCP/skills do NOT. - Workspace macros copy the skills model + an MCP-style opt-out flag. From 03687c8981100ef9e67a0357be66416ee6985e67 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 11:40:39 -0600 Subject: [PATCH 14/18] feat: addressed review comments --- src/repl/completer.rs | 3 ++- src/repl/mod.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/repl/completer.rs b/src/repl/completer.rs index 17b4825..440edbb 100644 --- a/src/repl/completer.rs +++ b/src/repl/completer.rs @@ -81,7 +81,8 @@ impl Completer for ReplCompleter { .into_iter() .map(|(name, description)| (format!(".{name}"), description)) .filter(|(name, _)| { - command_filter.len() == 1 || name.starts_with(&command_filter[..2]) + command_filter.len() == 1 + || name.starts_with(command_filter.get(..2).unwrap_or(&command_filter)) }) .collect(); let macros = fuzzy_filter(macros, |(name, _)| name.as_str(), &command_filter); diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 77e9233..9eb1b33 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -1086,7 +1086,7 @@ pub async fn run_repl_command( macro_execute(ctx, name, extra, abort_signal.clone()).await?; } Some(MacroState::DisabledRuntime) => bail!( - r#"Macro '{name}' is disabled. Enable it with ".macro enable {name}""# + r#"Macro '{name}' is disabled. Re-enable it with ".macro enable {name}""# ), Some(MacroState::Locked { level }) => bail!( "Macro '{name}' is restricted by {} enabled_macros", From b43acac8eec338f6f65738df245c531311d02a0b Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 11:51:23 -0600 Subject: [PATCH 15/18] fix: surface macro parse errors on top-level invocation An invalid installed macro invoked as a top-level command fell through to the generic unknown-command error, while .macro reported the parse/validation failure. Both paths now surface the reason. --- src/repl/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 9eb1b33..6f7bbf2 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -1337,6 +1337,9 @@ pub async fn run_repl_command( "Macro '{name}' is restricted by {} enabled_macros", ctx.macro_lock_owner(*level) ), + Some(MacroState::Invalid { reason }) => { + bail!("Macro '{name}' is invalid: {reason}") + } _ => unknown_command()?, } } From 96e539062157c96ef82209d7277079c43380557f Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 12:03:11 -0600 Subject: [PATCH 16/18] test: use collision-proof temp dirs in macro_policy tests The with_macro_dirs fixture derived its temp-dir name from a wall-clock nanosecond timestamp, so parallel tests starting in the same clock tick shared a directory and saw each other's macro files (flaky on CI runners with coarse tick granularity). A process id + atomic counter makes the name unique by construction. --- src/config/macro_policy.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/config/macro_policy.rs b/src/config/macro_policy.rs index 6636778..4c08ed7 100644 --- a/src/config/macro_policy.rs +++ b/src/config/macro_policy.rs @@ -290,7 +290,8 @@ mod tests { use crate::utils::get_env_name; use serial_test::serial; use std::path::Path; - use std::{env, fs, time}; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::{env, fs, process}; fn valid_macro() -> Macro { Macro { @@ -748,10 +749,12 @@ mod tests { } fn with_macro_dirs(f: F) { - let unique = time::SystemTime::now() - .duration_since(time::UNIX_EPOCH) - .unwrap() - .as_nanos(); + static COUNTER: AtomicUsize = AtomicUsize::new(0); + let unique = format!( + "{}-{}", + process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + ); let root = env::temp_dir().join(format!("coyote-macro-policy-test-{unique}")); let workspace = root.join("workspace-macros"); let global = root.join("global-macros"); From bda37d9f38f0e87fe2033e20e2396154e0d1fcfc Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 12:15:31 -0600 Subject: [PATCH 17/18] docs: Added a description to the built-in `generate-commit-message` macro --- assets/macros/generate-commit-message.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/assets/macros/generate-commit-message.yaml b/assets/macros/generate-commit-message.yaml index 364cc26..3cc887c 100644 --- a/assets/macros/generate-commit-message.yaml +++ b/assets/macros/generate-commit-message.yaml @@ -1,2 +1,3 @@ +description: Generate a git commit message from the current diff steps: - - .file `git diff` -- generate a git commit message \ No newline at end of file + - .file `git diff` -- generate a git commit message From aea5f3d615d301171f7e62bcddce22e9fbd3c913 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 21 Aug 2026 12:22:48 -0600 Subject: [PATCH 18/18] test: updated embedded macro tests to expect descriptions for all built-in macros --- src/config/macros.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/config/macros.rs b/src/config/macros.rs index 93d9806..7a045e5 100644 --- a/src/config/macros.rs +++ b/src/config/macros.rs @@ -249,11 +249,11 @@ mod tests { use crate::config::{AppState, Session, WorkingMode}; use crate::utils::{create_abort_signal, get_env_name}; use serial_test::serial; - use std::env; use std::fs::{create_dir_all, remove_dir_all, write}; use std::future::Future; use std::path::PathBuf; use std::time::{SystemTime, UNIX_EPOCH}; + use std::{env, str}; struct TestConfigDirGuard { key: String, @@ -695,12 +695,11 @@ variables: fn embedded_macro_assets_deserialize_with_defaults() { for file in MacroAssets::iter() { let embedded = MacroAssets::get(&file).unwrap(); - let content = std::str::from_utf8(&embedded.data).unwrap(); + let content = str::from_utf8(&embedded.data).unwrap(); let m: Macro = serde_yaml::from_str(content) .unwrap_or_else(|e| panic!("asset '{}' failed to deserialize: {e}", file.as_ref())); - assert!(m.description.is_none(), "asset '{}'", file.as_ref()); assert!(m.isolated, "asset '{}'", file.as_ref()); assert!(!m.steps.is_empty(), "asset '{}'", file.as_ref()); }