Skip to content

docs(acp): update permission mediator status #9866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AaronZ345 wants to merge 8 commits into
base: main
Choose a base branch
from
+41 −51
Open

docs(acp): update permission mediator status #9866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9fa40d6
docs(acp): update permission mediator status
Aug 24, 2026
ee77198
docs(acp): refresh permission mediator comments
Aug 24, 2026
04d0284
docs(acp): narrow permission mediator status claims
Aug 24, 2026
2d83335
docs(acp): format permission policy union
Aug 24, 2026
cb93f83
docs(acp): clarify permission mediator status
Aug 24, 2026
6b14972
docs(acp): align permission mediator docs
Aug 25, 2026
04d0da5
docs(acp): align bridge permission docs
Aug 25, 2026
05146bf
docs(acp): clarify mediator identity semantics
Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 25 additions & 26 deletions packages/acp-bridge/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ and remote-control adapters. Lives in the monorepo, not published to npm.

Lift history (#4175 Mode B daemon roadmap):

| Slice | Scope | Status |
| -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- |
| **PR 22a** (#4295) | Skeleton + `EventBus` + `inMemoryChannel` + `AcpChannel` types + `PermissionMediator` type-only stub | ✅ merged |
| **PR 22b/1** (#4298) | Lift `status` + `workspacePaths` + `bridgeErrors` + `bridgeTypes` | ✅ merged |
| **PR 22b/2** (#4304) | Lift `BridgeOptions` + new `DaemonStatusProvider` injection seam | ✅ merged |
| **F1** (this PR) | Lift `defaultSpawnChannelFactory` + `BridgeClient` + `createHttpAcpBridge` factory closure + new `BridgeFileSystem` injection seam (22b' scope) | ✅ in this PR |
| **F3 PR 24** | Implement the four `PermissionMediator` strategies (`first-responder`, `designated`, `consensus`, `local-only`) + pair-token revocation + audit log | F3 in the feature-cohesive plan |
| Slice | Scope | Status |
| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------- |
| **PR 22a** (#4295) | Skeleton + `EventBus` + `inMemoryChannel` + `AcpChannel` types + `PermissionMediator` type-only stub | ✅ merged |
| **PR 22b/1** (#4298) | Lift `status` + `workspacePaths` + `bridgeErrors` + `bridgeTypes` | ✅ merged |
| **PR 22b/2** (#4304) | Lift `BridgeOptions` + new `DaemonStatusProvider` injection seam | ✅ merged |
| **F1** (#4490) | Lift `defaultSpawnChannelFactory` + `BridgeClient` + `createHttpAcpBridge` factory closure + new `BridgeFileSystem` injection seam (22b' scope) | ✅ merged |
| **F3 PR 24** | Implement the four `PermissionMediator` policies (`first-responder`, `designated`, `consensus`, `local-only`) plus audit/emit fan-out; pair-token binding and revocation stay future scope | ✅ implemented |

## What's here today

Expand All @@ -25,13 +25,12 @@ Lift history (#4175 Mode B daemon roadmap):
type contract that `createHttpAcpBridge` (now in this package) plus
the channels / VSCode IDE companion's own-spawn paths consume via
`BridgeOptions.channelFactory`.
- `permission` — type-only `PermissionMediator` interface,
- `permission` — `PermissionMediator` interface,
`PermissionPolicy` literal union (4 strategies), and
`PermissionResolution` discriminated union. **No implementation
yet** — first-responder voting still lives in
`BridgeClient.requestPermission` (in `bridgeClient.ts` after F1).
F3 PR 24 will move that and add the other three policies behind
this interface.
`PermissionResolution` discriminated union. `MultiClientPermissionMediator`
implements the four policies, owns pending/resolved permission state, and
handles strategy dispatch plus audit/emit fan-out; `BridgeClient` only
plumbs `requestPermission` into `mediator.request`.
Comment on lines +30 to +33

@qwen-code-ci-bot qwen-code-ci-bot Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This rewritten bullet contradicts a sibling bullet the diff left stale: the unchanged bridgeClient bullet in the same section (lines 76-84) still says BridgeClient implements the "first-responder permission flow" and "Exports the supporting PendingPermission / PermissionResolutionRecord / BridgeClientSessionEntry types + MAX_RESOLVED_PERMISSION_RECORDS cap". At this commit bridgeClient.ts exports only KNOWN_APPROVAL_MODES, BridgeClientSessionEntry, BridgeClient, and the a2ui helpers — PendingPermission exists nowhere in the package, and PermissionResolutionRecord / MAX_RESOLVED_PERMISSION_RECORDS are module-private in permissionMediator.ts. A reader following that bullet imports the three symbols and gets compile errors, and the same section now tells two opposite stories about where permission state lives — the exact staleness this PR exists to remove survives in its own file. Updating the sibling bullet in the same pass keeps the section self-consistent, e.g. drop the three vanished exports (keep BridgeClientSessionEntry) and replace "first-responder permission flow" with "permission requests plumbed into the injected PermissionMediator (mediator.request)"; the pending/resolved state and cap live in permissionMediator.ts.

中文说明

这条重写后的条目与 diff 未更新的一条相邻条目矛盾：同一节中未改动的 bridgeClient 条目（第 76-84 行）仍声称 BridgeClient 实现 "first-responder permission flow" 并 "Exports the supporting PendingPermission / PermissionResolutionRecord / BridgeClientSessionEntry types + MAX_RESOLVED_PERMISSION_RECORDS cap"。在当前提交，bridgeClient.ts 只导出 KNOWN_APPROVAL_MODES、BridgeClientSessionEntry、BridgeClient 和 a2ui 辅助函数——PendingPermission 在整个包中不存在，PermissionResolutionRecord / MAX_RESOLVED_PERMISSION_RECORDS 是 permissionMediator.ts 的模块私有符号。读者按该条目导入这三个符号会得到编译错误，同一节因此对 permission 状态归属给出两种相反说法——本 PR 要消除的过期内容恰好残留在它自己改动的文件里。建议顺手更新该条目保持一致：删掉三个已不存在的导出（保留 BridgeClientSessionEntry），把 "first-responder permission flow" 改为 "permission requests plumbed into the injected PermissionMediator (mediator.request)"；pending/resolved 状态与上限在 permissionMediator.ts。

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment on lines +30 to +33

@qwen-code-ci-bot qwen-code-ci-bot Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The unchanged bridge bullet (lines 84-90) still says the createHttpAcpBridge factory "Builds the bookkeeping closures (resolveEntry, registerPending, etc.) and wires them into BridgeClient" — but registerPending exists nowhere in production code at this commit: a repo-wide sweep finds exactly 2 matches, this README line itself and a test comment documenting its removal ("F3 Commit 3 replaced the pre-F3 registerPending + rollbackPending callbacks with a single MultiClientPermissionMediator reference"), zero production hits. A maintainer grepping for registerPending to understand permission bookkeeping finds nothing while this bullet says the mediator owns that state — two opposite answers in one section, and the update silently gets skipped because the bullet looks untouched. resolveEntry genuinely still exists, so this is a reword, not a deletion — e.g. "Builds the session-entry lookup (resolveEntry) wired into BridgeClient and constructs the MultiClientPermissionMediator that owns permission state".

中文说明

未改动的 bridge 条目（第 84-90 行）仍声称 createHttpAcpBridge 工厂 "Builds the bookkeeping closures (resolveEntry, registerPending, etc.) and wires them into BridgeClient"——但在当前提交，registerPending 在生产代码中已不存在：全仓库扫描只有 2 处匹配，即本 README 行本身和一条记录其移除的测试注释（"F3 Commit 3 replaced the pre-F3 registerPending + rollbackPending callbacks with a single MultiClientPermissionMediator reference"），生产代码零匹配。维护者想理解 permission 记账逻辑而 grep registerPending 会一无所获，而相邻条目却说 mediator 持有该状态——同一节给出两种相反答案，且该条目看起来未被触碰，更新很容易被跳过。resolveEntry 确实仍存在，所以这是改写而非删除——例如 "Builds the session-entry lookup (resolveEntry) wired into BridgeClient and constructs the MultiClientPermissionMediator that owns permission state"。

— qwen3.8-max via Qwen Code /review (v0.22.0)

- `status` (PR 22b/1) — wire-contract status types for
`/workspace/{mcp,skills,providers,env,preflight}` and
`/session/:id/{context,supported-commands,tasks}` routes, the
Expand Down Expand Up @@ -75,20 +74,20 @@ Lift history (#4175 Mode B daemon roadmap):
companion consume this directly instead of each reimplementing the
child lifecycle.
- `bridgeClient` (F1) — `BridgeClient` class implementing the ACP
`Client` surface: first-responder permission flow, session-update
fan-out into `EventBus`, child-side `extNotification` routing,
early-event buffer + tombstone bookkeeping, inline fs proxy for
`writeTextFile` / `readTextFile`. Exports the supporting
`PendingPermission` / `PermissionResolutionRecord` /
`BridgeClientSessionEntry` types + `MAX_RESOLVED_PERMISSION_RECORDS`
cap that the factory's bookkeeping maps consume.
`Client` surface: permission requests delegated to
`PermissionMediator`, session-update fan-out into `EventBus`,
child-side `extNotification` routing, early-event buffer + tombstone
bookkeeping, inline fs proxy for `writeTextFile` / `readTextFile`.
Exports the supporting `BridgeClientSessionEntry` type consumed by
the session-entry lookup passed in by the factory.
- `bridge` (F1) — `createHttpAcpBridge` factory closure (~3000 LOC)
- `ChannelInfo` / `SessionEntry` interfaces + factory-only
helpers (`withTimeout`, `canonicalizeExistingAncestor`,
`verifyParentWithinWorkspace`, debug log helpers,
`hasControlCharacter`) + factory constants. Builds the
bookkeeping closures (`resolveEntry`, `registerPending`, etc.)
and wires them into `BridgeClient`.
`hasControlCharacter`) + factory constants. Owns session
bookkeeping, constructs the `MultiClientPermissionMediator` that
owns permission state, and passes the session-entry lookup into
`BridgeClient`.
- `bridgeFileSystem` (F1) — `BridgeFileSystem` interface for the
ACP fs proxy. When wired through `BridgeOptions.fileSystem`,
`BridgeClient.readTextFile` / `BridgeClient.writeTextFile`
Expand Down Expand Up @@ -135,6 +134,6 @@ exported symbols (`createHttpAcpBridge`, `defaultSpawnChannelFactory`,
- #4175 Mode B daemon roadmap (feature-cohesive F1-F5 plan targeting
`daemon_mode_b_main`)
- #3803 `Stage 1.5-prereq AcpChannel lift` (chiga0's original framing)
- F3 PR 24 will replace the inline first-responder logic in
`BridgeClient.requestPermission` with the four `PermissionMediator`
strategies declared in `permission.ts`.
- `permissionMediator.ts` implements the four `PermissionMediator`
strategies declared in `permission.ts`; `BridgeClient.requestPermission`
delegates to that mediator.
41 changes: 16 additions & 25 deletions packages/acp-bridge/src/permission.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,32 @@
*/

/**
* `PermissionMediator` — type-only interface contract for daemon
* permission flow. **No implementation lives here.** Permission voting
* still runs inside `BridgeClient.requestPermission`
* (`@qwen-code/acp-bridge/bridgeClient`) and
* `respondToPermission` (inside `createHttpAcpBridge` factory closure
* at `@qwen-code/acp-bridge/bridge` after F1 step 3), hard-coded to
* `first-responder`. A future change will move that code behind this
* interface and add the other three policies.
* `PermissionMediator` — interface contract for daemon permission flow.
* `MultiClientPermissionMediator` in `permissionMediator.ts` owns the
* policy dispatch and pending/resolved permission state used by
Comment on lines +9 to +10

@qwen-code-ci-bot qwen-code-ci-bot Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This header rewrite presents the four policy bullets below it as the implemented semantics of MultiClientPermissionMediator, but the unchanged designated bullet omits the implemented anonymous-originator case: voteDesignated falls back to first-responder when originatorClientId is undefined (permissionMediator.ts:707-713). The fallback is deliberate and tested (permissionMediator.test.ts:651) and documented in the operator-facing settings schema, but an operator selecting designated for the bullet's stated use case (per-tenant approval ownership) may believe non-originator votes are always rejected — on a prompt issued with no registered client id, any attached client can resolve it. Conversely, a maintainer treating this JSDoc as the contract might "fix" voteDesignated to reject non-originators on anonymous prompts and break the deliberate fallback; the implementation file already carries "Documented here so a future maintainer doesn't 'fix' the bypass", but this bullet has no equivalent caveat. Consider extending it, e.g. "designated — only the originatorClientId that started the prompt may answer; other clients see permission_forbidden. Prompts with no originator fall back to first-responder."

中文说明

这段头部重写使其下方的四条策略条目成为 MultiClientPermissionMediator 已交付语义的描述，但未改动的 designated 条目漏掉了已实现的匿名发起方场景：当 originatorClientId 为 undefined 时，voteDesignated 会回退到 first-responder（permissionMediator.ts:707-713）。该回退是有意设计且有测试（permissionMediator.test.ts:651）、并在面向运维的 settings schema 中有说明，但一位为该条目所述场景（按租户隔离审批权）选择 designated 的运维者可能以为非发起方投票总会被拒绝——而对没有注册客户端 id 的提示，任何接入的客户端都能解决它。反向风险同样存在：把这段 JSDoc 当作契约的维护者可能去 "修复" voteDesignated，让它在匿名提示时拒绝非发起方，从而破坏这个有意的回退；实现文件里已写了 "Documented here so a future maintainer doesn't 'fix' the bypass"，但这条条目没有等价警示。建议补充，例如 "designated — only the originatorClientId that started the prompt may answer; other clients see permission_forbidden. Prompts with no originator fall back to first-responder."

— qwen3.8-max via Qwen Code /review (v0.22.0)

* `BridgeClient.requestPermission` plus the `respondToPermission` route
* in `createHttpAcpBridge`.
*
* The four policies are ordered from cheapest to strongest:
* The four policy contracts are ordered from cheapest to strongest:
*
* - `first-responder` — first valid `POST /permission/:requestId`
* wins; later voters get `permission_already_resolved`. Today's
* default; preserves the live-collaboration UX.
* - `designated` — only the `originatorClientId` that started the
* prompt may answer; other clients see `permission_forbidden`.

@qwen-code-ci-bot qwen-code-ci-bot Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R3-3: The header rewrite presents the four policy bullets below it as the implemented semantics of MultiClientPermissionMediator, but this designated bullet still omits the implemented anonymous-originator case: voteDesignated falls back to first-responder when originatorClientId is undefined (permissionMediator.ts). The fallback is deliberate and tested, and the operator-facing settings schema documents it, but this bullet carries no equivalent caveat — an operator selecting designated for the bullet's stated use case (per-tenant approval ownership) may believe non-originator votes are always rejected, while on a prompt issued with no registered client id any attached client can resolve it; conversely a maintainer treating this JSDoc as the contract might "fix" voteDesignated to reject non-originators on anonymous prompts and break the deliberate fallback.

Suggested change
* prompt may answer; other clients see `permission_forbidden`.
* prompt may answer; other clients see `permission_forbidden`.
* Prompts with no originator fall back to first-responder.
中文说明

头部重写使其下方的四条策略条目成为 MultiClientPermissionMediator 已交付语义的描述，但这条 designated 条目仍漏掉了已实现的匿名发起方场景：当 originatorClientId 为 undefined 时，voteDesignated 会回退到 first-responder（permissionMediator.ts）。该回退是有意设计且有测试，面向运维的 settings schema 也有说明，但这条条目没有等价警示——为条目所述场景（按租户隔离审批权）选择 designated 的运维者可能以为非发起方投票总会被拒绝，而对没有注册客户端 id 的提示，任何接入的客户端都能解决它；反向风险是把这段 JSDoc 当契约的维护者可能去 "修复" voteDesignated，让它在匿名提示时拒绝非发起方，从而破坏这个有意的回退。

— qwen3.8-max via Qwen Code /review (v0.22.0)

* Use case: per-tenant SaaS where a UI surface must own its own
* approvals.
* - `consensus` — N-of-M quorum across pair-token-authenticated
* clients before resolving; intermediate `permission_partial_vote`
* events let UIs render progress. Use case: enterprise change
* review where two operators must agree.
* Prompts with no originator fall back to first-responder. Use case:
* per-tenant SaaS where a UI surface must own its own approvals.
* - `consensus` — N-of-M quorum across the session client IDs
* captured when the permission request is issued. Client identity is
* self-declared until pair-token authentication lands; intermediate
* `permission_partial_vote` events let UIs render progress. Use case:
* enterprise change review where two operators must agree.
* - `local-only` — refuses any HTTP voter; the prompt blocks until
* a loopback client (the local TUI super-client) resolves it.
* Use case: workstations where remote control should never grant
* privilege escalation.
*
* See `bridgeClient.ts BridgeClient.requestPermission` for the
* current first-responder implementation; the `FIXME(stage-1.5)`
* block above that method scoped this contract.
* See `permissionMediator.ts` for the implementation details.
*/
export type PermissionPolicy =
| 'first-responder'
Expand All @@ -43,10 +39,7 @@ export type PermissionPolicy =
| 'local-only';

/**
* One pending permission tracked by a `PermissionMediator`. The
* shape mirrors the current `PendingPermission` record in
* `@qwen-code/acp-bridge/bridgeClient`
* so the mediation implementation's lift is a structural rename rather than a redesign.
* One pending permission tracked by a `PermissionMediator`.
*/
export interface PermissionRequestRecord {
/** ACP `RequestPermission` request id, unique per session. */
Expand Down Expand Up @@ -148,9 +141,7 @@ export type PermissionResolution =

/**
* The contract `qwen serve`'s permission route layer talks to.
* Today there is one implementation (first-responder) wired
* inline in `BridgeClient`; The implementation will provide all four behind
* this surface plus pair-token authentication and an audit log.
* `MultiClientPermissionMediator` provides the implementation.
*/
export interface PermissionMediator {
/** Active policy. May be reconfigured per session in future
Expand Down
Loading