Feat/task memory evidence pipeline

.agents/sessions/external-read-roots-2026-08/SPEC.md

@@ -0,0 +1,151 @@
+# SPEC — Read-only external read roots
+
+## Overview
+
+Openbuff tools are contained to the project root, plus a narrow exception for the
+openbuff-owned OS temp namespace (`owned-temp`). Users cannot read files outside
+the project — including their own openbuff config directory (logs, harness
+state) — even though those are the user's own files and reading them is a
+legitimate, non-mutating action.
+
+This work adds a third containment scope, `external-read`: a **read-only**,
+**default-closed**, **configure-once** allowlist of roots outside the project
+that path-taking READ tools may reach. The write path is structurally excluded.
+
+Wave 1 (the `common` primitive) is COMPLETE, gate-approved, and committed.
+Wave 2 (product wiring) is implemented in the working tree but NOT yet green.
+
+## Goals
+
+- A user can read files in their openbuff config dir (e.g. logs, harness state)
+  through `read_files` / `read_logs` / `read_image` / `list_directory`.
+- A user can allowlist additional absolute roots via `openbuff.json`
+  → `readableRoots: string[]`.
+- Mandatory-sensitive files stay blocked **inside** allowlisted roots
+  (`credentials.json`, `.env`, private keys, kubeconfig, tfstate, …).
+- Writes can never reach an allowlisted root, enforced structurally rather than
+  by handler discipline.
+- Default posture is closed: with nothing configured, behavior is byte-identical
+  to before this work.
+
+## Non-goals
+
+- `glob` stays project-only. It is a pattern-driven directory walk; widening it
+  would let a pattern enumerate an allowlisted root. Deliberately out of scope.
+- No write, move, delete, or `cwd` access to external roots, ever.
+- No relative entries in `readableRoots` (ambiguous in a global config file;
+  dropped rather than guessed).
+- No mid-session reconfiguration. Changing `readableRoots` requires a restart.
+- No UI/slash-command surface for managing roots in this iteration.
+
+## Requirements
+
+### R1 — Containment primitive (`common`) — DONE, committed
+
+- `ContainedProjectPath['scope']` is `'project' | 'owned-temp' | 'external-read'`.
+- `configureExternalReadRoots(roots)` normalizes/dedupes/sorts; skips filesystem
+  roots and `..` entries; idempotent for an equivalent set; THROWS on a
+  differing set.
+- `ensureExternalReadRootsConfigured(roots)` — non-throwing wrapper returning
+  `'configured' | 'unchanged' | 'refused-changed'`.
+- `getExternalReadRoots()`, `resetExternalReadRootsForTesting()`,
+  `isExternalReadPath(input)`.
+- `resolveProjectPathForRead` / `resolveProjectPathForFileSystemRead` — the ONLY
+  entry points that can produce `external-read`. They delegate to the existing
+  write resolvers first and only fall back to the external branch on `null`.
+- `resolveProjectPath` / `resolveProjectPathForFileSystem` are UNCHANGED.
+- The external resolver refuses mandatory-sensitive basenames on BOTH the
+  lexical and dereferenced path, so consumers inherit the refusal fail-closed.
+- `credentials.json` / `.yaml` / `.yml` added to `SENSITIVE_BASENAMES`.
+
+### R2 — Config schema (`sdk/src/provider-config.ts`) — implemented, unverified
+
+- `readableRoots: z.array(z.string().min(1)).default([])`.
+- `.default([])` is intentional: downstream code never handles `undefined`.
+  Consequence: `readableRoots: string[]` is REQUIRED in the schema OUTPUT type
+  and therefore in `LoadedProviderConfig['config']`. This is what breaks the
+  hand-built test fixtures (see PLAN task T1).
+
+### R3 — Read-only operation resolvers (`sdk/src/tools/path-utils.ts`) — implemented
+
+- `resolveFilePathForReadOperation`,
+  `resolveFilePathForFileSystemReadOperation`.
+- Follow-symlink shape only; no `followFinalSymlink: false` (that option exists
+  for unlink-style mutations).
+- The existing `resolveFilePathFor*Operation` write twins are unchanged.
+
+### R4 — Read handlers rewired — implemented
+
+`read-files.ts` (`authorizeReadTarget`), `read-logs.ts`, `read-image.ts`,
+`list-directory.ts`. Plus: `read-files.ts` extends its owned-temp fileFilter
+alias block to cover `external-read` (both scopes carry an ABSOLUTE
+`relativePath`, so a host filter written against project-relative globs would
+silently fail OPEN). Alias key: `external-read/<basename>`.
+
+### R5 — Run-start configuration (`sdk/src/run.ts`) — implemented
+
+In `runOnce`, before tool dispatch: `ensureExternalReadRootsConfigured([...])`
+with the openbuff config dir (`getConfigDir(env)`) plus absolute-only entries
+from `loadProviderConfigSync().config.readableRoots`. Wrapped in try/catch so a
+malformed config cannot block a run. `'refused-changed'` logs a warn naming
+counts, not paths (home-dir paths are mildly sensitive and logs get shared).
+
+### R6 — Agent-runtime read backstop (`tool-executor.ts`) — implemented
+
+The pre-dispatch scope check's `ownedTempRead` condition extended so an
+allowlisted external READ is not hard-blocked. Writes there still hard-block.
+
+### R7 — Fixtures + validation — BLOCKED (this is the remaining work)
+
+Six typecheck failures, all "missing required `readableRoots`" in hand-built
+`LoadedProviderConfig` fixtures. See PLAN task T1 for exact sites.
+
+### R8 — Security review — NOT STARTED
+
+`security-reviewer` on the permission-boundary widening.
+
+### R9 — Documentation — NOT STARTED
+
+`docs/configuration.md` + `openbuff.json.example`.
+
+## Acceptance criteria
+
+- AC1 — `bun run typecheck` exits 0 for all 11 workspace packages.
+- AC2 — `bun test sdk/src/__tests__/` reports 0 fail AND 0 error.
+- AC3 — `bun test common/src/util/__tests__/` and the agent-runtime tool tests
+  report 0 fail.
+- AC4 — Test named for the write-path invariant still passes: `resolveProjectPath`
+  returns `null` for a path inside a configured allowlisted root.
+- AC5 — `credentials.json` inside an allowlisted root is refused by BOTH
+  `isExternalReadPath` and the resolver.
+- AC6 — With `readableRoots` unset and no config dir on the allowlist, external
+  paths are refused exactly as before (default-closed).
+- AC7 — Full-directory SDK test run is order-independent: the read-logs external
+  suite passes both alone and after a suite that exercises the SDK run path.
+- AC8 — `security-reviewer` returns no unresolved BLOCKING finding.
+- AC9 — Docs state: reads only, absolute-only entries, sensitive files still
+  blocked, restart required to apply changes.
+
+## Relevant files
+
+Committed (wave 1):
+- `common/src/util/project-path-containment.ts`
+- `common/src/util/sensitive-paths.ts`
+- `common/src/util/__tests__/{project-path-containment,sensitive-paths}.test.ts`
+- `sdk/src/tools/filesystem-authority.ts` — `toAuthorizedPath` fails closed on
+  `external-read` with code `external_read_scope_unsupported`
+
+Dirty (wave 2, in working tree):
+- `sdk/src/provider-config.ts`, `sdk/src/run.ts`
+- `sdk/src/tools/{path-utils,read-files,read-logs,read-image,list-directory}.ts`
+- `packages/agent-runtime/src/tools/tool-executor.ts`
+- `sdk/src/__tests__/{path-utils,read-files,read-logs,model-provider}.test.ts`
+- `packages/agent-runtime/src/__tests__/run-agent-step-tools.test.ts`
+- `common/src/util/project-path-containment.ts` (+ its test) — `ensureExternalReadRootsConfigured`
+
+To touch in T1:
+- `sdk/src/__tests__/model-provider.test.ts`
+- `sdk/src/impl/__tests__/failover.test.ts`
+
+To touch in T4:
+- `docs/configuration.md`, `openbuff.json.example`

