# Plan — Make `feature/iframe-preview-tier13` fully green for release

## Context

This branch carries v0.1 (iframe preview, web_serve unification, sandbox hardening) plus v0.2 (#155 pentest hardening: HMAC chain, env allowlist, RLIMIT_NPROC, secrets-subtree path-guard, RequireNotBypass middleware, master.key 0600, NET_CONNECT_TCP, PR_SET_DUMPABLE).

Per CLAUDE.md hard-constraint #7 (release-responsibility), pre-existing failures are NOT a closure path. Every CI gate must exit clean before this branch ships. The user has explicitly approved deferring exactly two product gaps (#105 offline send queue, #107 mock-media tool for file-download) with allowlist entries — every other failing test must be fixed.

**Current measured state on `c12a463` (post `-s ours` merge of main, with 163 uncommitted files from in-flight session work):**

| Gate | Status | Notes |
|---|---|---|
| Go build (`CGO_ENABLED=0 -tags goolm,stdjson,test_harness`) | green | 79 packages |
| Race tests (`CGO_ENABLED=1 ./pkg/sysagent/... ./pkg/config/... ./pkg/credentials/... ./pkg/channels/...`) | green | 19 packages |
| `tsc --noEmit` + `vitest` | green | 820/820 |
| `govulncheck` | green | go.mod bumped to 1.26.2 |
| `golangci-lint` (CI v2.10.1, local v2.5.0) | red | ~120 stylistic residue. **Local cannot run today** — local v2.5.0 errors with "go1.25 used to build < 1.26.2 targeted". Implementer must `go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.10.1` first. |
| Playwright | red | 23 active failures + 10 SKIP_ALLOWLIST entries (#101–#111, #133) currently passing under softSkip / `test.skip` |

Two failing tests use `expect(false, '…BLOCKED…').toBe(true)` (`tests/e2e/chat.spec.ts:152` for #105, `tests/e2e/media.spec.ts:64` for #107). Per user decision, these convert to `test.skip()` calls plus new SKIP_ALLOWLIST entries with `until: 2026-09-30`. No code is implemented for #105/#107 in this branch.

**Verified gotchas from exploration that the draft missed:**

1. **`ws-reconnect.spec.ts` is a real source bug.** `src/lib/ws.ts:496` attaches `visibilitychange` to `window`. The Web Platform spec only fires this event on `document`. The test (and any real browser) dispatches it on `document`, so the listener never fires. Fix is one line: `document.addEventListener('visibilitychange', this._onVisibilityChange)` and matching `removeEventListener`. The `online` listener (`window.addEventListener('online', …)`) is correct as-is — `online`/`offline` ARE `window` events.
2. **`agents.spec.ts:89` and `skills.spec.ts:44` are test bugs, not source bugs.** `CreateAgentModal.tsx:124` already feeds `isApiError(err) ? err.userMessage : …` into the toast. `userMessage` for status 409 is `"This conflicts with the current state. Please refresh and try again."` (`src/lib/api-error.ts:50`). The test asserts `text=409`, which never appears. Fix: change the locator to `text=conflicts with the current state`.
3. **`retention.spec.ts:194` hits the wrong URL.** Test POSTs `/api/v1/admin/retention-sweep`; production endpoint is `/api/v1/security/retention/sweep` (`pkg/gateway/rest.go:2149`, handler at `pkg/gateway/rest_retention.go:163`). Fix: update test URL.
4. **`ExecApprovalBlock.tsx:108` already has `data-testid="approval-modal"`** and `:195` already has `data-testid="always-allow-toggle"`. Allowlist entries #106 and #108 are NOT blocked on adding the testid — they are blocked on the test's ability to deterministically trigger a `policy=ask` tool call. The fix is on the test side (set up an agent with `experimental.workspace_shell_enabled=true` + a workspace policy of `ask` for `workspace.shell`, then issue a deterministic prompt) plus persistence wiring for "Always Allow" toggle (verify it actually mutates the per-agent policy).
5. **`/api/v1/version` does not exist.** `pkg/gateway/version_hash_test.go:27` is `t.Skip(…pending implementation…)` and `pkg/gateway/api_e2e_test.go:328` is also skipped. Fixing #110 is a real backend+frontend implementation task, not a wiring fix.
6. **CLAUDE.md context note:** the historical plan files referenced in the draft (`quizzical-marinating-frog.md`, `feedback_release_responsibility.md`) don't exist in this environment. Authoritative facts live in CLAUDE.md hard-constraints + the failing tests themselves.

---

## Failure inventory (categorized after exploration)

| Cat | Count | Tests | Root cause class |
|---|---|---|---|
| **A — Source bug, narrow fix** | 4 | `ws-reconnect:31` (visibilitychange on wrong target), `ws-reconnect:131` (verify online flow ends with input enabled), `iframe-preview-5xx:49`, `iframe-preview-warmup:27`, `serve-iframe:109`, `web-serve-canonical:144` | Component wiring; trace single Playwright run, fix component or test assertion |
| **B — Test bug** | 3 | `agents:89` (text=409 → text=conflicts), `skills:44` (same), `retention:54` + `retention:134` (wrong URL + auth) | Test selector/URL drift; pure test edit |
| **C — Error UI rendering** | 2 | `api-errors:183` (network-timeout typed error), `api-errors:261` (server-500 typed error retry) | Verify ApiError flows through ChatScreen to user-visible toast/inline-block |
| **D — LLM determinism** | 6 | `chat:117` (cancel-stream prompt too short), `media:12` (Max screenshot prompt nondeterministic), `handoff:48`, `subagent:78`, `subagent:150`, `subagent:244` | Tighten prompts, plumb `temperature=0`/`seed=42` to OpenRouter, replace text assertions with structural ones (data-testid presence, attribute checks) |
| **E — Test timing/state machine** | 3 | `multi-turn-render:50`, `replay-fidelity:546`, `settings:54` | DOM state assertions; use `expect.poll` and verify state machine |
| **F — Allowlist clearance** | 10 | #101, #102, #103, #104, #106, #108, #109, #110, #111, #133 | New components + features (see Wave 3 breakdown) |
| **G — Explicit feature-skip (USER APPROVED)** | 2 | `chat:136` (#105 offline queue), `media:50` (#107 mock-media) | Convert from `expect(false).toBe(true)` to `test.skip()` + new SKIP_ALLOWLIST entry, `until: 2026-09-30` |
| **H — Lint** | ~120 | (entire backend) | Re-run `golangci-lint v2.10.1`; per-issue triage (no blanket `//nolint`) |

Total: 30 active fixes + 10 allowlist clearances + 2 explicit feature-skips + 1 lint sweep.

---

## Wave dependency graph

```
                    ┌──────────────┐
                    │ Pre-flight   │  install golangci-lint v2.10.1
                    │ (lead)       │  measure baseline counts
                    └──────┬───────┘
                           │
        ┌──────────────────┼──────────────────────────┐
        │                  │                          │
        ▼                  ▼                          ▼
   ┌──────────┐     ┌────────────────┐         ┌────────────┐
   │ W1.1     │     │ W1.2 IframePrev│         │ W1.3 ws.ts │
   │ Lint     │     │ +serve-iframe  │         │ reconnect  │
   │ (backend)│     │ (frontend)     │         │ (frontend) │
   └──────────┘     └────────────────┘         └────────────┘
        │                  │                          │
        ▼                  ▼                          ▼
   ┌──────────┐     ┌────────────────┐         ┌────────────────┐
   │ W1.4     │     │ W1.5 Error UI  │         │ W1.6 Test-only │
   │ Retention│     │ + agents/skills│         │ + skip-allowlist│
   │ (backend)│     │ (frontend)     │         │ conversions     │
   └──────────┘     └────────────────┘         └────────────────┘
                           │
              ┌────────────┴────────────┐
              ▼                         ▼
        ┌──────────────┐          ┌──────────────┐
        │ W2.1 Subagent│          │ W2.2 Replay/ │
        │ + handoff    │          │ render/timing│
        │ (frontend)   │          │ (frontend)   │
        └──────────────┘          └──────────────┘
                           │
                           ▼
              ┌────────────┴────────────┬────────────────┐
              ▼                         ▼                ▼
        ┌──────────────┐         ┌──────────────┐ ┌───────────────┐
        │ W3.1 UI gap A│         │ W3.2 UI gap B│ │ W3.3 Behavior │
        │ #101,102,104 │         │ #103,108,109 │ │ #111,#133     │
        │ #106         │         │ #110         │ │               │
        └──────────────┘         └──────────────┘ └───────────────┘
                           │
                           ▼
                    ┌──────────────┐
                    │ W4 Integrate │  full CI gate, smoke test, push
                    └──────────────┘
```

Wave 1 = 6 parallel tracks; Wave 2 = 2 parallel tracks; Wave 3 = 3 parallel tracks. Lead serializes integration in W4.

---

## Pre-flight (lead, ~15 min)

1. Install matching toolchain: `go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.10.1`. Verify `golangci-lint version` reports `2.10.1` and `built with go1.26+`.
2. Sync SPA embed before any Go tests:
   ```bash
   npm run build
   rm -rf pkg/gateway/spa
   cp -r dist/spa pkg/gateway/spa
   ```
3. Snapshot the baseline lint count and Playwright failure list to a working file (`/tmp/v01-baseline.txt`) for diff'ing after each wave:
   ```bash
   golangci-lint run --build-tags=goolm,stdjson 2>&1 | tail -200 > /tmp/v01-lint-baseline.txt
   OMNIPUS_HOME=/tmp/omnipus-e2e-baseline npx playwright test --reporter=list 2>&1 | tail -200 > /tmp/v01-pw-baseline.txt
   ```
4. Commit the in-flight session changes (if any) before spawning agents — agents work in worktrees but the shared starting commit must be clean. Do NOT push yet (push happens after each wave per the policy below).

---

## Wave 1 — Independent file ownership (6 agents in parallel, ~4–6h)

Each agent runs in `isolation: "worktree"`. Agents may commit + push to their own worktree branch (`v01-w1.<n>-<slug>`); lead cherry-picks into `feature/iframe-preview-tier13` after the wave.

### W1.1 — Lint cleanup (backend-lead, worktree)

**Scope:** drive `golangci-lint run --build-tags=goolm,stdjson` to 0 issues. Triage each linter family (do NOT blanket-suppress with `//nolint`):

- `unused` (~18 dead identifiers) → delete (agents may NOT add `_ =` underscore-binds as workarounds)
- `predeclared` (~5: `cap`, `max`) → rename
- `errname` (~3) → rename to `Err…` / `…Error` convention
- `unconvert`, `mirror`, `prealloc`, `wastedassign`, `dupl` (~14) → fix per-issue
- `tparallel` (~5) → add `t.Parallel()` or remove the parent's
- `recvcheck` (~1) → unify pointer/value receiver (likely `pkg/sandbox/sandbox.go`)
- `govet` (~33) → field-by-field; common patterns are loopclosure, shadow, printf
- `godoclint` (~34) → fix exported doc comments to start with the identifier name
- `nilerr` (~5) → return the error or document why nil is correct (likely `pkg/config/validator.go`)

**Verification:** `golangci-lint run --build-tags=goolm,stdjson` exits 0. No `//nolint` directives added.

**Files:** broad — touches `pkg/audit/`, `pkg/agent/loop.go` (wastedassign at the `forceCompression` call site), `pkg/config/validator.go` (nilerr), `pkg/sandbox/sandbox.go` (recvcheck), `cmd/omnipus/internal/audit/command.go` (unused exitCodeError), and dozens more. Agent should iterate one linter at a time and re-run after each batch.

### W1.2 — Iframe preview cluster (frontend-lead, worktree)

**Scope:** 4 tests that share `IframePreview.tsx` + `serve-iframe` cross-origin contract. Trace each via `npx playwright test <file> --debug`, then fix.

- `tests/e2e/iframe-preview-5xx.spec.ts:49` — verify `IframePreview.tsx:826-836` `probeStatus === 'server_error'` branch fires. Likely cause: HEAD probe response classification logic in `IframePreview.tsx` doesn't recognize the simulated 503; check `setProbeStatus('server_error')` call site and HEAD-probe routing.
- `tests/e2e/iframe-preview-warmup.spec.ts:27` — verify `IframePreview.tsx:800-808` `warmupPhase === 'error'` text "Dev server did not respond in time" appears after `page.clock.fastForward(62_000)`. Likely cause: `setInterval` not firing under fake clock, or `effectiveTimeout = aboutInfo?.warmup_timeout_seconds ?? 60` not being read from the about response after intercept.
- `tests/e2e/serve-iframe.spec.ts:109` — verify cross-origin contract: iframe `src` matches preview origin, `sandbox` attribute is exact, no `/api/v1/*` requests originate from preview origin during test lifetime.
- `tests/e2e/web-serve-canonical.spec.ts:144` — verify `run_in_workspace` (legacy alias) result also renders the iframe. Likely cause: `ServeWorkspaceUI.tsx` only handles `serve_workspace`, not `run_in_workspace`.

**Files:** `src/components/chat/IframePreview.tsx`, `src/components/chat/tools/ServeWorkspaceUI.tsx`, possibly `pkg/gateway/rest_preview.go::HandleAbout` (if `warmup_timeout_seconds` is missing from response shape).

**Verification:** the 4 specs pass against the embedded SPA at port 6060.

### W1.3 — WebSocket reconnect (frontend-lead, worktree)

**Scope:** 2 tests in `tests/e2e/ws-reconnect.spec.ts`. Single-line source fix + verification.

- `:31 visibilitychange_triggers_reconnect_with_persistent_banner` — change `src/lib/ws.ts:496` from `window.addEventListener('visibilitychange', this._onVisibilityChange)` to `document.addEventListener('visibilitychange', this._onVisibilityChange)` (and matching removeEventListener at `:502`). Verify the ChatScreen banner uses persistent UI element with `data-testid="reconnect-banner"` (test accepts multiple fallback locators).
- `:131 online_event_triggers_reconnect` — `online` event listener is correct (window-scoped per spec). Likely passing once test (a) runs after fix to (1) and (b) banner clears properly. Verify chat input `disabled` state is driven by the `isConnected` flag (check `ChatScreen.tsx` / `MessageInput.tsx`).

**Files:** `src/lib/ws.ts:496,502`. Possibly `src/components/chat/ChatScreen.tsx` for the persistent banner (verify `data-testid="reconnect-banner"` exists; if not, add).

**Verification:** `npx playwright test tests/e2e/ws-reconnect.spec.ts` passes both.

### W1.4 — Retention + auth (backend-lead, worktree)

**Scope:** `tests/e2e/retention.spec.ts` (2 tests).

- `:54 seven_day_old_session_replays_cleanly` — fix two issues:
  1. `page.request.get('/api/v1/sessions')` is unauthenticated → returns 401. Fix by reusing the page-context auth from `global-setup.ts` (storageState provides cookies/headers; pass through with `extraHTTPHeaders`).
  2. Verify `GET /api/v1/sessions` does enumerate session directories on disk, not only those created via REST. Trace `pkg/gateway/rest.go` sessions handler → `pkg/session/UnifiedStore.List()`.
- `:134 session_past_retention_threshold_is_swept` — change test URL from `/api/v1/admin/retention-sweep` to `/api/v1/security/retention/sweep` (handler exists, see `pkg/gateway/rest_retention.go:163`). Add admin-role auth header. Verify retention sweep at gateway boot also catches this case (`pkg/gateway/retention_goroutine_test.go` shows the loop is wired; confirm boot triggers an immediate sweep via `pkg/gateway/gateway.go`).

**Files:** `tests/e2e/retention.spec.ts`, possibly `pkg/gateway/gateway.go` (boot-time sweep trigger if missing), `pkg/session/unified_store.go` (list enumeration if it filters out fixture sessions).

**Verification:** `npx playwright test tests/e2e/retention.spec.ts` passes both.

### W1.5 — Error UI (frontend-lead, worktree)

**Scope:** 4 tests that exercise the `ApiError` flow.

- `tests/e2e/agents.spec.ts:89 (f) name collision` — change locator from `text=409` to `text=conflicts with the current state` (matches `defaultUserMessage(409)` in `src/lib/api-error.ts:50`). Verify the `useUiStore.getState().addToast({ message: err.userMessage, ... })` path at `CreateAgentModal.tsx:124-125` renders the toast.
- `tests/e2e/skills.spec.ts:44 (c) MCP duplicate` — same fix on `McpServerModal`. Verify it uses `isApiError(err) ? err.userMessage : err.message`. If not, migrate.
- `tests/e2e/api-errors.spec.ts:183 network-timeout` — confirm `request()` in `src/lib/api.ts` constructs `new ApiError(0, …)` with `userMessage = 'Network unavailable. Check your connection.'` on transport failure, and that ChatScreen's onError surfaces it.
- `tests/e2e/api-errors.spec.ts:261 server-500 typed error retry` — confirm 5xx → `ApiError(5xx, 'The server is unavailable…')`, and that the retry-button path is wired in the chat error block.

**Files:** `src/components/agents/CreateAgentModal.tsx`, `src/components/skills/McpServerModal.tsx`, `src/components/chat/ChatScreen.tsx`, `src/lib/api.ts`, plus the corresponding test files.

**Verification:** the 4 specs pass.

### W1.6 — Skip-allowlist conversions for #105 + #107 (qa-lead, worktree)

**Scope:** convert the 2 user-approved feature-skip tests from `expect(false).toBe(true)` to `test.skip()` calls plus SKIP_ALLOWLIST entries.

- `tests/e2e/chat.spec.ts:136-154` — replace `expect(false, 'BLOCKED: #105 …').toBe(true)` with `test.skip(true, 'BLOCKED on #105 — offline send queue not implemented; see SKIP_ALLOWLIST')`. Keep the explanatory comment.
- `tests/e2e/media.spec.ts:50-66` — same treatment for #107.
- `tests/e2e/fixtures/skip-tracking.ts:107` — append two new `SKIP_ALLOWLIST` entries:
  - `{ test: '(f) queue-on-disconnect: messages sent during WS disconnect send in order after reconnect', issue: 'https://github.com/dapicom-ai/omnipus/issues/105', until: '2026-09-30', note: '…' }`
  - `{ test: '(b) file-download fallback: large binary request triggers browser download dialog', issue: 'https://github.com/dapicom-ai/omnipus/issues/107', until: '2026-09-30', note: '…' }`
- Update `tests/e2e/fixtures/skip-baseline.json` if needed (set `baseline_unauthorized_skips` so the gate passes).

**Verification:** `npx playwright test tests/e2e/chat.spec.ts tests/e2e/media.spec.ts` reports the 2 tests as skipped (allowed); the run does NOT fail on unauthorized-skip gate.

---

## Wave 2 — Behavior + LLM determinism (2 agents, ~4–6h)

Lead must finish W1 cherry-picks before spawning W2 (Wave 2 work shares files like `ChatScreen.tsx` with W1.5).

### W2.1 — Subagent + handoff cluster (frontend-lead + qa-lead together, worktree)

**Scope:** 4 LLM-determinism tests. Three-pronged fix:

1. **Plumb `temperature=0` and `seed=42` into the OpenRouter provider** — verify `pkg/providers/openrouter/provider.go` request body includes both. If the SDK type doesn't expose `seed`, add as a raw field. Confirm the test-harness `ScenarioProvider` honours these too.
2. **Tighten prompts** in `tests/e2e/handoff.spec.ts`, `tests/e2e/subagent.spec.ts` to remove discretion: e.g. `"Spawn the subagent named 'researcher' NOW. After spawning, reply with the literal string 'spawned'."` instead of "ask the researcher to look into X".
3. **Replace text assertions with structural ones**: assert presence of `[data-testid="subagent-collapsed-block"]`, `[data-testid="subagent-grandchild"]`, attribute counts. Remove every `softSkip(test, 'LLM did not …')` — they currently exit silently and mask non-determinism.

Specific tests: `handoff:48`, `subagent:78` (grandchild refused), `subagent:150` (sibling spawns), `subagent:244` (live step counter).

**Files:** `src/components/chat/SubagentBlock.tsx` (add data-testids if missing), `pkg/providers/openrouter/provider.go`, `tests/e2e/handoff.spec.ts`, `tests/e2e/subagent.spec.ts`.

**Verification:** these 4 specs pass 3 consecutive runs (run with `--repeat-each=3`) without softSkip firing.

### W2.2 — Replay / render / timing (frontend-lead, worktree)

**Scope:** 3 tests that fail on async DOM assertions.

- `tests/e2e/multi-turn-render.spec.ts:50 turn1-badges` — pure DOM, fix waitFor timing. Use `expect.poll` for badge count per turn bubble.
- `tests/e2e/replay-fidelity.spec.ts:546 send-disabled` — verify `MessageInput.tsx` derives `disabled` from `isReplaying` (or the canonical replay flag). Check the state machine in `useChatStore` correctly toggles the flag during replay.
- `tests/e2e/settings.spec.ts:54 keyboard-nav` — Radix Tabs re-render triggers focus loss. Use `page.keyboard.press('Tab')` with `expect.poll` for `document.activeElement` testid.

**Files:** `src/components/chat/ChatScreen.tsx`, `src/components/chat/MessageInput.tsx`, `src/components/settings/SettingsPage.tsx`, the 3 spec files.

**Verification:** these 3 specs pass.

### W2.3 — Chat streaming + media determinism (frontend-lead, worktree, optional split from W2.1)

**Scope:** 2 LLM-determinism tests not in W2.1.

- `tests/e2e/chat.spec.ts:117 cancel-stream` — change test prompt from "Write a very long essay about the history of the internet with at least 500 words" to a deterministic generator: `"Count from 1 to 500, one number per line. Output every number in sequence."` Confirm `MessageInput.tsx` renders `button[aria-label="Stop generation"]` while streaming.
- `tests/e2e/media.spec.ts:12 (a) screenshot inline render` — tighten Max system prompt or test prompt to deterministically invoke screenshot tool. Structural assert on `<img src="/api/v1/media/...">` presence in transcript instead of natural-image-dimension assertions where possible (or relax dim thresholds).

**Files:** `tests/e2e/chat.spec.ts:122-124`, `tests/e2e/media.spec.ts:29`, possibly `pkg/coreagent/core.go` Max prompt section if test prompt alone is insufficient.

**Verification:** these 2 specs pass 3 consecutive runs.

---

## Wave 3 — Allowlist clearance (3 agents in parallel, ~10–14h)

Spawn after W2 is integrated. These tracks largely don't share files.

### W3.1 — UI gap batch A (frontend-lead, worktree)

Issues: #101 locked-fields, #102 deleted-agent-404, #104 dev-mode-bypass-banner, #106 approval-modal trigger.

- **#101 locked fields read-only on core agents:** in `AgentProfile.tsx:353` (the `canEdit` guard hides the Identity accordion entirely). Render the accordion in disabled mode for locked agents so the test can find `data-testid="agent-name-input"` as a disabled input. Update `AgentForm.tsx` similarly.
- **#102 branded 404:** add a `NotFoundPage.tsx` component + route in `src/routes/` for `/agents/:nonexistent-slug`. Must include text matching `Back to Agents` (case-sensitive per allowlist note).
- **#104 dev-mode-bypass banner:** when `/api/v1/state` reports `dev_mode_bypass: true`, render a persistent red banner across every route. New `DevModeBanner.tsx` mounted in `AppShell.tsx`. Use `data-testid="dev-mode-banner"`.
- **#106 approval modal trigger:** ExecApprovalBlock already has the testid (verified at `:108`). Test needs a deterministic `policy=ask` trigger. Set up the test to (a) configure an agent with `experimental.workspace_shell_enabled=true` + per-agent policy `workspace.shell: ask`, (b) issue a deterministic prompt that invokes the tool, (c) assert the modal appears, (d) click Approve and assert the tool ran.

**Files:** `src/components/agents/AgentProfile.tsx`, `src/components/agents/AgentForm.tsx`, `src/components/auth/` + new `DevModeBanner.tsx`, `src/components/app/AppShell.tsx`, `src/routes/` + new `NotFoundPage.tsx`, `tests/e2e/agents.spec.ts`, `tests/e2e/auth.spec.ts`, `tests/e2e/command-center.spec.ts`. Remove the corresponding allowlist entries when each test passes.

### W3.2 — UI gap batch B (frontend-lead, worktree)

Issues: #103 agent-removed-banner, #108 always-allow-toggle persistence, #109 skill-hash-mismatch dialog, #110 version-drift toast (backend + frontend).

- **#103 agent-removed banner:** new `AgentRemovedBanner.tsx`. Wire `ChatScreen.tsx` to read `agent_removed` from session GET response (`pkg/gateway/rest.go` already returns this for sessions of deleted agents — see `TestDeletedAgentSessionReadOnly` in `pkg/gateway/api_e2e_test.go:204`). Render banner with `data-testid="agent-removed-banner"`, disable composer.
- **#108 Always Allow persistence:** the toggle button exists at `ExecApprovalBlock.tsx:194-202`. Verify clicking it invokes `respondToApproval(id, 'always')` and that the per-agent policy is mutated server-side (`pkg/policy/`). After page reload, the same tool call must NOT trigger another approval. May need new `SecuritySection.tsx` listing for "Always Allowed" tools with a revoke button.
- **#109 skill hash-mismatch dialog:** add file input on `/skills` route in `SkillBrowser.tsx`. New `SkillUploadDialog.tsx` that POSTs to skill install endpoint and surfaces the hash-mismatch block dialog when the server returns a hash error.
- **#110 version-drift toast:** **Real backend work** — implement `GET /api/v1/version` endpoint in `pkg/gateway/rest.go` that returns `{ "build_sha": "...", "version": "..." }`. Use Go `runtime/debug.ReadBuildInfo()` for the SHA. Add to the public-routes allow-list. SPA polls on window focus + every 60s; on mismatch from initial load, show non-blocking toast with `data-testid="version-toast"` and "New version available — refresh" copy. Unskip both `pkg/gateway/api_e2e_test.go:328` and `pkg/gateway/version_hash_test.go:27`.

**Files:** `src/components/chat/ChatScreen.tsx` + new `AgentRemovedBanner.tsx`, `src/components/settings/SecuritySection.tsx`, `src/components/skills/SkillBrowser.tsx` + new `SkillUploadDialog.tsx`, `src/components/app/AppShell.tsx` (version polling + toast), `pkg/gateway/rest.go` (new endpoint), `pkg/gateway/version_hash_test.go` + `pkg/gateway/api_e2e_test.go` (unskip), corresponding spec files. Remove allowlist entries on success.

### W3.3 — Behavior batch C (frontend-lead + backend-lead pair, worktree)

Issues: #111 Ray→Max→Jim transcript labels, #133 attach-during-active-turn.

- **#111 handoff transcript labels:** trigger a deterministic Ray→Max→Jim handoff (tighten Ray's prompt and the test prompt). In `AssistantMessage.tsx`, render the per-message `agent_id` as a label/badge. Verify the transcript shows all three labels in order.
- **#133 attach-during-active-turn:** Go-level coverage exists (`TestAttach_RegistersLiveEventsBeforeReplay`). E2E version requires a second browser context that attaches mid-turn and receives both the in-flight events and the replayed history. Trace `pkg/agent/loop.go` and the WS replay path in `pkg/gateway/websocket.go`.

**Files:** `src/components/chat/AssistantMessage.tsx`, possibly `pkg/agent/loop.go` and `pkg/gateway/websocket.go` (race fix between live events and replay), `tests/e2e/handoff.spec.ts`, `tests/e2e/replay-fidelity.spec.ts`. Remove allowlist entries on success.

---

## Wave 4 — Integration + verification (lead, sequential, ~2–3h)

(See Wave 5 below for the full review pipeline that gates "done".)


1. Cherry-pick or merge each worktree's changes into `feature/iframe-preview-tier13`. Most splits are clean (Wave 1 isolates files); expect a small conflict at `ChatScreen.tsx` between W1.5 / W2.2 / W3.2.
2. Re-sync SPA embed (`npm run build && rm -rf pkg/gateway/spa && cp -r dist/spa pkg/gateway/spa`).
3. Run the full CI gate locally:

```bash
# Build gates
CGO_ENABLED=0 go build -tags goolm,stdjson ./...

# Test gates
CGO_ENABLED=0 go test -tags goolm,stdjson,test_harness ./...
CGO_ENABLED=1 go test -race -tags goolm,stdjson,test_harness -count=1 -timeout 240s \
  ./pkg/sysagent/... ./pkg/config/... ./pkg/credentials/... ./pkg/channels/...

# Lint + vuln (use the v2.10.1 binary installed in pre-flight)
~/go/bin/golangci-lint run --build-tags=goolm,stdjson
govulncheck -tags goolm,stdjson ./...

# Frontend
npx tsc --noEmit
npx vitest run

# E2E (rebuild binary first, then point Playwright at the embedded SPA)
CGO_ENABLED=0 go build -o /tmp/omnipus ./cmd/omnipus/
OMNIPUS_HOME=/tmp/omnipus-e2e-final npx playwright test --reporter=list
```

Each line must exit 0. Any failure is a fix, not a defer.

4. Inspect the skip manifest at `test-results/skip-manifest.json`:
   - `unauthorized_skips` = `[]`
   - `allowlisted` = exactly 2 entries (#105 and #107) plus any **net-new** entries (only #105/#107 are user-approved; everything else from the original 10 must be cleared).
5. Smoke-test the rebuilt binary by hand: gateway boots → onboarding → chat round-trip → iframe preview renders.
6. Push `feature/iframe-preview-tier13` to origin (`git push -u origin feature/iframe-preview-tier13`). **Do NOT open a PR or merge** — that requires explicit user demand.

---

## Wave 5 — PR review pipeline + 2 fix-and-re-review rounds (lead orchestrates, ~6–10h)

CI gates green is necessary but not sufficient. Per CLAUDE.md "Review pipeline (run after implementation subagents complete)", we run all 14 review agents in parallel, then fix, then re-review — twice.

**Round 1 (parallel — 14 agents in a single message):**

Architect-tier (8 agents — spawn one per concern, all in parallel):

1. `architect` — cross-cutting Go design review (focus on the new iframe preview + retention sweep + version endpoint)
2. `architect` — frontend integration coherence (focus on AppShell, ChatScreen, AgentProfile changes)
3. `architect` — security/threat-model review (focus on `pkg/sandbox/`, `pkg/audit/`, `pkg/policy/` deltas + the new `/api/v1/version` exposure)
4. `architect` — release-readiness review (CLAUDE.md hard-constraint compliance: pure Go, no CGo, kernel sandbox graceful degradation, deny-by-default)
5. `architect` — test architecture review (Playwright determinism strategy, SKIP_ALLOWLIST hygiene, race coverage scope)
6. `architect` — API contract review (REST + WS endpoints touched, backward compat where applicable for v0.1)
7. `architect` — config + onboarding flow review (any settings/onboarding regressions)
8. `architect` — observability + audit log review (any new events, structured-log key consistency)

PR-review-toolkit tier (6 agents — spawn all in parallel):

9. `pr-review-toolkit:code-reviewer` — CLAUDE.md compliance, bugs, quality
10. `pr-review-toolkit:code-simplifier` — simplify for clarity and maintainability
11. `pr-review-toolkit:comment-analyzer` — verify comment accuracy (especially the heavy comments added in `IframePreview.tsx` and `ws.ts`)
12. `pr-review-toolkit:pr-test-analyzer` — test coverage quality (does each Wave-1/2/3 fix have a regression test?)
13. `pr-review-toolkit:silent-failure-hunter` — find silent failures and bad error handling (especially in W3.2 version-drift toast and W3.3 attach-during-active-turn)
14. `pr-review-toolkit:type-design-analyzer` — type/interface design quality (focus on any new types added in W3.x)

**Round 1 fix pass (lead, sequential):**

- Triage every finding: severity (CRIT / HIGH / MED / LOW), category (bug / smell / nit), and which subagent type owns the fix.
- Spawn the relevant implementing subagent (`backend-lead`, `frontend-lead`, `security-lead`, `qa-lead`) per cluster of findings, in parallel where files don't overlap.
- After fixes land, re-run the full CI gate from Wave 4 step 3. Any new failures introduced by the fixes are fixed before proceeding.
- Commit fixes as `fix(v0.1): Wave 5 round-1 review findings — <area>`. Push.

**Round 2 (parallel — same 14 agents, fresh context):**

Spawn the same 14 agents again, in parallel, each given the round-1 findings list + a directive: "verify these are addressed AND look for regressions introduced by the fixes." This catches: (a) findings that were marked "fixed" but weren't, and (b) new issues the fix pass introduced.

**Round 2 fix pass (lead, sequential):**

- Same workflow as round-1 fix pass.
- After fixes, re-run the full CI gate from Wave 4 step 3.
- Commit as `fix(v0.1): Wave 5 round-2 review findings — <area>`. Push.

**Exit criterion:**

After round 2 is closed, branch is "ship-ready" only if:
- Every CI gate from Wave 4 step 3 still exits clean
- Skip manifest still shows exactly 2 authorized skips (#105 + #107) and zero unauthorized
- All round-2 review findings are closed (either fixed, or downgraded to follow-up issues with explicit user approval per CLAUDE.md hard-constraint #7)
- Manual smoke test still passes

If a third round of issues surfaces during round 2, escalate to the user with a punch list rather than silently spawning a round 3 — the user owns the decision to keep cycling vs. accept residual findings as tracked follow-ups.

---

## Critical files (master list)

**Backend** (paths verified during exploration):
- `pkg/audit/audit.go`, `pkg/agent/loop.go` (W1.1 wastedassign + W3.3), `pkg/config/validator.go` (W1.1 nilerr), `pkg/coreagent/core.go` (W2.3 Max prompt), `pkg/gateway/rest.go` (W3.2 `/api/v1/version`), `pkg/gateway/rest_retention.go` (already exists), `pkg/providers/openrouter/provider.go` (W2.1 temp/seed), `pkg/sandbox/sandbox.go` (W1.1 recvcheck), `pkg/agent/testutil/gateway_harness.go` (already updated this session), `pkg/gateway/websocket.go` (W3.3 attach race), `cmd/omnipus/internal/audit/command.go` (W1.1 unused).

**Frontend** (paths verified):
- `src/lib/ws.ts:496,502` (W1.3 visibilitychange target fix)
- `src/lib/api.ts`, `src/lib/api-error.ts` (already correct — only W1.5 test selectors need adjustment)
- `src/components/chat/IframePreview.tsx` (W1.2)
- `src/components/chat/tools/ServeWorkspaceUI.tsx` (W1.2)
- `src/components/chat/ChatScreen.tsx` (W1.3 banner, W1.5 ApiError, W2.2 replay state, W3.2 agent-removed)
- `src/components/chat/MessageInput.tsx` (W2.2 replay-disabled)
- `src/components/chat/SubagentBlock.tsx` (W2.1 testids)
- `src/components/chat/AssistantMessage.tsx` (W3.3 per-agent label)
- `src/components/chat/ExecApprovalBlock.tsx` (already has testids — W3.1 only needs deterministic trigger; W3.2 needs persistence wiring)
- `src/components/agents/CreateAgentModal.tsx` (already correct — only W1.5 test fix; W3.1 #101 needs AgentProfile/AgentForm changes)
- `src/components/agents/AgentProfile.tsx`, `AgentForm.tsx` (W3.1)
- `src/components/skills/McpServerModal.tsx` (W1.5)
- `src/components/skills/SkillBrowser.tsx` + new `SkillUploadDialog.tsx` (W3.2)
- `src/components/settings/SettingsPage.tsx` (W2.2 keyboard nav)
- `src/components/settings/SecuritySection.tsx` (W3.2 #108)
- `src/components/auth/` + new `DevModeBanner.tsx` (W3.1 #104)
- `src/components/app/AppShell.tsx` (W3.1, W3.2 version drift)
- `src/routes/` + new `NotFoundPage.tsx` (W3.1 #102)

**Tests + fixtures:**
- All affected `tests/e2e/*.spec.ts` listed per wave above
- `tests/e2e/fixtures/skip-tracking.ts` — W1.6 adds 2 new entries; W3.1–3.3 remove 10 cleared entries
- `tests/e2e/fixtures/skip-baseline.json` — update only after final wave to match the new authorized count

**Config:**
- `go.mod` (already at `go 1.26.2`)
- `.github/workflows/pr.yml` (already updated this session)

---

## Reused functions / patterns (do NOT reinvent)

- `testutil.StartTestGateway` (`pkg/agent/testutil/gateway_harness.go:145`) and `seedTestCredentials` (added this session) — use directly for W1.4, W3.1 #106 backend setup.
- `gateway.SetTestProviderOverride` (`pkg/gateway/testhook.go`) — already wired under `test_harness` tag; reuse for ScenarioProvider injection in W2.1.
- `ApiError` (`src/lib/api-error.ts`) and `isApiError` predicate — reuse everywhere; do not introduce new error types in W1.5.
- `softSkip` and `SKIP_ALLOWLIST` (`tests/e2e/fixtures/skip-tracking.ts`) — already wired with validation + manifest writer; W1.6 just adds entries, doesn't extend the API.
- `seedAndOpenSession` fixture (`tests/e2e/fixtures/session-setup.ts`) — used by all iframe-preview tests; reuse for any new session-state tests.
- `defaultUserMessage(status)` (`src/lib/api-error.ts:45`) — already maps every relevant HTTP code to a user-friendly string; W1.5 test fixes pivot on these strings.
- `data-testid` selector pattern — already pervasive; add only specific testids called out per wave.

---

## Commit + push policy (per existing branch convention)

- Each wave lands as one or more commits with messages like `fix(v0.1): Wave 1 lint cleanup` / `fix(v0.1): Wave 1.3 ws.ts visibilitychange target`.
- `git push -u origin feature/iframe-preview-tier13` after each wave so progress survives session boundaries.
- **Hard line:** no PRs. `gh pr create / close / merge` and force-pushes require explicit user demand.

---

## Verification checklist (final, before declaring done)

- [ ] `golangci-lint run --build-tags=goolm,stdjson` exits 0
- [ ] `govulncheck -tags goolm,stdjson ./...` exits 0
- [ ] `CGO_ENABLED=0 go test -tags goolm,stdjson,test_harness ./...` exits 0
- [ ] `CGO_ENABLED=1 go test -race -tags goolm,stdjson,test_harness ./pkg/sysagent/... ./pkg/config/... ./pkg/credentials/... ./pkg/channels/...` exits 0
- [ ] `npx tsc --noEmit` exits 0
- [ ] `npx vitest run` exits 0
- [ ] `OMNIPUS_HOME=/tmp/omnipus-e2e-final npx playwright test` exits 0 with skip manifest showing exactly 2 authorized skips (#105 + #107) and zero unauthorized skips
- [ ] Manual smoke: gateway boot → onboarding → chat round-trip → iframe preview renders
- [ ] **Wave 5 round 1:** all 14 review agents have run; all findings triaged + fixed (or explicitly deferred with user approval); CI re-runs clean
- [ ] **Wave 5 round 2:** all 14 review agents have run again; all new findings closed; CI re-runs clean
- [ ] Branch pushed to `origin/feature/iframe-preview-tier13`; no PR opened
