Skip to content

feat(permission): restore permission ask hook safely - #42633

Open
kernel-oops wants to merge 1 commit into
anomalyco:devfrom
kernel-oops:fix/permission-ask-hook
Open

feat(permission): restore permission ask hook safely#42633
kernel-oops wants to merge 1 commit into
anomalyco:devfrom
kernel-oops:fix/permission-ask-hook

Conversation

@kernel-oops

@kernel-oopskernel-oops commented Aug 14, 2026

Copy link
Copy Markdown

Issue for this PR

Closes#7006

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Restores the declared permission.ask plugin hook before OpenCode creates an interactive permission request. Static deny remains final, static allow bypasses the hook, and plugins can resolve an ask to allow or deny.

This follows #39442 but preserves Effect interruption instead of turning cancellation into a pending prompt. Ordinary plugin failures fail closed to the normal prompt, and structuredClone prevents nested plugin mutation of the retained request. The SDK hook input uses the current v2 PermissionRequest type.

How did you verify your code works?

Added tests for allow, deny, failure fallback, interruption without a pending request, nested metadata isolation, and sequential multi-plugin overrides. Ran 86 permission tests (132 assertions), plugin trigger tests, OpenCode/plugin typechecks, the plugin build, and format/diff checks.

Screenshots / recordings

N/A — plugin API and permission-flow change only.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actionsgithub-actionsBot added the needs:compliance This means the issue will auto-close after 2 hours. label Aug 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Based on the search results, I found related PRs, but none that appear to be duplicates of PR #42633:

Related PRs (not duplicates):

  1. fix(plugin): restore permission ask hook #39442 - fix(plugin): restore permission ask hook

  2. feat(core): optional plugin gate in PermissionV2 for allow→ask #34329 - feat(core): optional plugin gate in PermissionV2 for allow→ask

    • Related to permission hook infrastructure but addresses a different feature (optional plugin gate).
  3. feat(permission): wire permission.ask plugin hook #30509 - feat(permission): wire permission.ask plugin hook

    • Historical permission hook implementation, not a duplicate.
  4. fix(opencode): add permission.ask plugin hook back #19453 - fix(opencode): add permission.ask plugin hook back

    • Historical permission hook fix, not a duplicate.

No duplicate PRs found

@github-actionsgithub-actionsBot removed the needs:compliance This means the issue will auto-close after 2 hours. label Aug 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Scope: restores the permission.ask plugin hook: plugins receive a structuredClone of the request info and may set output.status to "allow"/"deny"; anything else falls through to the interactive prompt.

  • Security posture is right where it matters most: plugin failure resets to "ask" (interactive prompt) rather than failing open to allow, interrupts propagate instead of being swallowed, and structuredClone(info) stops plugins from mutating shared request state. A crashing plugin degrades to the pre-hook behavior — exactly what "restore safely" should mean.
  • Cause.hasInterrupts re-failure distinction is a nice touch; generic catch-all handlers that also eat interrupts are a common Effect bug.
  • Design question worth confirming: the hook only runs when the configured rules resolved to "ask", so plugins can upgrade an ask → allow/deny but can never tighten an explicit config allow/deny. That seems like the intended layering (config is authoritative, plugins advise) — just make sure docs say plugins cannot restrict what config allows.
  • Test-infra nit: waitForPending timeout went 1s → 10s. Understandable with plugin loading in the loop, but blanket 10x timeouts can mask real latency regressions; consider scoping the longer timeout to the plugin-specific tests only.
  • The deny path rebuilds the ruleset-filtered DeniedError — consistent with native denials, so downstream consumers can't distinguish plugin denials from rule denials. Intentional? Fine either way, but worth a sentence in the PR.

Sign up for freeto 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.

permission.ask plugin hook is defined but not triggered

2 participants

@kernel-oops@Enough1122