31 Commits
Author SHA1 Message Date
ZacharyZhang-NY 11c3ca9803 chore(release): bump workspace version to 0.1.10
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
All 62 crates inherit `workspace.package.version`, so the single edit
plus the refreshed lockfile carries the release. The `v0.1.10` tag is
what triggers the build; release.yml gates the tag against this value.

Also records the shared-SSRF-policy invariant in AGENTS.md, so a future
port does not reintroduce a second copy. The zero-egress entry is
compressed by the same number of lines to keep the file at 500.
2026-07-27 13:26:25 -04:00
ZacharyZhang-NY bac8470c80 fix(hooks): share one SSRF policy instead of a second, stale copy
The hook runner carried its own `is_blocked_ip`, a line-for-line copy of
the pre-hardening web_fetch predicate: loopback allowed unconditionally,
and no TEST-NET, 198.18/15, 240/4, 0.0.0.0/8, multicast, IPv6 site-local
or embedded-v4 wrapper coverage. Hook URLs come from settings, and
project settings from an untrusted repo are loaded today, so the gap is
reachable.

`kigi-hooks` already depends on `kigi-tools`, so the fix is to delete the
copy and call the shared predicate — no new crate, and no third
implementation to drift.

`allow_local` is on for hooks: a loopback hook receiver is a legitimate
local setup, which the old copy also allowed. It is now allowed only
when the URL names the host literally, so a public name resolving to
loopback is refused.

Range coverage now lives with the policy; the runner's tests pin what it
adds on top. The URL-scrubbing regression test moves off TEST-NET-1,
which the policy now blocks, onto a closed loopback port — faster, and
no real egress from a test.
2026-07-27 13:01:50 -04:00
ZacharyZhang-NY 74402f3078 fix(pager): stop a sibling test flattening the diff-band assertion
`committed_edit_keeps_diff_line_backgrounds` passed alone and
single-threaded but failed under parallel execution, so the workspace
gate was red.

`terminal_native_lock_paints_only_native_colors` engages the
process-global terminal-native lock. While it is held `Theme::current()`
returns `terminal_default()`, whose `diff_*_bg` are all `Color::Reset`,
so no band is painted at all — and `current_kind()` reports a nominal
`KigiNight`, which is what made the failure read as a theme mismatch.

Two changes, both at the cause:
- hold the shared theme lock via `theme_cache::pin_theme()`, the helper
  written for this and until now unused, so the lock cannot be engaged
  mid-test
- assert the invariant (two distinct non-Reset bands) instead of exact
  RGB: the entry line cache is keyed on the GLOBAL theme kind, so an
  exact-color assertion races by construction. Span-level colors stay
  pinned by the `tool::edit` tests that own them

The failure message now reports the theme, height, and the bands it
actually found, which is how the cause was located.
2026-07-27 12:38:14 -04:00
ZacharyZhang-NY 649c5f5641 fix(models): drop the ultra reasoning effort, which no backend accepts
The hardcoded Codex catalog advertised `ultra` for gpt-5.6-sol and
gpt-5.6-terra, so selecting it sent `reasoning.effort = "ultra"` and the
Responses endpoint answered 400 with its real menu, which ends at `max`.

The tier was kigi's invention: the enum documented it as codex-only, the
catalog was its only producer, and the upstream model snapshot never
listed it. Removing the variant as well as the catalog entries means
`/effort ultra` is rejected at the TUI instead of parsing and failing on
the wire; a persisted `ultra` degrades to the model default through
`lenient_reasoning_effort_opt`, as designed for vocabulary changes.
2026-07-27 12:12:14 -04:00
ZacharyZhang-NY ad3840f9ec fix(web_fetch): block non-public targets by default and gate every hop
kigi allowed loopback unconditionally and missed several non-public
ranges, and the SSRF check ran only on the initial URL.

Policy (ssrf.rs):
- loopback is blocked unless `[toolset.web_fetch] allow_local` (or
  KIGI_WEB_FETCH_ALLOW_LOCAL) is on, AND the URL names it explicitly,
  so a public name resolving to loopback stays blocked (DNS rebinding)
