fix(guardrails): stop primary agent deny overriding config merge allow - #293
Merged
Conversation
#292) The default primary agent (implement) declared "gh pr merge *": "deny" in its frontmatter. agent.ts merges a user-defined agent's permission AFTER the user config and Permission.evaluate is last-wins, so the deny overrode the explicit "gh pr merge *": "allow" in opencode.json — silently blocking the main session from merging despite the config granting it (pattern I over-restriction). Remove the only primary-agent deny that contradicts a config allow. Specialized primary agents (planner) keep their intentional read-only denies. Add real-file regression tests (the prior mirror fixture missed this because it evaluated implement.md in isolation, not the merged effective ruleset): the effective permission now allows gh pr merge, the falsifiable deny case still blocks, destructive denies (rm -rf / force-push / reset --hard) still hold, and planner may still self-restrict.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #292.
The default primary agent (
implement) declared"gh pr merge *": "deny"in its frontmatter.agent.tsbuilds a user-defined agent's permission asmerge(merge(defaults, userConfig), agentDefinition), andPermission.evaluateis last-wins (findLast). So the agent definition is applied AFTER the user config and overrides it. For the default primary agent (the main session agent), the deny therefore overrode the explicit"gh pr merge *": "allow"inopencode.json— silently blocking the main session from merging despite the config granting it (pattern I over-restriction).This is the root cause of the 2026-08-04 incident where 4 CI-green PRs were blocked for hours after the entrypoint was re-pinned to a profile whose
implement.mdcarried the deny.Why the existing symmetry test missed it
implementBashRuleset()evaluatedimplement.md's bash block in isolation, never the merged effective ruleset thatagent.tsactually produces. The bug only appears in the merge.Change
packages/guardrails/profile/agents/implement.md: removed the single conflicting"gh pr merge *": "deny"(the only primary-agent deny that contradicts a config allow). Added a maintainer comment explaining why a deny must not be re-added here. All other denies (rm -rf,force-push,reset --hard, …) remain — they are consistent with the config.packages/guardrails/profile/agents/planner.md: unchanged.planneris a read-only primary agent; itsgh pr merge*deny is intentional and not the bug.Tests (falsifiable, real-file)
New
anti-pattern I — primary-agent permission leak (issue #292)block inanti-pattern-guards.test.tsreads the real agent files (not a hand-mirrored fixture) so drift cannot hide the regression again:gh pr merge …per configdeny(proves the test mechanism detects the regression; flippingimplement.mdback resurfaces it)rm -rf,git push --force,git reset --hard) remain deniedProof
expect(...).toBe("allow")→Received: "deny"anti-pattern-guards.test.tstsgo --noEmitcleanlocal:deploygreen (binary0.0.0-fix-merge-perm-leak-*, entrypoint repinned);local:checkpermission/guardrail/team smokes all passScope notes / follow-ups (tracked, not fixed inline)
git merge *onimplement.mdis a milder case of the same pattern (config=ask, agent=deny). Not reported, left out of this PR for focus.agent.ts:293merge-order asymmetry (user-defined agents override config; native agents do not) is an upstream concern, intentionally not patched here per the profile's "prefer profile over core runtime" rule.local:checkhas 2 pre-existing failures unrelated to this change: OpenRouter catalog freshness (qwen/qwen3.8-maxnot yet whitelisted) and theentrypoint_guardrails_smoke(manualopencode debug infois clean).Closes #292