CLAUDE.md

ShipIt is a browser-based AI editor — describe what you want in chat, the agent writes the code, and you see results live. The agent runs as a CLI inside a session container; Claude Code CLI is the default backend, Codex CLI is also supported, and the architecture is agent-agnostic so additional backends can be added later. Authentication normally uses the user's existing subscription with the chosen provider; a metered API key exists only as a fallback ranked below connected accounts.

Product principles

These govern what ShipIt is. They override convenience, "what other tools do," and "this is how the underlying platform works." A proposal that conflicts with one is wrong. Design docs cite them by number — §1–§5 are stable identifiers; compress the prose, never renumber.

1. ShipIt is the surface. The user does not leave it.

You build, review, ship, and debug inside one chat-shaped IDE. PRs, CI status, deploy status, diffs, commit history, conversation history, terminal output, preview, and merge conflicts all surface inline — no GitHub tab, hosting dashboard, CI tab, or local terminal required. Sending the user elsewhere is a failure of the product, not a feature.

2. Inline beats link-out. Always.

If the upstream system has the data, ShipIt fetches and renders it. Links to GitHub or the cloud provider are escape hatches in overflow menus, never the happy path — "View on GitHub" sits beside the PR card, and we never bounce anyone to GitHub's diff viewer. The reason is compounding: once the user is reading a PR on github.com, the next comment, re-request, and fixup happen there too. The cycle has to start somewhere; we keep it inside.

3. External tabs are reserved for things ShipIt does not own.

The whole list: OAuth / auth flows (Anthropic, GitHub own their login screens), account and billing pages (provider billing, GitHub repo creation and settings), and external documentation the user explicitly clicks through to. "The PR was created so let's open it" is not on it.

4. If we don't render it inline yet, that's a backlog item, not a license to link out.

The link-out acknowledges we haven't built the inline view. It isn't the design.

5. Chat is the input surface. The agent is the actor.

The user describes intent; the agent runs the commands, edits the files, reads the logs, runs the tests. We deliberately do not give the user shell-shaped affordances — quick-action button rows, command palettes that execute shell, hotkey-bound task runners, "click to run npm test" buttons. Those belong to terminal-shaped IDEs; here they're a category mistake that nudges the product back toward the CLI wrapper it's trying to replace. The legitimate needs already have primitives:

Corollary: "saves an LLM round-trip" is not a feature. Spending a turn on a routine command is the intended cost of chat-shaped UX — it keeps the agent in the loop and the chat history complete. The user still navigates, reviews, instructs, accepts, rolls back, branches, merges; they just don't operate the box. That's what they hired the agent for.

Corollary: how to evaluate proposals

1. Does it need a tab outside ShipIt, or assume GitHub is open elsewhere? Redesign, unless it's a §3 exception.
2. Is the link-out the primary affordance rather than an overflow escape hatch? Redesign.
3. Does it give the user a shell-shaped affordance for a command the agent could run? It solves a problem ShipIt doesn't have — §5.
4. Does it miss this user's goal or way of working, even while it follows the principles above? Redesign.

Runtime

ShipIt runs inside Docker containers: the orchestrator runs in a container and spawns session worker containers. The one exception is the dogfood inner instance (RUNTIME_MODE=local), which skips Docker entirely — see Dogfooding ShipIt in ShipIt.

Setup

npm install

Important: If any npm command fails with missing node_modules (e.g., Cannot find package), run npm install first.

Commands