- add 0.0.0.0/8, 100.64/10, 192.0.0.0/24, TEST-NET-1/2/3, 198.18/15,
  240/4, IPv6 site-local and documentation prefixes
- inherit the IPv4 verdict through mapped, compatible, NAT64 and 6to4
  wrappers; network-specific NAT64 prefixes remain uncovered (see doc)

Plumbing (client.rs), where the exploitable half lived:
- re-check every redirect hop, not just the first
- compare hosts exactly; a `www` sibling has its own A records, so it
  is a cross-host redirect rather than an auto-followed hop
- run the check before the fetch service, so a blocked URL is never
  posted to an endpoint that egresses elsewhere
- exempt explicit local hosts from the https upgrade and from the
  single-label filter, and re-upgrade each followed hop

Wiring: allow_local reaches WebFetchParams from both construction
paths; documented in the config guide and the README env table.
2026-07-27 11:44:20 -04:00
ZacharyZhang-NY ac23ebc9a1 feat(turn): halt a turn stuck repeating one identical tool call
Ports grok-build's action stationarity breaker. A polling loop looks like
progress from inside the turn — every step succeeds — so nothing stops it
and it burns the whole budget re-reading one file. Nudge at 8 identical
batches, halt at 16, halt a pure no-op run at 4.

Three defects were found and fixed before this landed, none of which the
gates caught:

- The check runs AFTER execute_tool_calls, not before. Before it, the
  assistant's tool_calls are recorded with no results, so pushing the
  nudge triggers repair_dangling_tool_calls: synthetic "cancelled by the
  user" results get spliced in, the real results land after a user
  message, and dedup misses them — the orphan tool_result shape that
  draws a provider 400. It is Ok-gated too, since one `?` inside can
  leave a call resultless.
- `TurnOutcome::StationarityHalted` is its own variant grouped with
  Completed, not a reuse of Cancelled. As a cancellation it would report
  StopReason::Cancelled, fire the abort lifecycle, kill the turn's
  subagents, grow the goal harness back-off streak until three halts
  paused a running goal, and let completion-requirement recovery re-run
  the very turn halted for looping.
- The no-op gate is size-based, not name-based. kigi registers the shell
  tool as `run_terminal_cmd` and renames it `run_terminal_command`, so
  matching on "bash" silently disabled the tighter ceiling while every
  test still passed.
2026-07-27 10:37:41 -04:00
ZacharyZhang-NY a07d889ad9 fix(swarm): label each member with its own item
Every member was launched with the swarm's description, so the pager drew
N identical subagent blocks: the user could not tell which member was
running, and a failure could not be attributed to an item.

This is also why K6 does not port upstream's progress grid. A member IS
an ordinary subagent, so the pager already renders one block per member
with live status, duration, tool-call and turn counts — richer than a
grid cell. The gap was the label, not the widget; upstream's grid (and
the estimator behind it, which infers progress from tool-call rate)
answers a different TUI's shortcomings. The parent tool call already
reads "<description> (N members)" and `is_task_variant` already knows
`AgentSwarm`, so the blocking-wait spinner registers.
2026-07-27 03:01:42 -04:00
ZacharyZhang-NY 16328e55f9 feat(swarm): /swarm mode — a standing instruction to fan work out
The `agent_swarm` tool works with the mode off; what the mode adds is the
doctrine — decompose finely, give every member a disjoint scope, do not
do the work yourself. `/swarm`, `/swarm on|off`, `/swarm <task>`, gated
on the tool actually being in the toolset.

Two triggers, not upstream's three. Upstream's `tool` trigger exists so
invoking the tool makes the doctrine appear; kigi's tool carries its own
description and works with the mode off, so that trigger would add a
state only the tool can reach. `Manual` persists until switched off,
`Task` expires at turn end — both are real user intents.

Load-bearing decisions, each from a defect found in review:

- Expiry is a DROP guard, not a post-loop call. A user interrupt aborts
  the turn future rather than cancelling it, so post-loop code never
  runs; three `?` paths skip it too. Either way a `/swarm <task>` mode
  would leak into the next, unrelated prompt.
