Wave 1 — stop the bleeding: three CRITICALs, the cost cap, and ADR-0073/0074

.claude/agents/relavium-reviewer.md

@@ -41,33 +41,45 @@ finding, and you list what you verified as clean.
    in Node, the Tauri WebView, and the extension host.
    - Signal: `grep -rn "node:\|from 'fs'\|from 'path'\|@tauri-apps\|\bwindow\.\|\bdocument\." packages/core/src`
 
-4. **No new dependency without an ADR.** A new runtime dependency — especially in
-   `packages/core`/`packages/llm` or a new provider SDK — needs an
+4. **`packages/mcp`'s own fences hold.** The inbound MCP client is the one package besides
+   `apps/cli` that imports `@relavium/core`, and the only one importing the MCP SDK. The SDK
+   stays behind `packages/mcp`'s adapters; a server-supplied tool description or schema is
+   untrusted input (bounded, and never admitted verbatim into a prompt); every network
+   transport keeps its SSRF pre-connect floor and its connect timeout.
+   - Signal: `grep -rn "@modelcontextprotocol/sdk" packages apps | grep -v 'packages/mcp/src/'` — any hit outside is a boundary break.
+   - Source: [ADR-0052](../../docs/decisions/0052-inbound-mcp-client-package-lifecycle-registration.md), [ADR-0053](../../docs/decisions/0053-mcp-network-transport-egress-security.md)
+
+5. **No new dependency without an ADR.** A new runtime dependency — especially in
+   `packages/core`/`packages/llm`/`packages/mcp` or a new provider SDK — needs an
    [ADR](../../docs/decisions/README.md). Reject a casual install that adds or re-introduces
    a banned framework (Vercel AI SDK, LangChain, a Python sidecar).
    - Signal: inspect the `package.json` / `pnpm-lock.yaml` diff.
    - Source: [code-review.md](../../docs/standards/code-review.md)
 
-5. **Secrets never in plaintext, logs, or the frontend.** Keys live only in the OS keychain;
+6. **Secrets never in plaintext, logs, or the frontend.** Keys live only in the OS keychain;
    none in an IPC payload to the WebView, a Zustand store, a React prop, localStorage, a log,
-   an unencrypted DB column, or an error/`node:failed`/`run:failed` event. On the **desktop**
-   the WebView adapter holds only a key *reference*; the raw key is read and attached inside
-   the Rust `llm_stream` command and never crosses into the WebView (ADR-0018).
+   an unencrypted DB column, or an error/`node:failed`/`run:failed` event. On the **CLI** — the
+   surface that actually ships today — a key is read from stdin, never argv; `history.db` and
+   `config.toml` stay `0600`; a tool-approval preview, a persisted run summary, and any `--json`
+   payload go through the secret-shaped redaction helper before they are written. On the
+   **desktop** (not yet built) the WebView adapter holds only a key *reference*; the raw key is
+   read and attached inside the Rust `llm_stream` command and never crosses into the WebView
+   (ADR-0018).
    - Signal: `grep -rni "apikey\|api_key\|secret\|process.env.*KEY" $changed_files` then trace each hit.
    - Source: [security-review.md](../../docs/standards/security-review.md), [ADR-0006](../../docs/decisions/0006-os-keychain-for-api-keys.md), [ADR-0018](../../docs/decisions/0018-desktop-execution-and-rust-egress.md)
 
