Skip to content

refactor: extract and test permission pending check - #112

Merged
mohak34 merged 1 commit into
mainfrom
fix/permission-pending-followup
Sep 17, 2026
Merged

mohak34 merged 1 commit into
mainfrom
fix/permission-pending-followup

Conversation

@mohak34

@mohak34 mohak34 commented Sep 17, 2026

Copy link
Copy Markdown
Owner

Follow-up to #98 (review findings).

  • The inline pending-list probe is now an exported isPermissionStillPending(client, id) helper. Same fail-open semantics, one place instead of buried in the event handler.
  • The 300ms auto-approve grace period is a named PERMISSION_PENDING_GRACE_MS constant with a comment explaining why the wait exists (the event fires before the TUI/CLI auto-reply).
  • getPermissionIDFromEvent is exported like the other parse helpers.
  • The decision matrix from fix: skip permission notification when request is auto-approved #98 (pending notifies, resolved stays silent, every lookup failure notifies) is now covered by tests against stub raw clients, including both response envelope shapes.

Review notes on #98 worth recording: the permission.ask plugin hook path needs no change. Core deleted its only Plugin.trigger("permission.ask") call in opencode@2fc06c5, so the event path #98 gates is the only live path; the hook handler stays as a legacy fallback for older servers.

Checks: tsc clean, bun test 127 pass, bun build clean.

Follow-up to #98: the inline pending-list probe becomes an exported
isPermissionStillPending helper (fail-open on every lookup failure),
the 300ms grace becomes a named constant, getPermissionIDFromEvent is
exported, and the author's claimed decision matrix is now covered by
tests against stub clients.
@mohak34
mohak34 merged commit 60eb554 into main Sep 17, 2026
1 check passed
@mohak34
mohak34 deleted the fix/permission-pending-followup branch September 23, 2026 19:34
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.

1 participant