- `enter` is a total no-op while armed, so the `/swarm <task>` shorthand
  cannot downgrade a standing `/swarm on` into a per-turn mode that then
  disarms itself. This matches upstream; the first cut diverged, and the
  test asserting the divergence was inverted with the fix.
- An explicit `/swarm off` retracts UNCONDITIONALLY. The doctrine rides
  the conversation and so survives a resume, a fork and a compaction that
  an in-memory flag does not; trusting the flag left a session reading
  "off" with the instruction still steering and no way to clear it. That
  also retires the `reminder_live` field, whose doc comment claimed to
  know something the code cannot.
- The mode is deliberately NOT persisted, unlike /goal and /graph: those
  strand real work when lost, this is a prompt hint whose recovery is
  retyping one command. Recorded in AGENTS.md so the omission reads as a
  decision, not a gap.
- The pre-session gate is `subagents_enabled`, not a hardcoded `true`:
  the builder strips `agent_swarm` wherever it strips `task`, and
  advertising it then offers a menu entry that resolves to literal text.
- The exit reminder is as emphatic as the doctrine it revokes; a single
  weak clause is the likelier of the two to be summarised away.

The doctrine rides the existing `push_system_reminder` channel rather
than a second injection path of its own.
2026-07-27 02:39:55 -04:00
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
ZacharyZhang-NY d3c9380307 release: v0.1.7
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
Since v0.1.6 — the cross-provider replay audit (Pi transform-messages
policy: every wire builder emits only items valid for its target):
- fix(responses): reasoning items without a native rs_* id dropped (the
  GPT/codex 400 Invalid 'input[N].id'); provenance gate drops foreign
  reasoning and demotes foreign backend tool calls on model switches
- fix(codex): bare rs_* reasoning references dropped (stateless backend)
- fix(wire): shared ASCII tool-call id sanitizer, symmetric call+result
  on both the Messages and Responses legs
- fix(messages): image-source whitelist (raster base64 only, http(s)
  urls only) and empty-user-content guard
- fix(conversation): char-boundary-safe code preview (CJK/emoji code
  panicked every request build)
- docs: AGENTS.md records the replay policy
2026-07-23 01:16:55 -04:00
ZacharyZhang-NY c950f8087c fix(messages): whitelist image sources and guard empty user content
Cross-provider audit M4/M5/M6 (Anthropic Messages builder):
- Non-base64 data: URIs rode as ImageSource::Url — url sources are
  http(s) only → 400; and the two image paths parsed data URIs
  differently (user path split on first comma, tool-result path on
  ';base64,').
- No media-type whitelist: image/svg+xml and param-carrying headers
  ('image/webp;name=x') reached the wire → 400.
- Empty user content shipped empty arrays/text blocks → 400.

One shared parse_base64_image_data_uri (raster whitelist: jpeg/png/gif/
webp) now serves both paths; rejected images degrade to a SHORT
'[unsupported image]' placeholder (never the multi-megabyte payload);
empty user turns get '[empty message]' (mirrors the assistant guard).

Part 6 of the cross-provider replay audit.

Verified: sampling-types 292 + downstream green, clippy clean.
2026-07-23 01:16:03 -04:00
ZacharyZhang-NY 40c71a8343 fix(responses): provenance-gate foreign turns (Pi transform-messages)
Cross-provider audit R5 + R2-residual: BackendToolCall items round-trip
as their typed shapes with provider-issued ids (grok x_search
CustomToolCall, web_search/code_interpreter calls) and Reasoning items
carry model-bound encrypted payloads — replaying either to a DIFFERENT
Responses target names undeclared tools / undecryptable material → 400.

New transform_items_for_responses pre-pass: each [Reasoning|
BackendToolCall]* Assistant run carries provenance in
AssistantItem.model_id; on confirmed mismatch with the request's target
model, Reasoning siblings are dropped and BackendToolCall demoted to the
same text summary the Messages/ChatCompletions builders already emit.
Same-model and unknown-provenance turns stay byte-verbatim (KV-cache
prefix stability preserved). The legacy-upgrade round-trip test now
models the same-model continuation it always described.

Part 5 of the cross-provider replay audit.

