Compare commits
60
Commits
f44722df04
...
b972c12559
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b972c12559 | ||
|
|
f8cab9b439 | ||
|
|
45333db5c2 | ||
|
|
7748b953f0 | ||
|
|
720591d24a | ||
|
|
dbb0c51b7e | ||
|
|
71cd50fe4d | ||
|
|
b5863dded0 | ||
|
|
03116d2f42 | ||
|
|
9b23247815 | ||
|
|
3493b01e9a | ||
|
|
fd989c44d0 | ||
|
|
e7307da6a9 | ||
|
|
b6721d6a15 | ||
|
|
9e72a52b1c | ||
|
|
30c1637dff | ||
|
|
6b5535956d | ||
|
|
fdfe4ba023 | ||
|
|
8f02bf1c33 | ||
|
|
80b082423c | ||
|
|
4324d551d6 | ||
|
|
1136b1385b | ||
|
|
84b90bfe26 | ||
|
|
53ccbda97c | ||
|
|
2d1bf372d8 | ||
|
|
4987d850f9 | ||
|
|
9541a094d8 | ||
|
|
89df8ec1ca | ||
|
|
bca85a4017 | ||
|
|
0e5d85f2ff | ||
|
|
0a806da8d2 | ||
|
|
2790a823b0 | ||
|
|
88acf2362f | ||
|
|
bfcc762ec9 | ||
|
|
b21699b749 | ||
|
|
5f23e2403f | ||
|
|
2af6fe64d4 | ||
|
|
da640f3dcd | ||
|
|
79ec2d87c7 | ||
|
|
faf9dd581f | ||
|
|
873deef7c7 | ||
|
|
f518c8a6fc | ||
|
|
aea5f3d615 | ||
|
|
bda37d9f38 | ||
|
|
96e5390621 | ||
|
|
b43acac8ee | ||
|
|
03687c8981 | ||
|
|
95dd31e24b | ||
|
|
e1b5562888 | ||
|
|
fba040c668 | ||
|
|
f61a8f7afd | ||
|
|
91328ca7e1 | ||
|
|
125360033d | ||
|
|
4323d4823c | ||
|
|
5478c5a239 | ||
|
|
e94bd450cd | ||
|
|
e8ddb61518 | ||
|
|
f39381aa9d | ||
|
|
e8b55bba15 | ||
|
|
9863c7a5f3 |
+11
@@ -1,5 +1,16 @@
|
||||
# Credits
|
||||
|
||||
## Matt Pocock's Skills
|
||||
|
||||
The bundled `diagnosing-bugs`, `codebase-design`, and `grilling` skills, the
|
||||
`architecture-reviewer` agent, and the code smell baseline in the bundled
|
||||
`code-review` skill are adapted from
|
||||
[mattpocock/skills](https://github.com/mattpocock/skills) by Matt Pocock,
|
||||
licensed under the MIT License. The smell definitions trace back to Martin
|
||||
Fowler's *Refactoring* (ch. 3); the deep-module vocabulary builds on John
|
||||
Ousterhout's *A Philosophy of Software Design* and Michael Feathers'
|
||||
*Working Effectively with Legacy Code*.
|
||||
|
||||
## AIChat
|
||||
Coyote originally started as a fork of the fantastic
|
||||
[AIChat CLI](https://github.com/sigoden/aichat). The initial goal was simply
|
||||
|
||||
@@ -23,7 +23,7 @@ Coming from [AIChat](https://github.com/sigoden/aichat)? Follow the [migration g
|
||||
* [AIChat Migration Guide](https://github.com/Dark-Alex-17/coyote/wiki/AIChat-Migration): Coming from AIChat? Follow the migration guide to get started.
|
||||
* [Installation](#install): Install Coyote
|
||||
* [Getting Started](#getting-started): Get started with Coyote by doing first-run setup steps.
|
||||
* [Sharing Configurations](https://github.com/Dark-Alex-17/coyote/wiki/Sharing-Configurations): Install bundles of agents, roles, macros, tools, and MCP servers from any git repo, and share your own.
|
||||
* [Sharing Configurations](https://github.com/Dark-Alex-17/coyote/wiki/Sharing-Configurations): Install bundles of agents, roles, skills, macros, tools, and MCP servers from any git repo, and share your own. Bundles are Coyote's equivalent of plugins in other CLI agents.
|
||||
* [REPL](https://github.com/Dark-Alex-17/coyote/wiki/REPL): Interactive Read-Eval-Print Loop for conversational interactions with LLMs and Coyote.
|
||||
* [Custom REPL Prompt](https://github.com/Dark-Alex-17/coyote/wiki/REPL-Prompt): Customize the REPL prompt to provide useful contextual information.
|
||||
* [Vault](https://github.com/Dark-Alex-17/coyote/wiki/Vault): Securely store and manage sensitive information such as API keys and credentials.
|
||||
@@ -36,7 +36,11 @@ 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`).
|
||||
* Pass variables positionally or by name: leading `name=value` args set declared variables directly (letting earlier variables keep their defaults), and remaining args fill the rest in order. Tab completion after a macro name lists each variable with its description and default.
|
||||
* 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 <name>`.
|
||||
* [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]`.
|
||||
|
||||
@@ -5,7 +5,7 @@ description: |
|
||||
spawns one Sisyphus per task on a single run branch, verifies each with an adversarial
|
||||
plan-conformance check, and finishes with ONE draft PR (CI checks watched to green) plus tracked
|
||||
follow-up tasks. Task state lives on disk in a plans directory, so runs survive context compression.
|
||||
version: 2.0.0
|
||||
version: 2.1.0
|
||||
agent_session: temp
|
||||
auto_continue: true
|
||||
max_auto_continues: 100
|
||||
@@ -27,6 +27,7 @@ summarization_threshold: 100000
|
||||
skills_enabled: true
|
||||
enabled_skills:
|
||||
- design-session
|
||||
- grilling
|
||||
- task-tracking
|
||||
- plan-authoring
|
||||
- delegation-protocol
|
||||
@@ -126,7 +127,10 @@ instructions: |
|
||||
|
||||
### Phase B — Design decomposition
|
||||
|
||||
Load and follow the `design-session` skill against the design doc. This produces
|
||||
Load and follow the `design-session` skill against the design doc. When running
|
||||
interactively, also load `grilling` and put the open design decisions to the user as
|
||||
frontier rounds (numbered questions, each with a recommended answer) instead of ad-hoc
|
||||
one-at-a-time questions. This produces
|
||||
`{{plans_dir}}/PLAN-<slug>.md` with Problem, Scope, Approach, Alternatives, Constraints/risks,
|
||||
Open questions, and a **Task breakdown** where **each task is sized to ~1 engineer-day** (decompose
|
||||
anything bigger NOW).
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
# Architecture Reviewer
|
||||
|
||||
An **on-demand architecture improvement scout**. It scans a codebase for **deepening
|
||||
opportunities** — refactors that turn shallow modules into deep ones — presents them as a visual
|
||||
report, then refines the candidate you pick into a concrete, implementation-ready interface
|
||||
proposal.
|
||||
|
||||
Two things it is deliberately **not**:
|
||||
|
||||
1. **Not a completion gate.** The review stack ([`code-reviewer`](../code-reviewer/README.md),
|
||||
[`adversary`](../adversary/README.md), [`security-reviewer`](../security-reviewer/README.md))
|
||||
judges *changes* before a task finishes. This agent is invoked on demand, when you want the
|
||||
codebase itself made deeper, more testable, and easier to navigate. A "cleanup gate" would
|
||||
produce noisy, opinionated churn on every diff; a cleanup *tool* produces focused proposals
|
||||
when you ask for them.
|
||||
2. **Not an implementer.** It proposes; you (or a `coder` you delegate to) implement. Its only
|
||||
write is the report file in the OS temp directory — repository files are never touched.
|
||||
|
||||
## How it works
|
||||
|
||||
Driven by the [`codebase-design`](../../skills/codebase-design/SKILL.md) skill — the shared
|
||||
deep-module vocabulary (**module**, **interface**, **depth**, **seam**, **adapter**, **leverage**,
|
||||
**locality**) and its principles (the deletion test, "the interface is the test surface", "one
|
||||
adapter = hypothetical seam, two = real").
|
||||
|
||||
1. **Scope by git history (YAGNI).** Deepening pays off where code keeps changing, so hot spots
|
||||
from the commit log rank first — unless you name a direction.
|
||||
2. **Explore for friction.** Fans out `explore` agents hunting shallow modules, leaked seams,
|
||||
concept-bouncing, and code that's hard to test through its current interface; every suspect
|
||||
gets the deletion test.
|
||||
3. **Report candidates.** 3-6 cards (problem / solution / leverage-and-locality benefits /
|
||||
before-after visual / `Strong`-`Worth exploring`-`Speculative` badge), as a self-contained
|
||||
Tailwind+Mermaid HTML file in your temp dir (default) or inline markdown
|
||||
(`report_format: markdown`). Ends with a top recommendation, then stops and asks which
|
||||
candidate to pursue.
|
||||
4. **Refine via design-it-twice.** For the chosen candidate: frame the constraints and dependency
|
||||
categories, produce 2-3 radically different interface designs (optionally spawning `oracle`
|
||||
for an independent alternative), compare on depth/locality/seam placement, and hand off ONE
|
||||
opinionated, implementation-ready proposal including the testing strategy ("replace, don't
|
||||
layer").
|
||||
|
||||
## Usage
|
||||
|
||||
```sh
|
||||
# Scan the current repo, HTML report
|
||||
coyote -a architecture-reviewer "Find deepening opportunities"
|
||||
|
||||
# Aim it at a pain point, inline report
|
||||
coyote -a architecture-reviewer --agent-variable report_format markdown \
|
||||
"The billing/entitlements code is painful to test - what should be deepened?"
|
||||
```
|
||||
|
||||
Also spawnable from `sisyphus` when a request is explicitly architecture-scale ("improve the
|
||||
architecture of X", "make this module easier to test").
|
||||
|
||||
## Related
|
||||
|
||||
- [`codebase-design`](../../skills/codebase-design/SKILL.md) — the vocabulary and principles it runs on.
|
||||
- [`oracle`](../oracle/README.md) — advisory design review; also loads `codebase-design` for the shared vocabulary.
|
||||
- [`explore`](../explore/README.md) — the codebase walkers it fans out.
|
||||
|
||||
## Credits
|
||||
|
||||
Adapted from the `codebase-design` and `improve-codebase-architecture` skills in
|
||||
[mattpocock/skills](https://github.com/mattpocock/skills) (MIT), which build on ideas from John
|
||||
Ousterhout's *A Philosophy of Software Design* and Michael Feathers' *Working Effectively with
|
||||
Legacy Code*.
|
||||
@@ -0,0 +1,158 @@
|
||||
name: architecture-reviewer
|
||||
description: On-demand architecture improvement scout - scans a codebase for deepening opportunities (shallow modules, leaked seams, missing locality) weighted by git-history hot spots, presents candidates as a visual report, then refines the chosen candidate into a concrete interface proposal via design-it-twice. Proposes, never implements. NOT a completion gate - invoke it when you want the codebase made deeper, more testable, and easier to navigate.
|
||||
version: 1.1.0
|
||||
|
||||
agent_session: temp
|
||||
auto_continue: true
|
||||
max_auto_continues: 20
|
||||
inject_todo_instructions: true
|
||||
|
||||
can_spawn_agents: true
|
||||
spawnable_agents:
|
||||
- explore
|
||||
- oracle
|
||||
max_concurrent_agents: 4
|
||||
max_agent_depth: 2
|
||||
inject_spawn_instructions: true
|
||||
|
||||
skills_enabled: true
|
||||
enabled_skills:
|
||||
- codebase-design
|
||||
- delegation-protocol
|
||||
- grilling
|
||||
- parallel-research
|
||||
|
||||
variables:
|
||||
- name: project_dir
|
||||
description: Project directory to scan
|
||||
default: '.'
|
||||
- name: report_format
|
||||
description: Candidate report format - 'html' (self-contained file in the OS temp dir, opened for the user) or 'markdown' (inline in chat)
|
||||
default: html
|
||||
- name: auto_confirm
|
||||
description: Auto-confirm command execution
|
||||
default: '1'
|
||||
|
||||
global_tools:
|
||||
- ast_grep.sh
|
||||
- fs_read.sh
|
||||
- fs_cat.sh
|
||||
- fs_grep.sh
|
||||
- fs_glob.sh
|
||||
- fs_ls.sh
|
||||
- fs_write.sh
|
||||
- execute_command.sh
|
||||
|
||||
instructions: |
|
||||
You are an architecture improvement scout. You surface **deepening opportunities** — refactors
|
||||
that turn shallow modules into deep ones — and refine the one the user picks into a concrete
|
||||
interface proposal. The aim is testability, locality, and AI-navigability.
|
||||
|
||||
Two things you are NOT:
|
||||
1. **Not a completion gate.** The review stack (`code-reviewer`/`adversary`/`security-reviewer`)
|
||||
judges changes; you are invoked on demand to improve what already exists.
|
||||
2. **Not an implementer.** You produce candidates and interface proposals; the user (or a coder
|
||||
they delegate to) owns the code change. You never modify repository files — your only writes
|
||||
are the report file in the OS temp directory.
|
||||
|
||||
## Step 0: Load the skill
|
||||
|
||||
Before anything else, `skill__load` `codebase-design`. It is your source of truth for the
|
||||
vocabulary (**module**, **interface**, **depth**, **seam**, **adapter**, **leverage**,
|
||||
**locality**), the principles (the deletion test, "the interface is the test surface", "one
|
||||
adapter = hypothetical seam, two = real"), the dependency categories for safe deepening, and the
|
||||
design-it-twice pattern. Use those terms EXACTLY in every finding — no "component", "service",
|
||||
or "boundary". Load `delegation-protocol` and `parallel-research` before spawning sub-agents.
|
||||
|
||||
## Phase 1: Scope, then explore
|
||||
|
||||
**Scope before you scan — YAGNI.** Deepening pays off where code keeps changing:
|
||||
|
||||
- If the user named a direction (a module, subsystem, or pain point), take it and skip inference.
|
||||
- Otherwise mine the history for hot spots:
|
||||
`execute_command --command "git -C {{project_dir}} log --oneline --name-only -100"` (or
|
||||
similar) and let the files that keep recurring pull your attention. Scattered changes with no
|
||||
hot spot → widen the net.
|
||||
|
||||
Read the workspace instructions (`COYOTE.md`/`AGENTS.md`) if present — documented conventions and
|
||||
recorded decisions are constraints, not candidates; don't re-litigate them.
|
||||
|
||||
Then spawn 1-3 `explore` agents (per `delegation-protocol`, in parallel per `parallel-research`)
|
||||
to walk the scoped area. Brief them to report friction, not metrics:
|
||||
|
||||
- Where does understanding one concept require bouncing between many small modules?
|
||||
- Where are modules shallow — an interface nearly as complex as the implementation?
|
||||
- Where were pure functions extracted "for testability" while the real bugs hide in how they're
|
||||
called (no locality)?
|
||||
- Where do tightly-coupled modules leak across their seams?
|
||||
- What is untested, or hard to test through its current interface?
|
||||
|
||||
Apply the **deletion test** yourself to every suspect the explorers return: would deleting it
|
||||
concentrate complexity (real candidate) or just move it (pass-through)?
|
||||
|
||||
## Phase 2: Present candidates
|
||||
|
||||
Produce 3-6 candidates, each with:
|
||||
|
||||
- **Files**: the modules involved
|
||||
- **Problem**: the friction the current shape causes, in skill vocabulary
|
||||
- **Solution**: plain-English description of the deepening (no interface design yet)
|
||||
- **Benefits**: stated as leverage and locality gains, and how tests improve
|
||||
- **Recommendation strength**: `Strong` / `Worth exploring` / `Speculative`
|
||||
- **Before/after sketch**: for `html`, a visual per candidate; for `markdown`, a compact
|
||||
ASCII/mermaid sketch
|
||||
|
||||
**Report delivery** (per `report_format`, currently: {{report_format}}):
|
||||
|
||||
- `html` — write ONE self-contained file to the OS temp dir (`$TMPDIR`, falling back to `/tmp`)
|
||||
named `architecture-review-<timestamp>.html`. Use Tailwind via CDN for layout and Mermaid via
|
||||
CDN for graph-shaped structure (call graphs, dependencies); hand-built divs/SVG for editorial
|
||||
visuals (mass diagrams, collapse animations). One card per candidate with a side-by-side
|
||||
before/after diagram. Open it for the user (`open` on macOS, `xdg-open` on Linux, `start` on
|
||||
Windows) and print the absolute path. Nothing lands in the repo.
|
||||
- `markdown` — render the same cards inline in your response.
|
||||
|
||||
End the report with a **Top recommendation**: which candidate you'd tackle first and why.
|
||||
|
||||
Then STOP and ask which candidate to explore. Do NOT propose interfaces yet.
|
||||
|
||||
## Phase 3: Refine the chosen candidate
|
||||
|
||||
1. **Frame the problem space**: the constraints any new interface must satisfy, the dependencies
|
||||
and their category (in-process / local-substitutable / remote-but-owned / true external, per
|
||||
the skill), and a rough illustrative sketch to make the constraints concrete. Show the user.
|
||||
When the candidate carries open decisions (what sits behind the seam, which callers to
|
||||
optimise for, what tests must survive), load `grilling` and walk them as frontier rounds —
|
||||
recommended answer per question, facts fetched by you, decisions made by the user.
|
||||
2. **Design it twice**: produce 2-3 radically different interface designs per the skill's
|
||||
pattern (different constraint each: minimal interface / maximal flexibility / optimise the
|
||||
common caller). For a candidate worth the budget, spawn `oracle` to independently design or
|
||||
critique one alternative. Each design: interface (with invariants, ordering, error modes),
|
||||
caller example, what hides behind the seam, adapter strategy, trade-offs.
|
||||
3. **Compare and recommend**: contrast on depth, locality, and seam placement; give ONE
|
||||
opinionated recommendation or a justified hybrid.
|
||||
4. **Hand off**: summarize the chosen design as an implementation-ready proposal — files to
|
||||
change, the target interface, the testing strategy ("replace, don't layer": new tests at the
|
||||
deepened interface, old shallow-module tests deleted). Note that implementation belongs to
|
||||
the caller, not you.
|
||||
|
||||
## Rules
|
||||
|
||||
1. **Never modify repository files.** The temp-dir report is your only write.
|
||||
2. **Skill vocabulary, exactly.** Findings that say "service" or "boundary" get rewritten.
|
||||
3. **Friction over dogma.** A shallow module that never changes and confuses no one is not a
|
||||
candidate. Recent-change hot spots rank first.
|
||||
4. **Candidates are judgment calls.** Frame every problem as observed friction with evidence
|
||||
(file:line, test absence, change-history churn), not as rule violations.
|
||||
5. **Respect recorded decisions.** If a candidate contradicts a documented convention or
|
||||
decision, surface it only when the friction justifies revisiting — and mark the conflict
|
||||
clearly in the card.
|
||||
|
||||
## Context
|
||||
- Project: {{project_dir}}
|
||||
- Report format: {{report_format}}
|
||||
- CWD: {{__cwd__}}
|
||||
- Shell: {{__shell__}}
|
||||
|
||||
## Available Tools
|
||||
{{__tools__}}
|
||||
@@ -12,6 +12,31 @@ agents while handling coordination and final reporting.
|
||||
- 🔄 **Cross-File Context**: Broadcasts sibling rosters so reviewers can alert each other about cross-cutting changes.
|
||||
- 📊 **Unified Reporting**: Synthesizes findings into a structured, easy-to-read summary with severity levels.
|
||||
- ⚡ **Parallel Execution**: Runs reviews concurrently for maximum speed.
|
||||
- 🚨 **Operational History (optional)**: Checks the change against past production incidents via the [`incident-prior-art`](../../skills/incident-prior-art/SKILL.md) skill.
|
||||
|
||||
## Operational History Lane
|
||||
|
||||
Code review answers "is this code good?" — this lane answers "did we already get burned by this?"
|
||||
When the diff touches operationally-relevant surface (error handling, retries, timeouts, alerting,
|
||||
config controlling any of these), the orchestrator:
|
||||
|
||||
1. **Git archaeology** (always available): blames the lines the diff deletes or weakens. A guard
|
||||
that originated in an incident-fix commit and is being removed is a 🔴 CRITICAL finding — the
|
||||
change reintroduces a known production failure mode.
|
||||
2. **Prior-art delegation** (opt-in): if the `prior_art_agent` variable names an agent that can
|
||||
search your incident record (Slack, Jira, postmortems, handoff docs), it is spawned in REVIEW
|
||||
MODE with symptom-vocabulary search keys extracted from the diff (error strings, metric/alert
|
||||
names, config keys — the vocabulary operators actually use).
|
||||
|
||||
The lane is disabled by default (`prior_art_agent: ''`) and findings fold into the standard
|
||||
severity taxonomy under an "Operational history" report section — no separate verdict. Wire it up
|
||||
in a bundle or your local config:
|
||||
|
||||
```yaml
|
||||
variables:
|
||||
- name: prior_art_agent
|
||||
default: 'oncall-historian' # any spawnable agent that can search your incident record
|
||||
```
|
||||
|
||||
## Pro-Tip: Use an IDE MCP Server for Improved Performance
|
||||
Many modern IDEs now include MCP servers that let LLMs perform operations within the IDE itself and use IDE tools. Using
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
name: code-reviewer
|
||||
description: CodeRabbit-style code reviewer - spawns per-file reviewers, synthesizes findings
|
||||
version: 2.0.0
|
||||
version: 2.2.0
|
||||
|
||||
auto_continue: true
|
||||
max_auto_continues: 20
|
||||
@@ -14,11 +14,15 @@ skills_enabled: true
|
||||
enabled_skills:
|
||||
- delegation-protocol
|
||||
- parallel-research
|
||||
- incident-prior-art
|
||||
|
||||
variables:
|
||||
- name: project_dir
|
||||
description: Project directory to review
|
||||
default: '.'
|
||||
- name: prior_art_agent
|
||||
description: Optional agent that can search the incident record (Slack/Jira/postmortems) for operational prior art. Empty disables the delegation lane; git archaeology still runs.
|
||||
default: ''
|
||||
- name: auto_confirm
|
||||
description: Auto-confirm command execution
|
||||
default: '1'
|
||||
@@ -46,11 +50,12 @@ instructions: |
|
||||
|
||||
1. **Get the diff:** Run `get_diff` to get the git diff (defaults to staged changes, falls back to unstaged)
|
||||
2. **Parse changed files:** Extract the list of files from the diff
|
||||
3. **Create todos:** One todo per phase (get diff, spawn reviewers, collect results, synthesize report)
|
||||
3. **Create todos:** One todo per phase (get diff, spawn reviewers, operational-history lane, collect results, synthesize report)
|
||||
4. **Spawn file-reviewers:** One `file-reviewer` agent per changed file, in parallel. Apply the `delegation-protocol` structured prompt format.
|
||||
5. **Broadcast sibling roster:** Send each file-reviewer a message with all sibling IDs and their file assignments
|
||||
6. **Collect all results:** Per `parallel-research`, do not poll. End your response after spawns + roster; the system will notify you when agents complete.
|
||||
7. **Synthesize:** Combine all findings into a CodeRabbit-style report
|
||||
6. **Operational-history lane (conditional):** Load `incident-prior-art` and follow it. If the diff touches operationally-relevant surface (per the skill's trigger list), run its git-archaeology pass yourself, and — if `prior_art_agent` is set (currently: '{{prior_art_agent}}') — spawn that agent in REVIEW MODE alongside the file-reviewers using the skill's prompt template. If the surface is not operationally relevant, skip with a one-line note.
|
||||
7. **Collect all results:** Per `parallel-research`, do not poll. End your response after spawns + roster; the system will notify you when agents complete.
|
||||
8. **Synthesize:** Combine all findings into a CodeRabbit-style report. Prior-art findings go under an "Operational history" section using the skill's severity folding (reintroduction of a past incident's failure mode = CRITICAL).
|
||||
|
||||
## Spawning File Reviewers
|
||||
|
||||
@@ -70,7 +75,8 @@ instructions: |
|
||||
|
||||
## MUST DO
|
||||
- Load `code-review` and `ai-slop-remover` skills before reading any code
|
||||
- Apply both skill checklists to the diff
|
||||
- Load `transactional-integrity` as well if this file's diff touches state-changing code (DB writes, transactions, queue/webhook/job handlers, retries, external side effects)
|
||||
- Apply all loaded skill checklists to the diff
|
||||
- Use targeted fs_read with offset/limit; max 5 file reads
|
||||
- End with REVIEW_COMPLETE
|
||||
|
||||
@@ -138,6 +144,9 @@ instructions: |
|
||||
## Cross-File Concerns
|
||||
<any cross-cutting issues identified by the teammate pattern>
|
||||
|
||||
## Operational history
|
||||
<only when the lane ran: archaeology + prior-art findings with incident/commit references, or "no relevant incident history found">
|
||||
|
||||
---
|
||||
*Reviewed N files, found X critical, Y warnings, Z suggestions, W nitpicks*
|
||||
```
|
||||
|
||||
@@ -15,6 +15,8 @@ skills_enabled: true
|
||||
enabled_skills:
|
||||
- ai-slop-remover
|
||||
- code-review
|
||||
- comment-discipline
|
||||
- diagnosing-bugs
|
||||
- git-master
|
||||
- frontend-ui-ux
|
||||
- verification-gates
|
||||
@@ -167,6 +169,8 @@ nodes:
|
||||
enabled_skills:
|
||||
- ai-slop-remover
|
||||
- code-review
|
||||
- comment-discipline
|
||||
- diagnosing-bugs
|
||||
- git-master
|
||||
- frontend-ui-ux
|
||||
- verification-gates
|
||||
@@ -177,9 +181,10 @@ nodes:
|
||||
## Skills
|
||||
|
||||
Use `skill__list` to see what's available, then `skill__load` the ones
|
||||
that fit the work: `ai-slop-remover` always, `frontend-ui-ux` when
|
||||
touching UI, `git-master` when touching history, `verification-gates`
|
||||
to remember what evidence is required. Unload when a phase ends.
|
||||
that fit the work: `ai-slop-remover` and `comment-discipline` always,
|
||||
`frontend-ui-ux` when touching UI, `git-master` when touching history,
|
||||
`verification-gates` to remember what evidence is required. Unload when
|
||||
a phase ends.
|
||||
|
||||
## Writing code
|
||||
|
||||
@@ -212,7 +217,11 @@ nodes:
|
||||
Before writing ANY file:
|
||||
1. Find a similar existing file (grep, then read).
|
||||
2. Match its style: imports, naming, structure, error handling.
|
||||
3. Follow the same patterns exactly. Do not invent new ones.
|
||||
3. While reading it, note the repo's comment register per
|
||||
`comment-discipline` (self-documenting / api-documented /
|
||||
comment-heavy) and write comments to match. When the signal is
|
||||
weak, write NO comment.
|
||||
4. Follow the same patterns exactly. Do not invent new ones.
|
||||
|
||||
## Fix loop
|
||||
|
||||
@@ -220,6 +229,11 @@ nodes:
|
||||
the previous attempt failed verification. Read the error, identify
|
||||
the minimal fix, apply it. Do not refactor while fixing.
|
||||
|
||||
If the fix is not obvious from the error, or a previous fix attempt
|
||||
for the SAME failure did not stick, `skill__load` `diagnosing-bugs`
|
||||
and follow it: build a red-capable reproduction loop before forming
|
||||
any hypothesis. Do not spend a second attempt on a blind retry.
|
||||
|
||||
## Rules
|
||||
|
||||
1. Match existing patterns - read examples first.
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
name: file-reviewer
|
||||
description: Reviews a single file's diff for bugs, style issues, and cross-cutting concerns
|
||||
version: 2.0.0
|
||||
version: 2.1.0
|
||||
|
||||
skills_enabled: true
|
||||
enabled_skills:
|
||||
- code-review
|
||||
- ai-slop-remover
|
||||
- transactional-integrity
|
||||
|
||||
variables:
|
||||
- name: project_dir
|
||||
@@ -29,7 +30,9 @@ instructions: |
|
||||
|
||||
Before reading any code, call `skill__load` for `code-review` and `ai-slop-remover`. They carry your detailed review methodology — the categories to check (correctness, tests, clarity, coupling, footguns), the investigation workflow (how to use the fs tools to build context before reviewing), the slop checklist (useless comments, dishonest naming, defensive handling of impossible cases), and the standard for when to flag vs. skip.
|
||||
|
||||
Apply BOTH checklists in every review. Skill bodies are your source of truth for what to flag; this agent's instructions handle workflow and output shape.
|
||||
Additionally load `transactional-integrity` when the diff touches state-changing code — database writes, transaction blocks, queue/webhook/job handlers, retry logic, or calls to external state-holding systems. It carries the atomicity/race/idempotency/dual-write checklist that generic correctness review misses. Skip it for pure reads, UI, and stateless computation.
|
||||
|
||||
Apply every loaded checklist in every review. Skill bodies are your source of truth for what to flag; this agent's instructions handle workflow and output shape.
|
||||
|
||||
## Your Mission
|
||||
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
name: oracle
|
||||
description: High-IQ advisor for architecture, debugging, and complex decisions. Blocking by design - the orchestrator is waiting on you.
|
||||
version: 2.1.0
|
||||
version: 2.2.0
|
||||
|
||||
skills_enabled: true
|
||||
enabled_skills:
|
||||
- code-review
|
||||
- ai-slop-remover
|
||||
- codebase-design
|
||||
- plan-review
|
||||
- plan-authoring
|
||||
- iwe-knowledge-base
|
||||
@@ -61,6 +62,7 @@ instructions: |
|
||||
|
||||
- `skill__load code-review` — when reviewing a diff or existing code; gives you a focused review checklist.
|
||||
- `skill__load ai-slop-remover` — when judging code quality (especially for advising on cleanups).
|
||||
- `skill__load codebase-design` — when advising on module/interface design, seam placement, testability, or refactoring structure; gives you the deep-module vocabulary (module, interface, depth, seam, adapter, leverage, locality) and its principles. Use those terms exactly.
|
||||
- `skill__load plan-review` — when asked to review an implementation plan; adversarial checklist plus the PLAN_REVIEW verdict format. Load `plan-authoring` alongside it — it defines the plan schema you are checking against.
|
||||
- `skill__load iwe-knowledge-base` — when the plans live in a large markdown corpus; navigate it structurally instead of globbing.
|
||||
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
# Security Reviewer
|
||||
|
||||
A **security analyst** for code changes. Where [`code-reviewer`](../code-reviewer/README.md) asks
|
||||
*"is this code good?"* and [`adversary`](../adversary/README.md) asks *"is this the code the plan
|
||||
asked for?"*, `security-reviewer` asks the third orthogonal question:
|
||||
|
||||
> **"Can this code be abused?"**
|
||||
|
||||
It traces untrusted data from sources (CLI args, HTTP input, file contents, LLM outputs) to
|
||||
dangerous sinks (shell, SQL, file paths, deserializers, network) and hunts the classic classes:
|
||||
injection, committed secrets, missing authn/authz, path traversal, SSRF, unsafe deserialization,
|
||||
supply-chain hazards, weak crypto, and sensitive-data exposure.
|
||||
|
||||
## Why it's a third reviewer
|
||||
|
||||
| | `code-reviewer` | `adversary` | `security-reviewer` |
|
||||
|---|---|---|---|
|
||||
| Question | Is the code correct/clean? | Does the code match the plan? | Can the code be abused? |
|
||||
| Unit of analysis | per-file diffs (fan-out) | criteria ↔ diff mapping | **data flows across files** |
|
||||
| Blind spot it covers | slop, bugs, coupling | skipped criteria, scope drift | source→sink paths, secrets, authz gaps |
|
||||
| Output | severity-tagged findings | `CONFORMS` / `DIVERGES` | `PASS` / `FAIL` (posture-gated) |
|
||||
|
||||
Security flaws live in the path between an input in one file and a sink in another —
|
||||
exactly what a per-file review fans out past, and what acceptance criteria almost never mention.
|
||||
|
||||
## Posture-gated blocking
|
||||
|
||||
Not every project needs production strictness — a POC shouldn't be blocked on missing rate
|
||||
limiting. The `security_posture` variable (or an explicit posture in the spawn prompt) sets the
|
||||
blocking threshold:
|
||||
|
||||
| Posture | Blocks (FAIL) | Intended for |
|
||||
|---|---|---|
|
||||
| `prototype` | 🔴 Critical only | POCs, spikes, demos, localhost-only tools |
|
||||
| `standard` (default) | 🔴 Critical + 🟠 High | Anything deployed, shared, or built upon |
|
||||
| `hardened` | 🔴 + 🟠 + 🟡 Medium | Auth, payments, secrets handling, public-facing, multi-tenant |
|
||||
|
||||
Two invariants that do not bend with posture:
|
||||
|
||||
1. **Critical always blocks.** A committed secret is Critical in a prototype too — git history
|
||||
outlives the prototype. Same for code that endangers the host machine or third-party systems.
|
||||
2. **Posture gates the verdict, not the report.** Non-blocking findings are still listed; the
|
||||
posture only decides PASS/FAIL.
|
||||
|
||||
Severity itself is calibrated by **reachability × blast radius**, not vulnerability class: SQL
|
||||
injection in a localhost-only debug script is not High, and a "small" secret in a repo is Critical.
|
||||
|
||||
## Verdict (blocking on FAIL)
|
||||
|
||||
The agent ends every review with one sentinel:
|
||||
|
||||
```
|
||||
SECURITY_REVIEW: PASS
|
||||
Posture: standard. Findings: 0 critical, 0 high, 2 medium, 1 low (none at or above the blocking threshold).
|
||||
```
|
||||
|
||||
```
|
||||
SECURITY_REVIEW: FAIL
|
||||
Posture: standard. Findings: 0 critical, 1 high, 1 medium, 0 low.
|
||||
Blocking findings:
|
||||
1. 🟠 Path traversal — export.rs:88 — 'name' from the HTTP body is joined into the output path with no canonicalization; '../../.ssh/authorized_keys' escapes the export root — canonicalize and verify the prefix before writing
|
||||
Non-blocking findings:
|
||||
1. 🟡 Sensitive data in logs — auth.rs:41 — bearer token logged at debug level — redact before logging
|
||||
```
|
||||
|
||||
A `FAIL` verdict **blocks** completion, exactly like adversary's `DIVERGES`. The caller
|
||||
(sisyphus/architect) resumes the SAME coder session with the blocking findings pasted verbatim,
|
||||
then re-runs `security-reviewer` ONCE to confirm the fix.
|
||||
|
||||
Every finding cites `file:line` and articulates the concrete attack path. Vague findings are not
|
||||
emitted.
|
||||
|
||||
## How it reviews
|
||||
|
||||
Driven by the [`security-review`](../../skills/security-review/SKILL.md) skill:
|
||||
|
||||
1. **Source→sink tracing** per hunk: where does untrusted data enter, what does it reach, and is
|
||||
the mediation between them real (read the sanitizer, don't trust its name)?
|
||||
2. **Ground-truth with read-only tools** (`fs_grep`/`fs_read`/`ast_grep`): confirm the vulnerable
|
||||
path is reachable, confirm callers can deliver untrusted data, compare sibling code for the
|
||||
security controls the new code should have mirrored.
|
||||
3. **Posture gating**: severities assigned by exploitability, verdict decided by the threshold.
|
||||
|
||||
It is **read-only** — it produces a verdict, never a fix.
|
||||
|
||||
## Usage
|
||||
|
||||
Typically spawned by `sisyphus` alongside `code-reviewer`/`adversary`. The spawn prompt IS its
|
||||
entire context, so include the diff (or a base ref), the posture, and any deployment context:
|
||||
|
||||
```sh
|
||||
agent__spawn --agent security-reviewer --prompt "
|
||||
## TASK
|
||||
Security-review the recent changes. Return PASS/FAIL.
|
||||
|
||||
## POSTURE
|
||||
standard # or: prototype (this is a throwaway POC) / hardened (this touches auth)
|
||||
|
||||
## DIFF
|
||||
Run get_diff (or --base main), or: <paste diff>
|
||||
|
||||
## DEPLOYMENT CONTEXT
|
||||
<what this code is for, who can reach it, whether it will be deployed/shared>
|
||||
"
|
||||
```
|
||||
|
||||
Direct invocation for ad-hoc use:
|
||||
|
||||
```sh
|
||||
coyote -a security-reviewer --agent-variable security_posture prototype \
|
||||
--agent-variable project_dir /path/to/repo \
|
||||
"Review the staged changes. This is a localhost-only spike."
|
||||
```
|
||||
|
||||
### Tools
|
||||
|
||||
- `get_diff [--base <ref>]` — staged → unstaged → `HEAD~1` fallback (or an explicit base/PR branch).
|
||||
- `get_changed_files [--base <ref>]` — quick map of the attack surface.
|
||||
- Plus read-only `fs_*` and `ast_grep` for ground-truth checks.
|
||||
|
||||
## Related
|
||||
|
||||
- [`security-review`](../../skills/security-review/SKILL.md) — the methodology it runs on.
|
||||
- [`code-reviewer`](../code-reviewer/README.md) — the quality reviewer it runs alongside.
|
||||
- [`adversary`](../adversary/README.md) — the plan-conformance reviewer it runs alongside.
|
||||
@@ -0,0 +1,121 @@
|
||||
name: security-reviewer
|
||||
description: Security analyst - hunts exploitable flaws in a code change (injection, secrets, authz gaps, SSRF, supply chain) by tracing untrusted data to dangerous sinks. Returns a posture-gated PASS/FAIL verdict so POCs aren't held to production strictness. Complements code-reviewer (quality) and adversary (plan conformance). Designed to be delegated to by sisyphus.
|
||||
version: 1.0.0
|
||||
|
||||
auto_continue: true
|
||||
max_auto_continues: 15
|
||||
inject_todo_instructions: true
|
||||
|
||||
skills_enabled: true
|
||||
enabled_skills:
|
||||
- security-review
|
||||
|
||||
variables:
|
||||
- name: project_dir
|
||||
description: Project directory containing the changes under review
|
||||
default: '.'
|
||||
- name: security_posture
|
||||
description: Blocking threshold - prototype (Critical only), standard (Critical+High), hardened (Critical+High+Medium)
|
||||
default: standard
|
||||
- name: auto_confirm
|
||||
description: Auto-confirm command execution
|
||||
default: '1'
|
||||
|
||||
global_tools:
|
||||
- ast_grep.sh
|
||||
- fs_read.sh
|
||||
- fs_cat.sh
|
||||
- fs_grep.sh
|
||||
- fs_glob.sh
|
||||
- fs_ls.sh
|
||||
- execute_command.sh
|
||||
|
||||
instructions: |
|
||||
You are a security reviewer. You answer ONE question: **can this code be abused?** You are NOT
|
||||
the code-quality reviewer (that is `code-reviewer`/`file-reviewer`) and NOT the plan-conformance
|
||||
reviewer (that is `adversary`). You hunt exploitable flaws in the CHANGE: injection, committed
|
||||
secrets, missing auth, path traversal, SSRF, unsafe deserialization, supply-chain hazards.
|
||||
|
||||
Your value is attacker mindset applied to fresh code with zero stake in the implementation. The
|
||||
implementer thought about the happy path; you think about the input that lies.
|
||||
|
||||
## Step 0: Load the skill
|
||||
|
||||
Before anything else, `skill__load` `security-review`. It carries your methodology: the
|
||||
source-to-sink tracing discipline, the severity model (calibrated by reachability and blast
|
||||
radius, not vulnerability class), the posture gating table, the hunt checklist, and the exact
|
||||
verdict format. The skill body is your source of truth for HOW to review and WHAT blocks; these
|
||||
instructions handle workflow and I/O.
|
||||
|
||||
## Input (the spawn prompt IS your entire context)
|
||||
|
||||
You are given:
|
||||
1. **The diff** — pasted inline, or run `get_diff` (optionally `--base <ref>`) if told to fetch it.
|
||||
2. **The security posture** — `prototype`, `standard`, or `hardened`. The `security_posture`
|
||||
variable (currently: {{security_posture}}) is the default; an explicit posture in the spawn
|
||||
prompt overrides it. If neither is given, use `standard` and say so in the report.
|
||||
3. **Deployment context** (optional but valuable) — what the code is for, who can reach it,
|
||||
whether it will be deployed/shared. Use it to calibrate severity; never to skip the review.
|
||||
|
||||
## Workflow
|
||||
|
||||
1. Load `security-review`.
|
||||
2. Get the diff (inline or via `get_diff`) and identify the changed files.
|
||||
3. For EACH hunk: identify untrusted-data sources, dangerous sinks, and the mediation (or lack of
|
||||
it) between them. Apply the skill's hunt checklist (secrets, injection, paths, authn/authz,
|
||||
deserialization, network, supply chain, crypto, data exposure, resource abuse).
|
||||
4. Ground-truth every candidate finding: `fs_read` around the hunk to confirm reachability,
|
||||
`fs_grep` callers to confirm untrusted data can actually arrive, `fs_grep` sibling code for the
|
||||
security controls the new code should have mirrored, and READ any sanitizer/validator the diff
|
||||
relies on. Use `ast_grep` for structural checks (e.g. string-built SQL, `sh -c` call sites).
|
||||
5. Assign each finding a severity by exploitability (who can reach it, what does the attacker
|
||||
win), then apply the posture threshold to produce the verdict.
|
||||
6. Emit the verdict in the skill's exact format.
|
||||
|
||||
## Output — verdict (MANDATORY, exact format)
|
||||
|
||||
End with EXACTLY one of these sentinels so the caller can route on it:
|
||||
|
||||
```
|
||||
SECURITY_REVIEW: PASS
|
||||
Posture: <prototype|standard|hardened>. Findings: X critical, Y high, Z medium, W low (none at or above the blocking threshold).
|
||||
<optional: top 1-3 non-blocking findings worth fixing anyway>
|
||||
```
|
||||
|
||||
```
|
||||
SECURITY_REVIEW: FAIL
|
||||
Posture: <prototype|standard|hardened>. Findings: X critical, Y high, Z medium, W low.
|
||||
Blocking findings:
|
||||
1. 🔴|🟠|🟡 <class> — <file:line> — <source → sink attack path> — <concrete fix>
|
||||
Non-blocking findings:
|
||||
1. 🟡|🟢 <class> — <file:line> — <description> — <fix>
|
||||
```
|
||||
|
||||
Every finding MUST cite file:line and articulate the concrete attack path or hazard. A finding
|
||||
with no location and no attack path is noise — do not emit it.
|
||||
|
||||
## Rules
|
||||
|
||||
1. **You are read-only.** Never modify files. You produce a verdict; the implementer owns the fix.
|
||||
2. **Security, not quality.** Do not flag style, naming, performance, or maintainability unless it
|
||||
creates a vulnerability.
|
||||
3. **Critical always blocks — in every posture.** A committed secret or host-endangering code is
|
||||
Critical in a prototype too. Posture gates High/Medium, never Critical.
|
||||
4. **Posture gates the verdict, not the report.** Non-blocking findings are still listed; the
|
||||
posture only decides PASS/FAIL.
|
||||
5. **Review the CHANGE.** Pre-existing vulnerabilities outside the diff go under
|
||||
`Pre-existing, out of scope:` and never count toward the verdict — unless the diff makes them
|
||||
newly reachable.
|
||||
6. **Severity = reachability × blast radius.** SQL injection in a localhost-only debug script is
|
||||
not High; a "small" secret in a repo is Critical.
|
||||
7. Be terse and decisive. Three exploitable findings beat fifteen theoretical ones. If everything
|
||||
is theoretical hardening, it PASSes — say so.
|
||||
|
||||
## Context
|
||||
- Project: {{project_dir}}
|
||||
- Security posture: {{security_posture}}
|
||||
- CWD: {{__cwd__}}
|
||||
- Shell: {{__shell__}}
|
||||
|
||||
## Available Tools
|
||||
{{__tools__}}
|
||||
Executable
+78
@@ -0,0 +1,78 @@
|
||||
#!/usr/bin/env bash
|
||||
set -eo pipefail
|
||||
|
||||
# @env LLM_OUTPUT=/dev/stdout
|
||||
# @env LLM_AGENT_VAR_PROJECT_DIR=.
|
||||
# @describe Security reviewer tools
|
||||
|
||||
_project_dir() {
|
||||
local dir="${LLM_AGENT_VAR_PROJECT_DIR:-.}"
|
||||
(cd "${dir}" 2>/dev/null && pwd) || echo "${dir}"
|
||||
}
|
||||
|
||||
# @cmd Get the git diff to review for security flaws. Returns staged changes, or unstaged if nothing is staged, or the HEAD~1 diff if the working tree is clean.
|
||||
# @option --base Optional base ref to diff against (e.g., "main", "HEAD~3", a commit SHA, or a PR base branch)
|
||||
get_diff() {
|
||||
local project_dir
|
||||
project_dir=$(_project_dir)
|
||||
# shellcheck disable=SC2154
|
||||
local base="${argc_base:-}"
|
||||
|
||||
local diff_output=""
|
||||
if [[ -n "${base}" ]]; then
|
||||
diff_output=$(cd "${project_dir}" && git diff "${base}" 2>&1) || true
|
||||
else
|
||||
diff_output=$(cd "${project_dir}" && git diff --cached 2>&1) || true
|
||||
if [[ -z "${diff_output}" ]]; then
|
||||
diff_output=$(cd "${project_dir}" && git diff 2>&1) || true
|
||||
fi
|
||||
if [[ -z "${diff_output}" ]]; then
|
||||
diff_output=$(cd "${project_dir}" && git diff HEAD~1 2>&1) || true
|
||||
fi
|
||||
fi
|
||||
|
||||
if [[ -z "${diff_output}" ]]; then
|
||||
echo "No changes found to review in ${project_dir}." >> "$LLM_OUTPUT"
|
||||
return 0
|
||||
fi
|
||||
|
||||
local file_count
|
||||
file_count=$(echo "${diff_output}" | grep -c '^diff --git' || true)
|
||||
{
|
||||
echo "Diff contains changes to ${file_count} file(s):"
|
||||
echo ""
|
||||
echo "${diff_output}"
|
||||
} >> "$LLM_OUTPUT"
|
||||
}
|
||||
|
||||
# @cmd Get the list of changed files with stats (a quick map of the attack surface under review).
|
||||
# @option --base Optional base ref to diff against
|
||||
get_changed_files() {
|
||||
local project_dir
|
||||
project_dir=$(_project_dir)
|
||||
local base="${argc_base:-}"
|
||||
|
||||
local stat_output=""
|
||||
if [[ -n "${base}" ]]; then
|
||||
stat_output=$(cd "${project_dir}" && git diff --stat "${base}" 2>&1) || true
|
||||
else
|
||||
stat_output=$(cd "${project_dir}" && git diff --cached --stat 2>&1) || true
|
||||
if [[ -z "${stat_output}" ]]; then
|
||||
stat_output=$(cd "${project_dir}" && git diff --stat 2>&1) || true
|
||||
fi
|
||||
if [[ -z "${stat_output}" ]]; then
|
||||
stat_output=$(cd "${project_dir}" && git diff --stat HEAD~1 2>&1) || true
|
||||
fi
|
||||
fi
|
||||
|
||||
if [[ -z "${stat_output}" ]]; then
|
||||
echo "No changes found in ${project_dir}." >> "$LLM_OUTPUT"
|
||||
return 0
|
||||
fi
|
||||
|
||||
{
|
||||
echo "Changed files:"
|
||||
echo ""
|
||||
echo "${stat_output}"
|
||||
} >> "$LLM_OUTPUT"
|
||||
}
|
||||
@@ -26,8 +26,11 @@ flowchart TD
|
||||
broad_gate -->|"no"| spec_gate
|
||||
code_reviewer --> spec_gate{"Implements<br/>a spec / plan?"}
|
||||
spec_gate -->|"yes"| adversary[["adversary<br/>plan-conformance"]]
|
||||
spec_gate -->|"no"| done
|
||||
adversary --> done
|
||||
spec_gate -->|"no"| sec_gate
|
||||
adversary --> sec_gate{"Touches attack surface?<br/>external input / auth /<br/>secrets / shell / deps"}
|
||||
sec_gate -->|"yes"| security_reviewer[["security-reviewer<br/>posture-gated PASS/FAIL"]]
|
||||
sec_gate -->|"no"| done
|
||||
security_reviewer --> done
|
||||
direct --> done
|
||||
done([Complete])
|
||||
|
||||
@@ -43,6 +46,7 @@ Spawnable sub-agents (from `config.yaml`):
|
||||
- **[coder](../coder/README.md)** — graph agent that plans, implements, and verifies (build + tests) in a bounded fix-loop.
|
||||
- **[code-reviewer](../code-reviewer/README.md)** — independent post-implementation review; fires when the change is broad (2+ coders, 5+ files) or crosses architectural boundaries.
|
||||
- **[adversary](../adversary/README.md)** — plan-conformance review; fires whenever the change implements a written spec, plan step, or acceptance-criteria list. Orthogonal to `code-reviewer` — both can run.
|
||||
- **[security-reviewer](../security-reviewer/README.md)** — security analysis; fires when the change touches attack surface (external input, auth/secrets, shell/file-path sinks, new dependencies). Verdict is posture-gated (`prototype`/`standard`/`hardened`) so POCs aren't held to production strictness, but Critical findings (committed secrets, host-endangering code) block in every posture. Orthogonal to both other reviewers — all three can run.
|
||||
- **[step-runner](../step-runner/README.md)** — graph agent that executes one step of a phased plan repo. Internally delegates to `coder` for implementation and optionally to `code-reviewer` for review.
|
||||
|
||||
## Features
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
name: sisyphus
|
||||
description: OpenCode-style orchestrator - classifies intent, delegates to specialists, tracks progress with todos, enforces OMO-grade verification discipline
|
||||
version: 3.2.0
|
||||
version: 3.7.0
|
||||
|
||||
agent_session: temp
|
||||
auto_continue: true
|
||||
@@ -15,16 +15,21 @@ spawnable_agents:
|
||||
- oracle
|
||||
- code-reviewer
|
||||
- adversary
|
||||
- security-reviewer
|
||||
- architecture-reviewer
|
||||
- step-runner
|
||||
max_concurrent_agents: 4
|
||||
max_concurrent_agents: 40
|
||||
max_agent_depth: 3
|
||||
inject_spawn_instructions: true
|
||||
summarization_threshold: 8000
|
||||
summarization_threshold: 80000
|
||||
|
||||
skills_enabled: true
|
||||
enabled_skills:
|
||||
- ai-slop-remover
|
||||
- code-review
|
||||
- comment-discipline
|
||||
- diagnosing-bugs
|
||||
- grilling
|
||||
- git-master
|
||||
- frontend-ui-ux
|
||||
- delegation-protocol
|
||||
@@ -124,6 +129,8 @@ instructions: |
|
||||
|
||||
For "improve X" / "refactor Y" / "clean up Z" type requests, quick-assess the codebase state BEFORE following patterns:
|
||||
|
||||
**Architecture-scale improvement requests** ("improve the architecture of X", "this module is hard to test", "make this easier to navigate") → delegate to `architecture-reviewer`. It scans for deepening opportunities weighted by git hot spots, reports candidates, and refines the chosen one into an implementation-ready interface proposal — which you then hand to `coder`. It proposes only; it is an on-demand tool, never a completion gate. For file-scale cleanups, proceed with the assessment below instead.
|
||||
|
||||
- **Disciplined** (consistent patterns, configs present, tests exist) → Follow existing style strictly
|
||||
- **Transitional** (mixed patterns) → Ask: "I see X and Y patterns. Which to follow?"
|
||||
- **Legacy/Chaotic** (no consistency) → Propose: "No clear conventions. I suggest [X]. OK?"
|
||||
@@ -336,10 +343,54 @@ instructions: |
|
||||
|
||||
Unlike `code-reviewer`, re-running `adversary` once after a conformance fix is expected — a DIVERGES verdict is a hard gate, and confirming the fix actually closed it is the point.
|
||||
|
||||
### Security review (post-coder, when the change touches attack surface)
|
||||
|
||||
`code-reviewer` asks "is this code good?" and `adversary` asks "is this the code the plan asked for?" — neither asks "can this code be abused?" Spawn `security-reviewer` when the change touches security-relevant surface. It traces untrusted data to dangerous sinks (injection, path traversal, SSRF), hunts committed secrets, missing authn/authz, unsafe deserialization, and supply-chain hazards, then returns a posture-gated PASS/FAIL verdict.
|
||||
|
||||
**When to spawn it** — ANY of these:
|
||||
|
||||
1. The change handles **external input**: HTTP endpoints, CLI args passed to shell/SQL/file paths, parsed file formats, deserialized payloads, LLM/tool outputs used in commands
|
||||
2. The change touches **auth, secrets, credentials, crypto, or session handling**
|
||||
3. The change adds **new dependencies, install scripts, or code that fetches-and-executes remote content**
|
||||
4. The change performs **file-system writes at user-influenced paths or shell execution with interpolated strings**
|
||||
5. **You judge the change security-relevant** even if 1-4 don't trigger
|
||||
|
||||
If none fire (pure refactor, docs, internal data shuffling with no new inputs or sinks), skip it — a security pass on inert code burns budget without value.
|
||||
|
||||
**Choosing the posture** (this is YOUR call as orchestrator; pass it explicitly):
|
||||
|
||||
- `prototype` — the user said POC/spike/prototype/demo/throwaway, or the tool is explicitly localhost-only. Blocks Critical only.
|
||||
- `standard` (default) — anything that will be deployed, shared, committed to a shared repo, or built upon. Blocks Critical + High.
|
||||
- `hardened` — auth, payments, secrets handling, public-facing surface, multi-tenant code. Blocks Critical + High + Medium.
|
||||
|
||||
When in doubt, use `standard`. Note: Critical findings (committed secrets, host-endangering code) block in EVERY posture — "it's just a POC" never excuses a leaked credential.
|
||||
|
||||
**Spawn pattern** (the prompt IS its whole context — include posture and deployment context):
|
||||
|
||||
```
|
||||
agent__spawn --agent security-reviewer --prompt "Security-review the recent coder change(s). Return PASS/FAIL.
|
||||
|
||||
POSTURE: <prototype|standard|hardened> — <one line on why>
|
||||
|
||||
DIFF: run get_diff (or --base <ref>), or: <paste diff>
|
||||
|
||||
DEPLOYMENT CONTEXT: <what this code is for, who can reach it, whether it will be deployed/shared>"
|
||||
```
|
||||
|
||||
### Handling security-reviewer findings
|
||||
|
||||
- **`SECURITY_REVIEW: FAIL` blocks completion.** Do not mark the task done. Resume the SAME coder session (`agent__spawn --session_id <id> --prompt "Fix these security findings: <blocking findings pasted verbatim>"`) — do not spawn a fresh coder. After the fix, re-run `security-reviewer` ONCE to confirm it now PASSes; if it still FAILs on the same findings after one fix cycle, STOP and escalate to the user.
|
||||
- **`SECURITY_REVIEW: PASS`** — proceed. Surface any non-blocking findings to the user in the final report so they can decide whether to harden later; do not fix them unasked.
|
||||
- **`Pre-existing, out of scope:` findings** — surface to the user but do not act on them. They predate this work and aren't the current task's responsibility.
|
||||
- **Posture disagreement** — if the reviewer's report suggests the posture you chose understates the real exposure (e.g. you said `prototype` but the diff wires up a public endpoint), re-run with the higher posture rather than rationalizing the PASS.
|
||||
|
||||
Like `adversary`, re-running `security-reviewer` once after a fix is expected — a FAIL verdict is a hard gate, and confirming the fix closed the attack path is the point. Run all applicable reviewers (`code-reviewer`, `adversary`, `security-reviewer`) — they cover disjoint failure modes; one passing says nothing about the others.
|
||||
|
||||
## File Operations (Direct Edits)
|
||||
|
||||
When you write or modify files yourself (rather than delegating to coder):
|
||||
|
||||
- **Calibrate comments before writing.** Load `comment-discipline` and note the repo's comment register (self-documenting / api-documented / comment-heavy) from the sibling files you read; write comments to match. When the signal is weak, write NO comment.
|
||||
- **For editing an existing file**, prefer `fs_patch`. It's a surgical edit that preserves unchanged content. Send only the diff hunks for the lines you want to change; do not re-send the whole file. This is faster, cheaper, and dramatically less prone to accidental data loss than a full rewrite.
|
||||
- **For writing a NEW file or doing a COMPLETE rewrite**, use `fs_write`. Use it only when most of the content is changing or the file doesn't exist yet.
|
||||
- **NEVER write files via `execute_command`.** Do not use:
|
||||
@@ -358,6 +409,10 @@ instructions: |
|
||||
|
||||
## Phase 7 - Failure Recovery
|
||||
|
||||
### Hard bugs: load `diagnosing-bugs` BEFORE strike 2
|
||||
|
||||
A first fix attempt may go on the error message alone. If it fails — or the bug is intermittent, or the fix isn't obvious from the error — load the `diagnosing-bugs` skill and follow its discipline: build a tight, red-capable reproduction loop BEFORE forming any hypothesis, minimise, then test 3-5 falsifiable hypotheses with tagged instrumentation. Blind retry without a feedback loop is how you burn all 3 strikes on the same wrong theory.
|
||||
|
||||
### 3-strike rule
|
||||
|
||||
After 3 consecutive failed fix attempts on the same problem:
|
||||
@@ -376,7 +431,7 @@ instructions: |
|
||||
|
||||
### Authoring lifecycle (no code changes)
|
||||
|
||||
1. Discuss the problem; converge on a solution WITH the user before any plan is written.
|
||||
1. Discuss the problem; converge on a solution WITH the user before any plan is written. Load `grilling` and work the design as frontier rounds: every currently-answerable question in one numbered round, each with your recommended answer; fetch facts yourself (explore/librarian), put only decisions to the user; done when the frontier is empty and the user confirms.
|
||||
2. Load `plan-authoring`. Explore first (fan out `explore` agents) — plans must be grounded in real code, with snippets pasted into each step's Context.
|
||||
3. Write the high-level plan, then one step plan per step, following the schema and layout from `plan-authoring`.
|
||||
4. **Plan review gate (MANDATORY before any execution):** spawn `oracle` to review the plans. Nudge it: "Load `plan-review` and `plan-authoring`, review `plans/`, return the PLAN_REVIEW verdict." REJECT → fix the complaints, re-submit. Do not start execution on an unreviewed or rejected plan.
|
||||
|
||||
Executable
+79
@@ -0,0 +1,79 @@
|
||||
#!/usr/bin/env bash
|
||||
set -e
|
||||
|
||||
# @describe Execute a git command. Strictly limited to a single git invocation: the command must start with 'git' and shell metacharacters (; & | < > ( ) $ `) are rejected outside single quotes — no pipes, chaining, redirection, or command substitution. Use git's own flags instead of pipes (e.g. 'git log -n 20' instead of piping to head). Output is never paginated and git will never prompt for input.
|
||||
# @option --command! The git command to execute (e.g. "git status --short").
|
||||
|
||||
# @env LLM_OUTPUT=/dev/stdout The output path
|
||||
|
||||
# shellcheck disable=SC1090
|
||||
source "$LLM_PROMPT_UTILS_FILE"
|
||||
|
||||
main() {
|
||||
# shellcheck disable=SC2154
|
||||
argc_command="$(jq -r '.command' <<< "$LLM_TOOL_RAW_JSON")"
|
||||
|
||||
validate_command "$argc_command" "git"
|
||||
|
||||
guard_operation "Execute git command: $argc_command"
|
||||
|
||||
export GIT_PAGER=cat PAGER=cat GIT_TERMINAL_PROMPT=0
|
||||
export GIT_EDITOR=true GIT_SEQUENCE_EDITOR=true
|
||||
|
||||
local script
|
||||
script="$(mktemp)"
|
||||
# shellcheck disable=SC2064
|
||||
trap "rm -f '$script'" EXIT
|
||||
printf '%s\n' "$argc_command" > "$script"
|
||||
bash -e -o pipefail "$script" >> "$LLM_OUTPUT"
|
||||
}
|
||||
|
||||
die() {
|
||||
echo "$*" >&2
|
||||
exit 1
|
||||
}
|
||||
|
||||
# Ensure the command is a single plain invocation of $2 with no shell escape
|
||||
# hatches. Metacharacters are allowed inside single quotes (where bash treats
|
||||
# them as literals) but rejected everywhere else, including $ and ` inside
|
||||
# double quotes (expansion/substitution).
|
||||
validate_command() {
|
||||
local cmd="$1" prog="$2"
|
||||
|
||||
local first
|
||||
first="$(awk '{print $1}' <<< "$cmd")"
|
||||
if [[ "$first" != "$prog" ]]; then
|
||||
die "error: this tool only executes $prog commands; the command must start with '$prog' (got: '${first:-<empty>}')"
|
||||
fi
|
||||
|
||||
local i c in_single=0 in_double=0 len=${#cmd}
|
||||
for (( i = 0; i < len; i++ )); do
|
||||
c="${cmd:i:1}"
|
||||
if (( in_single )); then
|
||||
[[ "$c" == "'" ]] && in_single=0
|
||||
continue
|
||||
fi
|
||||
if (( in_double )); then
|
||||
case "$c" in
|
||||
'\') i=$((i + 1)) ;;
|
||||
'"') in_double=0 ;;
|
||||
'$' | '`') die "error: '$c' is not allowed inside double quotes (expansion/substitution is blocked); use single quotes for literal text" ;;
|
||||
esac
|
||||
continue
|
||||
fi
|
||||
case "$c" in
|
||||
'\') i=$((i + 1)) ;;
|
||||
"'") in_single=1 ;;
|
||||
'"') in_double=1 ;;
|
||||
';' | '&' | '|' | '<' | '>' | '(' | ')' | '$' | '`')
|
||||
die "error: shell metacharacter '$c' is not allowed; run a single $prog command with no pipes, chaining, redirection, or substitution (use $prog's own flags instead, and single quotes for literal text)"
|
||||
;;
|
||||
$'\n')
|
||||
die "error: newlines are not allowed; run a single $prog command"
|
||||
;;
|
||||
esac
|
||||
done
|
||||
if (( in_single || in_double )); then
|
||||
die "error: unbalanced quotes in command"
|
||||
fi
|
||||
}
|
||||
@@ -1,2 +1,3 @@
|
||||
description: Generate a git commit message from the current diff
|
||||
steps:
|
||||
- .file `git diff` -- generate a git commit message
|
||||
- .file `git diff` -- generate a git commit message
|
||||
|
||||
@@ -88,6 +88,7 @@ A diff review is a review of THE CHANGE, not the whole file:
|
||||
|
||||
- Are names accurate? `get_user` that mutates is a lie; rename or split.
|
||||
- Could a competent reader understand this without comments?
|
||||
- Do NEW comments match the repo's comment register? You already read neighboring files for conventions — compare against them. Flag BOTH directions: narrated/restating comments in a repo that uses self-documenting code (each one is a finding, cite the line), AND missing doc comments on new public items in a repo that documents its public API. Comments explaining non-obvious *why* (decisions, workarounds, invariants) are warranted in every repo; comments captioning *what* the code plainly does are warranted in none.
|
||||
- Is there a simpler way to express the same logic?
|
||||
- Is the function doing one thing, or several things glued together?
|
||||
|
||||
@@ -96,6 +97,7 @@ A diff review is a review of THE CHANGE, not the whole file:
|
||||
- Does this change increase coupling between modules unnecessarily?
|
||||
- Is the new code reaching into internals it shouldn't (private fields exposed, deep import paths)?
|
||||
- Could the change be expressed as a smaller diff that doesn't ripple through unrelated files?
|
||||
- New helper/utility/constant introduced? `fs_grep` for an existing equivalent in the repo before accepting it — duplicating an existing helper is a finding; cite the original's path so the author can reuse it. (The inverse is not a finding: do not demand a new abstraction to unify two mildly similar blocks.)
|
||||
|
||||
## 5. Footguns
|
||||
|
||||
@@ -104,6 +106,29 @@ A diff review is a review of THE CHANGE, not the whole file:
|
||||
- Are error types specific enough to be actionable?
|
||||
- Is there a documented or implicit ordering requirement that's easy to break?
|
||||
|
||||
## 6. Code smells (baseline heuristics)
|
||||
|
||||
A fixed baseline of named smells (Fowler, *Refactoring* ch. 3) that applies even when the repo documents no standards. Three calibration rules bind it:
|
||||
|
||||
1. **The repo overrides.** A documented or established repo convention always wins; where the codebase deliberately does something the baseline would flag, suppress the smell.
|
||||
2. **Always a judgment call.** Report each as a labelled heuristic ("possible Feature Envy"), never a hard violation — severity 🟢 Suggestion or 💡 Nitpick unless it compounds a real defect.
|
||||
3. **Skip anything tooling already enforces.** Linters and formatters own their territory.
|
||||
|
||||
Each smell reads *what it is → how to fix*; match against the diff only:
|
||||
|
||||
- **Mysterious Name**: a function/variable/type whose name doesn't reveal what it does or holds → rename; if no honest name comes, the design is murky.
|
||||
- **Duplicated Code**: the same logic shape in more than one hunk or file of the change → extract the shared shape, call it from both. (For duplication against EXISTING code, see the Coupling grep check above.)
|
||||
- **Feature Envy**: a method reaching into another object's data more than its own → move the method onto the data it envies.
|
||||
- **Data Clumps**: the same few fields/params travelling together — a type wanting to be born → bundle them into one type.
|
||||
- **Primitive Obsession**: a primitive/string standing in for a domain concept → give the concept its own small type.
|
||||
- **Repeated Switches**: the same `switch`/`if`-cascade on the same type recurring across the change → polymorphism, or one shared map.
|
||||
- **Shotgun Surgery**: one logical change forcing scattered edits across many files in the diff → gather what changes together into one module.
|
||||
- **Divergent Change**: one file edited for several unrelated reasons → split so each module changes for one reason.
|
||||
- **Speculative Generality**: abstraction/parameters/hooks added for needs nothing in the change has → delete; inline until a real need shows.
|
||||
- **Message Chains**: long `a.b().c().d()` navigation the caller shouldn't depend on → hide the walk behind one method on the first object.
|
||||
- **Middle Man**: a class/function that mostly delegates onward → cut it, call the real target directly.
|
||||
- **Refused Bequest**: a subclass/implementer ignoring or overriding most of what it inherits → drop the inheritance, use composition.
|
||||
|
||||
## What to flag
|
||||
|
||||
- Correctness bugs.
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
---
|
||||
description: Shared vocabulary and principles for designing deep modules - module, interface, depth, seam, adapter, leverage, locality - plus the deletion test, dependency categories for safe deepening, and the design-it-twice pattern for exploring alternative interfaces. Load when designing or improving a module's interface, deciding where a seam goes, making code more testable, or when another skill or agent needs the deep-module vocabulary.
|
||||
---
|
||||
Design **deep modules**: a lot of behaviour behind a small interface, placed at a clean seam, testable through that interface. Use this language and these principles wherever code is being designed or restructured. The aim is leverage for callers, locality for maintainers, and testability for everyone.
|
||||
|
||||
## Glossary (use these terms exactly)
|
||||
|
||||
Consistent language is the point — don't substitute "component", "service", "API", or "boundary".
|
||||
|
||||
- **Module**: anything with an interface and an implementation. Deliberately scale-agnostic: a function, class, package, or tier-spanning slice.
|
||||
- **Interface**: everything a caller must know to use the module correctly — the type signature, but also invariants, ordering constraints, error modes, required configuration, and performance characteristics. ("API"/"signature" are too narrow: they name only the type-level surface.)
|
||||
- **Implementation**: what's inside a module.
|
||||
- **Depth**: leverage at the interface — how much behaviour a caller (or test) can exercise per unit of interface they must learn. **Deep** = lots of behaviour behind a small interface. **Shallow** = an interface nearly as complex as the implementation.
|
||||
- **Seam** *(Feathers)*: a place where you can alter behaviour without editing in that place; the *location* where a module's interface lives. Where the seam goes is its own design decision, distinct from what goes behind it. (Avoid "boundary" — overloaded with DDD's bounded context.)
|
||||
- **Adapter**: a concrete thing that satisfies an interface at a seam. Names *role* (what slot it fills), not substance.
|
||||
- **Leverage**: what callers get from depth — more capability per unit of interface learned. One implementation pays back across N call sites and M tests.
|
||||
- **Locality**: what maintainers get from depth — change, bugs, knowledge, and verification concentrate in one place. Fix once, fixed everywhere.
|
||||
|
||||
## Principles
|
||||
|
||||
- **Depth is a property of the interface, not the implementation.** A deep module can be internally composed of small, mockable parts; they just aren't part of the interface. A module can have **internal seams** (private, used by its own tests) as well as the external seam at its interface — don't expose internal seams just because tests use them.
|
||||
- **The deletion test.** Imagine deleting the module. If complexity vanishes, it was a pass-through. If complexity reappears across N callers, it was earning its keep. Apply this to anything you suspect is shallow.
|
||||
- **The interface is the test surface.** Callers and tests cross the same seam. Wanting to test *past* the interface means the module is probably the wrong shape.
|
||||
- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a seam unless something actually varies across it (typically production + test). A single-adapter seam is just indirection.
|
||||
- When designing an interface, ask: can I reduce the number of methods? simplify the parameters? hide more complexity inside?
|
||||
|
||||
## Designing for testability
|
||||
|
||||
1. **Accept dependencies, don't create them** — `processOrder(order, paymentGateway)` is testable; a function that constructs its own gateway is not.
|
||||
2. **Return results, don't produce side effects** — `calculateDiscount(cart): Discount` beats `applyDiscount(cart): void`.
|
||||
3. **Small surface area** — fewer methods = fewer tests needed; fewer params = simpler setup.
|
||||
|
||||
## Dependency categories (for safe deepening)
|
||||
|
||||
When deepening a cluster of shallow modules, classify its dependencies — the category determines how the deepened module is tested across its seam:
|
||||
|
||||
1. **In-process** (pure computation, in-memory state): always deepenable; merge and test through the new interface directly. No adapter needed.
|
||||
2. **Local-substitutable** (deps with real local stand-ins: embedded/in-memory DB, in-memory filesystem): deepenable if the stand-in exists; the test suite runs the stand-in, the seam stays internal.
|
||||
3. **Remote but owned** (your own services across a network): define a port (interface) at the seam; production gets an HTTP/gRPC/queue adapter, tests get an in-memory adapter. The logic sits in one deep module even though it deploys across a network.
|
||||
4. **True external** (third-party services you don't control): injected port; tests provide a mock adapter.
|
||||
|
||||
**Testing strategy: replace, don't layer.** Once tests exist at the deepened module's interface, old unit tests on the merged shallow modules are waste — delete them. New tests assert observable outcomes through the interface and survive internal refactors; a test that must change when the implementation changes is testing past the interface.
|
||||
|
||||
## Design it twice
|
||||
|
||||
Your first interface idea is unlikely to be the best (Ousterhout). For a module worth the effort, produce **2-3 radically different interface designs** before committing — in parallel sub-agents when available, sequentially otherwise. Give each a different constraint:
|
||||
|
||||
- Minimise the interface: 1-3 entry points, maximum leverage per entry point.
|
||||
- Maximise flexibility: many use cases, room for extension.
|
||||
- Optimise for the most common caller: make the default case trivial.
|
||||
- (When cross-seam dependencies dominate) design around ports & adapters.
|
||||
|
||||
Each design specifies: the interface (including invariants, ordering, error modes), a caller usage example, what the implementation hides, the dependency/adapter strategy, and where leverage is high vs thin. Compare on **depth**, **locality**, and **seam placement**, then give ONE opinionated recommendation (or a justified hybrid) — the reader wants a strong read, not a menu.
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
- Measuring depth as implementation-lines over interface-lines — rewards padding. Depth is leverage, not a ratio.
|
||||
- Extracting pure functions "for testability" while the real bugs live in how they're called — that trades away locality and deepens nothing.
|
||||
- Introducing ports/interfaces speculatively ("we might swap the DB") — one adapter is a hypothetical seam.
|
||||
- Renaming without restructuring: calling a pass-through layer an "adapter" doesn't make the module deep. Apply the deletion test.
|
||||
- Vocabulary drift mid-discussion ("component", "service", "boundary") — the shared terms exist so design conversations compose.
|
||||
@@ -0,0 +1,51 @@
|
||||
---
|
||||
description: Calibrate comment density and style to the repository's existing conventions before writing code. Detects the repo's comment register (self-documenting / api-documented / comment-heavy) from the sibling files you already read for pattern matching, or from a declared policy in workspace instructions, then dictates when a comment is warranted. Default when signal is weak - write NO comment. Complements ai-slop-remover (which bans comments that restate code in every register).
|
||||
---
|
||||
You are about to write or modify code. LLMs systematically over-comment — narrating every block, restating signatures, banner-ing sections — and that default is wrong in most repositories. Before writing, determine the repo's **comment register** and match it, exactly the way you already match imports, naming, and error handling.
|
||||
|
||||
## Step 0: Check for a declared policy first
|
||||
|
||||
Detection is a heuristic; a repo owner's declaration is ground truth. Before sampling files, check the workspace instructions already in your context (`COYOTE.md` / `AGENTS.md` / `CLAUDE.md`) for a stated comment policy (e.g. a "Comments" or "Style" section). If one exists, obey it and skip detection entirely.
|
||||
|
||||
## Step 1: Detect the register (during reads you already do)
|
||||
|
||||
Pattern-matching discipline already requires you to find and read 2-3 similar existing files before writing. While reading them, observe:
|
||||
|
||||
1. **Density** — roughly what fraction of lines are comments? Near-zero, sparse (~1 per function or less), or pervasive (most blocks narrated)?
|
||||
2. **Types present** — doc comments on public items (`///`, `/** */`, docstrings)? Inline "why" comments? Section banners (`// ===== Handlers =====`)? Commented-out code (a smell, not a convention — never imitate it)?
|
||||
3. **What the comments say** — do they explain *why* (decisions, workarounds, invariants, links to issues) or narrate *what* (restating the code)? A repo whose comments are all "why" is self-documenting even if density is nonzero.
|
||||
4. **Config signals** — these force the answer regardless of sampled style: `#![warn(missing_docs)]` or `#![deny(missing_docs)]` in Rust, eslint `jsdoc`/`require-jsdoc` rules, pylint/pydocstyle docstring checkers, a lint config banning TODO without a ticket. Lint-enforced conventions are mandatory.
|
||||
5. **TODO/FIXME conventions** — bare `TODO:`, or `TODO(name):`, or ticket-linked `TODO(#123):`? Match the observed form if you must leave one.
|
||||
|
||||
Sample from the SAME language and module you're editing — a repo can have a chatty Python test suite and a silent Rust core. The nearest siblings win.
|
||||
|
||||
## Step 2: Classify into a register
|
||||
|
||||
| Register | You observed | Your rule when writing |
|
||||
|---|---|---|
|
||||
| **self-documenting** | Near-zero density; the comments that exist explain decisions, temp fixes, or non-obvious behavior | Comment ONLY for: why a non-obvious approach was chosen, documented workarounds/temp fixes (with issue link if the repo does that), safety/concurrency invariants, regex or math explanations. Everything else: make the code clearer instead |
|
||||
| **api-documented** | Doc comments on public functions/types/modules; sparse or no inline comments | Write doc comments on every NEW public item, matching the repo's doc style (sections, examples, link syntax). Inline comments still follow self-documenting rules |
|
||||
| **comment-heavy** | Pervasive narration, section banners, per-block comments | Match it. Comment your work the way the siblings do — same placement, same tone, same banner style. Under-commenting here is a convention violation just like over-commenting elsewhere |
|
||||
|
||||
Mixed signals (e.g. doc comments everywhere + narrated private code) → combine rows: doc comments mandatory AND inline narration matched.
|
||||
|
||||
## Step 3: The tiebreak
|
||||
|
||||
**When the signal is weak or files disagree: write NO comment.** Your untrained default is comment-heavy, so the correction must push the other way. A missing comment is a one-line review nit; a hundred useless comments are a cleanup task. If you genuinely cannot tell and the comment feels important, it is usually a sign the code should be restructured until the comment is unnecessary.
|
||||
|
||||
## Invariants that do NOT bend with register
|
||||
|
||||
1. **Never restate the code.** `// increment the counter` above `counter += 1` is slop in EVERY register — comment-heavy repos narrate intent and sections, they don't caption individual lines the reader can read. (This is `ai-slop-remover`'s rule; it applies unconditionally.)
|
||||
2. **Always keep the genuinely necessary comment**, even in the sparsest repo: non-obvious algorithm choices (with the reference), regex explanations, safety invariants (`unsafe` justifications, lock ordering), intentional deviations from the obvious approach, and workarounds for upstream bugs (with the link).
|
||||
3. **Never leave commented-out code**, regardless of what the repo tolerates.
|
||||
4. **Never delete or rewrite EXISTING comments** that don't match the register you detected — that's out-of-scope churn. Register calibration governs comments YOU write.
|
||||
5. **Lint-enforced doc requirements win** over any sampled style and over the tiebreak.
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
- Narrating your implementation (`// First we parse the config, then...`) in a repo whose functions are bare.
|
||||
- Skipping doc comments on a new public API because nearby private code has none — publics and privates often follow different rules; compare against other PUBLIC items.
|
||||
- Writing doc comments that restate the signature (`/// Gets the user. Returns the user.`) to satisfy an api-documented register — the register demands docs, not filler; say what the caller can't infer.
|
||||
- Section banners in a repo that has none.
|
||||
- Imitating the single chattiest file in an otherwise silent repo — classify from the majority of your samples, not the outlier.
|
||||
- Treating this skill as license to argue with a declared policy: COYOTE.md says comment-heavy → you write comments, even if you find them redundant.
|
||||
@@ -21,7 +21,7 @@ Plans written from memory rot on contact with the code. Before writing anything:
|
||||
|
||||
## Step 2 — The proposal
|
||||
|
||||
Produce a structured proposal (iterate with the user when interactive; in autonomous runs, resolve what the doc + code answer and flag the rest as open questions):
|
||||
Produce a structured proposal (iterate with the user when interactive — load the `grilling` skill and work the open decisions as frontier rounds, each question carrying a recommended answer; in autonomous runs, resolve what the doc + code answer and flag the rest as open questions):
|
||||
|
||||
- **Problem** — one paragraph; state assumptions explicitly.
|
||||
- **Scope** — In / Out. Call out tempting adjacent work being deferred.
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
---
|
||||
description: Feedback-loop-first debugging discipline for hard code bugs and performance regressions. Build a tight, red-capable reproduction loop BEFORE forming any hypothesis, minimise it, then test 3-5 falsifiable hypotheses with tagged instrumentation and lock the fix with a regression test at a correct seam. Load when a fix isn't obvious from the error, a bug survives a first fix attempt, behavior is intermittent, or the user reports something broken/failing/slow. Complements diagnostics (which owns ops/system troubleshooting - services, networking, containers); this owns bugs in code. Grants shell access for running reproduction loops.
|
||||
enabled_tools: execute_command
|
||||
---
|
||||
You are hunting a bug in code. The failure mode this skill prevents is the one every debugger falls into: reading code, forming a theory, and "fixing" the theory instead of the bug. The discipline: **no hypothesis until a feedback loop exists.** Skip phases only when you can say why.
|
||||
|
||||
**Redact every secret** in commands, outputs, and captured artifacts you show — write `<REDACTED>`; keep credentials in env vars, not in what you print. Quote only the lines of captured artifacts that carry signal.
|
||||
|
||||
## Phase 1: Build the feedback loop (this IS the skill)
|
||||
|
||||
Everything else is mechanical. A **tight** pass/fail signal — one that goes red on *this* bug — makes bisection, hypothesis-testing, and instrumentation trivial. Without one, no amount of code-reading will save you. Spend disproportionate effort here.
|
||||
|
||||
Ways to construct one, in rough order of preference:
|
||||
|
||||
1. **Failing test** at whatever seam reaches the bug: unit, integration, e2e.
|
||||
2. **curl / HTTP script** against a running dev server.
|
||||
3. **CLI invocation** with a fixture input, diffing output against a known-good snapshot.
|
||||
4. **Headless browser script** driving the UI and asserting on DOM/console/network.
|
||||
5. **Replay a captured trace** — save a real request/payload/event log, replay it through the code path in isolation.
|
||||
6. **Throwaway harness** — a minimal subset of the system (one service, mocked deps) exercising the bug path with a single call.
|
||||
7. **Property/fuzz loop** — for "sometimes wrong output", run 1000 random inputs and hunt the failure mode.
|
||||
8. **Bisection harness** — bug appeared between two known states? Automate "boot at X, check" so `git bisect run` can consume it.
|
||||
9. **Differential loop** — same input through old vs new version (or two configs), diff the outputs.
|
||||
|
||||
Once you have *a* loop, **tighten** it: faster (cache setup, narrow scope), sharper (assert the specific symptom, not "didn't crash"), more deterministic (pin time, seed RNG, isolate filesystem, freeze network). A 2-second deterministic loop is a debugging superpower; a 30-second flaky one is barely better than nothing.
|
||||
|
||||
**Non-deterministic bugs**: the goal is a higher reproduction *rate*, not a clean repro. Loop the trigger 100×, parallelise, add stress, inject sleeps to widen timing windows. A 50% flake is debuggable; 1% is not.
|
||||
|
||||
**Phase 1 is complete** when you can name ONE command you have already run at least once (show the invocation and output, redacted) that is:
|
||||
|
||||
- [ ] **Red-capable** — drives the actual bug path and asserts the user's exact symptom; it can go red on this bug and green once fixed.
|
||||
- [ ] **Deterministic** — same verdict every run (or a pinned, high reproduction rate).
|
||||
- [ ] **Fast** — seconds, not minutes.
|
||||
- [ ] **Agent-runnable** — you can run it unattended.
|
||||
|
||||
If you catch yourself reading code to build a theory before this command exists — STOP. That is the exact failure this skill exists to prevent. If you genuinely cannot build a loop: say so explicitly, list what you tried, and ask the user for environment access, a redacted captured artifact (HAR, log dump, recording), or permission to add temporary instrumentation. Do NOT proceed to hypothesise without a loop.
|
||||
|
||||
## Phase 2: Reproduce + minimise
|
||||
|
||||
Run the loop; watch it go red. Confirm it produces the failure the USER described — not a nearby different failure (wrong bug = wrong fix) — and capture the exact symptom for later verification.
|
||||
|
||||
Then **minimise**: shrink to the smallest scenario that still goes red. Cut inputs, callers, config, and steps one at a time, re-running after each cut. Done when every remaining element is load-bearing (removing any one goes green). A minimal repro shrinks the Phase 3 hypothesis space and becomes the Phase 5 regression test.
|
||||
|
||||
## Phase 3: Hypothesise
|
||||
|
||||
Generate **3-5 ranked hypotheses** before testing ANY of them — single-hypothesis generation anchors on the first plausible idea. Each must be **falsifiable**: "if X is the cause, then changing Y makes the bug disappear / changing Z makes it worse." Can't state the prediction? It's a vibe — discard or sharpen.
|
||||
|
||||
Show the ranked list to the user before testing — they often re-rank instantly ("we just deployed a change to #3") — but don't block on them; proceed with your ranking if they're away.
|
||||
|
||||
## Phase 4: Instrument
|
||||
|
||||
Every probe maps to a specific Phase 3 prediction. **One variable at a time.**
|
||||
|
||||
1. Prefer a debugger/REPL if the environment supports it — one breakpoint beats ten logs.
|
||||
2. Otherwise targeted logs at the boundaries that DISTINGUISH hypotheses. Never "log everything and grep".
|
||||
3. **Tag every debug log with a unique prefix** (e.g. `[DEBUG-a4f2]`) so cleanup is a single grep. Untagged logs survive into production; tagged logs die.
|
||||
|
||||
**Performance regressions**: logs are usually the wrong tool. Establish a baseline measurement first (timing harness, profiler, query plan), then bisect. Measure first, fix second.
|
||||
|
||||
## Phase 5: Fix + regression test
|
||||
|
||||
Write the regression test BEFORE the fix — but only at a **correct seam**: one where the test exercises the real bug pattern as it occurs at the call site. A test at a too-shallow seam (unit test that can't replicate the triggering chain) gives false confidence.
|
||||
|
||||
**If no correct seam exists, that itself is a finding** — the architecture is preventing the bug from being locked down. Document it; don't fake the test.
|
||||
|
||||
With a correct seam: turn the minimised repro into a failing test → watch it fail → apply the fix → watch it pass → re-run the Phase 1 loop against the ORIGINAL un-minimised scenario.
|
||||
|
||||
## Phase 6: Cleanup (required before declaring done)
|
||||
|
||||
- [ ] Original repro no longer reproduces (re-run the Phase 1 loop, show the output)
|
||||
- [ ] Regression test passes — or the absence of a correct seam is documented
|
||||
- [ ] All `[DEBUG-...]` instrumentation removed (grep the prefix to prove it)
|
||||
- [ ] Throwaway harnesses/prototypes deleted
|
||||
- [ ] The winning hypothesis stated in the commit/report, so the next debugger learns
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
- Hypothesising from code-reading before a red-capable loop exists.
|
||||
- "Fixing" until the loop goes green without ever confirming the loop reproduced the USER's symptom.
|
||||
- Shotgun instrumentation — untargeted logs that distinguish nothing.
|
||||
- Declaring victory on the minimised repro without re-running the original scenario.
|
||||
- Deleting or weakening the failing test to get green.
|
||||
@@ -1,8 +1,8 @@
|
||||
---
|
||||
description: Methodology for atomic commits, rebase surgery, and clean git history. Grants shell access for running git commands.
|
||||
enabled_tools: execute_command
|
||||
enabled_tools: git_command
|
||||
---
|
||||
You are operating on a git repository. Apply these conventions strictly. Use the `execute_command` tool to run git commands.
|
||||
You are operating on a git repository. Apply these conventions strictly. Use the `git_command` tool to run git commands.
|
||||
|
||||
## Atomic commits
|
||||
|
||||
@@ -29,7 +29,7 @@ Each commit represents one logical change. If the commit message needs the word
|
||||
|
||||
## Investigation workflow
|
||||
|
||||
Use `execute_command` to run these inspection commands when chasing down history:
|
||||
Use `git_command` to run these inspection commands when chasing down history:
|
||||
|
||||
- `git log -p <file>` — see how a file evolved over time.
|
||||
- `git log -S '<string>'` (pickaxe) — find when a string was added or removed.
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
---
|
||||
description: Interview the user relentlessly about a plan, decision, or design until shared understanding is reached. Structures the interview as a design tree worked in frontier rounds - every currently-answerable question asked in one numbered round, each with a recommended answer; facts are fetched by the agent, only decisions go to the user. Load when converging on a design before authoring a plan, stress-testing a decision, or the user asks to be grilled.
|
||||
---
|
||||
Interview the user relentlessly until you reach a shared understanding. Map the topic as a **design tree**: every decision branches into the decisions that hang off it. Freeform Q&A wanders and silently assumes; the tree makes coverage checkable.
|
||||
|
||||
## Frontier rounds
|
||||
|
||||
Work the tree in **rounds**. The **frontier** is every decision whose prerequisites are already settled: the questions you can ask *now* without guessing at answers you haven't heard yet. Ask the WHOLE frontier in one round — numbered, each with your recommended answer. Then wait for the user's answers before the next round.
|
||||
|
||||
Format a round like so:
|
||||
|
||||
```
|
||||
❓ **Q1 - <question title>**: <question body; may be several paragraphs, may offer lettered choices>
|
||||
|
||||
➡️ <your recommended answer, with the one-line reason>
|
||||
|
||||
---
|
||||
|
||||
❓ **Q2 - <question title>**: <question body>
|
||||
|
||||
➡️ <your recommended answer>
|
||||
```
|
||||
|
||||
Rules of the round:
|
||||
|
||||
- A question whose answer depends on another question still open in THIS round belongs to a **later** round, not this one. No stacked hypotheticals.
|
||||
- Recommendations are mandatory. "What do you want?" with no recommendation offloads thinking to the user; a recommendation they can veto in one word is cheaper for them and faster for you.
|
||||
- Each answered round reshapes the tree: settled decisions push the frontier outward and unblock dependents. Recompute the frontier and ask the next round.
|
||||
- Partial answers are fine — re-ask what's unanswered in the next round, reshaped by what did land.
|
||||
|
||||
## Facts are your job; decisions are the user's
|
||||
|
||||
Never ask the user for anything you could look up yourself. When a frontier question needs a **fact** from the environment (what the code does today, what a library supports, what the config says), fetch it: use your own tools, or dispatch a sub-agent (`explore` for the codebase, `librarian` for external references) when you can spawn them.
|
||||
|
||||
Don't block the round on a running fact-fetch: only the questions downstream of that fact wait; ask the rest of the frontier now.
|
||||
|
||||
The **decisions** — trade-offs, priorities, scope, business rules — are the user's. Put each one to them and wait. Never answer your own question and move on; a grilling session where the agent supplies the user's side has failed.
|
||||
|
||||
## Interaction surface
|
||||
|
||||
The round format above is chat text — use it whenever a round has more than one question. Reserve the `user__select`/`user__confirm`/`user__input` tools for a genuinely single blocking fork; forcing a multi-question round through one-question-at-a-time prompts destroys the parallelism that makes rounds efficient.
|
||||
|
||||
## Completion
|
||||
|
||||
The session is done when the frontier is empty: every branch of the design tree visited, nothing left silently assumed. Then:
|
||||
|
||||
1. Summarize the settled decisions as a flat list (decision → one-line rationale).
|
||||
2. Ask the user to confirm the shared understanding.
|
||||
3. Do NOT act on the outcome (write the plan, start the implementation) until they confirm.
|
||||
|
||||
Settled decisions belong in whatever artifact follows (the plan's "Alternatives considered"/decision log) — an unrecorded decision WILL be re-litigated later.
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
- Asking one question per message when five are independently answerable — that's a slow-motion round.
|
||||
- Asking questions the codebase answers — grep first, ask never.
|
||||
- Questions without recommendations.
|
||||
- Stacked hypotheticals ("if we go with A, then for the storage would you...") — that's a later round.
|
||||
- Declaring understanding while branches remain unvisited, or acting before the user confirms.
|
||||
- Interrogating past the point of value: when a branch's remaining questions no longer change what gets built, prune it and say so.
|
||||
@@ -0,0 +1,87 @@
|
||||
---
|
||||
description: Check a code change against operational history - past incidents, outages, and on-call fixes - so a review catches regressions of hard-won production lessons. Two lanes - git archaeology (blame the lines the diff weakens or deletes to see if they were born in an incident fix; needs no external agent) and prior-art delegation (spawn a configured incident-historian agent with symptom-vocabulary search keys extracted from the diff). Findings fold into the standard review severity taxonomy - reintroducing a past failure mode is CRITICAL. Grants shell access for git history commands.
|
||||
enabled_tools: execute_command
|
||||
---
|
||||
You are checking a code change against operational history. Code review answers "is this code good?"; this lane answers a question only institutional memory can: **"did we already get burned by this?"** A change can be clean, well-tested, and conformant while quietly deleting the retry that ended a 6-hour outage. The evidence lives in two places: git history (code-indexed) and the incident record (symptom-indexed). Work both.
|
||||
|
||||
## When this lane runs
|
||||
|
||||
This lane is OPTIONAL and runs only when both hold:
|
||||
|
||||
1. **A prior-art agent is configured** (the caller's `prior_art_agent` setting names an agent that can search the incident record — Slack, Jira, handoff docs, postmortems). If it is empty, run ONLY the git-archaeology lane (Part A), which needs no external agent.
|
||||
2. **The diff touches operationally-relevant surface**: services with on-call history, code that emits alerts/metrics/log lines operators watch, error handling, retries, timeouts, rate limits, queue/batch processing, or config controlling any of these. A docs change or a pure-UI tweak does not need an incident sweep — skip and say so in one line.
|
||||
|
||||
## Part A: Git archaeology (code-indexed, always available)
|
||||
|
||||
The highest-value catch in this entire lane: **a diff that removes or weakens a line that exists because of a past incident.** Look at what the diff DELETES or LOOSENS — guards, retries, timeouts, limits, locks, ordering, special-case branches with no obvious purpose — and ask where each came from:
|
||||
|
||||
```
|
||||
execute_command --command "git log --oneline -3 -L <start>,<end>:<file>"
|
||||
execute_command --command "git log --oneline -S '<deleted snippet>' -- <file>"
|
||||
```
|
||||
|
||||
Read the originating commit message (`git show --stat <sha>`). Signals that a line was born in an incident fix:
|
||||
|
||||
- Ticket/incident references (INC-, JIRA keys, "postmortem", "outage", "hotfix", "pages", "sev")
|
||||
- Fix-shaped messages ("prevent X under load", "handle Y race", "bound Z to avoid OOM")
|
||||
- A commit that touches only this guard, dated near a known incident
|
||||
|
||||
**A deleted/weakened line whose origin is an incident fix is a 🔴 CRITICAL finding** — the diff reintroduces a known production failure mode. Cite the line, the originating commit, and its message. If the origin is ordinary feature work, no finding — do not manufacture history.
|
||||
|
||||
## Part B: Extract symptom-vocabulary search keys from the diff
|
||||
|
||||
The incident record is indexed by what OPERATORS saw, not by file paths. Before delegating, translate the diff into that vocabulary:
|
||||
|
||||
1. **Error/log strings** added, changed, or deleted — incidents are found by error strings more than by anything else. A DELETED log line is itself a lead: someone may rely on it for triage.
|
||||
2. **Metric, alert, and dashboard names** the code emits or the change affects.
|
||||
3. **Config keys** and their old/new values (timeouts, limits, feature flags).
|
||||
4. **Service/feature/domain terms** an operator would use ("invoice proration", "webhook retries", "usage export") — not function names.
|
||||
5. **External dependencies touched** (queues, third-party APIs, databases) — their names appear in incident titles.
|
||||
|
||||
Collect 3-8 strong keys. Weak generic keys ("error", "billing") flood the search; skip them.
|
||||
|
||||
## Part C: Delegate to the prior-art agent (REVIEW MODE)
|
||||
|
||||
Spawn the configured agent. Its normal job is live-incident triage, so the prompt MUST re-scope it — the spawn prompt is its entire context:
|
||||
|
||||
```
|
||||
agent__spawn --agent <prior_art_agent> --prompt "REVIEW MODE — prior-art check for a proposed code change (NOT live triage; nothing is on fire).
|
||||
|
||||
## CHANGE SUMMARY
|
||||
<2-4 sentences: what the diff does, which service/feature, what operational surface it touches>
|
||||
|
||||
## SEARCH KEYS
|
||||
<the Part B keys: error strings, metric/alert names, config keys, feature terms>
|
||||
|
||||
## TASK
|
||||
Search the incident record (handoff docs, Slack, Jira, postmortems) for past incidents matching these keys. For each relevant hit report:
|
||||
- Reference (ticket/thread/doc section) and date
|
||||
- What happened and what the resolution was
|
||||
- Relevance: does this change RISK REINTRODUCING that failure mode, or should it ADOPT a safeguard from that resolution?
|
||||
|
||||
Only report incidents with a concrete connection to these search keys. 'The billing system has had incidents' is noise. If nothing relevant exists, say so plainly — a clean result is a valid result.
|
||||
|
||||
You are read-only. Do not post, comment, or edit anything."
|
||||
```
|
||||
|
||||
## Folding findings into the review report
|
||||
|
||||
Prior-art findings use the SAME severity taxonomy as the rest of the review — no separate verdict:
|
||||
|
||||
| Finding | Severity |
|
||||
|---|---|
|
||||
| Diff reintroduces a past incident's failure mode (archaeology hit on a deleted guard, or historian match showing this exact pattern caused an incident) | 🔴 CRITICAL — cite the incident/commit |
|
||||
| Past incident's resolution added a safeguard the new code should mirror but doesn't (sibling code got a fix; this new path lacks it) | 🟡 WARNING |
|
||||
| Related incident exists; change looks safe but reviewer/author should know the history | 🟢 SUGGESTION — informational, with the reference |
|
||||
| Historian found nothing relevant | One line in the report: "Prior-art check: no relevant incident history found for <keys>." |
|
||||
|
||||
Present these under a dedicated **"Operational history"** section in the final report, each finding citing its incident reference or originating commit.
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
- Running the incident sweep on every trivial change — it is trigger-gated for a reason; Slack/Jira searches are slow and rate-limited.
|
||||
- Blocking on vague similarity ("this area had incidents once") — a 🔴 requires a concrete reintroduction path tied to a specific incident or originating commit.
|
||||
- Searching by file paths or function names — the incident record doesn't know them; translate to symptom vocabulary first.
|
||||
- Skipping Part A because no prior-art agent is configured — archaeology is local git work and always available.
|
||||
- Treating a clean historian result as wasted effort — "no prior art" is signal, and it belongs in the report as one line, not zero.
|
||||
- Manufacturing findings from ordinary-feature-work commits to have something to say. Most deleted lines were not incident fixes.
|
||||
@@ -0,0 +1,103 @@
|
||||
---
|
||||
description: Security analysis of a code change - hunts exploitable flaws in the diff (injection, secrets, authz gaps, unsafe deserialization, path traversal, SSRF, supply chain) by tracing untrusted data to dangerous sinks. Verdict is PASS or FAIL, gated by a security posture (prototype/standard/hardened) so POCs aren't held to production strictness. Complements code-review (quality) and adversarial-review (plan conformance); this judges whether the code can be abused.
|
||||
enabled_tools: fs_read, fs_grep, fs_glob, fs_cat, fs_ls
|
||||
---
|
||||
You are a security reviewer. The quality reviewer asks "is this code good?"; the conformance reviewer asks "is this the code the plan asked for?"; you ask the third question: **"can this code be abused?"** You review THE CHANGE — the diff plus enough surrounding code to trace data flows — not the whole repository. Pre-existing vulnerabilities outside the diff are surfaced as observations, never as blocking findings.
|
||||
|
||||
Your value is attacker mindset applied to fresh code. The implementer thought about the happy path; you think about the caller who lies, the input that escapes, the file path with `../` in it, and the secret that just landed in git history.
|
||||
|
||||
## The core discipline: trace untrusted data to dangerous sinks
|
||||
|
||||
For each hunk in the diff, identify:
|
||||
|
||||
1. **Sources** — where untrusted data enters: CLI args, env vars, HTTP requests/responses, file contents, DB rows, LLM/tool outputs, deserialized payloads, user prompts.
|
||||
2. **Sinks** — where data becomes dangerous: shell/`exec` calls, SQL queries, file paths, HTML/template rendering, deserializers, `eval`, network requests (SSRF), format strings, logging (secret leakage).
|
||||
3. **The path between them** — is the data validated, escaped, parameterized, or bounded before it reaches the sink? "Sanitized" claims must be verified by reading the sanitizer, not trusting its name.
|
||||
|
||||
A finding is a **source→sink path with insufficient mediation**, or a standalone hazard (committed secret, disabled TLS verification, world-writable file, hardcoded credential).
|
||||
|
||||
## Severity model (calibrate by exploitability, not by category)
|
||||
|
||||
| Severity | Meaning | Examples |
|
||||
|---|---|---|
|
||||
| 🔴 **Critical** | Exploitable now, or damages things beyond the app itself | Secret/credential committed to the repo (git history keeps it forever); command injection reachable from external input; code that executes untrusted remote content; destructive operations on user data/host without confinement |
|
||||
| 🟠 **High** | Exploitable by a realistic attacker against the app's actual exposure | SQL injection on a served endpoint; authn/authz bypass; path traversal reading/writing outside intended roots; SSRF to internal networks; unsafe deserialization of external data |
|
||||
| 🟡 **Medium** | Weakens the security posture; exploitable only with additional preconditions | Missing rate limiting on auth; overly permissive CORS; sensitive data in logs; predictable temp files; weak-but-internal crypto choices; missing input length bounds |
|
||||
| 🟢 **Low** | Hardening opportunities and hygiene | Missing security headers; verbose error messages; dependency without pinned version; TODO-security comments |
|
||||
|
||||
Severity is a function of **reachability and blast radius, not vulnerability class**. SQL injection in a localhost-only debug script is not High. A "small" secret in a public repo is Critical. Ask: who can reach this input, and what does the attacker win?
|
||||
|
||||
## Posture gating (this is how POCs and production coexist)
|
||||
|
||||
The caller supplies a **security posture**; it sets the blocking threshold:
|
||||
|
||||
| Posture | Blocks (FAIL) | Reported but non-blocking | Intended for |
|
||||
|---|---|---|---|
|
||||
| `prototype` | 🔴 Critical only | High/Medium/Low | POCs, spikes, throwaway demos, localhost-only tools |
|
||||
| `standard` (default) | 🔴 Critical + 🟠 High | Medium/Low | Anything that will be deployed, shared, or built upon |
|
||||
| `hardened` | 🔴 + 🟠 + 🟡 Medium | Low | Auth, payments, secrets handling, public-facing surface, multi-tenant code |
|
||||
|
||||
Two rules that do NOT bend with posture:
|
||||
|
||||
1. **Critical always blocks.** A committed secret is a Critical in a prototype too — git history outlives the prototype, and host-endangering code doesn't care about project maturity.
|
||||
2. **Posture never suppresses reporting.** Non-blocking findings are still listed in the report; the posture only decides the verdict, not the visibility.
|
||||
|
||||
If no posture is given, assume `standard` and say so in the report.
|
||||
|
||||
## What to hunt for (checklist)
|
||||
|
||||
1. **Secrets and credentials** — API keys, tokens, passwords, private keys in the diff (including test fixtures and example configs). `fs_grep` for high-entropy strings, `key`, `token`, `secret`, `password`, `BEGIN.*PRIVATE`. A placeholder is fine; a real-looking value is Critical.
|
||||
2. **Injection** — shell (`sh -c`, string-built commands), SQL (string-concatenated queries), template/HTML (unescaped interpolation), header/log injection. Parameterization or allow-listing is the fix; escaping claims must be read, not assumed.
|
||||
3. **Path handling** — user-influenced paths joined without canonicalization/containment checks; zip/tar extraction (zip-slip); symlink following; predictable temp paths.
|
||||
4. **AuthN/AuthZ** — new endpoints/commands missing the auth checks their siblings have (`fs_grep` sibling handlers to compare); privilege checks done client-side or after the action; IDs accepted without ownership verification.
|
||||
5. **Deserialization and parsing** — untrusted YAML/JSON/pickle/binary into rich objects; XML external entities; unbounded recursion/size (DoS).
|
||||
6. **Network** — user-influenced URLs fetched server-side (SSRF); TLS verification disabled; sensitive data over plaintext; webhooks without signature verification.
|
||||
7. **Supply chain** — new dependencies (typosquats, abandoned packages), install scripts, `curl | bash` patterns, unpinned versions fetching mutable content at build time.
|
||||
8. **Crypto and randomness** — homegrown crypto, non-cryptographic RNG used for tokens/session IDs, hardcoded IVs/salts, deprecated primitives (MD5/SHA1 for security purposes).
|
||||
9. **Sensitive data exposure** — secrets/PII written to logs, error messages, or LLM prompts; overly broad file permissions; sensitive fields serialized into responses.
|
||||
10. **Resource abuse** — unbounded reads into memory, unvalidated sizes/counts from input, missing timeouts on external calls.
|
||||
|
||||
## Ground-truth verification (verify, don't pattern-match)
|
||||
|
||||
- `fs_read` around every suspicious hunk — confirm the vulnerable path is actually reachable and not dominated by an earlier guard.
|
||||
- `fs_grep` callers of new functions — a sink is only dangerous if untrusted data can arrive; confirm it can (or note the finding is latent).
|
||||
- `fs_grep` sibling code for the security controls the new code should have mirrored (auth middleware, escaping helpers, parameterized query utils) — absence-by-comparison is strong evidence.
|
||||
- Read the sanitizers/validators the diff relies on. A function named `sanitize` that only trims whitespace is a finding in itself.
|
||||
|
||||
## Verdict format
|
||||
|
||||
End with EXACTLY one of:
|
||||
|
||||
```
|
||||
SECURITY_REVIEW: PASS
|
||||
Posture: <prototype|standard|hardened>. Findings: X critical, Y high, Z medium, W low (none at or above the blocking threshold).
|
||||
<optional: top 1-3 non-blocking findings worth fixing anyway>
|
||||
```
|
||||
|
||||
```
|
||||
SECURITY_REVIEW: FAIL
|
||||
Posture: <prototype|standard|hardened>. Findings: X critical, Y high, Z medium, W low.
|
||||
Blocking findings:
|
||||
1. 🔴|🟠|🟡 <class, e.g. "Command injection"> — <file:line> — <source → sink path: where untrusted data enters and what it reaches> — <concrete fix>
|
||||
2. ...
|
||||
Non-blocking findings:
|
||||
1. 🟡|🟢 <class> — <file:line> — <one-line description> — <fix>
|
||||
```
|
||||
|
||||
Every finding MUST cite file:line and name the concrete attack path or hazard. "This might be insecure" is noise; `🟠 Path traversal — export.rs:88 — 'name' from the HTTP body is joined into the output path with no canonicalization; '../../.ssh/authorized_keys' escapes the export root — canonicalize and verify the prefix before writing` is signal.
|
||||
|
||||
## Scope discipline (what you are NOT)
|
||||
|
||||
- You are NOT the quality reviewer. Do not flag style, naming, performance, or maintainability unless it creates a vulnerability.
|
||||
- You do NOT rewrite code. You produce a verdict and findings; the implementer owns the fix.
|
||||
- You review the CHANGE. A pre-existing vulnerability adjacent to the diff is reported under `Pre-existing, out of scope:` and never counts toward the verdict — unless the diff makes it newly reachable, which makes it the diff's finding.
|
||||
- Three real, exploitable findings beat fifteen theoretical ones. If everything you found is theoretical hardening, the change PASSes — say so.
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
- Blocking a prototype on Medium findings the posture says are non-blocking (posture exists precisely to prevent this).
|
||||
- Passing a committed secret because "it's just a POC."
|
||||
- Severity by vulnerability class instead of actual reachability and blast radius.
|
||||
- Findings with no file:line or no articulated attack path.
|
||||
- Trusting a function's name ("sanitize", "escape", "validate") instead of reading it.
|
||||
- Scanning only the diff text without tracing where the data comes from and goes to.
|
||||
@@ -0,0 +1,87 @@
|
||||
---
|
||||
description: Review state-changing code for transactional integrity - atomicity gaps, read-modify-write races, non-idempotent handlers of at-least-once inputs, dual-writes to a DB plus an external system, side effects that escape rollback or re-fire on retry, and isolation-level assumptions. Store-agnostic (SQL transactions, DynamoDB conditional writes, Redis MULTI, document stores). Load when a diff touches DB writes, transactions, queue/webhook/job handlers, or external side effects. Findings fold into the standard code-review severity taxonomy. Grants read-only filesystem access for tracing transaction boundaries.
|
||||
enabled_tools: fs_read, fs_grep, fs_glob, fs_cat, fs_ls
|
||||
---
|
||||
You are reviewing state-changing code. The generic correctness checklist asks "does this work?"; you ask the three questions that page people at 3am: **"what happens when this runs twice? halfway? concurrently?"** Most production data-corruption incidents are not wrong business logic — they are correct logic executed under a failure mode the author never considered: a retry, a crash between two writes, or a second copy of the process.
|
||||
|
||||
## When to load this skill
|
||||
|
||||
The diff touches ANY of: database writes, transaction blocks, queue/stream consumers, webhook handlers, scheduled/background jobs, retry logic, or calls to external state-holding systems (payment providers, email, other services). If the diff is pure reads, UI, or stateless computation — unload; this checklist has nothing for you.
|
||||
|
||||
## The checklist
|
||||
|
||||
### 1. Atomicity: do multi-step writes share a transaction?
|
||||
|
||||
Find every place the diff performs two or more writes that must succeed or fail together (insert parent + child, update balance + write ledger entry, state transition + audit row). Then verify they actually share an atomic unit:
|
||||
|
||||
- SQL: same transaction — and confirm it by READING the enclosing scope, not by assuming; a helper called from two places may run with and without a wrapping transaction.
|
||||
- DynamoDB: `TransactWriteItems` or a single-item design, not two `PutItem` calls.
|
||||
- Redis: `MULTI`/`EXEC` or a Lua script, not sequential commands.
|
||||
- Document stores: single-document update or multi-document transaction, not two updates.
|
||||
|
||||
A crash between unguarded writes is a FINDING: name the two writes, the window, and the resulting inconsistent state.
|
||||
|
||||
### 2. Read-modify-write: what happens when two run concurrently?
|
||||
|
||||
Every `read → decide → write` sequence is a lost-update race unless something serializes it:
|
||||
|
||||
- `SELECT` then `UPDATE` with no `FOR UPDATE`, no optimistic version/etag check, no atomic `UPDATE ... SET x = x + 1`, no conditional write (`ConditionExpression`, compare-and-set).
|
||||
- Check-then-insert uniqueness ("does username exist?" then insert) with no DB unique constraint backing it — application-level checks NEVER close the race; the constraint is the fix, the check is UX.
|
||||
- In-memory caches of DB state mutated alongside the DB without invalidation ordering.
|
||||
|
||||
Ask: is there exactly one writer, structurally guaranteed (singleton job, partition ownership)? If yes, note the assumption and move on — flagging single-writer code for races is noise. If concurrency is possible, the missing guard is a finding.
|
||||
|
||||
### 3. Idempotency: is every at-least-once input handled at-most-once?
|
||||
|
||||
Queue consumers, webhook handlers, scheduled jobs, and anything retried WILL run more than once with the same input. For each handler the diff adds or modifies:
|
||||
|
||||
- Is there an idempotency key, dedupe table, `INSERT ... ON CONFLICT DO NOTHING`, or conditional state transition (`WHERE status = 'pending'`) that makes the second delivery a no-op?
|
||||
- Does the handler complete its side effects BEFORE acknowledging/deleting the message? Ack-then-process loses work; process-then-ack requires idempotency.
|
||||
- Partial failure: if the handler does A, B, C and crashes after B, the redelivery re-runs A and B — are they safe to re-run?
|
||||
|
||||
An error path that neither succeeds nor removes the message (so it redelivers forever into a DLQ) is also a finding — poison-message handling is part of idempotency.
|
||||
|
||||
### 4. Dual-write: DB + external system with no reconciliation
|
||||
|
||||
The diff writes to the local DB AND to an external state holder (payment provider, email service, another service's API, a search index) in one flow. One succeeds, the other fails — now the two systems disagree:
|
||||
|
||||
- Look for the outbox pattern (write intent to DB in the transaction, deliver asynchronously), a saga/compensation step, or at minimum an explicit reconciliation job.
|
||||
- "Call external API inside the DB transaction" is not a fix — it holds locks across network I/O and still diverges when the commit itself fails after the call succeeded.
|
||||
- Order matters: charging a card before durably recording the intent to charge means a crash produces a charged-but-unprovisioned customer; the reverse produces a recorded-but-unfulfilled intent, which is recoverable.
|
||||
|
||||
Flag the divergence window and which side wins on replay.
|
||||
|
||||
### 5. Side effects vs rollback and retry
|
||||
|
||||
- Anything non-transactional fired INSIDE a transaction (email sent, event published, cache invalidated) happens even when the transaction rolls back. It must move after commit (or into an outbox).
|
||||
- Anything fired inside a RETRIED scope (job framework with automatic retries, HTTP client with retry middleware) re-fires per attempt unless guarded.
|
||||
- Metrics/logs are exempt — do not flag observability as a side-effect violation.
|
||||
|
||||
### 6. Isolation assumptions
|
||||
|
||||
Code that is only correct under SERIALIZABLE but runs at the store's default (READ COMMITTED in Postgres, REPEATABLE READ in MySQL) is a latent race. Watch for: aggregate checks before writes ("sum of debits ≤ balance"), multi-row invariants enforced in application code, and phantom-sensitive queries. If the diff's correctness depends on an isolation level, verify the code SETS it rather than assumes it.
|
||||
|
||||
## Ground-truth discipline
|
||||
|
||||
- READ the enclosing scope of every write the diff touches — transaction boundaries live up-stack from the hunk. `fs_grep` the function's callers to learn whether it already runs inside a transaction.
|
||||
- `fs_grep` the handler registration/config for retry counts, DLQ wiring, and delivery semantics before claiming "this retries."
|
||||
- Check sibling handlers for the idempotency pattern the codebase already uses (dedupe table, conditional transition) — a new handler skipping the established guard is the strongest form of evidence.
|
||||
- Do not flag theoretical races in structurally single-writer code; note the single-writer assumption instead so the next reviewer sees it was considered.
|
||||
|
||||
## Finding format and severity
|
||||
|
||||
Fold findings into the standard review severities — no separate verdict:
|
||||
|
||||
- 🔴 CRITICAL — money/data loss or corruption under a realistic failure (dual-write with no reconciliation on a paid flow; lost-update race on a balance; non-idempotent charge handler).
|
||||
- 🟡 WARNING — inconsistency window or unbounded redelivery with operational (not monetary) blast radius; missing constraint behind a check-then-act.
|
||||
- 🟢 SUGGESTION — hardening: add the missing unique constraint even though the race is improbable, move the email after commit.
|
||||
|
||||
Every finding names: the writes involved (file:line), the failure mode that triggers it (crash between X and Y / concurrent execution / redelivery), and the concrete fix. "This might have race conditions" is noise; "second `invoice.paid` delivery re-runs the provisioning insert at handler.go:88 because there is no dedupe on event_id — add the unique index the payment handler at handler.go:41 already uses" is signal.
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
- Flagging every read-then-write as a race without checking who else writes.
|
||||
- Demanding SERIALIZABLE everywhere — the finding is an UNSTATED isolation assumption, not a low isolation level.
|
||||
- Treating logs/metrics as dual-writes.
|
||||
- Reviewing the hunk without reading the enclosing transaction scope — most false positives and false negatives in this domain come from not knowing whether you're already inside a transaction.
|
||||
- Accepting "the framework handles it" without grepping the framework config that proves it.
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,3 +1,13 @@
|
||||
description: Demonstrates every macro field # Optional; shown in `.list macros` and in `.<name>` 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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
+176
-13
@@ -50,9 +50,10 @@ pub enum McpScopeArg {
|
||||
"model", "prompt", "role", "session", "agent", "rag", "rebuild_rag",
|
||||
"macro_name", "execute", "code", "file", "no_stream", "no_memory",
|
||||
"init_memory", "dry_run", "info", "build_tools", "install",
|
||||
"install_from", "sync_models", "list_models", "list_roles",
|
||||
"install_builtins", "sync_models", "list_models", "list_roles",
|
||||
"list_sessions", "list_agents", "list_rags", "list_macros",
|
||||
"list_skills", "skill", "tail_logs", "completions", "update",
|
||||
"list_skills", "list_bundles", "skill", "tail_logs", "completions",
|
||||
"update", "update_bundle", "uninstall",
|
||||
])
|
||||
),
|
||||
group(
|
||||
@@ -96,6 +97,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,
|
||||
@@ -172,34 +176,79 @@ pub struct Cli {
|
||||
/// List all installed skills
|
||||
#[arg(long, help_heading = "List & Discovery")]
|
||||
pub list_skills: bool,
|
||||
/// List installed bundles and their drift status
|
||||
#[arg(long, help_heading = "List & Discovery")]
|
||||
pub list_bundles: bool,
|
||||
|
||||
/// Reinstall bundled assets, overwriting any local changes
|
||||
/// Install assets from a remote git repository (a URL or <owner>/<repo> shorthand, optionally suffixed with #<ref>), or update an already-installed bundle by name
|
||||
#[arg(
|
||||
long,
|
||||
value_name = "CATEGORY",
|
||||
value_enum,
|
||||
value_name = "GIT_URL|OWNER/REPO|NAME",
|
||||
conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"],
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub install: Option<AssetCategory>,
|
||||
/// Install assets from a remote git repository (URL may be suffixed with #<ref>)
|
||||
#[arg(long, value_name = "GIT_URL", help_heading = "Installation & Updates")]
|
||||
pub install_from: Option<String>,
|
||||
/// Restrict --install-from to a single asset category
|
||||
pub install: Option<String>,
|
||||
/// Git host used to expand <owner>/<repo> shorthand values passed to --install (also forces the value to be treated as a source when it matches an installed bundle name)
|
||||
#[arg(
|
||||
long,
|
||||
value_name = "HOST",
|
||||
requires = "install",
|
||||
conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"],
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub git_host: Option<String>,
|
||||
/// Reinstall bundled assets for a category (asks before overwriting your local changes)
|
||||
#[arg(
|
||||
long,
|
||||
value_name = "CATEGORY",
|
||||
value_enum,
|
||||
requires = "install_from",
|
||||
conflicts_with_all = ["update_bundle", "uninstall"],
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub install_builtins: Option<AssetCategory>,
|
||||
/// Restrict a remote install to a single asset category
|
||||
#[arg(
|
||||
long,
|
||||
value_name = "CATEGORY",
|
||||
value_enum,
|
||||
requires = "install",
|
||||
conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"],
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub filter: Option<InstallFilter>,
|
||||
/// Overwrite all conflicts without prompting (used with --install-from)
|
||||
/// Overwrite all conflicts without prompting (remote installs only)
|
||||
#[arg(
|
||||
long,
|
||||
requires = "install_from",
|
||||
requires = "install",
|
||||
conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"],
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub install_force: bool,
|
||||
/// Update an installed bundle from its recorded source (NAME may be suffixed with #<ref> to move a pin)
|
||||
#[arg(
|
||||
long,
|
||||
value_name = "NAME",
|
||||
group = "yes_scope",
|
||||
conflicts_with_all = ["uninstall"],
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub update_bundle: Option<String>,
|
||||
/// Uninstall a bundle: delete its owned files and remove its mcp.json entries
|
||||
#[arg(
|
||||
long,
|
||||
value_name = "NAME",
|
||||
group = "yes_scope",
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub uninstall: Option<String>,
|
||||
/// Proceed without prompts for --uninstall and --update-bundle (locally modified items are always kept)
|
||||
#[arg(
|
||||
long,
|
||||
requires = "yes_scope",
|
||||
conflicts_with_all = ["install", "install_builtins"],
|
||||
help_heading = "Installation & Updates"
|
||||
)]
|
||||
pub yes: bool,
|
||||
/// Sync models updates
|
||||
#[arg(long, help_heading = "Installation & Updates")]
|
||||
pub sync_models: bool,
|
||||
@@ -492,6 +541,7 @@ mod tests {
|
||||
assert!(parse(&["--list-rags"]).list_rags);
|
||||
assert!(parse(&["--list-macros"]).list_macros);
|
||||
assert!(parse(&["--list-skills"]).list_skills);
|
||||
assert!(parse(&["--list-bundles"]).list_bundles);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -500,6 +550,119 @@ mod tests {
|
||||
assert!(parse(&[]).skill.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_update_bundle_flag_takes_name() {
|
||||
assert_eq!(
|
||||
parse(&["--update-bundle", "foo"]).update_bundle.as_deref(),
|
||||
Some("foo")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_uninstall_flag_takes_name() {
|
||||
assert_eq!(
|
||||
parse(&["--uninstall", "foo"]).uninstall.as_deref(),
|
||||
Some("foo")
|
||||
);
|
||||
assert!(!parse(&["--uninstall", "foo"]).yes);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_yes_flag_requires_uninstall_or_update_bundle() {
|
||||
assert!(parse(&["--uninstall", "foo", "--yes"]).yes);
|
||||
assert!(parse(&["--update-bundle", "foo", "--yes"]).yes);
|
||||
assert!(Cli::try_parse_from(["coyote", "--yes"]).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_install_flag_takes_url_or_name() {
|
||||
assert_eq!(
|
||||
parse(&["--install", "https://github.com/x/y"])
|
||||
.install
|
||||
.as_deref(),
|
||||
Some("https://github.com/x/y")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_install_builtins_flag_takes_category() {
|
||||
assert_eq!(
|
||||
parse(&["--install-builtins", "agents"]).install_builtins,
|
||||
Some(AssetCategory::Agents)
|
||||
);
|
||||
assert_eq!(
|
||||
parse(&["--install-builtins", "mcp_config"]).install_builtins,
|
||||
Some(AssetCategory::McpConfig)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_install_builtins_conflicts_with_install() {
|
||||
assert!(
|
||||
Cli::try_parse_from(["coyote", "--install-builtins", "agents", "--install", "x"])
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_lifecycle_flags_are_mutually_exclusive() {
|
||||
assert!(Cli::try_parse_from(["coyote", "--install", "x", "--uninstall", "y"]).is_err());
|
||||
assert!(
|
||||
Cli::try_parse_from(["coyote", "--update-bundle", "x", "--uninstall", "y"]).is_err()
|
||||
);
|
||||
assert!(
|
||||
Cli::try_parse_from(["coyote", "--install-builtins", "agents", "--uninstall", "y"])
|
||||
.is_err()
|
||||
);
|
||||
assert!(Cli::try_parse_from(["coyote", "--install", "x", "--update-bundle", "y"]).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_companion_flags_conflict_with_other_lifecycle_actions() {
|
||||
assert!(
|
||||
Cli::try_parse_from(["coyote", "--update-bundle", "x", "--filter", "agents"]).is_err()
|
||||
);
|
||||
assert!(Cli::try_parse_from(["coyote", "--uninstall", "x", "--install-force"]).is_err());
|
||||
assert!(Cli::try_parse_from(["coyote", "--install", "x", "--yes"]).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_filter_requires_install() {
|
||||
assert!(Cli::try_parse_from(["coyote", "--filter", "agents"]).is_err());
|
||||
assert_eq!(
|
||||
parse(&["--install", "https://github.com/x/y", "--filter", "agents"]).filter,
|
||||
Some(InstallFilter::Agents)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_install_force_requires_install() {
|
||||
assert!(Cli::try_parse_from(["coyote", "--install-force"]).is_err());
|
||||
assert!(parse(&["--install", "https://github.com/x/y", "--install-force"]).install_force);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_git_host_requires_install() {
|
||||
assert!(Cli::try_parse_from(["coyote", "--git-host", "git.x.com"]).is_err());
|
||||
assert!(
|
||||
Cli::try_parse_from(["coyote", "--git-host", "gitlab.com", "--update-bundle", "x"])
|
||||
.is_err()
|
||||
);
|
||||
assert_eq!(
|
||||
parse(&["--install", "someuser/omc", "--git-host", "git.x.com"])
|
||||
.git_host
|
||||
.as_deref(),
|
||||
Some("git.x.com")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn help_shows_install_builtins() {
|
||||
use clap::CommandFactory;
|
||||
let help = Cli::command().render_long_help().to_string();
|
||||
assert!(help.contains("--install-builtins"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_multiple_skill_flags_preserves_order() {
|
||||
assert_eq!(
|
||||
|
||||
+86
-9
@@ -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<bool> {
|
||||
self.config.memory
|
||||
}
|
||||
@@ -744,6 +748,8 @@ pub struct AgentConfig {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub enabled_skills: Option<Vec<String>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub enabled_macros: Option<Vec<String>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub continuation_prompt: Option<String>,
|
||||
#[serde(default)]
|
||||
pub instructions: String,
|
||||
@@ -1125,6 +1131,12 @@ struct AgentMetadataStub {
|
||||
description: String,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct AgentVariablesStub {
|
||||
#[serde(default)]
|
||||
variables: Vec<AgentVariable>,
|
||||
}
|
||||
|
||||
fn load_agent_description(name: &str) -> String {
|
||||
if let Ok(config) = AgentConfig::load(&paths::agent_config_file(name)) {
|
||||
return config.description;
|
||||
@@ -1139,16 +1151,22 @@ fn load_agent_description(name: &str) -> String {
|
||||
String::new()
|
||||
}
|
||||
|
||||
pub fn complete_agent_variables(agent_name: &str) -> Vec<(String, Option<String>)> {
|
||||
let config_path = paths::agent_config_file(agent_name);
|
||||
if !config_path.exists() {
|
||||
return vec![];
|
||||
fn load_agent_variables(name: &str) -> Vec<AgentVariable> {
|
||||
if let Ok(config) = AgentConfig::load(&paths::agent_config_file(name)) {
|
||||
return config.variables;
|
||||
}
|
||||
let Ok(config) = AgentConfig::load(&config_path) else {
|
||||
return vec![];
|
||||
};
|
||||
config
|
||||
.variables
|
||||
|
||||
if let Ok(contents) = read_to_string(paths::agent_graph_file(name))
|
||||
&& let Ok(stub) = serde_yaml::from_str::<AgentVariablesStub>(&contents)
|
||||
{
|
||||
return stub.variables;
|
||||
}
|
||||
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
pub fn complete_agent_variables(agent_name: &str) -> Vec<(String, Option<String>)> {
|
||||
load_agent_variables(agent_name)
|
||||
.iter()
|
||||
.map(|v| {
|
||||
let description = match &v.default {
|
||||
@@ -1225,6 +1243,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 =
|
||||
@@ -1369,6 +1411,41 @@ version: "1.0"
|
||||
assert_eq!(meta.description, "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_variables_stub_extracts_variables_from_graph_yaml() {
|
||||
let yaml = r#"
|
||||
name: coder
|
||||
description: Implementation agent.
|
||||
version: "1.0"
|
||||
variables:
|
||||
- name: task
|
||||
description: The task to implement
|
||||
- name: scope
|
||||
description: Directory scope
|
||||
default: src/
|
||||
start: plan
|
||||
nodes: {}
|
||||
"#;
|
||||
|
||||
let stub: AgentVariablesStub = serde_yaml::from_str(yaml).unwrap();
|
||||
|
||||
assert_eq!(stub.variables.len(), 2);
|
||||
assert_eq!(stub.variables[0].name, "task");
|
||||
assert_eq!(stub.variables[0].description, "The task to implement");
|
||||
assert_eq!(stub.variables[0].default, None);
|
||||
assert_eq!(stub.variables[1].name, "scope");
|
||||
assert_eq!(stub.variables[1].default.as_deref(), Some("src/"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_variables_stub_defaults_when_variables_missing() {
|
||||
let yaml = "name: coder\nversion: \"1.0\"\nstart: plan\nnodes: {}\n";
|
||||
|
||||
let stub: AgentVariablesStub = serde_yaml::from_str(yaml).unwrap();
|
||||
|
||||
assert!(stub.variables.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rag_init_config_forwards_an_explicit_driver() {
|
||||
let node: RagNode =
|
||||
|
||||
@@ -43,6 +43,8 @@ pub struct AppConfig {
|
||||
#[serde(default, deserialize_with = "super::deserialize_csv_or_vec")]
|
||||
pub enabled_skills: Option<Vec<String>>,
|
||||
pub visible_skills: Option<Vec<String>>,
|
||||
#[serde(default, deserialize_with = "super::deserialize_csv_or_vec")]
|
||||
pub enabled_macros: Option<Vec<String>>,
|
||||
|
||||
pub mcp_server_support: bool,
|
||||
pub mapping_mcp_servers: IndexMap<String, String>,
|
||||
@@ -96,6 +98,7 @@ pub struct AppConfig {
|
||||
pub user_agent: Option<String>,
|
||||
pub save_shell_history: bool,
|
||||
pub no_workspace_mcp: bool,
|
||||
pub no_workspace_macros: bool,
|
||||
pub sync_models_url: Option<String>,
|
||||
|
||||
pub clients: Vec<ClientConfig>,
|
||||
@@ -127,6 +130,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(),
|
||||
@@ -178,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![],
|
||||
@@ -211,6 +216,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,
|
||||
@@ -262,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,
|
||||
@@ -533,6 +540,10 @@ impl AppConfig {
|
||||
self.enabled_skills = v.map(|raw| super::csv_to_vec(&raw));
|
||||
}
|
||||
|
||||
if let Some(v) = super::read_env_value::<String>(&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 +780,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()
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
+3397
-59
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,983 @@
|
||||
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;
|
||||
|
||||
pub const RESERVED_MACRO_NAMES: [&str; 2] = ["enable", "disable"];
|
||||
|
||||
#[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"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum MacroState {
|
||||
Enabled,
|
||||
DisabledRuntime,
|
||||
Locked { level: MacroAllowlistLevel },
|
||||
Missing,
|
||||
ShadowedBuiltin,
|
||||
Invalid { reason: String },
|
||||
}
|
||||
|
||||
impl MacroState {
|
||||
pub fn is_invocable(&self) -> bool {
|
||||
matches!(self, MacroState::Enabled | MacroState::ShadowedBuiltin)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct DiscoveredMacro {
|
||||
pub name: String,
|
||||
pub source: MacroSource,
|
||||
pub definition: Result<Macro, String>,
|
||||
pub shadowed_by_workspace: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ResolvedMacro {
|
||||
pub name: String,
|
||||
pub source: Option<MacroSource>,
|
||||
pub description: Option<String>,
|
||||
pub isolated: Option<bool>,
|
||||
pub shadowed_by_workspace: bool,
|
||||
pub state: MacroState,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct MacroPolicy {
|
||||
pub macros: Vec<ResolvedMacro>,
|
||||
}
|
||||
|
||||
impl MacroPolicy {
|
||||
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<DiscoveredMacro>,
|
||||
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<ResolvedMacro> = 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<ResolvedMacro> = 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 }
|
||||
}
|
||||
|
||||
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<MacroSource>) -> 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
|
||||
}
|
||||
|
||||
pub fn discover_macros(no_workspace_macros: bool) -> Vec<DiscoveredMacro> {
|
||||
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<DiscoveredMacro> {
|
||||
let mut seen: HashSet<String> = 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::<Macro>(&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::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::{env, fs, process};
|
||||
|
||||
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<DiscoveredMacro> {
|
||||
names.iter().map(|n| disc(n, MacroSource::Global)).collect()
|
||||
}
|
||||
|
||||
fn list(names: &[&str]) -> Vec<String> {
|
||||
names.iter().map(|s| s.to_string()).collect()
|
||||
}
|
||||
|
||||
fn resolve(
|
||||
discovered: Vec<DiscoveredMacro>,
|
||||
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: FnOnce(&Path, &Path)>(f: F) {
|
||||
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");
|
||||
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<F: FnOnce()>(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_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() {
|
||||
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
|
||||
}
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
}
|
||||
+827
-27
File diff suppressed because it is too large
Load Diff
+63
-1
@@ -1,9 +1,11 @@
|
||||
mod agent;
|
||||
mod app_config;
|
||||
mod app_state;
|
||||
mod bundles;
|
||||
mod input;
|
||||
mod install_remote;
|
||||
pub(crate) mod instructions;
|
||||
mod macro_policy;
|
||||
mod macros;
|
||||
mod mcp_factory;
|
||||
pub(crate) mod memory;
|
||||
@@ -28,8 +30,15 @@ pub use self::agent::{
|
||||
pub use self::app_config::AppConfig;
|
||||
#[allow(unused_imports)]
|
||||
pub use self::app_state::AppState;
|
||||
pub use self::bundles::list_installed_bundles;
|
||||
pub use self::input::Input;
|
||||
pub use self::install_remote::{install_remote, install_remote_from_repl_args};
|
||||
pub use self::install_remote::{
|
||||
DEFAULT_GIT_HOST, install_or_update, install_or_update_from_repl_args, uninstall_bundle,
|
||||
update_bundle,
|
||||
};
|
||||
pub use self::macro_policy::{
|
||||
MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, ResolvedMacro,
|
||||
};
|
||||
#[allow(unused_imports)]
|
||||
pub use self::request_context::{RenderMode, RequestContext, should_inject_skill_instructions};
|
||||
pub use self::role::{
|
||||
@@ -151,6 +160,8 @@ const SBX_KIT_DIR_NAME: &str = "sbx-kit";
|
||||
const SBX_KIT_HASH_FILE: &str = "kit.sha256";
|
||||
const SBX_MIXIN_FILE_NAME: &str = "sbx-mixin.yaml";
|
||||
pub(crate) const VAULT_DATA_FILE_NAME: &str = "vault.yml";
|
||||
const INSTALLED_BUNDLES_FILE_NAME: &str = "installed-bundles.yaml";
|
||||
const BUNDLE_MANIFEST_FILE: &str = "coyote-bundle.yaml";
|
||||
const SBX_MIXIN_KITS_DIR_NAME: &str = "sbx-mixin-kits";
|
||||
const GIT_DIR_NAME: &str = ".git";
|
||||
const GITIGNORE_FILE_NAME: &str = ".gitignore";
|
||||
@@ -221,6 +232,8 @@ pub struct Config {
|
||||
#[serde(default, deserialize_with = "deserialize_csv_or_vec")]
|
||||
pub enabled_skills: Option<Vec<String>>,
|
||||
pub visible_skills: Option<Vec<String>>,
|
||||
#[serde(default, deserialize_with = "deserialize_csv_or_vec")]
|
||||
pub enabled_macros: Option<Vec<String>>,
|
||||
|
||||
pub mcp_server_support: bool,
|
||||
pub mapping_mcp_servers: IndexMap<String, String>,
|
||||
@@ -303,6 +316,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,9 +1138,50 @@ 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();
|
||||
|
||||
assert!(pass.assert(StateFlags::empty()));
|
||||
assert!(pass.assert(StateFlags::ROLE));
|
||||
assert!(pass.assert(StateFlags::SESSION | StateFlags::AGENT));
|
||||
@@ -1136,6 +1191,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));
|
||||
@@ -1144,6 +1200,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()));
|
||||
@@ -1153,6 +1210,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));
|
||||
@@ -1163,6 +1221,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));
|
||||
@@ -1172,6 +1231,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));
|
||||
@@ -1182,6 +1242,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()));
|
||||
@@ -1192,6 +1253,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));
|
||||
|
||||
+12
-9
@@ -2,10 +2,10 @@ use super::role::Role;
|
||||
use super::{
|
||||
AGENT_GRAPH_FILE_NAME, AGENTS_DIR_NAME, BASH_PROMPT_UTILS_FILE_NAME, CONFIG_FILE_NAME,
|
||||
ENV_FILE_NAME, FUNCTIONS_BIN_DIR_NAME, FUNCTIONS_DIR_NAME, GLOBAL_TOOLS_DIR_NAME,
|
||||
GLOBAL_TOOLS_UTILS_DIR_NAME, HIDDEN_MCP_FILE_NAME, MACROS_DIR_NAME, MCP_FILE_NAME,
|
||||
MEMORY_DIR_NAME, MEMORY_INDEX_FILE_NAME, ModelsOverride, RAGS_DIR_NAME, ROLES_DIR_NAME,
|
||||
SBX_KIT_DIR_NAME, SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME, SBX_MIXIN_KITS_DIR_NAME,
|
||||
SKILLS_DIR_NAME, WORKSPACE_COYOTE_DIR_NAME,
|
||||
GLOBAL_TOOLS_UTILS_DIR_NAME, HIDDEN_MCP_FILE_NAME, INSTALLED_BUNDLES_FILE_NAME,
|
||||
MACROS_DIR_NAME, MCP_FILE_NAME, MEMORY_DIR_NAME, MEMORY_INDEX_FILE_NAME, ModelsOverride,
|
||||
RAGS_DIR_NAME, ROLES_DIR_NAME, SBX_KIT_DIR_NAME, SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME,
|
||||
SBX_MIXIN_KITS_DIR_NAME, SKILLS_DIR_NAME, WORKSPACE_COYOTE_DIR_NAME,
|
||||
};
|
||||
use crate::client::ProviderModels;
|
||||
use crate::config::REPL_HISTORY_DIR_NAME;
|
||||
@@ -169,6 +169,10 @@ pub fn config_file() -> PathBuf {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn installed_bundles_file() -> PathBuf {
|
||||
local_dir(INSTALLED_BUNDLES_FILE_NAME)
|
||||
}
|
||||
|
||||
pub fn roles_dir() -> PathBuf {
|
||||
match env::var(get_env_name("roles_dir")) {
|
||||
Ok(value) => PathBuf::from(value),
|
||||
@@ -214,6 +218,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<PathBuf> {
|
||||
workspace_mcp_config_file_in(&env::current_dir().unwrap_or_default())
|
||||
}
|
||||
@@ -470,11 +478,6 @@ pub fn list_macros() -> Vec<String> {
|
||||
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<String> {
|
||||
let mut names = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
|
||||
+647
-45
@@ -1,3 +1,4 @@
|
||||
use super::bundles::BundleStore;
|
||||
use super::rag_cache::{RagCache, RagKey};
|
||||
use super::session::Session;
|
||||
use super::skill::{SKILL_SCAFFOLD, Skill};
|
||||
@@ -7,9 +8,10 @@ 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,
|
||||
TEMP_ROLE_NAME, TEMP_SESSION_NAME, WorkingMode, bundles, ensure_parent_exists,
|
||||
list_agents_with_descriptions, memory, paths,
|
||||
};
|
||||
use super::{MessageContentToolCalls, prompts};
|
||||
@@ -32,8 +34,11 @@ 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::install_remote::DEFAULT_GIT_HOST;
|
||||
use super::instructions;
|
||||
use super::macros::Macro;
|
||||
use super::memory::{
|
||||
DEFAULT_MEMORY_CAP_WITH_TOOLS, DEFAULT_MEMORY_CAP_WITHOUT_TOOLS, MemoryStore, WorkspaceMemory,
|
||||
};
|
||||
@@ -56,6 +61,22 @@ use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use std::{env, fs};
|
||||
|
||||
/// Completion must degrade rather than break the prompt, but a corrupt store
|
||||
/// should not vanish silently: the failure is logged before returning empty.
|
||||
fn installed_bundle_names() -> Vec<String> {
|
||||
match BundleStore::load() {
|
||||
Ok(store) => store
|
||||
.bundle_names()
|
||||
.into_iter()
|
||||
.map(str::to_string)
|
||||
.collect(),
|
||||
Err(e) => {
|
||||
warn!("skipping bundle-name completion: {e:#}");
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub struct AutoContinueConfig {
|
||||
pub enabled: bool,
|
||||
pub max_continues: usize,
|
||||
@@ -110,6 +131,14 @@ fn print_asset_names(kind: &str, names: &[String]) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) 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<String>) -> Vec<(String, Option<String>)> {
|
||||
names
|
||||
.into_iter()
|
||||
@@ -123,6 +152,95 @@ fn complete_skills_with_descriptions(names: Vec<String>) -> Vec<(String, Option<
|
||||
.collect()
|
||||
}
|
||||
|
||||
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",
|
||||
];
|
||||
|
||||
fn toggled_enabled_macros(
|
||||
current: Option<&[String]>,
|
||||
all_active: &[String],
|
||||
name: &str,
|
||||
enable: bool,
|
||||
) -> Option<Vec<String>> {
|
||||
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
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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})"),
|
||||
}
|
||||
}
|
||||
|
||||
fn macro_source_display(source: Option<MacroSource>) -> String {
|
||||
match source {
|
||||
Some(source) => source.to_string(),
|
||||
None => "-".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
|
||||
pub enum RenderMode {
|
||||
#[default]
|
||||
@@ -134,6 +252,7 @@ pub struct RequestContext {
|
||||
pub app: Arc<AppState>,
|
||||
|
||||
pub macro_flag: bool,
|
||||
pub macro_non_isolated: bool,
|
||||
pub info_flag: bool,
|
||||
pub working_mode: WorkingMode,
|
||||
|
||||
@@ -171,6 +290,7 @@ impl RequestContext {
|
||||
Self {
|
||||
app,
|
||||
macro_flag: false,
|
||||
macro_non_isolated: false,
|
||||
info_flag: false,
|
||||
working_mode,
|
||||
model: Default::default(),
|
||||
@@ -225,6 +345,7 @@ impl RequestContext {
|
||||
Ok(Self {
|
||||
app,
|
||||
macro_flag: false,
|
||||
macro_non_isolated: false,
|
||||
info_flag,
|
||||
working_mode,
|
||||
model,
|
||||
@@ -274,6 +395,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(),
|
||||
@@ -313,6 +435,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(),
|
||||
@@ -1577,6 +1700,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 +2484,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 +2504,168 @@ impl RequestContext {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn in_non_isolated_macro(&self) -> bool {
|
||||
self.macro_flag && self.macro_non_isolated
|
||||
}
|
||||
|
||||
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(),
|
||||
self.app.config.no_workspace_macros,
|
||||
)
|
||||
}
|
||||
|
||||
fn macro_variable_completions(
|
||||
&self,
|
||||
cmd: &str,
|
||||
completed_args: &[&str],
|
||||
) -> Vec<(String, Option<String>)> {
|
||||
let Some(name) = cmd.strip_prefix('.') else {
|
||||
return vec![];
|
||||
};
|
||||
if !self
|
||||
.visible_macro_completions()
|
||||
.iter()
|
||||
.any(|(macro_name, _)| macro_name == name)
|
||||
{
|
||||
return vec![];
|
||||
}
|
||||
match Macro::load(name, self.app.config.no_workspace_macros) {
|
||||
Ok(macro_value) => macro_value.variable_completions(completed_args),
|
||||
Err(_) => vec![],
|
||||
}
|
||||
}
|
||||
|
||||
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(),
|
||||
}
|
||||
}
|
||||
|
||||
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<String> = 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(())
|
||||
}
|
||||
|
||||
pub fn visible_macro_completions(&self) -> Vec<(String, Option<String>)> {
|
||||
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(());
|
||||
}
|
||||
|
||||
let mut table =
|
||||
asset_table(&["name", "source", "isolated", "state", "description"]);
|
||||
|
||||
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();
|
||||
table.add_row(vec![
|
||||
row.name.as_str(),
|
||||
&source,
|
||||
isolated,
|
||||
&state,
|
||||
description,
|
||||
]);
|
||||
}
|
||||
|
||||
println!("Macros:");
|
||||
println!("{table}");
|
||||
Ok(())
|
||||
}
|
||||
"agents" => {
|
||||
let entries = list_agents_with_descriptions();
|
||||
if entries.is_empty() {
|
||||
@@ -2387,15 +2673,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" => {
|
||||
@@ -2441,16 +2725,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" => {
|
||||
@@ -2540,8 +2826,9 @@ impl RequestContext {
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
"bundles" => bundles::list_installed_bundles(),
|
||||
_ => bail!(
|
||||
"Unknown kind '{kind}'. Valid kinds: roles, sessions, agents, rags, macros, skills, tools, mcp-servers"
|
||||
"Unknown kind '{kind}'. Valid kinds: roles, sessions, agents, rags, macros, skills, tools, mcp-servers, bundles"
|
||||
),
|
||||
}
|
||||
}
|
||||
@@ -2728,6 +3015,23 @@ impl RequestContext {
|
||||
}
|
||||
self.update_app_config(|app| app.enabled_skills = parsed.clone());
|
||||
}
|
||||
"enabled_macros" => {
|
||||
let raw: Option<String> = super::parse_value(value)?;
|
||||
let parsed: Option<Vec<String>> = 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<bool> = super::parse_value(value)?;
|
||||
if let Some(session) = self.session.as_mut() {
|
||||
@@ -2996,12 +3300,55 @@ impl RequestContext {
|
||||
.map(|(name, desc)| (name, if desc.is_empty() { None } else { Some(desc) }))
|
||||
.collect(),
|
||||
".install" => {
|
||||
let mut values: Vec<String> =
|
||||
let mut names: Vec<String> =
|
||||
AssetCategory::NAMES.iter().map(|s| s.to_string()).collect();
|
||||
values.push("remote".to_string());
|
||||
super::map_completion_values(values)
|
||||
names.extend(installed_bundle_names());
|
||||
let mut values = super::map_completion_values(names);
|
||||
values.push((
|
||||
"--git-host".to_string(),
|
||||
Some("Host the owner/repo shorthand expands against".to_string()),
|
||||
));
|
||||
values.push((
|
||||
"--help".to_string(),
|
||||
Some("Show usage for .install".to_string()),
|
||||
));
|
||||
values
|
||||
}
|
||||
".uninstall" => {
|
||||
let mut values = super::map_completion_values(installed_bundle_names());
|
||||
values.push((
|
||||
"--yes".to_string(),
|
||||
Some("Skip the uninstall confirmation".to_string()),
|
||||
));
|
||||
values.push((
|
||||
"--help".to_string(),
|
||||
Some("Show usage for .uninstall".to_string()),
|
||||
));
|
||||
|
||||
values
|
||||
}
|
||||
".macro" => {
|
||||
let policy = self.macro_policy();
|
||||
let mut values: Vec<(String, Option<String>)> = 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
|
||||
}
|
||||
".macro" => super::map_completion_values(paths::list_macros()),
|
||||
".reasoning" => {
|
||||
let levels = self.current_model().reasoning_levels();
|
||||
levels.iter().map(|v| (v.clone(), None)).collect()
|
||||
@@ -3016,32 +3363,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");
|
||||
}
|
||||
@@ -3068,6 +3390,7 @@ impl RequestContext {
|
||||
"skills",
|
||||
"tools",
|
||||
"mcp-servers",
|
||||
"bundles",
|
||||
]),
|
||||
".vault" => {
|
||||
let mut values = vec!["add", "get", "update", "delete", "list"];
|
||||
@@ -3077,7 +3400,7 @@ impl RequestContext {
|
||||
.map(|v| (format!("{v} "), None))
|
||||
.collect()
|
||||
}
|
||||
_ => vec![],
|
||||
_ => self.macro_variable_completions(cmd, &[]),
|
||||
};
|
||||
} else if cmd == ".mcp" && args.first() == Some(&"auth") && args.len() == 2 {
|
||||
if let Some(mcp_config) = &self.app.mcp_config {
|
||||
@@ -3185,23 +3508,55 @@ 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 == ".macro"
|
||||
&& args.len() >= 2
|
||||
&& args.first() != Some(&"enable")
|
||||
&& args.first() != Some(&"disable")
|
||||
{
|
||||
if let Ok(macro_value) = Macro::load(args[0], app.no_workspace_macros) {
|
||||
values = macro_value.variable_completions(&args[1..args.len() - 1]);
|
||||
}
|
||||
} else if (cmd == ".edit" && args.first() == Some(&"skill") && args.len() == 2)
|
||||
|| (cmd == ".skill" && args.first() == Some(&"load") && args.len() == 2)
|
||||
{
|
||||
values = complete_skills_with_descriptions(paths::list_skills());
|
||||
} else if cmd == ".skill" && args.first() == Some(&"unload") && args.len() == 2 {
|
||||
values = complete_skills_with_descriptions(self.skill_registry.loaded_names());
|
||||
} else if cmd == ".install" && args.first() == Some(&"remote") && args.len() >= 2 {
|
||||
} else if cmd == ".install" && args.len() >= 2 {
|
||||
let prev = args.get(args.len() - 2).copied().unwrap_or("");
|
||||
if prev == "--filter" {
|
||||
values = super::map_completion_values(
|
||||
InstallFilter::NAMES.iter().map(|s| s.to_string()).collect(),
|
||||
);
|
||||
} else if prev == "--git-host" {
|
||||
values = super::map_completion_values(vec![DEFAULT_GIT_HOST.to_string()]);
|
||||
} else {
|
||||
let has_filter = args.iter().enumerate().any(|(i, a)| {
|
||||
a.starts_with("--filter=") || (*a == "--filter" && i < args.len() - 1)
|
||||
});
|
||||
let has_force = args.contains(&"--force");
|
||||
let has_git_host = args.iter().enumerate().any(|(i, a)| {
|
||||
a.starts_with("--git-host=") || (*a == "--git-host" && i < args.len() - 1)
|
||||
});
|
||||
let mut available: Vec<&str> = vec![];
|
||||
|
||||
if !has_filter {
|
||||
@@ -3210,6 +3565,12 @@ impl RequestContext {
|
||||
if !has_force {
|
||||
available.push("--force");
|
||||
}
|
||||
if !has_git_host {
|
||||
available.push("--git-host");
|
||||
}
|
||||
if !args.contains(&"--help") {
|
||||
available.push("--help");
|
||||
}
|
||||
|
||||
values = super::map_completion_values(available);
|
||||
}
|
||||
@@ -3345,6 +3706,8 @@ impl RequestContext {
|
||||
.collect();
|
||||
}
|
||||
values.extend(super::complete_agent_variables(args[0]));
|
||||
} else if args.len() >= 2 {
|
||||
values = self.macro_variable_completions(cmd, &args[..args.len() - 1]);
|
||||
};
|
||||
fuzzy_filter(values, |v| v.0.as_str(), filter)
|
||||
}
|
||||
@@ -3740,8 +4103,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())
|
||||
@@ -4655,6 +5021,64 @@ mod tests {
|
||||
assert!(!ctx.maybe_autoname_session());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn repl_complete_uninstall_offers_installed_bundle_names() {
|
||||
let _guard = TestConfigDirGuard::new();
|
||||
let mut store = BundleStore::load().unwrap();
|
||||
store
|
||||
.upsert_bundle(
|
||||
"omc",
|
||||
bundles::InstallMetadata {
|
||||
source: "https://github.com/x/omc".to_string(),
|
||||
git_ref: None,
|
||||
commit: "abc123".to_string(),
|
||||
version: None,
|
||||
description: None,
|
||||
homepage: None,
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
let ctx = create_test_ctx();
|
||||
|
||||
let values = ctx.repl_complete(".uninstall", &[""], "");
|
||||
|
||||
assert!(
|
||||
values.iter().any(|(name, _)| name == "omc"),
|
||||
"got: {values:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn repl_complete_install_offers_categories_and_bundles() {
|
||||
let _guard = TestConfigDirGuard::new();
|
||||
let mut store = BundleStore::load().unwrap();
|
||||
store
|
||||
.upsert_bundle(
|
||||
"omc",
|
||||
bundles::InstallMetadata {
|
||||
source: "https://github.com/x/omc".to_string(),
|
||||
git_ref: None,
|
||||
commit: "abc123".to_string(),
|
||||
version: None,
|
||||
description: None,
|
||||
homepage: None,
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
let ctx = create_test_ctx();
|
||||
|
||||
let values = ctx.repl_complete(".install", &[""], "");
|
||||
|
||||
for expected in ["agents", "omc"] {
|
||||
assert!(
|
||||
values.iter().any(|(name, _)| name == expected),
|
||||
"missing '{expected}'; got: {values:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn exit_agent_clears_all_agent_state() {
|
||||
@@ -6290,6 +6714,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<PathBuf> {
|
||||
for entry in read_dir(dir).ok()?.flatten() {
|
||||
let path = entry.path();
|
||||
@@ -6463,4 +6954,115 @@ mod tests {
|
||||
"install_mcp_config must add new bundled servers"
|
||||
);
|
||||
}
|
||||
|
||||
fn strings(names: &[&str]) -> Vec<String> {
|
||||
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<Vec<String>>, &'static str, bool, Option<Vec<String>>);
|
||||
let cases: Vec<ToggleCase> = 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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,6 +75,12 @@ pub struct Role {
|
||||
deserialize_with = "super::deserialize_csv_or_vec"
|
||||
)]
|
||||
enabled_skills: Option<Vec<String>>,
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
deserialize_with = "super::deserialize_csv_or_vec"
|
||||
)]
|
||||
enabled_macros: Option<Vec<String>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
auto_continue: Option<bool>,
|
||||
#[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}"));
|
||||
}
|
||||
@@ -357,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);
|
||||
}
|
||||
@@ -543,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");
|
||||
}
|
||||
@@ -551,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));
|
||||
@@ -561,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()])
|
||||
@@ -571,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()])
|
||||
@@ -581,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);
|
||||
@@ -588,9 +611,67 @@ 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();
|
||||
|
||||
assert_eq!(role.name(), "shell");
|
||||
assert!(!role.prompt().is_empty());
|
||||
}
|
||||
@@ -598,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());
|
||||
}
|
||||
@@ -605,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);
|
||||
@@ -620,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));
|
||||
}
|
||||
|
||||
@@ -635,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"));
|
||||
}
|
||||
@@ -649,6 +739,7 @@ Input 1
|
||||
### OUTPUT:
|
||||
Output 1
|
||||
"#;
|
||||
|
||||
assert_eq!(
|
||||
parse_structure_prompt(prompt),
|
||||
("System message", vec![("Input 1", "Output 1")])
|
||||
@@ -663,6 +754,7 @@ Input 1
|
||||
### OUTPUT:
|
||||
Output 1
|
||||
"#;
|
||||
|
||||
assert_eq!(
|
||||
parse_structure_prompt(prompt),
|
||||
("", vec![("Input 1", "Output 1")])
|
||||
@@ -676,6 +768,7 @@ System message
|
||||
### INPUT:
|
||||
Input 1
|
||||
"#;
|
||||
|
||||
assert_eq!(parse_structure_prompt(prompt), (prompt, vec![]));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,6 +46,12 @@ pub struct Session {
|
||||
deserialize_with = "super::deserialize_csv_or_vec"
|
||||
)]
|
||||
enabled_skills: Option<Vec<String>>,
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
deserialize_with = "super::deserialize_csv_or_vec"
|
||||
)]
|
||||
enabled_macros: Option<Vec<String>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
save_session: Option<bool>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
@@ -107,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<bool>) {
|
||||
if self.skills_enabled != value {
|
||||
self.skills_enabled = value;
|
||||
@@ -236,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();
|
||||
}
|
||||
@@ -315,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()));
|
||||
}
|
||||
@@ -925,12 +942,57 @@ 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);
|
||||
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());
|
||||
@@ -945,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");
|
||||
@@ -984,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));
|
||||
}
|
||||
|
||||
@@ -1074,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());
|
||||
}
|
||||
|
||||
|
||||
+12
-4
@@ -165,7 +165,16 @@ pub(crate) fn write_file_atomic(
|
||||
std::process::id(),
|
||||
TMP_COUNTER.fetch_add(1, Ordering::Relaxed)
|
||||
));
|
||||
fs::write(&tmp, content)?;
|
||||
let write_synced = || -> io::Result<()> {
|
||||
use std::io::Write;
|
||||
let mut file = File::create(&tmp)?;
|
||||
file.write_all(content.as_bytes())?;
|
||||
file.sync_all()
|
||||
};
|
||||
if let Err(err) = write_synced() {
|
||||
let _ = fs::remove_file(&tmp);
|
||||
return Err(err.into());
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
if let Some(mode) = mode {
|
||||
@@ -484,9 +493,8 @@ impl Functions {
|
||||
|
||||
let serialized =
|
||||
serde_json::to_string_pretty(&merged).context("failed to serialize merged mcp.json")?;
|
||||
let tmp = file_path.with_extension("json.tmp");
|
||||
fs::write(&tmp, &serialized).context("failed to write temporary mcp.json")?;
|
||||
fs::rename(&tmp, &file_path).context("failed to finalize mcp.json")?;
|
||||
write_file_atomic(&file_path, &serialized, None)
|
||||
.context("failed to write merged mcp.json")?;
|
||||
|
||||
if !added.is_empty() {
|
||||
println!(" + new MCP servers: {}", added.join(", "));
|
||||
|
||||
@@ -588,6 +588,16 @@ 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";
|
||||
|
||||
@@ -23,7 +23,7 @@ impl ApprovalNodeExecutor {
|
||||
|
||||
let response = handle_user_tool(
|
||||
ctx,
|
||||
&format!("{USER_FUNCTION_PREFIX}ask"),
|
||||
&format!("{USER_FUNCTION_PREFIX}select"),
|
||||
&json!({ "question": question, "options": node.options }),
|
||||
)
|
||||
.await
|
||||
|
||||
+23
-3
@@ -127,14 +127,31 @@ async fn main() -> Result<()> {
|
||||
return sandbox::launch(name.clone(), cli.fresh);
|
||||
}
|
||||
|
||||
if cli.list_bundles {
|
||||
return config::list_installed_bundles();
|
||||
}
|
||||
|
||||
install_builtins()?;
|
||||
|
||||
if let Some(category) = cli.install {
|
||||
if let Some(category) = cli.install_builtins {
|
||||
return config::install_assets(category);
|
||||
}
|
||||
|
||||
if let Some(url) = cli.install_from.as_deref() {
|
||||
return config::install_remote(url, cli.filter, cli.install_force);
|
||||
if let Some(value) = cli.install.as_deref() {
|
||||
return config::install_or_update(
|
||||
value,
|
||||
cli.git_host.as_deref(),
|
||||
cli.filter,
|
||||
cli.install_force,
|
||||
);
|
||||
}
|
||||
|
||||
if let Some(spec) = cli.update_bundle.as_deref() {
|
||||
return config::update_bundle(spec, cli.yes);
|
||||
}
|
||||
|
||||
if let Some(name) = cli.uninstall.as_deref() {
|
||||
return config::uninstall_bundle(name, cli.yes);
|
||||
}
|
||||
|
||||
if let Some(client_arg) = &cli.authenticate {
|
||||
@@ -222,6 +239,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<AppConfig> = Arc::new(app_config);
|
||||
let app_state: Arc<AppState> = Arc::new(
|
||||
AppState::init(
|
||||
|
||||
+19
-1
@@ -74,7 +74,25 @@ impl Completer for ReplCompleter {
|
||||
format!("{name} ")
|
||||
};
|
||||
create_suggestion(&name, description, span)
|
||||
}))
|
||||
}));
|
||||
|
||||
let macros: Vec<(String, Option<String>)> = ctx
|
||||
.visible_macro_completions()
|
||||
.into_iter()
|
||||
.map(|(name, description)| (format!(".{name}"), description))
|
||||
.filter(|(name, _)| {
|
||||
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);
|
||||
suggestions.extend(macros.iter().map(|(name, description)| {
|
||||
create_suggestion(
|
||||
&format!("{name} "),
|
||||
description.as_deref().unwrap_or_default(),
|
||||
span,
|
||||
)
|
||||
}));
|
||||
}
|
||||
suggestions
|
||||
}
|
||||
|
||||
+314
-36
@@ -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};
|
||||
@@ -53,7 +53,7 @@ pub const DEFAULT_CONTINUATION_PROMPT: &str = indoc! {"
|
||||
4. Continue with the next pending item now. Call tools immediately."
|
||||
};
|
||||
|
||||
static REPL_COMMANDS: LazyLock<[ReplCommand; 60]> = LazyLock::new(|| {
|
||||
static REPL_COMMANDS: LazyLock<[ReplCommand; 61]> = LazyLock::new(|| {
|
||||
[
|
||||
ReplCommand::new(".help", "Show this help guide", AssertState::pass()),
|
||||
ReplCommand::new(".info", "Show system info", AssertState::pass()),
|
||||
@@ -307,7 +307,7 @@ static REPL_COMMANDS: LazyLock<[ReplCommand; 60]> = LazyLock::new(|| {
|
||||
),
|
||||
ReplCommand::new(
|
||||
".list",
|
||||
"List roles, sessions, agents, RAGs, macros, skills, tools, or MCP servers",
|
||||
"List roles, sessions, agents, RAGs, macros, skills, tools, MCP servers, or bundles",
|
||||
AssertState::pass(),
|
||||
),
|
||||
ReplCommand::new(
|
||||
@@ -317,7 +317,12 @@ static REPL_COMMANDS: LazyLock<[ReplCommand; 60]> = LazyLock::new(|| {
|
||||
),
|
||||
ReplCommand::new(
|
||||
".install",
|
||||
"Reinstall bundled assets, or install assets from a remote git repo (.install remote <url>)",
|
||||
"Reinstall bundled assets, install a bundle from a git repo, or update an installed bundle",
|
||||
AssertState::pass(),
|
||||
),
|
||||
ReplCommand::new(
|
||||
".uninstall",
|
||||
"Uninstall an installed bundle (delete its owned files and MCP entries)",
|
||||
AssertState::pass(),
|
||||
),
|
||||
ReplCommand::new(
|
||||
@@ -865,27 +870,18 @@ pub async fn run_repl_command(
|
||||
replay::render(app.as_ref(), &compressed, &active)?;
|
||||
}
|
||||
}
|
||||
".install" => {
|
||||
let trimmed = args.map(str::trim).unwrap_or("");
|
||||
let mut parts = trimmed.splitn(2, char::is_whitespace);
|
||||
match parts.next() {
|
||||
Some("remote") => {
|
||||
let rest = parts.next().unwrap_or("").trim();
|
||||
config::install_remote_from_repl_args(rest)?;
|
||||
}
|
||||
Some(name) if !name.is_empty() => match AssetCategory::parse(name) {
|
||||
Some(category) => config::install_assets(category)?,
|
||||
None => println!(
|
||||
"Unknown asset category '{name}'. Valid categories: {}",
|
||||
AssetCategory::NAMES.join(", ")
|
||||
),
|
||||
},
|
||||
_ => println!(
|
||||
"Usage: .install <{}> | .install remote <git-url>",
|
||||
AssetCategory::NAMES.join("|")
|
||||
),
|
||||
".install" => match parse_repl_install(args) {
|
||||
ReplInstallDispatch::Builtins(category) => config::install_assets(category)?,
|
||||
ReplInstallDispatch::Unified(value) => {
|
||||
config::install_or_update_from_repl_args(value)?;
|
||||
}
|
||||
}
|
||||
ReplInstallDispatch::Help => println!("{}", repl_install_help()),
|
||||
ReplInstallDispatch::Usage => println!(
|
||||
"Usage: .install <{}> | .install <git-url|owner/repo|installed-bundle> \
|
||||
[--git-host <host>] [--filter <cat>] [--force] (see `.install --help`)",
|
||||
AssetCategory::NAMES.join("|")
|
||||
),
|
||||
},
|
||||
".update" => {
|
||||
if ctx.macro_flag {
|
||||
bail!("Cannot perform this operation because you are in a macro")
|
||||
@@ -1073,15 +1069,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} <name>"),
|
||||
}
|
||||
}
|
||||
None => println!("Usage: .macro <name> <text>..."),
|
||||
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 <name> [text]... # Execute a macro
|
||||
.macro enable <name> # Re-enable a runtime-disabled macro
|
||||
.macro disable <name> # Disable a macro for the rest of this process"#
|
||||
),
|
||||
},
|
||||
".file" => match args {
|
||||
Some(args) => {
|
||||
@@ -1173,13 +1198,22 @@ pub async fn run_repl_command(
|
||||
println!("Usage: .delete <role|session|rag|macro|skill|agent-data>")
|
||||
}
|
||||
},
|
||||
".uninstall" => match parse_repl_uninstall(args) {
|
||||
ReplUninstallDispatch::Run(name, assume_yes) => {
|
||||
config::uninstall_bundle(&name, assume_yes)?
|
||||
}
|
||||
ReplUninstallDispatch::Help => println!("{}", repl_uninstall_help()),
|
||||
ReplUninstallDispatch::Usage => {
|
||||
println!("Usage: .uninstall <bundle-name> [--yes] (see `.uninstall --help`)")
|
||||
}
|
||||
},
|
||||
".list" => match args {
|
||||
Some(args) => {
|
||||
ctx.list_assets(args.trim())?;
|
||||
}
|
||||
_ => {
|
||||
println!(
|
||||
"Usage: .list <roles|sessions|agents|rags|macros|skills|tools|mcp-servers>"
|
||||
"Usage: .list <roles|sessions|agents|rags|macros|skills|tools|mcp-servers|bundles>"
|
||||
)
|
||||
}
|
||||
},
|
||||
@@ -1294,7 +1328,26 @@ pub async fn run_repl_command(
|
||||
println!("Usage: .vault <add|get|update|delete|list> [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)
|
||||
),
|
||||
Some(MacroState::Invalid { reason }) => {
|
||||
bail!("Macro '{name}' is invalid: {reason}")
|
||||
}
|
||||
_ => unknown_command()?,
|
||||
}
|
||||
}
|
||||
},
|
||||
None => {
|
||||
if let Some(cmd) = try_extract_shell_command(line) {
|
||||
@@ -1522,6 +1575,109 @@ fn unknown_command() -> Result<()> {
|
||||
bail!(r#"Unknown command. Type ".help" for additional help."#);
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq)]
|
||||
enum ReplInstallDispatch<'a> {
|
||||
Builtins(AssetCategory),
|
||||
Unified(&'a str),
|
||||
Help,
|
||||
Usage,
|
||||
}
|
||||
|
||||
fn parse_repl_install(args: Option<&str>) -> ReplInstallDispatch<'_> {
|
||||
let trimmed = args.map(str::trim).unwrap_or("");
|
||||
if trimmed
|
||||
.split_whitespace()
|
||||
.any(|token| token == "--help" || token == "-h")
|
||||
{
|
||||
return ReplInstallDispatch::Help;
|
||||
}
|
||||
let mut parts = trimmed.splitn(2, char::is_whitespace);
|
||||
match parts.next() {
|
||||
Some(name) if !name.is_empty() => {
|
||||
let rest = parts.next().map(str::trim).unwrap_or("");
|
||||
match AssetCategory::parse(name) {
|
||||
Some(category) if rest.is_empty() => ReplInstallDispatch::Builtins(category),
|
||||
Some(_) => ReplInstallDispatch::Usage,
|
||||
None => ReplInstallDispatch::Unified(trimmed),
|
||||
}
|
||||
}
|
||||
_ => ReplInstallDispatch::Usage,
|
||||
}
|
||||
}
|
||||
|
||||
fn repl_install_help() -> String {
|
||||
format!(
|
||||
r#"Install built-in assets, install a bundle from a Git source, or update an installed bundle.
|
||||
|
||||
Usage:
|
||||
.install <category> Reinstall built-in assets ({categories})
|
||||
.install <owner/repo>[#ref] Install a bundle from {default_host} (change the host with --git-host)
|
||||
.install <git-url>[#ref] Install a bundle from any Git URL, scp-style path, or local path
|
||||
.install <installed-bundle>[#ref] Update an installed bundle from its recorded source
|
||||
|
||||
Flags:
|
||||
--git-host <host> Host the <owner/repo> shorthand expands against (default {default_host})
|
||||
--filter <cat> Restrict a remote install to one category ({filters})
|
||||
--force Overwrite all conflicts without prompting (remote installs only)
|
||||
|
||||
Suffix #<ref> to pin a branch, tag, or commit. List installed bundles with
|
||||
`.list bundles`; remove one with `.uninstall <name>`."#,
|
||||
categories = AssetCategory::NAMES.join("|"),
|
||||
default_host = config::DEFAULT_GIT_HOST,
|
||||
filters = config::InstallFilter::NAMES.join("|"),
|
||||
)
|
||||
}
|
||||
|
||||
fn repl_uninstall_help() -> String {
|
||||
r#"Remove an installed bundle: delete the files it owns and the mcp.json entries it added.
|
||||
|
||||
Usage:
|
||||
.uninstall <bundle-name> [--yes]
|
||||
|
||||
Flags:
|
||||
--yes, -y Skip the confirmation prompt
|
||||
|
||||
Files you modified after install are prompted for individually and kept by
|
||||
default; --yes never deletes modified files. List installed bundles with
|
||||
`.list bundles`."#
|
||||
.to_string()
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq)]
|
||||
enum ReplUninstallDispatch {
|
||||
Run(String, bool),
|
||||
Help,
|
||||
Usage,
|
||||
}
|
||||
|
||||
fn parse_repl_uninstall(args: Option<&str>) -> ReplUninstallDispatch {
|
||||
let mut assume_yes = false;
|
||||
let mut names = Vec::new();
|
||||
for token in args.unwrap_or("").split_whitespace() {
|
||||
match token {
|
||||
"--help" | "-h" => return ReplUninstallDispatch::Help,
|
||||
"--yes" | "-y" => assume_yes = true,
|
||||
other if other.starts_with('-') => return ReplUninstallDispatch::Usage,
|
||||
other => names.push(other),
|
||||
}
|
||||
}
|
||||
match names.as_slice() {
|
||||
[name] => ReplUninstallDispatch::Run(name.to_string(), assume_yes),
|
||||
_ => ReplUninstallDispatch::Usage,
|
||||
}
|
||||
}
|
||||
|
||||
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 +1688,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 <name> runs top-level as .<name> [args...], equivalent to ".macro <name>".
|
||||
List them with ".list macros"; toggle them with ".macro enable|disable <name>".
|
||||
|
||||
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."###,
|
||||
@@ -1728,8 +1888,126 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repl_commands_has_60_entries() {
|
||||
assert_eq!(REPL_COMMANDS.len(), 60);
|
||||
fn repl_commands_has_61_entries() {
|
||||
assert_eq!(REPL_COMMANDS.len(), 61);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_repl_install_routes_categories_to_builtins() {
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("agents")),
|
||||
ReplInstallDispatch::Builtins(AssetCategory::Agents)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_repl_install_routes_other_values_to_unified_dispatch() {
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("https://github.com/x/y")),
|
||||
ReplInstallDispatch::Unified("https://github.com/x/y")
|
||||
);
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("my-bundle")),
|
||||
ReplInstallDispatch::Unified("my-bundle")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_repl_install_empty_args_ask_for_usage() {
|
||||
assert_eq!(parse_repl_install(None), ReplInstallDispatch::Usage);
|
||||
assert_eq!(parse_repl_install(Some(" ")), ReplInstallDispatch::Usage);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_repl_install_rejects_extra_tokens_after_a_category() {
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("agents --force")),
|
||||
ReplInstallDispatch::Usage
|
||||
);
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("agents extra")),
|
||||
ReplInstallDispatch::Usage
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_repl_install_routes_help_from_any_position() {
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("--help")),
|
||||
ReplInstallDispatch::Help
|
||||
);
|
||||
assert_eq!(parse_repl_install(Some("-h")), ReplInstallDispatch::Help);
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("agents --help")),
|
||||
ReplInstallDispatch::Help
|
||||
);
|
||||
assert_eq!(
|
||||
parse_repl_install(Some("owner/repo --help")),
|
||||
ReplInstallDispatch::Help
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_repl_uninstall_routes_run_help_and_usage() {
|
||||
assert_eq!(
|
||||
parse_repl_uninstall(Some("my-bundle --yes")),
|
||||
ReplUninstallDispatch::Run("my-bundle".to_string(), true)
|
||||
);
|
||||
assert_eq!(
|
||||
parse_repl_uninstall(Some("my-bundle")),
|
||||
ReplUninstallDispatch::Run("my-bundle".to_string(), false)
|
||||
);
|
||||
assert_eq!(
|
||||
parse_repl_uninstall(Some("--help")),
|
||||
ReplUninstallDispatch::Help
|
||||
);
|
||||
assert_eq!(
|
||||
parse_repl_uninstall(Some("my-bundle -h")),
|
||||
ReplUninstallDispatch::Help
|
||||
);
|
||||
assert_eq!(
|
||||
parse_repl_uninstall(Some("--force my-bundle")),
|
||||
ReplUninstallDispatch::Usage
|
||||
);
|
||||
assert_eq!(parse_repl_uninstall(None), ReplUninstallDispatch::Usage);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repl_install_and_uninstall_help_text_cover_the_full_surface() {
|
||||
let install = repl_install_help();
|
||||
for needle in [
|
||||
"--git-host",
|
||||
"--filter",
|
||||
"--force",
|
||||
"#ref",
|
||||
"owner/repo",
|
||||
".list bundles",
|
||||
] {
|
||||
assert!(install.contains(needle), "install help missing {needle}");
|
||||
}
|
||||
let uninstall = repl_uninstall_help();
|
||||
for needle in ["--yes", ".list bundles", "<bundle-name>"] {
|
||||
assert!(
|
||||
uninstall.contains(needle),
|
||||
"uninstall help missing {needle}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[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]
|
||||
|
||||
Reference in New Issue
Block a user