• npm run test:dev — dev default. Only tests affected by uncommitted, staged, and untracked (newly created) changes + smoke tests (-- --list to dry-run). Use while iterating.
• npm run test:smoke — smoke tests only (core connectivity, HTTP bootstrap, git, one client component).
• npm test — full suite. The exception, not the habit — CI runs it on every PR; the paragraph below this list says when it is warranted. Targeted run: npx vitest run <file>.test.ts.
• npm run lint:dev — dev default. ESLint over files changed vs origin/main + uncommitted + untracked (newly created) (-- --list to dry-run). The full lint loads all ~2400 TS files; CI runs it, so this is the inner loop.
• npm run lint — full ESLint on src/, uncached on purpose (~215 s every time). Sparingly — when you suspect a cross-file rule (e.g. no-deprecated) tripped elsewhere. That is exactly what a cache breaks: ESLint keys an entry on the file's own content plus the config (getValidCachedLintResults in eslint's lint-result-cache.js), and never on the files it imports, so a cached clean result for an unchanged caller survives its import being deprecated. Measured: adding @deprecated to stripAnsi was 5 errors uncached and 0 cached. So this command must not be cached — CI runs it as the source of truth, and lint:dev is the cached inner loop, which is incremental and correspondingly not authoritative. Its old-space cap is LINT_HEAP_MB, defaulting to 3072 so the run still fits a floor-sized (4 GiB) session container; CI sets 6144, the smallest tested cap matching 8192's runtime. Cold, on a 43 GiB container 2026-09-16: 230 s / 3319 MiB peak RSS at 3072, 176 s / 4766 MiB at 6144. Raise LINT_HEAP_MB locally if a cold full lint is slow, and re-measure before moving CI's number.
• npm run typecheck — tsc --noEmit, incremental (warm ~5 s). Whole-project by design, no per-file variant.
• npm run build — Vite client build. (npm run dev is the Vite/tsx dev server, but don't start it in bash to preview — ShipIt serves the preview via the dev Compose service in docker-compose.yml, which runs npm run dev itself; see Dogfooding ShipIt in ShipIt. A bash-started server is also reaped when the container goes idle.)

Session containers are sized from host capacity (docs/229), so npm test and the integration tests can run in-box — but "can" is not "should". A full run holds a pool of workers for minutes (vitest.config.ts caps it at 8 on a host with more cores than that, because an uncapped pool sizes itself from the container's visible cores). Many sessions share one host with the orchestrator, whose single main thread is what serves the UI: three concurrent full runs on a 16-core host pushed load to ~75 and made static GET / take 12–14 s, which every user of the instance saw as "Reconnecting to server".

So run the affected tests, not the suite: npm run test:dev, or npx vitest run <files> for a target you already know. The full suite is for CI, the release gate, an explicit request from the user, or a change you have reason to believe is wide — a shared type, a manager used everywhere, a new required field on an interface test fakes implement. "I'm about to call this done" is not one of those reasons.

A genuine OOM means the host is undersized — raise DEFAULT_SESSION_MEMORY_MB rather than concluding the suite can't run locally.

Always kill child processes via killChild() (shared/kill-child.ts), never child.kill() — on a spawn that never exec'd, child.kill() signals an arbitrary unrelated pid (full mechanism: that file's docstring). An agent CLI takes killProcessTree() instead, from the same file: it is the root of a tree (MCP servers, and a Playwright browser under those), and a pid-only kill leaves that tree running on pid 1 for the container's lifetime — docs/289-agent-process-tree-teardown. Diagnose before naming an OOM: a suite dying part-way at exit 143 is that friendly fire, not memory; a real OOM is exit 137 and is recorded at /sys/fs/cgroup/memory.events → oom_kill.

Debugging the UI

Before you say a change works, run the path the user takes, in the state the user is in, and look at the result. For UI, start the dev service (the dogfooding-shipit skill) and open the page as the user has it — a session already open when it merges, the second message — and check layout and scroll. For a shared definition such as "done", check every list that uses it. For an install or Compose change, run a fresh install and an update. Say what you could not run.

The Playwright MCP server is configured and launches its own browser. Use browser_navigate to open the ShipIt UI (e.g. http://127.0.0.1:3000), then browser_snapshot to read page state, browser_click to press buttons, browser_fill_form to type text, and browser_take_screenshot for visual checks.

Dogfooding ShipIt in ShipIt

The repo runs inside itself via the manual dev Compose service (shipit service start dev), an inner orchestrator on RUNTIME_MODE=local. Load the dogfooding-shipit skill for local-mode limitations, which credentials to set (almost none), seeding, and driving the inner instance over HTTP.

Project structure

Entry point is src/server/orchestrator/index.ts (buildApp()). src/server/orchestrator/ is the main process, src/server/session/ runs inside session containers, src/server/shared/ is used by both, and src/client/ is the React 19 + Tailwind v4 frontend. android-snapshot-test/ and android-overlay-test/ are separate Gradle builds that Node tooling ignores (docs/213). Per-area file maps live in the architecture skills; ls beats a directory tree that silently rots.

Architecture

Three-layer system: browser (React SPA) → orchestrator (Fastify) → session workers (Docker containers).

Architecture reference lives in .agents/skills/ (.claude/skills is a symlink to it), disclosed progressively. Edit a skill at its .agents/skills/ path: Claude Code asks for approval on any path with a .claude segment, including one that resolves through that symlink. Both backends read them — Claude and Codex auto-disclose the same set with descriptions (no .codex/skills/ needed, and no catalog duplicated here), so detail demoted into a skill reaches both; see docs/209-cross-agent-skill-disclosure. Always-on invariants belong in this file (shared with Codex via the AGENTS.md symlink), not in a skill — and a skill can lag it, so where they disagree the invariant below wins and the skill needs fixing.

Key patterns

These are non-obvious architectural patterns that aren't apparent from the file structure alone.

Orchestrator ↔ container communication

HTTP-only, never Docker exec: commands out via worker-http.ts, events back over SSE. ProxyAgentProcess delegates to the container so local and remote agents look identical. Detail: session-containers, session-processes.

WebSocket lifecycle MUST NOT affect server behavior

The WebSocket connection is a transport between the browser and the orchestrator. It must not be allowed to drive server-side state, agent lifecycle, container lifecycle, or persistence. Disconnects, reconnects, browser crashes, and network blips are all expected and routine — none of them should change what the server is doing.

• Capture per-connection state at the top of long-running functions, never inside async callbacks. runAgentWithMessage and wireAgentListeners capture runner, capturedSessionId, capturedSessionDir once at entry; anything in agent.on(...), setTimeout, Promise.then, or a recursive call reads ONLY those.
• Resolve runners through resolveRunner(ctx) / the registry, never ctx.getRunner() — that returns the per-connection attachedRunner, which becomes null on close, while the registry lives for the whole process. Then read and mutate the resolved runner directly (runner.running = false); the old ctx.setIsClaudeRunning-style setters were deleted precisely because they became silent no-ops after disconnect (docs/095).
• Emit via runner.emitMessage(), not ctx.send(). It broadcasts to every attached viewer AND buffers into the turn-event log, so reconnecting viewers see post-turn messages; ctx.send writes to one socket and silently drops on a closed one.
• A close handler only calls detachFromRunner(). Never runner.dispose(), agent.kill(), terminal.kill(), or container.destroy() from any WS lifecycle event. Disposal belongs to the periodic idle enforcer (docs/284: reclaims ONLY when ShipIt is over its memory budget, longest-idle first — never a runner whose agentBusy is true or that has an attached viewer; elapsed idle time alone reclaims nothing) and explicit user actions (archive, repo delete, full reset, shutdown), which pass { force: true }.

Executable contract: integration_tests/ws-disconnect-resilience.test.ts.

Service layer & type system

Three-tier Routes/WS handlers → Services → Managers: services/*.ts are pure async fns over domain types (not handler context), reusable by both; errors are ServiceError(statusCode, message). WS messages are discriminated unions keyed on a type literal, narrowed by the dispatch switch. WS handlers compose ConnectionCtx / RunnerCtx / AppCtx and declare only what they need. Detail: server-architecture.

Post-turn flow

After a turn (agent_result in agent-execution.ts): postTurnCommit() auto-commits → PR lifecycle card emitted (if a remote exists) → the auto-push is armed LAST (if GitHub auth). The push is armed after the card flow, not inside the commit, because that flow does its own synchronous git push (a forcePush when re-arming past a merged PR) and a debounced plain push racing it is rejected non-fast-forward — reported to the user as a branch divergence that never happened. The commit hands the arm to turn-executor.ts via postTurnCommit's deferPushArm; the merged-branch REFUSAL is still decided at commit time, since the docs/202 re-arm clears mergedAt inside the flow. Ordering it this way is what lets the debounce be 0 (autoPushDebounceMs, whose doc comment carries the evidence that it never coalesced anything). Critical: session context (sessionId, sessionDir) is captured at turn start, not at "done", so a mid-turn session switch can't corrupt commits.

Five invariants govern the rest. Each looks reorderable and is not; each is pinned by a guard test.

1. The queue never drains before the finished turn's work is committed (planning#264). A queued turn may begin by discarding working-tree state (git reset --hard, a branch reset), and edits that never entered git have no reflog entry and no recovery — so tryDrain (turn-executor.ts) awaits the LOCAL commit first. Only the local half: the NETWORK half (PR card, merged-session re-arm, release flow) stays after the drain, so back-to-back messages never wait on a GitHub round-trip. The guarantee lives inside tryDrain rather than at the call sites because the non-streaming path drains at agent_result and commits later in done. Also load-bearing: session_agent_finished fires after the drain (no finished→started flicker) but before the network flows; the runner "idle" event (signalIdleIfIdle, which drives CI-fix / conflict remediation) fires last of all, so remediation never runs against a pre-commit tree. A rewind is the same hazard from outside the queue, so rollback-handlers.ts refuses while runner.turnCommitPending — a hold the executor takes with the lease and drops when the local commit settles (planning#636; guard: rewind-uncommitted-turn.test.ts). So is a turn that arrives with the queue EMPTY: running is already false while that hold is open, so dispatchOnRunner queues a dispatch behind it (turnBlockedByPendingCommit), the executor releases the queue when the commit settles or its hold expires, and a typed message waits for it (turnCommitSettled) rather than queue — docs/240-unlosable-turn-dispatch Fix G; guard: turn-admission-commit-pending.test.ts.

2. Every terminal path runs the commit — including the ones where the agent process dies. runCommitAndPr is reachable from four places in turn-executor.ts, not two: the clean end, the streaming abnormal exit (done with no agent_result — crash, OOM, SIGTERM), the adapter-level error path (where a dispatched crash lands), and the failed auth heal. The latter three once ended at tryDrain() alone — which commits only when something is queued, so a dead agent with an empty queue committed nothing at all and its edits sat in the working tree. runCommitAndPr is memoized precisely so all four can call it unconditionally without duplicating a GitHub round-trip. Adding a terminal path? Call it. Guards: turn-crash-commit.test.ts, turn-drain-commit-ordering.test.ts.

3. Reachable is not enough — the commit must be UNSKIPPABLE, so every step preceding it runs through postTurnStep (planning#279). The commit is sequenced last, behind steps that touch SQLite, the credentials tree and viewer transports, inside un-awaited async listeners. A throw there used to abandon the rest as an unhandled rejection, and nothing ran again: the transcript was already persisted, running cleared, every viewer told the turn finished — and a resident streaming process never fires done, so no reconciler picked it up. Edits sat uncommitted with no error anywhere until a later turn's git add -A swept them up under the wrong summary (PR #1890 shipped one commit short this way). postTurnStep logs and continues; it reorders nothing. Guard: turn-commit-not-skippable.test.ts.

4. A branch whose work shipped under a different SHA returns to base via shipit branch reset-to-base --force --reason "<why>" — never a rebase (planning#279). After a squash merge, computeResetEligible's HEAD === mergedHeadSha clause is terminal, so without the override the session can never open another PR. Rebase is not the alternative it appears to be: the base holds one commit with the branch's final state while the branch's first commit adds the same paths in their initial state, so git rebase origin/<base> hits add/add conflicts instead of dropping already-shipped patches. --force bypasses that one clause and nothing else — the clean-tree and coherence checks stay, and the required reason plus the forced transcript card are the record that replaces the gate. git reset --hard stays blocked by block-branch-ops. Guard: reset-to-base-force.test.ts.

5. Post-turn work is held on a lease, and the push does not live on the runner. A turn's terminal sequence runs after running goes false, so for its whole duration the runner read as idle and the enforcer was free to dispose it. post-turn-hold.ts closes that: beginPostTurnWork/endPostTurnWork (counted, deadline-bounded, paired in a finally) fold into agentBusy AND make a non-forced dispose() decline, so the two answers cannot disagree. The debounced auto-push is part of that sequence and takes the same lease from the moment it is armed — but it lives in services/auto-push-scheduler.ts, keyed by session and lived for the process, never as a setTimeout handle on the runner. Storing it there gave one push ways to vanish with no log line at all: the scheduler resolved a runner and returned silently when the registry and attachedRunner were both empty, and dispose() cancelled an armed timer outright. So: never store post-turn work on the runner — resolve one only to report and to take the lease — and say something on every path that ends without a push. Relatedly, the adapter-error path signals idle after onError has run the drain and commit, the last-of-all ordering invariant 1 states and that path alone used to violate. Guards: services/auto-push-scheduler.test.ts, integration_tests/auto-push-success.test.ts, integration_tests/turn-settlement.test.ts, integration_tests/quota-exhaustion-retry.test.ts, turn-crash-commit.test.ts.

Message group boundaries

Agent events group into chat-history entries at tool-result boundaries: each agent_tool_result sets needsNewMessageGroup so the next agent_assistant starts a fresh group, keeping groups 1:1 with message bubbles on reload. Key file: agent-listeners.ts.

Chat transcript content MUST be persisted, not just emitted

Anything that renders inline in the chat transcript — a message bubble or a card (MessageList.tsx) — must be written to persisted chat history, not merely emitted over WS. runner.emitMessage() is transport only: it broadcasts to attached viewers and buffers into the per-turn turn-event log, which a WS reconnect replays. It persists nothing. A session switch or a full page reload rehydrates from persisted chat history (ChatHistoryManager → GET /history), so an emit-only card renders live, survives a reconnect, and then vanishes. This bug class has recurred (voice notes docs/163, bug-report cards docs/164).

The dividing line: transient signals (spinners, preview_status, queue counts, live activity) are emit-only and correctly disappear. Transcript content (any card the user expects to still be there tomorrow, and any terminal state like "filed"/"failed") must persist. If it has a place in the scrollback, it has a row in the DB.

For a side-channel card (one arriving outside the agent-event stream, so buildTurnMessages won't capture it): emit via emitChatCard (chat-card-persistence.ts), never bare emitMessage. It atomically emits, records the card in-band (anchored by afterGroupIndex so it interleaves at its true position), and persists — and it decides, on runner.running, whether the card rides the in-progress turn or is appended as an already-final row. That branch is exactly why you must never hand-roll recordChatCard + persistTurnInProgress at a call site that can run post-turn (a backgrounded shipit agent run, a user-filed bug report): reviving a finished turn as in_progress=1 gets the whole set deleted by the next turn's replaceInProgress, which silently destroyed an 18-minute cross-agent review (docs/236).

Adding a card then means: a typed PersistedMessage field (+ column, toRow/fromRow, database.ts migration); rehydration in loadSessionHistory; registration in CARD_MESSAGE_FIELDS (visual-elements.ts) if it renders on an empty-text message; history round-trip + no-duplicate-on-replay tests; and extending EVERY_OPTIONAL_FIELD_MESSAGE. Two guard tests (chat-history.test.ts, visual-elements.test.ts) make this self-enforcing — a forgotten step is a red build naming the field. Full recipe: docs/188-persist-transcript-cards, docs/191-card-persist-on-emit.

Every transcript message carries its owning sessionId, and the client drops the foreign ones. The browser holds exactly ONE transcript in memory (the active session's messages array), while the per-session WS is keyed off the route (urlSessionId) and handlers write through the store (useSessionStore.sessionId) — those agree in the steady state but not across every switch/fork/claim transition, so an unscoped card lands in whichever session happens to be active. So: put sessionId on the WS type (a card without one cannot be filtered), and add the type to TRANSCRIPT_SCOPED_MESSAGES in client/hooks/message-handlers/index.ts — dispatchMessage drops it on a mismatch. Dropping is safe because the card is persisted in its owner's history (switching there rehydrates it). Messages that legitimately describe other sessions (session_status, pr_lifecycle_update, reset_eligible, usage_update, session_forked) stay out of that set.

Preview routing

Reverse proxy (preview-proxy.ts): subdomain routing {sessionId}--{port}.<host> is the only mode — the path-based /preview/:sessionId/:port/* route was deleted in docs/175 (absolute asset paths 404 without HTML rewriting), and preview.url's /preview/… string survives solely as the container-mode sentinel the client tests with startsWith. The proxy matches {uuid}--{port}.anything (the hostname regex in preview-proxy.ts), so it is domain-agnostic; HMR URLs are patched to the iframe's own origin so hot-reload survives the proxy. Reachability is never probed — an iframe loads its URL once, so a request that arrives before the dev server listens is retried inside the proxy (bodyless GET/HEAD only, target re-resolved per attempt) and, past that window, answered with a self-refreshing connecting page; the client has no gate of its own (docs/286). Consequence: previews need a host that can carry a wildcard subdomain, so the client refuses to build a URL for a non-loopback IP literal (usePreviewSlot.ts, the subdomain-URL builder) and renders an explanatory empty state instead — 127.x/::1 are exempt, normalized to localhost one line earlier. Full detail: docs/009-preview-system, docs/175-preview-subdomain-only; local/Tailscale access: docs/254-local-bind-and-tailnet-access.

Ownership of a shared git tree, and how to read a permission error

A git tree ShipIt owns rather than a session — the bare caches under repo-cache/, the catalog caches under marketplace-cache/ — is owned by the orchestrator's own identity, uniformly, and no session-derived identity ever appears inside it (orchestrator/shared-tree-ownership.ts, docs/272-shared-cache-ownership). Two rules follow, and both were violated in production simultaneously. No recursive chown may descend into a tree that shares inodes with a shared cache: clone --local hardlinks .git/objects, an inode has exactly one owner across every link, so chownRecursive on the clone side is a chown inside the cache — use the object-aware handbacks (handWorkspaceBackToWorker, handPluginCheckoutToWorker), which chown object directories and never object data files. And resolveGitTreeUid's "root-owned tree ⇒ no drop" clearance is a repaired condition, not an observation — a foreign-owned cache is a defect to fix at the operation, never a state to accommodate, because a mixed tree has no uid that can safely write all of it.

A root process receiving EACCES/EPERM reads as impossible, and that has twice sent the reader down the wrong path for days. Read it FIRST as "the process dropped uid (docs/266), and the tree is not uniformly owned." Both halves are load-bearing: since docs/266 orchestrator-side git runs as the uid that owns the tree, so "we are root" stopped implying "we are root when it counts"; and the drop is resolved from ONE stat of the top level, so a tree whose insides disagree with its root produces an error that checking the process's own privileges can never explain.

Compose service status is event-driven — never per-session polling

Service state is reconciled from the one host-wide Docker event subscription (container-health.ts), routed to the owning session by its com.docker.compose.project label (service-event-router.ts), plus one host-wide Engine API listing every 5 minutes for all sessions together. A per-session timer is what this replaced: ServicePoller ran docker compose ps (3 Go processes) plus a docker inspect per container every 5 s per session, which on prod was ~18 process spawns/s and ~85% of the orchestrator's CPU — paid in full by sessions idle for hours. So: no new per-session interval, and no new spawned Compose query on a status path. Use the Engine API (compose-container-listing.ts); pollIntervalMs exists for tests and defaults to 0.

Two consequences are easy to get wrong. A rule needing a second observation after a delay — the 30 s missing-container and docker-unreachable graces, the install-gate watchdog — no longer gets one for free: it arms one follow-up at its own deadline (ServicePoller.armFollowUp / requestFollowUp), self-cancelling when the condition clears, so a healthy session holds no timer. And an exit must come from the event, not a listing: /containers/json carries no exit code, and a container with a restart policy is replaced before any listing could observe its exit — applyServiceExit takes the die event's code and the OOM flag correlated from the preceding oom event. Detail: docs/332-event-driven-service-status.

Disk cleanup

Each surface prunes where the leak happens, sorted by what clock the leak grows on (planning#198): per-session teardown drops named volumes (archive/fullReset only, never idle/restart); deploy.sh owns build-time image prunes; startup-janitor.ts runs boot-only for crash-recovery leftovers; steady-state-reclaim.ts runs on the disk-tier escalation pass (escalateDiskTiers) for what grows with the clock. That pass fires from three triggers — startup, per session activation (the primary one), and an hourly activity-independent backstop — all guarded by a single in-flight flag, and it is the single steady-state entry point. Detail: those files' docstrings, docs/183-overlay-dep-store, session-containers.

Client communication & stores

Two browser channels: per-session WebSocket (/ws/sessions/{id}) and global SSE (/api/events). Stores cross-reference via useXStore.getState() (not subscriptions — avoids cycles); resets are centralized in stores/actions/session-actions.ts; hydration is HTTP bootstrap → WS loadSessionHistory() → live WS, guarded by sessionId against stale messages. Detail: client-architecture.

Integration test patterns

TestClient buffers WS messages from connect (no send-before-listen races); isTestMode in buildApp() enables POST /api/_test/sessions (no Docker); fakes expose injection methods. Detail: testing-and-quality.

Code comments

• Default to no comment. Add one only when removing it would hide a constraint, a non-obvious reason, or a failure mode needed to change the code safely. Prefer clear names and simple code.
• Keep it short. Usually one sentence; use more only when needed for correctness. Put detailed rationale in the relevant feature doc and link to it.
• Do not narrate the code. Omit comments that repeat names, types, branches, test titles, or assertions. Avoid section banners, change histories, and routine file/function summaries.
• Keep essential information. Preserve license notices, tool directives, required API documentation, and explanations of subtle invariants or workarounds. Keep directive reasons concise.
• Read before trimming. Read each file and its relevant context. Remove redundant comments; shorten useful ones. Do not bulk-strip comments or target a deletion percentage.
• Review comments before finishing. Check every added or changed comment against these rules. Remove stale text and incomplete fragments; do not restore verbosity removed by earlier cleanup.

Workflow

• Read before coding — before changing a feature, read its docs/NNN-feature/requirements.md (if present) and plan.md, plus the source files listed under "Key files". Trace the data flow for similar features to understand existing patterns. A new feature starts at requirements.md, not at plan.md — see Every new feature is under requirements discipline.
• Verify an inherited guarantee at the source; a doc describing one is a claim, not a contract. When your design leans on a neighbouring mechanism ("the retry supervisor covers restart", "that lease prevents concurrent entry"), read the code that would have to hold for it — and likewise read a PR's CI, the code on current main, and a vendor's own terms rather than recall them. This repo has repeatedly shipped plans asserting guarantees the code did not provide — docs/196 claimed a failed delivery retried "on the next poll" when the only retry was at bootstrap; planning#257's write-up ruled out a re-narrowing that planning#261 then did by accident days later. Both read as settled fact. State such dependencies as "verified at file.ts:fn", not as inheritance.
• Requirements are usually stated at the UX level — don't promote your mechanism into one. "Ship several PRs in a row without shepherding each merge" is the whole requirement; it implies nothing about unattended turns or a new subsystem. Build the smallest mechanism that produces the stated experience, and keep what was actually asked for separate from what you inferred (docs/239's "Requirement provenance" section is the pattern). A feature that starts needing platform primitives to support it is evidence the shape is wrong, not a work estimate.
• Adversarial review hardens a design; it does not simplify it. A reviewer asked "what's wrong with this" will answer that question and never "this shouldn't exist" — so rounds of review reliably add mechanism and never remove it. Between rounds, ask the opposite explicitly: for each element, would anyone notice if it were removed? Run at least one review with that brief before implementing anything sizeable; on docs/239 it cut roughly a third of the design after five ordinary rounds had only grown it.
• Identify all touchpoints — plan which files need changes (server, client, types, tests) before writing code, and which existing flows, controls and meanings of shared terms (such as "done") the change reaches.
• In a security design, write for each boundary what it must stop. Secrets in the session guard against the agent's accidental reach; other sessions and the host are blocked fully. Never write that a root-owned or unmounted file "stays hidden": a hostile agent in the session can still serve it. Compare options on the isolation gaps they leave, not on the size of the change.
• When you find a defect, fix it or offer a fix session; do not park it in an issue. A defect tied to your task, or of the same kind right beside your fix, is fixed in the same change unless it is large. A separate defect gets an offer of a fix session. File a bare issue only when a fix is not possible or the user asks for a report.
• Co-locate tests — place tests next to source files (foo.ts → foo.test.ts). Follow patterns from neighboring test files.
• Lint and typecheck before finishing — run npm run lint:dev and npm run typecheck after code changes and fix any errors before considering work complete. lint:dev is the dev-loop default; CI runs the full npm run lint so the source of truth is unchanged.
• Run the reviewer on every PR you open, docs-only ones too, unless the user says to skip it. Apply every reasonable finding yourself; ask only about a finding that is disputed or changes a requirement. After the fixes, review again, until no important finding remains. Run it via shipit agent run --role reviewer --prompt-file -. Name the role, never a backend (docs/261): ShipIt owns which reviewer runs and picks the one furthest from what you are running — a different model family first, then a different model, then a different harness — so working out who shares your blind spots is no longer your job, and it stops being wrong the moment the implementer changes. Use that, not an in-process Agent subagent — the Claude CLI's built-in "don't call the AgentTool unless asked" is baked into its binary and can't be turned off here, but a brokered out-of-process run isn't what it means. Two things the prompt must carry: review only, do not edit (it shares this workspace and its writes get auto-committed under your session), and git diff main...HEAD plus git diff / git status --short, since this turn's work isn't committed yet. Don't block on it — a real review outlives a foreground command, so launch it detached and collect with shipit agent result <RUN-ID> --wait. If no reviewer can run — Multi-agent sessions is off, or neither configured reviewer has a credential with quota left — the command says so; say so too, in the PR and in your reply, and do not call the work reviewed.
• Update docs when done — update the relevant plan.md with new subsystems, patterns, or key files you added. Mark completed checklist items with [x].
• Update shipit-docs when changing agent-facing behavior — when changing platform behavior visible to the agent inside session containers (preview config, shipit.yaml schema, container environment, GitHub integration), update the corresponding file in src/server/shipit-docs/. These docs are baked into the session worker image at /shipit-docs/ and are the agent's primary reference for the platform.
• Update the wiki when changing user-facing behavior — src/server/shipit-docs/wiki/ is the only material shipped into a session that describes ShipIt as a product, and it is what the agent answers "can ShipIt do X?" from (docs/305-shipit-capability-wiki). A new panel or control, a renamed one, a changed default, a capability gained or removed: the wiki page changes in the same PR. Two rules when you write there. The reader is an agent and the subject is the user — if ShipIt gives the agent a way to do the thing, the page tells the agent to do it, and never hands the user a command to type; where the act is genuinely the user's, it names the control and the panel. And nothing with a live query gets written down: settings values, running services, configured roles and trackers are shipit settings list / service list / agent roles / issue list, because a value copied into a page is a value that goes stale.

Code style

• ESM throughout — "type": "module" in package.json. Use .js extensions in relative imports (e.g., import { foo } from "./bar.js").
• Type imports — use import type { X } from "./path.js" for type-only imports.
• Node built-ins — use node: prefix (e.g., import fs from "node:fs").
• Naming — classes: PascalCase, functions: camelCase, events/WS message types: snake_case, constants: UPPER_SNAKE_CASE.
• React — functional components only, hooks for all state/effects. React 19 JSX transform (no import React needed).
• Icons — use @phosphor-icons/react for all icons. Never hardcode <svg> elements for one; the single exception is a bespoke illustration in an empty/onboarding state that depicts a relationship no glyph carries, which the design-language skill gates on named conditions. Use the ICON_SIZE constants from src/client/design-tokens.ts (XS=12, SM=16, MD=20, LG=32, XL=48) for icon sizes. See the design-language skill for full icon and styling guidance.
• Styling — Tailwind CSS v4 utilities over the semantic color tokens; concrete values live in per-theme CSS under src/client/themes/. Never hardcode a palette value — ShipIt is multi-theme, not dark-mode-only. See the design-language skill.
• Strict TypeScript — strict: true in tsconfig. Target ES2022, module ESNext with bundler resolution.

Prompts

LLM prompts (agent system instructions, voice cleanup, session naming) are content, not logic: prompt text lives in .md files loaded at module top level; composition stays in TypeScript. The prompt-cache contract is load-bearing — every variant renders once at module load into a frozen constant, so the per-turn path is a pure lookup and the CLI string stays byte-stable. Never move composition or the .md reads onto a per-call path. Load the prompt-architecture skill before editing prompts or their tests.

Dependency policy

Two rules, both enforced by npm run check-deps (scripts/check-dependency-age.ts):

1. Pin to exact versions — "react": "19.2.4", never ^/~/latest/a range/a tag/a git URL. Floating ranges let a fresh checkout silently pick up a version nobody has run; bumps are deliberate edits, not a side effect of re-running install.
2. Minimum age of 7 days since npm publication — the window lets scanners and the registry's abuse pipeline catch a compromised release before it reaches our build. If you genuinely need a younger release (a security fix in a transitive, a CLI bump the user asked for by version), the escape hatch is an entry in .dependency-age-allowlist.json — manifest + package + version + a reason + an expires date — never a lowered MIN_AGE_DAYS. Get explicit sign-off from the user before adding one; don't bypass silently. A waiver covers one exact version, so it can't carry into a bump nobody weighed (moving the pin back to the waived version before expires does re-apply it — same artifact, same approved window); expires is capped at 90 days; and it suppresses too-new only — a floating range or an unresolvable version stays a hard failure. Note the gate reads dependencies, devDependencies, and optionalDependencies; a transitive pinned via overrides is governed by rule 3's audit check, not by this cooldown.

Both rules cover both manifests — the root package.json and docker/agent-cli/package.json, the agent CLIs baked into the session-worker image. The list is POLICY_MANIFESTS in the script; a manifest absent from it is unchecked, so add one there the day it appears. Renovate's minimumReleaseAge is a scheduling convenience on top of this, not the gate: it is configured per rule, so a package no rule matches gets no cooldown at all, which is how an opencode-ai bump published that same morning once passed CI green.

To bump: edit the manifest to the exact version, npm install to refresh the lockfile, then npm run check-deps before opening the PR.

Container image pins (docker/Dockerfile.*) are outside check-deps, and no cooldown enforces their age today. Renovate's Docker datasource carries a release timestamp for Docker Hub only — ghcr.io is upstream renovatebot/renovate#39064 — so minimumReleaseAge has nothing to evaluate there, and minimumReleaseAgeBehaviour: timestamp-optional lets the candidate through with no age delay. timestamp-required is worse, not stricter: while the timestamp is missing it holds every candidate pending, so no branch and no PR is raised at all and the pin rots with only the Dependency Dashboard to show it. The human on automerge: false is the gate.

So when a bot PR proposes a container version younger than 7 days, retarget the PR at the newest release that is both past the 7-day window and newer than the current pin — do not hold it. Holding can rotate the candidate instead of converging on one (#2926: the proposal moved on twice while it was held). If no aged release is newer than the pin, defer; never retarget at the pinned version or below.

The other direction — a pin that is too OLD — is npm run check-image-vulns (CI job "Image Vulnerability Scan", on every PR). It Grype-scans each digest-pinned image in docker/Dockerfile.* and fails on a HIGH/CRITICAL whose fix has been available for 30+ days; newer fixes are only listed. When it fails, bump that image's digest under the rules above — for node, by hand, with its BASE_IMAGE_DIGEST (Renovate does not manage it).

Two things to get right when retargeting. The digest decides what Docker pulls — in image:tag@sha256:… the tag is cosmetic, so a stale digest ships the old image under a new tag and nothing fails. Read the digest of the exact tag you are pinning, keep the image variant and platform coverage the bot's candidate had, and say in the PR where the digest came from; where ghcr.io is outside the session's egress allowlist, GitHub's package-versions page (https://github.com/<owner>/<repo>/pkgs/container/<image>/versions) is a usable fallback once you confirm its mapping against a digest Renovate itself pinned. The edit makes the branch yours: Renovate stops updating a branch that carries a non-bot commit, and asking it to rebase afterwards discards the retarget and restores the newer candidate. Append your own commit, merge main in by hand if it conflicts, re-run the checks, and re-read the final tag and digest before merging.

A third rule is enforced separately by npm run check-audit (scripts/check-audit.ts):

3. No advisory at high or above may affect the dependency tree unless it is allowlisted in .audit-allowlist.json with a reason and an expiry date. The allowlist — not a lowered threshold — is the escape hatch for an advisory with no fix available yet; an expired entry fails the build on purpose, as the reminder to revisit it.

Transitive vulnerabilities are fixed in the overrides block, and nothing updates that block for you. Dependabot manages dependencies / devDependencies only, and an override is absolute, so npm audit fix and npm update cannot move a pin either. A pin left alone becomes the vulnerable version it was added to escape, and then blocks every automatic fix — that is exactly how the block silently went stale for roughly three months before check-audit existed. When check-audit names a transitive package, raise its pin (or add one) in overrides, then run npm install.

Releasing

ShipIt's own repo uses the release-branch mechanism (shipit.yaml release: block, docs/214): releases are merge-triggered. Cut one by opening a version-bump PR into stable and merging it — .github/workflows/release.yml then derives the tag v<package.json version> from the merged commit, gates on a green build, and creates + pushes the tag and Release itself. Never hand-push a final vX.Y.Z tag — CI owns that. rc's are the exception: cut via the tag path (push vX.Y.Z-rc.N), never by merging into stable. Use the shipit release command (plan to propose, prepare to open/update the PR, --prerelease --confirm for an rc), not a hand-rolled bump + gh pr create. The stable channel follows the latest final tag reachable from origin/stable (not the branch tip), failing closed if none exists. Full ritual: RELEASING.md; agent-facing copy: src/server/shipit-docs/release.md + prompts/releases.md.

Docs structure

Feature docs live in docs/NNN-feature-name/ (requirements.md, plan.md, checklist.md). Docs are reference material — what a feature is, why, and how, including planned-but-unimplemented designs; work tracking lives in the issue tracker. Read a feature's plan.md before changing it, and its checklist.md for remaining work. Load the docs-navigator skill for the folder layout, frontmatter schema, issue: pointer syntax, and prototype conventions.

The scan surfaces every .md file in the workspace, not just docs/ — so treat any markdown you write as user-visible.

A docs/NNN number is not a unique identifier — dozens of numbers are shared by two or more folders. So a citation that points into a document — req N, E N, a section — MUST name the folder: docs/266-plugin-service-ports req 2, never docs/266 req 2. A bare docs/NNN as provenance is fine.

Every new feature is under requirements discipline

If the work warrants a docs/NNN-* folder, that folder gets a requirements.md, written before plan.md. Materially reworking an older feature that has none starts by writing them. The shape:

# Feature name

1. A numbered, plain-language statement of what the feature must do.
2. Another observable requirement — what, never how.

## Open questions

- A decision the human has not made yet.

## Resolved questions

- YYYY-MM-DD — the question, the human's answer, and any constraint it carries.

The numbers are the IDs: keep them stable, append rather than renumber, and cite them as docs/241-spec-discipline req 3.

At the start of each turn that touches feature work, identify the one active feature — from the session's issue or feature context, or the one the user named — and check its folder for requirements.md. Bullets under ## Open questions block implementation code whether or not you raised them; resolve them at step 3 below. Only the active feature is gated: don't scan unrelated features for blockers.

That means, in order:

1. requirements.md first, from what the human actually said. Numbered, plain-language, observable statements of what the feature must do — never how. Anything you had to supply yourself is not a requirement, and neither is a "there is no X" line the human did not say: it goes under ## Open questions.
2. Ask, don't assume. Batch the open questions into one structured question with concrete options and a recommendation. Do not write implementation code while any bullet remains under ## Open questions; requirements and design work may continue.
3. Record the answer where it happened. A human answer adds/edits the numbered requirement and leaves a dated receipt under ## Resolved questions, with the open-question bullet removed in the same change — receipt, removal, and requirement change all in one diff. An agent inference never clears an open question.
4. Then design. plan.md implements requirements.md and cites requirements as (req 3); it opens with a link to the requirements doc. Later human input lands in requirements.md first — editing plan.md from human input while the requirements stay unchanged makes the design a second, hidden source of requirements.
5. Independent check before you call it done. ShipIt's configured reviewer (shipit agent run --role reviewer --prompt-file -, per Run the reviewer on every PR) compares the branch diff against every numbered requirement, told to review only and not edit. That is a brokered out-of-process agent, not an in-process Agent subagent, so a harness rule about spawning subagents does not govern it. Your own final pass doesn't count, and neither does a subagent under your own model.

Nothing enforces this mechanically — the pull-request diff is the enforcement, so a skipped question or a self-promoted requirement is visible to review (mechanical enforcement is planning#275). docs/241-spec-discipline/ is the worked example: read its requirements.md alongside its plan.md for the shape.

Not every change is a feature. Bug fixes, refactors, and chores that don't get a docs folder don't get a requirements doc either — the rule attaches to the folder, not to the commit.

Keep the tracker in sync when you touch a design doc

Creating or materially updating a docs/NNN-* doc syncs its tracker item in the same turn, via the tracker-neutral shipit issue command — never gh issue, gh api, or a tracker's own MCP.

• Doc has an issue: pointer: comment on it — shipit issue comment <pointer> --body-file -. The pointer identifies its own tracker; pass it verbatim. Comment; don't open a second item.
• Doc has no pointer: create one and cross-link, same turn (docs/187) — shipit issue create --tracker planning --label <name> --title "<doc title>" --body-file -. --tracker is required so a forgotten flag can't file into this public repo's own issues (docs/248), and always set a label matching intent. Write the printed identifier back into the doc's issue: frontmatter as planning#NNN. Creation is do-then-surface — a provenance card with Undo is posted automatically, so don't propose-and-wait. If the tracker isn't connected, surface that rather than filing elsewhere.

Where does a fact live — checklist.md, plan.md, or the tracker issue? Never mirror; duplicated sources drift. One mechanical test: "Would this fact require a commit to change?" Changed because the code changed → committed file. Changed for planning/coordination (priority, status, ownership, scheduling, cross-issue relations, discussion) → the tracker. Does it belong to the diff, or to the conversation about the work?

When you do work you comment on the issue; you do not commit a status update. A committed doc MAY name sibling issue IDs inline as stable identifiers ("blocked on planning#81") but MUST NOT record their priority/status — no sibling-status tables. A design doc may carry the author's analyzed ordering as settled narrative ("sequenced first"); live priority stays in the tracker, and the two are deliberately not reconciled. Quick test: if a checklist item could be copied verbatim into the issue as a sub-task, it's in the wrong place.