Verified: sampling-types 290 + downstream 5794 green, clippy clean.
2026-07-23 01:10:51 -04:00
ZacharyZhang-NY a3e3973453 docs(tests): restore doc comments displaced by the summary-test insertion
The new truncation test's doc landed between backend_tool_call_position_
stable's doc and its #[test] attribute (clippy: duplicated attribute).
Each test owns its own doc block again.
2026-07-23 01:02:19 -04:00
ZacharyZhang-NY 9cdc0ccfa3 fix(wire): shared ASCII tool-call id sanitizer, symmetric on both legs
Cross-provider audit R4+M3: the Responses leg passed tool-call ids
verbatim (call_id on both function_call and function_call_output) while
the Messages leg sanitized — and its closure used Unicode
is_alphanumeric, letting CJK ids through to Anthropic's ASCII-only
contract, with no empty-id fallback. Both providers enforce
[A-Za-z0-9_-]+ (the codex 400's own words). One module-scope
sanitize_tool_call_id now serves both builders, ASCII-only, empty → "_",
applied identically on call+result so pairing survives.

Part 4 of the cross-provider replay audit.

Verified: 6081 tests green across the four crates, clippy clean.
2026-07-23 01:00:56 -04:00
ZacharyZhang-NY 6f9f550308 fix(codex): drop bare rs_* reasoning references — stateless backend
Cross-provider audit R3: reasoning captured on stateful api.openai.com
sessions (no include requested) carries a server-issued rs_* id but NO
encrypted_content; replaying that bare reference to the stateless
(store:false) codex backend points at server state chatgpt.com does not
have. adapt_body_for_codex_backend step 3 drops such items (encrypted
ones pass through verbatim). Capture-side include for the API-key path
is deferred: the typed CreateResponse is shared with the xai Responses
leg and changing its bytes needs separate validation.

Part 3 of the cross-provider replay audit.

Verified: sampling-types+sampler+chat-state+shell all green.
2026-07-23 00:55:23 -04:00
ZacharyZhang-NY 2b43f54669 test(conversation): fix code-preview fixture to actually exceed the char cap
The multibyte-truncation test used 80 chars — below the 100-char cap, so
the truncation assertion failed (the previous commit's suite count was
misread; the FIX itself was correct and the panic repro held). 120 chars
now exercises both the boundary safety and the truncation.
2026-07-23 00:51:04 -04:00
ZacharyZhang-NY 6979407f22 fix(conversation): char-boundary-safe code preview in text_summary
The code-interpreter preview truncated at BYTE 100 — a guaranteed panic
on any CJK/emoji boundary in interpreted code. One poisoned history item
then crashed every subsequent request build on every backend (the
summary feeds both the Messages and ChatCompletions builders). Truncate
at 100 chars via char_indices instead. Panic pinned by test.

Part 2 of the cross-provider replay audit.

Verified: sampling-types 287 tests green.
2026-07-23 00:50:29 -04:00
ZacharyZhang-NY 1fa87566d9 fix(responses): drop reasoning items without a native rs_* id
Root cause of 400 Invalid 'input[N].id': '' on chatgpt.com/backend-api/
codex/responses: the Responses input builder replayed every stored
Reasoning item verbatim, and rs::ReasoningItem.id serializes
unconditionally — so foreign items (Messages-captured Anthropic
signatures, chat-completions-synthesized reasoning, stream-delta
fallbacks, legacy upgrades — all id '') reached the wire with an empty
id the API rejects. This also self-poisoned pure codex sessions whose
reasoning arrived only as deltas.

A native item always carries a server-issued rs_* id: empty id = foreign
= unusable by any Responses provider = dropped at the builder — the
exact mirror of the Messages builder's prune_replayed_thinking. The
encrypted-only fixture that pinned the poison shape now uses a native id
(the pass-through case it always meant to cover).

Part 1 of the cross-provider replay audit (Pi transform-messages
policy: builders emit only items valid for their target).

Verified: sampling-types 286 + sampler/chat-state/shell 5791 green.
2026-07-23 00:49:18 -04:00
ZacharyZhang-NY d6e49bcc7d release: v0.1.6
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.5:
- fix(messages): Claude models no longer 400 with 'Invalid signature in
  thinking block' — thinking is replayed only for the active tool loop
  and only when genuinely signed (cross-backend/cross-model histories
  are stripped)
- fix(codex): ChatGPT Codex no longer 400s with 'System messages are
  not allowed' — system prompts ride the instructions field and
  encrypted reasoning is requested for stateless replay
- docs: AGENTS.md records both wire contracts
2026-07-22 23:36:22 -04:00
ZacharyZhang-NY d2358b037c fix(codex): adapt the Responses body to the ChatGPT/Codex backend contract
Root cause of 400 {"detail":"System messages are not allowed"} at
chatgpt.com/backend-api/codex/responses (both platforms): the codex
adaptation covered only IDENTITY HEADERS (originator/OpenAI-Beta/UA/
chatgpt-account-id) — the BODY still carried the system prompt as
role:system input items, which the codex backend rejects outright. Its
system channel is the top-level  field, and stateless
(store:false) reasoning replay requires
include:["reasoning.encrypted_content"] — both per the same reference
the headers were ported from (official Codex CLI + Pi's
api/openai-codex-responses.ts).

New adapt_body_for_codex_backend (kigi-sampling-types): hoists every
system input item into  (order preserved, appended to any
existing instructions; string and parts content shapes) and requests
encrypted reasoning. Idempotent. Applied at both Responses send paths,
openai_codex-GATED — the API-key  path stays byte-identical
(pinned by a control wire test).

Tests: adapter unit tests (hoist+include, no-system no-op, idempotence)
plus two mock-server wire tests (codex body has no system role,
instructions + include present; plain Responses body unchanged).

Verified: sampling-types + sampler + chat-state + shell 6076 tests
green, clippy clean.
2026-07-22 23:35:31 -04:00
ZacharyZhang-NY 6ba24db019 fix(messages): replay thinking blocks only for the active tool loop
Root cause of 'Invalid signature in thinking block' (400 at
messages.1.content.0, Claude models): build_messages_request replayed
EVERY stored Reasoning item as a thinking block with no origin check —
history synthesized by other backends (encrypted_content: None → the
mandatory signature field serialized as ""), Responses-API tco_* blobs
(signature bytes, no text), and blocks signed by a DIFFERENT model after
a mid-session /model switch. Anthropic validates every replayed
signature (model-bound), so such histories 400 deterministically. The
platform split was circumstantial: Windows sessions started on the
default model and switched to Claude; macOS sessions were Claude-native
from turn 1. Adversarially verified — no platform-divergent byte path
exists in capture, storage, or replay.

New prune_replayed_thinking pass (Pi/Claude Code replay policy): keep
exactly the final assistant message's thinking when its tool loop is
still open (request ends on the tool results — an open loop can never
span a model switch) and the block is genuinely signed; strip every
other thinking block (the API ignores valid prior-turn thinking and
rejects invalid). Assistant messages emptied by the strip (thinking-only
aborted turns) are removed — empty content arrays are rejected too.

Tests: three unit tests pin strip-outside-loop (unsigned, tco_*, stale
signed), keep-in-active-loop (verbatim text+signature at content.0), and
emptied-message removal; the legacy-upgrade integration test now proves
both wire fidelity in the active loop AND stripping once the loop
closes.

Verified: sampling-types + sampler + chat-state + shell 6072 tests
green, clippy clean.
2026-07-22 23:26:49 -04:00
ZacharyZhang-NY 10149f50dd install: stop persisting KIGI_GRAPH — the binary default is the product default
The README always shipped graph engineering enabled; the enablement was
delegated to installer env plumbing that diverged per platform:
install.sh exported KIGI_GRAPH=1 into shell rc (worked), install.ps1
wrote the User registry variable — which running Windows terminals (and
new tabs of an open Windows Terminal) never pick up, so /graph was
'missing on Windows' despite a successful install.

With resolve_graph() defaulting true in the binary (e53a66d), the env
writes are redundant complexity: drop them from both installers, keep
KIGI_GRAPH=0 as the documented opt-out (env still beats the default),
and correct the flag comment to tell this story instead of a
'gray release' one. Both scripts syntax-checked (sh -n / pwsh parser).

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

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