Skip to content

fix: respect user decisions on permission.acp_status in host config write (#457, #465) - #466

Merged
ranxianglei merged 2 commits into
masterfrom
2026-09-29_respect-explicit-acp-status
Sep 30, 2026
Merged

ranxianglei merged 2 commits into
masterfrom
2026-09-29_respect-explicit-acp-status

Conversation

@ranxianglei

@ranxianglei ranxianglei commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Problem ([#457], [#465])

An explicit user decision on permission.acp_status in opencode.json was silently rewritten to "allow" by the plugin's config hook:

if (!hasExplicitToolPermission(opencodeConfig.permission, "compress")) {
    opencodeConfig.permission = {
        ...permission,
        compress: config.compress.permission,
        acp_status: "allow",   // ← re-keys an explicit user deny/ask (later key wins)
    } as typeof permission
}

[#457] — the guard only checked for the compress key, never acp_status. A user config of { "permission": { "acp_status": "deny" } } (no compress rule) came out as "allow". The inverted map was then cached into hostPermissions.global, and because the V1 host reads its own permissions from the same object, the deny was honoured by nobody — no log, no error, recovery only via config edit + restart.

[#465] — even after the exact-key guard, a user wildcard decision resolving acp_status ("*": "deny", "acp*": "deny") still got overwritten: no own acp_status key exists, so ACP appended acp_status: "allow" after the user's rules — and last-match resolution makes the appended default win. Grouped here per owner scoping; both instances share one root cause: the guard tested key presence instead of rule resolution.

Fix

Extracted the merge into a pure function applyAcpToolPermissions(permissionConfig, compressPermission) in lib/host-permissions.ts (the plugin factory in index.ts has heavy runtime deps and isn't unit-testable directly):

  • Explicit user compress rule → the user owns the map, returned unchanged (same identity) — defer-by-design preserved.
  • Otherwise → write compress from ACP config, and default acp_status to "allow" only when no user rule resolves it.

New helper anyRuleResolves(permissionConfig, tool): true for an exact own key in any value form (scalar, nested pattern object, degenerate empty object — key presence short-circuits), or any wildcard key ("*", "acp*") matched through the existing rule machinery (getPermissionRules + findLastMatchingRule + wildcardMatch) — the same last-match model the host resolution uses. An explicit acp_status (deny/ask) and a wildcard resolving it now survive untouched.

index.ts replaces the 8-line inline block with one call to this function. The bili-proxy denyAcpTools path (intentional full deny) is untouched. The compress defer guard deliberately stays key-based (owner scoped this fix to acp_status; wildcard-deny on compress is already handled by the separate self-disable path).

Behavior change disclosure (old → new)

Scenario (user opencode.json) Old New
permission.acp_status absent, no wildcard matches it acp_status: "allow" written unchanged
permission.acp_status: "deny" / "ask", no compress rule ([#457]) silently rewritten to "allow" ❌ survives untouched ✅ (a denied acp_status removes the tool from the LLM tool list, as denying it should do)
wildcard resolving acp_status ("*", "acp*") with deny/ask/allow, no compress rule ([#465]) ACP appends acp_status: "allow", which wins last-match and erases the user's decision ❌ nothing written — host resolves acp_status from the user's wildcard ✅
non-matching wildcard only ("bash*": "deny") acp_status: "allow" written unchanged
explicit compress rule present full defer, nothing written unchanged (same object identity preserved)

No state format, config schema, persistence, or public API change. No version bump (release workflow handles that).

Verification

Devlog: devlog/2026-09-29_respect-explicit-acp-status/ (REQ + WORKLOG, extended for #465 scope).

Fixes #457
Fixes #465

…te (#457)

The config hook wrote { ...permission, compress: X, acp_status: "allow" } whenever no explicit compress rule existed — the spread re-keyed an explicit user acp_status to "allow" (later key wins), silently inverting the access decision. The inverted map was then cached in hostPermissions.global and read back by the V1 host itself.

Extract the merge into pure applyAcpToolPermissions() in lib/host-permissions.ts: explicit compress rule defers fully (map returned unchanged); otherwise acp_status defaults to allow only when absent. Regression tests verified RED against the buggy logic, GREEN after fix.
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Plugin Artifact

Branch: 2026-09-29_respect-explicit-acp-status (087e25c)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-466 --global

Each push to this PR publishes a new version under the pr-466 npm tag.

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-09-29_respect-explicit-acp-status" --global

Option C — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf opencode-acp-pr466.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
  1. Restart opencode to pick up changes.

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Reviewing this PR now (fix for #457). Pulling the branch, re-verifying locally: typecheck/build, targeted suite, RED→GREEN of the regression tests, and CI status. Full review conclusion to follow shortly.

…te (#465)

The #457 guard only checked for an exact own acp_status key, so a user wildcard decision resolving acp_status ('*': 'deny', 'acp*': 'deny') was still overwritten by ACP appending acp_status: 'allow' last (last-match wins). New anyRuleResolves() gates the default write on rule resolution instead of key presence, reusing the same rule machinery the host resolution models.
@ranxianglei ranxianglei changed the title fix: never override explicit permission.acp_status in host config write (#457) fix: respect user decisions on permission.acp_status in host config write (#457, #465) Sep 29, 2026
@ranxianglei
ranxianglei merged commit b67767c into master Sep 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant