mirror of
https://bitbucket.org/siakitem/my-pi.git
synced 2026-08-28 08:35:57 +00:00
feat: vendor permission auto-review source
This commit is contained in:
叶林立
@@ -0,0 +1,466 @@
|
||||
import type { DenialCircuitBreaker } from '../src/circuit-breaker.js'
|
||||
import type { AutoReviewConfigFileSystem } from '../src/config-store.js'
|
||||
import type { AutoReviewExtensionDependencies } from '../src/extension.js'
|
||||
import type {
|
||||
ExtensionAPI,
|
||||
ExtensionCommandContext,
|
||||
ExtensionContext,
|
||||
RegisteredCommand,
|
||||
} from '@earendil-works/pi-coding-agent'
|
||||
import type { Authorizer, PermissionsService } from '@gotgenes/pi-permission-system'
|
||||
import { PERMISSIONS_READY_CHANNEL } from '@gotgenes/pi-permission-system'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { AutoReviewConfigStore } from '../src/config-store.js'
|
||||
import { autoReviewConfigSchema } from '../src/config.js'
|
||||
import { createAutoReviewExtension, createAutoReviewExtensionWithConfigStore } from '../src/extension.js'
|
||||
|
||||
type Handler = (...arguments_: unknown[]) => unknown
|
||||
|
||||
function createPiHarness() {
|
||||
const handlers = new Map<string, Handler[]>()
|
||||
const eventHandlers = new Map<string, Handler[]>()
|
||||
const commands = new Map<string, Omit<RegisteredCommand, 'name' | 'sourceInfo'>>()
|
||||
const add = (target: Map<string, Handler[]>, name: string, handler: Handler): void => {
|
||||
target.set(name, [...(target.get(name) ?? []), handler])
|
||||
}
|
||||
const pi = {
|
||||
on: vi.fn((name: string, handler: Handler) => add(handlers, name, handler)),
|
||||
events: {
|
||||
on: vi.fn((name: string, handler: Handler) => add(eventHandlers, name, handler)),
|
||||
},
|
||||
registerCommand: vi.fn((name: string, command: Omit<RegisteredCommand, 'name' | 'sourceInfo'>) => {
|
||||
commands.set(name, command)
|
||||
}),
|
||||
} as unknown as ExtensionAPI
|
||||
|
||||
return {
|
||||
pi,
|
||||
emit(name: string, ...arguments_: unknown[]) {
|
||||
for (const handler of handlers.get(name) ?? []) {
|
||||
handler(...arguments_)
|
||||
}
|
||||
},
|
||||
emitEvent(name: string, ...arguments_: unknown[]) {
|
||||
for (const handler of eventHandlers.get(name) ?? []) {
|
||||
handler(...arguments_)
|
||||
}
|
||||
},
|
||||
getCommand(name: string) {
|
||||
return commands.get(name)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
function context(): ExtensionContext {
|
||||
return {
|
||||
cwd: '/project',
|
||||
modelRegistry: {},
|
||||
sessionManager: {},
|
||||
} as ExtensionContext
|
||||
}
|
||||
|
||||
function configResult() {
|
||||
return {
|
||||
config: autoReviewConfigSchema.parse({}),
|
||||
issues: [],
|
||||
globalPath: '/global/config.json',
|
||||
projectPath: '/project/config.json',
|
||||
}
|
||||
}
|
||||
|
||||
function createConfigStore(initial: Record<string, string>) {
|
||||
const files = new Map(Object.entries(initial))
|
||||
const fileSystem: AutoReviewConfigFileSystem = {
|
||||
readFile: path => files.get(path),
|
||||
writeFile: (path, source) => {
|
||||
files.set(path, source)
|
||||
},
|
||||
rename: (sourcePath, destinationPath) => {
|
||||
const source = files.get(sourcePath)
|
||||
if (source === undefined) {
|
||||
throw new Error(`missing ${sourcePath}`)
|
||||
}
|
||||
files.set(destinationPath, source)
|
||||
files.delete(sourcePath)
|
||||
},
|
||||
mkdir: () => {},
|
||||
unlink: path => {
|
||||
files.delete(path)
|
||||
},
|
||||
}
|
||||
return {
|
||||
files,
|
||||
store: new AutoReviewConfigStore({ agentDir: '/agent', fileSystem }),
|
||||
}
|
||||
}
|
||||
|
||||
function commandContext(notify = vi.fn()): ExtensionCommandContext {
|
||||
return {
|
||||
...context(),
|
||||
mode: 'tui',
|
||||
hasUI: true,
|
||||
ui: {
|
||||
confirm: vi.fn(async () => true),
|
||||
notify,
|
||||
},
|
||||
waitForIdle: vi.fn(async () => {}),
|
||||
reload: vi.fn(),
|
||||
} as unknown as ExtensionCommandContext
|
||||
}
|
||||
|
||||
describe('extension lifecycle', () => {
|
||||
it('registers once when session_start happens before permissions:ready', () => {
|
||||
const harness = createPiHarness()
|
||||
const dispose = vi.fn()
|
||||
const registerAuthorizer = vi.fn(() => dispose)
|
||||
let service: PermissionsService | undefined
|
||||
const authorize = vi.fn<Authorizer['authorize']>()
|
||||
|
||||
createAutoReviewExtension(harness.pi, {
|
||||
loadConfig: configResult,
|
||||
getPermissionsService: () => service,
|
||||
createReviewer: () => authorize,
|
||||
})
|
||||
|
||||
harness.emit('session_start', {}, context())
|
||||
expect(registerAuthorizer).not.toHaveBeenCalled()
|
||||
|
||||
service = { registerAuthorizer } as unknown as PermissionsService
|
||||
harness.emitEvent(PERMISSIONS_READY_CHANNEL, {})
|
||||
harness.emitEvent(PERMISSIONS_READY_CHANNEL, {})
|
||||
|
||||
expect(registerAuthorizer).toHaveBeenCalledOnce()
|
||||
expect(registerAuthorizer).toHaveBeenCalledWith('auto-review', authorize)
|
||||
|
||||
harness.emit('session_shutdown')
|
||||
expect(dispose).toHaveBeenCalledOnce()
|
||||
|
||||
harness.emit('session_start', {}, context())
|
||||
expect(registerAuthorizer).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('registers when permissions:ready happens before session_start', () => {
|
||||
const harness = createPiHarness()
|
||||
const registerAuthorizer = vi.fn(() => vi.fn())
|
||||
const service = {
|
||||
registerAuthorizer,
|
||||
} as unknown as PermissionsService
|
||||
|
||||
createAutoReviewExtension(harness.pi, {
|
||||
loadConfig: configResult,
|
||||
getPermissionsService: () => service,
|
||||
createReviewer: () => vi.fn<Authorizer['authorize']>(),
|
||||
})
|
||||
|
||||
harness.emitEvent(PERMISSIONS_READY_CHANNEL, {})
|
||||
expect(registerAuthorizer).not.toHaveBeenCalled()
|
||||
|
||||
harness.emit('session_start', {}, context())
|
||||
expect(registerAuthorizer).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('makes later instances passive for a shared service and leaves disposal to the owner', async () => {
|
||||
const ownerHarness = createPiHarness()
|
||||
const passiveHarness = createPiHarness()
|
||||
const ownerDispose = vi.fn()
|
||||
const registerAuthorizer = vi.fn(() => ownerDispose)
|
||||
const service = { registerAuthorizer } as unknown as PermissionsService
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const globalPath = '/agent/extensions/pi-permission-auto-review/config.json'
|
||||
const { store } = createConfigStore({
|
||||
[globalPath]: JSON.stringify({ reasoning: 'high' }),
|
||||
})
|
||||
|
||||
createAutoReviewExtension(ownerHarness.pi, {
|
||||
loadConfig: configResult,
|
||||
getPermissionsService: () => service,
|
||||
createReviewer: () => vi.fn<Authorizer['authorize']>(),
|
||||
})
|
||||
createAutoReviewExtensionWithConfigStore(passiveHarness.pi, store, {
|
||||
getPermissionsService: () => service,
|
||||
createReviewer: () => vi.fn<Authorizer['authorize']>(),
|
||||
})
|
||||
|
||||
ownerHarness.emit('session_start', {}, context())
|
||||
passiveHarness.emit('session_start', {}, context())
|
||||
passiveHarness.emitEvent(PERMISSIONS_READY_CHANNEL, {})
|
||||
passiveHarness.emitEvent(PERMISSIONS_READY_CHANNEL, {})
|
||||
|
||||
expect(registerAuthorizer).toHaveBeenCalledOnce()
|
||||
expect(warn).not.toHaveBeenCalled()
|
||||
|
||||
const notify = vi.fn()
|
||||
await passiveHarness.getCommand('permission-auto-review')?.handler('reset global', commandContext(notify))
|
||||
|
||||
expect(registerAuthorizer).toHaveBeenCalledOnce()
|
||||
expect(ownerDispose).not.toHaveBeenCalled()
|
||||
expect(notify).toHaveBeenCalledWith(expect.stringContaining('managed by the main Pi session'), 'error')
|
||||
|
||||
passiveHarness.emit('session_shutdown')
|
||||
expect(ownerDispose).not.toHaveBeenCalled()
|
||||
|
||||
ownerHarness.emit('session_shutdown')
|
||||
expect(ownerDispose).toHaveBeenCalledOnce()
|
||||
|
||||
const replacementHarness = createPiHarness()
|
||||
createAutoReviewExtension(replacementHarness.pi, {
|
||||
loadConfig: configResult,
|
||||
getPermissionsService: () => service,
|
||||
createReviewer: () => vi.fn<Authorizer['authorize']>(),
|
||||
})
|
||||
replacementHarness.emit('session_start', {}, context())
|
||||
expect(registerAuthorizer).toHaveBeenCalledTimes(2)
|
||||
replacementHarness.emit('session_shutdown')
|
||||
|
||||
warn.mockRestore()
|
||||
})
|
||||
|
||||
it('keeps a replacement service owner when the old owner shuts down late', () => {
|
||||
const oldHarness = createPiHarness()
|
||||
const newHarness = createPiHarness()
|
||||
const observerHarness = createPiHarness()
|
||||
const oldDispose = vi.fn()
|
||||
const newDispose = vi.fn()
|
||||
const oldRegisterAuthorizer = vi.fn(() => oldDispose)
|
||||
const newRegisterAuthorizer = vi.fn(() => newDispose)
|
||||
const oldService = { registerAuthorizer: oldRegisterAuthorizer } as unknown as PermissionsService
|
||||
const newService = { registerAuthorizer: newRegisterAuthorizer } as unknown as PermissionsService
|
||||
|
||||
for (const [harness, service] of [
|
||||
[oldHarness, oldService],
|
||||
[newHarness, newService],
|
||||
[observerHarness, newService],
|
||||
] as const) {
|
||||
createAutoReviewExtension(harness.pi, {
|
||||
loadConfig: configResult,
|
||||
getPermissionsService: () => service,
|
||||
createReviewer: () => vi.fn<Authorizer['authorize']>(),
|
||||
})
|
||||
}
|
||||
|
||||
oldHarness.emit('session_start', {}, context())
|
||||
newHarness.emit('session_start', {}, context())
|
||||
expect(oldRegisterAuthorizer).toHaveBeenCalledOnce()
|
||||
expect(newRegisterAuthorizer).toHaveBeenCalledOnce()
|
||||
|
||||
oldHarness.emit('session_shutdown')
|
||||
observerHarness.emit('session_start', {}, context())
|
||||
|
||||
expect(oldDispose).toHaveBeenCalledOnce()
|
||||
expect(newRegisterAuthorizer).toHaveBeenCalledOnce()
|
||||
|
||||
observerHarness.emit('session_shutdown')
|
||||
expect(newDispose).not.toHaveBeenCalled()
|
||||
newHarness.emit('session_shutdown')
|
||||
expect(newDispose).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('registers a defer-only reviewer when config is invalid', async () => {
|
||||
const harness = createPiHarness()
|
||||
let registered: Authorizer['authorize'] | undefined
|
||||
const service = {
|
||||
registerAuthorizer: vi.fn((_name: string, authorize: Authorizer['authorize']) => {
|
||||
registered = authorize
|
||||
return vi.fn()
|
||||
}),
|
||||
} as unknown as PermissionsService
|
||||
|
||||
createAutoReviewExtension(harness.pi, {
|
||||
loadConfig: () => ({
|
||||
config: undefined,
|
||||
issues: [],
|
||||
globalPath: '/global/config.json',
|
||||
projectPath: '/project/config.json',
|
||||
}),
|
||||
getPermissionsService: () => service,
|
||||
})
|
||||
harness.emit('session_start', {}, context())
|
||||
|
||||
const log = { review: vi.fn(), debug: vi.fn() }
|
||||
await expect(
|
||||
registered?.(
|
||||
{
|
||||
requestId: 'request',
|
||||
source: 'tool_call',
|
||||
agentName: null,
|
||||
payload: {
|
||||
kind: 'tool',
|
||||
request: {
|
||||
requester: { agentName: null, forwarded: false, sessionId: null },
|
||||
surface: 'test-tool',
|
||||
toolName: 'test-tool',
|
||||
invokedToolName: null,
|
||||
value: 'request',
|
||||
matchedPattern: '*',
|
||||
commandContext: null,
|
||||
executedUnit: null,
|
||||
},
|
||||
evidence: [],
|
||||
annotations: [],
|
||||
},
|
||||
},
|
||||
{} as never,
|
||||
log,
|
||||
),
|
||||
).resolves.toEqual({ kind: 'defer' })
|
||||
expect(log.review).toHaveBeenCalledWith(
|
||||
'auto_review.decision',
|
||||
expect.objectContaining({ errorCategory: 'config-invalid' }),
|
||||
)
|
||||
})
|
||||
|
||||
it('hot-swaps only the reviewer generation after a config reset', async () => {
|
||||
const globalPath = '/agent/extensions/pi-permission-auto-review/config.json'
|
||||
const { files, store } = createConfigStore({
|
||||
[globalPath]: JSON.stringify({ model: 'old-review-model' }),
|
||||
})
|
||||
const harness = createPiHarness()
|
||||
const firstDispose = vi.fn()
|
||||
const secondDispose = vi.fn()
|
||||
const firstAuthorize = vi.fn<Authorizer['authorize']>()
|
||||
const secondAuthorize = vi.fn<Authorizer['authorize']>()
|
||||
const createReviewer = vi
|
||||
.fn<NonNullable<AutoReviewExtensionDependencies['createReviewer']>>()
|
||||
.mockReturnValueOnce(firstAuthorize)
|
||||
.mockReturnValueOnce(secondAuthorize)
|
||||
const registerAuthorizer = vi.fn().mockReturnValueOnce(firstDispose).mockReturnValueOnce(secondDispose)
|
||||
const service = { registerAuthorizer } as unknown as PermissionsService
|
||||
|
||||
createAutoReviewExtensionWithConfigStore(harness.pi, store, {
|
||||
getPermissionsService: () => service,
|
||||
createReviewer,
|
||||
})
|
||||
harness.emit('session_start', {}, context())
|
||||
const circuitBreaker: DenialCircuitBreaker | undefined = createReviewer.mock.calls[0]?.[0].circuitBreaker
|
||||
if (circuitBreaker === undefined) {
|
||||
throw new Error('reviewer was not created')
|
||||
}
|
||||
circuitBreaker.recordDenied()
|
||||
circuitBreaker.recordDenied()
|
||||
circuitBreaker.recordDenied()
|
||||
expect(circuitBreaker.isOpen()).toBe(true)
|
||||
|
||||
const command = harness.getCommand('permission-auto-review')
|
||||
const ctx = commandContext()
|
||||
await command?.handler('reset global', ctx)
|
||||
|
||||
expect(files.has(globalPath)).toBe(false)
|
||||
expect(firstDispose).toHaveBeenCalledOnce()
|
||||
expect(registerAuthorizer).toHaveBeenNthCalledWith(1, 'auto-review', firstAuthorize)
|
||||
expect(registerAuthorizer).toHaveBeenNthCalledWith(2, 'auto-review', secondAuthorize)
|
||||
expect(createReviewer.mock.calls[0]?.[0]).toMatchObject({
|
||||
config: { model: 'old-review-model' },
|
||||
})
|
||||
expect(createReviewer.mock.calls[1]?.[0]).toMatchObject({
|
||||
config: { model: 'codex-auto-review' },
|
||||
})
|
||||
expect(circuitBreaker.isOpen()).toBe(false)
|
||||
|
||||
harness.emit('session_shutdown')
|
||||
expect(secondDispose).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('preserves the old reviewer when reset leaves the merged config invalid', async () => {
|
||||
const globalPath = '/agent/extensions/pi-permission-auto-review/config.json'
|
||||
const projectPath = '/project/.pi/extensions/pi-permission-auto-review/config.json'
|
||||
const { files, store } = createConfigStore({
|
||||
[globalPath]: JSON.stringify({ reasoning: 'high' }),
|
||||
[projectPath]: JSON.stringify({
|
||||
includeBaselinePolicy: false,
|
||||
additionalPolicy: 'Review conservatively.',
|
||||
}),
|
||||
})
|
||||
const harness = createPiHarness()
|
||||
const firstDispose = vi.fn()
|
||||
const registerAuthorizer = vi.fn(() => firstDispose)
|
||||
const service = { registerAuthorizer } as unknown as PermissionsService
|
||||
const createReviewer = vi.fn(() => vi.fn<Authorizer['authorize']>())
|
||||
const notify = vi.fn()
|
||||
|
||||
createAutoReviewExtensionWithConfigStore(harness.pi, store, {
|
||||
getPermissionsService: () => service,
|
||||
createReviewer,
|
||||
})
|
||||
harness.emit('session_start', {}, context())
|
||||
files.set(projectPath, JSON.stringify({ includeBaselinePolicy: false }))
|
||||
|
||||
await harness.getCommand('permission-auto-review')?.handler('reset global', commandContext(notify))
|
||||
|
||||
expect(files.has(globalPath)).toBe(false)
|
||||
expect(firstDispose).not.toHaveBeenCalled()
|
||||
expect(registerAuthorizer).toHaveBeenCalledOnce()
|
||||
expect(createReviewer).toHaveBeenCalledOnce()
|
||||
expect(notify).toHaveBeenCalledWith(
|
||||
expect.stringContaining('the merged config is invalid; the previous reviewer remains active'),
|
||||
'error',
|
||||
)
|
||||
})
|
||||
|
||||
it('restores the old reviewer if candidate registration fails', async () => {
|
||||
const globalPath = '/agent/extensions/pi-permission-auto-review/config.json'
|
||||
const { store } = createConfigStore({
|
||||
[globalPath]: JSON.stringify({ reasoning: 'high' }),
|
||||
})
|
||||
const harness = createPiHarness()
|
||||
const firstDispose = vi.fn()
|
||||
const restoredDispose = vi.fn()
|
||||
const firstAuthorize = vi.fn<Authorizer['authorize']>()
|
||||
const secondAuthorize = vi.fn<Authorizer['authorize']>()
|
||||
const registerAuthorizer = vi
|
||||
.fn()
|
||||
.mockReturnValueOnce(firstDispose)
|
||||
.mockImplementationOnce(() => {
|
||||
throw new Error('candidate rejected')
|
||||
})
|
||||
.mockReturnValueOnce(restoredDispose)
|
||||
const service = { registerAuthorizer } as unknown as PermissionsService
|
||||
const notify = vi.fn()
|
||||
|
||||
createAutoReviewExtensionWithConfigStore(harness.pi, store, {
|
||||
getPermissionsService: () => service,
|
||||
createReviewer: vi.fn().mockReturnValueOnce(firstAuthorize).mockReturnValueOnce(secondAuthorize),
|
||||
})
|
||||
harness.emit('session_start', {}, context())
|
||||
|
||||
await harness.getCommand('permission-auto-review')?.handler('reset global', commandContext(notify))
|
||||
|
||||
expect(firstDispose).toHaveBeenCalledOnce()
|
||||
expect(registerAuthorizer).toHaveBeenNthCalledWith(2, 'auto-review', secondAuthorize)
|
||||
expect(registerAuthorizer).toHaveBeenNthCalledWith(3, 'auto-review', firstAuthorize)
|
||||
expect(notify).toHaveBeenCalledWith(expect.stringContaining('old reviewer was restored'), 'error')
|
||||
|
||||
harness.emit('session_shutdown')
|
||||
expect(restoredDispose).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('keeps a saved generation pending until permission-system becomes ready', async () => {
|
||||
const globalPath = '/agent/extensions/pi-permission-auto-review/config.json'
|
||||
const { store } = createConfigStore({
|
||||
[globalPath]: JSON.stringify({ reasoning: 'high' }),
|
||||
})
|
||||
const harness = createPiHarness()
|
||||
const oldAuthorize = vi.fn<Authorizer['authorize']>()
|
||||
const pendingAuthorize = vi.fn<Authorizer['authorize']>()
|
||||
const createReviewer = vi.fn().mockReturnValueOnce(oldAuthorize).mockReturnValueOnce(pendingAuthorize)
|
||||
const registerAuthorizer = vi.fn(() => vi.fn())
|
||||
let service: PermissionsService | undefined
|
||||
const notify = vi.fn()
|
||||
|
||||
createAutoReviewExtensionWithConfigStore(harness.pi, store, {
|
||||
getPermissionsService: () => service,
|
||||
createReviewer,
|
||||
})
|
||||
harness.emit('session_start', {}, context())
|
||||
await harness.getCommand('permission-auto-review')?.handler('reset global', commandContext(notify))
|
||||
|
||||
expect(registerAuthorizer).not.toHaveBeenCalled()
|
||||
expect(notify).toHaveBeenCalledWith(
|
||||
expect.stringContaining('will activate when pi-permission-system is ready'),
|
||||
'warning',
|
||||
)
|
||||
|
||||
service = { registerAuthorizer } as unknown as PermissionsService
|
||||
harness.emitEvent(PERMISSIONS_READY_CHANNEL, {})
|
||||
expect(registerAuthorizer).toHaveBeenCalledWith('auto-review', pendingAuthorize)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user