refactor(permission): simplify Auto review ownership

This commit is contained in:
pku-xht committed 2026-08-29 06:24:58 +08:00
1 parent 64fa60d3d4
commit e6ae1bc751
13 files changed
+22 -30

No files matched your search

+2 -2
View File
@@ -2,5 +2,5 @@
# side as of the last confirmed-consistent state. Both languages carry equal authority;
# after editing either side, bring the other along and re-record with:
# pnpm run verify-translation-pairing --write docs/subsystems/permission-presets.md
permission-presets.md: 128d6d99c7f61ec8801a8bb28b280c3d2f73b247
permission-presets.zh.md: 0a23d3c73be433a9a899bcb21267a59398f9a1a8
permission-presets.md: 1264b7295e934aded05f2c3c19504bfcf8be6714
permission-presets.zh.md: 7e1429011fdf7b86d4e805a962fa6e50aa01a051
+1 -1
View File
@@ -95,7 +95,7 @@ Owns the deployment's configured permission presets, the fixed Auto integration
* @param admit - synchronous gate run before live Auto selection or restore.
* @returns the async effect disposer that removes Auto.
*/
registerAuto(admit: (session: Session) => void): () => Promise<void>
registerAuto(admit: () => void): () => Promise<void>
/**
* Resolve the preset matching the effective knob values. A still-matching
+1 -1
View File
@@ -95,7 +95,7 @@ Owns the deployment's configured permission presets, the fixed Auto integration
* @param admit - synchronous gate run before live Auto selection or restore.
* @returns the async effect disposer that removes Auto.
*/
registerAuto(admit: (session: Session) => void): () => Promise<void>
registerAuto(admit: () => void): () => Promise<void>
/**
* Resolve the preset matching the effective knob values. A still-matching
@@ -189,7 +189,6 @@ export function AskQuestionRow({ toolName, block, inspect, t }: AskQuestionRowPr
body={transcript === null ? model.body : null}
output={transcript === null ? model.output : null}
askQuestion={transcript}
autoReviewDenial={model.autoReviewDenial}
state={state}
inspect={inspect}
/>
@@ -13,7 +13,6 @@ import {
terminalCardModel,
terminalFailed,
} from '../models/terminal-card-model.ts'
import { localizeAutoReviewDenial } from '../models/auto-review-denial.ts'
import { toolRowModel, type ToolRowState } from '../models/tool-call-model.ts'
import { CONVERSATION_NS as NS } from '../../locale.ts'
import css from './bash-sample.module.css'
@@ -42,12 +41,11 @@ function stateStatus(state: ToolRowState, t: BashRowProps['t']): string | null {
/** Renders expandable Bash output with an accessible lifecycle label. */
export function BashRow({ toolName, block, sessionId, useSessions, inspect, t }: BashRowProps) {
const model = toolRowModel(toolName, block)
const autoReview = model.autoReviewDenial === null ? null : localizeAutoReviewDenial(model.autoReviewDenial, t)
// An omitted shell workdir is the session workspace; relative values resolve
// against it before reaching the terminal primitive.
const cwd = useSessions(list => list.byId[sessionId]?.cwd)
const terminalModel = terminalCardModel(block, cwd)
const terminal = autoReview !== null || terminalModel === null ? null : localizeTerminalCardModel(terminalModel, t)
const terminal = terminalModel === null ? null : localizeTerminalCardModel(terminalModel, t)
// A failing exit status is the terminal card's own error signal (the call
// itself settles isError:false), surfaced as the row's red state dot.
const state = model.state === 'ok' && terminalModel !== null && terminalFailed(terminalModel)
@@ -58,14 +56,14 @@ export function BashRow({ toolName, block, sessionId, useSessions, inspect, t }:
// Execution failures and persistent-shell results have no terminal card.
// Keep their recorded args and complete output reachable through the generic
// body; background acknowledgements and malformed calls remain collapsed.
const body = autoReview === null ? model.body : null
const output = autoReview?.output ?? model.output
const body = model.body
const output = model.output
const genericBody = terminal === null
&& (autoReview !== null || model.state === 'error' || isSettledPersistentShellCall(block))
&& (model.state === 'error' || isSettledPersistentShellCall(block))
&& (body !== null || output !== null)
const expandable = terminal !== null || genericBody
const open = expanded && expandable
const failureLine = autoReview?.summary ?? (model.state === 'error' ? model.errorSummary : null)
const failureLine = model.state === 'error' ? model.errorSummary : null
const toggleExpand = () => {
setExpanded(v => !v)
}
@@ -26,7 +26,6 @@ export function FileMutationRow({ toolName, block, cwd, home, openFile, inspect,
body={null}
output={model.output}
errorSummary={model.errorSummary}
autoReviewDenial={model.autoReviewDenial}
diff={diff}
state={model.state}
filePath={model.filePath}
@@ -26,7 +26,6 @@ export function ReadRow({ toolName, block, cwd, home, openFile, inspect, t }: Re
body={null}
output={model.output}
errorSummary={model.errorSummary}
autoReviewDenial={model.autoReviewDenial}
read={read}
state={model.state}
filePath={model.filePath}
@@ -33,7 +33,6 @@ export function SearchRow({ toolName, block, inspect, t }: SearchRowProps) {
// preserves the generic fallback for errors and legacy results.
output={model.output}
errorSummary={model.errorSummary}
autoReviewDenial={model.autoReviewDenial}
search={search}
state={model.state}
inspect={inspect}
@@ -61,7 +61,6 @@ export function TodoRow({ toolName, block, inspect, t }: TodoRowProps) {
body={model.body}
output={model.output}
errorSummary={model.errorSummary}
autoReviewDenial={model.autoReviewDenial}
state={model.state}
inspect={inspect}
/>
@@ -32,7 +32,6 @@ export function WebRow({ toolName, block, inspect, t }: WebRowProps) {
body={null}
output={model.output}
errorSummary={model.errorSummary}
autoReviewDenial={model.autoReviewDenial}
web={web}
state={model.state}
inspect={inspect}
@@ -1230,7 +1230,7 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [
description: 'Owns the deployment\'s configured permission presets, the fixed Auto integration hook, and their write path. Requires a confining `ctx.shell` executor and `ctx.approval`; unmatched knob values are reported as CUSTOM_PRESET, not an error.',
methods: [
{
signature: 'registerAuto(admit: (session: Session) => void): () => Promise<void>',
signature: 'registerAuto(admit: () => void): () => Promise<void>',
description: 'Publish the fixed current-session Auto preset for the calling integration\'s effect lifetime.',
parameters: [{ name: 'admit', description: 'synchronous gate run before live Auto selection or restore.' }],
returns: 'the async effect disposer that removes Auto.',
@@ -198,7 +198,7 @@ export class PermissionPresetService extends Service {
static inject = ['shell', 'approval', 'sessions', 'sessionProjections']
private readonly presets: Record<string, PresetSpec>
private autoAdmit: ((session: Session) => void) | undefined
private autoAdmit: (() => void) | undefined
private readonly permissionsRegistration: (() => void) & { republish(session: Session): void }
private defaultSettings: () => PermissionSettings
@@ -308,7 +308,7 @@ export class PermissionPresetService extends Service {
* @param admit - synchronous gate run before live Auto selection or restore.
* @returns the async effect disposer that removes Auto.
*/
registerAuto(admit: (session: Session) => void): () => Promise<void> {
registerAuto(admit: () => void): () => Promise<void> {
return this.ctx.effect(() => {
if (this.autoAdmit !== undefined) throw new Error('permission: preset "auto" is already registered')
this.autoAdmit = admit
@@ -423,7 +423,7 @@ export class PermissionPresetService extends Service {
/** Apply one preset with the caller-selected live or initialization policy writer. */
private apply(session: Session, name: string, setApproval: (policy: ApprovalPolicy) => void): void {
const spec = this.resolve(name)
if (name === AUTO_PRESET) this.autoAdmit?.(session)
if (name === AUTO_PRESET) this.autoAdmit?.()
if (this.current(session) !== name) {
session.append('permission/preset', { preset: name })
}
@@ -451,7 +451,7 @@ export class PermissionPresetService extends Service {
if (this.autoAdmit === undefined) {
throw new Error('permission: cannot restore preset "auto" without its active integration')
}
this.autoAdmit(session)
this.autoAdmit()
}
if (preset === null && sandbox === null && approval === null && !seeded) {
const name = this.defaultPreset
@@ -50,7 +50,7 @@ function freshSession(id: string): Session {
return Session.create(SessionId(id))
}
async function mountAuto(ctx: Context, admit: (session: Session) => void = () => {}) {
async function mountAuto(ctx: Context, admit: () => void = () => {}) {
return ctx.plugin(Object.assign((pluginCtx: Context) => {
pluginCtx.permissionPresets.registerAuto(admit)
}, { inject: ['permissionPresets'] }))
@@ -153,12 +153,12 @@ describe('PermissionPresetService', () => {
it('runs Auto admission before any write, including a no-op selection', async () => {
const ctx = await mounted()
const admitted: Session[] = []
await mountAuto(ctx, (session) => { admitted.push(session) })
let admissions = 0
await mountAuto(ctx, () => { admissions += 1 })
const session = freshSession('sess-auto-admit')
ctx.permissionPresets.set(session, AUTO_PRESET)
expect(admitted).toEqual([session])
expect(admissions).toBe(1)
expect(session.events.map(event => [event.type, event.data])).toEqual([
['permission/preset', { preset: AUTO_PRESET }],
['sandbox/mode', { mode: 'danger-full-access' }],
@@ -167,7 +167,7 @@ describe('PermissionPresetService', () => {
expect(ctx.permissionPresets.current(session)).toBe(AUTO_PRESET)
ctx.permissionPresets.set(session, AUTO_PRESET)
expect(admitted).toEqual([session, session])
expect(admissions).toBe(2)
expect(session.events).toHaveLength(3)
})
@@ -311,15 +311,15 @@ describe('new-session default', () => {
it('admits persisted Auto through the live integration without rewriting it', async () => {
const ctx = await mounted()
const admitted: Session[] = []
await mountAuto(ctx, (session) => { admitted.push(session) })
let admissions = 0
await mountAuto(ctx, () => { admissions += 1 })
const source = freshSession('auto-source-present')
source.append('permission/preset', { preset: AUTO_PRESET })
source.append('sandbox/mode', { mode: 'danger-full-access' })
source.append('approval/policy', { policy: 'never' })
const resumed = ctx.sessions.create(SessionId('auto-with-integration'), { seed: source.events })
expect(admitted).toEqual([resumed])
expect(admissions).toBe(1)
expect(ctx.permissionPresets.current(resumed)).toBe(AUTO_PRESET)
expect(resumed.events.filter(event => event.type === 'permission/preset')).toHaveLength(1)
})