# Architecture decisions (why, not what)

This file captures **intentional choices** — patterns that look unusual but are deliberate. It prevents agents from "fixing" things that aren't broken.

## Decisions

| Decision | Why chosen | What NOT to do | Source |
|----------|-----------|---------------|--------|
| SQLite single-file DB (no PostgreSQL) | Self-hosted simplicity, zero-config, WAL mode sufficient for local use | Do NOT migrate to PostgreSQL | `backend/src/db/`, `docker-compose.yml` |
| Inline styles (no CSS framework) | No build complexity, no class name conflicts, theming not needed yet | Do NOT add Tailwind/CSS modules | `docs/inconsistencies-tech-debt.md` (TD-20260306-inline-styles) |
| Single Mutex on SQLite (now with spawn_blocking) | SQLite single-writer model, connection pool adds complexity for local app | Do NOT add r2d2 or deadpool | `backend/src/db/` |
| Agent CLIs spawned as child processes | Universal compatibility, no API lock-in, works with any CLI agent | Do NOT use agent SDKs/APIs directly | `backend/src/workflows/`, `backend/src/api/` |
| All docs/ files in English | Universal agent compatibility, consistent context regardless of project language | Do NOT write docs/ files in project language | `docs/AGENTS.md` |
| Economy tier for summaries | Save tokens, summary doesn't need reasoning capability | Do NOT use default/reasoning tier for background summaries | `docs/AGENTS.md` (§ Stack — ModelTier system) |
| Template redirectors are autonomous (not just redirects) | Small-context agents can't follow redirects; they need actionable content inline | Do NOT revert to 2-line redirect format | `templates/CLAUDE.md`, `templates/.cursorrules`, etc. |
| Agent stall timeout configurable (1–60 min, default 5) | Different agents/tasks need different timeouts; hard-coded 5min too short for audits, too long for simple chats | Do NOT hard-code stall timeout; always read from ServerConfig | `backend/src/agents/runner.rs`, `backend/src/api/setup.rs` |
| TTS/STT 100% local (Piper + Whisper WASM) | Privacy-first, no cloud dependency, works offline, no API costs | Do NOT add cloud TTS/STT providers (Google, Azure, etc.) | `frontend/src/lib/tts-*.ts`, `frontend/src/lib/stt-*.ts` |
| Tauri desktop: backend embedded, not sidecar | Simpler deployment, single binary, no process management, no port conflicts | Do NOT use Tauri sidecar pattern for the backend | `desktop/src-tauri/` |
| RTK spawned with container's own `HOME`, not `KRONN_HOST_HOME` | In Docker, `/home/kronn/.claude`, `.codex`, `.gemini` are bind-mounted rw from the host — writing through the container HOME lands on the real host file. `KRONN_HOST_HOME` is the *host path* (e.g. `/home/priol`) which doesn't exist inside the container; passing it as HOME makes RTK try `mkdir /home/priol/.claude` and fail with "failed to create directory" | Do NOT override `HOME` when spawning `rtk init ...`. If a new feature needs host access, go through a dedicated bind-mount, not via HOME tricks | `backend/src/api/rtk.rs::activate` (doc-comment), `backend/src/core/rtk_detect.rs::resolve_home` |
| RTK activation is per-agent, not one-shot | `rtk init -g` alone only wires Claude Code. Codex/Gemini need `--codex` / `--gemini`, and `--hook-only` is incompatible with those (the hook *is* the flow). `--auto-patch` is mandatory for non-interactive — without it RTK waits on a TTY prompt, exits 0, does nothing while reporting "success" | Do NOT fit all agents under a single `rtk init` call. The `rtk_args_for(agent_type)` matrix is the single source of truth — update it if a new flag surfaces, don't inline per-agent tweaks | `backend/src/api/rtk.rs::rtk_args_for` |
| Toast severity asymmetric: success ephemeral, error sticky + copyable | A success confirms an action the user initiated. An error interrupts flow, needs diagnostic time, gets pasted to tech colleagues. Same treatment = bad UX. Validated by both a senior UX designer and a lambda-user persona | Do NOT pass `persistent: false` to a `toast(..., 'error', ...)` unless truly transient. Always attach raw stderr as `copyable` so the user selects/copies without screenshotting | `frontend/src/hooks/useToast.tsx` |
| Partial response checkpoint preserves `started_at` via `COALESCE` | Recovered Agent message must sit chronologically BEFORE any user message sent after restart. Without this the user sees "my question" → "their old answer" and thinks the agent replied twice (2026-04-13 bug) | Do NOT use `Utc::now()` inside `set_partial_response` — always COALESCE with the first checkpoint timestamp | `backend/src/db/discussions.rs:set_partial_response`, migration 032 |
| Dismiss-partial endpoint reuses the boot recovery function | Single source of truth — same code path scans all dangling partials, inserts footer message, broadcasts WS. Prevents drift between "user clicked dismiss" and "backend just restarted" recovery semantics | Do NOT duplicate recovery logic in the dismiss handler | `backend/src/api/discussions.rs:dismiss_partial` + `main.rs:110-124` |
| Cancel cascade via `parent_run_id`, not a registry map | Parent→child linkage is stored in DB (`workflow_runs.parent_run_id`) and queried by cancel_run. Survives backend restart; child discovery is SQL-native | Do NOT keep parent/child relationships in-memory only | migration 030, `backend/src/api/workflows.rs:cancel_run` |
| Module-level tracker for in-flight step tests | React unmount on tab switch wipes `useState`. A module-level Map + subscribe/notify lets StepCard observe its own test state across remounts, and the tracker owns the AbortController so navigating away doesn't kill the stream | Do NOT move the test stream state back into component-local `useState` — it will break tab-switch persistence | `frontend/src/components/workflows/WorkflowDetail.tsx` (activeStepTests, subscribeStepTest, patchStepTest) |
| Shared API mock factory (`buildApiMock`) over inline `vi.mock` factories | Inline slim mocks break every time a shared code path (e.g. I18nContext importing `config.getUiLanguage`) gains a new dependency. Centralized factory + completeness test = one place to update. Must be used via `vi.hoisted` because `vi.mock` is hoisted above imports | Do NOT add more inline `vi.mock('../../lib/api', () => ({...}))` factories in new tests | `frontend/src/test/apiMock.ts`, `apiMock.complete.test.ts` |
| Cross-agent parameterized tests over hardcoded per-agent tests | Adding Gemini mount but not Claude mount is invisible — per-agent tests only cover what they explicitly test. Parameterized tests iterate ALL agents and fail on ANY gap. Same principle as i18n parity tests | Do NOT write agent-specific tests for things that should be universal (DB round-trip, macOS skip, frontend label). Use the cross-agent suite instead | `agents/mod.rs` (5 tests), `db/tests.rs` (1 test), `constants.test.ts` (3 tests) |
| `Option<Option<String>>` doesn't work with serde JSON null — use `""` for "unset" | Serde deserializes both JSON `null` and absent key as `None` for `Option<Option<T>>`. The double-option trick for distinguishing "not sent" vs "sent null" doesn't work. Frontend sends `""`, backend treats `""` as unset | Do NOT rely on JSON `null` to mean "clear this field" with `Option<Option<T>>` — always send `""` and handle it server-side | `ChatHeader.tsx:475`, `api/discussions.rs:379` |
| MCP per-config edition lives ONLY in the McpPage drawer (single-source-of-edit) | Phase-3 host_sync first surfaced the radio in BOTH McpPage AND a dedicated SettingsPage > "Portée CLI locale" section. The user pushed back: same data edited in two places ⇒ "which one wins?" cognitive load. Unified to one editor (Plugins drawer = canonical) + read-only chip on the card grid for at-a-glance state. Antoine UX review (Stripe / Linear / Notion pattern) + user intuition matched. | Do NOT re-add a Config-side editor for an MCP attribute (host_sync, env, project_ids, label, args_override). If a future feature seems to need a "bulk view", add a filter/pill on the Plugins page instead — route clicks through the canonical drawer. | `frontend/src/components/HostSyncChip.tsx`, `frontend/src/components/HostSyncPreview.tsx`, `frontend/src/pages/McpPage.tsx`, memory `feedback_mcp_single_edit.md` |
| Claude scope-aware host write — `is_global` and `project_ids` route to the right Claude scope | The first Phase-3 implementation had `host_sync` as an independent dimension (3 modes None/GlobalOnly/MirrorAll) that overrode the user's project_ids selection. The user spotted the inconsistency: "if I scope to APP_ANDROID, why does Claude write top-level?". Fixed by routing `is_global=true` → top-level mcpServers (Claude scope `user`), `is_global=false + project_ids` → `projects[<host-path>].mcpServers` for each project (Claude scope `local`). Other CLIs lack a native local scope so they stay top-level — visible asymmetry, not silent. Migration 038 collapsed `MirrorAll` to `is_global=true + host_sync='GlobalOnly'`. | Do NOT add a third "scope" dimension orthogonal to `is_global` + `project_ids`. The CLI sync MUST follow the Kronn scope. If a future CLI gains a per-project scope (Codex, Gemini), route to it the same way Claude is routed today. | `backend/src/core/mcp_scanner.rs::sync_claude_global_config` (Phase-3 refactor), migration 038, `frontend/src/components/HostSyncPreview.tsx` (renders the asymmetry), `docs/AGENTS.md § Host MCP sync` |
| `_kronn` marker on every Kronn-written entry, hash fallback for ownership | Initial design used hash-only ownership detection. MCP Expert flagged it as fragile: hash breaks on JSON reformatting, manual env edits, key reorders → orphan cleanup misclassifies + orphans accumulate forever. Switched to explicit marker `{managed: true, config_id: <uuid>}` on every entry Kronn writes. Hash kept as fallback for entries written by older Kronn versions or copy-pasted by the user. | Do NOT remove the `_kronn` marker emit when adding a new sync target — orphan cleanup walks the marker, not the hash. Treat the marker as the canonical ownership signal and hash as best-effort legacy detection. | `backend/src/core/mcp_scanner.rs::build_kronn_managed_json_entry` + `is_kronn_managed` + `drop_all_kronn_entries`, plan v2 review (MCP Expert P0) |
| Windows: resolve bare program names via `which` in `cmd.rs`, don't expect CreateProcess to find `.cmd`/`.bat` | `Command::new("npx")` fails on Windows even when Node.js is installed because Win32 CreateProcess refuses to execute `.cmd`/`.bat` wrappers when called by their bare name — only `.exe` works without an extension. Reported by a Windows user (npm-installed Node.js, npx accessible from PowerShell, but Kronn raised "Spawn failed for npx: program not found"). Fix: `async_cmd`/`sync_cmd` resolve bare names through `which::which` on Windows before passing to `Command::new`, returning the fully-qualified `.cmd` path. Skipped for paths that already contain `\`, `/`, or an extension. | Do NOT bypass `cmd::async_cmd`/`cmd::sync_cmd` on Windows — using raw `Command::new("npx")` will reintroduce the bug. The cross-platform contract is: pass a bare name, the helper handles the rest. | `backend/src/core/cmd.rs::async_cmd` + `sync_cmd` + `resolve_windows_program` |
