10 Commits
Author SHA1 Message Date
ZacharyZhang-NY 9edb8729ef feat(swarm): agent_swarm — one prompt over many items, paced as a fleet
Ports kimi-code's AgentSwarm: a `prompt_template` containing `{{item}}`
expanded over an `items` list into up to 128 subagents, run to completion
and returned as one aggregate. Kigi already exceeds upstream on planning,
verification, isolation and merge via /graph; what it lacked was cheap
immediate fan-out. Entirely client-side — no new backend surface.

The engine is three pure pieces plus a runner: `plan` validates and
expands (every fault reported before a single member starts — a
half-launched swarm is expensive to unwind), `schedule` is the launch
ramp as testable arithmetic, `run` drives it against the existing
`SubagentBackend`. Reuses the single-spawn coordinator rather than
inventing a batch API.

Load-bearing decisions, each the result of a defect found in review:

- `backgrounded` is its own outcome. A member that outlives the 600s
  foreground budget is detached by the coordinator and KEEPS RUNNING;
  reporting it as failed invites the model to relaunch its item, putting
  a second agent on the same files. It is never offered for resume.
- `InFlightGuard` cancels live members on Drop. Send-now cancels the turn
  WITHOUT cancelling subagents and aborts the task; the dropped receivers
  read as "parent gone" and each child re-attaches itself. There is no
  cooperative path to use instead — `Cancellation` is constructed nowhere
  in the tree — so Drop is the only seam that fires.
- Retries and wall clock are both bounded. The swarm blocks the caller's
  turn, so every wait needs a ceiling it cannot argue past; stragglers at
  the deadline are reported as still-running, with their ids.
- `ToolKind::AgentSwarm` is its own variant: `TemplateRenderer`'s
  `by_kind` map holds one tool name per kind, so sharing `Task` would
  silently redirect `${{ tools.by_kind.task }}` in other tools' prompts.
- An explicitly requested model that cannot be validated is refused, as
  the task tool already does — one loud error beats `items.len()` quiet
  ones. Depth stays capped at 1: upstream's unlimited nesting is a
  hazard, not a feature.
- `SubagentResult.rate_limited` is classified where the typed ACP error
  code is still in hand; a scheduler re-deriving it from a formatted
  string would stop adapting the day the wording changed.
- Aggregate output is clamped per member (head+tail, loss stated):
  native tool output is truncated nowhere downstream.

42 agent_swarm tests. The fake backend awaits, so the concurrency and
ordering assertions can actually fail; the cap test also proves the
fixture can exceed the cap.
2026-07-27 01:22:52 -04:00
ZacharyZhang-NY ed8049cf77 fix(memory): scope embedding credentials to the endpoint that may receive them
`MemoryBackendParams` carried the primary session `AuthManager` and the
session api-key provider unconditionally, while `embed_base_url` is the
CURRENT MODEL's endpoint and `AuthRetryMiddleware` stamps `Authorization`
on every request it wraps. A user who enables `[memory.embedding] model`
while running a BYOK or subscription-OAuth model therefore sent the Kimi
session bearer to that third party.

`EndpointScopedCredentials` binds the credential to the one endpoint it
may reach: `for_endpoint` drops the handle unless the caller vouches for
the URL, and `approved_for` re-checks at provider-build time in release
too, because `MemoryBackendParams` is `Clone` and callers rewrite fields
on the copy.

The shell decides through `CredentialAuthority::manager_for` rather than
a second URL predicate — it answers both whether a credential may ride
and which manager governs it, so a subscription-OAuth platform gets its
own pooled manager. The session's `SharedApiKeyProvider` is not
forwarded at all: it is hard-wired to the primary manager, so at a
pooled platform's host it would resolve the wrong bearer. A platform's
own `embed_api_key` is untouched and keeps serving its own endpoint.

The background reindex built a second provider straight from
`ApiEmbeddingProvider::from_session`, outside the chokepoint and without
401 refresh; it now embeds through the session's own params.

Test strength verified by mutation: with the guard reverted, exactly
`session_credentials_are_withheld_from_a_foreign_endpoint` and
`a_cloned_param_set_cannot_redirect_scoped_credentials` fail.
2026-07-26 23:41:45 -04:00
ZacharyZhang-NY 867b3e110b docs(comments): trim paste-fix commentary to the crucial constraints 2026-07-24 13:09:35 -04:00
ZacharyZhang-NY 7380576e50 release: v0.1.9
Release / build (aarch64-apple-darwin) (push) Waiting to run
Release / build (x86_64-apple-darwin) (push) Waiting to run
Release / build (aarch64-unknown-linux-gnu) (push) Waiting to run
Release / build (x86_64-pc-windows-msvc) (push) Waiting to run
Release / publish GitHub Release (push) Blocked by required conditions
Release / build (x86_64-unknown-linux-gnu) (push) Failing after 7s
2026-07-24 12:46:25 -04:00
ZacharyZhang-NY 7ade135e76 fix(tui): reassemble Windows key-burst pastes so repaste-to-expand works
On Windows, crossterm's console backend never emits Event::Paste: a
terminal paste arrives as a per-character key burst that the event loop
reassembles with timing heuristics. Three of its bounds split real
pastes into fragments:

- the 10 ms continue window is below one Windows scheduler quantum
  (~15.6 ms), so a routine mid-burst gap ended collection early;
- the 5000-event safety cap truncated any paste past ~5 KB;
- the 2 ms detection window missed bursts whose second chunk lagged,
  so collection never started.

Each fragment became its own synthetic paste. The paste chip then held
only the first fragment, and pasting again could never byte-equal the
chip content, so repaste-to-expand inserted a duplicate instead of
expanding — while macOS worked, because the unix parser buffers a
bracketed paste to the end marker and always delivers one whole event.

Widen the continue window past one quantum (25 ms), raise the cap above
any real paste (200k events), and skip the detection window entirely
when the batch already holds a >=8-key burst, which typing and
auto-repeat never produce. Unix paths are untouched: batches containing
a real Event::Paste never enter this reassembly.

Covered by a paused-clock test that trickles a burst tail with
one-quantum gaps and asserts it reassembles into a single paste.
2026-07-24 12:41:26 -04:00
ZacharyZhang-NY f5b9000c5d fix(cgroup): stop swallowing inotify read errors in the memory monitor
wait_and_drain treated every read() <= 0 as "queue drained", so a real
error (bad fd, EINVAL) silently ended the drain and the monitor kept
polling a broken fd. Distinguish the cases: EAGAIN ends the drain, EINTR
retries, and any other error propagates. monitor_loop logs the error
before exiting so a dead memory monitor is visible in the session log.
2026-07-24 11:30:20 -04:00
ZacharyZhang-NY a02b555e66 docs(comments): rewrite comments across all crates to the guidelines
Sweep every first-party crate source (1956 .rs files) to the project comment
guidelines: delete redundant restatements, decorative banners, change
narration, and end-of-line comments; keep and tighten the crucial ones
(invariants, bug rationale, SAFETY blocks, ported-source attribution).

No functional code changed. Every edit is proven comment-only against the
prior tree by a comment-stripping lexer (string/char/raw-string aware) plus a
separate doctest-fence check. Where removing a comment made rustfmt or clippy
want to re-lay-out adjacent code, the minimal triggering comment is restored so
code tokens stay byte-identical.

Gates green: cargo fmt --all --check (0 diffs), cargo check and cargo clippy
--workspace --all-targets (0 warnings).

Adds scripts/check_codegen_comment_guidelines.py — the enforcement gate for
these guidelines (flags banners, end-of-line comments, change narration, and
commented-out code).
2026-07-23 16:55:39 -04:00
ZacharyZhang-NY ff0fb56c67 release: v0.1.8
Release / build (aarch64-apple-darwin) (push) Waiting to run
Release / build (x86_64-apple-darwin) (push) Waiting to run
Release / build (aarch64-unknown-linux-gnu) (push) Waiting to run
Release / build (x86_64-pc-windows-msvc) (push) Waiting to run
Release / publish GitHub Release (push) Blocked by required conditions
Release / build (x86_64-unknown-linux-gnu) (push) Failing after 8s
Since v0.1.7 — chat-completions dialect correctness (Pi-referenced):
- fix(chat): BYOK/custom endpoints default to Passthrough (vanilla
  OpenAI semantics; Kimi body mutations no longer leak to third-party
  servers); house/Kimi coding endpoints keep the Kimi dialect via
  base-url detection
- fix(chat): dedicated Mistral dialect — exactly-nine-alphanumeric
  tool-call id normalizer (deterministic, call/result-symmetric)
- fix(chat): tool-result images relocate to a batched user message
  (tool messages are text-only on the OpenAI chat wire)
- docs: AGENTS.md records the dialect-selection contract
2026-07-23 10:39:18 -04:00
ZacharyZhang-NY 2524c33a5b fix(chat): relocate tool-result images to a batched user message
C3 (Pi openai-completions relocation): tool messages are text-only on
the OpenAI chat wire — image parts inside role:tool content 400 on
strict validators. Images from a consecutive tool-result run now batch
into ONE synthetic user message appended after the run (a user message
may not interrupt tool responses answering the same assistant's
tool_calls); an image-only result carries a '(see attached image)'
pointer placeholder. Applied at the typed conversion layer, so every
chat dialect gets the universally-valid shape.

Verified: models+sampling-types+sampler+chat-state+shell all green,
clippy clean.
2026-07-23 10:38:37 -04:00
ZacharyZhang-NY 301eb61de4 fix(chat): BYOK dialect defaults to Passthrough; dedicated Mistral dialect
C1 (decision): custom/BYOK ChatCompletions entries defaulted to the Kimi
dialect, leaking Kimi-specific body mutations (thinking:{…} control,
replayed reasoning_content, schema rewrites) to arbitrary third-party
OpenAI-compatible servers. The default is now Passthrough (vanilla
OpenAI semantics), with ONE exception mirroring Pi's base-url quirk
sniffing: entries pointed at the house/Kimi coding endpoint keep the
Kimi dialect. Registry platforms are unaffected (all declare
explicitly).

C2 (Pi mistral-conversations normalizer): Mistral requires tool-call ids
of EXACTLY nine [a-zA-Z0-9] chars; even same-session synthesized UUIDs
violate it. New ChatCompat::Mistral = StrictOpenAi behavior + the
normalizer — strip non-alphanumerics, keep exact-9 ids, else FNV-1a →
base36 (build-stable, deterministic across requests for prefix-cache
stability) with collision retry; ONE map covers tool_calls[].id and
tool_call_id so pairing survives. The mistral registry row and the
persisted 'mistral' serde value both resolve to it (pre-rename Mistral
sessions gain the contract automatically); Cerebras/NVIDIA stay on
StrictOpenAi untouched.

