Skip to content

fix(permission): fire permission.ask hook for first-encounter commands#20634

Open
YumaKakuya wants to merge 5 commits intoanomalyco:devfrom
sorted-ai:fix/permission-ask-hook-bypass
Open

fix(permission): fire permission.ask hook for first-encounter commands#20634
YumaKakuya wants to merge 5 commits intoanomalyco:devfrom
sorted-ai:fix/permission-ask-hook-bypass

Conversation

@YumaKakuya
Copy link
Copy Markdown

Issue for this PR

Closes #19927

Type of change

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

What does this PR do?

The permission.ask plugin hook is defined in @opencode-ai/plugin
and other
hooks (shell.env, chat.system.transform, etc.) are triggered
correctly via
Plugin.trigger() — but permission.ask is never called.

The root cause is the early return if (!needsAsk) return on L183 of
packages/opencode/src/permission/index.ts. When needsAsk is true
(first-encounter command), execution skips past this guard into the
permission dialog — but there is no Plugin.trigger("permission.ask", ...)
call anywhere in the function, so the hook never fires regardless of
needsAsk.

This PR adds the missing Plugin.trigger("permission.ask") call before
the
early-return guard. The hook receives the current status ("ask",
"allow",
or "deny") and plugins can override it — e.g. a security plugin can
deny
a command that rules would otherwise allow.

Three existing tests that assumed synchronous pending-list population
are
updated to use the existing waitForPending() helper, since the async
Plugin.trigger call now precedes the deferred creation.

How did you verify your code works?

  • bun test test/permission/ — 84 tests pass (0 fail)
  • bun typecheck — 0 errors
  • Added 4 regression tests in
    test/permission/permission-hook.test.ts:
    • Hook fires for first-encounter commands (needsAsk=true)
    • Hook fires for already-allowed commands (needsAsk=false)
    • Hook can override ask → allow (skips permission dialog)
    • Hook can override allow → deny (blocks allowed command)

Screenshots / recordings

N/A (no UI change)

Checklist

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

The permission.ask plugin hook was inside an if (!needsAsk) guard,
preventing it from firing for first-encounter commands. Plugins could
not participate in permission decisions for new commands.

This moves the hook trigger before the early return so it fires
regardless of needsAsk, allowing plugins to override the permission
status (ask/allow/deny) for all commands.

Existing tests updated to account for the async plugin hook trigger.

Closes anomalyco#19927
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

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

Found potential related PRs:

  1. PR feat(llm): integrate GitLab DWS tool approval with permission system #19955 - "feat(llm): integrate GitLab DWS tool approval with permission system"

    • Related because it integrates the permission system with tool approval
  2. PR fix(opencode): add permission.ask plugin hook back #19453 - "fix(opencode): add permission.ask plugin hook back"

    • Directly related - appears to address the same permission.ask hook functionality
  3. PR feat(opencode): wire permission.ask plugin hook #19470 - "feat(opencode): wire permission.ask plugin hook"

    • Directly related - wires the permission.ask plugin hook, likely covers similar ground

These PRs should be reviewed to ensure they're not duplicates or if they conflict with the changes in PR #20634.

@YumaKakuya
Copy link
Copy Markdown
Author

I noticed #19453 and #19470 also address this issue. This PR was based on the latest dev branch and includes regression tests for the hook behavior. Happy to close this if either of the existing PRs is preferred — just wanted to make sure the test coverage was available regardless of which approach is merged.

Prevent Plugin.trigger from hanging the permission flow in environments
where the plugin runtime initializes slowly. Falls back to the original
permission status if the hook does not respond within 5 seconds.
@YumaKakuya
Copy link
Copy Markdown
Author

Windows unit runner lost connection during post-test cleanup (tests completed: 300 pass / 0 fail before disconnect). All other checks are green. Re-running the Windows job.

The permission.ask hook trigger has a 5s timeout, but waitForPending()
polls with Bun.sleep(0) — in slow CI environments, the pending entry
isn't created before polling ends, causing the test to hang until the
30s test timeout.
@YumaKakuya YumaKakuya force-pushed the fix/permission-ask-hook-bypass branch from ec330e3 to 8f42e6f Compare April 2, 2026 18:18
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.

permission.ask plugin hook is bypassed for first-encounter commands (needsAsk=true)

1 participant