MythEclipse/shiro-neko
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix the loop stalling after compaction, add an external registry

Browse Source 
The compaction bug, which is the important one:

beta.2 taught the pruner to drop any assistant part whose reasoning item it had
removed. That was right about the 400 and wrong about everything else. On a
reasoning model every tool call carries a provider itemId, so past the threshold
the model could no longer see what it had already run, and re-ran the same tools
until maxSteps ended the turn. Reproduced at 12 model calls for a job needing 4,
with nothing but the user message reaching the wire.

The dependency is not the part, it is the itemId. A part carrying one is
serialised as `{ type: 'item_reference', id }`, a pointer to an item stored
provider-side that depends on its reasoning item. Without the itemId the same
content goes out inline and carries no dependency at all. Verified against the
provider's own serialiser: `text` with an itemId becomes item_reference, the
identical part without one becomes output_text.

So `dropOrphanedItems` becomes `detachOrphanedItems`: strip the itemId, keep the
content. Compaction may shorten the history; it must not blank it. The new test
asserts behaviour rather than shape — the loop must end because the model chose
to, and every call after the first must still carry the earlier exchange. A shape
assertion passed the whole time the model was losing its memory.

Registry, via `/registry [list|search|add|remove|installed]`:

Skills and plugins are treated differently on purpose. A skill is prompt text, so
installing one puts a stranger's words into the system prompt of every future
session in this project; the install shows the body first and the origin is
recorded, so /skills always says where an instruction came from. A plugin is a
JSON manifest of deny rules, evaluated by compiled code identical for every
install. Loading TypeScript from a URL is declined outright: a plugin that can
block tool calls could otherwise lie about blocking them.

Validated before anything is written: https only (file: and data: rejected), name
matched against ^[a-z0-9][a-z0-9-]*$ so it cannot escape its directory, size
caps on index and body, every regex compiled, pattern length capped since it runs
on every tool call, and the body's own name checked against the index. Installed
skills rank below your own, so an install can never shadow a skill you wrote.

Interface:
- Context is a percentage of the compaction threshold, amber from two thirds and
  red at 90. A turn about to lose history now says so beforehand.
- Aligned command menu and registry tables; /skills and /plugins name origins.

