Fix #1606: revive Stream Deck Review dials for BUGFIX/AIR phases + honest none-mode label

apps/streamdeck/README.md

@@ -266,16 +266,20 @@ builder (+ its phase and position); PR / Spawn show the item + `i/N`.
 - **Spawn from Backlog** — rotate the backlog; **push** spawns a builder for the
  selected issue (VSCode prompts for the protocol).
 - **Review: Files / Headings** — phase-aware coarse review dial. The selected
- builder's phase picks the mode. *Diff phase* (implement / review, or blocked at
- dev-approval / pr): rotate = next / previous file, **push** = submit the current
- file as feedback, **tap** = jump to the first file. *Spec/plan phase* (specify /
- plan, or blocked at spec-approval / plan-approval): rotate = next / previous heading
- in the artifact canvas, **push** = open the composer at the focused block, **tap** =
- jump to the document top. The touch strip names the live mode and, in diff phase,
- the delivery mode (`Files · send` vs `Files · queue`).
-- **Review: Changes / Blocks** — phase-aware fine review dial, same mode split.
- *Diff phase*: rotate = next / previous change, **push** = submit the current change
- as feedback, **tap** = jump to the first change. *Spec/plan phase*: rotate = next /
+ builder's phase picks the mode. *Diff phase* (implement / review / verify, the
+ BUGFIX/AIR pr phase, or blocked at dev-approval / pr / verify-approval): rotate =
+ next / previous file, **push** = submit the current file as feedback, **tap** = jump
+ to the first file. *Spec/plan phase* (specify / plan, or blocked at spec-approval /
+ plan-approval): rotate = next / previous heading in the artifact canvas, **push** =
+ open the composer at the focused block, **tap** = jump to the document top. When the
+ selected builder has no reviewable artifact (an `init` / no-live-status builder, or a
+ work-in-progress BUGFIX investigate / fix phase), both dials go inert and the strip
+ reads `No review target` rather than a mode label. The touch strip names the live mode
+ and, in diff phase, the delivery mode (`Files · send` vs `Files · queue`).
+- **Review: Changes / Blocks** — phase-aware fine review dial, same mode split
+ (including the inert `No review target` state on a builder with no reviewable
+ artifact). *Diff phase*: rotate = next / previous change, **push** = submit the current
+ change as feedback, **tap** = jump to the first change. *Spec/plan phase*: rotate = next /
  previous block, **push** = open the composer, **tap** = walk forward to the next
  commented block. The touch strip names the live mode (`Changes · send` /
  `Changes · queue`, or `Blocks`).

apps/streamdeck/src/__tests__/actions.test.ts

@@ -635,6 +635,53 @@ describe('encoders', () => {
  expect(ctx.canvasSent).toHaveLength(0);
  });
 
