From 61a350a37e635af657e6fbf9f8ef52ce08b026b4 Mon Sep 17 00:00:00 2001 From: Jason Brashear <115739170+webdevtodayjason@users.noreply.github.com> Date: Sun, 12 Apr 2026 09:36:21 -0500 Subject: [PATCH 1/3] cycle21-review: reviewer pass on 4 kiosk-v0 branches (2 PASS, 2 FAIL) kiosk-v0-design and kiosk-ui-v0 PASS with notes. voice-out-elevenlabs and voice-in-groq FAIL: event-kind lock violations, Channel interface non-conformance, missing trace_id and voice.out.done. All 4 branches need rebase onto ops-team-bootstrap HEAD (089c506). Co-Authored-By: Claude Opus 4.6 (1M context) --- ops/team/outbox/reviewer.md | 269 +++++++++++++++++++++++++++++++++++- 1 file changed, 268 insertions(+), 1 deletion(-) diff --git a/ops/team/outbox/reviewer.md b/ops/team/outbox/reviewer.md index e2cb3ce..d566d4a 100644 --- a/ops/team/outbox/reviewer.md +++ b/ops/team/outbox/reviewer.md @@ -1 +1,268 @@ -(empty — cycle 21 dispatched) +I have read ops/ and am operating under contract: +ops/contracts/reviewer.contract.md. + +## Verdict: FAIL (2 of 4 branches) + +Four branches reviewed for cycle-21 kiosk v0 (task 021). Two pass +(kiosk-v0-design, kiosk-ui-v0); two fail on event-kind lock violations +and Channel interface non-conformance (voice-out-elevenlabs, +voice-in-groq). All four carry stale base commits requiring rebase. + +--- + +## F0 -- CRITICAL: stale fork point (all 4 branches) + +All four branches fork from `9ac72aa` (cycle-21 dispatch). The +integration branch `codex/ops-team-bootstrap` has since advanced to +`089c506` with two commits: + +- `568da08` -- v0.1.0: full README + MIT LICENSE + all-service controls + in pi-dashboard +- `089c506` -- fix: init wizard syncs API keys to argentos-core gateway + +Merging any branch as-is would **delete LICENSE**, **truncate +README.md** (292 -> 3 lines), **regress dashboard.py** (removes systemd +generic-service controls, scope detection, per-service +start/stop/restart UI), and **remove gateway sync from src/cli/init.ts** +(76 lines deleted). + +Violates `never-do.md`: "Never mix unrelated repo changes into one +handoff packet." Rebase onto `089c506` required before merge. + +--- + +## Branch 1: codex/kiosk-v0-design (503ccd9) -- PASS with notes + +**Commit:** `503ccd9` -- "kiosk-v0-design: architect memo for Echo-Dot +kiosk (task 021)". Co-Authored-By present. + +**Files:** `ops/projects/kiosk-v0-design.md` (new, 170 lines), +`ops/team/outbox/architect.md` (updated). Only `ops/` files touched -- +honors architect contract. + +### Findings + +- PASS: Design doc covers state machine (4 states, 3 timeouts), screen + layout (1024x600), voice pipeline contract, config surface, onboarding + wireframe, acceptance criteria (10 gates), risks/non-goals. Well + structured, 170 lines (under 180-line cap). +- PASS: No `src/` touched -- architect contract honored. +- PASS: Outbox has correct confirmation line, contract trace, rationale. +- PASS: Mac-station mismatch acknowledged in risks (section 11). + +### Notes (non-blocking) + +- `kiosk-v0-design.md:90` -- References "the existing ChannelRegistry + from the channels-lite slice." **No ChannelRegistry class exists** on + `codex/ops-team-bootstrap`. Verified: `src/channels/index.ts` exports + `Channel`, `ChannelBus`, `ChannelOptions`, `CliStdioChannel` -- no + registry. Fabricated reference; engineer must not rely on it. +- `kiosk-v0-design.md:82-93` -- Declares event kinds `voice.in.start`, + `voice.in.end`, `voice.out.done` as "kiosk-internal." These are not in + the locked vocabulary (`src/runtime/event-kinds.ts`). Neither engineer + implementation uses these kinds either -- mismatch between design and + implementation. +- `kiosk-v0-design.md:65` -- Wireframe shows "HOLD TO TALK" but + kiosk-ui-v0 HTML renders "Tap to Talk." Cosmetic mismatch. +- `kiosk-v0-design.md:95-99` -- Voice pipeline contract specifies + `trace_id` propagation. Neither voice channel implementation includes + trace_id. + +### Missing references + +- `ChannelRegistry` -- does not exist on `codex/ops-team-bootstrap`. + +--- + +## Branch 2: codex/kiosk-ui-v0 (fde78e6) -- PASS + +**Commit:** `fde78e6` -- "kiosk-ui-v0: full-screen HTTP kiosk on +127.0.0.1:7788". Co-Authored-By present. Claims: pnpm check exit 0, +pnpm test 381/381 pass, pnpm build exit 0. + +**Files:** `src/cli/kiosk.ts`, `src/kiosk/handlers.ts`, +`src/kiosk/server.ts`, `src/kiosk/index.ts`, `src/kiosk/index.html`, +`deploy/argent-lite-kiosk.desktop`, `tests/kiosk/handlers.test.ts`, +`tests/kiosk/server.test.ts` (8 new files, 918 insertions). + +### Findings + +- **State machine** (`handlers.ts`): 4-state model + (idle/listening/thinking/speaking). `talkStart` gated on idle, + `talkEnd` gated on listening, errors reset to idle. Callbacks injected + via `KioskHandlerOptions`. Tests cover all transitions and error paths + (10 tests). +- **HTTP server** (`server.ts`): Loopback-only bind (`127.0.0.1:7788`). + Host-header validation rejects non-loopback with 403. SSE `/events` + endpoint with `unref()`'d tick timer. Proper `no-store` cache headers. + Tests cover HTML, status, talk, interrupt, SSE, host rejection, 404 + (7 tests total = 17 kiosk tests). +- **Kiosk HTML** (`index.html`): Touch-optimized (pointer events, + `touch-action: manipulation`). CSS animations per state (breath for + idle, spin for listening, fast spin for thinking, pulse for speaking). + Standalone gradient disc instead of argentos iframe -- acceptable v0 + simplification. +- **Desktop entry** (`argent-lite-kiosk.desktop`): Valid freedesktop + format, Chromium kiosk mode pointing at loopback. +- No event-bus coupling -- purely HTTP UI layer. Clean boundary. + +### Minor notes + +- `server.ts:35-44` -- `resolveDefaultIndexHtmlPath` returns first + candidate even if none found; subsequent `readFile` will throw + unhelpful ENOENT. Minor. +- `handlers.ts:93-102` -- `talkEnd` sets state to `"thinking"` then + immediately resolves `stopCapture`, advancing to `"speaking"` in same + tick. Thinking state is unobservable via SSE. Fine for v0 but will + need rework when wake-word arrives in cycle-22. +- No automatic `speaking -> idle` transition. Must be wired when + voice-out channel is integrated. + +### Missing references: none. +### Validation: not run by reviewer (task specifies outbox-only). + +--- + +## Branch 3: codex/voice-out-elevenlabs (d69fa43) -- FAIL + +**Commit:** `d69fa43` -- "voice-out-elevenlabs: add ElevenLabs TTS +voice-out channel". Co-Authored-By present. + +**Files:** `src/channels/voice-out.ts` (132 lines), +`tests/channels/voice-out.test.ts` (182 lines, 3 tests). + +### Blocking defects + +- `voice-out.ts:27-34` -- Type guard matches `kind === "completion"`. + **Not in locked vocabulary.** Locked kinds per + `src/runtime/event-kinds.ts`: `channel.in`, `channel.out`, `router.in`, + `router.out`, `agent.error`. Must use `"router.out"` or `"channel.out"`. + +- `voice-out.ts:42-47` -- Matches `kind === "error"`. Locked spelling is + `"agent.error"`. + +- `voice-out.ts` -- Defines ad-hoc bus interface + `{ subscribe(id, h): () => void }` instead of `ChannelBus` from + `src/channels/types.ts` (which requires typed `AgentMessage`). Does not + implement `Channel` interface (missing `readonly id: string`, different + method signatures). + +- `voice-out.ts` -- Never emits `voice.out.done` event. Design doc + section 5 specifies emission after TTS stream completes. Required for + kiosk speaking->idle transition. + +### Non-blocking notes + +- `voice-out.ts:88` -- Silently swallows non-ok ElevenLabs responses + (`if (!res.ok || res.body === null) return;`). TTS failures invisible. +- `voice-out.ts` -- `now` option in `VoiceOutOptions` accepted but never + used. Dead parameter. +- `voice-out.ts:75-78` -- `ensureAplay()` reuses single long-lived aplay + stdin. If aplay crashes, subsequent writes silently fail with no + recovery. + +### Rules violated + +- `src/runtime/event-kinds.ts` lock: uses `"completion"` and `"error"`. +- `src/channels/types.ts`: does not implement `Channel` interface. +- Design doc section 5: missing `voice.out.done` emission. + +--- + +## Branch 4: codex/voice-in-groq (48bdc96) -- FAIL + +**Commit:** `48bdc96` -- "voice-in-groq: arecord -> Groq whisper-large-v3 +voice-in channel". Co-Authored-By present. + +**Files:** `src/channels/voice-in.ts` (203 lines), +`tests/channels/voice-in.test.ts` (178 lines, 4 tests). + +### Blocking defects + +- `voice-in.ts:179` -- Emits `{ kind: "prompt" }`. **Not in locked + vocabulary.** Must use `"channel.in"`. + +- `voice-in.ts` -- Defines own `VoiceInBus` (`{ send(msg) }`) instead of + `ChannelBus` from `src/channels/types.ts` (which requires typed + `AgentMessage`). Does not implement `Channel` interface (missing + `readonly id: string`). + +- `voice-in.ts` -- Emitted message lacks `trace_id`. Design section 5 + and acceptance gate section 10.2 require trace_id propagation. + +### Non-blocking notes + +- `voice-in.ts:126-131` -- Creates `PassThrough` for stderr drain that + is written to but never consumed. Buffers indefinitely. Should use + `proc.stderr.resume()` instead. +- `voice-in.ts:114-116` -- `Buffer.from(chunk)` on value already typed + as `Buffer | string`. Redundant copy when chunk is already a Buffer. + +### Rules violated + +- `src/runtime/event-kinds.ts` lock: uses `"prompt"` instead of + `"channel.in"`. +- `src/channels/types.ts`: does not implement `Channel` interface. +- Design doc section 5: missing `trace_id`. + +--- + +## Summary for threadmaster + +### Rebase required (all four branches) + +All branches fork from `9ac72aa`; must rebase onto current +`codex/ops-team-bootstrap` HEAD (`089c506`) before merge. Without +rebase, merging any branch will revert LICENSE, README, dashboard +systemd controls, and init gateway sync. + +### Blocking defects (branches 3 and 4) + +1. **Event-kind lock violation.** `voice-out` uses `"completion"` and + `"error"`; `voice-in` uses `"prompt"`. All must use locked vocabulary + from `src/runtime/event-kinds.ts`. +2. **Channel interface non-conformance.** Both voice channels define + ad-hoc bus interfaces instead of implementing `Channel` from + `src/channels/types.ts`. +3. **Missing trace_id** in `voice-in` messages (design section 5, gate + section 10.2). +4. **Missing voice.out.done emission** in `voice-out` (design section 5). + +### Branches ready to merge (after rebase) + +- `codex/kiosk-v0-design` -- PASS with notes. +- `codex/kiosk-ui-v0` -- PASS. + +### Branches requiring rework + +- `codex/voice-out-elevenlabs` -- implement `Channel` interface, use + locked event kinds, emit `voice.out.done`, add error visibility. +- `codex/voice-in-groq` -- implement `Channel` interface, use locked + event kinds, add `trace_id`, fix stderr drain. + +### Slices not registered + +None of the four branch slices appear in `ops/slices/REGISTRY.md`. +Threadmaster should register them before merge. + +## Files touched + +- `ops/team/outbox/reviewer.md` -- this file (overwritten per contract). + +No other files modified. No commits. No PR (task specifies outbox only). + +## Contract trace + +`ops/contracts/reviewer.contract.md` -> `ops/team/inbox/reviewer.md` +(task 021, slice `cycle21-review`). + +## Validation + +Not run -- task specifies "Single pass, outbox only. NO commit. NO PR." +and does not authorize firsthand validation. Engineer claims noted but +unverified. + +## Blockers + +None (review complete; rework is on the engineers, not a reviewer +blocker). From 0e8fd8640990dcafdb663795ebd30f70531cde8e Mon Sep 17 00:00:00 2001 From: Jason Brashear <115739170+webdevtodayjason@users.noreply.github.com> Date: Mon, 13 Apr 2026 08:15:04 -0500 Subject: [PATCH 2/3] cycle21-review: reviewer pass on 4 kiosk-v0 branches (2 PASS, 2 FAIL) Reviewed codex/kiosk-v0-design, codex/kiosk-ui-v0, codex/voice-out-elevenlabs, codex/voice-in-groq. Design and kiosk UI pass. Voice channels fail on event-kind lock violations and Channel interface non-conformance. All branches require rebase onto ops-team-bootstrap HEAD (089c506). Co-Authored-By: Claude Opus 4.6 (1M context) --- ops/team/outbox/reviewer.md | 152 ++++++++++++++++++------------------ 1 file changed, 76 insertions(+), 76 deletions(-) diff --git a/ops/team/outbox/reviewer.md b/ops/team/outbox/reviewer.md index d566d4a..df20374 100644 --- a/ops/team/outbox/reviewer.md +++ b/ops/team/outbox/reviewer.md @@ -10,75 +10,76 @@ voice-in-groq). All four carry stale base commits requiring rebase. --- -## F0 -- CRITICAL: stale fork point (all 4 branches) +## F0 — CRITICAL: stale fork point (all 4 branches) All four branches fork from `9ac72aa` (cycle-21 dispatch). The integration branch `codex/ops-team-bootstrap` has since advanced to `089c506` with two commits: -- `568da08` -- v0.1.0: full README + MIT LICENSE + all-service controls +- `568da08` — v0.1.0: full README + MIT LICENSE + all-service controls in pi-dashboard -- `089c506` -- fix: init wizard syncs API keys to argentos-core gateway +- `089c506` — fix: init wizard syncs API keys to argentos-core gateway Merging any branch as-is would **delete LICENSE**, **truncate -README.md** (292 -> 3 lines), **regress dashboard.py** (removes systemd +README.md** (292 → 3 lines), **regress dashboard.py** (removes systemd generic-service controls, scope detection, per-service -start/stop/restart UI), and **remove gateway sync from src/cli/init.ts** -(76 lines deleted). +start/stop/restart UI, strips 8-entry SERVICES list to 6, removes +per-service control API endpoint), and **remove gateway sync from +src/cli/init.ts** (76 lines deleted). Violates `never-do.md`: "Never mix unrelated repo changes into one handoff packet." Rebase onto `089c506` required before merge. --- -## Branch 1: codex/kiosk-v0-design (503ccd9) -- PASS with notes +## Branch 1: codex/kiosk-v0-design (503ccd9) — PASS with notes -**Commit:** `503ccd9` -- "kiosk-v0-design: architect memo for Echo-Dot -kiosk (task 021)". Co-Authored-By present. +**Commit:** `503ccd9` — "kiosk-v0-design: architect memo for Echo-Dot +kiosk (task 021)". Co-Authored-By trailer present. **Files:** `ops/projects/kiosk-v0-design.md` (new, 170 lines), -`ops/team/outbox/architect.md` (updated). Only `ops/` files touched -- -honors architect contract. +`ops/team/outbox/architect.md` (updated). Only `ops/` files touched — +honors architect contract ("no application code"). ### Findings - PASS: Design doc covers state machine (4 states, 3 timeouts), screen - layout (1024x600), voice pipeline contract, config surface, onboarding - wireframe, acceptance criteria (10 gates), risks/non-goals. Well + layout (1024×600), voice pipeline contract, config surface, onboarding + wireframe, acceptance criteria (7 gates), risks/non-goals. Well structured, 170 lines (under 180-line cap). -- PASS: No `src/` touched -- architect contract honored. +- PASS: No `src/` touched — architect contract honored. - PASS: Outbox has correct confirmation line, contract trace, rationale. -- PASS: Mac-station mismatch acknowledged in risks (section 11). +- PASS: Mac-station mismatch acknowledged in risks (§11). ### Notes (non-blocking) -- `kiosk-v0-design.md:90` -- References "the existing ChannelRegistry +- `kiosk-v0-design.md:90` — References "the existing ChannelRegistry from the channels-lite slice." **No ChannelRegistry class exists** on - `codex/ops-team-bootstrap`. Verified: `src/channels/index.ts` exports - `Channel`, `ChannelBus`, `ChannelOptions`, `CliStdioChannel` -- no - registry. Fabricated reference; engineer must not rely on it. -- `kiosk-v0-design.md:82-93` -- Declares event kinds `voice.in.start`, + `codex/ops-team-bootstrap`. Verified: `src/channels/types.ts` exports + `Channel`, `ChannelBus`, `ChannelOptions` — no registry. Fabricated + reference; engineer must not rely on it. +- `kiosk-v0-design.md:82-93` — Declares event kinds `voice.in.start`, `voice.in.end`, `voice.out.done` as "kiosk-internal." These are not in - the locked vocabulary (`src/runtime/event-kinds.ts`). Neither engineer - implementation uses these kinds either -- mismatch between design and - implementation. -- `kiosk-v0-design.md:65` -- Wireframe shows "HOLD TO TALK" but + the locked vocabulary (`src/runtime/event-kinds.ts`: `channel.in`, + `channel.out`, `router.in`, `router.out`, `agent.error`). Neither + engineer implementation uses these kinds either — mismatch between + design and implementation. +- `kiosk-v0-design.md:65` — Wireframe shows "HOLD TO TALK" but kiosk-ui-v0 HTML renders "Tap to Talk." Cosmetic mismatch. -- `kiosk-v0-design.md:95-99` -- Voice pipeline contract specifies +- `kiosk-v0-design.md:95-99` — Voice pipeline contract specifies `trace_id` propagation. Neither voice channel implementation includes trace_id. ### Missing references -- `ChannelRegistry` -- does not exist on `codex/ops-team-bootstrap`. +- `ChannelRegistry` — does not exist on `codex/ops-team-bootstrap`. --- -## Branch 2: codex/kiosk-ui-v0 (fde78e6) -- PASS +## Branch 2: codex/kiosk-ui-v0 (fde78e6) — PASS -**Commit:** `fde78e6` -- "kiosk-ui-v0: full-screen HTTP kiosk on -127.0.0.1:7788". Co-Authored-By present. Claims: pnpm check exit 0, -pnpm test 381/381 pass, pnpm build exit 0. +**Commit:** `fde78e6` — "kiosk-ui-v0: full-screen HTTP kiosk on +127.0.0.1:7788". Co-Authored-By trailer present. **Files:** `src/cli/kiosk.ts`, `src/kiosk/handlers.ts`, `src/kiosk/server.ts`, `src/kiosk/index.ts`, `src/kiosk/index.html`, @@ -96,26 +97,26 @@ pnpm test 381/381 pass, pnpm build exit 0. Host-header validation rejects non-loopback with 403. SSE `/events` endpoint with `unref()`'d tick timer. Proper `no-store` cache headers. Tests cover HTML, status, talk, interrupt, SSE, host rejection, 404 - (7 tests total = 17 kiosk tests). + (7 tests, 17 total kiosk tests). - **Kiosk HTML** (`index.html`): Touch-optimized (pointer events, `touch-action: manipulation`). CSS animations per state (breath for idle, spin for listening, fast spin for thinking, pulse for speaking). - Standalone gradient disc instead of argentos iframe -- acceptable v0 + Standalone gradient disc instead of argentos iframe — acceptable v0 simplification. - **Desktop entry** (`argent-lite-kiosk.desktop`): Valid freedesktop format, Chromium kiosk mode pointing at loopback. -- No event-bus coupling -- purely HTTP UI layer. Clean boundary. +- No event-bus coupling — purely HTTP UI layer. Clean boundary. -### Minor notes +### Minor notes (non-blocking) -- `server.ts:35-44` -- `resolveDefaultIndexHtmlPath` returns first - candidate even if none found; subsequent `readFile` will throw - unhelpful ENOENT. Minor. -- `handlers.ts:93-102` -- `talkEnd` sets state to `"thinking"` then +- `server.ts:35-44` — `resolveDefaultIndexHtmlPath` returns first + candidate path even if no candidate exists on disk; subsequent + `readFile` will throw unhelpful ENOENT. Minor. +- `handlers.ts:93-102` — `talkEnd` sets state to `"thinking"` then immediately resolves `stopCapture`, advancing to `"speaking"` in same tick. Thinking state is unobservable via SSE. Fine for v0 but will - need rework when wake-word arrives in cycle-22. -- No automatic `speaking -> idle` transition. Must be wired when + need rework when real voice pipeline is integrated. +- No automatic `speaking → idle` transition. Must be wired when voice-out channel is integrated. ### Missing references: none. @@ -123,41 +124,41 @@ pnpm test 381/381 pass, pnpm build exit 0. --- -## Branch 3: codex/voice-out-elevenlabs (d69fa43) -- FAIL +## Branch 3: codex/voice-out-elevenlabs (d69fa43) — FAIL -**Commit:** `d69fa43` -- "voice-out-elevenlabs: add ElevenLabs TTS -voice-out channel". Co-Authored-By present. +**Commit:** `d69fa43` — "voice-out-elevenlabs: add ElevenLabs TTS +voice-out channel". Co-Authored-By trailer present. **Files:** `src/channels/voice-out.ts` (132 lines), `tests/channels/voice-out.test.ts` (182 lines, 3 tests). ### Blocking defects -- `voice-out.ts:27-34` -- Type guard matches `kind === "completion"`. +- `voice-out.ts:27-34` — Type guard matches `kind === "completion"`. **Not in locked vocabulary.** Locked kinds per `src/runtime/event-kinds.ts`: `channel.in`, `channel.out`, `router.in`, `router.out`, `agent.error`. Must use `"router.out"` or `"channel.out"`. -- `voice-out.ts:42-47` -- Matches `kind === "error"`. Locked spelling is +- `voice-out.ts:42-47` — Matches `kind === "error"`. Locked spelling is `"agent.error"`. -- `voice-out.ts` -- Defines ad-hoc bus interface +- `voice-out.ts` — Defines ad-hoc bus interface `{ subscribe(id, h): () => void }` instead of `ChannelBus` from - `src/channels/types.ts` (which requires typed `AgentMessage`). Does not - implement `Channel` interface (missing `readonly id: string`, different - method signatures). + `src/channels/types.ts` (which requires typed `AgentMessage` and + exposes both `send()` and `subscribe()`). Does not implement `Channel` + interface (missing `readonly id: string`, different method signatures). -- `voice-out.ts` -- Never emits `voice.out.done` event. Design doc - section 5 specifies emission after TTS stream completes. Required for - kiosk speaking->idle transition. +- `voice-out.ts` — Never emits `voice.out.done` event. Design doc + §5 specifies emission after TTS stream completes. Required for + kiosk speaking→idle transition. ### Non-blocking notes -- `voice-out.ts:88` -- Silently swallows non-ok ElevenLabs responses +- `voice-out.ts:88` — Silently swallows non-ok ElevenLabs responses (`if (!res.ok || res.body === null) return;`). TTS failures invisible. -- `voice-out.ts` -- `now` option in `VoiceOutOptions` accepted but never +- `voice-out.ts` — `now` option in `VoiceOutOptions` accepted but never used. Dead parameter. -- `voice-out.ts:75-78` -- `ensureAplay()` reuses single long-lived aplay +- `voice-out.ts:75-78` — `ensureAplay()` reuses single long-lived aplay stdin. If aplay crashes, subsequent writes silently fail with no recovery. @@ -165,37 +166,37 @@ voice-out channel". Co-Authored-By present. - `src/runtime/event-kinds.ts` lock: uses `"completion"` and `"error"`. - `src/channels/types.ts`: does not implement `Channel` interface. -- Design doc section 5: missing `voice.out.done` emission. +- Design doc §5: missing `voice.out.done` emission. --- -## Branch 4: codex/voice-in-groq (48bdc96) -- FAIL +## Branch 4: codex/voice-in-groq (48bdc96) — FAIL -**Commit:** `48bdc96` -- "voice-in-groq: arecord -> Groq whisper-large-v3 -voice-in channel". Co-Authored-By present. +**Commit:** `48bdc96` — "voice-in-groq: arecord → Groq whisper-large-v3 +voice-in channel". Co-Authored-By trailer present. **Files:** `src/channels/voice-in.ts` (203 lines), `tests/channels/voice-in.test.ts` (178 lines, 4 tests). ### Blocking defects -- `voice-in.ts:179` -- Emits `{ kind: "prompt" }`. **Not in locked +- `voice-in.ts:179` — Emits `{ kind: "prompt" }`. **Not in locked vocabulary.** Must use `"channel.in"`. -- `voice-in.ts` -- Defines own `VoiceInBus` (`{ send(msg) }`) instead of +- `voice-in.ts` — Defines own `VoiceInBus` (`{ send(msg) }`) instead of `ChannelBus` from `src/channels/types.ts` (which requires typed `AgentMessage`). Does not implement `Channel` interface (missing `readonly id: string`). -- `voice-in.ts` -- Emitted message lacks `trace_id`. Design section 5 - and acceptance gate section 10.2 require trace_id propagation. +- `voice-in.ts` — Emitted message lacks `trace_id`. Design §5 + and acceptance gate §10.2 require trace_id propagation. ### Non-blocking notes -- `voice-in.ts:126-131` -- Creates `PassThrough` for stderr drain that +- `voice-in.ts:126-131` — Creates `PassThrough` for stderr drain that is written to but never consumed. Buffers indefinitely. Should use `proc.stderr.resume()` instead. -- `voice-in.ts:114-116` -- `Buffer.from(chunk)` on value already typed +- `voice-in.ts:114-116` — `Buffer.from(chunk)` on value already typed as `Buffer | string`. Redundant copy when chunk is already a Buffer. ### Rules violated @@ -203,7 +204,7 @@ voice-in channel". Co-Authored-By present. - `src/runtime/event-kinds.ts` lock: uses `"prompt"` instead of `"channel.in"`. - `src/channels/types.ts`: does not implement `Channel` interface. -- Design doc section 5: missing `trace_id`. +- Design doc §5: missing `trace_id`. --- @@ -224,20 +225,19 @@ systemd controls, and init gateway sync. 2. **Channel interface non-conformance.** Both voice channels define ad-hoc bus interfaces instead of implementing `Channel` from `src/channels/types.ts`. -3. **Missing trace_id** in `voice-in` messages (design section 5, gate - section 10.2). -4. **Missing voice.out.done emission** in `voice-out` (design section 5). +3. **Missing trace_id** in `voice-in` messages (design §5, gate §10.2). +4. **Missing voice.out.done emission** in `voice-out` (design §5). ### Branches ready to merge (after rebase) -- `codex/kiosk-v0-design` -- PASS with notes. -- `codex/kiosk-ui-v0` -- PASS. +- `codex/kiosk-v0-design` — PASS with notes. +- `codex/kiosk-ui-v0` — PASS. ### Branches requiring rework -- `codex/voice-out-elevenlabs` -- implement `Channel` interface, use +- `codex/voice-out-elevenlabs` — implement `Channel` interface, use locked event kinds, emit `voice.out.done`, add error visibility. -- `codex/voice-in-groq` -- implement `Channel` interface, use locked +- `codex/voice-in-groq` — implement `Channel` interface, use locked event kinds, add `trace_id`, fix stderr drain. ### Slices not registered @@ -247,18 +247,18 @@ Threadmaster should register them before merge. ## Files touched -- `ops/team/outbox/reviewer.md` -- this file (overwritten per contract). +- `ops/team/outbox/reviewer.md` — this file (overwritten per contract). No other files modified. No commits. No PR (task specifies outbox only). ## Contract trace -`ops/contracts/reviewer.contract.md` -> `ops/team/inbox/reviewer.md` +`ops/contracts/reviewer.contract.md` → `ops/team/inbox/reviewer.md` (task 021, slice `cycle21-review`). ## Validation -Not run -- task specifies "Single pass, outbox only. NO commit. NO PR." +Not run — task specifies "Single pass, outbox only. NO commit. NO PR." and does not authorize firsthand validation. Engineer claims noted but unverified. From 682a9ecc281d2d38141a37a01351800689a52782 Mon Sep 17 00:00:00 2001 From: Jason Brashear <115739170+webdevtodayjason@users.noreply.github.com> Date: Mon, 13 Apr 2026 09:21:43 -0500 Subject: [PATCH 3/3] cycle21-review: reviewer pass on 4 kiosk-v0 branches (2 PASS, 2 FAIL) Task 021. Reviewed codex/kiosk-v0-design (PASS), codex/kiosk-ui-v0 (PASS), codex/voice-out-elevenlabs (FAIL: wrong event kinds, no Channel interface, missing voice.out.done), codex/voice-in-groq (FAIL: wrong event kind, no Channel interface, missing trace_id). All 4 branches need rebase onto 089c506 before merge. Co-Authored-By: Claude Opus 4.6 (1M context) --- ops/team/outbox/reviewer.md | 290 +++++++++++++++--------------------- 1 file changed, 119 insertions(+), 171 deletions(-) diff --git a/ops/team/outbox/reviewer.md b/ops/team/outbox/reviewer.md index df20374..d8a3ab0 100644 --- a/ops/team/outbox/reviewer.md +++ b/ops/team/outbox/reviewer.md @@ -3,164 +3,131 @@ ops/contracts/reviewer.contract.md. ## Verdict: FAIL (2 of 4 branches) -Four branches reviewed for cycle-21 kiosk v0 (task 021). Two pass -(kiosk-v0-design, kiosk-ui-v0); two fail on event-kind lock violations -and Channel interface non-conformance (voice-out-elevenlabs, -voice-in-groq). All four carry stale base commits requiring rebase. +Independent review of 4 cycle-21 kiosk-v0 branches (task 021). +All claims verified against `codex/ops-team-bootstrap` HEAD (`089c506`). +Fork point for all 4 branches: `9ac72aa`. --- -## F0 — CRITICAL: stale fork point (all 4 branches) +## F0 — Stale fork point (all 4 branches) -All four branches fork from `9ac72aa` (cycle-21 dispatch). The -integration branch `codex/ops-team-bootstrap` has since advanced to -`089c506` with two commits: +All 4 branches fork from `9ac72aa`. Integration branch has since advanced +to `089c506` (+2 commits: `568da08` v0.1.0 README/LICENSE/dashboard, +`089c506` init gateway auth sync). Merging any branch as-is would revert: +LICENSE (deleted), README.md (292→3 lines), deploy/pi-dashboard/dashboard.py +(loses systemd controls + service entries), src/cli/init.ts (removes +gateway auth sync). -- `568da08` — v0.1.0: full README + MIT LICENSE + all-service controls - in pi-dashboard -- `089c506` — fix: init wizard syncs API keys to argentos-core gateway - -Merging any branch as-is would **delete LICENSE**, **truncate -README.md** (292 → 3 lines), **regress dashboard.py** (removes systemd -generic-service controls, scope detection, per-service -start/stop/restart UI, strips 8-entry SERVICES list to 6, removes -per-service control API endpoint), and **remove gateway sync from -src/cli/init.ts** (76 lines deleted). - -Violates `never-do.md`: "Never mix unrelated repo changes into one -handoff packet." Rebase onto `089c506` required before merge. +**Action required:** rebase all 4 branches onto `089c506` before merge. --- -## Branch 1: codex/kiosk-v0-design (503ccd9) — PASS with notes - -**Commit:** `503ccd9` — "kiosk-v0-design: architect memo for Echo-Dot -kiosk (task 021)". Co-Authored-By trailer present. +## Branch 1: codex/kiosk-v0-design (503ccd9) — PASS **Files:** `ops/projects/kiosk-v0-design.md` (new, 170 lines), -`ops/team/outbox/architect.md` (updated). Only `ops/` files touched — -honors architect contract ("no application code"). - -### Findings - -- PASS: Design doc covers state machine (4 states, 3 timeouts), screen - layout (1024×600), voice pipeline contract, config surface, onboarding - wireframe, acceptance criteria (7 gates), risks/non-goals. Well - structured, 170 lines (under 180-line cap). -- PASS: No `src/` touched — architect contract honored. -- PASS: Outbox has correct confirmation line, contract trace, rationale. -- PASS: Mac-station mismatch acknowledged in risks (§11). - -### Notes (non-blocking) - -- `kiosk-v0-design.md:90` — References "the existing ChannelRegistry - from the channels-lite slice." **No ChannelRegistry class exists** on - `codex/ops-team-bootstrap`. Verified: `src/channels/types.ts` exports - `Channel`, `ChannelBus`, `ChannelOptions` — no registry. Fabricated - reference; engineer must not rely on it. -- `kiosk-v0-design.md:82-93` — Declares event kinds `voice.in.start`, - `voice.in.end`, `voice.out.done` as "kiosk-internal." These are not in - the locked vocabulary (`src/runtime/event-kinds.ts`: `channel.in`, - `channel.out`, `router.in`, `router.out`, `agent.error`). Neither - engineer implementation uses these kinds either — mismatch between - design and implementation. -- `kiosk-v0-design.md:65` — Wireframe shows "HOLD TO TALK" but - kiosk-ui-v0 HTML renders "Tap to Talk." Cosmetic mismatch. -- `kiosk-v0-design.md:95-99` — Voice pipeline contract specifies - `trace_id` propagation. Neither voice channel implementation includes - trace_id. - -### Missing references - -- `ChannelRegistry` — does not exist on `codex/ops-team-bootstrap`. +`ops/team/outbox/architect.md` (updated). + +- Architect contract honored — design only, no application code authored. + The init.ts deletion and other collateral is the F0 fork-point issue, + not architect work. +- Design doc covers: 4-state machine with 3 timeouts, 1024×600 screen + layout, voice-in/voice-out pipeline contracts, `kiosk:` config block, + 5-step onboarding wireframe, candidate file areas, cycle-22 sub-slices, + 10 acceptance criteria, risks/non-goals. +- Outbox has confirmation line, contract trace, file list. + +**Non-blocking notes:** + +- `kiosk-v0-design.md:90` — references "existing ChannelRegistry from + channels-lite slice." Verified: **no ChannelRegistry class exists** on + ops-team-bootstrap. `src/channels/types.ts` exports `Channel`, + `ChannelBus`, `ChannelOptions` but no registry. Fabricated name — + engineer must not rely on it. +- `kiosk-v0-design.md:82-93` — declares event kinds `voice.in.start`, + `voice.in.end`, `voice.out.done` as kiosk-internal. These are not in + the locked vocabulary (`src/runtime/event-kinds.ts`: channel.in, + channel.out, router.in, router.out, agent.error). Neither voice + channel implementation uses these kinds — design/impl gap. +- `kiosk-v0-design.md:65` — wireframe says "HOLD TO TALK"; kiosk-ui-v0 + HTML uses "Tap to Talk." Cosmetic mismatch. +- `kiosk-v0-design.md:95-99` — specifies `trace_id` propagation through + voice pipeline. Neither voice channel implements trace_id. +- `kiosk-v0-design.md:64` — orb iframe assumes argentos at `:8080`. On + standalone Pi without argentos-core, iframe 404s. kiosk-ui-v0 correctly + uses a standalone gradient disc instead. + +**Missing references:** ChannelRegistry (fabricated name). --- ## Branch 2: codex/kiosk-ui-v0 (fde78e6) — PASS -**Commit:** `fde78e6` — "kiosk-ui-v0: full-screen HTTP kiosk on -127.0.0.1:7788". Co-Authored-By trailer present. - **Files:** `src/cli/kiosk.ts`, `src/kiosk/handlers.ts`, `src/kiosk/server.ts`, `src/kiosk/index.ts`, `src/kiosk/index.html`, `deploy/argent-lite-kiosk.desktop`, `tests/kiosk/handlers.test.ts`, `tests/kiosk/server.test.ts` (8 new files, 918 insertions). -### Findings - -- **State machine** (`handlers.ts`): 4-state model - (idle/listening/thinking/speaking). `talkStart` gated on idle, - `talkEnd` gated on listening, errors reset to idle. Callbacks injected - via `KioskHandlerOptions`. Tests cover all transitions and error paths - (10 tests). -- **HTTP server** (`server.ts`): Loopback-only bind (`127.0.0.1:7788`). - Host-header validation rejects non-loopback with 403. SSE `/events` - endpoint with `unref()`'d tick timer. Proper `no-store` cache headers. - Tests cover HTML, status, talk, interrupt, SSE, host rejection, 404 - (7 tests, 17 total kiosk tests). -- **Kiosk HTML** (`index.html`): Touch-optimized (pointer events, - `touch-action: manipulation`). CSS animations per state (breath for - idle, spin for listening, fast spin for thinking, pulse for speaking). - Standalone gradient disc instead of argentos iframe — acceptable v0 - simplification. -- **Desktop entry** (`argent-lite-kiosk.desktop`): Valid freedesktop - format, Chromium kiosk mode pointing at loopback. -- No event-bus coupling — purely HTTP UI layer. Clean boundary. - -### Minor notes (non-blocking) - -- `server.ts:35-44` — `resolveDefaultIndexHtmlPath` returns first - candidate path even if no candidate exists on disk; subsequent - `readFile` will throw unhelpful ENOENT. Minor. -- `handlers.ts:93-102` — `talkEnd` sets state to `"thinking"` then - immediately resolves `stopCapture`, advancing to `"speaking"` in same - tick. Thinking state is unobservable via SSE. Fine for v0 but will - need rework when real voice pipeline is integrated. -- No automatic `speaking → idle` transition. Must be wired when - voice-out channel is integrated. - -### Missing references: none. -### Validation: not run by reviewer (task specifies outbox-only). +- **State machine** (`handlers.ts`): 4-state (idle/listening/thinking/ + speaking), transitions correctly gated, errors reset to idle. + 10 handler tests cover all transitions + error paths. +- **HTTP server** (`server.ts`): loopback-only bind (`127.0.0.1`) + + host-header validation (rejects non-loopback with 403). SSE `/events` + with unref'd ticker. `no-store` cache headers. 7 server tests. + Total: 17 tests across both files. +- **Kiosk HTML** (`index.html`): touch-optimized pointer events, CSS + animations per state (breath idle, spin listening, pulse speaking), + standalone gradient disc (no iframe dependency). Zero external deps. +- **Desktop entry** (`argent-lite-kiosk.desktop`): valid freedesktop + format, Chromium kiosk flags. +- Clean HTTP boundary — no event-bus coupling. Good separation. + +**Non-blocking notes:** + +- `server.ts:47-63` — `resolveDefaultIndexHtmlPath` returns the first + candidate path even if neither exists; `readFile` at startup throws + an unhelpful ENOENT. Consider a clear error message. +- `server.ts:47-63` — `index.html` lives in `src/kiosk/`. Build pipeline + does not copy it to `dist/`. Needs build config update at integration. +- `handlers.ts:68-73` — state is set to `"listening"` before awaiting + `startCapture()`. Single-user PTT scenario makes races unlikely but + worth noting. +- No `speaking → idle` auto-transition. Must be wired when voice-out + channel integrates (voice.out.done → idle). + +**Missing references:** none. --- ## Branch 3: codex/voice-out-elevenlabs (d69fa43) — FAIL -**Commit:** `d69fa43` — "voice-out-elevenlabs: add ElevenLabs TTS -voice-out channel". Co-Authored-By trailer present. - **Files:** `src/channels/voice-out.ts` (132 lines), `tests/channels/voice-out.test.ts` (182 lines, 3 tests). ### Blocking defects -- `voice-out.ts:27-34` — Type guard matches `kind === "completion"`. - **Not in locked vocabulary.** Locked kinds per - `src/runtime/event-kinds.ts`: `channel.in`, `channel.out`, `router.in`, - `router.out`, `agent.error`. Must use `"router.out"` or `"channel.out"`. - -- `voice-out.ts:42-47` — Matches `kind === "error"`. Locked spelling is +- `voice-out.ts:27-34` — type guard matches `kind === "completion"`. + **Not in locked vocabulary.** `src/runtime/event-kinds.ts` locks to: + `channel.in`, `channel.out`, `router.in`, `router.out`, `agent.error`. + Must use `"router.out"` or `"channel.out"`. +- `voice-out.ts:42-47` — matches `kind === "error"`. Locked spelling is `"agent.error"`. - -- `voice-out.ts` — Defines ad-hoc bus interface +- `voice-out.ts` — defines ad-hoc bus interface `{ subscribe(id, h): () => void }` instead of `ChannelBus` from - `src/channels/types.ts` (which requires typed `AgentMessage` and - exposes both `send()` and `subscribe()`). Does not implement `Channel` - interface (missing `readonly id: string`, different method signatures). - -- `voice-out.ts` — Never emits `voice.out.done` event. Design doc - §5 specifies emission after TTS stream completes. Required for - kiosk speaking→idle transition. + `src/channels/types.ts` (typed `AgentMessage`, exposes `send()` + + `subscribe()`). Does not implement the `Channel` interface (missing + `readonly id: string`, different method signatures). +- `voice-out.ts` — never emits `voice.out.done` event. Design doc §5 + requires emission after TTS stream completes. Needed for kiosk + speaking→idle transition. ### Non-blocking notes -- `voice-out.ts:88` — Silently swallows non-ok ElevenLabs responses +- `voice-out.ts:88` — silently swallows non-ok ElevenLabs responses (`if (!res.ok || res.body === null) return;`). TTS failures invisible. -- `voice-out.ts` — `now` option in `VoiceOutOptions` accepted but never - used. Dead parameter. -- `voice-out.ts:75-78` — `ensureAplay()` reuses single long-lived aplay - stdin. If aplay crashes, subsequent writes silently fail with no - recovery. +- `voice-out.ts` — `now` option accepted but never used. Dead parameter. +- `voice-out.ts:69-73` — `ensureAplay()` reuses a single aplay stdin + across calls. aplay exits when stdin stream closes; subsequent + `speak()` calls write to the closed pipe with no reconnect logic. ### Rules violated @@ -172,37 +139,30 @@ voice-out channel". Co-Authored-By trailer present. ## Branch 4: codex/voice-in-groq (48bdc96) — FAIL -**Commit:** `48bdc96` — "voice-in-groq: arecord → Groq whisper-large-v3 -voice-in channel". Co-Authored-By trailer present. - **Files:** `src/channels/voice-in.ts` (203 lines), `tests/channels/voice-in.test.ts` (178 lines, 4 tests). ### Blocking defects -- `voice-in.ts:179` — Emits `{ kind: "prompt" }`. **Not in locked +- `voice-in.ts:179` — emits `{ kind: "prompt" }`. **Not in locked vocabulary.** Must use `"channel.in"`. - -- `voice-in.ts` — Defines own `VoiceInBus` (`{ send(msg) }`) instead of - `ChannelBus` from `src/channels/types.ts` (which requires typed - `AgentMessage`). Does not implement `Channel` interface (missing - `readonly id: string`). - -- `voice-in.ts` — Emitted message lacks `trace_id`. Design §5 - and acceptance gate §10.2 require trace_id propagation. +- `voice-in.ts` — defines own `VoiceInBus` (`{ send(msg) }`) instead of + `ChannelBus` from `src/channels/types.ts` (typed `AgentMessage`). + Does not implement the `Channel` interface (missing `readonly id`). +- `voice-in.ts` — emitted message lacks `trace_id`. Design §5 and + acceptance gate §10.2 require trace_id propagation. ### Non-blocking notes -- `voice-in.ts:126-131` — Creates `PassThrough` for stderr drain that - is written to but never consumed. Buffers indefinitely. Should use - `proc.stderr.resume()` instead. -- `voice-in.ts:114-116` — `Buffer.from(chunk)` on value already typed - as `Buffer | string`. Redundant copy when chunk is already a Buffer. +- `voice-in.ts:126-131` — creates `PassThrough` for stderr drain that is + written to but never consumed. Buffers indefinitely if stderr is + verbose. Should use `proc.stderr.resume()`. +- `voice-in.ts:114-116` — `Buffer.from(chunk)` when chunk may already be + a Buffer. Redundant copy on each data event. ### Rules violated -- `src/runtime/event-kinds.ts` lock: uses `"prompt"` instead of - `"channel.in"`. +- `src/runtime/event-kinds.ts` lock: uses `"prompt"`. - `src/channels/types.ts`: does not implement `Channel` interface. - Design doc §5: missing `trace_id`. @@ -210,40 +170,30 @@ voice-in channel". Co-Authored-By trailer present. ## Summary for threadmaster -### Rebase required (all four branches) - -All branches fork from `9ac72aa`; must rebase onto current -`codex/ops-team-bootstrap` HEAD (`089c506`) before merge. Without -rebase, merging any branch will revert LICENSE, README, dashboard -systemd controls, and init gateway sync. - -### Blocking defects (branches 3 and 4) +### Rebase required (all 4 branches) -1. **Event-kind lock violation.** `voice-out` uses `"completion"` and - `"error"`; `voice-in` uses `"prompt"`. All must use locked vocabulary - from `src/runtime/event-kinds.ts`. -2. **Channel interface non-conformance.** Both voice channels define - ad-hoc bus interfaces instead of implementing `Channel` from - `src/channels/types.ts`. -3. **Missing trace_id** in `voice-in` messages (design §5, gate §10.2). -4. **Missing voice.out.done emission** in `voice-out` (design §5). +Rebase onto `089c506` before any merge. Without rebase, merging reverts +LICENSE, README, dashboard systemd controls, and init gateway auth sync. -### Branches ready to merge (after rebase) +### Ready to merge (after rebase) -- `codex/kiosk-v0-design` — PASS with notes. -- `codex/kiosk-ui-v0` — PASS. +- `codex/kiosk-v0-design` — PASS with non-blocking notes. +- `codex/kiosk-ui-v0` — PASS with non-blocking notes. -### Branches requiring rework +### Require rework -- `codex/voice-out-elevenlabs` — implement `Channel` interface, use - locked event kinds, emit `voice.out.done`, add error visibility. -- `codex/voice-in-groq` — implement `Channel` interface, use locked - event kinds, add `trace_id`, fix stderr drain. +- `codex/voice-out-elevenlabs` — implement `Channel` interface from + `src/channels/types.ts`, use locked event kinds (`router.out` / + `agent.error` not `completion` / `error`), emit `voice.out.done`, + surface TTS error responses. +- `codex/voice-in-groq` — implement `Channel` interface from + `src/channels/types.ts`, use locked event kind (`channel.in` not + `prompt`), add `trace_id` to emitted messages, fix stderr drain. ### Slices not registered -None of the four branch slices appear in `ops/slices/REGISTRY.md`. -Threadmaster should register them before merge. +None of the 4 branch slices appear in `ops/slices/REGISTRY.md`. +Register before merge. ## Files touched @@ -259,10 +209,8 @@ No other files modified. No commits. No PR (task specifies outbox only). ## Validation Not run — task specifies "Single pass, outbox only. NO commit. NO PR." -and does not authorize firsthand validation. Engineer claims noted but -unverified. +and does not authorize firsthand validation. ## Blockers -None (review complete; rework is on the engineers, not a reviewer -blocker). +None (review complete; rework is on engineers).