Skip to content

feat(provider): add onPermissionRequest callback option#17

Draft
DaniAkash wants to merge 1 commit into
mainfrom
feat/on-permission-request-callback
Draft

feat(provider): add onPermissionRequest callback option#17
DaniAkash wants to merge 1 commit into
mainfrom
feat/on-permission-request-callback

Conversation

@DaniAkash
Copy link
Copy Markdown
Owner

@DaniAkash DaniAkash commented May 6, 2026

⚠️ Draft — waiting on upstream

This PR is pre-implementation against the documented upstream shape. It will not pass typecheck until openclaw/acpx#299 merges and a corresponding `acpx` release ships (expected as `0.8.0` based on the documented type signatures).

Do not mark ready / merge until:

  1. feat: add onPermissionRequest callback for host-driven per-call gating openclaw/acpx#299 is merged.
  2. `acpx@0.8.0` (or whatever version lands the callback) is published to npm.
  3. Locally: `bun install` resolves the new acpx, then `bun run typecheck` passes — confirming upstream's published types match the documented shape.
  4. Verified the published `runtime.d.ts` exposes `onPermissionRequest` on `AcpRuntimeOptions` plus `AcpPermissionRequest` + `AcpPermissionDecision` exports. If upstream renamed during review, update `src/types.ts`, `src/index.ts`, and the README's `outcome` vocabulary table to match.

Summary

Hosts can now intercept the agent's per-call permission requests with their own UI by passing `onPermissionRequest` to `createAcpxProvider`. Returning `undefined` falls through to the existing mode-based resolver, so existing consumers see no behavior change.

Driven by downstream BrowserOS chat work that wants inline approve/deny CTA cards instead of an up-front mode gate. The mode gate is too coarse for that flow — `approve-reads` silently denies writes (codex's `apply_patch` aborts with no signal), `approve-all` skips approval entirely. A per-call callback is the right primitive.

const provider = createAcpxProvider({
  agent: 'codex',
  cwd: '/path/to/repo',
  permissionMode: 'approve-reads', // fallback for unhandled cases
  onPermissionRequest: async (req, { signal }) => {
    const decision = await myUi.prompt({
      title: req.raw.toolCall.title,
      kind: req.inferredKind,
    })
    return decision // undefined falls through to the mode resolver
  },
})

Files changed

File Change
`src/types.ts` Re-export `AcpPermissionRequest` + `AcpPermissionDecision` from `acpx/runtime`; add `onPermissionRequest` to `AcpxProviderSettings` with the documented signature and doc-comment
`src/provider.ts` Thread the option through `buildRuntimeOptions()` (one line)
`src/index.ts` Re-export the two new types
`README.md` Replace the "Permissions are mode-based" limitation bullet with a fall-through explanation; add new `## Per-call permissions` section with the full callback contract
`test/unit/provider.test.ts` NEW — three tests: callback forwards through, omitted when not configured, pre-built runtime path skips construction
`package.json` Version 0.0.1 → 0.1.0; `acpx` peer-dep + dev-dep `>=0.6.1` → `>=0.8.0`

Test plan

Check Status Why
`bun run typecheck` 7 errors All from missing upstream types — expected, will go green when `acpx@0.8.0` publishes
`bun run lint` ✅ clean
`bun run fallow` ✅ exit 0
`bun test` ✅ 183 pass / 15 skip / 0 fail / 280 expect() TS types are erased at runtime; the 3 new tests are part of the 183
`bun run build` ✅ ESM + .d.ts (32 KB) bunup's esbuild path strips types without re-checking

Known issue: dev-dep won't resolve from a clean install

The dev-dep range `acpx >=0.8.0` doesn't have a matching version published yet. `bun install` from a fresh checkout of this branch will fail. This is expected for the draft phase. Branch exists for review, not for fresh installs.

When upstream lands, the resolver will pick up `acpx@0.8.0` automatically and everything goes green.

Hosts can now intercept the agent's per-call permission requests
with their own UI by passing onPermissionRequest to
createAcpxProvider. Returning undefined falls through to the existing
mode-based resolver, so existing consumers see no behavior change.

Driven by downstream BrowserOS chat work that wants inline
approve/deny CTA cards instead of an up-front mode gate. The mode
gate is too coarse for that flow — approve-reads silently denies
writes (codex's apply_patch aborts with no signal), approve-all
skips approval entirely.

The provider's job is plumbing only: pass the option through to
createAcpRuntime, the runtime owns the wire-level resolution and
decision-to-optionId mapping.

- src/types.ts: re-export AcpPermissionRequest +
  AcpPermissionDecision from acpx/runtime; add onPermissionRequest
  to AcpxProviderSettings with the documented signature
- src/provider.ts: thread the option through buildRuntimeOptions()
- src/index.ts: re-export the two new types
- README.md: replace the mode-only limitation bullet with a
  fall-through explanation; add a new Per-call permissions section
  with the full callback contract
- test/unit/provider.test.ts: three tests covering callback
  forwarding, the omitted case, and the documented pre-built-runtime
  no-op
- package.json: bump to 0.1.0; bump acpx peer-dep + dev-dep to
  >=0.8.0 to require the runtime release that exposes the callback

Blocked on upstream openclaw/acpx#299 + a corresponding acpx release.
Typecheck fails until then because the new types don't exist in
acpx@0.7.0; runtime / lint / fallow / build / tests all green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant