fix(permission): address human review feedback
This commit is contained in:
Tianyi Cui
@@ -1,7 +1,7 @@
|
||||
# @deepseek-ai/dsh-permission
|
||||
|
||||
User-facing permission presets. Owns the `ctx.permission` service ([`PermissionService`](src/index.ts)): a config-defined preset table — by default `request` (`workspace-write` + `ask`) and `yolo` (`danger-full-access` + `never`) — where each name bundles the two mechanism knobs, `bash/sandbox-mode` and `approval/policy`. The product surface (the ACP bridge's single `Permissions` select) advertises `names` and calls `set()`; the mechanism tiers stay orthogonal capabilities that never learn the product vocabulary.
|
||||
User-facing permission presets. Owns the `ctx.permission` service ([`PermissionService`](src/index.ts)): a config-defined preset table — by default `workspace-write` (`workspace-write` + `ask`) and `danger-full-access` (`danger-full-access` + `never`) — where each name bundles the two mechanism knobs, `bash/sandbox-mode` and `approval/policy`. The product surface (the ACP bridge's single `Permissions` select) advertises `names` and calls `set()`; the mechanism tiers stay orthogonal capabilities that never learn the product vocabulary.
|
||||
|
||||
A switch WRITES THROUGH: `set(session, name)` appends one log-only `permission/preset` event when the name differs from the session's current preset (the audit fact reverse-mapping cannot recover — two presets may share knob values and differ only in composed policy, the planned `agent` preset being the standing example), then each knob event through its own THE-write-path setter, skipping values the session already effectively has — a net-zero switch appends nothing. The current preset DERIVES from the effective knob values (fold ?? composition default per knob): the last-chosen preset when its bundle still matches (presets may share bundles — the fold breaks the tie), else the first matching table entry, else the reserved `custom` — the honest not-a-preset state, shown as the current value only while it holds, switchable FROM and never a target. Every existing knob consumer (executor stamping, the approval gate, narrators, resume) keeps reading its own fold, untouched.
|
||||
|
||||
Composing it requires a confining `ctx.bash` executor and the `ctx.approval` seam; a table entry named `custom` throws at load (the name is reserved), while composition defaults outside the table are not an error — a zero-event session simply derives `custom`. See [the acp-agent example's sandbox variant](../../../examples/acp-agent/) for the composed leaf and [the sandbox RFC § Per-session modes](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md) for the switching design this layers over.
|
||||
Composing it requires a confining `ctx.bash` executor and the `ctx.approval` seam; a table entry named `custom` throws at load (the name is reserved), while composition defaults outside the table are not an error — a zero-event session simply derives `custom`. See [the acp-agent example's default tree](../../../examples/acp-agent/) for the composed leaf and [the sandbox RFC § Per-session modes](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md) for the switching design this layers over.
|
||||
|
||||
@@ -2,8 +2,8 @@
|
||||
* User-facing PERMISSION PRESETS: one product-level knob over the two
|
||||
* mechanism knobs. A preset names a bundle — its sandbox mode
|
||||
* (`bash/sandbox-mode`) and its approval policy (`approval/policy`) — so a
|
||||
* user picks `request` or `yolo` where the mechanism tiers stay orthogonal
|
||||
* capabilities. Switching a preset WRITES THROUGH: one `permission/preset` event
|
||||
* user picks `workspace-write` or `danger-full-access` while the mechanism
|
||||
* tiers stay orthogonal capabilities. Switching a preset WRITES THROUGH: one `permission/preset` event
|
||||
* records the chosen bundle (the audit fact reverse-mapping cannot recover —
|
||||
* two presets may share knob values and differ only in composed policy, the
|
||||
* planned `agent` preset being the standing example), then each knob event
|
||||
@@ -96,9 +96,9 @@ export function effectivePermissionPreset(events: readonly SessionEvent[]): stri
|
||||
/** The {@link PermissionService} config: the deployment's preset table. */
|
||||
export interface Config {
|
||||
/**
|
||||
* The preset table: name → knob bundle. Defaults to `request`
|
||||
* (workspace-write + ask) and `yolo` (danger-full-access + never). The
|
||||
* name `custom` is reserved for the derived not-a-preset state.
|
||||
* The preset table: name → knob bundle. Defaults to `workspace-write`
|
||||
* (workspace-write + ask) and `danger-full-access` (danger-full-access +
|
||||
* never). The name `custom` is reserved for the derived not-a-preset state.
|
||||
*/
|
||||
presets?: Record<string, PresetSpec>
|
||||
}
|
||||
@@ -121,13 +121,14 @@ export class PermissionService extends Service {
|
||||
name: z.string(),
|
||||
description: z.string(),
|
||||
})).default({
|
||||
request: {
|
||||
// Keep the user-facing preset names explicit about filesystem reach.
|
||||
'workspace-write': {
|
||||
sandbox: 'workspace-write', approval: 'ask',
|
||||
name: 'Request', description: 'Write inside the workspace; anything wider asks for your approval.',
|
||||
name: 'workspace-write', description: 'Write inside the workspace; anything wider asks for your approval.',
|
||||
},
|
||||
yolo: {
|
||||
'danger-full-access': {
|
||||
sandbox: 'danger-full-access', approval: 'never',
|
||||
name: 'YOLO', description: 'Full file access, no approval prompts.',
|
||||
name: 'danger-full-access', description: 'Full file access, no approval prompts.',
|
||||
},
|
||||
}),
|
||||
})
|
||||
|
||||
@@ -28,26 +28,26 @@ describe('effectivePermissionPreset', () => {
|
||||
it('folds to the last event, or undefined without one', () => {
|
||||
const session = freshSession('sess-fold')
|
||||
expect(effectivePermissionPreset(session.events)).toBeUndefined()
|
||||
session.append('permission/preset', { preset: 'yolo' })
|
||||
session.append('permission/preset', { preset: 'request' })
|
||||
expect(effectivePermissionPreset(session.events)).toBe('request')
|
||||
session.append('permission/preset', { preset: 'danger-full-access' })
|
||||
session.append('permission/preset', { preset: 'workspace-write' })
|
||||
expect(effectivePermissionPreset(session.events)).toBe('workspace-write')
|
||||
})
|
||||
})
|
||||
|
||||
describe('PermissionService', () => {
|
||||
it('advertises the preset table in declaration order and resolves bundles', async () => {
|
||||
const ctx = await mounted()
|
||||
expect(ctx.permission.names).toEqual(['request', 'yolo'])
|
||||
expect(ctx.permission.resolve('yolo')).toMatchObject({ sandbox: 'danger-full-access', approval: 'never' })
|
||||
expect(ctx.permission.names).toEqual(['workspace-write', 'danger-full-access'])
|
||||
expect(ctx.permission.resolve('danger-full-access')).toMatchObject({ sandbox: 'danger-full-access', approval: 'never' })
|
||||
expect(() => ctx.permission.resolve('plan')).toThrow(/unknown preset "plan"/)
|
||||
})
|
||||
|
||||
it('current() derives from the effective knobs: composition defaults hit request, a switch hits its preset', async () => {
|
||||
it('current() derives from the effective knobs: composition defaults hit workspace-write, a switch hits its preset', async () => {
|
||||
const ctx = await mounted()
|
||||
const session = freshSession('sess-current')
|
||||
expect(ctx.permission.current(session.events)).toBe('request')
|
||||
ctx.permission.set(session, 'yolo')
|
||||
expect(ctx.permission.current(session.events)).toBe('yolo')
|
||||
expect(ctx.permission.current(session.events)).toBe('workspace-write')
|
||||
ctx.permission.set(session, 'danger-full-access')
|
||||
expect(ctx.permission.current(session.events)).toBe('danger-full-access')
|
||||
})
|
||||
|
||||
it('a knob state matching no table entry derives custom — a state, not an error', async () => {
|
||||
@@ -57,8 +57,8 @@ describe('PermissionService', () => {
|
||||
expect(ctx.permission.current(session.events)).toBe(CUSTOM_PRESET)
|
||||
// Switching FROM custom is an ordinary write-through; custom itself is
|
||||
// never a target.
|
||||
ctx.permission.set(session, 'yolo')
|
||||
expect(ctx.permission.current(session.events)).toBe('yolo')
|
||||
ctx.permission.set(session, 'danger-full-access')
|
||||
expect(ctx.permission.current(session.events)).toBe('danger-full-access')
|
||||
expect(() => ctx.permission.resolve(CUSTOM_PRESET)).toThrow(/unknown preset/)
|
||||
})
|
||||
|
||||
@@ -70,26 +70,26 @@ describe('PermissionService', () => {
|
||||
|
||||
it('the fold breaks bundle ties; a stale fold no longer matching falls back to table order', async () => {
|
||||
const ctx = await mounted({ config: { presets: {
|
||||
request: { sandbox: 'workspace-write', approval: 'ask' },
|
||||
'workspace-write': { sandbox: 'workspace-write', approval: 'ask' },
|
||||
agentish: { sandbox: 'workspace-write', approval: 'ask' },
|
||||
yolo: { sandbox: 'danger-full-access', approval: 'never' },
|
||||
'danger-full-access': { sandbox: 'danger-full-access', approval: 'never' },
|
||||
} } })
|
||||
const session = freshSession('sess-tie')
|
||||
// Same bundle as request, chosen explicitly: the fold names it.
|
||||
// Same bundle as workspace-write, chosen explicitly: the fold names it.
|
||||
ctx.permission.set(session, 'agentish')
|
||||
expect(ctx.permission.current(session.events)).toBe('agentish')
|
||||
// A knob drifts: the fold's bundle no longer matches → reverse map wins.
|
||||
session.append('approval/policy', { policy: 'never' })
|
||||
session.append('bash/sandbox-mode', { mode: 'danger-full-access' })
|
||||
expect(ctx.permission.current(session.events)).toBe('yolo')
|
||||
expect(ctx.permission.current(session.events)).toBe('danger-full-access')
|
||||
})
|
||||
|
||||
it('set() writes through: one preset event plus both knob events', async () => {
|
||||
const ctx = await mounted()
|
||||
const session = freshSession('sess-set')
|
||||
ctx.permission.set(session, 'yolo')
|
||||
ctx.permission.set(session, 'danger-full-access')
|
||||
expect(session.events.map(e => [e.type, e.data])).toEqual([
|
||||
['permission/preset', { preset: 'yolo' }],
|
||||
['permission/preset', { preset: 'danger-full-access' }],
|
||||
['bash/sandbox-mode', { mode: 'danger-full-access' }],
|
||||
['approval/policy', { policy: 'never' }],
|
||||
])
|
||||
@@ -98,22 +98,22 @@ describe('PermissionService', () => {
|
||||
it('set() to the current preset is a no-op when the knobs already match (clicks are not switches)', async () => {
|
||||
const ctx = await mounted()
|
||||
const session = freshSession('sess-noop')
|
||||
ctx.permission.set(session, 'request')
|
||||
ctx.permission.set(session, 'workspace-write')
|
||||
expect(session.events).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('re-asserting a preset from a drifted (custom) state re-records the choice and repairs the knob', async () => {
|
||||
const ctx = await mounted()
|
||||
const session = freshSession('sess-drift')
|
||||
ctx.permission.set(session, 'yolo')
|
||||
ctx.permission.set(session, 'danger-full-access')
|
||||
// A knob drifts out from under the preset (a direct setter call, a test
|
||||
// scenario): the session derives custom, and re-asserting the preset is
|
||||
// a real switch again — choice re-recorded, only the drifted knob moves.
|
||||
session.append('bash/sandbox-mode', { mode: 'read-only' })
|
||||
ctx.permission.set(session, 'yolo')
|
||||
ctx.permission.set(session, 'danger-full-access')
|
||||
const tail = session.events.slice(4)
|
||||
expect(tail.map(e => [e.type, e.data])).toEqual([
|
||||
['permission/preset', { preset: 'yolo' }],
|
||||
['permission/preset', { preset: 'danger-full-access' }],
|
||||
['bash/sandbox-mode', { mode: 'danger-full-access' }],
|
||||
])
|
||||
})
|
||||
@@ -125,7 +125,7 @@ describe('PermissionService', () => {
|
||||
|
||||
it('optionOf() presents shipped labels/descriptions, falls back to the raw key, and fixes custom', async () => {
|
||||
const ctx = await mounted()
|
||||
expect(ctx.permission.optionOf('yolo')).toEqual({ value: 'yolo', name: 'YOLO', description: 'Full file access, no approval prompts.' })
|
||||
expect(ctx.permission.optionOf('danger-full-access')).toEqual({ value: 'danger-full-access', name: 'danger-full-access', description: 'Full file access, no approval prompts.' })
|
||||
expect(ctx.permission.optionOf('custom')).toEqual({ value: 'custom', name: 'Custom', description: 'A hand-set knob combination outside the preset table.' })
|
||||
const bare = await mounted({ config: { presets: { plain: { sandbox: 'workspace-write', approval: 'ask' } } } })
|
||||
expect(bare.permission.optionOf('plain')).toEqual({ value: 'plain', name: 'plain' })
|
||||
@@ -140,8 +140,8 @@ describe('PermissionService', () => {
|
||||
it('reads a schema-less approval stand-in as the ask default', async () => {
|
||||
const ctx = await mounted({ approvalDefault: undefined })
|
||||
const session = freshSession('sess-standin')
|
||||
ctx.permission.set(session, 'request')
|
||||
ctx.permission.set(session, 'workspace-write')
|
||||
expect(session.events).toHaveLength(0)
|
||||
expect(ctx.permission.current(session.events)).toBe('request')
|
||||
expect(ctx.permission.current(session.events)).toBe('workspace-write')
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user