docs: settled design ADRs from aikit issues audit

docs/adr/0012-sandbox-is-the-trust-boundary-network-perimeter-is-the-control.md

@@ -0,0 +1,59 @@
+# The sandbox is the trust boundary; the network perimeter is the only in-app control
+
+## Status
+
+accepted
+
+## Context
+
+`aikit serve` exposes the in-process `aikit` agent, whose default toolset includes
+`run_bash` (an unrestricted `sh -c`), `write_file`, and `git`. A security audit
+(`specs/ISSUES-2026-07-07.md`, SEC-2) framed this as unauthenticated prompt-driven RCE:
+a caller who reaches the API can steer the agent into arbitrary shell execution on the
+serving host. Two fixes were proposed — gate `run_bash`/`yolo` behind a `--allow-shell`
+flag, and/or make the per-agent tool policy (which already exists as
+`AgentPersona.tools` / `disallowed_tools`, hard-filtered at `loop_runner.rs:355-360`)
+default-deny for dangerous tools.
+
+We rejected both as the *primary* control. An autonomous coding agent whose entire
+purpose is to read, write, and run code is not made safe by removing its ability to run
+code; it is made safe by running it somewhere disposable. aikit's intended deployment is
+inside a sandboxed container (driven by agentrt, the optimization loop, or a chat BFF),
+where taking control of the execution environment is expected agent behaviour, not an
+exploit.
+
+## Decision
+
+**Protecting the host from the agent is the container's responsibility, not aikit's.**
+The in-process agent keeps its full default toolset (`run_bash` included); tool
+availability is a *capability* concern expressed per-agent via `AgentPersona.tools` /
+`disallowed_tools`, not a host-safety control, and is not gated by any serve flag. There
+is no `--allow-shell`.
+
+Because we deliberately do **not** constrain the agent internally, the **network
+perimeter is the sole remaining in-application control**, and it must fail closed:
+`aikit serve` MUST refuse to start when bound to a non-loopback address without an
+`--api-key` (a hard error, not the current warning at `serve/mod.rs:738`). The loopback
+default stays open so existing local consumers (agentrt, optimization loop, chat BFF) are
+unaffected.
+
+## Consequences
+
+- SEC-2's "RCE" reframes from a code defect to a **deployment contract**: aikit must be
+  run in a sandbox. This is documented in the `serve` help text and `webdocs`, and is a
+  precondition for any exposure.
+- The perimeter carries more weight, not less. The sandbox walls off the *host* but not
+  the two things the container still holds — **LLM credentials** (an exposed agent can
+  burn or exfiltrate `ANTHROPIC_API_KEY` / the OpenAI-compat key) and **network egress**
+  (the agent can reach whatever the container can). Fail-closed auth is the only thing
+  standing in front of those, so it is mandatory, not optional.
+- `yolo` (client-settable permission bypass) and client-supplied `mcp_servers`
+  (SEC-3) are **not** security bugs under this stance — they are in-sandbox capability
+  choices. SEC-3's *concurrency cap* (no `max_sessions` on live sessions) remains a real
+  resource-exhaustion defect and is tracked independently of this decision.
+- `AgentPersona.tools` / `disallowed_tools` already exists and is wired; the remaining
+  work is to **plumb a tool policy through serve** (`SendMessageRequest` cannot currently
+  carry one) so callers *may* restrict capability when they want to — an opt-in
+  least-privilege lever, not a default.
+- Operators who cannot sandbox must not expose aikit; there is no in-app substitute for
+  the container boundary, by design.

docs/adr/0013-package-install-writes-only-within-the-target-project.md

