Phase 2.5 (CLI Consolidation) complete — 2.5.I/2.5.J close-out + Batch A–E → main

[cemililik]

Phase 2.5 (CLI Consolidation) → main — release

This brings Phase 2.5 complete from development to main (152 commits). The Phase-2.5 spine + experience + observability workstreams (2.5.A–H) landed earlier on development via their own feature PRs (#60–#67); this PR additionally contains the 2.5 close-out completed in this session — the consolidation lanes 2.5.I / 2.5.J and the doable-now Batch A–E backlog — which reaches milestone M2.5-4 and declares Phase 2.5 complete.

The close-out followed a strict per-step cadence: implement → Opus review → fix → Sonnet review → fix, with a dedicated security-review pass before the review rounds on the security-touching steps. Every step's toolchain gates were green before commit.

Close-out contents (this session)

2.5.I — regression harness + DB concurrency hardening

• loadFull wrapped in a single read transaction (torn-read guard).
• Every multi-statement write txn is BEGIN IMMEDIATE with a bounded, deterministic (no-jitter, per ADR-0040) SQLITE_BUSY/SQLITE_LOCKED retry (withBusyRetry, over the pre-existing WAL + busy_timeout). ADR-0064 §5 records the amendment.
• New e2es: concurrent chat+run (two-connection coexistence + two-process contention with a READY handshake), Home → chat → resume → export cassette-chain, and query-shape perf budgets (EXPLAIN QUERY PLAN: index-served, no filesort) + the 80×24 degrade assertion. An advisory Windows CI lane runs the DB + harness suites + a headless no-TTY smoke.

2.5.J — docs-debt + color

• Corrected the "encrypted history" wording to the accurate CLI posture (unencrypted; 0700/0600 + keychain, no credentials at rest — ADR-0050) across uvp/vision/tutorial + the README spine.
• --color/--no-color > NO_COLOR (any non-empty ⇒ off) > FORCE_COLOR (0/false ⇒ off, supports-color convention) > default-on, orthogonal to the --json/CI output mode, with a full test matrix.

Batch A–E (doable-now backlog)

• A test-hardening: media-gc flake fix + a 2.5.A git_status union pin.
• B 2.5.H TUI polish: a persistent Esc to stop abort hint during streaming + a line-bounded reasoning panel (last-N rendered rows, not raw chars).
• C fix(core): AgentParseError line/col diagnostics (the session {{ctx.*}} interpolation half deferred — governed by the Proposed ADR-0060, Phase-2.6).
• D the ADR-0057 approval/security batch (each its own diff + a security-review pass), behind an append-only ADR-0057 amendment note:
  • [c] reject-with-typed-reason at the approval prompt (both surfaces; sanitized + length-bounded; the fail-closed floor unchanged);
  • the non-TTY fail-closed approval policy consolidated to one nonInteractiveApprovalPrompt;
  • SCOPE-denial conversational recovery (a denial-taxonomy change: only the fs pure scope-tier escape + media_scope_denied become recoverable on the recoverToolFailures surfaces; user/guardrail/SSRF/confidentiality denials stay fatal);
  • the view-only Ctrl+T reasoning toggle whitelisted through the approval key-swallow;
  • the Trojan-Source bidi floor (strip U+202A–202E / U+2066–2069 / LRM/RLM/ALM at the shared display boundary; ZWJ/ZWNJ preserved).
• E the profile-aware advertise-filter + the in-house, dependency-free, ReDoS-safe .gitignore / .relaviumignore matcher for @-mention.

Deferred (tracked in deferred-tasks.md)

• The two gate.ts cross-process-resume items — relavium budget resume and secret re-provide on gate resume — are deferred to a focused follow-up (both refactor the same security-sensitive resume path; Post-M0: comprehensive-review fixes + Phase 0 completion docs #4 relaxes a fail-closed guarantee, so it's best landed together with fresh context + a security review, not at the tail of this session).
• The session {{ctx.*}} interpolation stays with the Proposed ADR-0060 (Phase-2.6).
• project-tier extraRoots stays blocked on its config source.

Acceptance gate (green)

pnpm turbo run lint typecheck test build (24/24 tasks; 1641 CLI + 985 core tests), pnpm -w run format:check, pnpm lint:fence-check, pnpm lint:engine-deps — all pass. The @relavium/llm seam is untouched; packages/core stays platform-free; the two ADR touches are append-only amendment notes (ADR-0057, ADR-0064 §5); no new runtime dependency.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added typed “why” capture when rejecting pending approvals, including a fail-closed path for non-interactive runs.
  • Improved color controls with --color/--no-color and documented NO_COLOR/FORCE_COLOR precedence.
  • Enhanced workspace/session continuity with safer resume/export and stronger terminal sanitization.
• Bug Fixes
  • Hardened tool and filesystem denial handling, including clearer recoverable scope escapes and stricter confidentiality refusals.
  • Improved read-only tool hosting by filtering write actions appropriately.
  • Reduced risk of torn reads and lock contention during concurrent SQLite activity.
• Tests
  • Added concurrency, session-chain, and query-shape performance guardrails (including an advisory Windows lane).
• Documentation
  • Updated CLI references and architecture/security notes for the above behaviors.

[sourcery-ai]

Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b3d66467-1cc2-4e18-8128-29ccf3176d69

📥 Commits

Reviewing files that changed from the base of the PR and between 3dffc8c and e08b8d0.

📒 Files selected for processing (14)

• README.md
• apps/cli/src/harness/concurrency.e2e.test.ts
• apps/cli/src/harness/perf-budget.e2e.test.ts
• apps/cli/src/render/tui/chat-ink.tsx
• apps/cli/src/render/tui/chat-projection.ts
• apps/cli/src/render/tui/gitignore.test.ts
• apps/cli/src/render/tui/gitignore.ts
• apps/cli/src/render/tui/home-controller.ts
• docs/roadmap/current.md
• docs/roadmap/deferred-tasks.md
• docs/standards/security-review.md
• docs/uvp.md
• packages/db/src/media-reference-store.ts
• packages/db/src/model-catalog-store.ts

✅ Files skipped from review due to trivial changes (4)

• docs/uvp.md
• README.md
• docs/roadmap/deferred-tasks.md
• docs/roadmap/current.md

🚧 Files skipped from review as they are similar to previous changes (7)

• apps/cli/src/render/tui/gitignore.test.ts
• packages/db/src/media-reference-store.ts
• packages/db/src/model-catalog-store.ts
• apps/cli/src/render/tui/chat-ink.tsx
• docs/standards/security-review.md
• apps/cli/src/render/tui/chat-projection.ts
• apps/cli/src/render/tui/home-controller.ts

---

📝 Walkthrough

Walkthrough

This PR closes Phase 2.5 with concurrency hardening, approval-flow changes, parser and TUI updates, read-only tool filtering, and broad documentation refreshes.

Changes

SQLite Concurrency Hardening

Layer / File(s)	Summary
Retry helper and tests packages/db/src/retry.ts, packages/db/src/retry.test.ts	Adds bounded retrying for SQLite lock errors with deterministic backoff and contention tests.
Store write transactions packages/db/src/media-reference-store.*, model-catalog-store.*, provider-store.*, run-history-store.*	Wraps write paths in immediate transactions guarded by retrying, with tests asserting transaction mode.
Session snapshot reads packages/db/src/session-store.ts, session-store.test.ts	Makes loadFull read session and messages in one transaction and adds snapshot-isolation tests.
Concurrency harness and CI apps/cli/src/harness/*, .github/workflows/ci.yml, packages/db/src/{client,media-store,media-write}.test.ts	Adds concurrency, session-chain, and query-plan e2e suites, a child writer fixture, an advisory Windows job, and retry-hardened test cleanup.
Concurrency docs docs/reference/desktop/database-schema.md, docs/decisions/0064-live-model-catalog.md, docs/reference/cli/regression-harness.md	Documents transaction and retry conventions plus the new harness suites.

Estimated code review effort: 4 (Complex) | ~75 minutes

Approval Flow and Recoverable Denials

Layer / File(s)	Summary
Non-interactive prompt helper apps/cli/src/chat/chat-mode.ts, chat.ts, agent-run.ts, chat-mode-host.ts, chat-store.ts	Adds a shared fail-closed non-interactive approval prompt and wires it into chat and agent-run paths.
Typed reject-with-reason UI apps/cli/src/render/tui/chat-input.ts, chat-ink.tsx, home-controller.ts, home-app.tsx, chat-projection.ts	Adds typed denial-reason capture, threads reason state through the Home TUI, and sanitizes submitted reasons.
Recoverable denial taxonomy apps/cli/src/engine/tool-host/{errors,fs}.ts, packages/core/src/tools/errors.ts, packages/core/src/engine/agent-turn.ts	Adds recoverable flags to tool-dispatch errors and marks media scope and filesystem escape denials recoverable where appropriate.
Approval docs docs/decisions/0057-...md, docs/reference/shared-core/{agent-runner,tool-registry}.md, docs/standards/security-review.md	Documents the approval flow and recoverable denial behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Parser, Color, Mention, and Render Updates

Layer / File(s)	Summary
Parser diagnostics packages/core/src/agent-parser.ts, agent-parser.test.ts, apps/cli/src/chat/agent-source.ts, apps/cli/src/workflows/catalog.test.ts	Adds line/column YAML syntax diagnostics and secret-free messaging for agent parsing.
Color precedence apps/cli/src/process/options.ts, options.test.ts, program.ts, run.ts, docs/reference/cli/commands.md	Implements --color precedence with NO_COLOR/FORCE_COLOR, plus matching help text and docs.
Ignore matcher apps/cli/src/render/tui/gitignore.ts, gitignore.test.ts	Implements gitignore-style parsing and matching with dedicated tests.
Mention filtering apps/cli/src/render/tui/mention.ts, mention.test.ts	Wires compiled ignore rules into mention listing and adds an integration test.
Terminal rendering hardening apps/cli/src/render/tui/chat-projection.ts, chat-projection.test.ts, chat-ink.tsx, apps/cli/src/commands/provider.ts	Strips bidi controls, bounds the reasoning panel by rendered rows, adds an abort hint, and updates related tests and TUI plumbing.

Estimated code review effort: 4 (Complex) | ~70 minutes

Read-Only Tool Filtering and Misc Tests

Layer / File(s)	Summary
Read-only tool filtering apps/cli/src/engine/tool-host/assemble.ts, assemble.test.ts	Makes advertised tool IDs respect a read-only host mode.
Misc tests apps/cli/src/chat/session-host.test.ts, apps/cli/src/engine/media-gc.test.ts	Adds a git_status process-arm test and stabilizes the media-gc orphan sweep clock.

Estimated code review effort: 2 (Simple) | ~15 minutes

Documentation and Roadmap Updates

Layer / File(s)	Summary
Encryption posture docs docs/architecture/*, docs/decisions/*, docs/tech-stack.md, docs/tutorials/..., docs/uvp.md, docs/vision.md	Clarifies CLI vs desktop history.db encryption posture and related storage wording.
Roadmap status updates CLAUDE.md, README.md, docs/roadmap/*	Marks Phase 2.5 lanes complete and updates related milestone, ADR, and deferred-task text.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ChatInk
  participant HomeController
  participant Store
  User->>ChatInk: press c during pending approval
  ChatInk->>HomeController: reject-with-reason
  HomeController-->>ChatInk: open reasonDraft
  User->>ChatInk: Enter reason
  ChatInk->>HomeController: submit sanitized reason
  HomeController->>Store: reject approval

Loading

sequenceDiagram
  participant Writer1
  participant withBusyRetry
  participant SQLiteDB
  Writer1->>SQLiteDB: BEGIN IMMEDIATE
  withBusyRetry->>SQLiteDB: BEGIN IMMEDIATE
  SQLiteDB-->>withBusyRetry: SQLITE_BUSY
  withBusyRetry->>withBusyRetry: sleep(backoff)
  Writer1->>SQLiteDB: COMMIT
  withBusyRetry->>SQLiteDB: retry fn

Loading

Possibly related PRs

• HodeTech/Relavium#17: Extends the ToolDispatchError/ToolPolicyError hierarchy with the recoverable flag consumed by the same engine recovery logic.
• HodeTech/Relavium#19: Shares the same agent-turn.ts control-flow area and recovery handling around turn execution.
• HodeTech/Relavium#63: Related to the CLI chat mode and per-tool approval flow extended here with shared non-interactive prompting and typed rejection.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the PR’s main change: Phase 2.5 CLI consolidation completion and close-out into main.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch development

---

_{Comment @coderabbitai help to get the list of available commands.}

[gemini-code-assist]

Code Review

This pull request completes Phase 2.5 (CLI Consolidation) by introducing database concurrency hardening with BEGIN IMMEDIATE and deterministic retries, adding robust E2E test suites (concurrency, session-chain, and query-shape performance budgets), implementing a custom .gitignore matcher for completions, and addressing security concerns like Trojan-Source bidi overrides. Feedback from the review highlights a bug in the newly added .gitignore directory-matching logic, where files inside ignored directories bypass the rules, and provides a detailed refactoring suggestion to resolve it. Additionally, the reviewer recommends using optional chaining on process.stdout to prevent potential runtime crashes in headless or test environments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The current implementation of globToRegExp always appends (?:/|$) to the compiled regular expression. However, for directory-only rules (e.g., node_modules/), this causes files directly under the ignored directory (like node_modules/a.txt) to bypass the ignore rule when checked individually with isDir = false because rule.dirOnly && !isDir evaluates to true and skips the rule.

To fix this, we should allow globToRegExp to accept a dirOnly parameter and append / instead of (?:/|$) when dirOnly is true. This ensures that directory-only rules only match if the matched segment is followed by a slash (which is always true for descendants of the directory, or for the directory itself if we append a trailing slash).

function globToRegExp(glob: string, anchored: boolean, dirOnly = false): RegExp {
  let src = '';
  for (let i = 0; i < glob.length; i += 1) {
    const ch = glob[i];
    if (ch === '*') {
      if (glob[i + 1] === '*') {
        i += 1; // consumed the second '*'
        if (glob[i + 1] === '/') {
          i += 1; // consumed the '/'
          src += '(?:.*/)?'; // **/ - zero or more leading directories
        } else {
          src += '.*'; // ** (crosses /)
        }
      } else {
        src += '[^/]*'; // a single * stays within a segment
      }
    } else if (ch === '?') {
      src += '[^/]';
    } else if (ch !== undefined) {
      src += escapeRe(ch);
    }
  }
  const head = anchored ? '^' : '(?:^|/)';
  const tail = dirOnly ? '/' : '(?:/|$)';
  return new RegExp(`${head}${src}${tail}`);
}