agents/base2/quality-prompt-section.ts

@@ -74,7 +74,7 @@ Neither replaces the runtime hooks + automated code-reviewer path.
 
 ## Hard blocks while GATE: PENDING
 
-- \`suggest_followups\` — rejected
+- \`suggest_followups\` — rejected (end the turn instead; the rejection is agent-facing only and is not shown to the user)
 - \`git-committer\` — withheld until GATE: PASSED
 - Manual re-spawn of code-reviewer for the same pending set — do not; the automated gate owns that set. If phase is \`awaiting_validation\` / gate not yet passed, end the turn for the programmatic hooks→reviewer cycle.
 
@@ -88,5 +88,5 @@ Dirty working-tree files are not the same as pending: only task-related **review
 
 - Write the final user-visible completion summary first
 - Spawn optional \`git-committer\` (with \`params.owned_paths\` for task-owned paths) before followups if committing this turn
-- Call \`suggest_followups\` only as the absolute last tool after summary/commit; never mid-turn and never before remaining work
+- Call \`suggest_followups\` only as the absolute last tool after summary/commit — it is the FINAL output of the turn, so emit nothing after it except \`end_turn\`/\`task_completed\`; never mid-turn and never before remaining work
 - The gate re-arms on every new edit (back to GATE: PENDING); one more clear cycle is required. Treat early withhold as normal ordering; do not tight-loop committer spawns — wait for GATE: PASSED, then spawn once.`