+ it('BUGFIX pr-phase builder: the review dials drive the diff (dead before the pr gate fired, #1606)', async () => {
+ const ctx = makeStore();
+ // A BUGFIX/AIR builder in the pr *phase*, not yet blocked at the pr *gate* (blockedGate null).
+ // Before #1606 this resolved to reviewMode 'none' and every gesture no-opped until the gate.
+ ctx.store.overview = {
+ builders: [{ id: 'bugfix-1', roleId: null, issueId: '1606', issueTitle: 'Review dials', blocked: null, blockedGate: null, protocolPhase: 'pr', progress: 95, worktreePath: '/w' }],
+ pendingPRs: [], backlog: [], recentlyClosed: [],
+ } as never;
+ const nav = new DiffFileNav(ctx.store);
+ await nav.onDialRotate(dial(1) as never); // next file
+ await nav.onDialDown(); // submit feedback
+ await nav.onTouchTap(); // first file
+ expect(ctx.sent.map((s) => s.verb)).toEqual(['diff-next-file', 'feedback-file', 'diff-first-file']);
+ expect(ctx.canvasSent).toHaveLength(0); // diff mode never touches the canvas channel
+ });
+
+ it('BUGFIX investigate builder: the review dials stay inert (WIP is not a review target, #1606)', async () => {
+ const ctx = makeStore();
+ // Owner ruling: investigate/fix are work-in-progress, so the dials are none-mode and no-op.
+ ctx.store.overview = {
+ builders: [{ id: 'bugfix-1', roleId: null, issueId: '1606', issueTitle: 'Review dials', blocked: null, blockedGate: null, protocolPhase: 'investigate', progress: 10, worktreePath: '/w' }],
+ pendingPRs: [], backlog: [], recentlyClosed: [],
+ } as never;
+ const nav = new DiffFileNav(ctx.store);
+ await nav.onDialRotate(dial(1) as never);
+ await nav.onDialDown();
+ await nav.onTouchTap();
+ expect(ctx.sent).toHaveLength(0); // no diff verbs
+ expect(ctx.canvasSent).toHaveLength(0); // no canvas commands
+ });
+
+ it('legibility: a none-mode builder\'s strip says "No review target", not the diff label (#1606)', () => {
+ const ctx = makeStore();
+ // A porch `init`-phase (or soft/shell/task) builder: genuinely nothing to review. The strip
+ // must NOT borrow the diff label (`Files · send`) while every gesture is inert — that lie is
+ // the compounding half of #1606.
+ ctx.store.overview = {
+ builders: [{ id: 'task-x', roleId: null, issueId: null, issueTitle: null, blocked: null, blockedGate: null, protocolPhase: 'init', progress: 0, worktreePath: '/w' }],
+ pendingPRs: [], backlog: [], recentlyClosed: [],
+ } as never;
+ const fileAction = { isDial: () => true, setFeedback: vi.fn() };
+ new DiffFileNav(ctx.store).onWillAppear({ action: fileAction, payload: {} } as never);
+ const fb = fileAction.setFeedback.mock.calls.at(-1)?.[0] as Record<string, unknown>;
+ expect(fb.title).toBe('No review target');
+ expect(fb.title).not.toBe('Files · send'); // the old lie
+ });
+
  it('ScrollNav diff mode: rotate relays byte-for-byte editorScroll, press submits the selection as feedback', async () => {
  const ctx = makeStore();
  // State the mode the assertion depends on: pir-2 (implement, no gate) is the DIFF-mode builder.
@@ -939,6 +986,17 @@ describe('phaseArtifactVerb (shared resolver — recognised verb or undefined)',
  // The two callers diverge exactly here: dial → view-diff, Builder Action → open-terminal.
  expect(zoomInVerb(b({}))).toBe('view-diff');
  });
+ it('leaves BUGFIX investigate/fix and the pr PHASE unrecognised — the review dials own that vocabulary (#1606)', () => {
+ // Deliberate split: `phaseArtifactVerb` stays SPIR/PIR-scoped so the auto-open action key
+ // keeps opening a terminal and Zoom falls back to view-diff. Only `reviewMode` maps the pr
+ // phase to diff (see DIFF_REVIEW_PHASES); investigate/fix stay none as work-in-progress. If
+ // this ever changes, reconsider the Builder Action key's open-terminal→open-diff-first side
+ // effect first.
+ expect(phaseArtifactVerb(b({ protocolPhase: 'investigate' }))).toBeUndefined();
+ expect(phaseArtifactVerb(b({ protocolPhase: 'fix' }))).toBeUndefined();
+ expect(phaseArtifactVerb(b({ protocolPhase: 'pr' }))).toBeUndefined();
+ expect(zoomInVerb(b({ protocolPhase: 'pr' }))).toBe('view-diff'); // Zoom already correct for the pr phase
+ });
 });
 
 describe('reviewMode (dial mode from the shared resolver)', () => {
@@ -956,9 +1014,23 @@ describe('reviewMode (dial mode from the shared resolver)', () => {
  expect(reviewMode(b({ blockedGate: 'pr' }))).toBe('diff');
  expect(reviewMode(b({ blockedGate: 'verify-approval' }))).toBe('diff'); // #1431: dials navigate the diff while the human reviews finished work
  });
- it('an unknown phase, no live status, or no builder → none', () => {
+ it('the BUGFIX+AIR pr PHASE → diff (#1606: dead between entering the pr phase and the gate firing)', () => {
+ // The pr *phase* (BUGFIX `investigate/fix/pr`, AIR `implement/pr`) holds a finished diff, so
+ // the dials go live before the pr *gate* is requested. The pr *gate* already resolved to diff
+ // via phaseArtifactVerb (blockedGate 'pr', line above) — this covers the phase before it.
+ expect(reviewMode(b({ protocolPhase: 'pr' }))).toBe('diff');
+ });
+ it('BUGFIX work-in-progress phases investigate/fix → none (owner ruling: WIP is not a review target)', () => {
+ // #1606 owner ruling 2026-09-04: investigate/fix are work-in-progress (partial/no diff yet),
+ // so the dials stay inert with the honest "No review target" label rather than driving a
+ // half-written diff. Only the pr phase (finished work) is diffable.
+ expect(reviewMode(b({ protocolPhase: 'investigate' }))).toBe('none');
+ expect(reviewMode(b({ protocolPhase: 'fix' }))).toBe('none');
+ });
+ it('an unknown phase, no live status, or no builder → none (init/soft/shell/task have nothing to review)', () => {
  expect(reviewMode(b({}))).toBe('none');
  expect(reviewMode(b({ protocolPhase: 'mystery' }))).toBe('none');
+ expect(reviewMode(b({ protocolPhase: 'init' }))).toBe('none'); // #1606: porch init phase stays none
  expect(reviewMode(undefined)).toBe('none');
  });
 });

apps/streamdeck/src/actions.ts

@@ -548,18 +548,44 @@ export function zoomInVerb(b: OverviewBuilder): string {
 /** Which artifact form the selected builder's phase implies for the review dials. */
 export type ReviewMode = 'diff' | 'canvas' | 'none';
 
+/**
+ * Phase ids whose review dials drive a DIFF, but that `phaseArtifactVerb` deliberately does
+ * NOT auto-open (so the Builder Action key and Zoom keep their behavior — see the guard test):
+ *
+ * - `pr` (BUGFIX + AIR): by the pr *phase* the work already exists as a finished diff
+ * (owner ruling 2026-09-04), even before the pr *gate* is requested (`blockedGate === 'pr'`
+ * already resolves to diff via `phaseArtifactVerb`). This closes the window where an AIR /
+ * BUGFIX builder's dials were dead between entering the pr phase and the gate firing.
+ *
+ * Deliberately EXCLUDED, do not re-add without an owner ruling: BUGFIX `investigate` / `fix`
+ * are work-in-progress (owner ruling 2026-09-04). Their diff is partial or not yet written, so
+ * they are not a review target; the dials stay `none` (inert, with an honest `No review target`
+ * label, not the old lie), matching `phaseArtifactVerb`, which opens a terminal there.
+ *
+ * REGISTRATION NOTE (mirrors Tower's GATE_LABELS pattern, face.ts): `protocol.json` phase
+ * metadata carries no field expressing review mode (only id/name/type/steps/transition/
+ * gate), and the SDK's `PHASE_TO_STAGE` drives the VSCode Builders tree with different
+ * semantics, so neither can be reused verbatim. A new protocol whose phases review as a
+ * finished diff registers its phase id here (or maps onto `phaseArtifactVerb`'s SPIR/PIR
+ * vocabulary). An unregistered phase resolves to `none`, visibly inert with an honest label,
+ * never a silent lie.
+ */
+const DIFF_REVIEW_PHASES = new Set(['pr']);
+
 /**
  * The review mode for a builder: a builder still writing its spec/plan reviews as a
- * canvas (`open-spec` / `open-plan`), one with a diff reviews as a diff (`view-diff`),
- * and an unknown/no-status builder has neither. Derived from the shared phase/gate
- * resolver so the wire source stays single (`blockedGate` beats `protocolPhase`; never
- * guessed) — this is the same resolver family #1404's press keys off.
+ * canvas (`open-spec` / `open-plan`), one with a diff reviews as a diff (`view-diff`, or a
+ * `DIFF_REVIEW_PHASES` phase), and an unknown/no-status builder has neither. The
+ * canvas/diff-verb split derives from the shared phase/gate resolver (`blockedGate` beats
+ * `protocolPhase`; never guessed) — the same resolver family #1404's press keys off; the
+ * extra diff phases are the review-dial-only vocabulary `phaseArtifactVerb` omits.
  */
 export function reviewMode(b: OverviewBuilder | undefined): ReviewMode {
  if (!b) return 'none';
  const verb = phaseArtifactVerb(b);
  if (verb === 'open-spec' || verb === 'open-plan') return 'canvas';
  if (verb === 'view-diff') return 'diff';
+ if (DIFF_REVIEW_PHASES.has(b.protocolPhase ?? '')) return 'diff';
  return 'none';
 }
 
@@ -902,11 +928,16 @@ abstract class ReviewNav extends SingletonAction {
  private renderTo(action: DialAction): void {
  // Canvas line 1 pairs the rotate axis with the press meaning (`Blocks · Open/Submit`,
  // `Headings · Cancel`); diff mode pairs its axis with the feedback delivery mode
- // (`Files · queue` vs `Files · send`, #1410) so a press is never a surprise.
- const label =
- this.mode() === 'canvas'
- ? `${this.canvas.label} · ${this.canvas.pressLabel}`
- : `${this.diff.label} · ${this.store.feedbackMode() === 'queue' ? 'queue' : 'send'}`;
+ // (`Files · queue` vs `Files · send`, #1410) so a press is never a surprise. `none`
+ // states its own emptiness (`No review target`) rather than borrowing the diff label —
+ // every gesture in `none` mode is inert, so the strip must not imply otherwise (#1606).
+ const mode = this.mode();
+ let label = 'No review target';
+ if (mode === 'canvas') {
+ label = `${this.canvas.label} · ${this.canvas.pressLabel}`;
+ } else if (mode === 'diff') {
+ label = `${this.diff.label} · ${this.store.feedbackMode() === 'queue' ? 'queue' : 'send'}`;
+ }
  const b = this.store.selectedBuilder();
  void action.setFeedback({
  title: label,

codev/projects/bugfix-1606-streamdeck-review-dials-silent/status.yaml

@@ -0,0 +1,17 @@
+id: bugfix-1606
+title: streamdeck-review-dials-silent
+protocol: bugfix
+phase: verified
+plan_phases: []
+current_plan_phase: null
+gates:
+ pr:
+ status: approved
+ requested_at: '2026-09-04T01:53:53.303Z'
+ approved_at: '2026-09-04T09:18:48.942Z'
+iteration: 1
+build_complete: false
+history: []
+started_at: '2026-09-04T01:36:08.893Z'
+updated_at: '2026-09-04T09:19:35.190Z'
+pr_ready_for_human: false