[gemini-code-assist]

Pass the dirOnly flag to globToRegExp so that directory-only rules are compiled with a trailing slash requirement.

Suggested change

	function compileRule(raw: string): IgnoreRule | undefined {
	const negate = raw.startsWith('!');
	let pattern = negate ? raw.slice(1) : raw;
	const dirOnly = pattern.endsWith('/');
	if (dirOnly) pattern = pattern.slice(0, -1);
	// Anchored if a `/` is at the start or the middle (a trailing `/` was already stripped above).
	const anchored = pattern.startsWith('/') || pattern.includes('/');
	if (pattern.startsWith('/')) pattern = pattern.slice(1);
	if (pattern.length === 0) return undefined;
	return { re: globToRegExp(pattern, anchored), negate, dirOnly };
	}
	function compileRule(raw: string): IgnoreRule | undefined {
	const negate = raw.startsWith('!');
	let pattern = negate ? raw.slice(1) : raw;
	const dirOnly = pattern.endsWith('/');
	if (dirOnly) pattern = pattern.slice(0, -1);
	// Anchored if a / is at the start or the middle (a trailing / was already stripped above).
	const anchored = pattern.startsWith('/') || pattern.includes('/');
	if (pattern.startsWith('/')) pattern = pattern.slice(1);
	if (pattern.length === 0) return undefined;
	return { re: globToRegExp(pattern, anchored, dirOnly), negate, dirOnly };
	}

