fix(opencode): surface prompt denials - #3679
fix(opencode): surface prompt denials#3679Kerberosmansour (kerberosmansour) wants to merge 1 commit into
Conversation
Signed-off-by: kerberosmansour <13433538+kerberosmansour@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
|
🟡 Contributor Check: MEDIUM
Automated check by AGT Contributor Check. |
github-actions
Bot
added
the
needs-review:MEDIUM
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Imran Siddique (imran-siddique)
left a comment
Imran Siddique (imran-siddique)
left a comment
There was a problem hiding this comment.
Three changes here and the summary undersells the third, so taking them in order of how much they matter.
A missing explicitly-configured policy file is now an initialization error. This is the security fix in the PR. An operator who sets a policy path that does not resolve previously got a silent fall back to the bundled default: the plugin runs, governance appears active, and the rules being enforced are not the ones anyone chose. Every symptom points away from the cause, because a typo'd path in a config file produces a working system enforcing a different policy. Treating it as an initialization failure is right, and it is right specifically because it is loud.
Eager validation at init when denyOnPolicyError is set. Same principle one level up. A latent load error that surfaces at some later hook fails at an arbitrary moment, in the middle of a session, in whatever hook happens to run first. Failing at startup puts it where an operator is looking.
Surfacing the denial. Worth noting the comment this replaces was already in the tree: throwing here silently breaks the OpenCode session. Exception message is never displayed to the user, this is not the way to go. Someone had diagnosed it and left it. Closing a known gap that was already written down in the code is a good use of a PR.
I checked the part that could have quietly broken the guarantee. surfaceGovernanceDenial feature-detects both APIs with typeof ... === "function", and closes on await Promise.allSettled(notifications) rather than Promise.all. That matters: with all, a rejecting or absent toast would throw out of the notification helper and replace the governance exception with a UI error, so the request would still be blocked but the reason would be wrong and the audit trail would say the wrong thing. allSettled keeps notification failure from displacing the denial. The throw is also still unconditional after it, so visibility is additive to blocking rather than a precondition for it.
The gitleaks red on this was the same platform 503 that hit several PRs today, not your diff. I re-ran it and it passes.
No approve bit on this repo, so a comment, but I would merge it.
2 participants
Summary
Root cause
Prompt denials were represented only by a thrown exception, and explicit policy load failures could remain latent until a later hook. The adapter now uses OpenCode's supported toast/log APIs to make the denial visible while retaining the exception required to stop execution.
Validation
cd agent-governance-opencode && npm run check— 28/28 tests passedCloses #3663
Tracking and related work