fix(app): reflect global permission:allow in Settings auto-accept toggle - #46226
Open
d-morrison wants to merge 1 commit into
Open
fix(app): reflect global permission:allow in Settings auto-accept toggle#46226d-morrison wants to merge 1 commit into
d-morrison wants to merge 1 commit into
Conversation
When permission is set to "allow" globally (in opencode.json), the Settings -> Auto-accept permissions toggle previously showed off/disabled even though permissions were already auto-approved via config. This disconnect is reported across anomalyco#38154, anomalyco#41069, anomalyco#16258, anomalyco#37617. Make accepting() return true when global permission is "allow" or {"*":"allow"}, and disable the toggle (global config overrides per-directory auto-accept) so UI matches runtime. Fixes the visual mismatch where a new session shows toggle off after restarting with global auto-accept enabled.
github-actions
Bot
added
the
needs:compliance
Contributor
|
The following comment was made by an LLM, it may be inaccurate: The search results show related PRs to the current one (PR #46226), but they appear to be addressing similar but distinct issues: Related PRs found (not direct duplicates):
These appear to be related work in the same feature area (auto-accept permissions) but are distinct from PR #46226, which specifically addresses reflecting the global permission configuration in the Settings toggle UI. No duplicate PRs found |
github-actions
Bot
removed
the
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
d-morrison
added a commit
to Morrison-Lab/ai-config
that referenced
this pull request
Aug 30, 2026
Do: fetch and follow external repo's CONTRIBUTING.md and .github/pull_request_template.md before opening PR Don't: file with internal/generic template without checking required sections Fixes compliance failure on anomalyco/opencode#46226 where PR body missed required template sections (needs:compliance).
d-morrison
added a commit
to Morrison-Lab/ai-config
that referenced
this pull request
Aug 30, 2026
…ises (#2664) * ums: capture GIA learnings — heredoc hook, chapter wiring, empty promises - Security hook: don't heuristic-strip heredoc bodies; keep full scan - Quarto new chapter: format block, navbar, index, WORDLIST + lychee - Inline R: formatC with format="d" to avoid scientific notation - No empty promises: don't use 'will' for future work without mechanism; daytb covers GIA checkpoint * fix(plugin): use stable absolute path instead of extensionPath in hooks.json * fix(ums): address review findings (daytb claim, em-dashes, sembr) * fix(ums): apply sembr and do/dont formatting, use ascii dash * fix(ums): fix antigravity.md headings and verification * fix(ums): move quarto learnings to r-quarto.md to avoid max lines limit * memorize: check external repo guidelines and PR template before filing Do: fetch and follow external repo's CONTRIBUTING.md and .github/pull_request_template.md before opening PR Don't: file with internal/generic template without checking required sections Fixes compliance failure on anomalyco/opencode#46226 where PR body missed required template sections (needs:compliance). * fix(hooks): block unsupported CI forecasts * style: use standard markdown em-dashes in AGENTS.md * style(ums): semantic line breaks for GIA additions
d-morrison
added a commit
to Morrison-Lab/ai-config
that referenced
this pull request
Aug 31, 2026
…clean (#2736) * ums: capture GIA learnings — heredoc hook, chapter wiring, empty promises - Security hook: don't heuristic-strip heredoc bodies; keep full scan - Quarto new chapter: format block, navbar, index, WORDLIST + lychee - Inline R: formatC with format="d" to avoid scientific notation - No empty promises: don't use 'will' for future work without mechanism; daytb covers GIA checkpoint * fix(plugin): use stable absolute path instead of extensionPath in hooks.json * fix(ums): address review findings (daytb claim, em-dashes, sembr) * fix(ums): apply sembr and do/dont formatting, use ascii dash * fix(ums): fix antigravity.md headings and verification * fix(ums): move quarto learnings to r-quarto.md to avoid max lines limit * memorize: check external repo guidelines and PR template before filing Do: fetch and follow external repo's CONTRIBUTING.md and .github/pull_request_template.md before opening PR Don't: file with internal/generic template without checking required sections Fixes compliance failure on anomalyco/opencode#46226 where PR body missed required template sections (needs:compliance). * fix(hooks): block unsupported CI forecasts * style: use standard markdown em-dashes in AGENTS.md * style(ums): semantic line breaks for GIA additions * feat(review): support structured review-bot output in check-pr-fully-clean * fix(check-pr-fully-clean): import Any and Set and add future annotations * fix(review): address adversarial review findings on prompt builder and SHA matching * fix(review): gate structured reviewer identity on bot login or agent marker * fix(pre-push-review): tolerate agent disclosure footer after fingerprint * fix(pre-push-review): increase Claude CLI timeout to 600s * fix(pre-push-review): bound Observations section regex to next heading * fix(review): drop fence from reviewer prompt review-data and restore docstrings * fix(review): address PR review findings on gha claim, trailing guard, and SHA matching * fix(review): ensure prose findings override structured clean and preserve agent marker precedence * fix(pre-push-review): allow trailing status banner after fingerprint * fix(pre-push-review): anchor trailing verdict alternative and add unit test * fix(review): preserve heading and fingerprint requirements for structured review body admission * fix(pre-push-review): allow heading suffixes in section extractor * fix(pre-push-review): accept 1-6 hashes in extract_section and clarify Verification Steps in prompt * fix(review): close structured-payload fail-opens found in post-merge review The post-merge adversarial review of the merged branch returned twelve findings, five of them letting a not-clean artifact score clean. check-pr-fully-clean.py: - Drop the two Claude Code disclosure-footer entries from REVIEW_AGENT_MARKERS. CLAUDE.md mandates that footer on EVERY agent-posted comment -- claims, status updates, replies -- not on reviews only, so admitting it made each of those a quorum-eligible automated review with identity "Claude". Measured: an owner comment ending in the footer superseded a real bot "Needs more work", and satisfied quorum on a PR carrying no automated review at all. That reopened #2308's invariant (approval authority comes from author identity, never body text) and, via is_non_review_notice, the #1719 skip-notice admission path. - The LAST valid payload now wins, not the first. The persona template a reviewer may quote hardcodes "verdict": "CLEAN" and the authoritative payload comes last, so first-match-wins let a NOT_CLEAN review score clean. - Code spans and indented code blocks are masked alongside fences, so a comment that merely mentions the format is no longer read as a review. - Remove the <details>-plus-JSON-fence path. It spanned arbitrary distance and skipped the fence mask, so a collapsed earlier payload minted a blocking finding no ARD round could discharge (#2482 class). pre-push-review.py: - Remove the empty-matching \s* alternative from the fingerprint anchor's * quantifier. It duplicated the group's own leading \s* and backtracked catastrophically: measured 1.26s at 12 whitespace characters and 21.4s at 14, ~3x per added character, with no timeout around parse_review_verdict. Now linear (0.48ms at 2000). - Validate the payload the built prompt asks for. HTML comments were stripped before every check, so a report whose payload said NOT_CLEAN parsed as "Verdict: CLEAN" locally while check-pr-fully-clean.py scored the same artifact blocking. Both consumers now read the payload through one extractor, scripts/lib/review_payload.py, so they cannot drift apart again. Findings block regardless of the stated verdict; prose findings still override a structured clean. Also: state the finding-object schema (file/line/category/message) in both persona files and in the built prompt rather than asking for unspecified "finding objects"; accept a finding's "summary" key as a fallback, since the persona's own ReportFindings contract uses it; restore the emit-nothing-after constraint to the prompt; and move the structured-data section in adversarial-self-review.md out of the middle of the preceding section, whose #2444 provenance parenthetical it had orphaned. Verified: 510 + 49 tests pass; check-verdict-scan-parity.py over 400 bodies reports 0 widened, 0 narrowed, with its negative control discriminating (103 divergences); links, memory-file-size, skills, test-suite coverage, reviewer-placeholder and hook-catalog checks green; semantic-line-breaks clean on every changed markdown file. * fix(pre-push-review): clean heading and section body separation in extract_section * test(review_payload): add dedicated unit tests and wire into validate.yml * fix(pre-push-review): support optional bold and hyphen/space in fingerprint regex * fix(review): close round-2 fail-opens in the structured-payload path Second adversarial review of the fixes found six more, three of them letting a not-clean artifact score clean. - pre-push-review.py: the rewritten fingerprint anchor still backtracked exponentially. Removing the empty `\s*` alternative was necessary and not sufficient: `={3,}` is self-ambiguous under the outer `*`, since a run of `=` splits into chunks of size >= 3 exponentially many ways. Measured on this tool's OWN `"=" * 60` report banner followed by any non-matching text: 0.50s at 36, 4.01s at 42, 14.18s at 45. Replaced the nested-quantifier regex with a line scan, which is linear by construction and cannot reacquire the class from a future alternative (0.36ms at 4000 `=`). - review_payload.py: `find_fence_spans` defaults to `swallow_unclosed=False`, which records only an unclosed fence's OPENER line and leaves its interior live -- so the mask failed in both directions. A truncated review quoting a CLEAN template scored clean, and a quoted NOT_CLEAN payload minted an undischargeable finding. check-pr-fully-clean.py already passes `swallow_unclosed=True` for this reason; now so does this. - check-pr-fully-clean.py: a payload's self-declared `reviewer` no longer confers identity. It is body text the reviewer writes about itself, so trusting it inverts #2308's invariant -- and it is load-bearing for quorum: measured, two clean comments from the single `github-actions[bot]` login whose payloads named different reviewers satisfied `--quorum 2`, which shared/workflow/fully-clean.md makes the normal invocation. - Drop the `review-json` payload alias. Nothing in the corpus emitted or documented it, so it was a second verdict-bearing input spelling with no producer. - Drop the dead `(oid and oid.lower() == sha_lower)` term from `is_sha_match`. It is read only where `oid` is falsy, so it can never contribute -- while reading as having made formal-review OID matching case-insensitive, which `is_match = (oid == sha)` is not. - check-verdict-scan-parity.py: the corpus carried no `review-data` fragment, so its zero for this change was a coverage statement -- the same failure its own VOCAB comment records for #2668. Added a bounded payload arm crossing placement (bare, fenced, unclosed fence, code span, indented), which payload is authoritative, and verdict/findings agreement. Yielded FIRST, deliberately: `--limit N` truncates the generator and the full product is ~242k bodies, so an arm yielded last sat at index 241,920 and was unreachable at any limit run by hand. Verified: 516 + 50 + 12 tests pass; parity over 600 bodies (all 32 payload bodies among them) reports 0 widened, 0 narrowed, negative control 157 divergences -> DISCRIMINATES; links, memory-file-size, skills, test-suite coverage, reviewer-placeholder and hook-catalog checks green; semantic-line-breaks clean across all four changed markdown files. * fix(pre-push-review): one fingerprint pattern for both checks `parse_review_verdict` matched the `Reviewed-Commit:` line with two separate literals -- once to harvest the SHA, once to scan for content after it -- and they drifted apart within a single session. 9230833 loosened the harvest to accept `**Reviewed-Commit**` and `Reviewed Commit`; 6fe7d21 had independently replaced the second site with a line scan (the regex it replaced backtracked exponentially on this tool's own `"=" * 60` banner). A report in the loosened form then passed the harvest and matched nothing in the scan, so `last_fp` stayed None and `last_fp.end()` raised. Hoist both onto one `_FINGERPRINT_RE` carrying the loosened form, add a fail-closed guard on the now-unreachable None, and cover all five spellings -- plain, bold, space, bold-and-space, padded colon -- against both checks, each also asserting that trailing chatter is still rejected rather than silently unlocatable. Verified: 516 + 51 + review_payload + parity suites pass. * fix(pre-push-review): use extracted section variables in critical findings validation * fix(pre-push-review): allow heading variations in section extraction while preserving persona delegation * fix(review): close round-3 findings in the structured-payload path Third adversarial review found seven more, two of them fail-opens. - pre-push-review.py: a verdict restated AFTER the fingerprint was tolerated in position and then read by nothing -- `verdict_matches` scans the Summary section only -- so a report ending `### Verdict: Needs more work` parsed as (True, True, 'Verdict: CLEAN') where the pre-line-scan regex had rejected it outright. Trailing verdict lines now go through the same evaluation the Summary verdict does, so a not-clean restatement blocks and an unrecognized one invalidates. - review_payload.py: a `findings` value that is PRESENT but not a list was folded to `[]`, letting a type deviation clear a payload the way an empty array does. Measured end to end: a bot comment whose payload read `"findings": "3 defects listed above"` satisfied quorum and the gate reported fully clean. `payload_findings_malformed` now blocks it. - pre-push-review.py: `_FINGERPRINT_RE` was `^`-anchored and so rejected the three-space-indented `Reviewed-Commit:` line this file's own `build_review_prompt` renders. Re-derived from `hooks/no-push-without-self-review.py`'s REVIEWED_COMMIT -- the pattern the persona file names as authoritative -- so bold-after-colon and a backticked sha are accepted too, plus leading indentation. - check-verdict-scan-parity.py: every payload body carried a prose `## Verdict:` line, which decides before control reaches `payload_is_clean` -- the only new acceptance-widening branch. Reached 0 of 32 times, so the arm's zero was still a coverage statement. Added bodies with no prose verdict, plus malformed and findings-absent payloads; `payload_is_clean` is now reached 22 of 57 and the arm produces three distinct outcome pairs. - test_check_pr_fully_clean.py: the two short-prefix SHA checks re-implemented `startswith` and measured a string length in the test body rather than calling `check_review_comments`. Replaced with three that call it, with the full sha ABSENT from the comment so only the structured prefix can match. Mutation-checked: deleting `len(struct_sha) >= 7` now fails the suite (it did not before). - Corrected a comment attributing the `"=" * 60` banner to `log_error`, which prints only `Error: {msg}`; it is `main`'s report separator. - test_review_payload.py: three imports were unused. Added direct tests for `code_region_mask` (per-region attribution plus offset arithmetic on CRLF, tabs, multi-byte and a missing trailing newline) and for the two verdict vocabularies being disjoint and pre-normalized. Verified: 517 + 52 + 14 + 12 tests pass; both new regression tests mutation-checked to fail against the pre-fix behaviour; parity over 700 bodies reports 0 widened, 0 narrowed, control 189 divergences -> DISCRIMINATES; links, memory-file-size, skills, test-suite coverage, reviewer-placeholder and hook-catalog checks green; semantic-line-breaks clean across all four changed markdown files. * fix(review): close round-4 findings in the structured-payload path Fourth adversarial review found nine more. Two were fail-opens, one was a documented invariant that is false where it matters most. - review_payload.py: the payload must now BEGIN A LINE. Masking code regions was not enough on its own -- a payload written mid-sentence in ordinary prose ("Reviewers must end with <!-- review-data: ... -->") or behind a `> ` blockquote marker, which is what GitHub's Quote reply emits, sits in no code region at all. Measured both directions: a mention of the format scored the comment CLEAN, and a narrated earlier round minted a structured finding no push could discharge. - review_payload.py: `payload_is_clean` now requires `findings` to be present, a list, and empty. Both persona files and the built prompt say a CLEAN payload requires an empty array and nothing enforced it, so a payload that simply omitted the key cleared -- inconsistent with the blocking treatment a wrong-typed value already got, and an omitted required key is the commoner model failure of the two. - review_payload.py: dropped `except Exception: fenced_lines = set()` around the fence scan. It under-masked -- the direction the same docstring calls unsafe -- silently, on any failure, disabling fence masking for the whole body. The sibling call in check-pr-fully-clean.py is unwrapped for the same reason (fail-fast.md). - pre-push-review.py: a malformed `findings` field was reported as "the payload reports verdict CLEAN", which is not a contradiction and is not why it blocked. The reason now names the actual cause and its type. - check-pr-fully-clean.py: removed the structured `commit_sha` term from `is_sha_match`. It was provably unable to change the result -- `sha_short` is the target's own 7-character prefix, so any payload `commit_sha` long enough to be accepted contains it, and the body-substring disjunct already matched. Mutation confirmed: deleting the term left every structured-sha test passing. A dead disjunct that reads as a feature is worse than no feature, since the tests appear to cover it. - pre-push-review.py: `_FINGERPRINT_RE` claimed to accept every form the pre-push hook's REVIEWED_COMMIT accepts. It does not -- the `^` anchor rejects inline, blockquoted and list-item fingerprints the hook matches. The anchor is deliberate (the match position decides where trailing content begins), so the comment now states the two narrowings and two widenings instead of claiming parity, and the `{7,40}` length bound the hook applies is restored. - pre-push-review.py: moved the "lines tolerated after the fingerprint" comment back above the constant it describes. - adversarial-self-review.md and both persona files: the documented "findings block regardless of the stated verdict" invariant is FALSE for hooks/no-push-without-self-review.py, which blanks HTML comments before parsing -- the guard that actually blocks the push. Measured: a clean prose verdict with a NOT_CLEAN payload parses there as clean. Corrected the docs to say so and to tell reviewers to state the prose verdict truthfully. Teaching the hook to read the payload is deliberately NOT done here, since this branch's own pushes are gated by that hook; tracked as ai-config#2749. - Covered the two behaviours that survived mutation with every suite green (the persona-path `_structured_contradiction` guard, and the `review-data:` REVIEW_BODY_MARKERS entry), and added direct tests for `code_region_mask`. Verified: 520 + 52 + 17 + 12 tests pass; all six round-4 fixes mutation-checked to fail the suite when reverted; parity over 700 bodies reports 0 widened, 0 narrowed, control 189 divergences -> DISCRIMINATES; every checker in validate.yml green; semantic-line-breaks clean across all four changed markdown files; no non-ASCII in added lines. Refs #2749 * fix(review): close round-5 findings; payload prefix must be whitespace, not column zero Fifth adversarial review found seven. One was a regression the previous round introduced. - review_payload.py: the column-zero rule was worse than the hole it closed. `build_review_prompt` and both persona files render the payload three spaces in, so a reviewer following the prompt's own layout had its payload silently dropped -- a NOT_CLEAN payload ignored and the guard reporting clean, the exact failure this path exists to prevent. One leading space was enough. The rule is now that only WHITESPACE may precede the payload on its line, which still rejects a `> ` blockquote (GitHub's Quote reply), a `- ` list item, and a mid-sentence mention. Three spaces stays readable while four does not, and that boundary is CommonMark's rather than an arbitrary one: four spaces is an indented code block, which `code_region_mask` masks. The prompt and both personas now render the payload FLUSH LEFT and say so explicitly, so a four-space payload is a deviation from a stated instruction reading as the code block it is. This also makes the mask's indented-block test live again (round 4's review had it dead), mutation-confirmed. - check-pr-fully-clean.py: RESTORED the structured `commit_sha` term that round 4 deleted as "provably" inert. The proof was wrong: `json.loads` resolves escapes, so a payload carrying `"commit_sha": "abc1234..."` decodes to a match while the literal body text reads `abc12...`, which neither substring disjunct sees. Deleting it made that case fail closed -- a real review stops counting as evaluating HEAD, reported as "no review posted". The counterexample is now a test, and the comment states what is actually true rather than a proof that does not hold. - check-pr-fully-clean.py: `_unresolved_finding_pattern` now checks for a malformed `findings` field BEFORE the verdict branch, for the reason its sibling in pre-push-review.py already states -- otherwise it printed "structured blocking verdict (CLEAN)", naming the payload's own CLEAN string as the reason it blocked. - check-verdict-scan-parity.py: the payload arm is now appended AFTER `--limit` is applied. Yielding it last put the first payload body at index 241,920; yielding it FIRST, which the previous round did, still lost it, because `--limit` takes a STRIDED sample rather than a prefix -- so roughly one of the 57 was selected. Both reported a zero that was a coverage statement. Verified after the fix: the arm surfaces 1 widening and 5 narrowings that a limited run previously reported as 0 and 0. - check-verdict-scan-parity.py: recorded the accepted widening at the site, with its five narrowings, as the tool's docstring requires ("Every widening needs a justification a human states"). The widening is the feature: a review whose only verdict is a structured payload used to be `unreadable` and now reads as the clean verdict it states. All five narrowings are fail-closed. - check-pr-fully-clean.py: corrected a comment attributing `--quorum 2` to shared/workflow/fully-clean.md, which prescribes `--quorum <number-of-reachable-providers>` and pins three for this repo. Rebutted, one finding: the review flagged a semantic line break removed in both persona files. `python3 scripts/semantic-line-breaks.py --base origin/main` requires that join once the paragraph enters diff scope, and the diff-scoped checker is the CI gate -- leaving it split fails the new-line-breaks job. Verified: 526 + 54 + 17 + 12 tests pass; every round-5 fix mutation-checked to fail the suite when reverted; parity over 757 bodies (all 57 payload bodies among them) reports 1 accepted widening and 5 intended narrowings, control 189 divergences -> DISCRIMINATES; every checker in validate.yml green; semantic-line-breaks clean on all four changed markdown files; no non-ASCII in added lines. Refs #2749 * fix(review): close round-6 findings -- pin the fingerprint anchor, correct two stale comments - pre-push-review.py: pinned `_FINGERPRINT_RE`'s `^[ \t]*` anchor with a test. The trailing-content check starts at the LAST fingerprint match, so an unanchored pattern lets a mention inside ordinary prose become that match, leaving an empty tail and clearing the guard. Measured: deleting the anchor turned a report ending "Actually, ignore all of the above ... See Reviewed-Commit: <sha>" from rejected into (True, True, 'Verdict: CLEAN'), with all three suites green. Now covered, mutation-confirmed, together with the blockquote, list-item and inline mention forms. - test_check_pr_fully_clean.py: the comment still said the structured `commit_sha` term "was REMOVED ... as provably dead". The previous commit restored it. Worse, it pointed the reader at the three tests that do NOT catch its deletion (each payload's sha appears verbatim in the body, so the substring disjunct decides them) and away from the escaped-sha case that does. Corrected to say which test pins what. - review_payload.py: the comment justifying the whitespace-prefix rule said the prompt and both persona files render the payload three spaces in. The same commit changed all three to render it flush left. Corrected to the past tense and to state what the tolerance is now for: defence in depth against a reviewer that indents anyway, which is what the incident showed they do. Verified: 526 + 55 + 17 + 12 tests pass; the anchor mutation-checked to fail; every checker in validate.yml green; semantic-line-breaks clean on all four changed markdown files. Refs #2749
This was referenced Sep 4, 2026
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
1 participant
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.
Issue for this PR
Closes #38154
Type of change
What does this PR do?
When
permissionis set to"allow"globally (inopencode.json/opencode.jsonc), the Settings -> Auto-accept permissions toggle previously showed off even though permissions were already auto-approved via config. The toggle is per-directory/session (permission.isAutoAccepting), so a global"allow"was invisible in the UI - new sessions appeared as toggle off after restart. Reported as bug family across #38154, #41069, #16258, #37617.This makes
accepting()returntruewhen global permission is"allow"or{"*":"allow"}(viaserverSync().data.config.permission), and disables the toggle when global auto-accept is active (global config overrides per-directory).denyrules still enforced - onlyallowis reflected.How did you verify your code works?
opencode debug configshowspermission: {"*":"allow"}loaded from~/.config/opencode/opencode.jsonc:3bun run typecheckinpackages/app- passed (no new deps)permission: "allow"Settings toggle now shows on and disabled; without it, per-directory behaviour unchanged (checkedsettings-general.tsxandsettings-v2/general-controllers.ts)Screenshots / recordings
No visual change except toggle state. With global
permission: "allow"toggle showson+ disabled; without, existing enabled/disabled/checked states unchanged (same as #43193 coverage).Checklist