Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions packages/acp-bridge/src/permissionMediator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ describe('MultiClientPermissionMediator — first-responder', () => {
data: {
requestId: 'req-1',
outcome: { outcome: 'selected', optionId: 'proceed_once' },
// A4: canonical voterClientId in data, same value as the
// (deprecated) envelope originatorClientId below.
voterClientId: 'client_B',
},
originatorClientId: 'client_B',
},
Expand All @@ -189,18 +192,19 @@ describe('MultiClientPermissionMediator — first-responder', () => {
});
});

it('omits originatorClientId on permission_resolved when voter has no clientId', async () => {
it('omits both voterClientId and originatorClientId on permission_resolved when voter has no clientId', async () => {
const { mediator, events } = makeMediator();
const record = makeRecord();
const promise = mediator.request(record, 5_000);

mediator.vote(makeVote({ clientId: undefined }));
await promise;

// Loopback voter without X-Qwen-Client-Id — pre-F3 spread guard
// omits the field entirely (not `originatorClientId: undefined`).
// Loopback voter without X-Qwen-Client-Id — the spread guard omits
// both fields entirely (A4: no-voter resolutions carry neither).
expect(events).toHaveLength(1);
expect(events[0]!.event).not.toHaveProperty('originatorClientId');
expect(events[0]!.event.data).not.toHaveProperty('voterClientId');
});

it('returns already_resolved on a duplicate vote and re-emits the SSE notification', async () => {
Expand Down
13 changes: 12 additions & 1 deletion packages/acp-bridge/src/permissionMediator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1126,12 +1126,23 @@ export class MultiClientPermissionMediator implements PermissionMediator {
data: {
requestId: pending.requestId,
outcome: this.toAcpOutcome(resolution),
// A4 (doudouOUC #4484 follow-up): `voterClientId` is the canonical,
// unambiguous name for "who cast the resolving vote". The envelope
// `originatorClientId` below carries the SAME value for pre-F3 wire
// compat (it is semantically the voter on `permission_resolved`,
// unlike on `permission_request` where it is the prompt originator).
// Both are optional and omitted together for no-voter resolutions
// (timer expiry / session-closed / loopback voter with no clientId).
...(resolverClientId !== undefined
? { voterClientId: resolverClientId }
: {}),
},
// O8 — preserve pre-F3 behavior: voter's clientId is stamped
// here (not the prompt originator's). Documented inconsistency
// with `permission_request.originatorClientId` (which IS the
// prompt originator); F3 does not fix the inconsistency to
// avoid breaking the wire shape.
// avoid breaking the wire shape. A4 keeps it as a deprecated
// alias of `data.voterClientId`.
...(resolverClientId !== undefined
? { originatorClientId: resolverClientId }
: {}),
Expand Down
7 changes: 7 additions & 0 deletions packages/sdk-typescript/src/daemon/ui/normalizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,12 +639,19 @@ function normalizePermissionResolved(
},
];
}
// A4: the canonical voter is `data.voterClientId`; fall back to the
// envelope `originatorClientId` (deprecated alias) for daemons predating
// the rename. Both may be absent for no-voter resolutions (timer /
// session-closed). `originatorClientId` stays on the base unchanged.
const voterClientId =
getString(event.data, 'voterClientId') ?? base.originatorClientId;
return [
{
...base,
type: 'permission.resolved',
requestId,
outcome: describePermissionOutcome(event.data),
...(voterClientId ? { voterClientId } : {}),
},
];
}
Expand Down
10 changes: 10 additions & 0 deletions packages/sdk-typescript/src/daemon/ui/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,16 @@ export interface DaemonUiPermissionResolvedEvent extends DaemonUiEventBase {
type: 'permission.resolved';
requestId: string;
outcome: string;
/**
* A4: the client that cast the resolving vote (canonical name). On
* `permission_resolved` the base `originatorClientId` carries the same
* value for back-compat — but it means the *voter* here, vs the *prompt
* originator* on `permission_request`; prefer `voterClientId` for clarity.
* Absent for system-initiated resolutions (timer expiry / session-closed /
* loopback voter with no clientId). The prompt originator remains available
* by correlating with the matching `permission.request`.
*/
voterClientId?: string;
}

export interface DaemonUiModelChangedEvent extends DaemonUiEventBase {
Expand Down
92 changes: 92 additions & 0 deletions packages/sdk-typescript/test/unit/daemonUi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5081,3 +5081,95 @@ describe('cross-client event recognition (prompt_cancelled / replay_complete)',
expect(state.blocks.length).toBe(before);
});
});

describe('permission_resolved voterClientId (A4)', () => {
it('exposes voterClientId from data', () => {
const [evt] = normalizeDaemonEvent({
id: 1,
v: 1,
type: 'permission_resolved',
originatorClientId: 'client_B',
data: {
requestId: 'perm-1',
outcome: { outcome: 'selected', optionId: 'allow' },
voterClientId: 'client_B',
},
} as never);
expect(evt).toMatchObject({
type: 'permission.resolved',
requestId: 'perm-1',
voterClientId: 'client_B',
});
});

it('falls back to the envelope originatorClientId when data.voterClientId is absent (old daemon)', () => {
const [evt] = normalizeDaemonEvent({
id: 1,
v: 1,
type: 'permission_resolved',
originatorClientId: 'client_B',
data: {
requestId: 'perm-1',
outcome: { outcome: 'selected', optionId: 'allow' },
},
} as never);
expect(evt).toMatchObject({
type: 'permission.resolved',
voterClientId: 'client_B',
});
});

it('omits voterClientId for a no-voter resolution (neither field present)', () => {
const [evt] = normalizeDaemonEvent({
id: 1,
v: 1,
type: 'permission_resolved',
data: {
requestId: 'perm-1',
outcome: { outcome: 'cancelled' },
},
} as never);
expect(evt).toMatchObject({ type: 'permission.resolved' });
expect(evt).not.toHaveProperty('voterClientId');
});

it('distinguishes the prompt originator (request) from the voter (resolved) when they differ', () => {
// The whole point of A4: client A submits the prompt that triggers the
// permission request; a DIFFERENT client B casts the resolving vote.
// The request carries A as originator; the resolution carries B as voter.
const [request] = normalizeDaemonEvent({
id: 1,
v: 1,
type: 'permission_request',
originatorClientId: 'client_A',
data: {
requestId: 'perm-1',
toolCall: { name: 'Bash', command: 'rm -rf build' },
options: [{ optionId: 'allow', label: 'Allow', raw: null }],
},
} as never);
const [resolved] = normalizeDaemonEvent({
id: 2,
v: 1,
type: 'permission_resolved',
originatorClientId: 'client_B',
data: {
requestId: 'perm-1',
outcome: { outcome: 'selected', optionId: 'allow' },
voterClientId: 'client_B',
},
} as never);
expect(request).toMatchObject({
type: 'permission.request',
originatorClientId: 'client_A',
});
expect(resolved).toMatchObject({
type: 'permission.resolved',
voterClientId: 'client_B',
});
// The voter is NOT the prompt originator — the disambiguation A4 enables.
expect((resolved as { voterClientId?: string }).voterClientId).not.toBe(
(request as { originatorClientId?: string }).originatorClientId,
);
});
});