[gemini-code-assist]

Update the ignores function to append a trailing slash to path when isDir is true, and remove the manual rule.dirOnly check. Since directory-only rules are now compiled with a trailing slash requirement, they will correctly match directory paths (which now end in /) and descendant paths (which naturally contain /), while correctly ignoring file paths (which do not end in /).

    ignores(relPath, isDir) {
      const path = relPath.replace(/^\.\//, '').replace(/\/+$/, '') + (isDir ? '/' : '');
      if (path.length === 0 || path === '/') return false;
      let ignored = false;
      for (const rule of rules) {
        if (rule.re.test(path)) ignored = !rule.negate;
      }
      return ignored;
    },

[gemini-code-assist]

Using process.stdout.columns directly can cause a runtime TypeError if process.stdout is undefined (which can happen in certain non-TTY, headless, or test environments where standard streams are redirected or mocked).

Using optional chaining (process.stdout?.columns) is safer and prevents potential crashes.

Suggested change

	columns={process.stdout.columns}
	columns={process.stdout?.columns}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/roadmap/current.md (1)

417-424: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep the status note in one blockquote.

The empty line between the two quoted paragraphs trips markdownlint MD028, so the docs CI may fail. Remove the separator or quote it as > if you want a blank line.

Suggested fix

 > **Status:** ✅ **Done (PR `#66`, 2026-07-07)** — all **12** dependency-ordered steps (S1–S12) landed behind
 > [ADR-0063](../../decisions/0063-cli-config-write-contract.md)/[ADR-0064](../../decisions/0064-live-model-catalog.md)/[ADR-0065](../../decisions/0065-provider-economics-and-extensibility.md),
 > each with an Opus + Sonnet review round (six with an added security round: S2/S5/S6/S8/S9/S10); the post-2.5.G
 > model-UX follow-up ([ADR-0059](../../decisions/0059-cli-mid-session-model-reseat.md) mid-session reseat +
 > [ADR-0066](../../decisions/0066-normalized-reasoning-effort-control.md) reasoning-effort) landed in the same PR.
 > With 2.5.G done, **milestone M2.5-2** (Home + entry + onboarding) is reached.
->
 > **Scope expanded to Option A (2026-07-05).**

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/roadmap/current.md` around lines 417 - 424, The status note is split
into two blockquoted paragraphs with a blank line between them, which triggers
markdownlint MD028. In the current markdown section, keep the entire note in a
single blockquote by removing the empty separator or converting the blank line
to a quoted blank line, and verify the quoted text remains contiguous in the
same blockquote block.

Source: Linters/SAST tools

🧹 Nitpick comments (7)

docs/roadmap/deferred-tasks.md (1)

521-527: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Malformed link syntax: {@link gitignore.ts} won't render in Markdown.

{@link ...} is a JSDoc/TSDoc tag, not Markdown syntax — in a .md file it will render as literal text {@link gitignore.ts} rather than a link. As per coding guidelines, docs/**/*.md should use "relative links" for cross-references.

📝 Proposed fix

-  dependency-free, ReDoS-safe in-house matcher ({`@link` gitignore.ts}) folds the workspace-root `.gitignore` +
+  dependency-free, ReDoS-safe in-house matcher ([`gitignore.ts`](../../apps/cli/src/render/tui/gitignore.ts)) folds the workspace-root `.gitignore` +

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/roadmap/deferred-tasks.md` around lines 521 - 527, The roadmap entry
uses JSDoc-style `{`@link` gitignore.ts}` inside a Markdown file, so it will not
render as a link. Update the affected note in the deferred-tasks section to use
a proper relative Markdown link for the `gitignore.ts` reference, keeping the
rest of the wording intact. Use the existing `gitignore.ts` mention in that
bullet as the anchor and ensure the cross-reference follows the docs/*.md
relative-link convention.

Source: Coding guidelines

apps/cli/src/render/tui/chat-projection.ts (1)

85-90: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

charCodeAt here is correct — SonarCloud's codePointAt suggestion is a false positive.

Detecting a lone leading surrogate at the cap boundary requires inspecting the UTF-16 code unit; codePointAt would return the combined code point (or the low half) and defeat the check. Consider a // NOSONAR / inline disable to silence the analyzer rather than changing the logic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/cli/src/render/tui/chat-projection.ts` around lines 85 - 90, The UTF-16
boundary check in chat-projection’s truncation logic should stay based on
charCodeAt, not codePointAt, because it must detect a lone leading surrogate at
the MAX_APPROVAL_REASON_CHARS cap. Update the surrounding comment or add a
targeted NOSONAR/inline suppression near the truncation logic in chatProjection
so SonarCloud ignores this false positive without changing the behavior of the
slice/end calculation.

Source: Linters/SAST tools

apps/cli/src/render/tui/home-controller.ts (1)

1327-1355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the reason-capture routing to clear the failing complexity gate.

SonarCloud fails this function at cognitive complexity 19 (limit 15). The reasonDraft capture block is self-contained and follows the same shape as routeMentionKey/routeSearchKey — extracting it into a routeReasonKey(...) : boolean helper drops handleChatKey's complexity and keeps the router flat.

♻️ Extract routeReasonKey

+  // The open `[c]` typed-reason capture owns every key (Step 14) — parity with routeMentionKey/routeSearchKey.
+  // Returns whether the key was consumed (the capture was open).
+  const routeReasonKey = (active: HomeChatSession, input: string, key: ChatKey): boolean => {
+    const openReason = state.reasonDraft;
+    if (openReason === undefined) return false;
+    if (active.store.getSnapshot().approval === undefined) {
+      set({ reasonDraft: undefined }); // the approval vanished — discard the orphaned capture
+      return true;
+    }
+    if (key.escape === true) {
+      set({ reasonDraft: undefined }); // cancel the reason; the approval stays pending
+      return true;
+    }
+    if (key.return === true && key.shift !== true) {
+      const reason = sanitizeApprovalReason(openReason.text);
+      set({ reasonDraft: undefined });
+      active.store.answerApproval(
+        reason === undefined ? { outcome: 'reject' } : { outcome: 'reject', reason },
+      );
+      return true;
+    }
+    const edit = reduceEditorMotion(input, key);
+    if (edit !== undefined) set({ reasonDraft: applyEditorAction(openReason, edit) });
+    return true;
+  };
+
   const handleChatKey = (active: HomeChatSession, input: string, key: ChatKey): void => {
     if (tearingDown === active) return; // a key arriving mid-teardown must not drive sendMessage on a cancelled session
-    // The `[c]` typed-reason capture (Step 14) owns the keyboard while open — checked FIRST. ...
-    const openReason = state.reasonDraft;
-    if (openReason !== undefined) {
-      if (active.store.getSnapshot().approval === undefined) {
-        set({ reasonDraft: undefined });
-        return;
-      }
-      if (key.escape === true) {
-        set({ reasonDraft: undefined });
-        return;
-      }
-      if (key.return === true && key.shift !== true) {
-        const reason = sanitizeApprovalReason(openReason.text);
-        set({ reasonDraft: undefined });
-        active.store.answerApproval(
-          reason === undefined ? { outcome: 'reject' } : { outcome: 'reject', reason },
-        );
-        return;
-      }
-      const edit = reduceEditorMotion(input, key);
-      if (edit !== undefined) set({ reasonDraft: applyEditorAction(openReason, edit) });
-      return;
-    }
+    // The `[c]` typed-reason capture (Step 14) owns the keyboard while open — checked FIRST.
+    if (routeReasonKey(active, input, key)) return;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/cli/src/render/tui/home-controller.ts` around lines 1327 - 1355, The
`handleChatKey` function is over the cognitive complexity limit because the
`reasonDraft` capture logic is embedded inline. Extract that self-contained
branch into a new `routeReasonKey(...) : boolean` helper, following the same
routing pattern used by `routeMentionKey` and `routeSearchKey`, and have
`handleChatKey` delegate to it before continuing with the remaining key
handling. Keep the helper responsible for the full reason-capture flow,
including stale approval checks, Escape cancel, Enter-to-reject, and editor
updates, so the main router stays flat.

Source: Linters/SAST tools

apps/cli/src/harness/perf-budget.e2e.test.ts (1)

36-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Unsafe as casts on the DB boundary.

params as ReadonlyArray<...> and row as { detail?: string } are unsafe as casts, which the project's TS guideline for .ts files disallows in favor of type guards. Since this only touches test-harness code reading better-sqlite3's unknown/any-typed results, the risk is low, but a small runtime guard would keep this file compliant without much effort.

As per coding guidelines, **/*.{ts,tsx}: "TypeScript-first, strict. No any, no unsafe as."

♻️ Proposed type-guard alternative

+  function isBindValue(v: unknown): v is string | number | bigint | Buffer | null {
+    return (
+      v === null ||
+      typeof v === 'string' ||
+      typeof v === 'number' ||
+      typeof v === 'bigint' ||
+      Buffer.isBuffer(v)
+    );
+  }
+
   function planFor(query: { toSQL: () => { sql: string; params: unknown[] } }): string[] {
     const { sql, params } = query.toSQL();
-    const bind = params as ReadonlyArray<string | number | bigint | Buffer | null>;
+    if (!params.every(isBindValue)) {
+      throw new Error('unexpected non-primitive bind param from drizzle toSQL()');
+    }
     const rows = client.sqlite.prepare(`EXPLAIN QUERY PLAN ${sql}`).all(...bind);
-    return rows.map((row) => (row as { detail?: string }).detail ?? '');
+    return rows.map((row) =>
+      typeof row === 'object' && row !== null && 'detail' in row && typeof row.detail === 'string'
+        ? row.detail
+        : '',
+    );
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/cli/src/harness/perf-budget.e2e.test.ts` around lines 36 - 44, The
`planFor` helper in `perf-budget.e2e.test.ts` uses unsafe `as` casts at the
SQLite boundary for both `params` and `row.detail`, which violates the
TypeScript strictness guideline. Replace those casts with small runtime type
guards around the `toSQL()` params and the `EXPLAIN QUERY PLAN` rows, then only
pass validated bind values into `client.sqlite.prepare(...).all(...)` and safely
read `detail` from rows in the map step.

Source: Coding guidelines

docs/standards/security-review.md (1)

350-361: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Restates the canonical recoverable-denial taxonomy instead of linking to it.

This paragraph re-enumerates the same recoverable/fatal denial taxonomy already canonically documented in docs/reference/shared-core/tool-registry.md (§Error taxonomy, the "recoverable" subsection). As per coding guidelines, docs/**/*.md: "Specs live in [docs/reference/]; link, don't restate. One canonical home per artifact."

Consider trimming this to a short security summary that links to the tool-registry.md taxonomy for the authoritative enumeration, so future taxonomy changes only need updating in one place.

✏️ Example trim

-- **A recoverable SCOPE denial is safe by construction.** Only two `tool_denied`s are fed back to
-  the model for in-turn conversational recovery on the `recoverToolFailures` surfaces (the
-  chat-read-write host — `relavium chat` / the Home / the one-shot `agent run`; a WORKFLOW run stays
-  fatal/deterministic): a media scope denial and the fs **pure scope-tier escape** — both refused
-  BEFORE any side effect, with a **secret-free, path-free** reason, so the model adapts to an
-  in-bounds path while the floor still denies every attempt (no bypass, bounded by
-  `maxToolCorrections`). The scope-membership signal a probing model can gather over that bounded
-  window is an accepted, secret-free workspace-boundary oracle (the same round-bounded feedback
-  already existed for an idempotent not-found read). The **confidentiality** (secret-store read),
-  protected-path, symlink/hard-link, egress-SSRF, and user/guardrail denials stay FATAL — feeding
-  those back would re-deny, risk a re-execution, or leak a probe oracle.
+- **A recoverable SCOPE denial is safe by construction.** Only two `tool_denied` reasons are fed
+  back to the model for in-turn conversational recovery (a WORKFLOW run stays fatal/deterministic).
+  See [tool-registry.md §Error taxonomy](../reference/shared-core/tool-registry.md#error-taxonomy)
+  for the authoritative list of recoverable vs. fatal denial reasons and the safety rationale.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/standards/security-review.md` around lines 350 - 361, The
security-review paragraph is restating the recoverable/fatal denial taxonomy
instead of pointing to the canonical source. Trim this section in
docs/standards/security-review.md to a brief summary and add a link to the
authoritative taxonomy in docs/reference/shared-core/tool-registry.md, keeping
the detailed enumeration only in that canonical reference. Use the surrounding
security policy text to preserve the high-level guidance while removing
duplicated denial categories.

Source: Coding guidelines

packages/db/src/model-catalog-store.ts (1)

491-589: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the transaction callback to clear the SonarCloud nesting failure.

The withBusyRetry(() => db.transaction(() => { … })) wrapper pushed the rows.map/branch bodies past the 4-level nesting threshold, tripping the failing SonarCloud check at Line 565. Hoisting the transaction body into a named local removes one level without changing behavior; apply the same shape to upsert for consistency.

♻️ Sketch: hoist the transaction body

-    replaceProviderModels: (providerId, rows, now) =>
-      withBusyRetry(() =>
-        db.transaction(
-          () => {
-            let added = 0;
-            let updated = 0;
-            // …
-            return { added, updated, deactivated: deactivateResult.changes };
-          },
-          { behavior: 'immediate' },
-        ),
-      ),
+    replaceProviderModels: (providerId, rows, now) => {
+      const applyReplace = (): ReplaceResult => {
+        let added = 0;
+        let updated = 0;
+        // …
+        return { added, updated, deactivated: deactivateResult.changes };
+      };
+      return withBusyRetry(() => db.transaction(applyReplace, { behavior: 'immediate' }));
+    },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/db/src/model-catalog-store.ts` around lines 491 - 589, The nested
inline callback in withBusyRetry(db.transaction(...)) is pushing the
model-catalog refresh logic past SonarCloud’s nesting limit. Hoist the
transaction body in model-catalog-store into a named local function, then pass
that function to db.transaction and keep withBusyRetry wrapping it; preserve the
existing add/update/deactivate logic and apply the same refactor shape to upsert
for consistency.

Source: Linters/SAST tools

packages/db/src/media-reference-store.ts (1)

171-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Nesting depth flagged by SonarCloud (Lines 189, 244).

removeRunReferences and reclaimExpired each nest withBusyRetry(() => db.transaction(() => { ... for (...) {...} })) four function levels deep. Functionally fine and well-commented, but extracting each transaction body into a small named helper (e.g. runRemoveRunReferencesTxn(db, runId, ts)) would flatten this and make each piece independently testable/readable.

Also applies to: 223-262

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/db/src/media-reference-store.ts` around lines 171 - 202, The nested
transaction logic in removeRunReferences is too deep and should be flattened for
readability and testability. Extract the db.transaction body into a small named
helper such as runRemoveRunReferencesTxn(db, runId, ts) (and similarly for
reclaimExpired where applicable), then have removeRunReferences call that helper
inside withBusyRetry so the control flow is clearer while preserving the
existing behavior.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/cli/src/harness/concurrency.e2e.test.ts`:
- Around line 94-119: The runChild helper in the concurrency e2e test can leave
the ready promise unresolved when spawn fails, causing Promise.all waits to hang
instead of surfacing the error. Update runChild so the child.on('error') path
also calls signalReady() before rejecting, and keep the existing close fallback
in place; use the runChild, ready, done, and signalReady symbols to make the fix
in the same helper.

In `@apps/cli/src/render/tui/gitignore.ts`:
- Around line 42-76: The globToRegExp helper in gitignore.ts is turning
workspace ignore globs into potentially backtracking-heavy regexes, which can
stall ignores() during mention completion on crafted patterns. Update
globToRegExp and its callers to avoid using greedy regex matching for untrusted
patterns, either by enforcing strict length/complexity limits before compiling
or by replacing the regex approach with a linear-time glob matcher that handles
the same anchored and segment-boundary behavior.

In `@docs/uvp.md`:
- Around line 22-23: The wording in the UVP copy overstates the security
guarantee by saying API keys are “never at rest,” which is too absolute for the
OS keychain. Update the sentence in the docs text to explicitly say the keys are
stored in the OS keychain and do not reach history.db or plaintext storage,
keeping the rest of the surrounding copy unchanged.

In `@README.md`:
- Around line 135-139: Update the README status text to match the current
roadmap: the summary under the Phase 2.5 section should no longer say only the
regression-harness/concurrency and documentation-consolidation lanes remain,
since docs/roadmap/current.md now marks 2.5.I and 2.5.J complete and Phase 2.5
finished. Adjust the wording in the README paragraph so it reflects that Phase
2.5 is complete while preserving the rest of the release/status messaging.

---

Outside diff comments:
In `@docs/roadmap/current.md`:
- Around line 417-424: The status note is split into two blockquoted paragraphs
with a blank line between them, which triggers markdownlint MD028. In the
current markdown section, keep the entire note in a single blockquote by
removing the empty separator or converting the blank line to a quoted blank
line, and verify the quoted text remains contiguous in the same blockquote
block.

---

Nitpick comments:
In `@apps/cli/src/harness/perf-budget.e2e.test.ts`:
- Around line 36-44: The `planFor` helper in `perf-budget.e2e.test.ts` uses
unsafe `as` casts at the SQLite boundary for both `params` and `row.detail`,
which violates the TypeScript strictness guideline. Replace those casts with
small runtime type guards around the `toSQL()` params and the `EXPLAIN QUERY
PLAN` rows, then only pass validated bind values into
`client.sqlite.prepare(...).all(...)` and safely read `detail` from rows in the
map step.

In `@apps/cli/src/render/tui/chat-projection.ts`:
- Around line 85-90: The UTF-16 boundary check in chat-projection’s truncation
logic should stay based on charCodeAt, not codePointAt, because it must detect a
lone leading surrogate at the MAX_APPROVAL_REASON_CHARS cap. Update the
surrounding comment or add a targeted NOSONAR/inline suppression near the
truncation logic in chatProjection so SonarCloud ignores this false positive
without changing the behavior of the slice/end calculation.

In `@apps/cli/src/render/tui/home-controller.ts`:
- Around line 1327-1355: The `handleChatKey` function is over the cognitive
complexity limit because the `reasonDraft` capture logic is embedded inline.
Extract that self-contained branch into a new `routeReasonKey(...) : boolean`
helper, following the same routing pattern used by `routeMentionKey` and
`routeSearchKey`, and have `handleChatKey` delegate to it before continuing with
the remaining key handling. Keep the helper responsible for the full
reason-capture flow, including stale approval checks, Escape cancel,
Enter-to-reject, and editor updates, so the main router stays flat.

In `@docs/roadmap/deferred-tasks.md`:
- Around line 521-527: The roadmap entry uses JSDoc-style `{`@link` gitignore.ts}`
inside a Markdown file, so it will not render as a link. Update the affected
note in the deferred-tasks section to use a proper relative Markdown link for
the `gitignore.ts` reference, keeping the rest of the wording intact. Use the
existing `gitignore.ts` mention in that bullet as the anchor and ensure the
cross-reference follows the docs/*.md relative-link convention.

In `@docs/standards/security-review.md`:
- Around line 350-361: The security-review paragraph is restating the
recoverable/fatal denial taxonomy instead of pointing to the canonical source.
Trim this section in docs/standards/security-review.md to a brief summary and
add a link to the authoritative taxonomy in
docs/reference/shared-core/tool-registry.md, keeping the detailed enumeration
only in that canonical reference. Use the surrounding security policy text to
preserve the high-level guidance while removing duplicated denial categories.

In `@packages/db/src/media-reference-store.ts`:
- Around line 171-202: The nested transaction logic in removeRunReferences is
too deep and should be flattened for readability and testability. Extract the
db.transaction body into a small named helper such as
runRemoveRunReferencesTxn(db, runId, ts) (and similarly for reclaimExpired where
applicable), then have removeRunReferences call that helper inside withBusyRetry
so the control flow is clearer while preserving the existing behavior.

In `@packages/db/src/model-catalog-store.ts`:
- Around line 491-589: The nested inline callback in
withBusyRetry(db.transaction(...)) is pushing the model-catalog refresh logic
past SonarCloud’s nesting limit. Hoist the transaction body in
model-catalog-store into a named local function, then pass that function to
db.transaction and keep withBusyRetry wrapping it; preserve the existing
add/update/deactivate logic and apply the same refactor shape to upsert for
consistency.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7da5fc9-e826-48b5-861d-37d5ced7be73

📥 Commits

Reviewing files that changed from the base of the PR and between 84fb7a4 and 3dffc8c.

📒 Files selected for processing (81)

• .github/workflows/ci.yml
• CLAUDE.md
• README.md
• apps/cli/src/chat/agent-source.ts
• apps/cli/src/chat/chat-mode-host.ts
• apps/cli/src/chat/chat-mode.test.ts
• apps/cli/src/chat/chat-mode.ts
• apps/cli/src/chat/session-host.test.ts
• apps/cli/src/commands/agent-run.ts
• apps/cli/src/commands/chat.ts
• apps/cli/src/commands/provider.ts
• apps/cli/src/engine/media-gc.test.ts
• apps/cli/src/engine/tool-host/assemble.test.ts
• apps/cli/src/engine/tool-host/assemble.ts
• apps/cli/src/engine/tool-host/errors.ts
• apps/cli/src/engine/tool-host/fs.test.ts
• apps/cli/src/engine/tool-host/fs.ts
• apps/cli/src/harness/concurrency.e2e.test.ts
• apps/cli/src/harness/fixtures/concurrent-writer.mjs
• apps/cli/src/harness/perf-budget.e2e.test.ts
• apps/cli/src/harness/session-chain.e2e.test.ts
• apps/cli/src/process/options.test.ts
• apps/cli/src/process/options.ts
• apps/cli/src/program.ts
• apps/cli/src/render/tui/chat-ink.tsx
• apps/cli/src/render/tui/chat-input.test.ts
• apps/cli/src/render/tui/chat-input.ts
• apps/cli/src/render/tui/chat-projection.test.ts
• apps/cli/src/render/tui/chat-projection.ts
• apps/cli/src/render/tui/chat-store.ts
• apps/cli/src/render/tui/gitignore.test.ts
• apps/cli/src/render/tui/gitignore.ts
• apps/cli/src/render/tui/home-app.tsx
• apps/cli/src/render/tui/home-controller.test.ts
• apps/cli/src/render/tui/home-controller.ts
• apps/cli/src/render/tui/mention.test.ts
• apps/cli/src/render/tui/mention.ts
• apps/cli/src/run.ts
• apps/cli/src/workflows/catalog.test.ts
• docs/architecture/agent-sessions.md
• docs/architecture/local-first-and-security.md
• docs/decisions/0029-tool-policy-hardening.md
• docs/decisions/0057-cli-chat-modes-and-per-tool-approval.md
• docs/decisions/0064-live-model-catalog.md
• docs/decisions/README.md
• docs/reference/cli/commands.md
• docs/reference/cli/regression-harness.md
• docs/reference/desktop/database-schema.md
• docs/reference/desktop/tauri-plugins.md
• docs/reference/shared-core/agent-runner.md
• docs/reference/shared-core/tool-registry.md
• docs/roadmap/current.md
• docs/roadmap/deferred-tasks.md
• docs/roadmap/phases/phase-2.5-cli-consolidation.md
• docs/roadmap/phases/phase-2.6-conversational-authoring.md
• docs/standards/security-review.md
• docs/tech-stack.md
• docs/tutorials/cli/start-a-chat-session.md
• docs/uvp.md
• docs/vision.md
• packages/core/src/agent-parser.test.ts
• packages/core/src/agent-parser.ts
• packages/core/src/engine/agent-turn.test.ts
• packages/core/src/engine/agent-turn.ts
• packages/core/src/tools/builtins.test.ts
• packages/core/src/tools/errors.ts
• packages/db/src/client.test.ts
• packages/db/src/media-reference-store.test.ts
• packages/db/src/media-reference-store.ts
• packages/db/src/media-store.test.ts
• packages/db/src/media-write.test.ts
• packages/db/src/model-catalog-store.test.ts
• packages/db/src/model-catalog-store.ts
• packages/db/src/provider-store.test.ts
• packages/db/src/provider-store.ts
• packages/db/src/retry.test.ts
• packages/db/src/retry.ts
• packages/db/src/run-history-store.test.ts
• packages/db/src/run-history-store.ts
• packages/db/src/session-store.test.ts
• packages/db/src/session-store.ts

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.1% Duplication on New Code

See analysis details on SonarQube Cloud