Implements the invocation and management surfaces from
plans/custom-commands-design.md §4 and §6:
- Top-level dispatch: an enabled macro <name> now runs as ".<name> [args]"
from the command catch-all; runtime-disabled macros point at
".macro enable <name>", locked macros name the owning config, and
unknown commands keep the existing error verbatim
- .macro enable|disable <name>: runtime toggles over the in-memory
global-level enabled_macros list (disable with no list materializes
all-active-minus-name); toggles error when a role/agent/session
allowlist owns the field
- .set enabled_macros <csv|null> with workspace-then-global existence
validation; .set key completion gains enabled_macros and the
previously missing enabled_skills
- Dynamic completion: enabled macros (with descriptions) join built-ins
on ".<TAB>" without touching the static command registry;
".macro <TAB>" lists invocable macros (incl. built-in-shadowed ones)
plus the enable/disable subcommands; second-arg completion offers
toggle-eligible names
- .list macros: enriched table (name, source, isolated, state,
description) covering every resolver state incl. missing and
shadowed rows; .help gains a custom-commands section
- Session info/render and sysinfo display enabled_macros; Macro::load
resolves workspace-then-global; enable/disable rejected as macro
names in the creator
Adds src/config/macro_policy.rs: MacroPolicy::effective computes the
visible macro set on demand from the discovered definition files, the
four-level enabled_macros allowlists, and the built-in command names.
- Discovery scans workspace (.coyote/macros/) then global macros dirs on
every resolution; workspace shadows global by name, and the shadowed
global entry is retained and flagged so both stay listable (plan
custom-commands-design.md §5). Workspace scanning is gated on a bool
parameter so the future --no-workspace-macros flag wires in one line.
- Allowlist precedence is session > agent > role > global, first Some
wins, no merging; None falls through, an empty list is an explicit
zero, all-None enables everything (mirrors SkillPolicy).
- Per-macro states per plan §6: enabled, disabled (runtime, global-level
exclusions only), locked (role/agent/session exclusions, recording the
owning level), missing (unknown allowlist names warn instead of
bailing — deliberate divergence from skills), shadowed (built-in name
collisions), and invalid (parse failures and the reserved names
enable/disable). Invalid beats allowlist exclusion beats shadowing.
- Adds enabled_macros() accessors on Role, Session, and Agent alongside
their enabled_skills() counterparts, plus paths::workspace_macros_dir.
- 37 tests: state matrix, pairwise precedence, explicit-zero pinned at
every level, workspace shadowing, reserved names, builtin collisions,
missing rows, invalid YAML, and env-gated discovery (#[serial]).
Mirrors the enabled_skills plumbing per plans/custom-commands-design.md §5:
- global: Config + AppConfig structs, from_config copy, and the
COYOTE_ENABLED_MACROS env-override arm (csv_to_vec parsing)
- role: frontmatter via parse_string_or_array (list or csv string),
plus the export() mirror so Role::save round-trips the field
- agent (non-graph): plain serde on AgentConfig; graph.yaml silently
ignores the key (pinned by test, no field on Graph by design)
- session: plain serde with csv-or-vec deserializer
Empty list/string deserializes to Some([]) (explicit zero), distinct
from absent/null (None) — pinned by tests at every level, including
the env arm (serial-fenced against the from_config tests, which read
the process env via load_envs).
description (optional, default None) will surface in listings and
completion; isolated (default true) preserves today's forked-context
execution behavior exactly. Both fields use plain serde defaults so
every existing macro YAML deserializes unchanged, and unknown fields
in newer files remain tolerated by older binaries.
Adds Serialize to Macro/MacroVariable (None description skipped) and
back-compat, round-trip, and embedded-asset deserialization tests.
Per plans/custom-commands-design.md §3 / §9 step 1.
The Client trait's default chat_completions, chat_completions_streaming,
and embeddings methods now classify failures via ApiStatusError: on a
401 with a cached OAuth token, the token is distrusted (identity-aware
marker) and the call retried exactly once — the retry's prepare step
sees the marker and force-refreshes. Streaming retries only while the
SSE handler has received no content, preventing duplicate rendering.
A second 401 propagates the original error; other retry errors
propagate as-is. API-key clients never retry. No backoff by design:
cost is bounded to one refresh + one retry per failing request.
distrust_access_token compare-and-invalidates the in-memory entry only
when the cached token equals the rejected one, so a concurrent refresh
is never clobbered. is_valid_access_token and both expiry checks in
prepare_oauth_access_token treat marked tokens as expired, forcing a
refresh of provider-rejected tokens that are still locally unexpired.
The marker is cleared after every completed refresh, including ones
that return the same token.
catch_error and sse_stream now bail with ApiStatusError{status, message}
instead of bare anyhow strings, preserving every existing Display output
byte-for-byte. Enables structural status classification (e.g. 401
detection) via downcast through anyhow context chains.
Replace the spawn-time static Authorization header for OAuth-managed HTTP
MCP servers with McpOAuthClient, a custom implementation of rmcp's
StreamableHttpClient trait that resolves the bearer token on every
request via load_or_refresh_mcp_token. Tokens that expire mid-session
now refresh transparently instead of failing tool calls until restart.
On a 401 for an injected token, the wrapper force-refreshes (identity-
aware: a still-unexpired copy of the rejected token is not trusted) and
retries exactly once, matching Claude Code / official SDK semantics.
Both *_with_max_sse_event_size trait methods are overridden to preserve
the inner client's SSE size enforcement, and the inner reqwest client
mirrors rmcp's default (pool_max_idle_per_host(0), no redirects).
SSE, stdio, and static-header HTTP paths are unchanged; startup
warning semantics (McpAuthRequired reasons) are preserved. Verified
live: mid-session backdated token refreshed transparently during an
active atlassian session.
Compile-clean upgrade verified: zero source changes needed, full test
suite green, clippy clean. Coyote's rmcp API surface (14 items) dodges
all 2.0/3.0 breaking changes; the "Auth required" error string matched
by is_auth_required_error is intact in 3.1.2.
Distinguish why an OAuth MCP server was not started: never authenticated
(no stored credentials), stored token expired and refresh failed, or the
server rejected a token that looked valid. McpTokenStatus replaces the
Option<String> return of load_or_refresh_mcp_token, and McpAuthRequired
carries the reason across the error boundary via anyhow context.
Agent startup and graph rag nodes both run an interactive wizard when their
knowledge base has not been built, but neither offered the driver choice that
interactive named-RAG creation has, so both silently produced a yaml store.
A plain agent was the worse of the two: AgentConfig carries only documents, so
there was no way to get a duckdb RAG for one, interactively or declaratively. A
graph node could at least declare driver: in the workflow.
Agent startup now passes prompt_for_driver, and a rag node whose wizard runs is
asked too. The prompt is skipped when the node already declares a driver, and
sits inside the not-fully-specified branch after the non-interactive bail, so
declarative workflows and headless runs are unchanged. Temp RAGs still pass
false: they are deleted on the next run, so a persistent store would only leave
a sidecar behind.
The prompt moves to select_rag_driver rather than being duplicated.
A duckdb RAG is two files. The .yaml deliberately carries no vectors, and
open() runs CREATE TABLE IF NOT EXISTS, so a .yaml copied or synced without its
.duckdb sidecar produces a fresh empty store, hydrates to nothing, and answers
every query with nothing while .info rag still lists every indexed file.
Neither existing guard catches it: the anti-wipe check in rebuild_indexes needs
existing > 0, and the mandatory ? on hydration needs a genuine error, while an
absent store is the same Ok(empty) as a RAG with nothing indexed yet.
Warn rather than bail, so a store deleted on purpose still loads and can be
rebuilt.
Reverts the global proxy rework in 54685be and narrows it to the provider.
That commit took over proxy detection for every client in order to exempt
loopback and private ranges. Too broad: reqwest's detection also reads macOS
System Settings and the Windows registry behind its system-proxy feature, which
sits in its default set. Coyote disables default features today, so hand-rolling
the environment lookup happened to match — but re-enabling defaults later would
silently restore that support for main and not for the hand-rolled version. It
also made an explicitly configured proxy skip local hosts, which nobody asked
for: a proxy named for a LAN endpoint should be used.
build_client and utils are byte-identical to main again. The bypass now lives in
QdrantProvider::make_client, which is the only place that knows the target host,
and applies solely when that host is loopback, link-local, private or .local. A
public or cloud-hosted store keeps whatever the environment configures.
Also drops apply_proxy: with build_client reverted there was one caller left, and
set_proxy already covers it.
Both #[ignore]d live tests still pass against a Qdrant on loopback while an
ambient proxy that rejects it is in force.
Pre-existing on main, not introduced by the driver work, but it makes a local
RAG backend unusable so it belongs with this change.
build_client only called set_proxy when a client had configured one of its own.
With nothing configured, reqwest's own detection applied, which sends every
request through a *_PROXY variable including ones bound for 127.0.0.1 or a LAN
address. A proxy cannot usefully forward those, and anything that intercepts
proxied traffic answers on behalf of a service that is running perfectly well,
so the error names the proxy rather than the store and reads as a Coyote fault.
Concretely, an installed Socket Firewall exports HTTP_PROXY to the processes it
wraps and rejects hosts outside its allow list. That turned a healthy Ollama on
the LAN into 'error decoding response body: expected value at line 2 column 1' —
its HTML refusal page parsed as JSON — and a loopback Qdrant into an HTTP 405.
Proxy handling is now always applied and always exempts loopback and private
ranges, with NO_PROXY merged in since replacing reqwest's detection also
replaces its handling of that variable. HTTP_PROXY and HTTPS_PROXY are kept
separate because they are allowed to differ. An explicitly configured proxy
still wins, and '-' still means none.
This also supersedes the unconditional no_proxy() added to the Qdrant client in
af9622d: that made it the only client to ignore a proxy outright, on a
justification I got wrong. It now shares this path, so a remote store behind a
real proxy keeps working.
make_client used a bare reqwest builder, which honours whatever proxy the
environment advertises. That made it the only HTTP client in Coyote to do so:
utils::set_proxy discards ambient settings and applies only Coyote's configured
proxy, and every other client goes through it.
The symptom is that a perfectly healthy Qdrant is unreachable and the error
belongs to the interposing proxy, not the store, so it reads as a Coyote or
Qdrant fault. Locally an installed Socket Firewall answered `.rag attach`
against 127.0.0.1:6333 with an HTML 'Connection Required' page and HTTP 405.
Both #[ignore]d live tests now pass against a real Qdrant; they failed with that
same 405 before this change, which is the first time either has run green.
A remote store that genuinely needs Coyote's configured proxy is a follow-up:
that means threading the proxy config into the provider.
Sandbox provisioning only ever looked at driver_config["api_key"], so a driver
whose credential is called anything else would have been silently unprovisioned
inside a sandbox. It now scans every driver_config value and treats any that is
a secret placeholder as a credential, which is the same rule resolve_driver_config
already used at point of use.
The first one binds to the RAG's own service id, which is what the generated
mixin declares; any others register under their own names, as MCP secrets do.
The mixin still carries a single credential entry, so a driver needing two bound
secrets remains a follow-up.
Also drops the placeholder parser added in 74bc613. crate::vault::SECRET_RE is
already the canonical definition and was already imported here, so that was a
third implementation of the same grammar. Requiring the whole value to match is
what keeps a literal key from being read as a secret name and printed.
The api_key check is gone from RagData::validate: a generic config validator
should not know a provider's field names.
parse_search_hits filtered on score > min_score, and the only caller passes
0.0. Qdrant Euclid collections score by negative distance, so every hit was
dropped and an attached Euclid collection returned nothing at all, silently.
This is the same trap the surrounding code already documents: score_threshold
is deliberately not sent because it is metric-aware and a 0.0 floor filters
everything out on Euclid. The local filter then reproduced it exactly. Only a
positive floor is now treated as a floor.
Five review comments, all real:
- hybrid_search ran its vector and keyword legs sequentially after the
provider refactor; main ran them under tokio::join!. Restores the
concurrency while keeping the degrade-on-error keyword behaviour, so a
remote provider no longer pays two serial round trips per query.
- inject_rag_secrets derived a vault secret name by trimming braces, which
leaves a literal key untouched. A RAG holding a plaintext api_key therefore
looked the secret up by its own value and printed it to stderr on failure.
Parsing is now strict and a non-placeholder is skipped with a warning that
names no credential.
- validate() now refuses a driver_config.api_key that is not a {{NAME}}
placeholder, so a plaintext key cannot reach the RAG YAML at all.
- Rag::create's catch-all arm treated any unrecognised driver as yaml. A typo
built a yaml store, paid to embed the corpus, persisted the bad driver and
only failed on the next run. Unknown drivers now fail immediately.
- The qdrant arm's error was written for a developer; it now tells the user
that only attached collections are readable and points at .rag attach.
Dark-Alex-17
Alex Clarke