fix(webui): approval buttons did nothing — active id was nulled during render - #142
Merged
Conversation
…g render
removeActiveApprovalCard reset S.activeApprovalId, and renderApprovalCard
called it as its first line. Every freshly rendered card therefore had a
null id, so sendApproval's guard silently dropped every button click and
keyboard shortcut: the approval form rendered but never answered (the
agent side then timed out after 60s and denied).
- approvals.js: removeActiveApprovalCard now removes only the card; the
id is owned exclusively by showNextApproval (set to the shown request,
null when the queue empties)
- approvals.js: new clearApprovals() owns the full teardown (queue +
card + id); sessions.js uses it on new/switch session — the old
per-site pair left a stale id that suppressed later cards
- approvals.js: fix stale "request 1 of N" hint — a card rendered at
depth 1 never gained the indicator when further requests arrived;
updateQueuePosition now refreshes it live on arrival/dismissal
Tests (cmd/odek/ui/js/approvals.test.js): DOM-shim suite driving the real
module — render contract, approve/deny/trust wire format ({type,id,
action} as parsed by wsapprover.go), keyboard shortcuts incl. input/
trust-suppression guards, FIFO queueing, approval_ack dismissal,
clearApprovals regression guard, friction gate (typed word, disabled
clicks, Enter path), and live queue-position updates.
node --test cmd/odek/ui/js/*.test.js → 68 pass; go test ./cmd/odek/ ok.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
odek | af6542a | Commit Preview URL Branch Preview URL |
Aug 22 2026, 01:37 PM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The WebUI approval decision card rendered, but the approve / deny / trust buttons (and keyboard shortcuts) did nothing. The agent side then sat blocked until the 60s approval timeout and denied the operation.
Root cause
renderApprovalCard()calledremoveActiveApprovalCard()as its first line, and that helper unconditionally resetS.activeApprovalId = null. Every freshly rendered card therefore had a null id, andsendApproval's guard (if (!S.activeApprovalId) return;) silently dropped every response.This survived review because #135 rewrote everything around
approvals.jswithout exercising its DOM behavior — there was no test driving clicks through the real module.Fixes
approvals.js—removeActiveApprovalCardremoves only the card;S.activeApprovalIdis owned exclusively byshowNextApproval(set to the shown request, null when the queue empties).approvals.js— newclearApprovals()owns full teardown (queue + card + id);sessions.jsuses it on new/switch session. The old per-site pair left a stale id that suppressed all later cards after a session switch.approvals.js— stale "request 1 of N" hint fixed: a card rendered at queue depth 1 never gained the indicator when further requests arrived;updateQueuePosition()refreshes it live on arrival/dismissal (without re-rendering, preserving friction-mode typing).Test coverage
New
cmd/odek/ui/js/approvals.test.js(17 tests) drives the real module through a minimal DOM shim:{type,id,action}as parsed byapprovalResponseinwsapprover.goa/d/t), ignored while typing in inputs, trust shortcut suppressed whenallow_trust === falseapproval_ackdismissal from another client sends nothingclearApprovals()regression guard: post-teardown requests still render and answerValidation
node --test cmd/odek/ui/js/*.test.js→ 68 pass (51 existing + 17 new)go test -count=1 ./cmd/odek/→ ok (incl. WebUI asset-contract/E2E suites)go vetclean,golangci-lint0 issues