Bind ask selector rows by request id - #847
Conversation
TheGreatAxios
left a comment
There was a problem hiding this comment.
Independent critic found no blocking stale-label defect. Rows are id-keyed; missing ids fail closed; ids minted at gate/session. Follow-up in progress: remove overlay-host index fallback on gate accept, add sequential permission.gate paint test.
|
re-critic of origin/main..HEAD including 693c488 found no remaining blocking stale-label or index-keyed accept on live gate overlays. Gate Enter with a painted value missing from live itemIds denies/cancels. Sequential permission.gate paint of B's labels is covered. createApprovalResume unchanged. File-for-later only: sequential permission.gate then Accept-once-on-B expecting allow:true. |
|
re-critic including 951d142: sequential permission.gate Accept once on B now asserts allow:true and would fail if that path still fail-closed to deny. No remaining blocking stale-label issues on live gate overlays. |
TheGreatAxios
left a comment
There was a problem hiding this comment.
Critic · Comment
Permission and operator gate rows bind by minted request/ask id; Enter fail-closes when the painted Select value is missing from live itemIds.
Findings
src/permission/gate.ts:51—withRequestIdwas inserted under the classifyOutcome/finishApprovalWait comment, so that block now documents the wrong function.
Notes
- Sequential permission Accept once on B is locked (
src/tui/gate-wire.test.ts). No remaining blocking stale-label or index-keyed accept on live gate overlays.
|
follow-up test sequential permission.gate Accept once on B expects { allow: true }. CI green at 951d142. |
TheGreatAxios
left a comment
There was a problem hiding this comment.
Verdict: approve (cannot GitHub-approve own PR).
CL-7535 holds at 20cff935. Selector rows bind by request id, not index. Ids mint at gate/session emit (createGateRequestApproval / randomUUID at emit), not in the TUI overlay and not on PermissionRequest. Stale or empty Enter is unavailable, not Reject. Escape still denies. Sequential Accept once is locked on the live ask; a later ask cannot keep the previous labels or values. Missing ids fail closed.
No blocking findings. Docs/comment nits: docs/TUI.md and src/tui/gate-events.ts still say ids mint at overlay-open; they mint earlier, on the gate event.
Keep the sequential B-not-A paint, Accept-once-on-B, painted-id-unavailable, empty-Enter-unavailable, missing-event-id fail-closed, and Esc-denies tests.
TheGreatAxios
left a comment
There was a problem hiding this comment.
Verdict: approve (GitHub cannot approve own PR).
CL-7535 holds at 20cff935. Selector rows bind by request id, not index. Ids mint at gate/session emit (createGateRequestApproval / randomUUID at emit), not in the TUI overlay and not on PermissionRequest. Stale or empty Enter is unavailable, not Reject. Escape still denies. Sequential Accept once is locked on the live ask; a later ask cannot keep the previous labels or values. Missing ids fail closed.
No blocking findings. Docs/comment nits: docs/TUI.md and src/tui/gate-events.ts still say ids mint at overlay-open; they mint earlier, on the gate event.
Keep the sequential B-not-A paint, Accept-once-on-B, painted-id-unavailable, empty-Enter-unavailable, missing-event-id fail-closed, and Esc-denies tests.
Overlay ask ids mint on the gate event at emit, not on PermissionRequest. Escape still denies. An empty or mismatched Enter cannot impersonate Reject.
Ids exist on the gate event before the overlay opens, so the selector contract and PermissionGateEvent comment should not say they mint at overlay-open.
0212868 to
f809575
Compare
Summary
Verification
bun run typecheck,bun run build, andbun run testpassbun run checkpassesFixes CL-7535
Changelog: bug