-6. **One canonical home for specs.** A change to a schema (workflow/agent YAML, SSE/run
+7. **One canonical home for specs.** A change to a schema (workflow/agent YAML, SSE/run
    events, IPC, DB DDL, node types, tools, routes) updates its one
    [docs/reference/](../../docs/reference/) file — no pasted copy elsewhere. Run-event names
    are the canonical colon-namespaced form with `sequenceNumber`.
    - Signal: `grep -rn "node\.\(started\|completed\)\|agent\.token\|\bseqNo\b\|node:error\|run:error\|human_gate:pending" $changed_files` — legacy dotted names, `seqNo`, and the non-canonical `node:error`/`run:error`/`human_gate:pending` are all wrong (canonical: `node:failed`/`run:failed`/`human_gate:paused`, field `sequenceNumber`).
 
-7. **Desktop is an agent-management center, not an IDE.** A change under `apps/desktop`
+8. **Desktop is an agent-management center, not an IDE.** A change under `apps/desktop`
    adding a code editor, file browser, or terminal is out of scope; code-adjacent work
    belongs to the VS Code extension.
    - Source: ADR-0007, [architectural-principles.md](../../docs/standards/architectural-principles.md) §4
 
-8. **Conventional Commits.** `<type>(<scope>): <summary>`, imperative, scope per package
+9. **Conventional Commits.** `<type>(<scope>): <summary>`, imperative, scope per package
    (`llm`, `core`, `shared`, `db`, `ui`, `cli`, `desktop`, `vscode`, `api`, `portal`,
    `docs`, `repo`), `Refs: ADR-XXXX` when implementing a decision.
    - Source: [commit-style.md](../../docs/standards/commit-style.md)

.claude/skills/add-package/SKILL.md

@@ -35,7 +35,8 @@ Create a new `packages/<x>` or `apps/<x>` workspace that is born consistent with
 3. **Create the directory and the source/test skeleton.**
    ```bash
    PKG=run-history   # your slug
-   mkdir -p /Users/dev/Documents/Projects/Agent-Organizer/packages/$PKG/src
+   R=$(git rev-parse --show-toplevel)
+   mkdir -p "$R/packages/$PKG/src"
    ```
    Resulting tree for a shared package:
    ```text

.claude/skills/security-review/SKILL.md

@@ -76,6 +76,11 @@ encryption (SQLCipher) path, or a new third-party dependency. When in doubt, run
    user explicitly opted into a local endpoint. An agent-config URL must never make the
    engine call an internal address with a real key attached. Confirm TLS verification is not
    disabled and every outbound call carries an `AbortSignal` + timeout.
+   **Confirm the change REUSES the shared guard rather than hand-rolling a second one** — a
+   parallel range check is how the two drift and one of them silently stops covering a range
+   (CLAUDE.md rule 3: never reinvent a security-critical primitive).
+   - Signal: `grep -rn "isPrivateOrLocalHost\|safe-egress" $changed_files` — a new URL path with
+     no hit is either reusing nothing or re-deriving the check locally.
 
 5. **The `run_command` sandbox.** `run_command` spawns model-driven shell execution, so it
    runs sandboxed: only commands on the workflow's `allowedCommands` allowlist execute (never

.claude/skills/start-task/SKILL.md

@@ -28,7 +28,8 @@ Convert a roadmap phase workstream into a tightly scoped plan an engineer (human
 ## Workflow
 1. **Locate the workstream.** Read `docs/roadmap/current.md` for what is active, then open the phase file and find the workstream by id. Copy its Tasks + Acceptance as the starting point.
    ```bash
-   grep -rn "FallbackChain\|1\.K" /Users/dev/Documents/Projects/Agent-Organizer/docs/roadmap/phases/
+   R=$(git rev-parse --show-toplevel)
+   grep -rn "FallbackChain\|1\.K" "$R/docs/roadmap/phases/"
    ```
 2. **Check dependencies and build order.** Confirm the workstreams it depends on (per the phase's Mermaid graph) are done, and that you are not building a surface before its engine (architectural-principles §1, engine-first: `shared → llm → core → cli → desktop → vscode`).
 3. **Define scope explicitly — in and out.** Write a short scope block:

.claude/skills/write-architecture-doc/SKILL.md

@@ -40,7 +40,8 @@ Produce or update a `docs/architecture/*.md` doc that answers **"how is this bui
 5. **Explain the diagram in prose**, then the flow and the boundaries. Reinforce the invariants the topology depends on: engine zero-platform-imports, no vendor type across the `@relavium/llm` seam, secrets staying engine-side, the canonical `RunEvent` union.
 6. **Cite specs — never restate them.** Every concrete shape is a relative link to its `reference/` home (e.g. `[run-event schema](../reference/contracts/sse-event-schema.md)`, `[node types](../reference/shared-core/node-types.md)`, `[LLM-provider seam](../reference/shared-core/llm-provider-seam.md)`). If you catch yourself pasting a schema or event body, replace it with a link.
    ```bash
-   ls /Users/dev/Documents/Projects/Agent-Organizer/docs/reference/contracts/ /Users/dev/Documents/Projects/Agent-Organizer/docs/reference/shared-core/
+   R=$(git rev-parse --show-toplevel)
+   ls "$R/docs/reference/contracts/" "$R/docs/reference/shared-core/"
    ```
 7. **Link decisions and mark phases.** Cite the relevant ADRs for *why*; mark any Phase-2 (cloud) behavior explicitly with a bold marker or blockquote so it is never mistaken for shipped Phase-1 (documentation-style §9).
 8. **Checkpoint — run ../standards-check/SKILL.md docs checks.** Confirm: one H1, no front-matter, diagram-first, relative links resolve, **no duplicated spec body**, ISO dates, Phase-2 marked. Then commit with ../commit-and-pr/SKILL.md (`docs(architecture): …`).

CLAUDE.md

@@ -26,6 +26,7 @@ It is a **Turborepo + pnpm monorepo**:
 | `packages/llm` (`@relavium/llm`) | Relavium's **own** multi-LLM abstraction: the `LLMProvider` seam + thin hand-rolled adapters over the official provider SDKs. No Vercel AI SDK, no LangChain. |
 | `packages/core` (`@relavium/core`) | **The engine** — YAML→DAG parse, runner, checkpoint/resume, retry. **Zero platform-specific imports.** The most important package. |
 | `packages/db` (`@relavium/db`) | Drizzle schema + migrations — same schema for SQLite (local) and Postgres (cloud). |
+| `packages/mcp` (`@relavium/mcp`) | The inbound MCP client — the SDK-fenced package, the dependency-free JSON-Schema→Zod compiler, and the `http`/`sse`/`websocket` transports behind the SSRF floor. |
 | `packages/ui` (`@relavium/ui`) | Shared React components: ReactFlow node types + shadcn/ui. |
 | `apps/desktop` | Tauri v2 desktop app — the agent-management center (canvas, run monitoring). |
 | `apps/cli` | Terminal CLI (`commander.js` + `ink`). The engine's first real consumer + regression harness. |

apps/cli/src/chat/chat-mode-host.test.ts

@@ -2,7 +2,12 @@ import { mkdtemp, realpath, rm } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 
-import { BUILTIN_TOOLS, type SessionTurnPolicy } from '@relavium/core';
+import {
+  BUILTIN_TOOLS,
+  type SessionTurnPolicy,
+  type ToolActionPreview,
+  type ToolApprovalRequest,
+} from '@relavium/core';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { applyChatMode, makeChatModeEnv } from './chat-mode-host.js';
@@ -59,14 +64,41 @@ describe('makeChatModeEnv + applyChatMode', () => {
     expect(env.cache.isAlways('write_file')).toBe(true); // the cache lives on the env, not per-policy
   });
 
-  it('isProtectedTarget resolves a preview path against the workspace and matches a protected target', () => {
+  /** A minimal approval request: the classifier reads `target`, never `preview` (#91). */
+  const req = (
+    unredactedPreview: ToolActionPreview,
+    preview: ToolActionPreview = {},
+  ): ToolApprovalRequest => ({
+    toolId: 'write_file',
+    action: 'fs_write',
+    preview,
+    unredactedPreview,
+  });
+
+  it('isProtectedTarget resolves the TARGET path against the workspace and matches a protected target', () => {
+    const { session } = fakeSession();
+    const env = makeChatModeEnv({ session, tools: BUILTIN_TOOLS, workspaceDir: workspace, prompt });
+    expect(env.isProtectedTarget(req({ path: '.git/config' }))).toBe(true);
+    expect(env.isProtectedTarget(req({ path: '.ssh/authorized_keys' }))).toBe(true);
+    expect(env.isProtectedTarget(req({ path: 'notes.md' }))).toBe(false);
+    expect(env.isProtectedTarget(req({}))).toBe(false); // no path (egress/process) ⇒ never protected
+    expect(env.isProtectedTarget(req({ command: 'ls' }))).toBe(false);
+  });
+
+  it('classifies from the TARGET even when the redacted preview no longer looks protected (#91)', () => {
     const { session } = fakeSession();
     const env = makeChatModeEnv({ session, tools: BUILTIN_TOOLS, workspaceDir: workspace, prompt });
-    expect(env.isProtectedTarget({ path: '.git/config' })).toBe(true);
-    expect(env.isProtectedTarget({ path: '.ssh/authorized_keys' })).toBe(true);
-    expect(env.isProtectedTarget({ path: 'notes.md' })).toBe(false);
-    expect(env.isProtectedTarget({})).toBe(false); // no path (egress/process preview) ⇒ never protected
-    expect(env.isProtectedTarget({ command: 'ls' })).toBe(false);
+    // Exactly the collapse a whole-string scrub produces on this path: the display copy has lost its `.ssh`
+    // segment entirely. Reading `preview` here would return false and silently auto-approve; reading `target`
+    // — which is what the classifier does — still sees the real path.
+    expect(
+      env.isProtectedTarget(
+        req(
+          { path: 'Access Token Backup/.ssh/authorized_keys' },
+          { path: 'Access Token [redacted]' },
+        ),
+      ),
+    ).toBe(true);
   });
 
   it('auto uses isProtectedTarget: a protected write prompts, a normal write auto-approves', async () => {
@@ -82,14 +114,12 @@ describe('makeChatModeEnv + applyChatMode', () => {
     const confirm = policies[0]?.confirm;
     expect(confirm).toBeDefined();
     // A normal write auto-approves without prompting…
-    expect(
-      await confirm!({ toolId: 'write_file', action: 'fs_write', preview: { path: 'ok.md' } }),
-    ).toEqual({
+    expect(await confirm!(req({ path: 'ok.md' }, { path: 'ok.md' }))).toEqual({
       outcome: 'approve',
     });
     expect(promptSpy).not.toHaveBeenCalled();
     // …a protected write falls back to the prompt.
-    await confirm!({ toolId: 'write_file', action: 'fs_write', preview: { path: '.git/config' } });
+    await confirm!(req({ path: '.git/config' }, { path: '.git/config' }));
     expect(promptSpy).toHaveBeenCalledTimes(1);
   });
 });

apps/cli/src/chat/chat-mode-host.ts

@@ -1,6 +1,6 @@
 import { resolve } from 'node:path';
 
-import type { AgentSession, ToolActionPreview, ToolDef } from '@relavium/core';
+import type { AgentSession, ToolApprovalRequest, ToolDef } from '@relavium/core';
 
 import { isProtectedPath } from '../engine/tool-host/fs.js';
 import {
@@ -34,8 +34,8 @@ export interface ChatModeEnv {
   /** The REPL's interactive `[y] yes / [a] always / [n] no / [c] reason / [esc] abort` prompt (accept-edits, and
    *  auto's protected-path fallback). `[c]` opens the typed-reason capture (Step 14 — a reject carrying WHY). */
   readonly prompt: ApprovalPrompt;
-  /** Whether an approval preview targets a protected path — `auto` then falls back to a prompt (ADR-0057). */
-  readonly isProtectedTarget: (preview: ToolActionPreview) => boolean;
+  /** Whether an approval REQUEST targets a protected path — `auto` then falls back to a prompt (ADR-0057). */
+  readonly isProtectedTarget: (request: ToolApprovalRequest) => boolean;
 }
 
 export interface MakeChatModeEnvOptions {
@@ -55,11 +55,22 @@ export function makeChatModeEnv(opts: MakeChatModeEnvOptions): ChatModeEnv {
     governed: governedToolIds(opts.tools),
     cache: new ApprovalCache(),
     prompt: opts.prompt,
-    // fs_write is the only preview class with a path; egress/process have none, so they are never "protected"
+    // fs_write is the only action class with a path; egress/process have none, so they are never "protected"
     // (auto approves them directly). A relative path resolves against the session workspace — the SAME anchor
     // the fs jail uses — so the classification matches what the fs layer would enforce.
-    isProtectedTarget: (preview) =>
-      preview.path !== undefined && isProtectedPath(resolve(opts.workspaceDir, preview.path)),
+    //
+    // Reads `request.unredactedPreview`, NOT `request.preview` (#91): the preview is redacted for display,
+    // and a scrub
+    // that collapses `./x/.ssh/authorized_keys` would silently reclassify a protected path as unprotected.
+    // Precedence: the REAL target first, the display preview only as a fallback for a hand-built request
+    // that carries none (a test fixture / surface stub — the engine always sets `target`). Falling back is
+    // safe-by-default here because `previewFor` only ever LOSES information: a scrub can turn a protected
+    // path into an unprotected-looking one, never the reverse, so the fallback can under-classify but the
+    // fs floor still hard-denies the write — while the primary path cannot under-classify at all.
+    isProtectedTarget: (request) => {
+      const path = request.unredactedPreview?.path ?? request.preview.path;
+      return path !== undefined && isProtectedPath(resolve(opts.workspaceDir, path));
+    },
   };
 }
 

apps/cli/src/chat/chat-mode.test.ts

@@ -25,6 +25,8 @@ const req = (over: Partial<ToolApprovalRequest> = {}): ToolApprovalRequest => ({
   toolId: 'write_file',
   action: 'fs_write',
   preview: { path: 'notes.md' },
+  // The unredacted classification copy (#91) — the classifier's input, distinct from the display preview.
+  unredactedPreview: { path: 'notes.md' },
   ...over,
 });
 
@@ -316,17 +318,20 @@ describe('buildTurnPolicy — the mode → { advertise, confirm } mapping', () =
     );
     const policy = buildTurnPolicy('auto', {
       ...deps({ prompt }),
-      isProtectedTarget: (preview) => preview.path === '.git/config',
+      isProtectedTarget: (request) => request.unredactedPreview?.path === '.git/config',
     });
     // A normal write auto-approves…
-    expect(await policy.confirm!(req({ preview: { path: 'ok.md' } }))).toEqual({
+    expect(await policy.confirm!(req({ unredactedPreview: { path: 'ok.md' } }))).toEqual({
       outcome: 'approve',
     });
     expect(prompt).not.toHaveBeenCalled();
     // …but a protected-path write prompts, forwarding the request + cacheable:false (the REPL greys out
     // "always" here) + the cancel signal.
     const signal = new AbortController().signal;
-    const protectedReq = req({ preview: { path: '.git/config' } });
+    const protectedReq = req({
+      preview: { path: '.git/config' },
+      unredactedPreview: { path: '.git/config' }, // the classifier reads THIS copy (#91), not the display one
+    });
     const decision = await policy.confirm!(protectedReq, signal);
     expect(prompt).toHaveBeenCalledTimes(1);
     expect(prompt).toHaveBeenCalledWith(protectedReq, false, signal);

apps/cli/src/chat/chat-mode.ts

@@ -159,11 +159,15 @@ export interface TurnPolicyDeps {
   /** The session once/always memory. */
   readonly cache: ApprovalCache;
   /**
-   * Whether an approval preview targets a protected path, so `auto` falls back to a prompt rather than
+   * Whether the approval REQUEST targets a protected path, so `auto` falls back to a prompt rather than
    * auto-approving (ADR-0057). Absent ⇒ `auto` auto-approves every governed action (the fs-layer
    * protected-paths refusal is still the hard floor either way).
+   *
+   * Takes the whole request, not the preview: `request.preview` is a redacted DISPLAY projection (#91) whose
+   * scrub can turn a protected path into an unprotected-looking one, so a security classifier must read
+   * `request.unredactedPreview` — see {@link ToolApprovalRequest.unredactedPreview}.
    */
-  readonly isProtectedTarget?: (preview: ToolActionPreview) => boolean;
+  readonly isProtectedTarget?: (request: ToolApprovalRequest) => boolean;
 }
 
 /**
@@ -232,7 +236,7 @@ function confirmFor(mode: ChatMode, deps: TurnPolicyDeps): ConfirmActionHook {
         // answer is NOT cacheable: a protected-path prompt must re-ask every time, and — since the session
         // cache is shared across modes — an "always" here must not silently blanket-approve that tool id in a
         // later accept-edits turn (a cross-mode consent escalation). So the auto fallback never remembers.
-        if (deps.isProtectedTarget?.(request.preview) === true) {
+        if (deps.isProtectedTarget?.(request) === true) {
           const cacheable = false;
           const answer = await deps.prompt(request, cacheable, signal);
           return toDecision(answer, request.toolId, deps.cache, cacheable);