Restructure: agent-centric model + demolition - #673
Closed
Wirasm wants to merge 51 commits into
Closed
Conversation
Captures the reframe from Room-as-primitive to agent-as-primitive, and audits the whole tree against it. The model (docs/model.html): a kild is a worktree/workstream; the agent is the sole inhabitant type on one mechanism axis (owned vs attached); the honryo is an agent wearing human-authority intelligence — in practice the CLI a human drives. Messaging is directed and peer-to-peer; spawn defaults to sharing a tree, forking one is the deliberate act. All process and persona lives outside the engine. The audit (docs/DEMOLITION.md, docs/demolition.html) marks every file keep/reshape/move/die. Two separate teardowns fall out: the Flue lane is dead code no live path imports (kild/run.ts, kild/auth.ts, flue/, workflows/, flue.config.ts, the @flue/runtime dep), and the room choreography — router, lifecycle, decisions ledger, operator tier — is intelligence living in the engine. No code changes; these are design documents.
The extension stays and is rebuilt, not retired. It is not "the operator tier for pi" — it is the pi implementation of the attached-harness contract, peer to Claude Code's: attach a handle, drain an inbox at the turn boundary, act through the same API every client uses. pi's integration is the better of the two (a native turn-boundary drain rather than a shell hook), which is why it earns its keep. A third harness later is another implementation of the same contract, not another mechanism in the engine. Records what the rebuild deletes (the operator guide and prompt injection, the decision vocabulary, @Human semantics) and what it keeps (the REST/WS client, bootId restart detection, the event bridge that is pi's drain).
Five docs describing work that has either shipped or is being torn out: - engine/COMPARISON.md — the Flue-vs-Rust decision record, defending a bet that is being deleted from the tree - docs/fleet-observability.md — spec/tracker; phases 1-2 shipped, phase 3 is superseded by the agent-centric reframe - docs/room-mailbox-notes.md — review notes on the room module being dismantled - docs/pi-extension-plan.md — plan realized by the extension, which is now being rebuilt in a new shape - docs/upstream-worktree-sandbox.md — proposal to upstream the Flue sandbox that is going away docs/DEMOLITION.md records why each one goes.
Nothing on the live path imported any of it. The engine runs server.ts -> worker.ts, which resolves models through pi's ModelRuntime directly; the Flue modules were reachable only from each other and from frozen demos that workflows/README.md already described as off the hot path. Deleted kild/run.ts (Flue in-process runner), kild/auth.ts (bridged pi credentials into Flue's configureProvider, imported only by a demo), src/flue/ and src/workflows/ entire, and flue.config.ts. Dropped the @flue/runtime dependency, the @flue/cli devDependency, and the flue script. DEFAULT_MODEL goes with them: its only importers were deleted files, and model resolution reads KILD_MODEL directly. The one deleted test, flue/worktree-sandbox.test.ts, covered a deleted module. 260 tests pass, typecheck and lint clean.
Every MOVE needs a mechanism counterpart. Deleting the intelligence and its trigger together does not relocate a capability, it removes one — and the first pass of the audit tracked only what leaves. Two cases it coupled without noticing: Memory synthesis loses its only trigger when room-close goes. The engine keeps the seam: a lifecycle event on close/land carrying the facts it holds, and a generic hooks.onClose the engine runs without knowing what it is for. The charter and persona are what move to PRP; memory.synthesis generalizes into that hook rather than disappearing. finalNonSystemPost dies with room-events.ts, and it is what writes the ledger's outcome line. It should not be ported: it is a tail-grab standing in for facts the engine already owns. The ledger instead records the land result, commits vs base, changed files, and per-agent model and cost — all from git-review, worktree-status and the session bus. Prose outcome belongs to PRP's synthesis. Also records the cross-repo call: helm cannot land atomically with kild, but it is unaffected until the one commit that renames /api/rooms to /api/kilds. helm pins across that commit rather than the engine dual-serving both route families.
kild had three baked-in ranks: operator, lead, worker. The operator was a privileged session type the engine granted room-control tools to. In the agent-centric model the human's agent CLI drives the REST API as an ordinary client, so there is no tier to grant: kild ships no brain. Removes the four operator tools, the KILD_OPERATOR capability in worker.ts and server.ts, and the whole `kild operator` CLI group. fleetMemorySection goes with them — it existed only to give operator sessions cross-project memory and had no other caller. engine-client.ts and rooms-status.ts move up out of operator/: neither is an operator concern. rooms-status carries the cross-kild collision computation, which is general observability that was only ever filed in the wrong folder. One test goes with its subject (fleetMemorySection); no other test failed at any point. 259 tests pass, typecheck and lint clean. pi-extension still references the removed capability. It is rebuilt against the new shape in a later commit rather than patched twice.
Three things the engine enforced that are the intelligence layer's to
decide, not mechanism kild should ship.
The decisions ledger: needs-decision[key] markers folded from post text
into a room ledger that blocked close until resolved, with a force
escape hatch to bury it. A protocol, and one the engine had no business
parsing out of prose. Gone with its force parameter, its REST field and
its paragraph in the mechanism prompt.
The idle nudge: a participant that finished a turn without posting got
prompted to report. That is a reporting norm. Deleted — but note the line
drawn here: `idle` itself is kept. It is state, not a norm (it rides the
participant view so a client can see who is finished and waiting, and an
empty mailbox drain is how an attached participant signals the same
thing). What goes is the engine telling an agent how to behave.
Implicit replies: an agent's turn-final narration was auto-posted so the
human could see it, and the router then needed a rule to stop that
narration from waking other agents. Both halves go; reaching another
agent takes an explicit post, which is what the tool always claimed.
The mechanism prompt also advertised the nudge ("kild nudges IT to
report"), which would have become a false promise — removed.
24 tests go with their subjects, each listed in the review notes; no test
was removed to green a gate. 235 pass, typecheck and lint clean.
VOCABULARY.md fixes three collisions the codebase had grown. `agent` meant the persona file in the CLI, the same data was `/api/personas` over REST, and `listAgents()` in code — while the running thing had no stable name at all (worker, participant, session, depending where you looked). `role` meant both the persona wrapper tag and the process role. `session` meant both a running agent and pi's conversation. Now: persona is the file, agent is the process, session is pi's alone. One word per concept, one concept per word, plus the rules that keep it that way — no rank words, and upstream keeps its own vocabulary. helm-migration.md is the client-side guide: every route, payload field and WS frame that moves, what disappears entirely (implicit, decisions, posted, stopped, force) and why, and the pin/un-pin sequence. helm and kild cannot merge atomically, so the rename is deliberately one commit and therefore one pin boundary.
The migration guide claimed git and totals were unchanged "including collidesWith". Wrong: collidesWith is produced by computeCollisions inside compactLiveKilds, which is the CLI's compact view. It is not on GET /api/kilds and never was. What the endpoint does carry is git.changedFiles, which is everything a client needs to derive collisions itself — so deriving them client-side is the correct approach, not a workaround. A guide claiming to be everything a client needs has to be right about what is on the wire. Also documents why /api/worktrees survives next to /api/kilds (a worktree outlives its kild: listing and pruning a stranded tree must work when there is no kild to ask about), flags the duplicate transcript routes as resolved-before-you-migrate, and states that `from` is already engine-derived — supplying it is a hard rejection, not silent acceptance. worktree-disposal.md records a live bug found while auditing: any untracked file marks a tree dirty, dirty trees refuse removal, and prune swallows the failure in a bare catch. Tooling that writes residue into every tree at creation therefore makes every tree unreclaimable from birth, with nothing reporting it. Measured at 116 stranded trees. The silent catch is the defect that let a policy disagreement become a leak.
The rename was 1:1 by design — 25 endpoints in, 25 out — which is right for crossing a pin boundary but means the demolition has not yet reached the surface clients touch. These are the calls for the step that does. An agent is addressed by handle. This reverses helm-migration.md, which said id should win because handle is "an addressing concept, not a routing key" — but that distinction is exactly what produces two identifier schemes for one object, which is the shape the reframe exists to delete. Verified safe: an agent stays on the roster after it stops, so a handle never rebinds. Prompting an owned agent is therefore just sending it a message, and the two delivery paths collapse to one. Disposal gets a verb, guarded on authored commits rather than a clean tree. Measured: 116 worktrees, zero reclaimable, and 27 of 27 sibling trees refused as dirty from provisioning litter written before any agent ran. A branch carrying unmerged commits is real work and the branch is the safety net; uncommitted litter is not work, and treating it as work is what produced 116 permanent trees. /api/worktrees folds into /api/kilds, with the requirement that the listing enumerate from git rather than the registry — otherwise a tree whose kild record is gone has no id to address, which is the same stranding being fixed. Attribution moves to a credential minted at attach, sequenced with the router commit since send is where from is resolved. Notes two corrections: the engine's session id is engine-assigned, not a pi detail, and `from` is already rejected on request bodies rather than caller-supplied.
Agents in different kilds cannot reach each other, so a human copies
messages between them. Not only a cross-repo problem: the agent in
kild/auth that finds an API change affecting kild/api has no way to say
so either. Parallel workstreams that cannot talk are half-parallel.
The tempting design is a kild-to-kild channel. Rejected — the endpoint is
an agent, not a kild, and what is missing is address space, not a channel.
A second messaging system would grow its own delivery rules, its own
guard, and its own log, and drift. This whole reframe exists because a
second answer to "who is this addressed to?" was allowed to exist.
So: widen the address on the existing send. Structured {kild, handle},
kild always an id, never a name — names are not unique across an engine,
and resolving them in the engine would reintroduce exactly the
name-or-path guessing server.ts already removed after it opened a kild
whose worktree could not be created and still returned success. Friendly
name/handle resolution belongs at the caller.
One real change falls out: the wake cap must stop being attached-only.
It guards attached inboxes today because only attached agents could
ping-pong; once any agent can address any other, two owned agents loop
just as easily and cost more, since nobody watches a kild the way a human
watches their own terminal.
Also records the stopgap that works today with no new mechanism (attached
agents do not run in their kild's tree, so a kild can serve as a pure
channel), and why removing the human from the transport makes the keyed
decision protocol in PRP more load-bearing rather than less.
cross-kild-comms.md ended by naming this gap without answering it. PRP raises, kild carries, helm surfaces — and helm owns the surfacing permanently rather than until something better exists. A decision is the one thing the engine produces that is worthless if nobody sees it; everything else is designed to run unattended. Reaching a screen and persisting until acknowledged is a client's job by definition. kild adds nothing for this. `idle` already identifies an agent that went idle after asking — which is exactly why it survived the process-norms cut as observable state rather than a norm — and the planned monotonic `seq` covers "new since I last looked". No escalation type, no engine verb: deciding that a message is a question rather than a status update is interpretation, and the keyed-decision ledger was just removed for parsing exactly that out of prose. The requirement helm must meet is borrowed from the disposal leak, which only became visible when someone counted at 116 trees: unanswered must be loud and countable. If helm cannot answer "how many agents are blocked on me right now" at a glance, the mechanism is not finished. A second app is deferred rather than rejected — it is the right answer if escalations must reach someone when helm is closed, but that is a conclusion to reach after the cheap version shows how often this fires.
One atomic rename, no behavior change, so it reviews as names moving and crosses helm's pin boundary exactly once. Room becomes Kild — the unit of work is a git worktree, not a social construct with its own lifecycle. Participants become agents, the sole inhabitant type. The room/ slice flattens into the domain root, since Kild is now the domain rather than one slice within it. Three collisions are resolved in the same pass, because they were the reason the vocabulary could not be made consistent piecemeal: `agent` meant three things — the persona file in the CLI (`kild agent ls`), the same data as /api/personas over REST, and listAgents() in code — while the running thing had no stable name at all, being variously a worker, a participant, or a session. Now persona is the file and agent is the process. `role` meant both the persona wrapper tag (`withRole`, `<role>`) and the process role (KILD_ROLE). Now `<persona>` wraps personas and KILD_ROLE is the only role. `session` meant both a running agent and pi's conversation. It now means only pi's, which is whose word it is. worker goes with them. It is rank vocabulary — a worker implies a lead and an operator above it, which is the org chart this branch deleted — and it imports the job-queue model of an interchangeable executor pulled from a pool, which an addressable agent that holds context and spawns peers is not. The tell was server.ts having to explain that KILD_ROLE=worker "runs a single agent session". post goes too: you post to a board, you send to a person. Broadcast vocabulary in a system whose whole point is that the sender names the recipient. REST moves to /api/kilds and /api/agents, WS frames follow, and the CLI loses its room subcommand group. HUMAN, the lead default and the lifecycle states are renamed but deliberately intact — they are removed in the router commit, and mixing that into this diff would make it unreadable. 235 tests pass across 26 files, exactly the count before: a rename must not change it, and no test was deleted. Typecheck and lint clean. Engine boots and serves /api/kilds and /api/agents; /api/rooms/live is a 404.
The skill is the contract every agent drives kild through, and it documented commands that no longer exist: the whole `kild operator` group (deleted with the operator tier), `kild room *` (now top-level verbs), `kild sessions` (now `kild agents`), and `kild agent ls` (now `kild persona ls` — the word `agent` moved to the running process). Two claims were not just renamed but false. It described delegation via `invite_agent`/`post_message`, which are now `spawn`/`send`. And it promised "if a delegate finishes a turn without posting, kild nudges IT to report" — that failsafe was deleted as a process norm belonging to PRP, so the skill was teaching agents to rely on a mechanism that no longer exists. It now says plainly that reporting is the agent's responsibility and nothing will chase it. Adds the vocabulary section so the persona/agent/kild/handle distinction is stated where agents actually read it, and points at worktree-disposal.md on prune, since prune only reclaims merged and clean trees and abandoned work otherwise accumulates silently.
The hook drained with `kild room drain`, which is now `kild inbox`. Every failure path here exits 0 by design — a hook must never block a turn — so the rename would not have raised anything. It would simply have stopped delivering mail, silently, which is precisely the failure its own header warns about in the generation before it. Also moves the wiring env to KILD_KILD_ID / KILD_HANDLE, matching what the engine sets on the agents it spawns, so an attached agent describes itself the same way an owned one does. Adds the note this file was missing: because everything here fails silently, the CLI verbs are the one thing to re-check whenever the command surface moves. Verified end to end against a live engine — attach, send, drain returns the Stop-hook block JSON naming the new verbs, and a second drain prints nothing and exits 0.
The guide told helm to pin across the rename and un-pin after, which was true when the rename was the only breaking change. It is not anymore: the router commit drops `system` from messages and `state`/`stopped` from kilds, and requires `to` on every send; the reshape changes routes again. Porting to each intermediate state would mean doing the work three times, so helm stays pinned until the reshape lands and ports once against the final surface. Documents the combined delta of the rename plus the router commit, including the two things a client is most likely to have built on — system notices in the thread, and keying off `state`. Also records the split the router commit is careful about: the engine never infers a recipient, but the CLI may resolve `--to` client-side when a kild has exactly one agent. Convenience in the client, never a rule in the engine.
The extension was the operator tier for pi: it injected a <kild-operator> guide, spawned sessions with operator: true, spoke the needs-decision vocabulary and special-cased @Human. All of that was intelligence the engine no longer has and the extension has no business shipping. It is now the pi implementation of the attached-harness contract, peer to Claude Code's hook plus CLI. Eleven tools mirror the CLI verbs one to one, described in mechanism terms only. The bridge is now literally the drain. A {message} frame addressed to a handle this session attached is only the wake signal; the mail itself always comes from the inbox drain, so the engine's destructive-read and wake-cap semantics apply to pi exactly as to any other harness rather than pi having a privileged push. Reconnect drains each attached handle once, since the inbox is engine state and a socket gap loses nothing. Two things the rebuild surfaced. There is no REST route to spawn an agent into a kild — it is WS-only and fire-and-forget, so kild_spawn cannot report a truthful synchronous failure and instead polls and reports what it observed. And kild_new does not attach the caller, so entering the drain contract is an explicit act now that there is no privileged @Human push. 727 → 844 lines. The ~400 estimate predated the decision to mirror the CLI verbs one to one; the tool definitions alone are ~340.
Addressing bugs bit in three separate places because a shared broadcast
log forced the engine to guess recipients. The sender now names them, and
the whole class goes with the guess.
`to` is required and never inferred. Gone with the defaulting: the lead
(agents[0]-is-lead, KILD_LEAD, the lead-only guard on stop), the
@human-wakes-lead rule, and the 1:1 bare-send special case — a solo kild
now gets the same answer as any other, because a rule that only applies at
one size is a rule the engine has to remember.
HUMAN is deleted outright. A human-driven harness attaches a handle like
anything else; there is no reserved participant and no privileged push.
kild-router.ts did not survive as a file. Once the choreography went, what
remained was a two-branch loop over state the manager already owns — the
injected Delivery seam existed to unit-test routing decisions, and there
are no decisions left to isolate.
Also removed: system messages (roster changes come from the {kilds}
broadcast, not fabricated log entries), the lifecycle state machine and
its six guards (liveness is registry presence; halt collapsed into stop),
and opener notifications (an agent that wants to tell the opener something
sends it a message).
finalNonSystemPost deliberately survives, moved into memory.ts — its
replacement lands with the fact-based ledger, and removing it first would
delete the ledger's outcome line before anything could write one.
222 pass, 0 fail. 13 tests removed with their subjects and listed in the
review notes; the rest rewritten rather than dropped. New tests pin the
rules that replaced them, including cli.send.test.ts on the split that
matters: the engine never infers a recipient, but the CLI may resolve --to
client-side when a kild has exactly one agent.
Wirasm
force-pushed
the
claude/buzz-block-kild-comparison-jdcub4
branch
from
July 27, 2026 20:41
a7b4e3a to
1777a0d
Compare
The extension was rebuilt against the API as it stood an hour earlier, and the router commit moved four things under it. All seven claimed changes were checked against engine source first; every one held. `kild_send` advertised "omit to to address the kild lead" and sent the field only when non-empty. There is no lead and the engine now rejects an absent `to`, so the tool was documenting a rejection. `to` is required, stripped of `@`, and always sent. `kild_new` posted `kickoff` as a bare string, which is now a 400. It takes an explicit recipient. It also pre-flights that the recipient is one of the agents being spawned — without it the engine creates the kild, rejects the kickoff, and tears the kild back down, which is a confusing way to learn you typed a handle wrong. `kild_show` read `.state`; liveness is now which collection answers, so it queries live then archive and says which. Archived kilds became readable rather than an error on the way past. `system` is gone from the message type and the log rendering. Adds `attachAs` to `kild_new`: a pi session that creates a kild otherwise receives nothing until it attaches separately, since there is no privileged push any more. Two explicit calls in one tool — convenience in the client, not a default in the engine.
Two halves of the same obligation: when intelligence leaves, the mechanism
it was hanging from has to stay.
The engine no longer knows what memory synthesis is. On stop it emits a
kildClosed event carrying only facts it holds — kildId, name, cwd,
worktree, base, transcriptPath, ledgerPath — broadcasts them on the event
stream, and runs whatever hooks.onClose declares. A hook is an agent spec
with a prompt, an argv, or both; there is no registry and no hook name the
engine recognises, so it cannot tell a memory hook from a backup hook.
Facts substitute as {{fact}} rather than ${fact} so shell expansion stays
the author's. The hook is not awaited and every failure is swallowed — a
hook must never hold a stop open or prevent one.
The charter moves out: synthesisPrompt, configuredMemorySynthesis and the
memory.synthesis branch are gone. docs/onclose-hook.md carries the exact
old prompt so migrating is copy-paste rather than git archaeology, and
records that live memory.synthesis configs are no longer read and must
re-point.
The ledger stops guessing. finalNonSystemPost was a tail-grab standing in
for facts the engine already owns — a smoke test caught it writing
"outcome: second message" from a throwaway test post. It is deleted, there
is no outcome line, and a test now forbids one. In its place: whether the
branch landed, commits vs base, files changed, uncommitted count, and
per-agent model and spend, read from kildGitStatus and reviewCommits
rather than reimplemented. A git failure reports "landed: unknown" instead
of inventing a result.
239 pass. Two tests removed with the charter they covered; the rest
rewritten, and 21 added — including collectLedgerFacts against real temp
git repos for the unlanded, merged and non-repo cases.
Five things changed underfoot for anyone already running kild, and not one of them fails loudly. The state directory moved, so archived kilds vanish from view while their files sit where they always were. memory.synthesis is no longer read, so synthesis stops without complaint. The Stop hook's env was renamed, and that hook exits 0 on every failure by design, so it just quietly stops delivering mail. Collecting them in one place, ordered by how badly each fails if skipped, because "recoverable from git history" and "documented in the commit that made it" are not discoverable by someone whose setup went quiet. Also records the config-key and environment-variable tables in full, so the answer to "did this one change?" is a lookup rather than a search, and points API clients at helm-migration.md rather than letting them port to an intermediate state.
A kild is a worktree, so it gets one set of verbs. /api/worktrees and its
prune verb are gone: GET /api/kilds now enumerates kild/* FROM GIT rather
than the in-memory registry, so the trees stranded by previous engine runs
finally have an id to address. They surface as kilds with no agents and no
log. A worktree kild did not create is never listed, so disposal can never
reach one.
Disposal guards on authored commits, not a clean tree. Refusing dirt is
what produced 116 unreclaimable worktrees — 27 of 27 were dirty from
provisioning litter written before any agent ran. Now: commits in
base..HEAD refuse, uncommitted files are discarded and listed, and the
branch always survives, which is what makes --force safe rather than
reckless. An unresolvable base refuses rather than guessing.
The agent resource collapses to one address. /api/agents/:id/{prompt,stop,
transcript} are deleted — prompting an owned agent IS sending it a
message, so there is one delivery path instead of two. Stopping one agent
is DELETE on its handle. GET /api/agents survives as what it actually is:
an inventory of live processes on no roster, which no kild can answer.
Spawning gets a REST route that answers. It was WS-only and
fire-and-forget — the engine warned to its own log and returned nothing,
so a caller could not learn it failed. That is the shape this branch
exists to delete.
land is two verbs: GET reports and touches nothing, POST merges and
records the sha, which the ledger now prefers over inferring from
containment. It refuses rather than updating refs behind a checkout's back.
280 pass. Two tests removed with removeWorktree's clean-vs-dirty policy —
keeping it would have left two disposal policies for one object. 43 added,
including the guard, enumerate-from-git, and land against real temp repos.
Also corrects the skill doc, which still told agents to omit --to to reach
"the kild's lead". There is no lead; the engine rejects an unaddressed
send.
GET /api/kilds computed git status per kild and, once orphan trees started enumerating, per orphan too — then returned every message log alongside. Measured on an 8-worktree fixture with a counting shim on PATH: 57 git invocations and ~110ms for a query that is mostly identity. On the machine this is built for, 116 worktrees, that extrapolates to ~813 per call. Now GET /api/kilds is identity and roster only: 1 git invocation, ~20ms, and it stays 1 at any worktree count because the single call is the worktree enumeration itself. The git block, cost rollups and per-agent spend move to GET /api/kilds/status, which a client polls on its own cadence. GET /api/kilds/:id serves one kild with git, bounded by being one. ?state=reclaimable moved to /status rather than being answered cheaply, because it reads ahead/behind. On the cheap route it now returns 400 naming where to ask instead of quietly paying the cost the split exists to avoid. Messages leave the listings and become their own cursored resource. Message.seq is monotonic, assigned from the log's own tail rather than a side counter, so it survives reload; ts is Date.now() and can go backwards, which is why it was never usable as a cursor. Archives predating seq decode by position. WS frames carry the stamped seq, so a client can tell new from replay. 293 pass. One test removed with its subject — the "last two posts" teaser no longer exists now that no listing carries a log — and its role is taken by the ?since= cursor tests.
Adds the reshape to the migration guide: the cheap/costly listing split with the measurement that motivated it, seq as the cursor ts could never be, one address per agent, worktrees folded into kilds with orphan trees finally addressable, and land as a dry run plus an execute. Two things a client would otherwise learn the hard way. Poll the two listings on different cadences — that is the whole point, and a client that polls /status like it used to poll /api/kilds gains nothing. And land refuses while the main checkout is off-base or dirty, which is deliberate but will surface as a failure in the UI rather than a queue. Also corrects api-surface.md §5, which claimed /api/kilds/status would return collidesWith. It does not and no route ever did — collisions are a cross-kild derivation over changedFiles, computed client-side. This is the second time that error has been written down, so both places now say so explicitly rather than leaving the next reader to rediscover it.
Eight handlers read their JSON body unguarded, so an absent or malformed
one threw and Hono surfaced it as "Internal Server Error" — the server
blaming itself for a client mistake. Every handler already validates what
it requires, so treating a bad body as {} lets the existing checks return
the proper 400 naming the missing field.
The one that mattered: POST /api/kilds/:id/stop requires nothing, so a
request with no body is valid input — and it 500'd. A caller with nothing
to say, doing the obvious thing, got a server error. Found by the
end-to-end migration test, which called stop with no body for exactly that
reason.
Tests go in server.kilds.test.ts rather than a new file. Bun shares the
module registry across test files, and server.ts has load-time side effects
keyed on KILD_HOME, so a second importer silently wins the race and breaks
whichever file imported first — which is what a separate file did. One
owner for the app import.
Two bugs the end-to-end test surfaced. loadProjects returned JSON.parse(raw).projects unchecked, so any projects.json without that exact key — a hand-edited bare array, most likely — yielded undefined, and every caller does .map or .find on the result. One malformed registry 500'd routes across the engine, with the server blaming itself for the operator's typo. It now always returns an array, tolerates a bare array, drops entries missing name or path, and treats an unreadable registry as "no projects registered" rather than a failed request. forkFrom lost its way in, not its mechanism. agent.ts still calls PiSessionManager.forkFrom and the manager still passes KILD_FORK_SESSION; what the reshape deleted was POST /api/agents, the only REST route that reached it. It now lives where agents live, as an option on spawning one into a kild, and the path is validated up front — a bad path would otherwise fail inside the agent process after the spawn reported success, which reads as a spawn that half-worked. Worth stating why the copy matters: forkFrom COPIES the source session into a new file, so the source is never written. That is what makes forking a live agent's conversation safe rather than a two-writers corruption bug, and a test asserts the source is unchanged rather than trusting the comment that promises it.
Unit tests prove each part in isolation; this drives the whole reshaped surface over HTTP against a real engine, which is where the parts get a chance to disagree with each other. It has already earned the commit. It found POST /stop returning 500 for an empty body — valid input for a verb that requires nothing — and loadProjects handing back undefined for a hand-edited registry, which 500'd routes across the engine. Neither was visible to the unit suite, because each part was correct alone. Binds a configurable port defaulting to 4611, never 4517, so the operator's own engine survives a run. Uses a fresh mktemp rig each time, since a leftover worktree from a previous run would masquerade as a finding. The two land checks need a working model provider — an agent has to reach ensureWorktree before there is a branch to merge — so without auth they report INFO rather than failing; land itself is covered against real temp repos in kild-land.test.ts.
`ownership` was omitted for owned agents, on the reasoning that absent means owned and the wire stays quiet. But it is the one field a client switches on, so `agent.ownership === 'owned'` was false for every owned agent, and every consumer needed a `?? 'owned'` it had to remember. An implicit default on a discriminant is the same mistake as the lead default this restructure deleted: the engine knows the answer, so it should say it. The stored field stays optional — rosters persisted before it exists still decode — but the view resolves it rather than passing it through. Found by the end-to-end check, which is also refined here: it now resolves its engine dir before changing directory (a relative $0 stops resolving once the script cds into its rig), takes its port from KILD_E2E_PORT so a concurrent run cannot answer another engine's requests, uses a fresh mktemp rig per run so a leftover worktree cannot masquerade as a finding, and asserts ownership while a live kild still has agents rather than after it has been stopped. 52 checks, all passing.
The extension read `kild.log` off `GET /api/kilds`, which stopped carrying logs when the listing split. `KildView` requiring `log` was the bug, so it is replaced by shapes that mirror the engine's: identity for the cheap route, status for the costly one, and archive as the only shape that holds a log. Liveness is resolved by observation — which endpoint answers — via a live/orphan/archived union rather than a field. kild_ls uses the cheap route and routes `git`/`reclaimable` to /status, so the documented 400 is never provoked. It also drops its "last message" line, which cost a request per kild for a teaser. kild_log reads /messages with a `since` cursor. kild_spawn calls the real spawn route and its fire-and-forget-plus-poll hack is deleted — it now reports "duplicate agent: @coder [rejected]" instead of guessing from a roster. Adds kild_land (dry run by default), kild_rm (relaying the engine's refusal), and kild_stop_agent as its own tool rather than a flag on kild_stop, since a flag would silently change how destructive the call is. The bridge's gap recovery now uses seq: baselined at attach so history is never replayed, advanced by each WS frame, and standing down entirely on a capped drain so the engine's wake cap cannot be bypassed from here. Its dedupe originally keyed on `ts` and matched almost never, because the inbox copy is stamped with a fresh Date.now() — it matches on sender and text now. Both recovery paths were proven against a TCP proxy that drops the socket while the kild stays live, not asserted.
Two additions to the port, one of which is optional and one of which removes code. ownership is always on the wire now, so any `?? 'owned'` a client wrote to work around its absence can go. attach returns a token. Sending it as a Bearer header attributes a message to that handle instead of the generic `human` every attached sender used to collapse into — with the before/after log, since two attached agents being indistinguishable is easier to see than to describe. It is optional: no header behaves exactly as today, so helm needs no change unless it wants to be named. States plainly that this is attribution and not authorization, and that no route requires a token, so nobody reads the section as a new auth requirement to implement before porting.
It still opened with rooms, kild sessions, worker.ts and an operator/ dir — so the first thing any agent read was the model that was just deleted, and a layout with three paths that no longer exist. Records the ordering: simplicity, then dogfood, then apply the learnings. And the two boundaries this restructure was mostly about, stated as rules rather than left as scar tissue. Never read intent out of an agent's prose. Addressing parsed recipients out of message text and the "who is this for?" bug lived in that gap for months; the decisions ledger parsed a protocol out of English. A sender names recipients as data, a hook is declared in config, and anything an agent means it states through a tool call. Don't enforce what you can't enforce. The engine is loopback and single-operator, so any local process can read any credential it holds. Attribution earns its keep; authorization ceremony on top of it does not.
…t ran The "mechanism prompt" was a claim the file did not honour. It opened <how-to-operate> and then taught working practice — outcome-first, verify-before-believe, stay in scope, escalate when blocked. That is process, which is intelligence by kild's own boundary, shipped in every agent's first turn by the engine that forbids it. Same category as the decisions ledger this branch deleted, just better written, which is probably why the audit that caught the ledger missed this. It is now default-prompt.ts, named for what it is — the default persona, the one a bare session gets when none is supplied — and split so the two halves stop pretending to be one thing: BASELINE is competence for an agent with no intelligence layer applied, and it says outright that it is a floor a persona may override. MACHINERY is how the engine actually works: send is the only way words leave an agent, `to` is required, delegation is asynchronous, nothing chases you, stopping is destructive. That half is genuinely mechanism — an agent that does not know it will talk to nobody and believe it reported — and no persona should have to re-teach it. Also drops the two PRP mentions from engine comments. The rule is that kild knows nothing about it, and comments are how the next agent learns what is allowed. And the roster now records the RESOLVED persona. Spawning `coder` with no explicit persona ran persona `coder`, stored `undefined`, and had the ledger print `default` — three answers, none of them true. The ledger no longer invents one either: a genuinely absent persona is reported as absent rather than named, since the ledger holds facts. 336 pass, e2e 52.
Interactive `kild new` matched `/spawn <handle>` out of stdin before sending, so a line you typed could be swallowed and never delivered. Worse, whether it was depended on word count: the pattern accepted at most three tokens, so `/spawn a helper` was intercepted while `/spawn me a coder please` went through as a message. Nothing should read prose to decide what to do with it. That is the bug this restructure exists to remove, and it applies to a human's words exactly as much as an agent's — a shortcut that eats one message in ten is worse than no shortcut at all. What you type is now a message, unconditionally. Adding an agent is `kild spawn <id> --as <handle>`, a real command that reports a real error; the banner prints the kild id so it is to hand. With this the engine and CLI hold no regex over prose at all. What remains parses machine formats — an Authorization header, git --numstat output, a digits-only cursor — never anything anyone wrote.
An agent that can spawn had no way to learn what it could spawn. It guessed a name and took `unknown persona: x` — discovery by rejection, which is the same shape as learning the roster by mis-addressing a message. The catalog rides the first turn of any in-kild session, beside the model catalog it mirrors exactly: a list of what EXISTS, carrying no opinion about when to use it. The engine passes each `description` through verbatim and never reads or ranks one — that frontmatter line is the persona author's signal, which is the only reason a catalog beats a bare list of names. `default` is excluded from the list, since a catalog says what things are FOR and it describes nothing — but the section names it explicitly, because an agent still has to know it can ask for a general-purpose one. A persona with no description is listed anyway: a name it can spawn beats a name it cannot see. The section also states the thing that makes multiple instances usable: persona and handle are separate, so spawning one persona twice under two handles gives two agents you can address independently. Deliberately NOT the live roster. Personas are files, so this stays true for the session's lifetime; who is actually in the kild changes as agents spawn and stop, and a first-turn snapshot of that would be stale on arrival.
`default` named its place in the system rather than what it is. That is fine in a config key and useless in a catalog an agent reads to choose a delegate: "spawn `default`" says nothing about what you would get. It is now `general`, with a description — general-purpose, no specialisation, use when no other persona fits, given a clear goal, the outcome you want, and how you will judge it done. The last clause is deliberate: it tells a delegator what this persona needs in order to be useful, which is the one thing a general agent's description can usefully carry. This removes a special case rather than adding one. The catalog previously filtered the built-in out because it had no description and a list of what-things-are-FOR could not place it — so the persona that always exists was the one an agent could not see. Describing it was the fix; the filter and the "(not listed)" caveat both go. No alias. Per the no-shims rule `persona: "default"` now fails with `unknown persona: default` — loud, and documented in upgrading.md, since it will stop a script mid-run. Omitting persona still means the built-in; only the explicit spelling changed. A `general.md` file does not shadow it. Caught by the end-to-end check, whose own rig was the first stale caller — which is what it is for.
Spawning an agent and telling it what to do were two calls, so every real delegation was spawn-then-send and a spawn on its own produced an agent sitting idle. `spawn` now takes an optional `task`, delivered as the new agent's first message — the same fusion `kickoff` already does for `POST /api/kilds`. It is deliberately NOT a stored field. The record of what an agent was asked to do is its first message on the log, which a later revision follows naturally; a copy on the roster would be a second answer that goes stale the moment the spawner says "actually, do X instead". The consequence that had to be right: the spawner is now a message sender. `POST /api/kilds/:id/agents` took `invitedBy` from the request body, which was harmless while it was only roster metadata and a forgery path once it becomes a message's `from`. The route derives it from the caller's credential like every other actor, and a present `invitedBy` is refused naming that field. An agent's own spawn control line carries no sender and cannot — the engine takes it from the session the line arrived on. The agent says what it wants done; the engine says who asked. `invitedBy` is also exposed on the cheap roster view. It was already recorded and already free to read, so omitting it only meant nobody could see the spawn edge — the one thing that tells five agents on one persona apart by origin. Also: `--task` on `kild spawn`, `task` on the `kild_spawn` WS frame and the pi extension's tool, and the machinery prompt teaches both `task` and choosing handles you can tell apart. e2e covers the fused path and the refused forgery.
`memory.dir: 42` made `expandHome` call `.startsWith` on a number and throw out of a function documented as never throwing — a 500 on `POST /stop` for a typo in a config file. `plugins: "./prp-core"` iterated the string's characters and resolved a plugin dir per letter. Every reader trusted the declared type; only the parse was guarded, and it checked the object, not the fields. This is the same class as `loadProjects` reading `.projects` unchecked, so it gets the same fix: one narrowing pass where the untrusted file becomes a typed object, dropping any field of the wrong type. A typo in one field no longer disables the rest of the file, and every reader below can trust what it declares because exactly one place checks. A hook's inside is deliberately still unvalidated — the engine does not interpret hook content, and `runCloseHook` guards each form before running it.
Owner
Author
PR Review SummaryPR: #673 — Restructure: agent-centric model + demolition Seven specialist agents reviewed the diff. Every Critical and most Important findings below were then reproduced by hand on a live engine driving real pi agents (ports 4611–4619; the operator's engine on 4517 was never touched). Findings marked [verified live] are reproductions, not static reads. Validation
Critical Issues (3 found)
C-1 — Stopped agent swallows messages. The message vanished, the caller was told it landed, and C-2 — Silent CLI hang. The WS …then hung indefinitely (killed at 20s). Server-side only: C-3 — False "nothing discarded". Important Issues (13 found)
I-12 — A hook that never fires. I-13 — The test headers document the bug as a guarantee. All five files claim "KILD_HOME points at a temp dir BEFORE the dynamic import so the module's load-time side effects see empty state." Bun caches I-3 — The ledger contradicts itself exactly when work succeeds. A live run produced:
I-5 — The migration guide misdirects helm on the one route it warns about. I-6 — Same class the newest commit just fixed elsewhere. I-9 — Boundary judgment, not a defect. I-10 — e2e's land check has never run. Section 8's setup commit is a subshell with Suggestions (15 found)
Also: Documentation Issues
Verified accurate and unusually good: Strengths
VerdictNEEDS FIXES — appropriate for a draft. The architecture is sound and the demolition is disciplined; the problems are concentrated in failure paths and in docs that drifted from the code. Recommended Actions
Reviewed by Claude — 7 specialist agents plus live verification against real pi calls. |
Deletes the agent-facing `spawn` tool. Inside a kild, addressing a handle nobody holds creates that agent and delivers the message to it; `persona` and `model` on `send` say who a created recipient should be, and the handle names the instance. `task`-on-spawn fused the two calls but left two tools, and the seam showed: `spawn` could carry a message, `send` could not create, and an agent had to know which to reach for. Spawning was never the goal — it is what you do in order to say something — so the create step disappears into the verb that has the message. The just-created-and-idle agent is now unreachable rather than warned about, and an agent learns two tools instead of three. Not an inference: handles come from the caller as data, never from the text. What changed is what an unknown handle MEANS, and only for a caller that may grow the kild — the manager's `send` takes a `createMissing` spec, agents pass it, REST does not. At the boundary an unknown recipient stays a rejection naming the roster, because a typo from a client must not quietly cost a process; the same holds for `kild send` and the extension's `kild_send`, with the explicit spawn route right there. New recipients are validated as a batch, so one bad name creates none of them, and a create that throws mid-batch rolls back its siblings. Roster discovery falls out of this instead of needing a tool: a send names what it created, and an unresolvable recipient comes back listing both the personas that exist and who is in the kild. `kild show` / `kild ls` stay the roster view for humans and clients. Removes: spawn-tool.ts, the `spawn` control line, `SpawnOut`, `onSpawn`, and the `task` param on the tool (the REST route keeps its own).
Owner
Author
Review addendum —
|
| Check | Result |
|---|---|
tsc --noEmit |
PASS |
biome check src |
PASS (63 files) |
bun test |
FAIL — 358 pass, 3 fail (361 across 32 files) — the same three |
scripts/e2e.sh |
PASS 61/0 (up from 60) |
The new verb works, and the boundary distinction holds [verified live]
Driven with a real pi agent, an agent's send to a handle nobody holds created that agent and delivered to it in ~6s:
roster: coder | persona: general | ownership: owned | invitedBy: None
scout | persona: general | ownership: owned | invitedBy: coder
log: seq 2 from coder -> ['scout'] : PING
invitedBy: coder is right, and composes correctly with 8fad0ba's engine-derived spawner — a created agent is attributed to the agent that summoned it, not to the unattributed label. The asymmetry the commit message describes is real in practice: REST still refuses rather than creating, naming the roster —
POST /api/kilds/:id/messages {"to":["ghost"]}
→ {"error":"no such agent: @ghost (in the kild: @coder)"}
Batch rollback is implemented as described (kild-manager.ts:879 rollbackCreate, called at :216). Agents are down to two tools (send, stop); spawn-tool.ts is gone and the REST spawn route correctly survives at server.ts:632.
This is a good simplification — it removes a seam an agent had to reason about, and "spawning is what you do in order to say something" is the right observation.
C-1 survived the rewrite — this is the thing to fix
The stopped-agent silent send loss is unchanged at c0fbd81, in code this commit substantially reworked [verified live]:
DELETE /api/kilds/:id/agents/coder → {"ok":true,"message":"Stopped @coder in kild 'n'."}
roster → coder stopped: True idle: True
send to stopped @coder → {"ok":true,"message":"Sent to the kild."}
roster after → coder stopped: True idle: False ← corrupted
Root cause is still exactly where it was: stopped is consulted only inside stopAgent (kild-manager.ts:361-365) and never in recipient validation, so a stopped handle passes as a valid target, deliver() discards prompt()'s false, and idle is flipped on an agent whose session is already gone — with nothing left to reset it.
Worth noting the new semantics make this slightly sharper: send now distinguishes three cases for a recipient — exists, doesn't exist (create, for agents / reject, for clients) — but stopped is still silently folded into "exists". A fourth case is now the natural place to put it.
Unchanged findings
Everything else from the main review still stands at c0fbd81. Specifically re-checked:
BASELINEstill ships process content (default-prompt.ts:31) — this commit edited that file (34 lines) and left it.- The three failing tests, both root causes untouched.
helm-migration.md's transcript contradiction — this commit edited that file too (+7).loadArchive's bare cast, the ledger'scode: 0 commitson success,postson the drain wire,hooks.ts:8's "(stop/land)",engine/README.md's three deleted files.- WS
kild_newstill acceptsagents: [].
Verdict unchanged: NEEDS FIXES
c0fbd81 is a net improvement and introduces no new defect I could find. The priority list from the main review is unchanged, with one reordering: C-1 is now the clear top item — it has survived two consecutive rewrites of its own code path, which suggests it needs an explicit test rather than another pass over the same lines. The mock at kild-manager.test.ts returns true unconditionally from prompt(), so the suite structurally cannot catch it.
Re-verified against real pi calls on an isolated engine; the operator's engine on 4517 was not used.
…livery A stopped agent stays on the roster — its handle never rebinds and its transcript stays addressable — so membership alone said yes to a send nothing could receive. It validated, `prompt()`'s `false` was discarded, and the sender was told "Sent to the kild." while `idle` was flipped on a process that would never emit `agent_end` to flip it back. Two shapes, two fixes. A `stop` is known in advance, so a stopped recipient is refused up front, naming who can still read something; refused wholesale for a mixed list, exactly as an unknown recipient already is, because a partial send is the thing the sender would not be told about. A crash is only discoverable at the moment of delivery, so `deliver()` now honours the substrate's answer: an unreachable agent is marked stopped, its idle state is left honest, and the send result names who did not receive it. The suite could not see this: the fixture returned `true` unconditionally from `prompt()`, making the mock more forgiving than the thing it stands for. It now models a stopped and a crashed session, and five tests cover both paths.
Half the paths here come from git, which always reports the resolved form, and half are built from $KILD_HOME, which is whatever the environment says. On macOS those differ for anything under /var — a symlink to /private/var — so a worktree kild had just created did not compare equal to the same worktree in `git worktree list`. Every comparison between the two sides was true on Linux and false on the operator's machine, which is how the suite could be green here and red there with nothing in the diff to explain it. `canonicalPath` resolves the longest existing prefix and rebuilds the rest onto it, so it works for a path that does not exist yet — which `realpathSync` alone cannot do, and the worktree path is routinely resolved before it is created. `worktreesRoot()` returns it, so everything built from it inherits the canonical spelling, including the open-in-OS containment guard. `KildTree.repo` stays as the caller spelled it, deliberately: it echoes the identifier they asked about, so their own `?path=` and project-registry joins keep working. Repos compare with `samePath`, never `===`.
…er call
Two defects in the archive loader, both the same shape as bugs already fixed
elsewhere in this branch.
`JSON.parse(...) as PersistedKild` asserted a shape nothing had checked, and a
bare `catch {}` made a dropped archive and "this kild never existed" the same
answer to every reader. A history file is written by an older engine,
hand-edited, or truncated by a crash mid-write — the same class as the
`projects.json` and `config.json` inputs already narrowed at their boundaries.
It is now decoded rather than asserted, and every skip says so on stderr.
`dir` was a field initializer, so $KILD_HOME was read when this module was first
imported and frozen for the process. Fine for the engine, wrong for anything
importing it twice with different state: the first importer decided where every
later one would look. Bun shares the module registry across test files, so the
archive tests passed or failed on file order — on readdir order, in other words,
which differs by filesystem. Now read per call, with the archive loaded on first
read instead of at construction.
`changedFiles(dir).catch(() => [])` turned any git failure into "nothing will be discarded" — and that list is the whole of what the operator is told they are about to destroy, printed at the moment force-removal destroys it. Three lines above, `reviewCommits` refuses rather than guessing; the module header promises "named, never hidden". The failure now rides the plan as `discardedError`. It cannot refuse the disposal — the working tree is not evidence, which is the guard's whole point — but the route, the CLI and the client all now say "could not determine what was discarded" instead of implying nothing was.
… REST `kild new --agents not-a-persona` printed a confident header inviting you to type into a kild that never existed, then hung forever. Server-side it was one line: `kild_new … rejected: unknown persona`. The frame was fire-and-forget, the rejection was a console.warn inside the engine, and `KildOutbound` has no rejection frame — so the CLI waited on events that could never arrive. That is the exact silent-failure shape the REST spawn route was added to remove. It was fixed on the route and left on the transport the default verb used. The fix is not a rejection frame — that would be a second request/response protocol beside the one that already works. `kild_new`, `kild_send`, `kild_spawn` and `kild_stop` are deleted. `kild new` now subscribes first (so nothing said in the first instants is missed), then creates over REST and lets the failure be a thrown error carrying the engine's reason; typed lines and Ctrl-C are REST calls too, so a line nobody could receive says so instead of vanishing. The frame-ordering queue goes with them: it existed because a create and its kickoff raced over the socket, which is a problem REST does not have — you await the status code and only then send. What remains on the socket is the bare-agent substrate (spawn/prompt/stop) for one-shot `kild run`, which has no kild to answer for. Verified live: the rejection now exits 1 with "unknown persona: nope (available: general)" instead of hanging, and the happy path still streams.
`collectLedgerFacts` runs at close, after the merge — and `base..HEAD` is empty once the branch is contained in base. So the ledger wrote `code: 0 commits, 0 files changed` directly beneath `landed: yes`: the kild that finished its work recorded none of it, and the entry contradicted itself exactly when things went right. The land is the only moment those numbers exist, so it is the moment they are captured. `recordLand` now stores what the merge carried alongside the sha, and the ledger prefers it. The line's wording follows the state — "landed into main" for work that is now part of main, "vs main" only while it is still outside. Also makes e2e section 8 real. It waited for an agent to create the worktree, so without an authenticated provider the setup no-op'd, land found nothing, and the whole section degraded to INFO while the harness exited 0 — it had never run. The rig now creates that worktree itself, commits its own config (an untracked file in the main checkout is exactly the dirty-checkout state land refuses), and asserts the TOP-LEVEL merge sha rather than any `"sha"`, which matched a commit inside `commits[]` and passed on a land that merged nothing. e2e: 65 checks, 0 failures.
… `post` is dead Two things the reviewer's vocabulary sweep turned up, one of them live. The context the Claude Code Stop hook injects told the agent to reply with `kild send <id> "<text>"`. `kild send` requires `--to` — the engine never infers a recipient — so an agent that followed the instruction verbatim got a usage line instead of delivering its reply. The notice now teaches the form that works, and the test pins it. `post` is listed dead in VOCABULARY.md (you post to a board, you send to a person) and survived as the drain response's `posts[]`, i.e. on the wire, in the one place a client cannot avoid reading it. Renamed to `messages[]` across the inbox, the CLI, the client type and the pi extension.
The reviewer's doc audit, verified item by item. The highest-cost one first:
`helm-migration.md` told helm not to build against `GET /api/kilds/:id/agents/
:handle/transcript` — the only transcript route that exists. It said so at the
top and the opposite 130 lines down, and the PR body repeated the wrong half.
The reversal is now stated with its reasoning (api-surface.md §1), the rename
tables are marked as stage one of two with the deleted routes struck through,
`/api/worktrees` moved out of "unchanged" (the reshape deleted it), and the WS
section says what is actually true now: the socket is a subscription, kild
mutations are REST calls.
`hooks.ts` promised hook authors a moment that never fires ("stop/land" — land
does not end a kild). `attached-participants.md` was wholly stale and is rewritten
as `attached-agents.md` against the shipped surface, including the attach token
and the real cap/queue constants. `manual-smoke-skills-profile.md` no longer runs
a deleted command, and now says what KILD_SKILLS_PROFILE does NOT scope —
extensions, SYSTEM.md and context files still come from the operator's ~/.pi.
`engine/README.md`'s layout block named three files that no longer exist.
`escalation.md` called `seq` planned; it shipped. Plus room/session leftovers in
both READMEs and the `kild worktree rm` command in two guides.
The header told a hook author the engine holds the moment "(stop/land)". `runCloseHook` is reachable only from the private `close()`, which only `stop()` calls; `recordLand` sets the sha and persists, and a landed kild stays live. So a hook declared expecting to run after a land never ran and nothing said why. `docs/onclose-hook.md` already documented stop-only — the comment was the outlier.
Owner
Author
Re-review at
|
| Finding | Fix | Verified |
|---|---|---|
| C-1 stopped-agent send silently lost | 8eb0035 |
{"error":"stopped, nothing would read this: @other (still working: @coder)"} — and idle correctly stays True [live] |
| C-2 CLI hangs on a rejected WS command | 609bab4 |
exits in 2s: error: unknown persona: definitely-not-a-persona (available: general) [live] |
C-3 discard list [] on git failure |
5f8ccc3 |
discardedError + undetermined; "empty means empty only when discardedError is absent" |
Ledger code: 0 commits on success |
4572518 |
- code: 1 commit landed into main, 1 file changed beside landed: yes … as 8edb7ae [live] |
post dead vocabulary on the wire |
4a2c956 |
drain returns {"ok":true,"messages":[…]} [live] |
WS kild_new accepts agents: [] |
609bab4 |
rejected — the socket is subscription-only now; no kild created, no broadcast [live] |
loadArchive bare cast + silent drop |
ad3d546 |
decodePersisted() validates; both failure paths console.error the filename |
kild-trees path/repo spelling asymmetry |
f25fffb |
test green |
hooks.ts "(stop/land)" |
b7b7dc6 |
now "stop only; landing does not end a kild" |
| helm-migration transcript contradiction | 8e6096a |
gone |
| Stale docs (README, attached-participants, engine/README layout) | 8e6096a |
swept — attached-participants.md removed, layout block corrected |
C-1's error message is better than what I suggested — naming who is still working turns a rejection into a next step.
New regression — e2e.sh is now red
scripts/e2e.sh exits non-zero at b7b7dc6: PASS 65, FAIL 1.
══ 8. land: dry run touches nothing, execute records a sha
FAIL e2e worktree created | got: git worktree add failed
The section-8 rewrite is right in intent — replacing the 2>/dev/null no-op with a real assertion is exactly the fix. But the guard tests the wrong thing:
if [ ! -d "$WT/.git" ]; then
( cd "$REPO" && git worktree add -q -b kild/e2e "$WT" main ) || no "e2e worktree created" ...
fiIn a linked worktree, .git is a file, not a directory. By section 8 the engine has already created the tree via ensureWorktree, so -d is false, the guard re-runs git worktree add, and it fails:
$ git worktree add -b kild/x ../wt main # tree already exists
fatal: a branch named 'kild/x' already exists (exit 255)
[ -d wt/.git ] → FALSE [ -e wt/.git ] → TRUE
Land itself works — every following check (execute merged, returned the merge sha, base really moved) passes. It is only the guard that misfires, every run.
Fix: [ ! -e "$WT/.git" ]. AGENTS.md requires e2e green for anything touching the HTTP surface, so this currently blocks the gate for a one-character reason.
Still open — the suite is still order-dependent
bun test: 369 pass, 2 fail (371 across 32 files). kild-trees is fixed; the two archive tests are not.
ad3d546's diagnosis is exactly right ("Bun shares the module registry across test files, so the archive tests passed or failed on file order"), and moving dir to a per-call getter is the correct fix for the engine. But ensureArchive() keeps a one-shot latch:
private ensureArchive(): Map<string, ArchivedKild> {
if (!this.archiveLoaded) { this.archiveLoaded = true; this.loadArchive(); }
return this.archive;
}Construction is no longer the deciding moment — first read is. Whichever test file reads the archive first still latches the directory for the whole process. Precisely isolated:
bun test src/server.kilds.test.ts → 36 pass, 0 fail
bun test src/server.attached.test.ts src/server.kilds.test.ts → 46 pass, 0 fail
bun test src/server.git-review.test.ts src/server.kilds.test.ts → 38 pass, 2 fail ← latches first
server.git-review.test.ts reads the archive before server.kilds.test.ts seeds its fixture, so the fixture never loads and /api/kilds/:id/messages 404s.
Fix options: reset the latch when kildHome() changes (cheapest — key archiveLoaded on the resolved dir rather than a boolean), or give the archive tests their own process.
Unchanged (author's call)
BASELINE still ships working-practice content in default-prompt.ts (I-9). It predates this PR and breaks nothing — flagged only because commit 95e5ca4 names it a boundary violation itself.
Verdict: close
Every Critical is fixed and independently verified against real pi. What remains is two mechanical gate problems — a one-character e2e guard and a latch that needs keying on the directory — neither of which is a design issue.
Gate state at b7b7dc6: tsc PASS · biome PASS (63 files) · bun test 369/2 fail · e2e.sh 65/1 fail.
Verified on isolated engines (port 4621); the operator's engine on 4517 was not used.
…rship, projects
**The archive is a listing, so it carries no log.** `GET /api/kilds/archive`
shipped every stopped kild's full message history, and so did the WS
`{archivedKild}` frame. That is the duplication the cheap/costly split deleted
for live kilds, and worse here because the archive only grows: "list my stopped
kilds" meant "send me every conversation I have ever had". `GET /api/kilds/:id/
messages` already serves an archived kild's log — it is the one reader of logs,
and now it is the only one. A subscribed client already received every
`{message}` frame; one that was not reads the resource.
**`ownership` now holds its promise everywhere.** It is documented as always
present on the wire, and it was — except here. A live roster goes out through
`agentView`, which resolves absent → `owned`; an archive loaded from disk went to
the route untouched, so a file written before the field existed served agents
with no ownership at all: the one field a client switches on, missing, in the one
payload nobody re-derives. Resolved on decode now, exactly like `seq`.
**The project registry has no REST surface.** `GET|POST /api/projects` are
deleted. Nothing called them: the CLI reads and writes `$KILD_HOME/projects.json`
directly, the pi extension passes a registered name in a request body, and helm
confirmed zero references while porting. The registry itself is load-bearing and
stays — it scopes the unscoped `GET /api/kilds` — but a loopback write route for
a local file the operator owns was a second way to do what `kild project add`
already does. The server test that registered its fixture over REST now calls
`addProject()`, the same act; its "a required field fails loudly" assertion for
that route is dropped as obsolete (the property is still asserted for `/messages`
and `/agents/attach`), replaced by a route-is-gone test and two e2e checks.
Dropping `log` from the archive listing dropped time with it. Nothing in `ArchivedKildView` was temporal: the last message's `ts` had been carrying that job by accident, so ordering history newest-first meant one `/messages` request per archived kild — N requests to sort a list, worse than the fat payload the projection removed. helm found it reading the change against its own store, where the sort silently collapsed to `?? 0` and produced arbitrary order. `endedAt` is that fact as its own field: epoch millis for the last moment the engine knew the kild was alive, which for one that stopped normally is when it stopped. Wall-clock deliberately — `ts` cannot be a cursor because it can go backwards, but "when did this end" is a fact about the world and `seq` cannot answer it. A timestamp is not a log, so the listing stays cheap and the rule holds. Stamped on every persist rather than only at `remove()`. Stamping at the end alone leaves one shape with no time at all — a kild the engine died under, whose file exists from write-through and never reached `remove()` — which would then need a decode-time fallback. There is none, and none is wanted: an archive written before this field has no end time, and inferring one from its last message would be a guess dressed as a fact. We own both sides of this wire, so that is a fact to state and a client sorts the unknowns last. Also fixes the two gate problems from the re-review at b7b7dc6: - `e2e.sh` tested `-d "$WT/.git"`, but in a LINKED worktree `.git` is a FILE. On a machine where an agent had already created the tree the guard read "no worktree here", re-ran `git worktree add`, and failed on the existing branch — green where no agent starts, red where one does. `-e` is the correct test. - `ensureArchive`'s boolean latch moved the deciding moment from "whoever imported first" to "whoever read first" — still one caller silently choosing the directory for every later one, and `kildDir` consults the archive to tell "archived" from "never existed", so almost any route could be the one to latch it. Keyed on the resolved directory now: the archive in memory is always the archive of the home in effect, and there is no deciding moment at all. Gates: 376 tests, typecheck, lint, e2e 70/0.
Owner
Author
Re-review at
|
| Check | Result |
|---|---|
tsc --noEmit |
PASS |
biome check src |
PASS (63 files) |
bun test |
376 pass, 0 fail |
scripts/e2e.sh |
70 pass, 0 fail |
Matches the numbers stated in 6cc3d5f.
The order dependency is genuinely gone
Keying the archive on the resolved directory — rather than a boolean latch — removes the deciding moment entirely, as the commit says. Stress-tested across permutations, including the exact repro that failed before:
git-review, kilds → 42 pass, 0 fail (was 38/2)
kilds, git-review → 42 pass, 0 fail
attached, git-review, kilds → 52 pass, 0 fail
send-to, git-review, cwd, kilds→ 53 pass, 0 fail
kilds alone → 38 pass, 0 fail
e2e.sh's -d → -e fix is correct: the guard now recognises a linked worktree's .git file, so it no longer re-runs git worktree add against an existing branch. The commit's framing is exactly right — it was "green where no agent starts, red where one does," which is why it passed for you and failed here.
The helm-port fixes verified live
Two archived kilds, one attached agent each:
one | endedAt: 1785230316970 | has log key: False | agents: [('coder','owned'), ('w','attached')]
two | endedAt: 1785230321182 | has log key: False | agents: [('coder','owned'), ('w','attached')]
sortable newest-first: ['two', 'one']
listing carries message text? False
GET /api/kilds/:id/messages: 2 messages, still readable
endedAtis present and sorts correctly — the N-requests-to-sort-a-list problem helm hit is closed.- The archive listing carries no log — not just the
logkey removed, but no message text anywhere in the payload. ownershipis present on every agent, including the attached one, which is where it was missing.
Refusing a decode-time fallback for pre-endedAt archives is the right call, and consistent with how this branch has treated inferred facts throughout — an archive with no end time says so rather than borrowing a message's ts.
Status
Every Critical and every Important from the original review is now closed and verified. What remains is the advisory list, unchanged and all fine to defer:
BASELINEstill ships working-practice content (default-prompt.ts) — the boundary judgment, your call.UNATTRIBUTED = 'human'still reads as a privileged handle on the wire."kild session"survives as a dead word in the primary type file (kild-types.ts:64,76,89,kild-manager.ts:452) despiteVOCABULARY.mdretiring it.e2e.shis still not an npm script and there is still no CI — every gate is manual, including the oneAGENTS.mdcalls required for HTTP-surface changes. Given how much this harness has caught (four bugs it was written for, plus the land check and the worktree guard), wiring it tobun run e2eat minimum seems cheap.kild-manager.tsis now 962 lines, up from 787 at the start of this review — the god-module concern is trending the wrong way, though every line came from a legitimate fix. The messaging and close/hook clusters are still the clean seams.
Two numbers in the PR body are now well out of date: engine source is 7,657 lines, not 5,761, and the test count is 376, not 235.
Verdict: ready
No blocking issues remain. Gates are green, all three Criticals are fixed and independently reproduced as fixed, and the surface has been validated against a real client's port. Still marked draft — un-draft when you're ready.
Verified on isolated engines (port 4622); the operator's engine on 4517 was not used.
…d word **kild-manager.ts: 962 → 835.** The review is right that the trend was wrong, so the two seams it named are now their own slices — and the test for a slice is whether it needs the manager's state, not whether it makes the file shorter: - `kild-delivery.ts` — who receives a recorded message and what the roster learns by trying. Needs a kild, a message and a `prompt` function; nothing else. The interesting behaviour (a handle with no reader behind it) is now testable with no manager, no registry and no kild on disk. - `kild-close.ts` — the one moment a kild ENDS: ledger from facts, event, declared hook. The ordering is the interesting part (the ledger is written from the worktree before anything can land or prune it, and the event carries its path), and a caller that had to remember that would eventually forget. What stays is the manager's actual job — registry state and the verbs over it. Extracting those would produce modules whose every function takes the manager's private state, which is a namespace with a coupling, not a slice. **CI, on Linux AND macOS.** The repo had none, so every gate `AGENTS.md` requires was manual — which is how two bugs on this branch shipped green on one platform and red on the other (`/var` → `/private/var`; `.git` is a FILE in a linked worktree). A single-platform CI would have passed both. `bun run e2e` is a script now, so the HTTP gate is reachable without knowing the path. **`session` was the last dead word, and hunting it found a live bug.** The engine sets `KILD_SESSION_ID` on every agent it spawns, and an agent that shells `kild send` from bash is one of those — but the CLI never forwarded it, so an agent's own words landed on the log as `human`. The engine could not tell one of its own agents from the operator typing. The CLI forwards it now; a human's shell has no such variable and is still `human`, which is correct. Renamed with it, since `session` means pi's conversation and nothing else: `KILD_SESSION_ID` → `KILD_AGENT_ID`, `handleForSession` → `handleForAgentId`, and the REST field `sessionId` → `agentId` (no client sent it, and helm is pinned). Gates: 376 tests, typecheck, lint, e2e 70/0.
Owner
Author
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reframes kild from a Room primitive to an agent primitive, and carries out the teardown. Draft while the remaining work lands; commits are targeted so they review one at a time.
The model
The engine has one inhabitant type: the agent. Everything else is either mechanism kild owns or intelligence that lives outside it.
kild-0; children fork from a base branch.owned(kild runs the process) orattached(an external harness kild addresses but never spawns). Its role is a persona — invisible to the engine.send— directed, peer-to-peer. The sender names the recipients, so "who is this addressed to?" stops being a question the engine can get wrong. Addressing a handle nobody holds creates that agent, which is how delegation starts; there is no separate spawn tool.Hard authority (spawn/stop/kill, guardrails) stays with the human as the control seat — it cannot be delegated to something running inside what it may need to kill.
Why
kild's own rule is "ship mechanism, never intelligence," and the Room model broke it: the engine encoded an org chart (operator → lead → worker), a privileged
@human, a report-and-idle norm, and a decision protocol parsed out of message prose. The recurring "who is this addressed to?" bug — which bit in three separate places — was the symptom.VISION.mdalready described this model; the implementation drifted.$KILD_HOMEstate dir moved$KILD_HOME/rooms/<id>.json→$KILD_HOME/kilds/<id>.json. An engine with existing archives will not find them after this branch. One-time fix on any machine that has run kild before:No migration code ships for this — it is a single-operator tool and a one-line
mv, and auto-migration is the kind of machinery this branch exists to delete. Full list of what an existing setup must change:docs/upgrading.md.a63476dEverything before it is the old API; everything after is the new one.
docs/helm-migration.mdhas the full mapping, a checklist, and the pin/un-pin sequence. Three corrections have landed in it since it was first written, all worth reading before you port:collidesWithwas never on the wire — derive it client-side fromgit.changedFiles. That is the correct approach, not a workaround.GET /api/kilds/:id/agents/:handle/transcriptis the surviving transcript route, not the doomed one. An earlier draft said the reverse; reasoning for the reversal is inapi-surface.md§1.kild_new/kild_send/kild_spawn/kild_stopare deleted — every kild mutation is a REST call, because those frames could not report a rejection.State of the branch
bun testbun run typecheckbun run lint./scripts/e2e.shEngine source (non-test): 6,710 → 7,582 lines — the rename and demolition took it to 5,761, and the reshape (land, disposal, hooks, attribution, inbox, seq) added back deliberately.
Verified live against real pi calls: kild created → agent spawned → real turns → commit → land with a merge sha →
onClosehook → ledger entry.Review round: what a full review found, and what changed
Seven specialist agents plus live reproduction reviewed the branch at
bff0fbf/c0fbd81. Every Critical and every Important finding that reproduced is fixed on this branch. The interesting ones:prompt()'sfalsewas discarded andidlewas corrupted on a process that would never emitagent_end. Now: a stopped recipient is refused up front, and a session that died is discovered at delivery — marked stopped, and the sender is told. The suite could not see this because the fixture returnedtrueunconditionally; it now models both.kild new --agents not-a-personahung forever. The WS frame was fire-and-forget and the rejection was aconsole.warninside the engine. Fixed by deleting the mutating frames rather than inventing a rejection protocol.discardedError: an unanswerable question is not an empty list./varis a symlink to/private/var, and half the engine's paths came from git (resolved) and half from$KILD_HOME(not). One canonical spelling, plus a lazy archive load so$KILD_HOMEis no longer captured at import.code: 0 commitsbesidelanded: yes, because facts were collected after the merge. The land now records what it carried.kild send <id> "<text>", which fails without--to.One finding is deliberately not actioned:
BASELINEindefault-prompt.tsships working practice in engine code. That is the intended behaviour — a baregeneralagent with no intelligence layer should still be competent — and it is a boundary call, not a defect.Deferred
server.ts(~1,100 lines) andcli.ts(~900) are god-modules flagged inDEMOLITION.md. Splitting them is a large mechanical diff that would bury the review value of everything above, so it is its own branch.Planning docs on this branch
DEMOLITION.md(full-tree audit) ·VOCABULARY.md(canonical naming) ·helm-migration.md(client guide) ·api-surface.md(reshape decisions) ·upgrading.md(what an existing setup must change) ·onclose-hook.md(the lifecycle seam) ·attached-agents.md(the harness transport) ·cross-kild-comms.md·escalation.md·worktree-disposal.md(the 116-worktree leak)🤖 Generated with Claude Code
https://claude.ai/code/session_01NGEZwcNKSJZmuGuQhuFmcU