Skip to content

Fix #1431: map verify-approval gate to view-diff in phaseArtifactVerb - #1442

Merged
amrmelsayed merged 9 commits into
mainfrom
builder/bugfix-1431
Aug 13, 2026
Merged

Fix #1431: map verify-approval gate to view-diff in phaseArtifactVerb#1442
amrmelsayed merged 9 commits into
mainfrom
builder/bugfix-1431

Conversation

@amrmelsayed

@amrmelsayed amrmelsayed commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Stream Deck's phaseArtifactVerb did not map the verify-approval gate, so a builder blocked there returned undefined and the Builder Action Automatic press fell back to opening a terminal instead of the review diff. The #1428 faces already render this gate (yellow + verified glyph + Verify label), so today the key looks gate-blocked but the press opens a terminal — a face/behaviour contradiction, the same class codex caught during #1428.

Fixes #1431

Root Cause

phaseArtifactVerb (apps/streamdeck/src/actions.ts) mapped the spec-approval, plan-approval, dev-approval, and pr gates but not verify-approval — which the VS Code sidebar's gate vocabulary already knows (gateIconFor maps it to the verified codicon). The Stream Deck resolver never caught up, so the gate returned undefined and both callers that key off it (the Builder Action Automatic press and the phase-aware dial mode) took the fallback path.

Fix

Second behavioural effect: review dials

reviewMode() derives from the same resolver (verb === 'view-diff''diff'), so this change also flips a verify-approval-blocked builder's review dials from 'none' (dead) to 'diff' (navigating the diff). This is correct and desirable — symmetric with dev-approval/pr; the dials should work while a human reviews finished work. Pinned with an added reviewMode assertion.

Test Plan

  • Regression tests added — phaseArtifactVerb(verify-approval)view-diff and reviewMode(verify-approval)diff (both fail without the fix: phaseArtifactVerb returns undefined, reviewMode returns none)
  • Build passes
  • check-types clean
  • All tests pass (125/125)

Stream Deck's phaseArtifactVerb mapped the spec/plan/dev-approval/pr gates
but not verify-approval, so a builder blocked there returned undefined and
the Builder Action Automatic press fell back to a terminal instead of the
review diff (the #1428 faces already render the gate as blocked, so the key
looked gate-blocked but behaved otherwise). Add verify-approval to the gate
branch returning view-diff (same as dev-approval/pr: the human is reviewing
finished work); the press then resolves view-diff to open-diff-first via
BuilderAction.resolveVerb (#1414). Also drop the now-stale face.ts comment
noting the resolver gap.
The verify-approval → view-diff mapping also flips reviewMode from 'none' to
'diff' (reviewMode derives from phaseArtifactVerb: view-diff → diff), so a
verify-approval-blocked builder's review dials navigate the diff — symmetric
with dev-approval/pr and correct while a human reviews finished work. Pin it
with an assertion beside the existing dev-approval/pr reviewMode cases.
@amrmelsayed

Copy link
Copy Markdown
Collaborator Author

Integration review (streamdeck architect) — APPROVE. Low-risk tier, read line-by-line. The change is one clause added to an existing gate branch (verify-approvalview-diff, symmetric with dev-approval/pr — the human is reviewing finished work), its test, and deletion of the now-stale face.ts comment that pointed at this very issue as an open gap.

Board was degraded: codex was environmentally unavailable (OpenAI billing/credits, raised to the owner as an account item), so this shipped on a 2-way CMAP — gemini + claude, both APPROVE HIGH — plus architect source verification, per the interim policy both architects have adopted for low-risk lanes while the outage lasts.

That verification earned its place: the two-seat board missed a real untested behaviour change. Because reviewMode derives from phaseArtifactVerb, this mapping also flips a verify-approval-blocked builder's review dials from none (dead) to diff (navigating the diff). Correct and desirable — the dials should work exactly when a human is reviewing finished work — but unasserted. Now pinned by a regression assertion (2dfbd81) and called out in the PR body.

Closes the face/behaviour contradiction introduced by #1428's faces, which already rendered this gate as blocked (yellow + verified glyph + "Verify") while the press fell through to a terminal.

@amrmelsayed
amrmelsayed merged commit 805df27 into main Aug 13, 2026
7 checks passed
@amrmelsayed
amrmelsayed deleted the builder/bugfix-1431 branch August 13, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stream Deck: phaseArtifactVerb doesn't map the verify-approval gate (Automatic press falls back to terminal)

1 participant