cli/knowledge.md

@@ -907,3 +907,7 @@ Streaming markdown renders as plain text until the message or agent finishes. Th
 - _Knowledge refresh 2026-08-23: add `/memory` (alias `/mem`) slash command; staleness guard touch._
 
 - _Knowledge refresh 2026-08-31: live compaction status rendering (`context_compaction_status` consumption, run-correlated pending/settled pairing, replayed-pending-as-interrupted) in `cli/src/utils/sdk-event-handlers.ts` and `cli/src/components/renderers/compaction-box.tsx`._
+
+- _Knowledge refresh 2026-08-31 (followups): `handleRuntimeError` in `cli/src/utils/sdk-event-handlers.ts` now splits runtime error events by `autoRecovering` — auto-recovering notices log at `debug` (`'SDK auto-recovering runtime notice'`) with no visible error banner, while genuine failures still log at `error` (`'SDK runtime error event'`) and render. Tool-ordering rejections (the `suggest_followups` gate/ordering rejections and the pre-gate `git-committer withheld` chunk) arrive with a concise `userMessage` plus `autoRecovering: true`, so the model still receives the full `message` via the `TOOL_CALL_ERROR` path in `packages/agent-runtime/src/tools/stream-parser.ts`; `git-committer blocked by unvalidated dirty file(s)` deliberately stays user-visible because it asks the user to reply `COMMIT ANYWAY`._
+
+- _Knowledge refresh 2026-08-31 (UI polish): `cli/src/components/status-bar.tsx` is now three regions — status label left, chip cluster left-aligned in the growing middle (`flexGrow: 1` + `flexBasis: 0`), and every width-varying control (scroll-to-bottom, then the `■ Esc` stop hint) in a `flexShrink: 0` right region with no `minWidth: 0`, so a hover cannot reflow the label or the chips. `cli/src/components/scroll-to-bottom-button.tsx` exports `SCROLL_HINT_LABEL`/`SCROLL_GLYPH` plus `string-width`-derived `SCROLL_BUTTON_WIDTH` (10) and `SCROLL_BUTTON_COMPACT_WIDTH` (3) and renders at a fixed width in both hover states; its `isScrollButtonCompact(width)` predicate is the single source `StatusBar` also passes as `scrollButtonCompact`, so `statusBarChipBudget`'s duplicated `SCROLL_BUTTON_RESERVATION`/`SCROLL_BUTTON_COMPACT_RESERVATION` in `cli/src/utils/status-bar-chips.ts` always reserve the columns actually rendered (test-enforced agreement, since the util must not import a component module). `cli/src/components/renderers/completion-summary-box.tsx` renders a titled `Run summary` `HarnessBox` (`gap={0}`, `paddingBottom={0}`) of aligned `Label   value` rows built from `ROW_LABELS` + derived `LABEL_COLUMN_WIDTH`, with no status emoji — meaning lives in the value words, not color. Reconciler-level coverage for the status bar lives in `cli/src/components/__tests__/status-bar.test.tsx`, which reuses the dev-only `renderTest`/`renderFrame` convention from `text-nesting.test.tsx` (`@opentui/react/test-utils` cannot be imported under `NODE_ENV=production`)._