538 tests, up from 488. The registry is tested against a real local HTTP server,
and the guard is proven to refuse a .env write end to end rather than assumed to.
This commit is contained in:
Muhammad Zakir Ramadhan
2026-09-03 03:07:56 +07:00
parent 8b1895de98
commit 84c60f2022
26 changed files with 1929 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/architecture.md
+35 -9
View File
@@ -176,16 +176,25 @@ Only `api.openai.com` gets the chain. Third-party endpoints do not implement `/v
## Compaction and its repair
Pruning breaks two different provider invariants, and `src/prune.ts` repairs both.
Pruning breaks two provider invariants, and `src/prune.ts` repairs both.
**A message without its reasoning item.** `pruneMessages({ reasoning: 'all' })` strips a
reasoning item and keeps the message item from the same response. The responses API treats the
message as that reasoning item's dependent and returns 400.
**A message detached from its reasoning item.** `pruneMessages({ reasoning: 'all' })` strips a
reasoning item and keeps the message item from the same response. A part carrying a provider
`itemId` is not sent inline: the responses provider serialises it as
`{ type: 'item_reference', id }`, pointing at an item stored on their side, and that stored
item depends on the reasoning item that pruning just removed. The result is a 400.
The two carry different ids, so they cannot be matched by id. What links them is the assistant
message they arrived in: one message is one response, and its reasoning item covers every
other item in it. `dropOrphanedItems` drops the dependent parts of any turn whose reasoning was
removed — which costs nothing, since pruning was already discarding those turns.
other item in it. `detachOrphanedItems` strips the `itemId` from those parts, which is what
sends the same content inline instead — verified against the provider's own serialiser, where
a `text` part with an itemId goes out as `item_reference` and the identical part without one
goes out as `output_text`.
Dropping the parts was the first attempt and it broke the loop. On a reasoning model every
tool call carries an itemId, so after the first compaction the model could not see what it had
already run, and re-ran the same tools until the step limit ended the turn. **Compaction may
shorten the history; it must not blank it.**
**A tool result without its tool call.** `toolCalls: 'before-last-3-messages'` counts
*messages*, so the cut lands between an assistant `tool-call` and the `tool` message answering
@@ -199,6 +208,17 @@ it. What reaches the wire is a `function_call_output` with no `function_call`:
reverse pairing is deliberately left alone: a call still awaiting its result is exactly what a
suspended approval looks like, and dropping it would break resume.
## Registry
`/registry` fetches an index of external skills and plugins over https. Skills are prompt text
and are shown in full before install; plugins are a JSON manifest of refusal rules, never code.
The guard evaluating those rules is compiled, identical for every installed plugin, so an entry
from a registry cannot execute anything. See [registry](registry.md) for the validation and
the reasoning.
`src/registry.ts` has no UI and no side effects until `install()` is called, which is what lets
`stage()` show a body before it becomes part of every future prompt.
## Module map
| Module | Responsibility |
@@ -208,6 +228,7 @@ suspended approval looks like, and dropping it would break resume.
| `tools-git.ts` | read-only git tools, spawned with a fixed argv |
| `ignore.ts` | gitignore-aware walker, path jail |
| `complete.ts` | `@path` token extraction, ranking, insertion |
| `registry.ts` | external index, validation, install and removal |
| `prompt.ts` | system prompt assembly from live state |
| `agents.ts` | variants, thinking levels |
| `skills.ts` | discovery, catalogue, `skill` tool |
@@ -234,10 +255,15 @@ Every module is pure of the UI except `ui/`, and `ui/` never touches the SDK. Th
## Testing
482 tests, no mocking framework. `MockLanguageModelV4` from `ai/test` drives the loop;
538 tests, no mocking framework. `MockLanguageModelV4` from `ai/test` drives the loop;
`ink-testing-library` drives the UI with real keystrokes; MCP is tested against a real stdio
server subprocess; provider wire formats are tested against a local HTTP server; the interrupt
path spawns a real subprocess and asserts it died early rather than ran out.
server subprocess; provider wire formats and the registry are tested against a local HTTP
server; the interrupt path spawns a real subprocess and asserts it died early rather than ran
out.
The pattern throughout is to assert on what actually crossed a boundary — what went on the
wire, what is on screen, what is on disk — rather than on internal calls.
Compaction is asserted on **behaviour**, not shape: the loop must terminate because the model
chose to, and every call after the first must still carry the earlier exchange. A shape
assertion would have passed while the model was losing its memory.
docs/configuration.md
+5 -1
View File
@@ -22,6 +22,7 @@ Written by `/provider`, editable by hand. Every field is optional.
"maxRetries": 3,
"plugins": ["guard", "time"],
"toolSets": ["edit-plus", "git"],
"registryUrl": "https://example.com/my-registry/index.json",
"mcpServers": {
"fs": { "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "."] }
}
@@ -38,8 +39,9 @@ Written by `/provider`, editable by hand. Every field is optional.
| `agent` | default variant: `default`, `quick`, `deep`, `plan`, `review` |
| `thinking` | default level: `off`, `low`, `medium`, `high`, `max` |
| `maxRetries` | retries per model call for transient failures. Default 3 |
| `plugins` | which plugins to enable. Omit for `["guard", "time"]` |
| `plugins` | which builtin plugins to enable. Omit for `["guard", "time"]` |
| `toolSets` | optional tool sets beyond `core`: `edit-plus`, `git`. Omit for all of them. See [tools](tools.md) |
| `registryUrl` | index for `/registry`. Omit for the default. See [registry](registry.md) |
| `mcpServers` | see [MCP](mcp.md) |
## Provider presets
@@ -117,6 +119,8 @@ cat file | shiro -p prompt read from stdin
memory/<hash>.json durable per-project notes
history/<hash>.json prompt history for up-arrow recall
skills/*.md your own skills
registry/skills/*.md skills installed with /registry
registry/plugins/*.json plugin manifests installed with /registry
```
Project files:
docs/development.md
+4 -1
View File
@@ -16,7 +16,7 @@ faster and the fallback path is exercised without it.
```bash
bun run shiro # run from source
bun run typecheck # tsc --noEmit
bun test # 482 tests
bun test # 538 tests
bun run build # single binary for this platform -> dist/shiro
bun run release # all five platforms -> dist/release + SHA256SUMS
bun run install:local # build, then copy onto PATH
@@ -71,6 +71,9 @@ mock-verification test:
request body
- `pruneMessages` leaving a tool result without its tool call — same, and it took a stub
endpoint that rejected the pairing to prove the fix
- Compaction blanking the model's memory of its own tool calls — invisible in any single
request, and visible only as "the loop ran to its step limit". Caught by asserting the loop
terminated because the model chose to, not that the messages had a particular shape
- `--json` serialising `Error` as `{}` — visible only in the printed output
- Automatic approval requests prompting the user — visible only in the event sequence
- `ctrl-c` killing `cmd /c` but not the command under it — visible only as elapsed time, since
docs/memory.md
+13 -5
View File
@@ -139,17 +139,25 @@ Pruning breaks two provider invariants. `src/prune.ts` repairs both, and both we
before it did.
**A message without its reasoning item.** `pruneMessages({ reasoning: 'all' })` strips a
reasoning item and keeps the message item from the same response. The OpenAI responses API
treats the message as a dependent of that reasoning item and rejects the request:
reasoning item and keeps the message item from the same response. That message carries a
provider `itemId`, and the OpenAI responses provider serialises anything with one as
`{ type: 'item_reference', id }` — a pointer to an item stored on their side, which depends on
the reasoning item that is now gone:
```
400 Item 'msg_…' of type 'message' was provided without its required 'reasoning' item: 'rs_…'
```
The two carry different ids, so they cannot be matched by id. What links them is the
assistant message they arrived in — one message is one response. `dropOrphanedItems` drops the
dependent parts of any turn whose reasoning was removed. That costs nothing, because pruning
was already discarding those turns.
assistant message they arrived in — one message is one response. `detachOrphanedItems` strips
the `itemId` from those parts. Without one the same content is serialised **inline**, which
carries no dependency on anything stored, so the turn survives intact.
Dropping the parts instead was the first attempt, and it was wrong in a way that only showed
up over a long turn: on a reasoning model every tool call carries an itemId, so after the
first compaction the model could no longer see what it had already run. It re-ran the same
tools until the step limit ended the turn. The history is the model's memory; compaction may
shorten it but must not blank it.
**A tool result without its tool call.** `toolCalls: 'before-last-3-messages'` counts
*messages*, not pairs, so the cut can land between the assistant message holding a `tool-call`
docs/plugins.md
+25 -9
View File
@@ -4,8 +4,17 @@ A plugin extends the agent in four ways: it can add tools, mark tools auto-appro
a tool call before it runs, and append to the system prompt. It can also run something after
each turn.
Plugins are compiled into the binary. Loading them from disk is deliberately not supported
yet — see [ROADMAP.md](../ROADMAP.md).
Two kinds exist, and only one can contain code:
- **Builtin** plugins are compiled into the binary and may do anything in the interface below.
- **Installed** plugins come from a registry as a JSON manifest of refusal rules. They are
data: the guard evaluating them is compiled code, identical for every install. See
[registry](registry.md).
Loading TypeScript from disk or a URL is deliberately not supported. A plugin that can block
tool calls can also lie about blocking them, and one that could execute could read every file
the agent can read. That is a sandbox problem, not a loader problem — see
[ROADMAP.md](../ROADMAP.md).
## Enabling
@@ -13,9 +22,12 @@ yet — see [ROADMAP.md](../ROADMAP.md).
{ "plugins": ["guard", "time"] }
```
That is also the default when the field is absent. `--no-plugins` disables all of them,
including the guard. `/plugins` lists what is active and reports any name that did not
resolve.
That is also the default when the field is absent, and it lists **builtin** plugins only.
Installed plugins are always active once present, because installing one was the decision to
enable it; remove it with `/registry remove <name>`.
`--no-plugins` disables everything, builtin and installed, including the guard. `/plugins`
lists what is active, marks installed entries, and reports any name that did not resolve.
## The interface
@@ -92,7 +104,11 @@ intrusive, but it is genuinely useful when a turn takes minutes.
## Writing one
Plugins live in `src/plugins-builtin.ts` and are registered in `BUILTIN_PLUGINS`.
A refusal rule is usually better as an installed manifest: no rebuild, and nothing to review.
See [registry](registry.md) for the manifest shape. Reach for a builtin only when the plugin
needs to contribute a tool or run something after a turn.
Builtin plugins live in `src/plugins-builtin.ts` and are registered in `BUILTIN_PLUGINS`.
```ts
export const noSecretsPlugin: Plugin = {
@@ -122,6 +138,6 @@ refusal it was never told about and tries to route around it.
## Ordering
Plugins run in the order they are enabled. The first `beforeToolCall` to block wins;
later hooks are not consulted. `afterTurn` runs every hook, and one throwing does not stop
the rest.
Builtin plugins run first, in the order they are enabled, then installed ones. The first
`beforeToolCall` to block wins; later hooks are not consulted. `afterTurn` runs every hook, and
one throwing does not stop the rest.
docs/registry.md
+128
View File
@@ -0,0 +1,128 @@
# Registry
External skills and plugins, browsed and installed from the CLI.
```
/registry everything in the index
/registry search <query> narrow by name or description
/registry installed what is already here
/registry add <name> fetch, show, then install on confirmation
/registry remove <name> delete an installed entry
```
```
registry
3 of 3 available
S migration Write or run a database migration
S commit-style Write commits the way this team does
P no-secrets Refuses writes to credential files installed
S skill P plugin | /registry add <name>
```
`esc` dismisses the panel.
## The two kinds are not equally safe
**A skill is instructions.** Installing one puts a stranger's words into the system prompt of
every future session in this project. That is prompt injection by invitation, so the install
shows the body first and `/skills` always says the origin:
```
install skill "migration"?
https://raw.githubusercontent.com/example/registry/main/skills/migration.md
Migrations live in `db/migrations/` and are timestamped, never renumbered.
Run `bun run db:migrate` locally first. Staging runs them on deploy.
A skill is instructions the agent follows. This text joins your system prompt.
y install | n cancel
```
**A plugin is data.** Never code. A manifest declares refusal rules; the guard that evaluates
them is the same compiled code for every installed plugin:
```json
{
"name": "no-secrets",
"description": "Refuses writes to credential files",
"appendix": "The no-secrets plugin refuses writes to .env and credential files.",
"deny": [
{
"tools": ["write_file", "edit_file", "multi_edit"],
"pathPattern": "(^|/)\\.env|credentials|\\.pem$",
"reason": "refusing to write a credential file; add secrets yourself"
}
]
}
```
Loading TypeScript from a URL is not offered at any price. A plugin can block tool calls, so
one that could also execute could read every file the agent can read and lie about blocking
anything. See [plugins](plugins.md) for why this boundary exists.
## What is validated before anything is written
| Check | Why |
|---|---|
| `https` only, `localhost` for tests | `file:` would read a local path, `data:` would inline a payload |
| Name matches `^[a-z0-9][a-z0-9-]*$` | the name becomes a filename, so `../evil` must not parse |
| Index at most 256 KB, body at most 64 KB | a hostile index should not exhaust memory |
| Manifest against a strict schema | extra keys like `beforeToolCall` are dropped, not honoured |
| Every pattern compiles as a regex | a broken pattern would fail on the first tool call instead |
| Pattern at most 200 characters | it runs on every tool call; a pathological one is a denial of service |
| Body name matches the index name | an index entry cannot serve something else under a trusted name |
| At least one deny rule | a plugin with no rules is only prompt text, which is what a skill is for |
A malformed installed plugin is reported by `/plugins` and skipped. One bad install does not
stop the agent from starting.
## Where installs land
```
~/.shiro-neko/registry/
skills/<name>.md loaded as origin "registry"
plugins/<name>.json loaded as a declarative plugin
```
Precedence for skills, low to high: **builtin → registry → user → project**. A skill you wrote
in `~/.shiro-neko/skills/` or `.shiro/skills/` always beats one fetched from a registry, so an
install can never silently shadow your own work.
Installs take effect on the next start. A skill joins the system prompt and a plugin joins the
guard chain, and both are assembled once at boot; hot-swapping either mid-session would mean a
turn whose rules changed underneath it.
## Pointing at your own index
```json
{ "registryUrl": "https://example.com/my-registry/index.json" }
```
The index is one JSON document:
```json
{
"skills": [
{
"name": "migration",
"description": "Write or run a database migration",
"url": "https://example.com/skills/migration.md",
"author": "you"
}
],
"plugins": [
{
"name": "no-secrets",
"description": "Refuses writes to credential files",
"url": "https://example.com/plugins/no-secrets.json"
}
]
}
```
Both arrays are optional. A name may appear once as a skill and once as a plugin; `/registry
add skill:review` disambiguates, and an ambiguous name is refused rather than guessed.
A private index is just a URL you control. There is no account, no token, and no telemetry —
`/registry` makes exactly one GET for the index and one for the entry you install.
docs/skills.md
+6 -4
View File
@@ -30,14 +30,16 @@ to X" — not as a summary.
## Where they load from
Three sources, later overriding earlier by name:
Four sources, later overriding earlier by name:
1. **builtin** — compiled into the binary
2. **user** — `~/.shiro-neko/skills/*.md`
3. **project** — `.shiro/skills/*.md`
2. **registry** — `~/.shiro-neko/registry/skills/*.md`, installed with `/registry add`
3. **user** — `~/.shiro-neko/skills/*.md`
4. **project** — `.shiro/skills/*.md`
A project skill named `debug` replaces the bundled one entirely. `/skills` shows what
loaded and where each came from.
loaded and where each came from — which matters most for `registry`, since that body came
from someone else and is now in your system prompt. See [registry](registry.md).
`--no-skills` skips all of them, builtin included.