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

devlog/2026-09-29_respect-explicit-acp-status/REQ.md

@@ -0,0 +1,98 @@
+# REQ - Respect explicit `acp_status` in host permission map
+
+- Task ID: `2026-09-29_respect-explicit-acp-status`
+- Home Repo: `opencode-acp`
+- Created: 2026-09-29
+- Status: InProgress
+- Priority: P0 (BLOCKER per issue)
+- Owner: ework-daemon
+- References:
+    - https://github.com/ranxianglei/opencode-acp/issues/457 (BLOCKER — exact own-key case)
+    - https://github.com/ranxianglei/opencode-acp/issues/465 (MINOR — wildcard case; owner grouped it
+      with #457 for a proper fix)
+
+## 1. Background & Problem Statement
+
+- **Context**: The plugin's `config` hook (`index.ts`) writes ACP's own tool permissions into
+  opencode's host permission map so that ACP tools are usable by default. When the user has
+  not declared an explicit `compress` rule, it writes `{ compress: <acp config>, acp_status: "allow" }`.
+- **Current behavior (symptom)**: The spread unconditionally re-keys `acp_status` to `"allow"`,
+  silently overwriting an explicit user decision (`"deny"` or `"ask"`) in
+  `opencode.json → permission.acp_status`. The inverted map is then cached into
+  `hostPermissions.global`, and since the V1 host reads its own permissions from the same
+  object, the deny is not honoured by anyone. No log, no error. Recovery requires editing
+  the config and restarting.
+- **Expected behavior**: A user decision on `permission.acp_status` must survive the config
+  write untouched — whether expressed as an exact key (`"acp_status": "deny"`) or a wildcard
+  key resolving it (`"*": "deny"`, `"acp*": "deny"`; #465). The `"allow"` default applies only
+  when NO rule in the map resolves `acp_status`.
+- **Impact**: Silent inversion of an explicit access decision, shipped in the published
+  package; any user who denies only `acp_status` (without also declaring a `compress` rule)
+  gets the opposite of what they configured. #465 extends this to wildcard decisions: since
+  ACP appends its keys last and resolution is last-match, an appended `acp_status: "allow"`
+  erases a user wildcard deny/ask even though no own key exists.
+
+## 2. Reproduction
+
+- **Environment**:
+    - Node: 22+ (any supported)
+    - OS/Arch: platform-independent (pure JS object spread)
+- **Minimal reproduction steps**:
+    1. User config: `{ "permission": { "acp_status": "deny" } }` (no `compress` key).
+    2. `hasExplicitToolPermission(permission, "compress")` → `false`, guard passes.
+    3. `{ ...{ acp_status: "deny" }, compress: "allow", acp_status: "allow" }` → later key wins
+       → `acp_status: "allow"`.
+    4. `hostPermissions.global = opencodeConfig.permission` caches the inverted map.
+- **Relevant configuration**: `permission.acp_status` in `opencode.json`;
+  `compress.permission` in ACP config.
+
+## 3. Constraints & Non-Goals
+
+- **Constraints**:
+    - Backward compatibility: users with NO decision on `acp_status` (no exact key AND no
+      resolving wildcard) keep getting the `"allow"` default (unchanged). Users with an
+      explicit `compress` rule keep full defer behavior (unchanged) — including when other
+      wildcard keys are present. Bili-proxy denial path (`denyAcpTools`) is intentional and
+      untouched.
+    - The default write is strictly additive over the #457 fix: it only stops writing where a
+      user rule already resolves `acp_status`; no case that wrote before stops writing for any
+      other reason.
+    - Performance: pure function, O(map size) — negligible.
+    - The SDK `Config.permission` type is narrow (no arbitrary tool keys); keep the existing
+      `as typeof permission` cast pattern at the assignment site.
+- **Non-Goals** (explicitly out of scope):
+    - The pre-existing asymmetry where an explicit `compress` rule defers fully and
+      `acp_status` falls back to the host default ("ask") — consistent with defer-by-design.
+    - Making `dispose` restore the original host object (no clean hook available; the fix
+      removes the mutation hazard for `acp_status`).
+    - Bump version / changelog (feature branch; release workflow handles that).
+
+## 4. Acceptance Criteria (must be testable)
+
+- **Correctness**:
+    - [ ] Explicit `permission.acp_status: "deny"` survives the config write (value stays `"deny"`).
+    - [ ] Explicit `permission.acp_status: "ask"` survives the config write.
+    - [ ] Absent `acp_status` still gets the `"allow"` default (with `compress` written from ACP config).
+    - [ ] Unrelated host permission keys (e.g. `bash`) are preserved.
+    - [ ] Explicit `compress` rule → user owns the map, returned unchanged (identity preserved).
+    - [#465] Wildcard `"*"` deny/ask on the map → no `acp_status` default injected (user rule
+      wins last-match).
+    - [#465] Wildcard `"acp*"` deny → no `acp_status` default injected.
+    - [#465] Non-matching wildcards (e.g. `"bash*"`) still get the default injected.
+    - [#465] Nested pattern form `{ "acp_status": { "*": "deny" } }` counts as a user decision.
+- **Performance / Stability**:
+    - [ ] Full test suite + typecheck + build pass.
+- **Regression**:
+    - [ ] New regression tests added to `tests/host-permissions.test.ts`, verified RED against
+          the buggy logic and GREEN after the fix.
+
+## 5. Proposed Approach
+
+- **Affected modules & entry files**:
+    - `lib/host-permissions.ts` — new pure export `applyAcpToolPermissions(permissionConfig, compressPermission)`
+      (the plugin factory in `index.ts` has heavy runtime deps and is not unit-testable directly;
+      AGENTS.md §5.6 mandates extracting pure logic).
+    - `index.ts` — replace the 8-line inline block with a call to the extracted function.
+    - `tests/host-permissions.test.ts` — regression tests.
+- **Risks**: Low — single assignment site, all other consumers read the resulting map only.
+- **Rollback strategy**: revert the commit; behavior returns to the buggy overwrite.

devlog/2026-09-29_respect-explicit-acp-status/WORKLOG.md

@@ -0,0 +1,82 @@
+# WORKLOG - Respect explicit acp_status in host permission map
+
+- Task ID: `2026-09-29_respect-explicit-acp-status`
+- Home Repo: `opencode-acp`
+- Status: Done (extended for #465)
+- Updated: 2026-09-29
+
+## 1. Summary
+
+- **What was done**: Extracted the host-permission merge from the plugin factory into a new pure function `applyAcpToolPermissions(permissionConfig, compressPermission)` in `lib/host-permissions.ts`, and made it preserve user decisions on `permission.acp_status` instead of unconditionally rewriting them to `"allow"` — exact own key (#457) AND wildcard keys resolving the tool (`"*"`, `"acp*"`; #465). The inline block in `index.ts` now calls this function.
+- **Why**: [#457] — a user config of `{ "permission": { "acp_status": "deny" } }` (no `compress` key) passed the `hasExplicitToolPermission(…, "compress")` guard, then the object spread re-keyed `acp_status` to `"allow"` (later key wins), silently inverting the access decision. The inverted map is cached in `hostPermissions.global`, and the V1 host reads its own permissions from the same object, so the deny was honoured by nobody; recovery required editing config and restarting. [#465] — the #457 fix only checked for an exact own key (`hasOwnProperty`); a user WILDCARD decision resolving `acp_status` (`"*": "deny"`, `"acp*": "deny"`) still got overwritten, because ACP appends its keys last and resolution is last-match. Owner grouped #465 with #457 for a proper fix; the guard now gates on rule resolution instead of key presence.
+- **Behavior / compatibility changes**: Yes — user decisions on `permission.acp_status` survive the config hook unchanged instead of being rewritten to `"allow"`:
+    - exact own key (`"deny"`/`"ask"`) — from #457, unchanged here;
+    - wildcard keys resolving the tool (`"*"`, `"acp*"` — new with #465): old → ACP appended `acp_status: "allow"` which won last-match and silently erased the user's wildcard decision; new → nothing is written, so the host resolves `acp_status` from the user's wildcard (deny stays deny, ask stays ask).
+      When no rule resolves `acp_status`, the `"allow"` default is written exactly as before; explicit-`compress` defer behavior and all other keys are unchanged. A denied `acp_status` means the tool is absent from the LLM tool list (opencode semantics) — which is what denying it should do.
+- **Risk level**: Low — single assignment site; function is pure and fully unit-tested.
+
+## 2. Change Log
+
+### Commits
+
+| Commit       | Description                                                                                                |
+| ------------ | ---------------------------------------------------------------------------------------------------------- |
+| `(commit 1)` | fix: never override explicit permission.acp_status in host config write (#457)                             |
+| `(commit 2)` | fix: respect user wildcard decisions on acp_status in host config write (#465; groups with #457 per owner) |
+
+### Key Files
+
+- `lib/host-permissions.ts` — new export `anyRuleResolves(permissionConfig, tool)`: true when any rule resolves the tool — exact own key in any value form (incl. degenerate empty object via key presence) or a wildcard key (`"*"`, `"acp*"`) matched through the existing rule machinery (`getPermissionRules` + `findLastMatchingRule` + `wildcardMatch`). `applyAcpToolPermissions` now gates the `acp_status` default on `!anyRuleResolves(…, "acp_status")` instead of `hasExplicitToolPermission`. Explicit `compress` rule still returns the map unchanged (same identity); otherwise writes `compress` from ACP config.
+- `index.ts` — config hook calls `applyAcpToolPermissions(opencodeConfig.permission, config.compress.permission)` (unchanged by the #465 commit; comment updated). Bili-proxy `denyAcpTools` path untouched (intentional full deny).
+- `tests/host-permissions.test.ts` — 6 #457 tests + 8 #465 tests (wildcard `*`/`acp*` deny, wildcard ask/allow, non-matching wildcard still gets default, nested pattern form, wildcard+explicit-compress identity defer, `anyRuleResolves` unit matrix incl. degenerate empty object).
+- `devlog/2026-09-29_respect-explicit-acp-status/REQ.md` + `WORKLOG.md` — requirement/worklog docs (extended for #465 scope).
+
+## 3. Design & Implementation Notes
+
+- **Entry point / key function**: `applyAcpToolPermissions` in `lib/host-permissions.ts` (pure; extraction follows AGENTS.md §5.6 since the `index.ts` plugin factory has heavy runtime deps and is not unit-testable directly).
+- **Key logic**:
+    - `permissionConfig && hasExplicitToolPermission(permissionConfig, "compress")` → return `permissionConfig` as-is (preserves old defer-by-design and object identity; #465 deliberately does NOT extend defer to wildcards — owner scoped the fix to the `acp_status` guard; wildcard-deny on `compress` is already handled by the separate self-disable path via `compressDisabledByOpencode` at `index.ts`).
+    - Else build `{ ...permissionConfig, compress: compressPermission }`; add `acp_status: "allow"` only if `!anyRuleResolves(permissionConfig, "acp_status")`.
+    - `anyRuleResolves` = key presence (covers degenerate empty-object value form that produces no rules) OR any rule whose permission wildcard-matches the tool name (`getPermissionRules([config])` → `findLastMatchingRule(rules, r => wildcardMatch(tool, r.permission))`). Reuses the exact rule machinery the host resolution models, per the issue's suggested fix.
+- **Typing**: SDK `Config.permission` type is narrow (no arbitrary tool keys), so the assignment site keeps the existing `as typeof opencodeConfig.permission` cast pattern.
+- **Non-goals kept out**: pre-existing asymmetry where explicit `compress` defers fully and `acp_status` falls back to host default ("ask"); making `dispose` restore the original host object.
+
+## 4. Testing & Verification
+
+### Build & Test Commands
+
+```sh
+npm run typecheck                       # tsc --noEmit — PASS
+node --import tsx --test tests/host-permissions.test.ts   # 23/23 pass (15 baseline+457, 8 new for #465)
+npm test                                # full suite — see Results
+npm run build                           # PASS; applyAcpToolPermissions + anyRuleResolves present in dist/index.js
+npx prettier --check <changed files>    # clean
+```
+
+### Test Coverage
+
+- Modified file: `tests/host-permissions.test.ts`:
+    - 6 #457 tests (unchanged): undefined-map default path; unrelated host rules preserved; explicit `acp_status` deny/ask survive; explicit `compress` → identity defer / no injection.
+    - 8 new #465 tests: wildcard `"*": "deny"` not overridden; `"acp*": "deny"` not overridden; wildcard ask → no default; wildcard allow → no redundant default; non-matching wildcard (`"bash*"`) still gets default; nested form `{ acp_status: { "*": "deny" } }` counts as decision; wildcard map + explicit `compress` → identity defer; `anyRuleResolves` unit matrix (undefined / non-matching / exact / `*` / `acp*` / nested / degenerate empty object).
+- **RED→GREEN verified**:
+    - #457: temporarily reintroduced the unconditional overwrite → exactly the two #457 tests fail (13 pass / 2 fail); restored fix → green.
+    - #465: new tests written first against the unfixed guard → file fails to load (`anyRuleResolves` export missing, confirmed as the failure mode); behavioral repro pre-fix showed `{"*":"deny"}` → `{"*":"deny","compress":"allow","acp_status":"allow"}` (wildcard erased); post-fix the same input returns `{"*":"deny","compress":"allow"}`. Full file green (23/23).
+
+### Results
+
+- **PASS/FAIL**: PASS (typecheck, build, targeted suite, full suite modulo environment).
+- **Environment-only failures (pre-existing in this sandbox, NOT caused by this change)**:
+    - `tests/inactive-block-decompress.test.ts` "toFile on inactive block writes block summary" — hard-codes `/tmp/test-inactive-block-decompress.txt`; sandbox `/tmp` handling rejects it (validator requires `.tmp` or `~/.cache/opencode`).
+    - `tests/soft-block.test.ts` — `EACCES: mkdir '/tmp/opencode-dcp-dangerous-*'` at import time (read-only `/tmp`).
+    - Neither file imports any module changed here (verified by grep); both pass on CI where `/tmp` is writable.
+
+## 5. Risk Assessment & Rollback
+
+- **Risk points**: none identified beyond the intended behavior change (disclosed above). All other consumers read the resulting map only after the assignment.
+- **Rollback method**: revert this single commit.
+- **Compatibility notes**: No state format, config schema, persistence, or public API change. Internal naming untouched.
+
+## 6. Lessons Learned
+
+- Object-spread "defaults" silently clobber explicit user values when the presence check targets a different key — a guard must check every key it writes, not just one representative key.
+- Keeping the permission merge pure (in `host-permissions.ts`) makes this class of regression directly unit-testable without booting the plugin factory.

index.ts

@@ -10,8 +10,8 @@ import {
     createSearchContextTool,
 } from "./lib/compress"
 import {
+    applyAcpToolPermissions,
     compressDisabledByOpencode,
-    hasExplicitToolPermission,
     type HostPermissionSnapshot,
 } from "./lib/host-permissions"
 import { Logger } from "./lib/logger"
@@ -249,14 +249,12 @@ const server: Plugin = (async (ctx) => {
                 }
             }
 
-            if (!hasExplicitToolPermission(opencodeConfig.permission, "compress")) {
-                const permission = opencodeConfig.permission ?? {}
-                opencodeConfig.permission = {
-                    ...permission,
-                    compress: config.compress.permission,
-                    acp_status: "allow",
-                } as typeof permission
-            }
+            // [#457] Explicit user decisions in the host permission map are
+            // never overridden — ACP's defaults apply only to absent keys.
+            opencodeConfig.permission = applyAcpToolPermissions(
+                opencodeConfig.permission,
+                config.compress.permission,
+            ) as typeof opencodeConfig.permission
 
             hostPermissions.global = opencodeConfig.permission
             hostPermissions.agents = Object.fromEntries(

lib/host-permissions.ts

@@ -99,3 +99,53 @@ export const hasExplicitToolPermission = (
 ): boolean => {
     return permissionConfig ? Object.prototype.hasOwnProperty.call(permissionConfig, tool) : false
 }
+
+/**
+ * True when the user has made any decision that resolves `tool` in this map:
+ * an exact key in any value form (scalar or nested pattern object — including
+ * a degenerate empty object, which produces no rules but still counts as key
+ * presence), or a wildcard key (`"*"`, `"acp*"`) matching via the same rule
+ * machinery the host resolution models ([#465]).
+ */
+export const anyRuleResolves = (permissionConfig: PermissionConfig, tool: string): boolean => {
+    if (hasExplicitToolPermission(permissionConfig, tool)) {
+        return true
+    }
+
+    const match = findLastMatchingRule(getPermissionRules([permissionConfig]), (rule) =>
+        wildcardMatch(tool, rule.permission),
+    )
+
+    return match !== undefined
+}
+
+/**
+ * Merge ACP's tool permissions into the host map. An explicit user `compress`
+ * rule means the user owns the map (returned unchanged). Otherwise write
+ * `compress` + default `acp_status: "allow"` — but never override a user
+ * decision on `acp_status`, whether an exact key ([#457]: it used to be
+ * silently rewritten to `"allow"`, inverting the decision; the host reads its
+ * own permissions from this same map) or a wildcard like `"*"` / `"acp*"`
+ * ([#465]: appended after the user's rules it would win last-match and erase
+ * their decision). The default write is strictly additive over [#457]: it only
+ * stops where a user rule already resolves the tool.
+ */
+export const applyAcpToolPermissions = (
+    permissionConfig: PermissionConfig,
+    compressPermission: PermissionAction,
+): Record<string, PermissionValue> => {
+    if (permissionConfig && hasExplicitToolPermission(permissionConfig, "compress")) {
+        return permissionConfig
+    }
+
+    const next: Record<string, PermissionValue> = {
+        ...(permissionConfig ?? {}),
+        compress: compressPermission,
+    }
+
+    if (!anyRuleResolves(permissionConfig, "acp_status")) {
+        next.acp_status = "allow"
+    }
+
+    return next
+}