@@ -0,0 +1,54 @@
+# Package install writes only within the target project
+
+## Status
+
+accepted (companion to [0012](0012-sandbox-is-the-trust-boundary-network-perimeter-is-the-control.md))
+
+## Context
+
+`aikit install <source>` fetches a package and copies its artifacts into a project
+according to the package's `aikit.toml` `[artifacts]` table. The destination strings come
+from an untrusted manifest and were used as `project_root.join(dest_str)` with no
+validation (`aikit-sdk/src/install.rs:153`). `Path::join` with an absolute value replaces
+the base, and `..` was never rejected, so a package could write outside the project — e.g.
+`"payload/**" = "/home/victim/.ssh"` or `"../../../etc/cron.d"` (audit SEC-1). Related
+traversal exists via skill/subagent names and `source` paths (SEC-5).
+
+Unlike `aikit serve` (ADR 0012), `install` does **not** run in a sandbox. It runs on a
+developer's own machine with their real `~/.ssh`, cloud credentials, and shell rc files
+present. The container trust boundary does not apply here, so the "assume sandboxed"
+stance does not transfer: install is a distinct, un-sandboxed boundary. The npm/cargo
+"install runs arbitrary code" precedent is a known flaw to avoid, not a licence to copy —
+and a *declarative* artifact manifest never legitimately needs to write outside the
+project it is scaffolding.
+
+## Decision
+
+A package may write **only within the target project root**. Enforcement:
+
+1. **Parse-time validation.** Every artifact destination (and skill/subagent name and
+   `source`) is validated when the manifest is deserialized, before any file is written.
+   A single unsafe mapping **aborts the entire install** with a clear error — no partial
+   writes, one error site.
+2. **Reject lexically:** absolute paths and any `..` component are refused
+   (`safe_join(base, untrusted)`).
+3. **No-follow-symlinks on the write path.** Extraction/copy refuses to follow symlinks,
+   closing the escape where one artifact writes `link -> /etc` and a later mapping targets
+   `link/…` (which a lexical-only check would pass).
+4. Flat identifiers used to build filenames (package `name`/`version` → cache dir, SEC-4;
+   client `session_id` → session file, SEC-10) are validated with a strict id charset
+   (`is_safe_id`), not `safe_join`, since they are not path fragments.
+
+This logic lives once in `aikit-sdk` and is shared by every install/extract path,
+consistent with the ARCH-1 direction of unifying the duplicated install/fetch stacks.
+
+## Consequences
+
+- SEC-1 and SEC-5 are closed at the source: a hostile or malformed package cannot escape
+  the project directory.
+- Legitimate templates are unaffected — they only ever write within the project they
+  scaffold.
+- Zip-slip on *extraction* was already mitigated; this closes the separate
+  artifact-*mapping* layer that sat above it.
+- The `safe_join` / `is_safe_id` helpers become the single validation seam reused by
+  SEC-1/4/5/10, so future install/session code inherits the guarantee by construction.

docs/adr/0014-runs-are-cancellable-terminated-via-process-group-signal-escalation.md

@@ -0,0 +1,60 @@
+# Runs are cancellable; termination escalates SIGTERM→SIGKILL over the process group
+
+## Status
+
+accepted
+
+## Context
+
+aikit had no real notion of *stopping* a run. The subprocess Transport spawned the agent
+CLI without process-group isolation, and the only termination path was a watchdog calling
+`child.kill()` on the direct child (`aikit-sdk/src/runner/mod.rs:260-266`). Three P1 bugs
+share this root (audit `specs/ISSUES-2026-07-07.md`):
+
+- **BUG-1** — the prompt is written to stdin on the calling thread *before* the reader
+  threads and watchdog exist, so a large prompt (the documented "use stdin to avoid
+  ARG_MAX" path) deadlocks against a full stdout pipe with no timeout armed.
+- **BUG-2** — on timeout the serve layer emits a `run_timeout` event but does not stop the
+  run or close the response; the record is set `Idle` while the run keeps executing, so a
+  new turn passes the busy check and two runs execute on one session, corrupting the
+  session file.
+- **BUG-4** — `child.kill()` targets only the direct child; grandchildren (tools the agent
+  spawned) are orphaned and keep running, and if any inherited the stdout/stderr write end
+  the reader threads never see EOF and the run *never returns*.
+
+## Decision
+
+**1. stdin is written off the calling thread, after readers exist and the watchdog is
+armed.** The subprocess Transport spawns the stdout/stderr reader threads first, then
+writes the prompt on a dedicated writer thread (dropping `ChildStdin` to signal EOF), and
+the run timeout is armed before the write can block. This removes the BUG-1 deadlock.
+
+**2. Cancellation is a first-class primitive.** A single cancel token is threaded into a
+run and can be triggered by (a) the run timeout, (b) client disconnect, and (c) a future
+explicit interrupt — one mechanism, not three ad-hoc kill paths. This token is the seam
+that ARCH-3's `ControlHandle` later subsumes; it is built now to fix the P1s and is not
+throwaway.
+
+**3. Termination escalates over the process group.** Agent CLIs are spawned in their own
+process group (`setsid` / `process_group(0)`). Cancellation sends `SIGTERM` to the group,
+waits a short grace (~3s) so a well-behaved CLI can flush a session/checkpoint, then
+`SIGKILL`s the group. `kill_on_drop(true)` backstops dropped handles. Killing the *group*
+reaps grandchildren and guarantees the pipes reach EOF, so the run always returns.
+
+**4. On termination the run reaches a terminal state and the response closes.** The record
+becomes `Failed`/`Closed` (never `Idle`), the channel is closed so both sync-drain and SSE
+clients unblock promptly, and a terminal error frame (e.g. `run_timeout`) is the last thing
+the client sees — no late frames from a zombie run, and the busy check cannot admit a
+second concurrent run on the same session.
+
+## Consequences
+
+- BUG-1, BUG-2, BUG-4, and the client-disconnect leak in BUG-7 are fixed by one shared
+  mechanism rather than four point patches.
+- The timeout test must exercise a *real* slow run, not the current instant stub, or the
+  behaviour stays unverified.
+- The cancel token is forward-compatible with the `Session`/`ControlHandle` trait from the
+  ARCH-3 direction; when that lands, cancellation becomes one control op among several
+  (interrupt, set-permission, disconnect) over the same seam.
+- Process-group spawning is Unix-shaped (`setsid`); the Windows path (job objects) is noted
+  as follow-up but not required for the primary sandboxed-Linux deployment.

docs/adr/0015-two-agent-registries-deploy-layout-vs-runnable-backend.md

@@ -0,0 +1,63 @@
+# Two agent registries: deploy-layout vs runnable-backend
+
+## Status
+
+accepted (resolves audit ARCH-2; builds on [0008](0008-backend-identity-enum-transport-trait.md))
+
+## Context
+
+Agent identity/config was spread across three tables that had silently diverged
+(audit `specs/ISSUES-2026-07-07.md`, ARCH-2):
+
+- `src/core/agent.rs` `EXTRAS` — {install_url, requires_cli, arg_placeholder, folder};
+  keys include `cursor-agent`, `qwen`, `windsurf`, `copilot`.
+- `src/models/config.rs` `default_agents` — {name, folder, install_url, requires_cli,
+  output_format, output_dir, arg_placeholder, extensions}; key `cursor`.
+- `aikit-sdk/src/lib.rs` `AgentEntry` — {commands, skills, subagents, instruction_file};
+  key `cursor-agent`.
+
+The divergence was not merely stale values. The tables disagree on the **set of agents**
+and their **keys** (`cursor` vs `cursor-agent`), carry **different fields**, and use the
+same field name for **two different questions**. `requires_cli` is the clearest example:
+`core/agent.rs` says gemini `requires_cli: true` (gemini is an external CLI we spawn to
+*run* an agent — correct in the runnable sense), while `models/config.rs` says `false`
+(asking a different question — whether *scaffolding* `.gemini/` files needs the CLI
+present). Merging into one fatter table (the audit's original "one registry" framing)
+would preserve the tangle and force us to adjudicate conflicts that only exist because two
+concerns share a name.
+
+## Decision
+
+Split by responsibility into **two registries joined by one canonical key**:
+
+1. **Deploy-layout registry** — for *every* agent, including deploy-only ones (copilot,
+   windsurf) that are never spawned. Owns where an agent's files live: `folder`,
+   command/skill/subagent directories, `instruction_file`, `arg_placeholder`,
+   `output_format`, `install_url`. The SDK `AgentEntry` table is the most complete and
+   becomes the single source; `core/agent.rs::EXTRAS` and
+   `models/config.rs::default_agents` are deleted and their unique fields folded in.
+
+2. **Runnable-backend registry** — the existing closed `Backend` enum (ADR 0008):
+   claude, codex, gemini, opencode, cursor, aikit. This owns "can we spawn/drive this as
+   an agent." `requires_cli` **disappears as a stored field** — it is *implied*: every
+   external Backend requires its CLI, `aikit` does not, and deploy-only agents are not
+   Backends at all.
+
+3. **One canonical key per agent** across both registries (resolve `cursor` vs
+   `cursor-agent`), so a config value, a Backend, and a deploy layout always refer to the
+   same identifier.
+
+## Consequences
+
+- The value conflicts dissolve rather than being adjudicated: gemini is a runnable Backend
+  (CLI trivially required) *and* has a deploy layout — never the same fact, so they can no
+  longer disagree.
+- The `cursor`/`cursor-agent` key mismatch (a real routing bug where a configured `cursor`
+  missed the SDK's `cursor-agent` row) is closed by the canonical-key rule.
+- Adding or changing an agent touches one deploy-layout row (and, if runnable, the Backend
+  enum) — not three tables in two crates.
+- Deploy-only agents (copilot/windsurf) are representable without pretending they are
+  Backends; runnable-only concerns stop leaking into the deploy table.
+- Lands in Phase 2 of the remediation, alongside the ARCH-1 install/fetch unification;
+  the stringly per-agent `match` blocks in `aikit-sdk/src/lib.rs:88-114` become fields on
+  the deploy-layout registry.

docs/adr/0016-serve-emits-canonical-events-no-streamframe-no-back-compat.md

@@ -0,0 +1,45 @@
+# serve emits canonical agent events; StreamFrame is deleted, no back-compat shim
+
+## Status
+
+accepted (realizes [0005](0005-agent-events-are-the-shared-streaming-protocol.md); resolves audit ARCH-4)
+
+## Context
+
+`src/cli/serve/mod.rs` defines a second event vocabulary (`StreamFrame`) and lossily
+re-maps the canonical `AgentEventPayload` onto it before writing SSE. The remap drops
+events consumers want (token usage, reasoning, subagent activity, context compression,
+step-finish — per `specs/serve-agent-fidelity-and-ndjson-parity.md`) and overloads
+`ToolResult.name` with two meanings (agent-key for CLI backends, call-id for structured
+ones). This contradicts ADR 0005, which established the canonical agent events as *the*
+shared streaming protocol across serve, agentrt, and the optimization loop.
+
+serve's consumers (agentrt, the optimization loop, the chat UI) parse the current
+`StreamFrame` shape today, so changing the wire is a breaking change for them. The project
+stance is to **advance the design rather than preserve backward compatibility** (greenfield,
+break for the better shape). A dual-vocabulary/additive option was considered and rejected
+as the compatibility option it is.
+
+## Decision
+
+serve serializes the canonical `AgentEventPayload` **directly** over SSE. `StreamFrame` and
+its translation layer are **deleted** — no dual emission, no deprecation window, no
+back-compat shim. Consumers migrate to the canonical shape in one step.
+
+This lands in **Phase 2** of the remediation, **independent of and ahead of ARCH-3** (the
+session-trait seam). ARCH-4 overlaps ARCH-3 in the serve file but does not depend on it:
+emitting canonical events is a contained change at the serialization point, so it is not
+held back to be bundled with the larger seam rewrite.
+
+## Consequences
+
+- ADR 0005 is realized on the serve surface: one event vocabulary end-to-end, no lossy
+  translation.
+- Previously-dropped events (token usage, reasoning, subagent, compression, step-finish)
+  reach consumers, unblocking cost display and richer chat UI.
+- The overloaded `ToolResult.name` disappears with the remap; `call_id` and agent identity
+  are distinct canonical fields.
+- Consumers (agentrt, optimization loop, chat UI) take a one-time breaking migration in
+  Phase 2. This is accepted as the intended direction of travel, not a regression.
+- The duplicated `UsageSource`→string map (`serve/mod.rs:425` vs `run_progress.rs:199`) is
+  removed in the same change.