Verified: models+sampling-types+sampler+chat-state+shell all green,
clippy clean.
2026-07-23 10:31:45 -04:00
1475 changed files with 13804 additions and 21995 deletions
+11
View File
@@ -248,6 +248,17 @@ edges stay deterministic Rust. The harness appends a terminal
across all three builders in the same pass — three sequential
single-provider fixes (thinking signature → codex system role → codex
reasoning id) motivated this policy.
- ChatCompletions dialect selection: registry platforms declare
`chat_compat` explicitly; BYOK/custom entries default to `Passthrough`
(vanilla OpenAI semantics) EXCEPT entries pointed at the house/Kimi
coding endpoint, which keep the `Kimi` dialect (base-url detection —
Pi-style quirk sniffing). `ChatCompat::Mistral` = StrictOpenAi plus the
exactly-nine-`[a-zA-Z0-9]` tool-call id normalizer
(`normalize_mistral_tool_call_ids`, deterministic FNV-1a→base36, one
map for call+result; persisted `mistral` values resolve here). Chat
tool messages are TEXT-ONLY: tool-result images batch into one
synthetic user message after the consecutive tool-result run
(`conversation_to_chat_messages`).
- `openai-codex` (ChatGPT Plus/Pro, `scope_key oauth/openai-codex`, port
1455 `/auth/callback`, FORM body, authorize+token host `auth.openai.com`,
client `app_EMoam…`, scope `openid profile email offline_access`, the 3
Generated
+62 -62
View File
@@ -5442,7 +5442,7 @@ dependencies = [
[[package]]
name = "kigi-acp-lib"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"async-trait",
@@ -5456,7 +5456,7 @@ dependencies = [
[[package]]
name = "kigi-agent"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"chrono",
"dirs 6.0.0",
@@ -5486,7 +5486,7 @@ dependencies = [
[[package]]
name = "kigi-agent-lifecycle"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"async-trait",
"tokio",
@@ -5495,7 +5495,7 @@ dependencies = [
[[package]]
name = "kigi-auth"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"async-trait",
"http 1.4.2",
@@ -5508,7 +5508,7 @@ dependencies = [
[[package]]
name = "kigi-bin"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"clap",
@@ -5543,7 +5543,7 @@ dependencies = [
[[package]]
name = "kigi-chat-state"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"indexmap",
"kigi-compaction",
@@ -5560,7 +5560,7 @@ dependencies = [
[[package]]
name = "kigi-codebase-graph"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"ahash",
"clap",
@@ -5596,7 +5596,7 @@ dependencies = [
[[package]]
name = "kigi-compaction"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"async-trait",
@@ -5609,7 +5609,7 @@ dependencies = [
[[package]]
name = "kigi-config"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"base64",
"blake3",
@@ -5632,7 +5632,7 @@ dependencies = [
[[package]]
name = "kigi-config-types"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"indexmap",
@@ -5646,7 +5646,7 @@ dependencies = [
[[package]]
name = "kigi-crash-handler"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"backtrace",
"libc",
@@ -5657,7 +5657,7 @@ dependencies = [
[[package]]
name = "kigi-env"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"tracing",
"url",
@@ -5665,7 +5665,7 @@ dependencies = [
[[package]]
name = "kigi-fast-worktree"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"bytes",
@@ -5697,7 +5697,7 @@ dependencies = [
[[package]]
name = "kigi-file-utils"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"aws-config",
@@ -5721,7 +5721,7 @@ dependencies = [
[[package]]
name = "kigi-fsnotify"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"criterion",
"dunce",
@@ -5742,7 +5742,7 @@ dependencies = [
[[package]]
name = "kigi-gix-status"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"gix",
"kigi-test-utils",
@@ -5752,7 +5752,7 @@ dependencies = [
[[package]]
name = "kigi-hooks"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"fastrand",
"kigi-config",
@@ -5771,7 +5771,7 @@ dependencies = [
[[package]]
name = "kigi-hooks-plugins-types"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"serde",
"serde_json",
@@ -5779,7 +5779,7 @@ dependencies = [
[[package]]
name = "kigi-http"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"kigi-auth",
"kigi-log",
@@ -5794,7 +5794,7 @@ dependencies = [
[[package]]
name = "kigi-hunk-tracker"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"chrono",
"dunce",
@@ -5815,14 +5815,14 @@ dependencies = [
[[package]]
name = "kigi-interjection-core"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"serde",
]
[[package]]
name = "kigi-log"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"chrono",
@@ -5840,7 +5840,7 @@ dependencies = [
[[package]]
name = "kigi-markdown"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anstyle",
"anstyle-lossy",
@@ -5864,14 +5864,14 @@ dependencies = [
[[package]]
name = "kigi-markdown-core"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"pulldown-cmark",
]
[[package]]
name = "kigi-mcp"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"async-trait",
@@ -5908,7 +5908,7 @@ dependencies = [
[[package]]
name = "kigi-memory"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"arc-swap",
@@ -5942,7 +5942,7 @@ dependencies = [
[[package]]
name = "kigi-mermaid"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"fontdb",
"image",
@@ -5960,7 +5960,7 @@ dependencies = [
[[package]]
name = "kigi-models"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"kigi-env",
"serde",
@@ -5970,7 +5970,7 @@ dependencies = [
[[package]]
name = "kigi-pager-minimal"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"chrono",
"crossterm",
@@ -5987,7 +5987,7 @@ dependencies = [
[[package]]
name = "kigi-pager-pty-harness"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"alacritty_terminal",
"anyhow",
@@ -6012,7 +6012,7 @@ dependencies = [
[[package]]
name = "kigi-pager-render"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"anstyle",
@@ -6064,7 +6064,7 @@ dependencies = [
[[package]]
name = "kigi-paths"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"camino",
"serde",
@@ -6074,7 +6074,7 @@ dependencies = [
[[package]]
name = "kigi-prompt-queue"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"serde",
"serde_json",
@@ -6082,7 +6082,7 @@ dependencies = [
[[package]]
name = "kigi-proto-build"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"pbjson-build",
@@ -6093,7 +6093,7 @@ dependencies = [
[[package]]
name = "kigi-ratatui-inline"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"ansi-width",
"anstyle-parse 0.2.7",
@@ -6110,7 +6110,7 @@ dependencies = [
[[package]]
name = "kigi-ratatui-textarea"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"arboard",
"chrono",
@@ -6131,7 +6131,7 @@ dependencies = [
[[package]]
name = "kigi-sampler"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"async-openai",
"async-stream",
@@ -6154,7 +6154,7 @@ dependencies = [
[[package]]
name = "kigi-sampling-types"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"assert_matches",
"async-openai",
@@ -6171,7 +6171,7 @@ dependencies = [
[[package]]
name = "kigi-sandbox"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"chrono",
@@ -6192,7 +6192,7 @@ dependencies = [
[[package]]
name = "kigi-secrets"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"regex",
"serde_json",
@@ -6230,7 +6230,7 @@ dependencies = [
[[package]]
name = "kigi-shell"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"anyhow",
@@ -6367,7 +6367,7 @@ dependencies = [
[[package]]
name = "kigi-shell-base"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"chrono",
@@ -6392,7 +6392,7 @@ dependencies = [
[[package]]
name = "kigi-sqlite-journal"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"libc",
"rusqlite",
@@ -6403,7 +6403,7 @@ dependencies = [
[[package]]
name = "kigi-subagent-resolution"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"kigi-sampling-types",
"kigi-tool-types",
@@ -6418,7 +6418,7 @@ dependencies = [
[[package]]
name = "kigi-system-power"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"windows-sys 0.59.0",
"zbus",
@@ -6426,7 +6426,7 @@ dependencies = [
[[package]]
name = "kigi-test-support"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"anyhow",
@@ -6448,7 +6448,7 @@ dependencies = [
[[package]]
name = "kigi-test-utils"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"runfiles",
"tracing",
@@ -6457,11 +6457,11 @@ dependencies = [
[[package]]
name = "kigi-token-estimation"
version = "0.1.7"
version = "0.1.9"
[[package]]
name = "kigi-tool-protocol"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"kigi-tool-types",
"serde",
@@ -6472,7 +6472,7 @@ dependencies = [
[[package]]
name = "kigi-tool-runtime"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"async-trait",
@@ -6490,7 +6490,7 @@ dependencies = [
[[package]]
name = "kigi-tool-types"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"minijinja",
"schemars 1.2.1",
@@ -6500,7 +6500,7 @@ dependencies = [
[[package]]
name = "kigi-tools"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"arc-swap",
@@ -6577,7 +6577,7 @@ dependencies = [
[[package]]
name = "kigi-tools-api"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"kigi-proto-build",
"kigi-tool-protocol",
@@ -6590,11 +6590,11 @@ dependencies = [
[[package]]
name = "kigi-tracing-macros"
version = "0.1.7"
version = "0.1.9"
[[package]]
name = "kigi-tty-utils"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"libc",
"nix 0.30.1",
@@ -6604,7 +6604,7 @@ dependencies = [
[[package]]
name = "kigi-tui"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"ansi-to-tui",
@@ -6691,7 +6691,7 @@ dependencies = [
[[package]]
name = "kigi-update"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"dunce",
@@ -6720,14 +6720,14 @@ dependencies = [
[[package]]
name = "kigi-version"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"semver",
]
[[package]]
name = "kigi-workspace"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"agent-client-protocol",
"anyhow",
@@ -6806,7 +6806,7 @@ dependencies = [
[[package]]
name = "kigi-workspace-types"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"base64",
"chrono",
@@ -8840,7 +8840,7 @@ dependencies = [
[[package]]
name = "ptyctl"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"alacritty_terminal",
"anyhow",
@@ -8858,7 +8858,7 @@ dependencies = [
[[package]]
name = "ptyctl-cli"
version = "0.1.7"
version = "0.1.9"
dependencies = [
"anyhow",
"axum",
+1 -1
View File
@@ -76,7 +76,7 @@ members = [
]
[workspace.package]
version = "0.1.7"
version = "0.1.9"
edition = "2024"
license = "Apache-2.0"
@@ -24,22 +24,14 @@ fn is_github_actions() -> bool {
env::var_os("GITHUB_ACTIONS").is_some()
}
/// Find `protoc` command.
/// Locate `protoc`.
///
/// Search order:
/// 1. `$PROTOC` environment variable (set by Bazel `build_script_env` or user override)
/// 2. `bin/protoc` walking up parent directories (dotslash wrapper for local dev)
/// 3. `protoc` on `$PATH` (system install or other tooling)
///
/// When `bin/protoc` exists but fails to execute (e.g. the dotslash wrapper running
/// in Bazel remote execution where `dotslash` is not installed), the error is not fatal —
/// we fall through to the PATH-based lookup instead.
///
/// Returns `Ok(None)` if not found and not in a strict environment (GitHub Actions).
/// Search order: `$PROTOC`, then `bin/protoc` walking parents (dotslash
/// wrapper), then `$PATH`. A non-executable `bin/protoc` (e.g. dotslash
/// missing under Bazel remote execution) is non-fatal — lookup continues
/// on `$PATH`. Returns `Ok(None)` when missing outside GitHub Actions.
pub fn find_protoc() -> anyhow::Result<Option<PathBuf>> {
// 1. Check the PROTOC env var first. This is the standard override used by prost-build
// and is set by Bazel cargo_build_script build_script_env to point at a hermetic
// protoc binary instead of the dotslash wrapper.
// `$PROTOC` is the prost-build override; Bazel sets it to a hermetic binary.
if let Ok(protoc_env) = env::var("PROTOC") {
let protoc = PathBuf::from(&protoc_env);
if protoc.try_exists()? {
@@ -48,20 +40,17 @@ pub fn find_protoc() -> anyhow::Result<Option<PathBuf>> {
}
}
// 2. Walk up directories looking for bin/protoc (dotslash wrapper).
let cwd = env::current_dir()?;
let mut dir = cwd.clone();
let mut dir_rel = PathBuf::new();
loop {
// Return relative path to make build more deterministic.
// Relative path keeps cargo rerun fingerprints stable across machines.
let protoc = dir_rel.join("bin/protoc");
if protoc.try_exists()? {
match check_protoc_good(&protoc) {
Ok(()) => return Ok(Some(protoc)),
Err(e) => {
// bin/protoc exists but can't execute — likely the dotslash wrapper
// in an environment without dotslash (e.g. Bazel remote execution).
// Fall through to PATH-based lookup below.
// Dotslash wrapper present but not runnable — try PATH next.
eprintln!(
"bin/protoc found at `{}` but failed to execute: {e:#}; \
trying protoc from PATH as fallback",
@@ -77,12 +66,10 @@ pub fn find_protoc() -> anyhow::Result<Option<PathBuf>> {
dir_rel.push("..");
}
// 3. Try protoc from PATH (system install or other tooling).
if check_protoc_good(Path::new("protoc")).is_ok() {
return Ok(Some(PathBuf::from("protoc")));
}
// 4. Not found anywhere.
if is_github_actions() {
return Err(anyhow::anyhow!(
"`protoc` not found (checked $PROTOC env, bin/protoc, and PATH)"
+18 -40
View File
@@ -5,22 +5,16 @@ use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::{env, fs, iter};
/// Find the protoc well-known types include directory.
/// Resolve protoc's well-known-types include dir (`../include` next to `bin/protoc`).
///
/// When PROTOC is set (e.g., in Bazel), the include directory is typically
/// at `../include` relative to the `bin/protoc` binary. For example:
/// - PROTOC = `/path/to/external/protoc_linux_x86_64/bin/protoc`
/// - Include = `/path/to/external/protoc_linux_x86_64/include`
///
/// This is needed because Bazel places the protoc binary and include files
/// in separate locations within the sandbox, and protoc doesn't automatically
/// find them without an explicit -I flag.
/// Bazel keeps the binary and includes in separate sandbox paths; protoc will
/// not find them without an explicit `-I`.
fn find_protoc_include_dir(protoc: Option<&Path>) -> Option<PathBuf> {
let protoc = protoc?;
// protoc is typically at .../bin/protoc, so include is at .../include
let parent = protoc.parent()?; // .../bin
let grandparent = parent.parent()?; // .../
// Layout: `.../bin/protoc` → sibling `.../include`.
let parent = protoc.parent()?;
let grandparent = parent.parent()?;
let include_dir = grandparent.join("include");
if include_dir.is_dir() {
@@ -72,10 +66,8 @@ impl XaiProtoBuilder {
self
}
/// Serialize JSON using the original proto field names (snake_case) instead
/// of the proto3-JSON default (camelCase). Deserialization still accepts
/// both casings, so this is backward-compatible with already-stored
/// camelCase documents.
/// Emit JSON with original proto field names (snake_case) instead of
/// proto3-JSON camelCase. Deserialization still accepts both casings.
pub fn pbjson_preserve_proto_field_names(mut self) -> Self {
self.pbjson_preserve_proto_field_names = true;
self
@@ -93,10 +85,8 @@ impl XaiProtoBuilder {
self.map_builder(|b| b.field_attribute(path, attr))
}
// tonic-build generation of `rerun-if-changed` is lazy and incorrect.
// - everything is invalidated when anything inside include directories is changed
// - also they compute paths incorrectly: assuming paths are relative to current directory
// rather than
// tonic-build's `rerun-if-changed` is lazy and wrong: any include-dir
// touch invalidates everything, and paths are treated as CWD-relative.
fn emit_rerun_if_changed<'a>(
protoc: Option<&Path>,
protoc_include_dir: Option<&Path>,
@@ -112,11 +102,9 @@ impl XaiProtoBuilder {
);
}
// Can only process one input file when using --dependency_out=FILE.
// Both protoc outputs go to real files: /dev/stdout and /dev/null do
// not exist on Windows (the release build failed on exactly this).
// OUT_DIR is always set for build scripts; deterministic names make
// reruns overwrite instead of accumulate.
// `--dependency_out` accepts one input per invocation. Write real
// files (not /dev/stdout|/dev/null — missing on Windows). OUT_DIR
// names stay stable so reruns overwrite rather than accumulate.
let scratch_dir = env::var_os("OUT_DIR")
.map(PathBuf::from)
.unwrap_or_else(env::temp_dir);
@@ -135,9 +123,7 @@ impl XaiProtoBuilder {
descriptor_file.display()
));
// Add protoc's well-known types include directory first (if found).
// This is needed for Bazel sandboxed builds where protoc and its
// include files are in different locations.
// Well-known types first so Bazel sandboxes resolve them.
if let Some(include_dir) = protoc_include_dir {
command.arg(format!(
"-I{}",
@@ -162,9 +148,8 @@ impl XaiProtoBuilder {
let output = fs::read_to_string(&dep_file)
.with_context(|| format!("read protoc dependency file {}", dep_file.display()))?;
// Make-style `.d` format: `<descriptor path>: dep1 dep2 …`.
// Compare with normalized separators — protoc may spell the
// target path with forward slashes even on Windows.
// Make-style `.d`: `<descriptor path>: dep1 dep2 …`.
// Normalize separators — protoc may emit `/` even on Windows.
let mut lines = output.lines();
let first_line = lines.next().context("protoc dependency output is empty")?;
let normalized_first = first_line.replace('\\', "/");
@@ -179,9 +164,7 @@ impl XaiProtoBuilder {
for line in iter::once(rem).chain(lines) {
let line = line.trim();
let line = line.strip_suffix("\\").unwrap_or(line);
// Depending on absolute paths like
// /Users/user/homebrew/Cellar/protobuf/29.1/include/google/protobuf/timestamp.proto
// is valid, but we want to have output more deterministic.
// Skip host-absolute well-known includes so fingerprints stay portable.
if line.contains("/include/google/protobuf/") {
continue;
}
@@ -224,14 +207,10 @@ impl XaiProtoBuilder {
let protoc = find_protoc::find_protoc()?;
// Use fixed version of `protoc` binary.
if let Some(protoc) = &protoc {
config.protoc_executable(protoc);
}
// Find the protoc's well-known types include directory.
// This is needed for Bazel sandboxed builds where protoc and its
// include files are placed in different sandbox locations.
let protoc_include_dir = find_protoc_include_dir(protoc.as_deref());
let mut builder = builder.emit_rerun_if_changed(false);
@@ -256,8 +235,7 @@ impl XaiProtoBuilder {
None
};
// Build the full includes list, prepending the protoc include directory
// if found (for well-known types like google/protobuf/timestamp.proto).
// Prepend protoc includes so well-known types resolve under Bazel.
let all_includes: Vec<&Path> = protoc_include_dir
.as_deref()
.into_iter()
+2 -1
View File
@@ -78,7 +78,8 @@ mod acp_send_failure_tests {
#[tokio::test]
async fn send_failed_when_receiver_dropped_before_send() {
let (tx, rx) = mpsc::unbounded_channel::<AcpAgentMessage>();
drop(rx); // no peer listening -> enqueue fails
// no peer listening -> enqueue fails
drop(rx);
let err = acp_send(ext_request(), &tx).await.unwrap_err();
assert_eq!(
acp_channel_failure(&err),
+14 -15
View File
@@ -20,24 +20,24 @@ pub fn acp_internal_error(message: impl Into<String>) -> acp::Error {
/// The two distinct ways an [`acp_send`](crate::acp_send) round-trip can fail
/// when the underlying channel is closed. Both surface as a JSON-RPC
/// `INTERNAL_ERROR` (so existing callers and the wire format are unaffected);
/// this typed discriminant — carried in the error's `data` — lets callers tell
/// them apart WITHOUT substring-matching the human-readable `message`.
/// `INTERNAL_ERROR`; this typed discriminant — carried in the error's `data` —
/// lets callers tell them apart without substring-matching the human-readable
/// `message`.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum AcpChannelFailure {
/// The request could not be ENQUEUED: the receiver half (the peer's
/// The request could not be enqueued: the receiver half (the peer's
/// connection task) is already gone, so no peer is listening — e.g. a
/// headless run with no client wired.
SendFailed,
/// The request was enqueued but the RESPONSE channel was dropped before a
/// The request was enqueued but the response channel was dropped before a
/// reply arrived: a peer received the request, then went away (disconnect /
/// process exit) without answering.
RecvFailed,
}
impl AcpChannelFailure {
/// `data` object key under which [`acp_send`](crate::acp_send) records the
/// kind. Namespaced so it can never collide with other `with_data` payloads.
/// `data` object key under which the kind is recorded. Namespaced so it can
/// never collide with other `with_data` payloads.
const DATA_KEY: &'static str = "xaiAcpChannelFailure";
const fn tag(self) -> &'static str {
@@ -58,8 +58,8 @@ impl AcpChannelFailure {
/// Build the channel-closed error for [`acp_send`](crate::acp_send), tagging it
/// with a typed [`AcpChannelFailure`] discriminant in `data`. The error `code`
/// stays `INTERNAL_ERROR`, so this is purely additive for callers that just
/// propagate the error.
/// stays `INTERNAL_ERROR` so callers that merely propagate the error are
/// unaffected.
pub(crate) fn acp_channel_failure_error(
message: impl Into<String>,
kind: AcpChannelFailure,
@@ -68,8 +68,8 @@ pub(crate) fn acp_channel_failure_error(
}
/// Recover the [`AcpChannelFailure`] kind from an error, or `None` if the error
/// did not originate from [`acp_send`](crate::acp_send)'s channel-closed paths
/// (or predates the tag). Consumers use this instead of inspecting `message`.
/// did not originate from [`acp_send`](crate::acp_send)'s channel-closed paths.
/// Consumers use this instead of inspecting `message`.
pub fn acp_channel_failure(err: &acp::Error) -> Option<AcpChannelFailure> {
err.data
.as_ref()
@@ -78,10 +78,9 @@ pub fn acp_channel_failure(err: &acp::Error) -> Option<AcpChannelFailure> {
.and_then(AcpChannelFailure::from_tag)
}
/// Compact single-line JSON for gateway debug traces. Plain (uncolored)
/// output: this feeds `tracing::debug!`, which typically lands in log files
/// where ANSI colors are noise. Replaces the former `colored_json`-backed
/// `color_json` (dropped to shrink the shipped dependency tree).
/// Compact single-line JSON for gateway debug traces. Output is uncolored: it
/// feeds `tracing::debug!`, which typically lands in log files where ANSI
/// escapes are noise.
#[doc(hidden)]
pub fn compact_json<T: serde::Serialize>(value: &T) -> String {
serde_json::to_string(value).unwrap_or_default()
+4 -8
View File
@@ -613,7 +613,8 @@ mod tests {
})
.collect();
// Gate-open point; then concurrent producer emits live updates.
// Phase 2: a concurrent producer emits live updates while the
// replay completions are still draining.
let live_sender = sender.clone();
let producer = tokio::task::spawn_local(async move {
for i in 0..LIVE {
@@ -624,15 +625,14 @@ mod tests {
}
});
// Drain replay completions while producer runs.
for rx in completions {
let _ = rx.await;
}
// Mark response boundary.
log.borrow_mut().push("RESPONSE".into());
// Let producer and gateway finish remaining live updates.
// Give the producer and the gateway loop room to flush the
// remaining live updates before inspecting the log.
let _ = producer.await;
for _ in 0..LIVE + 5 {
tokio::task::yield_now().await;
@@ -644,7 +644,6 @@ mod tests {
.position(|s| s == "RESPONSE")
.expect("RESPONSE marker must be in the log");
// (1) Delta notifications are all present and before RESPONSE.
for i in 0..DELTA {
let tag = format!("delta-{i}");
let pos = log
@@ -657,7 +656,6 @@ mod tests {
);
}
// (2) Delta notifications preserve enqueue order.
let delta_positions: Vec<usize> = (0..DELTA)
.map(|i| log.iter().position(|s| s == &format!("delta-{i}")).unwrap())
.collect();
@@ -670,7 +668,6 @@ mod tests {
);
}
// (3) No live updates are lost.
for i in 0..LIVE {
let tag = format!("live-{i}");
assert!(
@@ -679,7 +676,6 @@ mod tests {
);
}
// (4) Live updates do not precede replay delta.
let last_delta = *delta_positions.last().unwrap();
for i in 0..LIVE {
let tag = format!("live-{i}");
@@ -126,7 +126,7 @@ impl AsyncRead for LineBufferedRead {
Poll::Ready(Ok(n))
}
Poll::Ready(Some(Err(e))) => Poll::Ready(Err(e)),
Poll::Ready(None) => Poll::Ready(Ok(0)), // EOF
Poll::Ready(None) => Poll::Ready(Ok(0)),
Poll::Pending => Poll::Pending,
}
}
@@ -146,7 +146,7 @@ async fn read_line_capped(
let (consumed, done) = {
let available = reader.fill_buf().await?;
if available.is_empty() {
return Ok(buf.len()); // EOF
return Ok(buf.len());
}
match available.iter().position(|&b| b == b'\n') {
Some(pos) => {
@@ -283,15 +283,12 @@ mod tests {
let mut reader = LineBufferedRead::spawn_local(source);
let mut small_buf = [0u8; 3];
// First read: "abc"
let n = reader.read(&mut small_buf).await.unwrap();
assert_eq!(&small_buf[..n], b"abc");
// Second read: "def"
let n = reader.read(&mut small_buf).await.unwrap();
assert_eq!(&small_buf[..n], b"def");
// Third read: "\n"
let n = reader.read(&mut small_buf).await.unwrap();
assert_eq!(&small_buf[..n], b"\n");
+12 -6
View File
@@ -26,16 +26,20 @@ pub trait AcpSide {
/// Marker type representing the agent's view of the ACP connection (as one side of that connection).
impl AcpSide for acp::AgentSide {
type InMessage = AcpAgentMessage; // inbound messages = messages meant *for* the agent
type OutMessage = AcpClientMessage; // outbound messages = messages meant *for* the client
// inbound messages = messages meant *for* the agent
type InMessage = AcpAgentMessage;
// outbound messages = messages meant *for* the client
type OutMessage = AcpClientMessage;
type OtherSide = acp::ClientSide;
const NAME: &'static str = "agent";
}
/// Marker type representing the agent's view of the ACP connection (as one side of that connection).
impl AcpSide for acp::ClientSide {
type InMessage = AcpClientMessage; // inbound messages = messages meant *for* the client
type OutMessage = AcpAgentMessage; // outbound messages = messages meant *for* the agent
// inbound messages = messages meant *for* the client
type InMessage = AcpClientMessage;
// outbound messages = messages meant *for* the agent
type OutMessage = AcpAgentMessage;
type OtherSide = acp::AgentSide;
const NAME: &'static str = "client";
}
@@ -241,7 +245,8 @@ mod client {
pub fn route_to_client(
self,
client: impl acp::Client + 'static, // note: acp::Client is auto-implemented for Rc/Arc
// note: acp::Client is auto-implemented for Rc/Arc
client: impl acp::Client + 'static,
spawn: impl Fn(LocalBoxFuture<'static, ()>) + 'static,
) {
match self {
@@ -540,7 +545,8 @@ mod agent {
pub fn route_to_agent(
self,
agent: impl acp::Agent + 'static, // note: acp::Agent is auto-implemented for Rc/Arc
// note: acp::Agent is auto-implemented for Rc/Arc
agent: impl acp::Agent + 'static,
spawn: impl Fn(LocalBoxFuture<'static, ()>) + 'static,
) {
match self {
+1 -1
View File
@@ -33,7 +33,7 @@
/// `\u2028` and surrogate pairs in text).
///
/// Any line that fails both parses passes through byte-identical —
/// deliberately: the acp crate keeps ownership of garbage handling.
/// Deliberately: the acp crate keeps ownership of garbage handling.
pub(crate) fn normalize_json_line(line: Vec<u8>) -> Vec<u8> {
if !line.windows(2).any(|w| w == br"\/") {
return line;
@@ -138,7 +138,8 @@ fn isolate_process_stdin() -> Option<std::fs::File> {
use std::os::windows::io::FromRawHandle as _;
// Win32 constants (inlined to avoid a dependency).
const STD_INPUT_HANDLE: u32 = 0xFFFF_FFF6; // (DWORD)-10
// (DWORD)-10
const STD_INPUT_HANDLE: u32 = 0xFFFF_FFF6;
const DUPLICATE_SAME_ACCESS: u32 = 0x0000_0002;
const GENERIC_READ: u32 = 0x8000_0000;
const FILE_SHARE_READ: u32 = 0x0000_0001;
@@ -186,7 +187,8 @@ fn isolate_process_stdin() -> Option<std::fs::File> {
process,
&mut duplicate,
0,
0, // not inheritable
// not inheritable
0,
DUPLICATE_SAME_ACCESS,
) == 0
{
@@ -4,8 +4,9 @@ use crate::send::contributors::command::{
CommandAction, CommandContributor, CommandInvocation, CommandSpec,
};
/// `?Send` twin of [`CommandContributor`] for single-threaded hosts like kigi build's TUI agent, whose session state is `Rc`/`RefCell`-based and can
/// never satisfy the `Send` bounds the send flavor bakes into its boxed hook futures.
/// `?Send` twin of [`CommandContributor`] for single-threaded hosts like kigi build's TUI agent,
/// whose session state is `Rc`/`RefCell`-based and can never satisfy the `Send` bounds the send
/// flavor bakes into its boxed hook futures.
#[async_trait(?Send)]
pub trait LocalCommandContributor {
fn advertised_commands(&self) -> Vec<CommandSpec>;
@@ -14,7 +15,8 @@ pub trait LocalCommandContributor {
-> Result<CommandAction, String>;
}
/// Send contributors work in single-threaded hosts as-is, so shared logic implements [`CommandContributor`] once and both hosts can register it.
/// Send contributors work in single-threaded hosts as-is, so shared logic implements
/// [`CommandContributor`] once and both hosts can register it.
#[async_trait(?Send)]
impl<T: CommandContributor> LocalCommandContributor for T {
fn advertised_commands(&self) -> Vec<CommandSpec> {
@@ -5,7 +5,8 @@ use crate::send::contributors::session_lifecycle::{SessionIdleInput, SessionLife
/// `?Send` twin of [`SessionLifecycleContributor`].
#[async_trait(?Send)]
pub trait LocalSessionLifecycleContributor {
/// Fired when the session settles idle (no running turn or queued work); the host owns the check.
/// Fired when the session settles idle (no running turn or queued work); the host owns the
/// check.
async fn on_session_idle(&self, _input: &SessionIdleInput) {}
}
@@ -4,8 +4,9 @@ use crate::send::contributors::turn_input::{
TurnInputContext, TurnInputContributor, TurnInputFragment,
};
/// `?Send` twin of [`TurnInputContributor`] for single-threaded hosts like kigi build's TUI agent, whose session state is `Rc`/`RefCell`-based
/// and can never satisfy the `Send` bounds the send flavor bakes into its boxed hook futures.
/// `?Send` twin of [`TurnInputContributor`] for single-threaded hosts like kigi build's TUI agent,
/// whose session state is `Rc`/`RefCell`-based and can never satisfy the `Send` bounds the send
/// flavor bakes into its boxed hook futures.
#[async_trait(?Send)]
pub trait LocalTurnInputContributor {
async fn contribute_turn_input(&self, _input: &TurnInputContext) -> Vec<TurnInputFragment> {
@@ -13,7 +14,8 @@ pub trait LocalTurnInputContributor {
}
}
/// Send contributors are usable in single-threaded hosts as-is, so shared logic implements [`TurnInputContributor`] once for both hosts.
/// Send contributors are usable in single-threaded hosts as-is, so shared logic implements
/// [`TurnInputContributor`] once for both hosts.
#[async_trait(?Send)]
impl<T: TurnInputContributor> LocalTurnInputContributor for T {
async fn contribute_turn_input(&self, input: &TurnInputContext) -> Vec<TurnInputFragment> {
@@ -6,7 +6,6 @@ use crate::local::contributors::{
LocalTurnLifecycleContributor,
};
/// Mutable registry used while hosts register typed runtime contributions.
#[derive(Default)]
pub struct LocalExtensionRegistryBuilder {
turn_lifecycle_contributors: Vec<Rc<dyn LocalTurnLifecycleContributor>>,
@@ -67,7 +66,6 @@ impl LocalExtensionRegistryBuilder {
}
}
/// Immutable typed registry produced after extensions are installed.
#[derive(Default)]
pub struct LocalExtensionRegistry {
turn_lifecycle_contributors: Vec<Rc<dyn LocalTurnLifecycleContributor>>,
@@ -94,7 +92,6 @@ impl LocalExtensionRegistry {
&self.command_contributors
}
/// The one contributor owning `name`, or `None` when no extension advertised it.
pub fn command_handler(&self, name: &str) -> Option<&Rc<dyn LocalCommandContributor>> {
self.command_handlers.get(name)
}
@@ -10,7 +10,8 @@ pub struct CommandSpec {
/// A parsed `/name args` invocation. The host owns parsing and routes it to the command's one owner.
pub struct CommandInvocation<'a> {
pub name: &'a str,
pub args: &'a str, // Whitespace-trimmed; empty for a bare `/name`.
// Whitespace-trimmed; empty for a bare `/name`.
pub args: &'a str,
}
/// What a handled command does to the turn; rejections travel as the `Err` reason.
@@ -1,10 +1,9 @@
use async_trait::async_trait;
/// Input supplied when the host observes the session settling idle.
pub struct SessionIdleInput;
#[async_trait]
pub trait SessionLifecycleContributor: Send + Sync {
/// Fired when the session settles idle (no running turn or queued work); the host owns the check.
/// Idle means no running turn and no queued work; the host owns that check.
async fn on_session_idle(&self, _input: &SessionIdleInput) {}
}
@@ -1,20 +1,17 @@
use async_trait::async_trait;
/// Turn facts supplied when the host pulls extension input at its sampling chokepoint.
pub struct TurnInputContext {
/// Stable host-owned turn identifier.
pub turn_id: String,
/// True when the harness produced the turn (auto-wake, drain, cron, continuation), not the user.
pub synthetic: bool,
}
/// A model-visible input fragment contributed into the active turn. The host owns wrapping, origin stamping, and placement.
/// Raw fragment text: the host owns wrapping, origin stamping, and placement.
pub struct TurnInputFragment {
pub text: String,
}
/// Contributes model-visible input fragments into the active turn when the host pulls at its sampling chokepoint.
/// Fragments land in the same turn, never a new one.
/// Fragments land in the turn the host is already sampling, never a new one.
#[async_trait]
pub trait TurnInputContributor: Send + Sync {
async fn contribute_turn_input(&self, _input: &TurnInputContext) -> Vec<TurnInputFragment> {
@@ -1,6 +1,5 @@
use async_trait::async_trait;
/// Input supplied when the host starts a turn.
pub struct TurnStartInput {
/// True when the harness produced the turn (auto-wake, drain, cron, continuation), not the user.
pub synthetic: bool,
@@ -12,19 +11,16 @@ impl TurnStartInput {
}
}
/// Input supplied when the host completes a turn.
pub struct TurnDoneInput;
/// Why the host aborted the turn instead of completing it.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum TurnAbortReason {
/// The client went away mid-turn.
Disconnected,
/// The user interrupted the turn before it completed.
/// The user cancelled mid-turn.
Interrupted,
}
/// Input supplied when the host aborts a turn.
pub struct TurnAbortInput {
pub reason: TurnAbortReason,
}
@@ -35,7 +31,6 @@ impl TurnAbortInput {
}
}
/// Input supplied when the host observes an error for a turn.
pub struct TurnErrorInput<'a> {
pub message: &'a str,
}
@@ -5,7 +5,6 @@ use crate::send::contributors::{
CommandContributor, SessionLifecycleContributor, TurnInputContributor, TurnLifecycleContributor,
};
/// Mutable registry used while hosts register typed runtime contributions.
#[derive(Default)]
pub struct ExtensionRegistryBuilder {
turn_lifecycle_contributors: Vec<Arc<dyn TurnLifecycleContributor>>,
@@ -38,8 +37,8 @@ impl ExtensionRegistryBuilder {
self.command_contributors.push(contributor);
}
/// Routes each advertised command to its one owner. Duplicate names are a composition bug:
/// first registration wins, panics in debug builds, logs in release.
/// Two extensions advertising one command name is a composition bug, so it trips a
/// `debug_assert`; release builds keep the first registration and log the loser.
pub fn build(self) -> ExtensionRegistry {
let mut command_handlers: HashMap<String, Arc<dyn CommandContributor>> = HashMap::new();
for contributor in &self.command_contributors {
@@ -63,7 +62,6 @@ impl ExtensionRegistryBuilder {
}
}
/// Immutable typed registry produced after extensions are installed.
#[derive(Default)]
pub struct ExtensionRegistry {
turn_lifecycle_contributors: Vec<Arc<dyn TurnLifecycleContributor>>,
@@ -90,7 +88,6 @@ impl ExtensionRegistry {
&self.command_contributors
}
/// The one contributor owning `name`, or `None` when no extension advertised it.
pub fn command_handler(&self, name: &str) -> Option<&Arc<dyn CommandContributor>> {
self.command_handlers.get(name)
}
+3 -7
View File
@@ -77,7 +77,7 @@ impl Agent {
}
}
// ── From definition ──────────────────────────────────────────────
// From definition
/// Agent name (unique identifier).
pub fn name(&self) -> &str {
@@ -99,14 +99,12 @@ impl Agent {
&self.definition.permission_mode
}
/// Completion requirement, if any.
pub fn completion_requirement(&self) -> Option<&CompletionRequirement> {
self.definition.completion_requirement.as_ref()
}
// ── Session-level ────────────────────────────────────────────────
// Session-level
/// The rendered system prompt.
pub fn system_prompt(&self) -> &str {
&self.system_prompt
}
@@ -123,12 +121,10 @@ impl Agent {
&self.tool_bridge
}
/// Compaction policy.
pub fn compaction_policy(&self) -> &CompactionPolicy {
&self.compaction_policy
}
/// Reminder policy.
pub fn reminder_policy(&self) -> &ReminderPolicy {
&self.reminder_policy
}
@@ -216,7 +212,7 @@ impl Agent {
/// Does NOT rebuild the tool registry or re-render prompts.
/// Used for mid-session mode switching.
pub async fn update_policies_from_definition(&self, _def: &AgentDefinition) {
// TODO: completion requirements and retry configs are now part of
// TODO: completion requirements and retry configs are part of
// ToolServerConfig and handled at registry finalization time.
// Mid-session policy updates are not yet supported in the new architecture.
}
+33 -4
View File
@@ -711,9 +711,19 @@ impl AgentBuilder {
kigi_tools::types::tool::ToolNamespace::Kigi,
"task"
);
// The swarm is a fan-out of subagent spawns, so it lives and dies with
// the task tool: any condition that leaves no subagent to spawn leaves
// the swarm with nothing to fan out to.
let swarm_tool_id = format!(
"{}:{}",
kigi_tools::types::tool::ToolNamespace::Kigi,
"agent_swarm"
);
let mut task_stripped = false;
if !self.subagents_enabled {
tool_config.tools.retain(|tc| tc.id != task_tool_id);
tool_config
.tools
.retain(|tc| tc.id != task_tool_id && tc.id != swarm_tool_id);
task_stripped = true;
} else {
let subagents = crate::discovery::all_subagents_with_plugins(
@@ -722,7 +732,9 @@ impl AgentBuilder {
self.plugin_registry.as_deref(),
);
if subagents.is_empty() {
tool_config.tools.retain(|tc| tc.id != task_tool_id);
tool_config
.tools
.retain(|tc| tc.id != task_tool_id && tc.id != swarm_tool_id);
task_stripped = true;
} else if self.prompt_audience == crate::prompt::context::PromptAudience::Subagent {
if let Some(task_tc) = tool_config
@@ -810,7 +822,13 @@ impl AgentBuilder {
.tools
.iter()
.any(|t| AGENT_TASK_CLASSIFIER_RE.is_match(t));
let task_deps = ["task", "get_task_output", "kill_task", "wait_tasks"];
let task_deps = [
"task",
"agent_swarm",
"get_task_output",
"kill_task",
"wait_tasks",
];
let registered_tool_ids = tool_bridge_builder.known_tool_ids();
let present_kinds: std::collections::HashSet<ToolKind> =
tool_config.tools.iter().filter_map(|tc| tc.kind).collect();
@@ -925,7 +943,13 @@ impl AgentBuilder {
}
}
if definition.allowed_subagent_types.as_deref() == Some(&[]) {
let task_deps = ["task", "get_task_output", "kill_task", "wait_tasks"];
let task_deps = [
"task",
"agent_swarm",
"get_task_output",
"kill_task",
"wait_tasks",
];
tool_config
.tools
.retain(|tc| !task_deps.contains(&short_tool_name(&tc.id)));
@@ -1601,6 +1625,11 @@ mod tests {
has_task, *subagents,
"[{label}] spawn_subagent presence should match subagents_enabled={subagents}; got tools: {names:?}"
);
let has_swarm = names.contains(&"agent_swarm");
assert_eq!(
has_swarm, *subagents,
"[{label}] agent_swarm fans out to subagents, so it must follow the same gate as spawn_subagent; got tools: {names:?}"
);
assert!(
names.contains(&"enter_plan_mode"),
"[{label}] enter_plan_mode must always be present (TUI plan-mode keybind needs it); got tools: {names:?}"
+9 -15
View File
@@ -1,24 +1,18 @@
//! Compaction policy — threshold, model, and memory flush configuration.
/// Session-level compaction policy.
///
/// Controls when and how the session's conversation is compacted
/// to free up context window space, and whether a memory flush
/// runs before each compaction.
/// Controls when and how the session's conversation is compacted to free up
/// context window space, and whether a memory flush runs before each compaction.
#[derive(Debug, Clone)]
pub struct CompactionPolicy {
/// Percentage of context window that triggers auto-compaction.
/// E.g., 85 means compact when 85% of the context window is used.
pub auto_compact_threshold_percent: u32,
/// Model to use for generating the compaction summary.
/// None = use the session's current model.
/// `None` uses the session's current model.
pub compact_model: Option<String>,
/// Whether to run a memory flush turn before each compaction.
/// When enabled, the session actor asks the model to summarize
/// important information from the conversation before it's compacted.
/// Requires the memory system to be enabled.
/// Run a memory flush turn before each compaction: the session actor asks
/// the model to summarize important information from the conversation
/// before it is discarded. Requires the memory system to be enabled.
pub memory_flush_enabled: bool,
/// Per-compaction wall-clock budget (seconds); a generation exceeding it is
@@ -27,9 +21,9 @@ pub struct CompactionPolicy {
/// Prefire two-pass compaction: when usage approaches the threshold,
/// speculatively summarize the history prefix in the background (pass 1);
/// at compaction, summarize NOTE₁ + the recent tail (pass 2). Resolved from
/// config (`two_pass_compaction` flag) at session build; `false` keeps the
/// legacy single-pass path. Default `false` (real sessions set it from config).
/// at compaction, summarize NOTE₁ + the recent tail (pass 2). `false`
/// selects the single-pass path. Real sessions resolve this from the
/// `two_pass_compaction` config flag at session build.
pub two_pass_enabled: bool,
}
+14 -4
View File
@@ -154,6 +154,11 @@ fn task_tool_config() -> ToolConfig {
.with_name("spawn_subagent")
.with_param_rename("run_in_background", "background")
}
/// Swarm tool. Keeps its registry name: unlike `task` it has no CLI-specific
/// alias, and the name is what the model is told to reuse for `resume_agent_ids`.
fn agent_swarm_tool_config() -> ToolConfig {
ToolConfig::from(&kigi::AgentSwarmTool)
}
/// Task output tool renamed for clarity:
/// `get_task_output` → `get_command_or_subagent_output`.
fn task_output_tool_config() -> ToolConfig {
@@ -270,6 +275,7 @@ fn default_kigi_toolset() -> ToolServerConfig {
task_output_tool_config(),
wait_tasks_tool_config(),
task_tool_config(),
agent_swarm_tool_config(),
(&kigi::SchedulerCreateTool).into(),
(&kigi::SchedulerDeleteTool).into(),
(&kigi::SchedulerListTool).into(),
@@ -318,6 +324,7 @@ pub fn kigi_hashline_toolset(
task_output_tool_config(),
wait_tasks_tool_config(),
task_tool_config(),
agent_swarm_tool_config(),
(&kigi::WebSearchTool).into(),
(&kigi::SchedulerCreateTool).into(),
(&kigi::SchedulerDeleteTool).into(),
@@ -400,6 +407,7 @@ fn kigi_plan_toolset() -> ToolServerConfig {
(&kigi::TodoWriteTool).into(),
task_output_tool_config(),
task_tool_config(),
agent_swarm_tool_config(),
(&kigi::SchedulerCreateTool).into(),
(&kigi::SchedulerDeleteTool).into(),
(&kigi::SchedulerListTool).into(),
@@ -428,6 +436,7 @@ fn orchestrator_toolset() -> ToolServerConfig {
(&kigi::ListDirTool).into(),
(&kigi::GrepTool).into(),
task_tool_config(),
agent_swarm_tool_config(),
task_output_tool_config(),
wait_tasks_tool_config(),
kill_task_tool_config(),
@@ -497,6 +506,7 @@ fn kigi_ask_user_toolset() -> ToolServerConfig {
task_output_tool_config(),
wait_tasks_tool_config(),
task_tool_config(),
agent_swarm_tool_config(),
(&kigi::SchedulerCreateTool).into(),
(&kigi::SchedulerDeleteTool).into(),
(&kigi::SchedulerListTool).into(),
@@ -1381,10 +1391,10 @@ impl AgentDefinition {
///
/// Used by the runtime turn-end TodoGate to gate firing on sessions
/// whose prompt actually references the rules the gate's reminder
/// text invokes. The block has been removed from every built-in
/// template, so this returns `false` unconditionally. Kept as a
/// helper so the gate's call-site stays stable in case the block
/// is reintroduced behind a future flag.
/// text invokes. No built-in template carries the block, so this
/// returns `false` unconditionally. Kept as a helper so the gate's
/// call-site stays stable in case the block is reintroduced behind
/// a future flag.
pub fn carries_task_completion_discipline(
&self,
_audience: crate::prompt::context::PromptAudience,
+16 -13
View File
@@ -39,7 +39,7 @@ pub fn project_agent_dirs_in(chain_dirs: &[PathBuf]) -> Vec<PathBuf> {
crate::repo::existing_subdirs_along(chain_dirs, PROJECT_AGENT_SUBDIRS)
}
// ── Subagent entry types ─────────────────────────────────────────────
// Subagent entry types
/// A subagent entry for the Task tool description and spawn-time validation.
#[derive(Debug, Clone)]
@@ -61,7 +61,7 @@ pub enum SubagentSource {
UserDefined { scope: AgentScope },
}
// ── all_subagents ────────────────────────────────────────────────────
// all_subagents
/// Build the complete list of enabled subagents.
///
@@ -102,7 +102,6 @@ fn merge_subagents(
}
}
// 1. Seed with built-in subagents
let mut entries: Vec<SubagentEntry> = BuiltinAgentName::subagent_variants()
.iter()
.map(|b| {
@@ -117,7 +116,6 @@ fn merge_subagents(
})
.collect();
// 2. Merge in discovered user-defined agents.
//
// IMPORTANT: Only project-level agents can shadow built-ins. This matches
// the runtime spawn precedence in by_name_in_cwd():
@@ -173,7 +171,6 @@ fn merge_subagents(
}
}
// 3. Filter by toggle (omitted = enabled)
entries
.into_iter()
.filter(|e| toggle.get(&e.name).copied().unwrap_or(true))
@@ -359,7 +356,7 @@ fn source_from_agent_def(def: &AgentDefinition) -> ConfigSource {
}
}
// ── Plugin-aware variants ─────────────────────────────────────────────
// Plugin-aware variants
/// Build the complete list of enabled subagents, including plugin agents.
pub fn all_subagents_with_plugins(
@@ -1031,7 +1028,7 @@ mod tests {
assert_eq!(def.scope, AgentScope::BuiltIn);
}
// ── all_subagents / merge_subagents tests ───────────────────────
// all_subagents / merge_subagents tests
/// Helper: build a minimal synthetic AgentDefinition for testing merge logic.
fn synthetic_agent(name: &str, desc: &str, scope: AgentScope) -> AgentDefinition {
@@ -1110,7 +1107,8 @@ mod tests {
AgentScope::Project,
)];
let entries = merge_subagents(discovered, &HashMap::new());
assert_eq!(entries.len(), 4); // 3 built-ins + 1 user
// 3 built-ins + 1 user
assert_eq!(entries.len(), 4);
let cr = entries.iter().find(|e| e.name == "code-reviewer").unwrap();
assert_eq!(cr.description, "Reviews code");
assert_eq!(
@@ -1131,7 +1129,8 @@ mod tests {
)];
let toggle = HashMap::from([("code-reviewer".to_string(), false)]);
let entries = merge_subagents(discovered, &toggle);
assert_eq!(entries.len(), 3); // only built-ins
// only built-ins
assert_eq!(entries.len(), 3);
assert!(entries.iter().all(|e| e.name != "code-reviewer"));
}
@@ -1143,7 +1142,8 @@ mod tests {
AgentScope::Project,
)];
let entries = merge_subagents(discovered, &HashMap::new());
assert_eq!(entries.len(), 3); // still 3 — replaced, not appended
// still 3 — replaced, not appended
assert_eq!(entries.len(), 3);
let explore = entries.iter().find(|e| e.name == "explore").unwrap();
assert_eq!(explore.description, "Custom explore agent");
assert_eq!(
@@ -1180,7 +1180,8 @@ mod tests {
AgentScope::User,
)];
let entries = merge_subagents(discovered, &HashMap::new());
assert_eq!(entries.len(), 3); // still 3 built-ins
// still 3 built-ins
assert_eq!(entries.len(), 3);
let explore = entries.iter().find(|e| e.name == "explore").unwrap();
// Should still be the built-in, not the user-level agent
assert!(
@@ -1215,7 +1216,8 @@ mod tests {
AgentScope::User,
)];
let entries = merge_subagents(discovered, &HashMap::new());
assert_eq!(entries.len(), 4); // 3 built-ins + 1 user
// 3 built-ins + 1 user
assert_eq!(entries.len(), 4);
// Verify ordering: built-ins first, then user
assert!(matches!(&entries[0].source, SubagentSource::Builtin(_)));
assert!(matches!(&entries[1].source, SubagentSource::Builtin(_)));
@@ -1262,7 +1264,8 @@ mod tests {
// Simulate: discover() skips invalid files (returns empty for that file).
// So if a user's explore.md is invalid, discover() won't include it,
// and the built-in explore remains.
let discovered = vec![]; // no valid user agents discovered
// no valid user agents discovered
let discovered = vec![];
let entries = merge_subagents(discovered, &HashMap::new());
assert_eq!(entries.len(), 3);
let explore = entries.iter().find(|e| e.name == "explore").unwrap();
+4 -11
View File
@@ -1,36 +1,29 @@
//! Error types for agent construction.
/// Errors that can occur during Agent construction.
#[derive(Debug, thiserror::Error)]
pub enum AgentBuildError {
/// Failed to parse the agent definition file (bad YAML frontmatter,
/// missing closing `---`, or invalid Markdown structure).
/// Bad YAML frontmatter, a missing closing `---`, or invalid Markdown
/// structure in the definition file.
#[error("failed to parse agent definition: {0}")]
ParseError(String),
/// Required fields are missing from the definition (name, description).
#[error("missing required field in agent definition: {0}")]
MissingField(String),
/// A tool name override references a tool that doesn't exist in the
/// registry (typo in the definition's `toolNameOverrides`).
/// Usually a typo in the definition's `toolNameOverrides`.
#[error("tool name override references nonexistent tool '{0}'")]
UnknownToolOverride(String),
/// IO error during AGENTS.md or skills discovery.
#[error("IO error during agent construction: {0}")]
IoError(#[from] std::io::Error),
/// MiniJinja template rendering failed (extend or full mode).
/// Includes line numbers and context from the template.
/// Carries template line numbers and surrounding context.
#[error("template rendering error: {0}")]
MiniJinjaError(#[from] minijinja::Error),
/// Tool registry error (e.g., unsatisfied requirements during finalization).
#[error("tool error: {0}")]
ToolError(String),
/// A configuration value is present but invalid (e.g. `max_turns = 0`).
#[error("invalid configuration: {0}")]
InvalidConfig(String),
}
-1
View File
@@ -1,6 +1,5 @@
//! Agent builder, definition parsing, and system prompt assembly.
//!
//! This crate extracts a first-class `Agent` type from `kigi-shell`.
//! An `Agent` bundles tools, system prompt, system-reminder policy,
//! compaction policy, and model configuration into a single, portable
//! object that any host can consume.
@@ -21,7 +21,7 @@ use sha2::{Digest, Sha256};
use super::manifest::{ManifestLoadResult, PluginManifest, load_manifest, name_from_dirname};
use super::trust::TrustStore;
// ── Public types ──────────────────────────────────────────────────────
// Public types
/// Where a plugin was discovered from.
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -204,12 +204,12 @@ impl DiscoveryConfig {
}
}
// ── Discovery entry point ─────────────────────────────────────────────
// Discovery entry point
/// User plugin directories in priority order: `$KIGI_SHARE_DIR/plugins` then
/// `~/.claude/plugins`.
///
/// Unlike agent discovery, plugins are intentionally NOT discovered from a
/// Unlike agent discovery, plugins are deliberately NOT discovered from a
/// legacy `~/.kigi/plugins`: plugin trust, persisted plugin-data, and install
/// paths all resolve under `kigi_home()`, so a plugin scanned from the legacy
/// tree would appear untrusted and lose its persisted state. Keeping plugins on
@@ -483,7 +483,7 @@ pub fn discover_plugins(
candidates
}
// ── Internal helpers ──────────────────────────────────────────────────
// Internal helpers
/// Scan a plugins parent directory (e.g. `~/.kigi/plugins/`) and collect
/// each subdirectory as a plugin candidate.
@@ -510,7 +510,8 @@ fn scan_plugin_dir(
let mut subdirs: Vec<PathBuf> = entries
.filter_map(|e| e.ok())
.filter(|e| e.path().is_dir()) // follows symlinks
// follows symlinks
.filter(|e| e.path().is_dir())
.map(|e| e.path())
.collect();
@@ -787,7 +788,7 @@ fn resolve_name_conflicts(candidates: &mut Vec<DiscoveredPlugin>) {
}
}
// ── Compat installed_plugins.json types ───────────────────────────────
// Compat installed_plugins.json types
/// Compat `installed_plugins.json` format.
#[derive(serde::Deserialize)]
@@ -1351,7 +1352,7 @@ mod tests {
let parts: Vec<&str> = id.0.split('/').collect();
assert_eq!(parts.len(), 3);
assert_eq!(parts[0], "user");
assert_eq!(parts[1].len(), 8); // 8 hex chars
assert_eq!(parts[1].len(), 8);
assert_eq!(parts[2], "my-plugin");
}
@@ -561,7 +561,7 @@ pub struct UpdateResult {
/// Status of an update attempt.
pub enum UpdateStatus {
/// Repo was updated successfully.
/// Repo updated successfully.
Updated(UpdateResult),
/// Repo is pinned to a tag or commit — no automatic update.
Pinned { ref_name: String },
@@ -633,7 +633,7 @@ pub fn update_repo(repo_key: &str, repo: &InstalledRepo) -> Result<UpdateStatus,
let new_commit = read_head_commit(repo_path);
let changed = old_commit.as_deref() != new_commit.as_deref();
// Re-discover plugins (new ones may have been added)
// Re-discover plugins: the pull may bring new ones
let plugins = discover_plugins_in_dir(repo_path, subdir.as_deref())?;
Ok(UpdateStatus::Updated(UpdateResult {
@@ -296,7 +296,8 @@ mod tests {
fn prefilter_handles_invalid_json() {
let json = "not valid json{";
let (filtered, skipped) = prefilter_unsupported_events(json);
assert_eq!(filtered, json); // returned as-is
// returned as-is
assert_eq!(filtered, json);
assert!(skipped.is_empty());
}
@@ -500,7 +501,7 @@ mod tests {
/// reference resolves to the plugin root exactly once, and the result
/// contains no leftover `$` placeholders. This is the contract the
/// hooks_adapter has long held, and it must continue to hold
/// now that `parse_hook_file` itself does an env-expansion pass with
/// because `parse_hook_file` itself does an env-expansion pass with
/// the per-hook `extra_env`. The first pass (in `parse_hook_file`)
/// runs against an EMPTY `extra_env` for plugin hooks (the adapter
/// only fills it in afterwards), so the placeholder survives that
@@ -297,7 +297,7 @@ impl InstallRegistry {
}
}
// ── Errors ────────────────────────────────────────────────────────────
// Errors
#[derive(Debug, thiserror::Error)]
pub enum InstallError {
@@ -323,7 +323,7 @@ pub enum InstallError {
InstallFailed { detail: String },
}
// ── Tests ─────────────────────────────────────────────────────────────
// Tests
#[cfg(test)]
mod tests {
@@ -154,7 +154,7 @@ pub struct PluginManifest {
#[serde(default)]
pub keywords: Vec<String>,
// ── Component path overrides (supplement convention dirs) ──────
// Component path overrides (supplement convention dirs)
#[serde(default)]
pub skills: Option<PathOrPaths>,
#[serde(default)]
@@ -247,7 +247,7 @@ impl PluginManifest {
/// Log informational messages about manifest features.
///
/// Called during discovery. Inline hooks and MCP servers are now
/// Called during discovery. Inline hooks and MCP servers are
/// fully supported; this method logs when they are detected.
pub fn warn_unsupported_features(&self, plugin_name: &str) {
if self.inline_hooks().is_some() {
@@ -287,7 +287,7 @@ fn resolve_dirs(
}
}
// ── Manifest loading ──────────────────────────────────────────────────
// Manifest loading
/// Manifest search order within a plugin directory.
const MANIFEST_PATHS: &[&str] = &[
@@ -375,7 +375,7 @@ pub fn normalize_inline_mcp_servers(value: &serde_json::Value) -> serde_json::Va
serde_json::json!({ "mcpServers": inner })
}
// ── Errors ────────────────────────────────────────────────────────────
// Errors
#[derive(Debug, thiserror::Error)]
pub enum ManifestError {
@@ -530,7 +530,8 @@ mod tests {
);
assert_eq!(
name_from_dirname(Path::new("/path/to/---")),
None // all hyphens after trim
// all hyphens after trim
None
);
}
@@ -147,7 +147,7 @@ pub fn load_enabled_disabled_plugins(path: &Path) -> (Vec<String>, Vec<String>)
parse_enabled_disabled_plugins(&json)
}
// ── Compat known_marketplaces.json ────────────────────────────────────
// Compat known_marketplaces.json
/// Entry in `~/.claude/plugins/known_marketplaces.json`.
#[derive(serde::Deserialize)]
+2 -8
View File
@@ -1,15 +1,9 @@
//! Plugin system — discover, load, and manage plugins (including compat layouts).
//! Plugin discovery, loading, and registry.
//!
//! A plugin is a self-contained directory that bundles skills, agents,
//! MCP server configs, and hooks into a namespaced unit. Plugins can
//! MCP server configs, and hooks into a namespaced unit. Plugins can
//! live under `~/.kigi/plugins/`, `.kigi/plugins/` (project-level),
//! or be passed via `--plugin-dir` on the CLI.
//!
//! This module handles:
//! - `manifest` — parsing `plugin.json` manifests
//! - `discovery` — scanning the filesystem for plugin directories
//! - `trust` — project-plugin trust management
//! - `registry` — in-memory registry of active plugins
pub mod discovery;
pub mod git_install;
@@ -301,7 +301,7 @@ impl PluginRegistry {
}
}
// ── Shared handle for cross-thread reload ─────────────────────────────
// Shared handle for cross-thread reload
/// Thread-safe handle for plugin registry lifecycle.
///
@@ -456,7 +456,7 @@ impl SharedPluginRegistryHandle {
}
}
// ── Component counting helpers ────────────────────────────────────────
// Component counting helpers
/// Collect the SKILL.md paths that load from the given skill dirs.
///
@@ -793,9 +793,11 @@ mod tests {
&["enabled-plugin".to_string()],
);
assert_eq!(reg.len(), 2); // Both in registry
// Both in registry
assert_eq!(reg.len(), 2);
let active = reg.active_plugins();
assert_eq!(active.len(), 1); // Only enabled one is active
// Only enabled one is active
assert_eq!(active.len(), 1);
assert_eq!(active[0].name, "enabled-plugin");
// Disabled one is in list but marked disabled
@@ -876,9 +878,12 @@ mod tests {
let reg = PluginRegistry::from_discovered(plugins, &[], &[]);
let list = reg.list();
assert_eq!(list[0].name, "alpha"); // CliOverride = 0
assert_eq!(list[1].name, "beta"); // Project = 1
assert_eq!(list[2].name, "zebra"); // User = 2
// CliOverride = 0
assert_eq!(list[0].name, "alpha");
// Project = 1
assert_eq!(list[1].name, "beta");
// User = 2
assert_eq!(list[2].name, "zebra");
}
#[test]
@@ -935,7 +940,7 @@ mod tests {
assert_eq!(reg.mcp_server_owner("my-server"), Some("mcp-plugin"));
}
// ── Combined disabled + untrusted scenarios ─────────────────
// Combined disabled + untrusted scenarios
#[test]
fn disabled_project_plugin_excluded_from_active_and_enabled() {
@@ -961,7 +966,7 @@ mod tests {
let bad = reg.get("bad-plugin").unwrap();
assert!(!bad.enabled);
// trusted is now propagated from discovery (was false for Project scope)
// trusted is propagated from discovery (was false for Project scope)
assert!(!bad.trusted);
}
@@ -1166,7 +1171,7 @@ mod tests {
assert_eq!(config.disabled.len(), 2);
}
// ── Security: trust propagation from discovery ──────────────
// Security: trust propagation from discovery
#[test]
fn untrusted_project_plugin_excluded_from_active_even_when_enabled() {
@@ -1174,12 +1179,14 @@ mod tests {
// pre-populated enabledPlugins) but NOT trusted. It must NOT
// appear in active_plugins() so its hooks never fire.
let plugins = vec![
make_discovered("malicious", PluginScope::Project, false), // untrusted
// untrusted
make_discovered("malicious", PluginScope::Project, false),
];
let reg = PluginRegistry::from_discovered(
plugins,
&[],
&["malicious".to_string()], // attacker got it into enabled list
// attacker got it into enabled list
&["malicious".to_string()],
);
// Plugin is enabled but not trusted
@@ -129,7 +129,8 @@ impl TrustStore {
})?;
if !self.trusted.remove(&canonical) {
return Ok(()); // wasn't trusted
// wasn't trusted
return Ok(());
}
// Rewrite the entire file without the revoked path
@@ -176,7 +177,7 @@ impl TrustStore {
}
}
// ── Internal ──────────────────────────────────────────────────────
// Internal
fn read_trust_file(path: &Path) -> HashSet<PathBuf> {
let file = match std::fs::File::open(path) {
@@ -208,7 +209,7 @@ impl TrustStore {
}
}
// ── Errors ────────────────────────────────────────────────────────────
// Errors
#[derive(Debug, thiserror::Error)]
pub enum TrustError {
@@ -320,7 +321,8 @@ mod tests {
// This test checks the logic but can't easily mock $HOME.
// We verify the function exists and returns a boolean.
let result = TrustStore::is_config_path_auto_trusted(Path::new("/nonexistent/path"));
assert!(!result); // nonexistent path can't be canonicalized
// nonexistent path can't be canonicalized
assert!(!result);
}
#[test]
@@ -239,7 +239,7 @@ mod tests {
git2::Repository::init(path).unwrap();
}
// ── find_agent_files unit tests ─────────────────────────────────
// find_agent_files unit tests
#[test]
fn find_agent_files_finds_agents_md() {
@@ -320,7 +320,7 @@ mod tests {
assert!(files[1].to_string_lossy().contains("style.md"));
}
// ── format_agents_md_section tests ──────────────────────────────
// format_agents_md_section tests
#[test]
fn format_agents_md_section_empty_returns_none() {
@@ -370,7 +370,7 @@ mod tests {
);
}
// ── Feature 2: Workspace user AGENTS.md via read_agents_config ───
// Feature 2: Workspace user AGENTS.md via read_agents_config
#[tokio::test]
async fn read_agents_config_includes_workspace_user_agents_md() {
@@ -537,7 +537,7 @@ mod tests {
assert!(!section.contains("globs:"));
}
// ── .claude/CLAUDE.md integration tests ─────────────────────────
// .claude/CLAUDE.md integration tests
#[tokio::test]
async fn read_agents_config_discovers_claude_subdir_claude_md() {
@@ -224,9 +224,9 @@ impl PromptContext {
}
/// Format the personas section content.
///
/// Always returns `None` — the `persona` parameter has been removed
/// from the task tool input, so persona summaries are no longer
/// injected into the conversation.
/// Always returns `None` — the task tool input carries no `persona`
/// parameter, so persona summaries are never injected into the
/// conversation.
pub fn format_personas_section(&self) -> Option<String> {
None
}
@@ -4,7 +4,6 @@ use ignore::gitignore::{Gitignore, GitignoreBuilder};
use std::path::{Path, PathBuf};
pub fn build_gitignore(repo_root: Option<&Path>) -> Option<Gitignore> {
// No repo root → no gitignore rules to apply.
let root = repo_root?;
let mut builder = GitignoreBuilder::new(root);
+25 -22
View File
@@ -726,7 +726,7 @@ mod tests {
fs::write(dir.join("SKILL.md"), content).unwrap();
}
// ── Server-synced skills (injected server_skill_dirs) ────────────────
// Server-synced skills (injected server_skill_dirs)
#[tokio::test]
async fn server_skills_discovered_and_shadowed_by_local() {
@@ -826,7 +826,7 @@ mod tests {
);
}
// ── Feature 3: Recursive skill reading ──────────────────────────────
// Feature 3: Recursive skill reading
#[test]
fn find_skill_paths_flat_layout() {
@@ -960,7 +960,7 @@ mod tests {
assert!(path_strs.iter().any(|p| p.contains("child/SKILL.md")));
}
// ── extract_first_paragraph ──────────────────────────────────────
// extract_first_paragraph
#[test]
fn first_paragraph_simple() {
@@ -1001,7 +1001,7 @@ mod tests {
assert!(extract_first_paragraph(body).is_none());
}
// ── UTF-8 safe body truncation ──────────────────────────────────
// UTF-8 safe body truncation
#[test]
fn description_fallback_does_not_panic_on_multibyte_boundary() {
@@ -1012,10 +1012,12 @@ mod tests {
// Strategy: fill with ASCII up to near the limit, then pack 4-byte
// emoji right at the boundary.
let prefix = "# Heading\n\n";
let filler_len = MAX_BODY_PEEK_BYTES - prefix.len() - 4; // leave room for emoji at boundary
// leave room for emoji at boundary
let filler_len = MAX_BODY_PEEK_BYTES - prefix.len() - 4;
let filler = "a".repeat(filler_len);
// Each emoji is 4 bytes. Place several so one straddles the 2048 mark.
let emoji_run = "\u{1F600}".repeat(10); // 40 bytes of emoji
// 40 bytes of emoji
let emoji_run = "\u{1F600}".repeat(10);
let body = format!("{prefix}{filler}{emoji_run}");
assert!(body.len() > MAX_BODY_PEEK_BYTES, "body must exceed limit");
@@ -1041,7 +1043,8 @@ mod tests {
// Body (after frontmatter): heading + paragraph with multibyte chars
// exceeding 2048 bytes.
let long_paragraph = "\u{00E9}".repeat(MAX_BODY_PEEK_BYTES); // 2-byte chars
// 2-byte chars
let long_paragraph = "\u{00E9}".repeat(MAX_BODY_PEEK_BYTES);
let content = format!("---\nname: emoji-skill\n---\n# Test\n\n{long_paragraph}\n");
fs::write(skill_dir.join("SKILL.md"), &content).unwrap();
@@ -1055,7 +1058,7 @@ mod tests {
);
}
// ── Frontmatter parsing (existing coverage + regression) ─────────
// Frontmatter parsing (existing coverage + regression)
#[test]
fn parse_valid_frontmatter() {
@@ -1122,7 +1125,7 @@ mod tests {
assert!(parsed.effort.is_none());
}
// ── agentskills.io spec parity ────────────────────────────────
// agentskills.io spec parity
#[test]
fn parse_license_and_compatibility() {
@@ -1296,7 +1299,7 @@ mod tests {
));
}
// ── Feature 1: Workspace user skills via list_skills ─────────────
// Feature 1: Workspace user skills via list_skills
/// Helper: initialize a bare git repo at `path` so git2::Repository::discover works.
fn init_git_repo(path: &Path) {
@@ -1423,7 +1426,7 @@ mod tests {
);
}
// ── collect_config_skills ────────────────────────────────────────
// collect_config_skills
#[test]
fn collect_config_skills_from_directory() {
@@ -1530,7 +1533,7 @@ mod tests {
}
}
// ── filter_skills ────────────────────────────────────────────────
// filter_skills
fn make_skill(name: &str, path: &str) -> SkillInfo {
SkillInfo {
@@ -1622,7 +1625,7 @@ mod tests {
assert_eq!(skills[0].plugin_name.as_deref(), Some("plugin-dev"));
}
// ── Manifest `skills` entries pointing directly at skill dirs ──
// Manifest `skills` entries pointing directly at skill dirs
fn make_registry_with_skill_dirs(
name: &str,
@@ -2006,11 +2009,11 @@ mod tests {
);
}
// discover_skills_for_paths and dedup_by_canonical_path tests removed --
// these functions now live in kigi-tools::implementations::skills::discovery
// and kigi-tools::types::skill_discovery_tracker, tested there.
// discover_skills_for_paths and dedup_by_canonical_path live in
// kigi-tools::implementations::skills::discovery and
// kigi-tools::types::skill_discovery_tracker, and are tested there.
// ── Disabled skills marking ─────────────────────────────────────
// Disabled skills marking
#[tokio::test]
async fn disabled_config_marks_skill_enabled_false() {
@@ -2091,7 +2094,7 @@ mod tests {
);
}
// ── Bundled skills discovery ─────────────────────────────────────
// Bundled skills discovery
#[tokio::test]
async fn bundled_skills_are_discovered() {
@@ -2180,7 +2183,7 @@ mod tests {
);
}
// ── Command file discovery ────────────────────────────────────────
// Command file discovery
/// Regression: project `.claude/commands` often sits under a full `.claude/**`
/// gitignore with only `!.claude/skills/**` re-included (local-only vendor
@@ -2312,7 +2315,7 @@ mod tests {
assert!(deploy[0].path.contains("SKILL.md"));
}
// ── Plugin skill identity ─────────────────────────────
// Plugin skill identity
fn min_plugin(name: &str) -> crate::plugins::LoadedPlugin {
use crate::plugins::discovery::PluginId;
@@ -2430,7 +2433,7 @@ mod tests {
);
}
// ── collect_skill_config_dirs vendor gating ────────────
// collect_skill_config_dirs vendor gating
#[test]
fn collect_skill_config_dirs_gates_vendor_dirs() {
@@ -2461,7 +2464,7 @@ mod tests {
assert!(ends_with(&dirs, ".kigi"), "kigi must remain: {dirs:?}");
}
// ── Same-scope frontmatter-name collisions (copied skill dirs) ──────
// Same-scope frontmatter-name collisions (copied skill dirs)
fn named_skill(name: &str, path: &str, scope: SkillScope) -> SkillInfo {
SkillInfo {
@@ -1,26 +1,10 @@
//! System prompts for built-in subagent profiles.
//!
//!
//! ## Tool name resolution
//!
//! All tool names in these prompts use the `${{ tools.by_kind.* }}` template
//! syntax from the `TemplateRenderer`. When the prompt is rendered via
//! `PromptContext::render()` → `ToolBridge::render_prompt()`, MiniJinja
//! resolves each variable to the current session's tool names.
//!
//! This means:
//! - Tool names are NEVER hardcoded — they adapt to name overrides and
//! alternate tool namespaces
//! - If a tool kind is absent from the renderer's context, MiniJinja
//! resolves it to an empty string (templates can also use
//! `${%- if tools.by_kind.X %}` conditionals to hide entire sections)
//!
//! Tool-kind mapping (common names → ToolKind):
//! Read → `${{ tools.by_kind.read }}`
//! Write/Edit → `${{ tools.by_kind.edit }}`
//! Glob → `${{ tools.by_kind.list }}`
//! Grep → `${{ tools.by_kind.search }}`
//! Bash → `${{ tools.by_kind.execute }}`
//! WebSearch → `${{ tools.by_kind.web_search }}`
//! Tool names inside these prompts are never hardcoded: they are
//! `${{ tools.by_kind.* }}` template variables that MiniJinja resolves to the
//! session's actual tool names during `ToolBridge::render_prompt()`, so they
//! follow name overrides and alternate namespaces. A kind that is absent from
//! the renderer context resolves to an empty string, which is why prompts guard
//! whole sections with `${%- if tools.by_kind.X %}`.
pub use kigi_tool_types::{EXPLORE_PROMPT, GENERAL_PURPOSE_PROMPT, PLAN_PROMPT};
@@ -149,7 +149,7 @@ mod tests {
.expect("codex template render failed")
}
// ── Variable substitution ───────────────────────────────────────
// Variable substitution
#[test]
fn test_variable_substitution_tool_kind() {
@@ -171,7 +171,7 @@ mod tests {
assert_eq!(result, "OS: macos, Shell: /bin/zsh");
}
// ── Conditionals ────────────────────────────────────────────────
// Conditionals
#[test]
fn test_conditional_tool_present() {
@@ -205,7 +205,7 @@ mod tests {
assert_eq!(result, "Use {{ literal_braces }} in prose.");
}
// ── Tool name overrides ─────────────────────────────────────────
// Tool name overrides
#[test]
fn test_tool_name_override() {
@@ -225,7 +225,7 @@ mod tests {
assert_eq!(result, "Use view_file and Edit.");
}
// ── Base template rendering ─────────────────────────────────────
// Base template rendering
#[test]
fn test_base_template_renders() {
@@ -355,7 +355,7 @@ mod tests {
);
}
// ── Required sections regression ────────────────────────────────
// Required sections regression
#[test]
fn test_base_template_contains_required_sections() {
@@ -380,7 +380,7 @@ mod tests {
);
}
// ── Mid-session mode switching ──────────────────────────────────
// Mid-session mode switching
#[test]
fn test_mid_session_switch_concise_to_full() {
@@ -430,7 +430,7 @@ mod tests {
);
}
// ── Determinism ─────────────────────────────────────────────────
// Determinism
#[test]
fn test_prompt_deterministic_across_renders() {
@@ -451,7 +451,7 @@ mod tests {
assert_eq!(a, b, "Full mode rendering must be deterministic");
}
// ── Disabled tools ──────────────────────────────────────────────
// Disabled tools
#[test]
fn test_disabled_tools_omit_sections() {
@@ -469,11 +469,11 @@ mod tests {
);
}
// ── Memory section ──────────────────────────────────────────────
// Memory section
#[test]
fn test_memory_enabled_does_not_render_memory_section() {
// The <memory> section was removed from the minimal base prompt.
// The <memory> section is absent from the minimal base prompt.
// Even when the memory tools are registered AND memory_enabled=true,
// the trimmed template must not render a memory section. (Complements
// test_memory_disabled_omits_memory_section, which covers the default.)
@@ -514,7 +514,7 @@ mod tests {
);
}
// ── Web search disabled ─────────────────────────────────────────
// Web search disabled
#[test]
fn test_web_search_disabled_renders_without_crash() {
@@ -534,7 +534,7 @@ mod tests {
);
}
// ── Apply-patch template rendering ───────────────────────────────────
// Apply-patch template rendering
#[test]
fn test_apply_patch_template_renders() {
@@ -634,9 +634,9 @@ mod tests {
assert_eq!(a, b, "Subagent template rendering must be deterministic");
}
// ── Task completion discipline ─────────────────────────────────
// Task completion discipline
//
// The `<task_completion_discipline>` block was removed from both
// The `<task_completion_discipline>` block is absent from both
// base and subagent templates. These tests pin the deletion so the
// block doesn't accidentally come back, and so the runtime TodoGate
// doesn't start firing reminders that reference a non-existent
@@ -681,7 +681,7 @@ mod tests {
assert_template_size_under(&prompt, "subagent");
}
// ── Guard invariant ─────────────────────────────────────────────
// Guard invariant
// Every `${{ tools.by_kind.X }}` must sit inside a `${%- if ... %}`
// whose condition requires X (contains `tools.by_kind.X` at a word
// boundary, with no top-level ` or `). If violated, X could render
@@ -770,12 +770,12 @@ mod tests {
assert_guards(&apply_patch_template(), "apply_patch_prompt.md");
}
// ── Combination sweep ───────────────────────────────────────────
// Combination sweep
// Belt-and-braces: renders the base template across tool-kind subsets
// and asserts no raw template tokens leak. The static guard test above
// is the authoritative check; this one just catches syntax drift.
// ── is_non_interactive gating ──────────────────────────────────
// is_non_interactive gating
// Headless / SDK / stdio / generic-ACP sessions have no human typing
// into a TUI prompt, so the `! <command>` shell-prefix tip and the
// `<user_guide>` TUI pointer are noise. Those sections must drop out
@@ -783,7 +783,7 @@ mod tests {
#[test]
fn interactive_renders_shell_prefix_tip_and_user_guide() {
// The `! <command>` shell-prefix tip was removed from the minimal
// The `! <command>` shell-prefix tip is absent from the minimal
// prompt. The <user_guide> block still renders for interactive
// sessions only, so that's what we assert here.
let mut p = default_placeholders();
@@ -334,7 +334,7 @@ mod tests {
assert_eq!(original, loaded);
}
}
/// A status under the cap passes through unchanged (trim is a no-op for
/// A status under the cap passes through `unchanged` (trim is a no-op for
/// real `git status --short --branch` output, which starts with `##`).
#[test]
fn normalize_git_status_passthrough_under_limit() {
@@ -48,7 +48,7 @@ mod tests {
use super::*;
use std::fs;
// ── resolve_workspace_user_dir (pure, no env vars) ───────────────
// resolve_workspace_user_dir (pure, no env vars)
#[test]
fn resolve_returns_none_for_empty_root() {
@@ -126,7 +126,7 @@ mod tests {
assert_eq!(result, Some(user_dir));
}
// ── workspace_user_relpath ───────────────────────────────────────
// workspace_user_relpath
#[test]
fn bare_username_is_nested_under_x() {
+30 -53
View File
@@ -1,10 +1,8 @@
//! Shared git-repo dir-chain primitive.
//!
//! One `git2` discovery + one cwd→root walk, reused across the many repo-local
//! config marker checks the folder-trust gate runs back-to-back. Lives in its
//! own module (rather than `discovery`) because it is a generic repo-walk
//! primitive consumed cross-crate by `kigi-workspace`, not agent-definition
//! discovery.
//! Lives in its own module rather than `discovery` because it is a generic
//! repo-walk primitive consumed cross-crate by `kigi-workspace`, not
//! agent-definition discovery.
use std::path::{Path, PathBuf};
@@ -14,42 +12,28 @@ use std::path::{Path, PathBuf};
///
/// The folder-trust gate's `repo_configs_present` probes a dozen repo-local
/// code-exec markers (`.mcp.json`, `.kigi/config.toml`, `.claude/settings.json`,
/// project plugin/agent dirs, …) back-to-back on the agent startup path. Each
/// marker walker used to run its own `discover` + cwd→root walk; sharing one
/// `RepoDirChain` collapses that to a single traversal (each redundant syscall
/// is taxed 10-100x on Windows, and on a non-git dir each `discover` walks to
/// the filesystem root). Both the gate and the real loaders consume the same
/// chain via `*_in` walker variants, so detection can't drift from loading.
///
/// The public cwd-taking delegators (`find_project_configs`,
/// `project_plugin_dirs`, `project_agent_dirs`, …) now resolve through this
/// chain too, so their non-gate callers (config watcher, reloader, the mcp/
/// config loaders, inspect, upload, mcp_doctor) gain the per-level canonicalize
/// below. That is deliberate: all those callers are cold (startup / file-change /
/// session-setup / manual commands), never per-keystroke, and the canonical stop
/// is strictly more correct.
/// project plugin/agent dirs, …) back-to-back on the agent startup path, so a
/// per-walker discovery + walk is a real cost: each redundant syscall is taxed
/// 10-100x on Windows, and on a non-git dir each `discover` walks to the
/// filesystem root. Both the gate and the real loaders consume the same chain
/// via `*_in` walker variants, so detection can't drift from loading.
///
/// Outside a git repo `git_root` is `None` and `dirs` is just `[cwd]`, matching
/// every walker's no-repo branch (probe `cwd` only).
#[derive(Debug, Clone)]
pub struct RepoDirChain {
/// Git worktree root (`workdir`), or `None` when `cwd` is not inside a repo.
pub git_root: Option<PathBuf>,
/// `cwd` up to and including `git_root`, cwd-first (`[cwd]` with no repo).
pub dirs: Vec<PathBuf>,
}
impl RepoDirChain {
/// Resolve the chain for `cwd`: ONE `git2` discovery + ONE upward walk.
pub fn resolve(cwd: &Path) -> Self {
let git_root = git2::Repository::discover(cwd)
.ok()
.and_then(|repo| repo.workdir().map(|p| p.to_path_buf()))
// Home-is-a-git-repo (dotfiles in $HOME): a discovery that walks up
// to $HOME must NOT treat the whole home subtree as one repo, or
// home-level `.kigi`/`.mcp.json`/plugins would look repo-local. Drop
// it so cwd is handled as no-repo (probe cwd only). Home is compared
// canonically to match the symlink handling in the walk below.
// Dotfiles in $HOME make home itself a repo; treating that subtree
// as repo-local would promote home-level `.kigi`/`.mcp.json`/plugins
// to project config. Dropping the root makes cwd behave as no-repo.
.filter(|root| !is_home_dir(root));
let mut dirs = Vec::new();
@@ -57,11 +41,10 @@ impl RepoDirChain {
// Canonicalize only for the stop test so a symlinked cwd/ancestor
// still halts AT the worktree root instead of over-walking to the
// filesystem root; pushed dirs keep their original spelling (callers
// `join` markers onto them, which resolve the same either way). The
// per-level canonicalize is required to stop at root through a
// symlinked ancestor while keeping raw spelling — do NOT reduce to a
// 2-call `starts_with` variant (it would mis-handle a mid-chain
// absolute symlink and reintroduce the over-walk).
// `join` markers onto them, which resolve the same either way).
// Canonicalizing per level is what makes that stop reliable — a
// 2-call `starts_with` variant mis-handles a mid-chain absolute
// symlink and over-walks.
let root_canonical = dunce::canonicalize(root).unwrap_or_else(|_| root.clone());
let mut current = Some(cwd.to_path_buf());
while let Some(dir) = current {
@@ -81,9 +64,9 @@ impl RepoDirChain {
}
}
/// Whether `path` canonicalizes to the user's home directory. Local (not reused
/// from `kigi-workspace`, which depends on THIS crate) to keep the dep edge
/// one-way; backs the home-is-dotfiles guard in [`RepoDirChain::resolve`].
/// Whether `path` canonicalizes to the user's home directory. Duplicated here
/// instead of reused from `kigi-workspace`, which depends on THIS crate, to keep
/// the dep edge one-way.
fn is_home_dir(path: &Path) -> bool {
let Some(home) = dirs::home_dir() else {
return false;
@@ -94,8 +77,7 @@ fn is_home_dir(path: &Path) -> bool {
/// Existing `<dir>/<subdir>` directories under each dir of a precomputed
/// cwd→git-root chain ([`RepoDirChain::dirs`]), in chain order (cwd-first, then
/// each `subdirs` entry in order). Shared body for the project plugin/agent dir
/// walkers so the byte-identical double-loop lives in one place.
/// each `subdirs` entry in order).
pub(crate) fn existing_subdirs_along(chain_dirs: &[PathBuf], subdirs: &[&str]) -> Vec<PathBuf> {
let mut found = Vec::new();
for dir in chain_dirs {
@@ -114,8 +96,8 @@ mod tests {
use super::*;
use serial_test::serial;
/// RAII guard: set an env var, restore the prior value (or unset) on drop,
/// so a test never leaves process-global env pointing at a dropped tempdir.
/// Restores the prior value (or unsets) on drop, so a test never leaves
/// process-global env pointing at a dropped tempdir.
struct EnvVarGuard {
key: &'static str,
prev: Option<std::ffi::OsString>,
@@ -140,8 +122,6 @@ mod tests {
#[test]
fn resolve_in_repo_yields_cwd_to_root_chain() {
// A git-init'd tmp with a 2-deep subdir: the chain is cwd→root inclusive,
// cwd-first, in the dirs' original spelling, and `git_root` is the root.
let tmp = tempfile::tempdir().unwrap();
git2::Repository::init(tmp.path()).unwrap();
let nested = tmp.path().join("a").join("b");
@@ -156,8 +136,8 @@ mod tests {
tmp.path().to_path_buf(),
]
);
// `git_root` is the canonical worktree root (git2's `workdir`); compare by
// canonical form so a `/tmp`→`/private/tmp` symlink doesn't fail the test.
// git2's `workdir` is canonical, so compare canonically or a
// `/tmp`→`/private/tmp` symlink fails the test.
let root = chain.git_root.expect("inside a repo");
assert_eq!(
dunce::canonicalize(&root).unwrap(),
@@ -167,10 +147,9 @@ mod tests {
#[test]
fn resolve_outside_repo_is_cwd_only() {
// A non-git tmp: no discovery hit, so the chain is just `[cwd]` and there
// is no git root. Only assert the no-repo shape when the temp dir is
// genuinely outside any repo (a dev/CI checkout may place $TMPDIR inside
// a larger git worktree).
// Only assert the no-repo shape when the temp dir is genuinely outside
// any repo: a dev/CI checkout may place $TMPDIR inside a larger git
// worktree.
let tmp = tempfile::tempdir().unwrap();
let plain = tmp.path().join("plain");
std::fs::create_dir_all(&plain).unwrap();
@@ -184,10 +163,8 @@ mod tests {
#[test]
#[serial(home_env)]
fn resolve_treats_home_git_repo_as_no_repo() {
// Home-is-a-git-repo (dotfiles in $HOME): discovery walks up to $HOME,
// but the guard drops that root so a subdir resolves as no-repo (probe
// cwd only) instead of spanning the whole home subtree. $HOME is guarded
// (dirs::home_dir reads it) and canonicalized to match the guard.
// $HOME is process-global (`dirs::home_dir` reads it) so it needs the
// guard, and canonicalized to match the comparison in `is_home_dir`.
let tmp = tempfile::tempdir().unwrap();
let home = dunce::canonicalize(tmp.path()).unwrap();
git2::Repository::init(&home).unwrap();
@@ -203,8 +180,8 @@ mod tests {
#[test]
#[serial(home_env)]
fn resolve_keeps_non_home_git_root() {
// The guard is home-EXACT: a git root that is NOT $HOME still resolves
// normally (no over-trigger), so $HOME points at an unrelated dir here.
// The guard is home-EXACT, so $HOME points at an unrelated dir here to
// prove a non-home git root still resolves normally.
let home = tempfile::tempdir().unwrap();
let _home_guard = EnvVarGuard::set("HOME", home.path());
let repo = tempfile::tempdir().unwrap();
@@ -1,22 +1,14 @@
//! Reminder policy — wraps kigi-tools reminder config.
/// Default per-prompt fire cap for the runtime turn-end TodoGate. Used
/// only as the default for `TodoGateConfig`; the runtime consumer reads
/// the live value from `ReminderPolicy.todo_gate.max_fires_per_prompt`,
/// so this constant is NOT a hardcoded cap.
/// Seeds `TodoGateConfig::max_fires_per_prompt`; the gate reads the live value
/// from `ReminderPolicy.todo_gate`, never this constant.
pub const DEFAULT_TODO_GATE_MAX_FIRES: u32 = 2;
/// Session-level system reminder policy.
///
/// Controls whether system reminders are enabled and configures
/// the TodoNudge and TodoGate behavior.
#[derive(Debug, Clone)]
pub struct ReminderPolicy {
/// Whether system reminders are enabled at all.
pub enabled: bool,
/// Configuration for the periodic TodoWrite nudge reminder.
pub todo_nudge: TodoNudgeConfig,
/// Configuration for the runtime turn-end TodoGate.
pub todo_gate: TodoGateConfig,
}
@@ -30,17 +22,13 @@ impl Default for ReminderPolicy {
}
}
/// Configuration for the TodoWrite nudge reminder.
///
/// The system will remind the model to use `todo_write` when it
/// hasn't done so within a configurable number of turns.
/// Reminds the model to call `todo_write` once it has gone
/// `turns_since_todo_write` turns without one, then stays quiet for
/// `turns_between_reminders` turns.
#[derive(Debug, Clone)]
pub struct TodoNudgeConfig {
/// Whether the TodoNudge reminder is enabled.
pub enabled: bool,
/// Number of turns since last `todo_write` call before nudging.
pub turns_since_todo_write: u32,
/// Minimum turns between nudge reminders.
pub turns_between_reminders: u32,
}
@@ -54,24 +42,19 @@ impl Default for TodoNudgeConfig {
}
}
/// Configuration for the runtime turn-end TodoGate.
///
/// The gate inspects `TodoState` after every content-only assistant
/// message and forces another turn via `<system-reminder>` injection
/// if pending/unbacked-in-progress todos remain — see
/// Turn-end gate: inspects `TodoState` after every content-only assistant
/// message and forces another turn via `<system-reminder>` injection if
/// pending/unbacked-in-progress todos remain — see
/// `kigi-shell::session::acp_session::evaluate_todo_gate`.
///
/// **Disabled by default.** Operators opt in via the remote
/// `todo_gate_enabled = true` remote settings key, or via the
/// `--todo-gate` CLI flag (session-scoped force-enable, highest
/// precedence).
/// **Disabled by default.** Operators opt in via the `todo_gate_enabled`
/// remote settings key, or via the `--todo-gate` CLI flag (session-scoped
/// force-enable, highest precedence).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct TodoGateConfig {
/// Whether the gate runs at all.
pub enabled: bool,
/// Hard cap on how many times the gate may fire per user prompt
/// before the next turn is allowed to end with `TurnOutcome::Completed`.
/// Bounds the worst-case extra inference cost.
/// Past this many fires per user prompt the next turn is allowed to end
/// with `TurnOutcome::Completed`, bounding worst-case extra inference cost.
pub max_fires_per_prompt: u32,
}
@@ -108,16 +91,11 @@ mod tests {
"TodoGate ships disabled; remote/local opt-in required"
);
assert_eq!(policy.todo_gate.max_fires_per_prompt, 2);
// The two reminder mechanisms are independent — flipping one
// must not change the other (regression guard).
assert!(policy.todo_nudge.enabled);
}
#[test]
fn todo_gate_enable_does_not_disturb_nudge() {
// Remote opt-in (or `[reminder.todo_gate] enabled = true` local
// config) flips the gate to on without touching the periodic
// TodoNudge as a side-effect.
let mut policy = ReminderPolicy::default();
policy.todo_gate.enabled = true;
assert!(policy.todo_gate.enabled);
+24 -32
View File
@@ -7,17 +7,16 @@ use reqwest::RequestBuilder;
use crate::visibility::HttpAuth;
/// Snapshot of the currently effective credentials. Used by callers
/// that build their own header maps (the OTel OTLP exporter) or that
/// need the bearer prefix for 401-attribution telemetry.
/// Snapshot of the currently effective credentials, for callers that build
/// their own header maps (the OTel OTLP exporter) or that need the bearer
/// prefix for 401-attribution telemetry.
#[derive(Clone, Debug, Default)]
pub struct CredentialSnapshot {
/// Bearer token. `None` when no auth is configured (CI / `--api-key` headless).
/// `None` when no auth is configured (CI / `--api-key` headless).
pub token: Option<String>,
/// User identifier matching the bearer token's owner. `None` when no auth
/// is configured or when the underlying provider has no concept of user
/// identity (`StaticAuthCredentialProvider`). Read by the OTel layer to
/// populate the `user.id` resource attribute.
/// Owner of `token`. `None` when no auth is configured or when the
/// provider has no concept of user identity
/// (`StaticAuthCredentialProvider`).
pub user_id: Option<String>,
/// `uuidv5(NAMESPACE_OID, deployment_key)`, set only for deployment-key auth.
pub deployment_id: Option<String>,
@@ -29,49 +28,42 @@ pub struct CredentialSnapshot {
///
/// Supertrait of `HttpAuth` so a single impl satisfies both this trait
/// (refresh-aware snapshot + 401 recovery) and the visibility seam
/// (header construction). Callers add headers via `HttpAuth::apply`.
/// (header construction).
#[async_trait::async_trait]
pub trait AuthCredentialProvider: HttpAuth + Send + Sync + 'static {
/// Return the current credential snapshot. Implementations should
/// issue a cheap disk re-read (`AuthManager::refresh`) before
/// snapshotting so callers see updates from sibling processes
/// (`kigi-desktop`, `kigi login`). The `token` field MUST mirror
/// the bearer that `HttpAuth::apply` would send on the wire so
/// 401-attribution prefixes match the actual request.
/// Implementations should issue a cheap disk re-read
/// (`AuthManager::refresh`) before snapshotting so callers see updates
/// from sibling processes (`kigi-desktop`, `kigi login`). The `token`
/// field MUST mirror the bearer that `HttpAuth::apply` would send on the
/// wire so 401-attribution prefixes match the actual request.
fn snapshot(&self) -> CredentialSnapshot;
/// Attempt to obtain a fresh token. Returns `true` if a different
/// token was obtained -- caller should retry the failed request once.
/// Returns `false` if no refresher is configured or refresh failed.
/// `true` if a different token was obtained, meaning the caller should
/// retry the failed request once; `false` if no refresher is configured
/// or the refresh failed.
async fn refresh_after_unauthorized(&self) -> bool;
/// Whether the provider holds a credential worth a real outbound attempt —
/// an unexpired token (in memory or on disk), or a static key. Default
/// `true` always attempts.
/// an unexpired token (in memory or on disk), or a static key.
fn has_usable_credential(&self) -> bool {
true
}
}
/// Static credential provider. Used by tests and by callers that pass a
/// raw `&str` token with no `AuthManager` available.
/// Non-refreshing provider for tests and for callers that pass a raw `&str`
/// token with no `AuthManager` available.
///
/// `apply()` delegates to the underlying `HttpAuth::apply()`.
/// `refresh_after_unauthorized()` always returns `false`.
///
/// `bearer` is the wire bearer the inner `HttpAuth` will send in the
/// `Authorization` header. Stored alongside the inner so `snapshot().token`
/// returns the same prefix that goes out on the wire (used by
/// 401-attribution telemetry). `None` when no bearer is configured.
/// `bearer` duplicates whatever `inner` stamps into the `Authorization`
/// header; it exists so `snapshot().token` reports the same prefix that goes
/// out on the wire, which 401-attribution telemetry relies on.
pub struct StaticAuthCredentialProvider {
inner: Box<dyn HttpAuth>,
bearer: Option<String>,
}
impl StaticAuthCredentialProvider {
/// Wrap `inner` so callers see it as an `AuthCredentialProvider`. Pass
/// the bearer token that `inner.apply()` will send in the `Authorization`
/// header so `snapshot().token` reflects the wire bearer truthfully.
/// `bearer` must be the token `inner.apply()` sends, or `snapshot()` will
/// misreport the wire credential.
pub fn new(inner: Box<dyn HttpAuth>, bearer: Option<String>) -> Self {
Self { inner, bearer }
}
@@ -1,5 +1,4 @@
//! `reqwest-middleware` layer: stamps auth headers and retries on 401.
//! Gated behind the `middleware` cargo feature.
use std::sync::Arc;
@@ -52,6 +51,8 @@ impl Middleware for AuthRetryMiddleware {
if resp.status() != StatusCode::UNAUTHORIZED || self.max_retries == 0 {
return Ok(resp);
}
// Streaming bodies do not clone, so such requests cannot be replayed
// and the 401 stands.
let Some(backup) = backup else {
return Ok(resp);
};
@@ -154,7 +155,7 @@ mod tests {
m.assert_async().await;
}
/// Simulates a real auth manager: starts with stale token, refresh swaps to fresh.
/// Starts with a stale token; refresh swaps in the fresh one.
struct SimulatedAuthManager {
token: Mutex<Option<String>>,
fresh_token: String,
+4 -4
View File
@@ -1,7 +1,7 @@
/// Apply auth headers to outbound visibility requests.
/// Implemented by `kigi-shell::util::kigi_auth_credentials::KigiAuthCredentials`
/// to keep credential construction owned by shell while letting data-collector
/// build the request without reaching back into shell types.
/// Applies auth headers to outbound visibility requests. Implemented by
/// `kigi-shell::util::kigi_auth_credentials::KigiAuthCredentials`, keeping
/// credential construction owned by shell while data-collector builds the
/// request without reaching back into shell types.
pub trait HttpAuth: Send + Sync {
fn apply(&self, builder: reqwest::RequestBuilder, base_url: &str) -> reqwest::RequestBuilder;
}
+2 -2
View File
@@ -702,7 +702,7 @@ async fn run_agent_command(
}
}
}
// Fire-and-forget model-catalog warmup (nothing joins the handle now that
// Fire-and-forget model-catalog warmup (nothing joins the handle because
// the xAI settings fetch it used to carry is gone).
drop(kigi_shell::agent::models::start_early_prefetch(None));
kigi_shell::agent::mvp_agent::warm_async_http_client();
@@ -1966,7 +1966,7 @@ mod tests {
assert!(s.last_session_id.is_none());
}
/// An UNCONFIRMED `session/new` (leader died before its response) must not
/// be replayed — its id was never assigned — but previously loaded
/// be replayed — its id was never assigned — but earlier loaded
/// sessions still restore.
#[tokio::test]
async fn replay_after_unconfirmed_session_new_restores_prior_sessions() {
@@ -116,7 +116,7 @@ impl ChatStateActor {
/// Dispatch a command to the appropriate mutation or query handler.
fn handle_command(&mut self, cmd: ChatStateCommand) {
match cmd {
// ═══ Mutations ═══
// Mutations
ChatStateCommand::PushUserMessage { item } => {
self.push_user_message(item);
}
@@ -240,7 +240,7 @@ impl ChatStateActor {
self.repair_dangling_after_harness_halt(class);
}
// ═══ Queries ═══
// Queries
//
// Read queries are pure reads — repair only at write boundaries:
// `ChatState::new()` (startup) and `push_user_message()` (new turn).
@@ -318,7 +318,7 @@ impl ChatStateActor {
self.truncate_to_prompt_index(target_prompt_index);
self.state.turn_capture = None;
self.state.prompt_usage = None;
// `harness_trace_buffer` / `harness_trace_turns` intentionally
// `harness_trace_buffer` / `harness_trace_turns` deliberately
// survive a rewind: the goal planner / verifier subagents
// genuinely ran, so their sealed trace turns stay uploadable as
// siblings even when the live turn that triggered them is undone.
@@ -358,7 +358,7 @@ impl ChatStateActor {
let _ = reply.send(std::mem::take(&mut self.state.harness_trace_turns));
}
// ─── Narrow targeted queries ──────────────────────────────────
// Narrow targeted queries
ChatStateCommand::GetConversationLen { reply } => {
let _ = reply.send(self.get_conversation_len());
}
@@ -123,7 +123,7 @@ impl ChatStateActor {
.unwrap_or_default()
}
// ─── Narrow targeted queries ─────────────────────────────────────────────
// Narrow targeted queries
/// Return the number of items in the conversation.
pub(super) fn get_conversation_len(&self) -> usize {
@@ -148,9 +148,7 @@ impl ChatStateActor {
}
}
// ============================================================================
// Pruning (standalone functions, no actor state needed)
// ============================================================================
/// Check whether pruning should run based on context utilization.
///
@@ -208,9 +206,7 @@ pub(crate) fn prune_conversation(conversation: &mut [ConversationItem], config:
}
}
// ============================================================================
// Image size-gated compaction (request-copy only)
// ============================================================================
/// Replaces an inline image evicted to keep the request body under the proxy's
/// 50 MB limit. Phrased so the model treats the image as gone rather than
@@ -376,7 +372,7 @@ fn conversation_body_bytes(conversation: &[ConversationItem]) -> usize {
/// always retain the newest images, an image only transitions image →
/// placeholder as *newer/larger* payloads push the body past the limit, never
/// placeholder → image within a stable prefix. (Token compaction removes old
/// turns wholesale and can free room to restore a previously-evicted image,
/// turns wholesale and can free room to restore an earlier-evicted image,
/// but that already rewrites the prefix and invalidates the server-side prompt
/// cache, so the restore is free.)
///
@@ -450,19 +446,17 @@ pub(crate) fn compact_images_to_byte_budget(
}
}
// ============================================================================
// Memory reminder injection
// ============================================================================
use crate::types::MEMORY_CONTEXT_OPEN_TAG;
/// Upsert a memory reminder into the conversation's system message.
///
/// If the first item is a `System` message, any previously injected memory
/// reminder section is replaced in-place; otherwise the reminder is appended.
/// If the first item is a `System` message, any existing memory reminder
/// section is replaced in-place; otherwise the reminder is appended.
/// If no system message exists, a new `System` item is prepended.
///
/// Returns `true` when the conversation was changed.
/// Returns `true` when the conversation changed.
pub(super) fn inject_memory_reminder(items: &mut Vec<ConversationItem>, reminder: &str) -> bool {
let reminder = reminder.trim();
if reminder.is_empty() {
@@ -505,9 +499,7 @@ fn upsert_memory_reminder_text(system_prompt: &mut std::sync::Arc<str>, reminder
}
}
// ============================================================================
// String helpers
// ============================================================================
fn safe_char_slice(s: &str, start: usize, count: usize) -> String {
s.chars().skip(start).take(count).collect()
@@ -529,9 +521,12 @@ mod tests {
fn should_prune_gating() {
use std::num::NonZeroU64;
let cw = NonZeroU64::new(10000).unwrap();
assert!(!should_prune(1000, cw)); // 10%
assert!(should_prune(6000, cw)); // 60%
assert!(!should_prune(5000, cw)); // 50% exact (> not >=)
// 10%
assert!(!should_prune(1000, cw));
// 60%
assert!(should_prune(6000, cw));
// 50% exact (> not >=)
assert!(!should_prune(5000, cw));
}
#[test]
@@ -558,7 +553,8 @@ mod tests {
assert!(sys.content.contains("Remember: user likes rust"));
assert!(sys.content.starts_with("You are helpful."));
}
assert_eq!(items.len(), 2); // no new item added
// no new item added
assert_eq!(items.len(), 2);
}
#[test]
@@ -569,7 +565,7 @@ mod tests {
assert!(matches!(&items[0], ConversationItem::System(_)));
}
// -- image size-gated compaction tests --
// image size-gated compaction tests
/// A user message with a small fixed inline image.
fn user_with_image(text: &str) -> ConversationItem {
@@ -661,8 +657,10 @@ mod tests {
// dropping a *batch* of the oldest, not just the one image needed to
// clear the trigger. This is the hysteresis that keeps the prefix
// cache-warm for the following turns.
let img_bytes = 1_000_000usize; // ~1 MB url each
let n = (IMAGE_COMPACT_TRIGGER_BYTES / img_bytes) + 2; // body just over trigger
// ~1 MB url each
let img_bytes = 1_000_000usize;
// body just over trigger
let n = (IMAGE_COMPACT_TRIGGER_BYTES / img_bytes) + 2;
let mut conv: Vec<ConversationItem> = (0..n)
.map(|i| user_with_image_of_bytes(&format!("i{i}"), img_bytes))
.collect();
@@ -726,7 +724,7 @@ mod tests {
assert!(has_placeholder(&conv[0]));
}
// -- conversation_body_bytes tests --
// conversation_body_bytes tests
#[test]
fn conversation_body_bytes_empty_is_json_array() {
@@ -777,7 +775,7 @@ mod tests {
assert!(conversation_body_bytes(&conv) >= IMAGE_COMPACT_TRIGGER_BYTES);
}
// -- edge cases: exactness, boundaries, ordering --
// edge cases: exactness, boundaries, ordering
#[test]
fn body_bytes_parity_multi_image_unicode_escaping() {
@@ -137,7 +137,7 @@ pub(crate) struct ChatState {
/// Opaque credential secrets (api key, optional extra auth, client version).
/// Stored opaquely — the actor never interprets them.
pub credentials: Credentials,
/// Bytes/4 estimate of tokens added since the last `record_token_usage`.
/// Bytes/4 estimate of tokens accumulated since the last `record_token_usage`.
/// Used by `check_preflight_overflow` to detect context window overflows
/// between model responses.
pub estimated_tokens_since_model: u64,
@@ -304,7 +304,8 @@ mod tests {
fn new_state_has_correct_defaults() {
let state = ChatState::new(vec![], test_sampling_config());
assert_eq!(state.prompt_index, 0);
assert_eq!(state.total_tokens, 0); // empty conversation → 0
// empty conversation → 0
assert_eq!(state.total_tokens, 0);
assert!(state.conversation.is_empty());
assert!(state.agent_edited_paths.is_empty());
assert!(state.prompt_texts.is_empty());
@@ -333,7 +334,8 @@ mod tests {
ConversationItem::tool_result("call-1", "w".repeat(4000).as_str()),
];
let state = ChatState::new(items, test_sampling_config());
assert_eq!(state.total_tokens, 4000); // 4 * (4000/4)
// 4 * (4000/4)
assert_eq!(state.total_tokens, 4000);
}
#[test]
@@ -91,9 +91,7 @@ impl TestHarness {
}
}
// ============================================================================
// Lifecycle tests
// ============================================================================
#[tokio::test]
async fn actor_spawns_and_shuts_down_via_cancellation() {
@@ -121,9 +119,7 @@ async fn actor_shuts_down_when_all_handles_dropped() {
tokio::time::sleep(Duration::from_millis(50)).await;
}
// ============================================================================
// Mutation tests
// ============================================================================
#[tokio::test]
async fn push_user_message_appends_and_persists() {
@@ -319,9 +315,10 @@ async fn estimated_tokens_tracks_tool_result_delta() {
.push_tool_result(ConversationItem::tool_result("call-1", "x".repeat(4000)));
let estimated = h.handle.get_estimated_total_tokens().await;
assert_eq!(estimated, 101_000); // 100K model-reported + 1K delta
// 100K model-reported + 1K delta
assert_eq!(estimated, 101_000);
// model-reported total_tokens is unchanged
// model-reported total_tokens is `unchanged`
let actual = h.handle.get_total_tokens().await;
assert_eq!(actual, 100_000);
}
@@ -359,7 +356,7 @@ async fn estimated_tokens_tracks_synthetic_user_message_delta() {
"expected ~1.1M tokens estimated, got {estimated}",
);
// model-reported `total_tokens` is unchanged — only the delta moved.
// model-reported `total_tokens` is `unchanged` — only the delta moved.
assert_eq!(h.handle.get_total_tokens().await, 100_000);
}
@@ -451,7 +448,7 @@ async fn replace_conversation_persists_and_emits_reset() {
h.handle.push_user_message(ConversationItem::user("b"));
// Drain the two Message records
let _ = h.handle.get_conversation().await; // sync point
let _ = h.handle.get_conversation().await;
h.drain_persistence();
let new_items = vec![ConversationItem::system("compacted")];
@@ -721,9 +718,7 @@ async fn restore_snapshot_restores_all_fields() {
assert_eq!(tokens, 500);
}
// ============================================================================
// Query tests
// ============================================================================
#[tokio::test]
async fn get_conversation_returns_current_state() {
@@ -765,7 +760,8 @@ async fn replace_system_head_noop_when_head_matches_modulo_newline() {
ConversationItem::system("same\n"),
ConversationItem::user("hi"),
]);
let _ = h.drain_persistence(); // clear any seed writes
// clear any seed writes
let _ = h.drain_persistence();
let changed = h.handle.replace_system_head("same").await;
assert_eq!(
changed,
@@ -878,9 +874,7 @@ async fn check_auto_compact_triggers_at_threshold() {
assert_eq!(t.utilization_percent, 86);
}
// ============================================================================
// Edge-case / integration tests
// ============================================================================
#[tokio::test]
async fn record_agent_edited_path_deduplicates() {
@@ -974,19 +968,19 @@ async fn truncate_removes_items_after_target_prompt_index() {
// Build 3 turns: system + 3x (user + assistant)
h.handle.push_user_message(ConversationItem::system("sys"));
h.handle.push_user_message(ConversationItem::user("q1"));
h.handle.increment_prompt_index(); // 1
h.handle.increment_prompt_index();
h.handle.cache_prompt_text("q1".to_string());
h.handle
.push_assistant_response(ConversationItem::assistant("a1"));
h.handle.push_user_message(ConversationItem::user("q2"));
h.handle.increment_prompt_index(); // 2
h.handle.increment_prompt_index();
h.handle.cache_prompt_text("q2".to_string());
h.handle
.push_assistant_response(ConversationItem::assistant("a2"));
h.handle.push_user_message(ConversationItem::user("q3"));
h.handle.increment_prompt_index(); // 3
h.handle.increment_prompt_index();
h.handle.cache_prompt_text("q3".to_string());
h.handle
.push_assistant_response(ConversationItem::assistant("a3"));
@@ -1000,7 +994,8 @@ async fn truncate_removes_items_after_target_prompt_index() {
h.handle.truncate_to_prompt_index(1).await;
let conv = h.handle.get_conversation().await;
assert_eq!(conv.len(), 3); // sys + q1 + a1
// sys + q1 + a1
assert_eq!(conv.len(), 3);
let idx = h.handle.get_prompt_index().await;
assert_eq!(idx, 1);
@@ -1032,7 +1027,8 @@ async fn truncate_to_zero_keeps_only_system() {
h.handle.truncate_to_prompt_index(0).await;
let conv = h.handle.get_conversation().await;
assert_eq!(conv.len(), 1); // just "sys"
// just "sys"
assert_eq!(conv.len(), 1);
assert!(matches!(&conv[0], ConversationItem::System(_)));
assert_eq!(h.handle.get_prompt_index().await, 0);
}
@@ -1040,7 +1036,7 @@ async fn truncate_to_zero_keeps_only_system() {
#[tokio::test]
async fn truncate_is_noop_when_already_at_target() {
let mut h = TestHarness::new();
h.handle.increment_prompt_index(); // 1
h.handle.increment_prompt_index();
let _ = h.handle.get_prompt_index().await;
h.drain_events();
@@ -1056,9 +1052,7 @@ async fn truncate_is_noop_when_already_at_target() {
assert!(events.is_empty());
}
// ============================================================================
// Snapshot/restore comprehensive tests
// ============================================================================
#[tokio::test]
async fn snapshot_restore_preserves_all_fields() {
@@ -1139,9 +1133,7 @@ async fn with_initial_conversation_preserves_items() {
assert_eq!(conv.len(), 2);
}
// ============================================================================
// BuildConversationRequest tests
// ============================================================================
#[tokio::test]
async fn build_request_includes_all_messages() {
@@ -1234,7 +1226,8 @@ async fn build_request_injects_memory_when_no_system() {
.await
.unwrap();
assert_eq!(request.items.len(), 2); // new System + original User
// new System + original User
assert_eq!(request.items.len(), 2);
assert!(matches!(&request.items[0], ConversationItem::System(_)));
}
@@ -1341,11 +1334,12 @@ async fn build_request_does_not_mutate_actor_state() {
.await
.unwrap();
// Actor's own conversation should be unchanged
// Actor's own conversation should be `unchanged`
let conv = h.handle.get_conversation().await;
assert_eq!(conv.len(), 2);
if let ConversationItem::System(ref sys) = conv[0] {
assert_eq!(sys.content.as_ref(), "sys"); // no memory injected into original
// no memory injected into original
assert_eq!(sys.content.as_ref(), "sys");
}
}
@@ -1424,9 +1418,7 @@ async fn build_request_with_multiple_tool_calls_and_results() {
assert_eq!(request.items.len(), 6);
}
// ============================================================================
// Parallel tool calls with mixed accept/reject
// ============================================================================
/// Simulates the exact sequence that `kigi-shell`'s `execute_tool_calls`
/// produces when the model emits 3 parallel tool calls and:
@@ -1451,7 +1443,7 @@ async fn parallel_tool_calls_accept_first_reject_second_skip_third() {
let h = TestHarness::new();
// ── Turn setup ──────────────────────────────────────────────────────
// Turn setup
// System prompt
h.handle.push_user_message(ConversationItem::system(
"You are a helpful coding assistant.",
@@ -1464,7 +1456,7 @@ async fn parallel_tool_calls_accept_first_reject_second_skip_third() {
h.handle.increment_prompt_index();
// ── Model response: 3 parallel tool calls ───────────────────────────
// Model response: 3 parallel tool calls
// The model's single assistant message contains all 3 tool calls.
// In the real code, this is built from the streaming response and pushed
// via `push_assistant_response`.
@@ -1493,7 +1485,7 @@ async fn parallel_tool_calls_accept_first_reject_second_skip_third() {
});
h.handle.push_assistant_response(assistant_with_tools);
// ── Tool execution results (simulating execute_tool_calls) ──────────
// Tool execution results (simulating execute_tool_calls)
// Tool #1: read_file — user accepted, tool executed successfully
h.handle.push_tool_result(ConversationItem::tool_result(
@@ -1517,7 +1509,7 @@ async fn parallel_tool_calls_accept_first_reject_second_skip_third() {
"Tool execution cancelled due to earlier permission rejection for tool `run_terminal_cmd`",
));
// ── Verify the conversation state ───────────────────────────────────
// Verify the conversation state
let conv = h.handle.get_conversation().await;
// Expected: System + User + Assistant(3 calls) + 3 ToolResults = 6 items
@@ -1731,9 +1723,7 @@ async fn parallel_tool_calls_with_rejection_persists_all_items() {
);
}
// ============================================================================
// Race condition: cancellation mid-tool-execution → dangling calls on reload
// ============================================================================
/// Simulates the race condition where:
/// 1. Model emits 3 parallel tool calls (single assistant message)
@@ -1972,9 +1962,7 @@ async fn all_tool_calls_dangling_after_crash() {
assert_eq!(request.items.len(), 6);
}
// ============================================================================
// Live-session cancellation: user cancels mid-tool-execution (no restart)
// ============================================================================
/// Simulates an in-session abort where:
/// 1. Model emits 3 parallel tool calls → assistant pushed to conversation
@@ -1984,7 +1972,7 @@ async fn all_tool_calls_dangling_after_crash() {
///
/// This is different from the reload scenario: `ChatState::new` doesn't run
/// again because the actor is still alive. The fix is that `push_user_message`
/// now calls `repair_dangling_tool_calls` before appending the new user
/// calls `repair_dangling_tool_calls` before appending the new user
/// message, so the conversation is cleaned up in-place.
#[tokio::test]
async fn live_cancel_before_any_tool_execution_repairs_on_next_user_message() {
@@ -1992,14 +1980,14 @@ async fn live_cancel_before_any_tool_execution_repairs_on_next_user_message() {
let h = TestHarness::new();
// ── Turn 1: normal conversation ─────────────────────────────────────
// Turn 1: normal conversation
h.handle
.push_user_message(ConversationItem::system("You are a helpful assistant."));
h.handle.push_user_message(ConversationItem::user("Hello"));
h.handle
.push_assistant_response(ConversationItem::assistant("Hi! How can I help?"));
// ── Turn 2: model wants 3 tool calls, user cancels immediately ──────
// Turn 2: model wants 3 tool calls, user cancels immediately
h.handle
.push_user_message(ConversationItem::user("Read, edit, and test everything"));
@@ -2023,7 +2011,7 @@ async fn live_cancel_before_any_tool_execution_repairs_on_next_user_message() {
},
]));
// *** USER CANCELS HERE (Ctrl+C) ***
// USER CANCELS HERE (Ctrl+C)
// The tokio task is aborted. execute_tool_calls never ran.
// Zero ToolResult items pushed. The conversation has dangling calls.
@@ -2127,7 +2115,7 @@ async fn live_cancel_after_partial_tool_results_repairs_remaining() {
"file contents here",
));
// *** USER CANCELS HERE — tool #2 and #3 never executed ***
// USER CANCELS HERE — tool #2 and #3 never executed
// User types a new prompt
h.handle.push_user_message(ConversationItem::user(
@@ -2178,7 +2166,6 @@ async fn live_cancel_after_partial_tool_results_repairs_remaining() {
}
// Turn message capture tests
// ============================================================================
#[tokio::test]
async fn turn_capture_collects_all_message_types() {
@@ -2537,7 +2524,6 @@ async fn turn_capture_survives_integrity_repair_prefix_shrink() {
// Capture starts after the 7-item prefix: turn_start_offset == 7.
h.handle.begin_turn_capture();
// First turn item lands while the prefix duplicates are still present.
h.handle
.push_assistant_response(ConversationItem::assistant("turn-1"));
@@ -2636,9 +2622,7 @@ async fn turn_capture_survives_persisted_memory_reminder_prepend() {
));
}
// ============================================================================
// Narrow targeted query tests
// ============================================================================
#[tokio::test]
async fn get_conversation_len_empty() {
@@ -2795,7 +2779,7 @@ async fn get_conversation_item_at_does_not_mutate_state() {
assert_eq!(conv.len(), 2);
}
// ── Multimodal regression tests for get_first_user_text() ────────────────────
// Multimodal regression tests for get_first_user_text()
/// Confirms that `get_first_user_text()` returns `None` when the first content
/// part of the first user message is an image (not text). This preserves the
@@ -2805,7 +2789,6 @@ async fn get_first_user_text_image_first_returns_none() {
use kigi_sampling_types::{ContentPart, UserItem};
let h = TestHarness::new();
// First message: image-only user message (no text part)
h.handle.push_user_message(ConversationItem::User(UserItem {
content: vec![ContentPart::Image {
url: "data:image/png;base64,abc".into(),
@@ -2865,7 +2848,7 @@ async fn get_first_user_text_text_then_image_returns_text() {
assert_eq!(text.as_deref(), Some("look at this"));
}
// ── Tests for GetLastUserQueryText, GetConversationCounts, GetSystemMessage ───
// Tests for GetLastUserQueryText, GetConversationCounts, GetSystemMessage
#[tokio::test]
async fn get_last_user_query_text_empty_conversation() {
@@ -2935,18 +2918,17 @@ async fn get_system_message_returns_first_system() {
assert!(matches!(sys, ConversationItem::System(s) if s.content.as_ref() == "You are helpful."));
}
// ============================================================================
// Subagent bootstrap regression tests
//
// These verify that `replace_conversation` correctly syncs the system prompt
// into a ChatStateActor that was spawned before the prompt was built — the
// exact sequence used by `spawn_session_actor` for subagents.
// ============================================================================
#[tokio::test]
async fn fresh_subagent_bootstrap_has_system_message_after_replace() {
// Simulate a fresh (non-forked) subagent: actor starts with an empty conversation.
let h = TestHarness::new(); // spawns with vec![]
// spawns with vec![]
let h = TestHarness::new();
// At this point the actor has no system message, mirroring the bug.
assert!(h.handle.get_system_message().await.is_none());
@@ -3005,9 +2987,7 @@ async fn forked_subagent_bootstrap_replaces_parent_system_message() {
assert_eq!(conv.len(), 3);
}
// ============================================================================
// In-memory retained pruning tests (PR3)
// ============================================================================
/// Helper: push N complete turns (user + assistant + tool-result) so the
/// conversation grows to a predictable length.
@@ -3165,8 +3145,10 @@ async fn prune_retained_bounds_long_session_footprint() {
use crate::persistence::MockChatPersistence;
use crate::types::PruningConfig;
const TURNS: usize = 50; // enough turns to clear many old tool results
const CONTENT_LEN: usize = 50_000; // 50 KB per tool result
// enough turns to clear many old tool results
const TURNS: usize = 50;
// 50 KB per tool result
const CONTENT_LEN: usize = 50_000;
const PLACEHOLDER_LEN: usize = "[Tool result omitted — too old]".len();
let (mock, _rx) = MockChatPersistence::new();
@@ -3324,7 +3306,8 @@ async fn prune_retained_synthetic_user_does_not_advance_age() {
// Three real turns, each with a large tool result.
for i in 0..3usize {
handle.push_user_message(ConversationItem::user(format!("real q{i}")));
handle.increment_prompt_index(); // prompt_index = i+1
// prompt_index = i+1
handle.increment_prompt_index();
handle.push_assistant_response(ConversationItem::assistant(format!("a{i}")));
handle.push_tool_result(ConversationItem::tool_result(
format!("call_{i}"),
@@ -3339,7 +3322,8 @@ async fn prune_retained_synthetic_user_does_not_advance_age() {
// Fourth real turn starts: prompt_index → 4, pruning fires inside push_user_message.
handle.push_user_message(ConversationItem::user("real q3"));
handle.increment_prompt_index(); // prompt_index = 4
// prompt_index = 4
handle.increment_prompt_index();
// Sync
let conv = handle.get_conversation().await;
@@ -3616,7 +3600,6 @@ async fn context_window_downgrade_triggers_auto_compact() {
"api_backend must not change"
);
// Now auto-compact sees the 128k window and fires
let trigger = h.handle.check_auto_compact_needed(85).await;
assert!(
trigger.is_some(),
@@ -3633,14 +3616,13 @@ async fn context_window_downgrade_triggers_auto_compact() {
);
}
// ============================================================================
// KV Cache Prefix Stability Tests
//
// These test `build_conversation_request()` output prefix stability through
// the full pipeline -- pruning, memory injection, image pruning, snapshot
// restore. Prefix stability within a compaction epoch is the invariant that
// keeps the inference engine's prefix / KV cache hitting. The sibling-Reasoning refactor
// deleted the placeholder/splice machinery these tests previously had to work
// deleted the placeholder/splice machinery these tests earlier had to work
// around.
//
// These target the refactored sibling-Reasoning shape:
@@ -3649,7 +3631,6 @@ async fn context_window_downgrade_triggers_auto_compact() {
// - Reasoning lives as `ConversationItem::Reasoning(rs::ReasoningItem)`
// siblings; the From<&ConversationRequest> for rs::CreateResponse impl
// emits them inline in `input` order.
// ============================================================================
/// Serialize a ConversationRequest using only the public
/// `From<&ConversationRequest> for rs::CreateResponse` trait impl.
@@ -4049,8 +4030,6 @@ async fn prefix_stable_after_image_pruning() {
// Image stripping mutates the old user turn's content, so full
// byte-level prefix stability cannot hold at that item. We verify:
// 1. System prompt preserved
// 2. Items grew
// 3. Text items appear in the same relative order
let body1 = serialize_via_public_api(&req1);
let body2 = serialize_via_public_api(&req2);
@@ -4164,7 +4143,8 @@ async fn prefix_stable_after_tool_result_pruning() {
h.handle
.push_tool_result(ConversationItem::tool_result("c2", "y".repeat(500)));
h.handle.push_user_message(ConversationItem::user("q3"));
h.handle.record_token_usage(6000); // > 50% of 10k context
// > 50% of 10k context
h.handle.record_token_usage(6000);
let req2 = h
.handle
@@ -4319,9 +4299,7 @@ async fn prefix_stable_after_session_resume() {
);
}
// ============================================================================
// Out-of-band history repair (kigi/session/repair)
// ============================================================================
/// Bricked-session shape: an orphaned tool result survives load (the eager
/// repairs only fix dangling calls) and 400s on every request. The
@@ -37,7 +37,7 @@ impl std::error::Error for RepairHistoryBlocked {}
/// Commands sent to the ChatStateActor via mpsc channel.
pub enum ChatStateCommand {
// ═══ Mutations (fire-and-forget) ═══
// Mutations (fire-and-forget)
/// Push a user message into the conversation.
PushUserMessage { item: ConversationItem },
@@ -64,7 +64,7 @@ pub enum ChatStateCommand {
RecordTokenUsage { total_tokens: u64 },
/// Stash the per-turn `TokenUsage` from the most recent model response.
/// Overwrites any previously stashed value.
/// Overwrites any earlier stashed value.
RecordLastTurnUsage { usage: TokenUsage },
RecordModelCallUsage {
@@ -176,7 +176,7 @@ pub enum ChatStateCommand {
/// Repair dangling tool calls after a harness-initiated halt.
RepairDanglingAfterHarnessHalt { class: &'static str },
// ═══ Queries (request/response via oneshot) ═══
// Queries (request/response via oneshot)
/// Build a ConversationRequest ready to send to the API.
/// Clones the conversation, prunes old tool results, repairs dangling
/// tool calls, injects memory reminder, and assembles the request.
@@ -280,7 +280,7 @@ pub enum ChatStateCommand {
reply: oneshot::Sender<Vec<Vec<ConversationItem>>>,
},
// ═══ Narrow targeted queries (avoid full-conversation clone) ═══
// Narrow targeted queries (avoid full-conversation clone)
/// Get the number of items in the conversation.
/// Cheaper than `GetConversation` when only the length is needed.
GetConversationLen { reply: oneshot::Sender<usize> },
@@ -32,7 +32,7 @@ impl CompactionMode {
}
}
/// Replace the detail level if this is `Segments`, else unchanged. Lets the
/// Replace the detail level if this is `Segments`, else `unchanged`. Lets the
/// resolver attach the separately-resolved `KIGI_COMPACTION_DETAIL`.
pub fn with_segment_detail(self, detail: CompactionDetail) -> Self {
match self {
@@ -77,7 +77,7 @@ pub const INDEX_HEADER: &str = "# Compaction Segment Index\n\n\
| Segment | File | Turns | Approx bytes | Keywords |\n\
|---|---|---|---|---|\n";
/// Zero-padded segment number, e.g. `007`. The single source of the pad width.
/// Zero-`padded` segment number, e.g. `007`. The single source of the pad width.
fn segment_label(index: u64) -> String {
format!("{index:03}")
}
@@ -724,7 +724,7 @@ mod tests {
assert_eq!(classify_compaction_path("compaction/notes.md"), None);
}
// --- Parity with the Python implementation's own test vectors (compaction_utils_test.py) ---
// Parity with the Python implementation's own test vectors (compaction_utils_test.py)
/// Keyword extraction: the Python `TestExtractKeywords` vectors (bare `8.`
/// headers, stopword filtering, dedup, no-section-8 fallback) plus our
@@ -299,7 +299,7 @@ pub fn extract_last_user_query(conversation: &[ConversationItem]) -> Option<Stri
.map(|item| extract_user_query(&item.text_content()))
.filter(|q| !q.is_empty())
}
/// The continuation prompt added to the conversation after auto-compaction.
/// The continuation prompt appended to the conversation after auto-compaction.
///
/// Stored here (rather than only in `kigi-shell`) so that query-extraction
/// helpers in this crate can recognise and exclude it from "real user prompt"
@@ -666,7 +666,7 @@ pub fn format_compact_summary(summary: &str) -> String {
/// A markdown "**Analysis**"-style header has no opening `<analysis>` tag for
/// step 1 to catch; it ends at an orphan `</analysis>`. Everything up to and
/// including the *last* `</analysis>` is dropped, so a scratchpad that itself
/// quotes `</analysis>` mid-reasoning is still removed whole. The peel is
/// quotes `</analysis>` mid-reasoning is still stripped whole. The peel is
/// skipped when the block already starts with a numbered section — including a
/// markdown-decorated one like `## 1.` or `**1.**` — so a `</analysis>` merely
/// echoed inside a real section never truncates the summary. Any leftover
+3 -6
View File
@@ -34,7 +34,7 @@ impl ChatStateHandle {
Self { cmd_tx }
}
// ═══ Fire-and-forget mutations ═══
// Fire-and-forget mutations
/// Push a user message into the conversation.
pub fn push_user_message(&self, item: ConversationItem) {
@@ -238,7 +238,6 @@ impl ChatStateHandle {
.send(ChatStateCommand::UpdateCredentials { credentials });
}
/// Restore from a snapshot.
pub fn restore_snapshot(&self, snapshot: ChatStateSnapshot) {
let _ = self
.cmd_tx
@@ -280,7 +279,7 @@ impl ChatStateHandle {
.send(ChatStateCommand::RepairDanglingAfterHarnessHalt { class });
}
// ═══ Async queries (via oneshot) ═══
// Async queries (via oneshot)
/// Send a query to the actor and await the reply.
///
@@ -419,7 +418,6 @@ impl ChatStateHandle {
.unwrap_or(0)
}
/// Get sampling config.
pub async fn get_sampling_config(&self) -> Option<SamplingConfig> {
self.query("GetSamplingConfig", |reply| {
ChatStateCommand::GetSamplingConfig { reply }
@@ -501,7 +499,6 @@ impl ChatStateHandle {
.unwrap_or_default()
}
/// Check if auto-compact is needed.
pub async fn check_auto_compact_needed(
&self,
threshold_percent: u8,
@@ -516,7 +513,7 @@ impl ChatStateHandle {
.flatten()
}
// ═══ Narrow targeted queries ═══
// Narrow targeted queries
/// Get the number of items in the conversation.
///
+2 -4
View File
@@ -1,8 +1,7 @@
//! kigi-chat-state — Actor-based chat state management for xAI agents.
//!
//! This crate extracts conversation state management from `kigi-shell`'s
//! `acp_session.rs` into a standalone actor. It follows the same actor pattern
//! as `kigi-hunk-tracker`:
//! Holds the conversation state driven by `kigi-shell`'s `acp_session.rs`,
//! following the same actor pattern as `kigi-hunk-tracker`:
//!
//! ```text
//! ┌────────────────┐ ┌──────────────────────────────────────┐
@@ -35,7 +34,6 @@ pub mod persistence;
pub mod types;
pub mod usage;
// Re-export main types for convenience
pub use actor::ChatStateActor;
pub use actor::state::{
estimate_conversation_tokens, estimate_item_tokens, estimate_messages_tokens,
@@ -27,9 +27,7 @@ pub trait ChatPersistence: Send + 'static {
fn flush(&mut self);
}
// ============================================================================
// Mock (test double) — channel-based, no locks, no atomics
// ============================================================================
/// A record of a persistence call, sent over a channel to the test.
#[derive(Debug, Clone)]
@@ -101,9 +99,7 @@ impl ChatPersistence for MockChatPersistence {
}
}
// ============================================================================
// Null (noop) — for benchmarks / scenarios where persistence is unwanted
// ============================================================================
/// No-op implementation: discards everything (for benchmarks / noop scenarios).
pub struct NullChatPersistence;
+11 -28
View File
@@ -13,54 +13,46 @@ use serde::{Deserialize, Serialize};
/// an injected block.
pub const MEMORY_CONTEXT_OPEN_TAG: &str = "<memory-context>";
/// Closing tag paired with [`MEMORY_CONTEXT_OPEN_TAG`].
pub const MEMORY_CONTEXT_CLOSE_TAG: &str = "</memory-context>";
/// Configuration for the ChatStateActor at spawn time.
#[derive(Debug, Clone)]
pub struct ChatStateConfig {
/// Initial conversation items to populate the state with.
pub initial_conversation: Vec<ConversationItem>,
/// Sampling configuration (model, context window, etc.).
pub sampling_config: SamplingConfig,
}
/// Immutable snapshot of the actor's state (for forking, rewind).
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ChatStateSnapshot {
/// The full conversation history.
pub conversation: Vec<ConversationItem>,
/// Current sampling configuration.
pub sampling_config: SamplingConfig,
/// Current prompt index (incremented per user turn).
/// Incremented per user turn.
pub prompt_index: usize,
/// Accumulated token usage.
pub total_tokens: u64,
/// Bytes/4 estimate of the conversation as of the last `record_token_usage`.
/// `0` means unknown (pre-field snapshot); restore re-estimates instead.
/// `0` means unknown (snapshot written without the field); restore
/// re-estimates instead.
#[serde(default)]
pub estimate_at_last_response: u64,
/// File paths the agent has edited.
pub agent_edited_paths: BTreeSet<String>,
/// Cached prompt texts for rewind preview.
/// Cached for rewind preview.
pub prompt_texts: Vec<String>,
/// Timestamp when the current stream started (epoch ms).
/// Epoch ms.
pub stream_start_ms: Option<i64>,
/// Timestamp when the current turn started (epoch ms).
/// Epoch ms.
pub turn_start_ms: Option<i64>,
/// Prompt index at which the last compaction occurred.
pub last_compaction_prompt_index: Option<usize>,
/// Opaque credential secrets (API key, optional extra auth, client version).
#[serde(default)]
pub credentials: Credentials,
}
/// Metadata for session notifications (timing info).
/// Timing metadata for session notifications.
#[derive(Debug, Clone)]
pub struct NotificationMeta {
/// Timestamp when the current stream started (epoch ms).
/// Epoch ms.
pub stream_start_ms: Option<i64>,
/// Timestamp when the current turn started (epoch ms).
/// Epoch ms.
pub turn_start_ms: Option<i64>,
}
@@ -70,7 +62,6 @@ pub struct NotificationMeta {
/// Two modes: soft trim (keep head + tail) and hard clear (replace entirely).
#[derive(Debug, Clone)]
pub struct PruningConfig {
/// Whether pruning is enabled.
pub enabled: bool,
/// Number of recent turns whose tool results are never pruned.
pub keep_last_n_turns: usize,
@@ -116,9 +107,7 @@ pub enum AuthType {
/// The actor just stores and returns them — it never interprets them.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct Credentials {
/// API key for authentication.
pub api_key: Option<String>,
/// Whether this is a session token (refreshable) or user-provided api key.
#[serde(default)]
pub auth_type: AuthType,
/// Optional extra auth material forwarded with requests when present.
@@ -130,7 +119,7 @@ pub struct Credentials {
/// Produced by `TakeTurnMessages` after a `BeginTurnCapture`/message-push cycle.
#[derive(Debug, Clone)]
pub struct TurnCapture {
/// The ordered sequence of messages appended during this turn.
/// In the order they were appended.
pub messages: Vec<ConversationItem>,
/// Whether compaction (conversation replacement) occurred mid-turn.
pub compaction_occurred: bool,
@@ -142,24 +131,18 @@ pub struct TurnCapture {
/// when only role counts and total length are needed (e.g. for telemetry).
#[derive(Debug, Clone, Default)]
pub struct ConversationCounts {
/// Total number of items in the conversation.
pub total: usize,
/// Number of `User` items.
pub user: usize,
/// Number of `Assistant` items.
pub assistant: usize,
/// Number of `ToolResult` items.
pub tool_result: usize,
}
/// Info returned when auto-compact threshold is exceeded.
#[derive(Debug, Clone)]
pub struct AutoCompactTrigger {
/// Current total token count.
pub total_tokens: u64,
/// Model's context window size.
pub context_window: NonZeroU64,
/// Current utilization as a percentage (0100).
/// 0100.
pub utilization_percent: u8,
}
@@ -45,7 +45,6 @@ fn main() {
println!("git2 (index only): {} files in {:?}", files.len(), elapsed);
}
_ => {
// Run all three methods multiple times for comparison
println!("Benchmarking file listing for: {}", root_path.display());
println!();
@@ -56,7 +55,6 @@ fn main() {
let _ = collect_files_git2(root_path, &registry);
let _ = collect_files_git2_index_only(root_path, &registry);
// CLI benchmark
let mut cli_times = Vec::with_capacity(iterations);
let mut cli_count = 0;
for _ in 0..iterations {
@@ -66,7 +64,6 @@ fn main() {
cli_count = files.len();
}
// git2 benchmark (with untracked)
let mut git2_times = Vec::with_capacity(iterations);
let mut git2_count = 0;
for _ in 0..iterations {
@@ -76,7 +73,6 @@ fn main() {
git2_count = files.len();
}
// git2 index-only benchmark
let mut git2_index_times = Vec::with_capacity(iterations);
let mut git2_index_count = 0;
for _ in 0..iterations {
@@ -86,7 +82,6 @@ fn main() {
git2_index_count = files.len();
}
// Print results
let cli_avg = cli_times.iter().sum::<std::time::Duration>() / iterations as u32;
let git2_avg = git2_times.iter().sum::<std::time::Duration>() / iterations as u32;
let git2_index_avg =
@@ -117,9 +112,7 @@ fn main() {
}
}
/// Collect files using git CLI (original approach)
fn collect_files_cli(root_path: &Path, registry: &LanguageRegistry) -> Vec<std::path::PathBuf> {
// Get tracked files
let tracked_output = Command::new("git")
.args(["ls-files"])
.current_dir(root_path)
@@ -130,7 +123,6 @@ fn collect_files_cli(root_path: &Path, registry: &LanguageRegistry) -> Vec<std::
_ => return vec![],
};
// Get untracked files
let untracked_output = Command::new("git")
.args(["ls-files", "--others", "--exclude-standard"])
.current_dir(root_path)
@@ -159,7 +151,6 @@ fn collect_files_cli(root_path: &Path, registry: &LanguageRegistry) -> Vec<std::
files
}
/// Collect files using git2 (new approach)
fn collect_files_git2(root_path: &Path, registry: &LanguageRegistry) -> Vec<std::path::PathBuf> {
let repo = match Repository::open(root_path) {
Ok(r) => r,
@@ -183,7 +174,6 @@ fn collect_files_git2(root_path: &Path, registry: &LanguageRegistry) -> Vec<std:
})
.collect();
// Get untracked files
let mut status_opts = StatusOptions::new();
status_opts
.include_untracked(true)
@@ -204,7 +194,6 @@ fn collect_files_git2(root_path: &Path, registry: &LanguageRegistry) -> Vec<std:
files
}
/// Collect files using git2 index only (tracked files only, no untracked)
fn collect_files_git2_index_only(
root_path: &Path,
registry: &LanguageRegistry,
@@ -21,7 +21,6 @@ fn main() {
std::process::exit(1);
};
// First, verify all queries compile
println!("Verifying query compilation...");
let registry = LanguageRegistry::new();
for ext in &["ts", "tsx", "js", "jsx", "rs", "go", "py"] {
@@ -176,14 +176,12 @@ fn main() {
}
}
/// Get the effective cache path - use custom if provided, otherwise default.
fn effective_cache_path(repo_path: &Path, custom_cache: Option<&Path>) -> PathBuf {
custom_cache
.map(|p| p.to_path_buf())
.unwrap_or_else(|| get_cache_path(repo_path))
}
/// Load index from cache or build if necessary.
fn load_or_build_index(repo_path: &Path, cache_path: &Path) -> ScopeGraphIndex {
if let Ok(index) = load_index(cache_path) {
println!("Loaded index from cache: {}", cache_path.display());
@@ -204,7 +202,6 @@ fn load_or_build_index(repo_path: &Path, cache_path: &Path) -> ScopeGraphIndex {
files, defs, refs, elapsed
);
// Save to cache
if let Err(e) = save_index(cache_path, &index) {
println!("Warning: Failed to save cache: {}", e);
} else {
@@ -260,7 +257,6 @@ fn cmd_definition(
let navigator = Navigator::new(index);
let result = match (file, row, col, symbol) {
// Position-based lookup
(Some(file_path), Some(r), Some(c), _) => {
let abs_path = if file_path.is_absolute() {
file_path
@@ -276,7 +272,6 @@ fn cmd_definition(
}
}
}
// Symbol-based lookup
(_, _, _, Some(sym)) => navigator.goto_definition_by_name(&sym, None),
_ => {
println!("Error: Must provide either --file, --row, --col OR --symbol");
@@ -310,7 +305,6 @@ fn cmd_references(
let navigator = Navigator::new(index);
let result = match (file, row, col, symbol) {
// Position-based lookup
(Some(file_path), Some(r), Some(c), _) => {
let abs_path = if file_path.is_absolute() {
file_path
@@ -326,7 +320,6 @@ fn cmd_references(
}
}
}
// Symbol-based lookup
(_, _, _, Some(sym)) => navigator.goto_references_by_name(&sym, None, include_definition),
_ => {
println!("Error: Must provide either --file, --row, --col OR --symbol");
@@ -361,7 +354,6 @@ fn cmd_stats(path: &Path, custom_cache: Option<&Path>) {
println!(" References: {}", refs);
println!(" Aliases: {}", index.alias_count());
// Top symbols by reference count
let ref_counts = index.top_referenced_symbols(10);
println!("\nTop 10 most referenced symbols:");
@@ -149,7 +149,6 @@ pub enum IndexCommand {
BackgroundRefresh {
/// Files that need reindexing (stale or new)
stale_files: Vec<String>,
/// Files that were deleted
deleted_files: Vec<String>,
},
/// Get the number of indexed files (lightweight, no clone)
@@ -330,7 +329,7 @@ impl IndexManagerHandle {
self.command_tx.send(IndexCommand::Shutdown)
}
// ========== Async Query APIs ==========
// Async Query APIs
/// Go to definition at the given position (async).
///
@@ -417,7 +416,7 @@ impl IndexManagerHandle {
Ok(rx.await.expect("IndexManager dropped before responding"))
}
// ========== Blocking Query APIs ==========
// Blocking Query APIs
/// Go to definition at the given position (blocking).
pub fn goto_definition_blocking(
@@ -518,7 +517,6 @@ impl IndexManagerConfig {
}
}
/// Set the cache path.
pub fn with_cache_path(mut self, path: PathBuf) -> Self {
self.cache_path = Some(path);
self
@@ -1349,12 +1347,12 @@ fn background_index_refresh(
if cached_meta.is_stale(path_ref) {
// Check if file exists or is deleted
if path_ref.exists() {
Some((Some(path.clone()), None)) // Stale
Some((Some(path.clone()), None))
} else {
Some((None, Some(path.clone()))) // Deleted
Some((None, Some(path.clone())))
}
} else {
None // Up to date
None
}
})
.fold(
@@ -1391,8 +1389,10 @@ fn background_index_refresh(
let registry = crate::languages::LanguageRegistry::new();
let new_files: Vec<String> = ignore::WalkBuilder::new(&root_path)
.hidden(true) // Skip hidden files/dirs
.git_ignore(true) // Respect .gitignore
// Skip hidden files/dirs
.hidden(true)
// Respect .gitignore
.git_ignore(true)
.git_global(true)
.git_exclude(true)
.build()
@@ -1530,7 +1530,7 @@ impl CoalescedEvents {
fn add(&mut self, event: FileEvent) {
// Renames are special: they carry two paths. Process the "to" path
// as Created (it needs indexing) and the "from" as Removed.
// as `Created` (it needs indexing) and the "from" as `Removed`.
if event.kind == FileEventKind::Renamed && event.paths.len() >= 2 {
self.insert(event.paths[0].clone(), FileEventKind::Removed);
self.insert(event.paths[1].clone(), FileEventKind::Created);
@@ -1551,11 +1551,11 @@ impl CoalescedEvents {
Entry::Occupied(mut e) => {
let prev = *e.get();
match (prev, kind) {
// Created/Modified then Removed → cancel both
// `Created`/`Modified` then `Removed` → cancel both
(FileEventKind::Created | FileEventKind::Modified, FileEventKind::Removed) => {
e.remove();
}
// Removed then Created/Modified → file replaced, treat as Created
// `Removed` then `Created`/`Modified` → file replaced, treat as `Created`
(FileEventKind::Removed, FileEventKind::Created | FileEventKind::Modified) => {
e.insert(FileEventKind::Created);
}
@@ -1649,8 +1649,10 @@ fn is_identifier_like(node: &tree_sitter::Node<'_>) -> bool {
|| kind == "field_identifier"
|| kind == "shorthand_property_identifier"
|| kind == "shorthand_property_identifier_pattern"
|| kind == "attribute" // Python
|| kind == "package_identifier" // Go
// Python
|| kind == "attribute"
// Go
|| kind == "package_identifier"
}
#[cfg(test)]
@@ -1830,7 +1832,8 @@ mod tests {
let dir = tempdir().unwrap();
let file_path = dir.path().join("huge.rs");
// Write a file larger than MAX_INDEXABLE_FILE_SIZE
let content = "fn a() {}\n".repeat(600_000); // ~6MB
// ~6MB
let content = "fn a() {}\n".repeat(600_000);
fs::write(&file_path, &content).unwrap();
let config = IndexManagerConfig::new(dir.path().to_path_buf())
@@ -1891,7 +1894,8 @@ mod tests {
fs::write(dir.path().join("binary.rs"), &binary).unwrap();
// Oversized file — should be skipped
let big = "fn big() {}\n".repeat(500_000); // ~6MB
// ~6MB
let big = "fn big() {}\n".repeat(500_000);
fs::write(dir.path().join("huge.rs"), &big).unwrap();
let index = IndexBuilder::new().build(dir.path()).unwrap();
@@ -1930,12 +1934,13 @@ mod tests {
let stats = handle.get_stats().unwrap();
assert_eq!(stats.files, 1);
assert!(stats.definitions >= 2); // hello + world
// hello + world
assert!(stats.definitions >= 2);
handle.shutdown().unwrap();
}
// ========== CoalescedEvents tests ==========
// CoalescedEvents tests
#[test]
fn test_coalesce_create_then_remove_cancels() {
@@ -2003,7 +2008,7 @@ mod tests {
let mut c = CoalescedEvents::new();
c.add(FileEvent::renamed("/a.rs".into(), "/b.rs".into()));
c.add(FileEvent::removed("/b.rs".into()));
// /a.rs should still be Removed, /b.rs Created+Removed = cancelled
// /a.rs should still be `Removed`, /b.rs `Created`+`Removed` = cancelled
assert_eq!(c.events.len(), 1);
assert_eq!(c.events[&PathBuf::from("/a.rs")], FileEventKind::Removed);
}
@@ -2013,7 +2018,7 @@ mod tests {
let mut c = CoalescedEvents::new();
c.add(FileEvent::renamed("/a.rs".into(), "/b.rs".into()));
c.add(FileEvent::modified("/b.rs".into()));
// /a.rs Removed, /b.rs Created+Modified → Modified (last writer wins)
// /a.rs `Removed`, /b.rs `Created`+`Modified``Modified` (last writer wins)
assert_eq!(c.events.len(), 2);
assert_eq!(c.events[&PathBuf::from("/a.rs")], FileEventKind::Removed);
assert_eq!(c.events[&PathBuf::from("/b.rs")], FileEventKind::Modified);
@@ -52,7 +52,6 @@ impl StringId {
Self(id)
}
/// Get the raw u32 value.
#[inline]
pub const fn as_u32(self) -> u32 {
self.0
@@ -209,7 +208,6 @@ impl StringInterner {
self.offsets.is_empty()
}
/// Total bytes used by the arena.
#[inline]
pub fn arena_bytes(&self) -> usize {
self.arena.len()
@@ -271,7 +269,7 @@ impl StringInterner {
///
/// After a bulk build the arena and offsets Vecs may hold up to 2× their
/// actual content due to doubling growth. Calling this reclaims that
/// wasted heap. The lookup table is intentionally left unshrunk because
/// wasted heap. The lookup table is deliberately left unshrunk because
/// it benefits from load-factor headroom.
///
/// This is an internal maintenance hook called by `ScopeGraphIndex::compact()`.
@@ -313,7 +311,7 @@ mod tests {
let id1 = interner.intern("src");
let id2 = interner.intern("lib");
let id3 = interner.intern("src"); // duplicate
let id3 = interner.intern("src");
assert_eq!(id1, id3);
assert_ne!(id1, id2);
@@ -348,7 +346,8 @@ mod tests {
// Invalid UTF-8
let invalid_utf8: &[u8] = &[0x80, 0x81, 0x82];
let id2 = interner.intern_bytes(invalid_utf8);
assert_eq!(interner.get(id2), None); // Not valid UTF-8
// Not valid UTF-8
assert_eq!(interner.get(id2), None);
assert_eq!(interner.get_bytes(id2), Some(invalid_utf8));
// Duplicate bytes return same ID
@@ -1,5 +1,3 @@
//! JavaScript/JSX language configuration.
use crate::languages::types::TSLanguageConfig;
pub fn js_lang() -> TSLanguageConfig {
@@ -114,7 +114,7 @@ impl LanguageRegistry {
/// Compute a hash of all tree-sitter queries across all languages.
///
/// This is used to detect when queries change, which should trigger
/// a rebuild of the index even if file contents haven't changed.
/// a rebuild of the index even if file contents are `unchanged`.
///
/// The hash is computed by:
/// 1. Sorting languages by their primary ID for deterministic ordering
@@ -1,5 +1,3 @@
//! Python language configuration.
use crate::languages::types::TSLanguageConfig;
pub fn python_lang() -> TSLanguageConfig {
@@ -12,7 +10,6 @@ pub fn python_lang() -> TSLanguageConfig {
"variable".to_owned(),
"module".to_owned(),
]],
// Python definitions query
r#"
; Class definitions
(class_definition
@@ -19,7 +19,6 @@ pub fn ts_lang() -> TSLanguageConfig {
"const".to_owned(),
"let".to_owned(),
]],
// Comprehensive TypeScript query with full type coverage
r#"
;; === DEFINITIONS ===
@@ -30,7 +30,6 @@ impl TSLanguageConfig {
}
}
/// Get the language IDs.
pub fn language_ids(&self) -> &[String] {
&self.language_ids
}
@@ -43,17 +42,14 @@ impl TSLanguageConfig {
.unwrap_or("unknown")
}
/// Get the file extensions.
pub fn file_extensions(&self) -> &[String] {
&self.file_extensions
}
/// Get the namespaces.
pub fn namespaces(&self) -> &[Vec<String>] {
&self.namespaces
}
/// Get the file definition queries.
pub fn file_definition_queries(&self) -> &str {
&self.file_definition_queries
}
@@ -235,7 +235,8 @@ impl IndexBuilder {
.git_ignore(self.respect_gitignore)
.git_global(self.respect_gitignore)
.git_exclude(self.respect_gitignore)
.threads(self.num_threads.min(12)) // Use parallel walking (capped at 12)
// Use parallel walking (capped at 12)
.threads(self.num_threads.min(12))
.build_parallel();
walker.run(|| {
@@ -309,7 +310,6 @@ impl IndexBuilder {
//
// New approach: for each batch of build_batch_size files:
// 1. Parse in parallel (par_chunks preserves thread-local cache locality)
// 2. Merge the batch into the index
// 3. Drop the batch before starting the next one
// Peak = O(build_batch_size) symbols + growing index simultaneously.
for batch in file_paths.chunks(build_batch_size) {
@@ -1,25 +1,21 @@
//! Index caching for fast loading.
//!
//! Uses a custom binary format with magic bytes "SGIX" for the new interned format.
//! Automatically detects and skips legacy bincode format (returns error so caller can rebuild).
//! The on-disk format is a custom binary layout tagged with the magic bytes
//! "SGIX". Caches written by the earlier bincode format are detected and
//! rejected rather than parsed, so the caller rebuilds from source.
use std::path::Path;
use crate::scope_graph::ScopeGraphIndex;
/// Default cache file name.
pub const CACHE_FILE_NAME: &str = ".goto_index.bin";
/// Error type for cache operations.
#[derive(Debug)]
pub enum CacheError {
/// IO error.
IoError(std::io::Error),
/// Serialization error.
SerializeError(String),
/// Deserialization error.
DeserializeError(String),
/// Legacy format detected (caller should rebuild).
/// A bincode-era cache was found; the caller is expected to rebuild.
LegacyFormat,
}
@@ -42,19 +38,14 @@ impl From<std::io::Error> for CacheError {
}
}
/// Result type for cache operations.
pub type Result<T> = std::result::Result<T, CacheError>;
/// Get the default cache path for a repository.
pub fn get_cache_path(root_path: &Path) -> std::path::PathBuf {
root_path.join(CACHE_FILE_NAME)
}
/// Load an index from cache.
///
/// Uses the new binary format with magic bytes "SGIX".
/// Returns `CacheError::LegacyFormat` if the file uses the old bincode format,
/// signaling to the caller that a rebuild is needed.
/// Returns `CacheError::LegacyFormat` for a bincode-format cache, signaling to
/// the caller that a rebuild is needed.
pub fn load_index(cache_path: &Path) -> Result<ScopeGraphIndex> {
if !cache_path.exists() {
return Err(CacheError::IoError(std::io::Error::new(
@@ -63,11 +54,10 @@ pub fn load_index(cache_path: &Path) -> Result<ScopeGraphIndex> {
)));
}
// Use ScopeGraphIndex::load which handles format detection
match ScopeGraphIndex::load(cache_path) {
Ok(Some(index)) => Ok(index),
// `Ok(None)` is how the loader reports a legacy-format file.
Ok(None) => {
// None means legacy format was detected
tracing::info!(
cache_path = %cache_path.display(),
"Legacy cache format detected, will rebuild"
@@ -78,15 +68,12 @@ pub fn load_index(cache_path: &Path) -> Result<ScopeGraphIndex> {
}
}
/// Save an index to cache using the new binary format.
pub fn save_index(cache_path: &Path, index: &ScopeGraphIndex) -> Result<()> {
index.save(cache_path).map_err(CacheError::IoError)
}
/// Save an index to cache asynchronously (in a background thread).
///
/// Returns immediately and spawns a thread to do the actual saving.
/// Useful for saving the index without blocking the main thread.
/// Saves on a detached thread: the caller gets no join handle and no result,
/// so a failed write is only visible in the logs.
pub fn save_index_async(cache_path: std::path::PathBuf, index: ScopeGraphIndex) {
std::thread::spawn(move || {
if let Err(e) = save_index(&cache_path, &index) {
@@ -95,12 +82,11 @@ pub fn save_index_async(cache_path: std::path::PathBuf, index: ScopeGraphIndex)
});
}
/// Check if a cache exists and return its metadata.
pub fn cache_exists(cache_path: &Path) -> bool {
cache_path.exists()
}
/// Get cache file size in bytes.
/// Size of the cache file in bytes, or `None` if it cannot be stat'd.
pub fn cache_size(cache_path: &Path) -> Option<u64> {
std::fs::metadata(cache_path).ok().map(|m| m.len())
}
@@ -52,7 +52,8 @@ impl IndexOperation {
/// Whether this operation requires exclusive access.
pub fn is_exclusive(&self) -> bool {
match self {
Self::Load => false, // Shared/read access
// Shared/read access
Self::Load => false,
Self::Save | Self::Build | Self::BackgroundRefresh => true,
}
}
@@ -77,8 +78,10 @@ impl std::fmt::Display for IndexOperation {
/// In-memory lock state for same-process deduplication.
struct InMemoryLockState {
operation: IndexOperation,
readers: usize, // Count for shared locks
exclusive: bool, // Whether an exclusive lock is held
// Count for shared locks
readers: usize,
// Whether an exclusive lock is held
exclusive: bool,
}
/// Global registry of in-memory locks (same process).
@@ -299,7 +302,6 @@ fn try_acquire_in_memory_lock(workspace: &Path, operation: IndexOperation) -> bo
true
}
/// Release an in-memory lock.
fn release_in_memory_lock(workspace: &Path, operation: IndexOperation) {
// Use entry API for atomic check-and-modify
if let dashmap::mapref::entry::Entry::Occupied(mut entry) =
@@ -439,7 +441,6 @@ mod tests {
// Drop first lock
drop(guard1);
// Now second should succeed
let guard3 = try_lock(workspace, IndexOperation::Build);
assert!(guard3.is_acquired());
}
@@ -490,7 +491,6 @@ mod tests {
// Drop shared lock
drop(guard1);
// Now exclusive should succeed
let guard3 = try_lock(workspace, IndexOperation::Build);
assert!(guard3.is_acquired());
}
@@ -1,4 +1,4 @@
//! Index management: building, caching, locking, and updating.
//! Index management: building, caching, and workspace locking.
mod builder;
pub mod cache;
@@ -238,7 +238,7 @@ impl Navigator {
///
/// # Arguments
/// * `file_path` - Path to the file
/// * `row` - 1-indexed line number
/// * `row` - 1-indexed line number
/// * `col` - 1-indexed column number
/// * `include_definition` - Whether to include the definition location in results
///
@@ -388,8 +388,8 @@ fn is_identifier_like(node: &tree_sitter::Node<'_>) -> bool {
| "field_identifier"
| "shorthand_property_identifier"
| "shorthand_property_identifier_pattern"
| "attribute" // Python
| "package_identifier" // Go
| "attribute"
| "package_identifier"
)
}
@@ -2,21 +2,21 @@
use serde::{Deserialize, Serialize};
/// Describes the relation between two nodes in the ScopeGraph.
/// Edge weight in the ScopeGraph. Every variant is directed source-to-target,
/// in the order its name reads.
#[derive(Serialize, Deserialize, PartialEq, Eq, Copy, Clone, Debug)]
pub enum EdgeKind {
/// The edge weight from a nested scope to its parent scope.
/// Nested scope to its parent scope.
ScopeToScope,
/// The edge weight from a definition to its definition scope.
/// Definition to the scope that owns it, which for a hoisted def is the
/// parent of the scope it was written in.
DefToScope,
/// The edge weight from an import to its definition scope.
/// Import to its defining scope.
ImportToScope,
/// The edge weight from a reference to its definition.
RefToDef,
/// The edge weight from a reference to its import.
RefToImport,
}
@@ -44,7 +44,7 @@ pub type ExtractedSymbols = (
/// even if file contents haven't changed.
#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)]
pub enum QueryVersion {
/// Legacy format - index was built before query versioning was added.
/// Legacy format - index was built without query versioning.
/// This triggers a rebuild since we don't know what queries were used.
/// Default for backwards compatibility with old cached indexes.
#[default]
@@ -394,7 +394,6 @@ impl ScopeGraph {
})
}
/// Find all references to a given name
pub fn find_references(&self, name: &str, src: &[u8]) -> Vec<Range> {
self.graph
.node_indices()
@@ -703,9 +702,7 @@ impl ScopeGraphIndex {
}
}
// ========================================================================
// String interning helpers
// ========================================================================
/// Intern a string and return its ID.
#[inline]
@@ -725,9 +722,7 @@ impl ScopeGraphIndex {
self.interner.get_id(s)
}
// ========================================================================
// File metadata operations
// ========================================================================
/// Update file metadata (size and mtime) for staleness tracking.
pub fn update_file_meta(&mut self, path: &Path) {
@@ -751,9 +746,7 @@ impl ScopeGraphIndex {
}
}
// ========================================================================
// Alias operations
// ========================================================================
/// Register an alias relationship: alias_name is an alias for original_name
pub fn add_alias(&mut self, alias_name: &str, original_name: &str) {
@@ -771,9 +764,7 @@ impl ScopeGraphIndex {
self.add_alias(&alias_name, &original_name);
}
// ========================================================================
// Symbol insertion (for builder/manager use)
// ========================================================================
/// Add a definition occurrence for a symbol.
pub fn add_definition(&mut self, symbol: &str, path: &str, line: usize) {
@@ -847,9 +838,7 @@ impl ScopeGraphIndex {
.filter_map(|(&id, meta)| self.get_str(id).map(|path| (path, meta)))
}
// ========================================================================
// File operations
// ========================================================================
/// Add a file's scope graph to the index
pub fn add_file(&mut self, file_path: PathBuf, graph: ScopeGraph, src: &[u8]) {
@@ -996,9 +985,7 @@ impl ScopeGraphIndex {
self.file_meta.len()
}
// ========================================================================
// Query operations
// ========================================================================
/// Find where a symbol is defined (includes resolving aliases)
pub fn find_definitions(&self, symbol: &str) -> Vec<(&str, usize)> {
@@ -1284,9 +1271,7 @@ impl ScopeGraphIndex {
.collect()
}
// ========================================================================
// Statistics and metadata
// ========================================================================
/// Get statistics: (files_count, total_definitions, total_references).
///
@@ -1301,7 +1286,6 @@ impl ScopeGraphIndex {
)
}
/// Get alias count
pub fn alias_count(&self) -> usize {
self.aliases.len()
}
@@ -1356,9 +1340,7 @@ impl ScopeGraphIndex {
self.interner.shrink_to_fit();
}
// ========================================================================
// Binary serialization (custom format with magic bytes)
// ========================================================================
/// Save the index to a file in binary format.
pub fn save(&self, path: &Path) -> io::Result<()> {
@@ -1617,7 +1599,8 @@ impl ScopeGraphIndex {
Ok(Self {
interner,
graphs: HashMap::new(), // Not serialized
// Not serialized
graphs: HashMap::new(),
definitions,
references,
aliases,
@@ -1716,7 +1699,8 @@ mod tests {
index.compact();
let (f1, d1, r1) = index.stats();
index.compact(); // second call must be a no-op
// second call must be a no-op
index.compact();
let (f2, d2, r2) = index.stats();
assert_eq!(f1, f2);
@@ -16,17 +16,12 @@ pub use nodes::{LocalDef, LocalImport, LocalScope, NodeKind, Reference, Symbol,
use crate::languages::TSLanguageConfig;
/// Result of building a scope graph, including alias pairs.
pub struct ScopeGraphResult {
/// The scope graph for the file.
pub graph: ScopeGraph,
/// Alias pairs: (alias_name, original_name).
/// Each pair is `(alias_name, original_name)`.
pub aliases: Vec<(String, String)>,
}
/// Build a ScopeGraph from tree-sitter query and source.
///
/// This is a convenience wrapper around `scope_graph_from_definitions_query`.
pub fn build_scope_graph(
query: &tree_sitter::Query,
root_node: tree_sitter::Node<'_>,
@@ -89,7 +89,6 @@ impl LocalDef {
&src[self.range.start_byte()..self.range.end_byte()]
}
/// Get the scope range.
pub fn scope_range(&self) -> &Range {
&self.scope.range
}
@@ -25,7 +25,6 @@ pub enum FileEvent {
/// A file was renamed/moved.
Renamed {
/// Original path.
from: PathBuf,
/// New path.
to: PathBuf,
@@ -49,7 +48,8 @@ impl FileEvent {
FileEvent::Created { .. } => true,
FileEvent::Modified { .. } => true,
FileEvent::Deleted { .. } => false,
FileEvent::Renamed { .. } => false, // Only path update needed
// Only path update needed
FileEvent::Renamed { .. } => false,
}
}
@@ -49,7 +49,6 @@ impl Location {
}
}
/// Get the file path.
pub fn file_path(&self) -> &PathBuf {
&self.file_path
}
@@ -113,7 +113,8 @@ impl FileMeta {
let current = Self::from_metadata(&meta);
*self != current
}
Err(_) => true, // File deleted or inaccessible
// File deleted or inaccessible
Err(_) => true,
}
}
}
@@ -70,7 +70,6 @@ impl Position {
self.character
}
/// Get the byte offset.
pub fn byte_offset(&self) -> usize {
self.byte_offset
}
@@ -80,7 +79,6 @@ impl Position {
self.byte_offset
}
/// Set the byte offset.
pub fn set_byte_offset(&mut self, byte_offset: usize) {
self.byte_offset = byte_offset;
}
@@ -120,7 +118,6 @@ impl Position {
}
}
/// Move to the next line.
pub fn move_to_next_line(mut self) -> Self {
self.line += 1;
self.character = 0;
@@ -188,12 +185,10 @@ impl Range {
Self::for_tree_node(node)
}
/// Get the start position.
pub fn start_position(&self) -> Position {
self.start_position
}
/// Get the end position.
pub fn end_position(&self) -> Position {
self.end_position
}
@@ -208,12 +203,10 @@ impl Range {
&self.end_position
}
/// Set the start position.
pub fn set_start_position(&mut self, position: Position) {
self.start_position = position;
}
/// Set the end position.
pub fn set_end_position(&mut self, position: Position) {
self.end_position = position;
}
@@ -1,29 +1,20 @@
//! Isolated RSS test for incremental reindexing.
//!
//! This test lives in its own integration-test file (and therefore its own
//! Bazel `rust_test` target / process) so that its whole-process RSS samples
//! are not polluted by the other allocation-heavy tests in
//! `memory_integration.rs` (e.g. `test_fresh_build_rss`,
//! `test_build_batch_peak_rss_is_bounded`, `test_compact_reduces_rss_vs_uncompacted`).
//! `libtest` runs a test binary's tests concurrently across `num_cpus` threads,
//! but VmRSS is measured per-*process*. Sharing a binary with the other
//! allocation-heavy tests in `memory_integration.rs` made this test observe
//! their allocator churn, intermittently pushing the measured incremental
//! growth delta over the 20 MB budget on aarch64 fastbuild CI (~31 MB).
//!
//! Background: `libtest` runs tests in a single binary concurrently across
//! `num_cpus` threads, and VmRSS is measured per-*process*. When this test
//! ran inside `memory_integration.rs` it observed allocator churn from the
//! other tests on the same process, intermittently pushing the measured
//! "incremental growth" delta over the 20 MB budget on aarch64 fastbuild CI
//! (`run_1_of_2` and `run_2_of_2` both failed at ~31 MB).
//!
//! Keep this file to a single test. If you need to add another RSS-sensitive
//! test, give it its own file too rather than reintroducing the
//! noisy-neighbor problem.
//! Hence its own integration-test file, and therefore its own Bazel
//! `rust_test` target and process. Keep this file to a single test; any other
//! RSS-sensitive test needs a file of its own rather than a noisy neighbor.
use kigi_codebase_graph::{FileEvent, IndexManager, IndexManagerConfig};
use std::fs;
use std::path::Path;
use tempfile::tempdir;
/// Read current process RSS in bytes. Supports Linux and macOS.
/// Returns `None` on unsupported platforms.
fn rss_bytes() -> Option<usize> {
#[cfg(target_os = "linux")]
{
@@ -65,7 +56,6 @@ fn fmt_rss(rss: Option<f64>) -> String {
rss.map_or("N/A".to_string(), |v| format!("{:.1}MB", v))
}
/// Create N Rust source files in `dir`, each with `defs_per_file` function defs.
fn create_rust_files(dir: &Path, count: usize, defs_per_file: usize) {
for i in 0..count {
let mut content = String::new();
@@ -122,7 +112,6 @@ fn test_bulk_incremental_indexing_memory() {
);
println!("RSS after incremental: {}", fmt_rss(rss_after_incremental));
// Incremental reindexing should not grow memory significantly.
if let (Some(after_inc), Some(after_build)) = (rss_after_incremental, rss_after_build) {
let growth = after_inc - after_build;
assert!(
@@ -80,9 +80,7 @@ fn create_binary_files(dir: &Path, count: usize, size: usize) {
}
}
// =========================================================================
// Tests
// =========================================================================
#[test]
#[serial_test::serial]
@@ -221,11 +219,14 @@ fn test_builder_skips_binary_and_oversized_in_bulk() {
let root = dir.path();
// Mix of valid, binary, and oversized files
create_rust_files(root, 100, 5); // 100 valid files
create_binary_files(root, 50, 10_000); // 50 binary files
// 100 valid files
create_rust_files(root, 100, 5);
// 50 binary files
create_binary_files(root, 50, 10_000);
// One oversized file
let big = "fn x() {}\n".repeat(600_000); // ~6MB
// ~6MB
let big = "fn x() {}\n".repeat(600_000);
fs::write(root.join("oversized.rs"), &big).unwrap();
drop(big);
@@ -234,7 +235,8 @@ fn test_builder_skips_binary_and_oversized_in_bulk() {
// Only the 100 valid files should be indexed
assert_eq!(files, 100);
assert!(defs >= 500); // 100 files × 5 defs
// 100 files × 5 defs
assert!(defs >= 500);
}
/// Measure RSS growth from a single `get_snapshot()` call on a representative index.
@@ -248,7 +250,8 @@ fn test_builder_skips_binary_and_oversized_in_bulk() {
fn test_single_snapshot_rss() {
let dir = tempdir().unwrap();
let root = dir.path();
create_rust_files(root, 500, 10); // 500 files, 5 000 defs
// 500 files, 5 000 defs
create_rust_files(root, 500, 10);
let config = IndexManagerConfig::new(root.to_path_buf())
.without_cache_load()
@@ -353,7 +356,8 @@ fn test_repeated_snapshots_rss_bounded() {
fn test_fresh_build_rss() {
let dir = tempdir().unwrap();
let root = dir.path();
create_rust_files(root, 500, 10); // 500 files, 5 000 defs
// 500 files, 5 000 defs
create_rust_files(root, 500, 10);
let rss_before = rss_mb();
@@ -451,7 +455,8 @@ fn test_cache_load_rss() {
fn test_build_batch_size_produces_correct_index() {
let dir = tempdir().unwrap();
let root = dir.path();
create_rust_files(root, 200, 5); // 200 files, 1 000 defs
// 200 files, 1 000 defs
create_rust_files(root, 200, 5);
// Build with a very small batch size (10 files per merge batch)
let batched = IndexBuilder::new()
@@ -587,9 +592,7 @@ fn test_build_batch_peak_rss_is_bounded() {
assert_eq!(b_refs, u_refs, "reference count must match");
}
// =============================================================================
// Structural compaction tests
// =============================================================================
/// Verify that an index survives a save/load round-trip after compact().
///
@@ -601,7 +604,8 @@ fn test_build_batch_peak_rss_is_bounded() {
fn test_compact_then_save_load_roundtrip() {
let dir = tempdir().unwrap();
let root = dir.path();
create_rust_files(root, 50, 4); // 50 files, 200 defs
// 50 files, 200 defs
create_rust_files(root, 50, 4);
// build() calls compact() internally via build_fast()
let original = IndexBuilder::new().build(root).unwrap();
@@ -1,5 +1,8 @@
//! Config-value resolution leaf types and per-model laziness config,
//! extracted from kigi-shell for dependency inversion.
//! Config-value resolution leaf types and per-model laziness config.
//!
//! They live outside kigi-shell so crates below it (kigi-memory,
//! kigi-shared, kigi-workspace) can share them without depending on the
//! shell.
use kigi_config::env_bool;
@@ -18,7 +21,6 @@ pub enum ConfigSource {
Default,
}
/// A resolved config value with its source for diagnostics.
#[derive(Debug, Clone)]
pub struct Resolved<T> {
pub value: T,
@@ -156,9 +158,8 @@ pub struct LazinessDetectorPerModelConfig {
pub min_confidence: Option<f32>,
/// When `Some(true)` (or `None` — the default), the classifier sees
/// the assistant's plain-text reasoning as `[assistant reasoning]`
/// lines. `Some(false)` drops them (the pre-2026-05 behavior).
/// `None` defers to the harness default (`LAZINESS_INCLUDE_REASONING`,
/// currently `true`).
/// lines; `Some(false)` drops them. `None` defers to the harness
/// default (`LAZINESS_INCLUDE_REASONING`, currently `true`).
#[serde(default)]
pub include_reasoning: Option<bool>,
}
+3 -11
View File
@@ -1,5 +1,4 @@
//! MCP server configuration value types, extracted from kigi-shell
//! (config dependency inversion).
//! MCP server configuration value types.
use agent_client_protocol as acp;
use indexmap::IndexMap;
@@ -8,14 +7,10 @@ use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use std::path::PathBuf;
/// serde default helper. Kept module-local rather than shared — the `pool`
/// module keeps its own copy for `PoolConfig`.
fn default_true() -> bool {
true
}
/// Read an MCP OAuth client secret from the named env var. Moved here with
/// `McpServerConfig` (its only caller).
fn resolve_oauth_client_secret(env_var: Option<&String>) -> Option<String> {
let env_var = env_var?;
match std::env::var(env_var) {
@@ -56,10 +51,8 @@ pub enum McpServerTransportConfig {
/// OAuth client ID for providers that don't support Dynamic Client Registration.
#[serde(default, skip_serializing_if = "Option::is_none")]
oauth_client_id: Option<String>,
/// Name of the env var holding the OAuth client secret (for BYO credentials).
#[serde(default, skip_serializing_if = "Option::is_none")]
oauth_client_secret_env_var: Option<String>,
/// OAuth scopes to request during authorization.
#[serde(default, skip_serializing_if = "Option::is_none")]
oauth_scopes: Option<Vec<String>>,
},
@@ -176,7 +169,6 @@ impl McpServerConfig {
})
.unwrap_or_default();
// Add bearer token from environment variable if specified
if let Some(env_var) = bearer_token_env_var {
match std::env::var(env_var) {
Ok(token) => {
@@ -213,7 +205,7 @@ impl McpServerConfig {
}
}
/// Extract OAuth configuration for this server, if any OAuth fields are set.
/// Inline `oauth_*` transport fields take precedence over the `oauth` block.
pub fn oauth_config(&self) -> Option<McpOAuthConfig> {
if let McpServerTransportConfig::StreamableHttp {
oauth_client_id,
@@ -260,7 +252,7 @@ pub struct RelaySyncConfig {
}
impl RelaySyncConfig {
/// Check if relay sync is enabled. Env var takes precedence over config.
/// `KIGI_RELAY_SYNC_ENABLED` overrides the configured value.
pub fn is_enabled(&self) -> bool {
if let Ok(env_val) = std::env::var("KIGI_RELAY_SYNC_ENABLED") {
return env_val.eq_ignore_ascii_case("true") || env_val == "1";
@@ -450,7 +450,8 @@ mod tests {
fn effective_half_life_converts_legacy_recency_decay() {
let mut s = MemorySearchConfig::default();
s.temporal_decay.enabled = false;
s.recency_decay = 0.5; // non-default → converted
// non-default → converted
s.recency_decay = 0.5;
let hl = s.effective_half_life_days().unwrap();
assert!(
(hl - 1.0).abs() < 1e-9,
@@ -32,7 +32,7 @@ pub enum PatternMode {
/// Action to take when rule matches.
///
/// CWE-1188: Default changed from Allow to Deny so that omitting the
/// CWE-1188: the default is Deny rather than Allow, so that omitting the
/// `action` field in a TOML permission rule does not silently create a
/// catch-all allow rule.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
+9 -14
View File
@@ -1,5 +1,4 @@
//! Worktree-pool configuration value type, extracted from kigi-shell
//! (config dependency inversion).
//! Worktree-pool configuration value type.
use serde::{Deserialize, Serialize};
@@ -18,27 +17,23 @@ use serde::{Deserialize, Serialize};
/// ```
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct PoolConfig {
/// Whether the pool is enabled at all.
/// Can be set to false to disable pooling regardless of repo size.
/// Default: true (auto-detect based on file_count_threshold)
/// When false, pooling is off regardless of repo size; otherwise
/// `file_count_threshold` decides.
#[serde(default = "default_true")]
pub enabled: bool,
/// Number of worktrees to keep ready in the pool.
/// 2 is the minimum useful value when forks need parallel worktrees.
/// Default: 2
/// Number of worktrees to keep ready. 2 is the minimum useful value when
/// forks need parallel worktrees.
#[serde(default = "default_pool_size")]
pub pool_size: usize,
/// Minimum number of tracked files for the pool to activate.
/// Below this threshold, on-demand creation is fast enough.
/// Default: 50_000
/// Minimum number of tracked files for the pool to activate. Below this,
/// on-demand creation is fast enough.
#[serde(default = "default_file_count_threshold")]
pub file_count_threshold: usize,
/// Number of threads to use for worktree creation when populating the pool.
/// This can speed up pool population on large repos, but also increases resource usage.
/// Default: 3.
/// Threads used to populate the pool. Higher values speed up population on
/// large repos at the cost of more concurrent resource use.
#[serde(default = "default_pool_parallelism")]
pub parallelism: usize,
}
+4 -9
View File
@@ -59,7 +59,6 @@ pub fn build_campaign_entries(
tracing::warn!(layer, "campaigns: entry missing id; skipped");
continue;
};
// Skip no-op entries (id only, no fields to overlay).
if entry.patch.is_empty() {
continue;
}
@@ -180,8 +179,8 @@ mod tests {
#[test]
fn apply_highest_priority_wins_on_leaf_conflict() {
// Two *distinct* ids both set models.default; the higher-priority source
// (earlier in the merged list) must win the leaf.
// Two *distinct* ids both set models.default, so dedup by id does not
// apply and the leaf conflict is settled by apply order alone.
let req = [CampaignEntry {
id: "req".into(),
patch: models_default_patch("from-req"),
@@ -200,8 +199,6 @@ mod tests {
#[test]
fn build_campaign_entries_skips_missing_id() {
// A `None` id and a whitespace-only id are both dropped (with a warn);
// only the entry carrying a real id survives.
let taken = vec![
ConfigOverrideEntry {
meta: CampaignMeta { id: None },
@@ -236,9 +233,8 @@ mod tests {
let entries = take_campaign_entries(&mut layer, "user");
assert_eq!(entries.len(), 1);
assert_eq!(entries[0].id, "c1");
// The id key (either spelling) must be consumed by the meta, never
// land in the patch — a leaked key would deep-merge a junk top-level
// `id` into every effective config.
// A leaked id key would deep-merge a junk top-level `id` into every
// effective config.
assert!(
entries[0].patch.get("id").is_none()
&& entries[0].patch.get("campaign_id").is_none(),
@@ -273,7 +269,6 @@ mod tests {
#[test]
fn effective_config_honors_dismiss() {
use crate::loader::ConfigLayers;
// A dismissed campaign id stops overriding; the user's stored value returns.
let mut layers = ConfigLayers {
user: parse("[models]\ndefault = \"user-old\"\n"),
..Default::default()
-2
View File
@@ -25,8 +25,6 @@ pub mod signed_policy;
mod validation;
pub mod version_overrides;
// Only the cross-crate campaign surface is re-exported at the root; the rest stays
// reachable via the `pub mod` paths for in-crate use without widening the API.
pub use campaigns::{
CampaignEntry, CampaignOverrides, filter_active_campaigns, ids_touching_paths,
};
-3
View File
@@ -84,7 +84,6 @@ pub fn load_from_disk() -> std::io::Result<toml::Value> {
load_user_config_layer(user_kigi_home().as_deref(), "config.toml")
}
/// Managed config filename, shared by the loaders in this module.
pub const MANAGED_CONFIG_FILENAME: &str = "managed_config.toml";
pub fn load_managed_config() -> std::io::Result<toml::Value> {
@@ -111,7 +110,6 @@ pub fn load_system_managed_config() -> std::io::Result<toml::Value> {
Ok(v)
}
/// One managed-config layer: the parsed TOML and the file it came from.
#[derive(Debug, Clone)]
pub struct ManagedConfigLayer {
pub value: toml::Value,
@@ -377,7 +375,6 @@ pub struct CampaignsState {
pub dismissed_ids: Vec<String>,
}
/// Path to `$KIGI_SHARE_DIR/campaigns_state.json` under `home`.
pub fn campaigns_state_path(home: &std::path::Path) -> std::path::PathBuf {
home.join(CAMPAIGNS_STATE_FILE)
}
@@ -1,6 +1,6 @@
//! macOS MDM managed-preferences layer.
//!
//! Admins push a device profile with standard-base64 (padded) TOML under
//! Admins push a device profile with standard-base64 (`padded`) TOML under
//! preference domain `ai.x.kigi` (`requirements_toml_base64`). Only admin-*forced*
//! values are read, so a local user can't forge it via their own preference
//! domain; trusted on every launch, independent of network/cache. `None` off macOS.
@@ -425,7 +425,8 @@ fn managed_config_stale_at(home: Option<&Path>, identity: &ServingIdentity) -> b
return false;
};
let Some(cache) = read_managed_config_cache(home) else {
return true; // no marker → never synced → stale
// no marker → never synced → stale
return true;
};
if cache_unusable_for(&cache, home, identity) {
return true;
@@ -65,7 +65,8 @@ fn signed_verdict_does_not_skip_deploy_key_fingerprint() {
// opted-in cache.
assert!(managed_policy_compromised_decision(
SignedVerdict::Trusted,
true, // deploy-key fingerprint mismatch
// deploy-key fingerprint mismatch
true,
Some(&opted_in),
home,
&dkey("fp-local")

Some files were not shown because too many files have changed in this diff Show More