cli/src/components/__tests__/completion-summary-box.test.tsx

@@ -13,6 +13,9 @@ initializeThemeStore()
 
 const theme = chatThemes.dark
 
+/** Icons the tightened box must never render again. */
+const REMOVED_ICONS = ['✅', '❌', '⚠️', '⚠', '🔴', '🟡', '🟢']
+
 function makeSummary(overrides: Partial<CompletionSummary>): CompletionSummary {
   return {
     filesEdited: 0,
@@ -190,11 +193,13 @@ describe('CompletionSummaryBox deriveTone', () => {
     expect(markup).toContain('1 failed')
   })
 
-  test('renders auxiliary section', () => {
+  test('renders auxiliary section under the Agents label', () => {
     const markup = renderSummary(
       makeSummary({ auxiliaryCompleted: 2, auxiliaryFailed: 1 }),
     )
-    expect(markup).toContain('auxiliary')
+    expect(markup).toContain('Agents')
+    expect(markup).toContain('2 completed')
+    expect(markup).toContain('1 failed')
   })
 
   test('renders errors section with error color', () => {
@@ -203,21 +208,64 @@ describe('CompletionSummaryBox deriveTone', () => {
     expect(markup).toContain(theme.error)
   })
 
-  test('renders BLOCKING with red icon', () => {
+  test('renders BLOCKING in the error tone, without an icon', () => {
     const markup = renderSummary(makeSummary({ reviewVerdict: 'BLOCKING' }))
     expect(markup).toContain('BLOCKING')
-    expect(markup).toContain('🔴')
+    expect(markup).toContain(theme.error)
   })
 
-  test('renders NON_BLOCKING with yellow icon', () => {
+  test('renders NON_BLOCKING in the warning tone, without an icon', () => {
     const markup = renderSummary(makeSummary({ reviewVerdict: 'NON_BLOCKING' }))
     expect(markup).toContain('NON_BLOCKING')
-    expect(markup).toContain('🟡')
+    expect(markup).toContain(theme.warning)
   })
 
-  test('renders LOOKS_GOOD with green icon', () => {
+  test('renders LOOKS_GOOD in the success tone, without an icon', () => {
     const markup = renderSummary(makeSummary({ reviewVerdict: 'LOOKS_GOOD' }))
     expect(markup).toContain('LOOKS_GOOD')
-    expect(markup).toContain('🟢')
+    expect(markup).toContain(theme.success)
+  })
+
+  test('titles the box and emits none of the removed status emoji', () => {
+    const markup = renderSummary(
+      makeSummary({
+        filesEdited: 2,
+        filesRolledBack: 1,
+        hooksPassed: 1,
+        hooksSkipped: 2,
+        reviewVerdict: 'NON_BLOCKING',
+        testPassed: 5,
+        testFailed: 1,
+        auxiliaryCompleted: 3,
+        auxiliaryFailed: 1,
+        errors: 2,
+      }),
+    )
+
+    expect(markup).toContain('Run summary')
+    for (const icon of REMOVED_ICONS) {
+      expect(markup).not.toContain(icon)
+    }
+    // Meaning still survives without color: the state words are in the values.
+    expect(markup).toContain('rolled back')
+    expect(markup).toContain('2 skipped')
+  })
+
+  test('pads the label column so every row value starts at the same offset', () => {
+    const markup = renderSummary(
+      makeSummary({ filesEdited: 2, reviewVerdict: 'LOOKS_GOOD' }),
+    )
+    // One row per <text>; strip the inline spans to get the rendered line.
+    const rows = [...markup.matchAll(/<text[^>]*>(.*?)<\/text>/g)].map(
+      (match) => match[1].replace(/<[^>]*>/g, ''),
+    )
+    const filesRow = rows.find((row) => row.startsWith('Files'))
+    const reviewRow = rows.find((row) => row.startsWith('Review'))
+
+    expect(filesRow).toBeDefined()
+    expect(reviewRow).toBeDefined()
+    // 'Files' is a column shorter than 'Review', so equal value offsets can
+    // only come from the shared padded label column.
+    expect(filesRow?.indexOf('2 edited')).toBe(reviewRow?.indexOf('LOOKS_GOOD'))
   })
 })

cli/src/components/__tests__/scroll-to-bottom-button.test.ts

@@ -0,0 +1,68 @@
+import { describe, expect, test } from 'bun:test'
+import stringWidth from 'string-width'
+
+import {
+  SCROLL_BUTTON_COMPACT_RESERVATION,
+  SCROLL_BUTTON_RESERVATION,
+} from '../../utils/status-bar-chips'
+import {
+  SCROLL_BUTTON_COMPACT_WIDTH,
+  SCROLL_BUTTON_WIDTH,
+  SCROLL_GLYPH,
+  SCROLL_HINT_LABEL,
+} from '../scroll-to-bottom-button'
+
+/**
+ * The button reserves a fixed width so hovering it cannot reflow the status
+ * bar. Asserted on the exported constants rather than through the reconciler:
+ * `@opentui/react/test-utils` is unavailable in this package's production-mode
+ * test run, so hover cannot be simulated here.
+ */
+describe('ScrollToBottomButton reserved width', () => {
+  test('the expanded width fits the hint label plus one column per side', () => {
+    expect(SCROLL_BUTTON_WIDTH).toBe(stringWidth(SCROLL_HINT_LABEL) + 2)
+    // Pinned: status-bar-chips.ts duplicates this as a literal reservation, so
+    // a change to the label must be a deliberate change to both.
+    expect(SCROLL_BUTTON_WIDTH).toBe(10)
+  })
+
+  test('the resting glyph occupies the same reserved width as the hover label', () => {
+    // Both expanded-mode states render inside the same fixed width, so the
+    // rendered button width is identical hovered and unhovered.
+    const restingWidth = stringWidth(SCROLL_GLYPH) + 2
+    expect(restingWidth).toBeLessThanOrEqual(SCROLL_BUTTON_WIDTH)
+    expect(SCROLL_BUTTON_COMPACT_WIDTH).toBe(restingWidth)
+    // Same pinning as the expanded width, against the duplicated reservation.
+    expect(SCROLL_BUTTON_COMPACT_WIDTH).toBe(3)
+  })
+
+  test('the hint label and the compact form share the resting glyph', () => {
+    // The compact width is derived from SCROLL_GLYPH, so the glyph the button
+    // renders when resting is the one the width was measured from.
+    expect(SCROLL_HINT_LABEL.startsWith(SCROLL_GLYPH)).toBe(true)
+    // Non-ASCII, so rendered width rather than `.length` is what matters.
+    expect(stringWidth(SCROLL_GLYPH)).toBe(1)
+  })
+
+  test('the hint stays short enough to leave the chips room', () => {
+    // The old '↓ Scroll to bottom ↓' label was 20 columns; permanently
+    // reserving that much would crowd the chip cluster.
+    expect(SCROLL_BUTTON_WIDTH).toBeLessThan(20)
+  })
+
+  test('the chip budget reservation matches the button width', () => {
+    // status-bar-chips.ts duplicates the value rather than importing this
+    // component module, so the two must be asserted to agree.
+    expect(SCROLL_BUTTON_RESERVATION).toBe(SCROLL_BUTTON_WIDTH)
+  })
+
+  test('the compact chip budget reservation matches the compact button width', () => {
+    // Same duplication as above, for the narrow 'xs'/'sm' form.
+    expect(SCROLL_BUTTON_COMPACT_RESERVATION).toBe(SCROLL_BUTTON_COMPACT_WIDTH)
+    // Strictly cheaper than the expanded form, so reserving the expanded width
+    // at 'xs'/'sm' would cost the chips columns for nothing.
+    expect(SCROLL_BUTTON_COMPACT_RESERVATION).toBeLessThan(
+      SCROLL_BUTTON_RESERVATION,
+    )
+  })
+})