From 42f0a0aaabbda34bc44784bebbcef512388add37 Mon Sep 17 00:00:00 2001 From: SikongJueluo Date: Mon, 17 Aug 2026 00:53:00 +0800 Subject: [PATCH] feat(permission): complete shadow review events for offline analysis - key inner-cmd decisive review events by requestId so link decisions join offline - record judge runtime id, prompt and tool schema versions, end-to-end and model latency, input and output usage, and evidence-quality flags on every judge result row - record forwarded and session-mismatch preflight defers so they stay visible in the offline denominator --- packages/pi-permission-ai-judge/src/index.ts | 251 ++++++++++++------ packages/pi-permission-ai-judge/src/model.ts | 18 ++ .../test/lifecycle.test.ts | 86 ++++++ .../pi-permission-ai-judge/test/model.test.ts | 4 + .../src/handlers/timeout.ts | 4 + .../test/authorizer.test.ts | 3 + 6 files changed, 289 insertions(+), 77 deletions(-) diff --git a/packages/pi-permission-ai-judge/src/index.ts b/packages/pi-permission-ai-judge/src/index.ts index ad5945f..7cc4ed0 100644 --- a/packages/pi-permission-ai-judge/src/index.ts +++ b/packages/pi-permission-ai-judge/src/index.ts @@ -2,6 +2,7 @@ import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import { getPermissionsService, PERMISSIONS_READY_CHANNEL, + type PromptPermissionDetails, } from "@gotgenes/pi-permission-system"; import { buildBashJudgmentEvidence } from "./evidence"; import { @@ -9,6 +10,7 @@ import { requestStructuredVerdict, type ModelAvailability, } from "./model"; +import { PROMPT_VERSION, TOOL_SCHEMA_VERSION } from "./prompt"; const LINK_NAME = "ai-bash-judge"; const REVIEW_SCHEMA_VERSION = 1; @@ -18,12 +20,64 @@ interface RootSession { readonly expectedSessionId: string; readonly model: ModelAvailability; readonly shutdown: AbortController; + /** Opaque per-runtime identity for cohort segmentation. */ + readonly judgeRuntimeId: string; } function reasonLength(reason: string): number { return [...reason].length; } +/** + * Evidence-quality flags without evidence content (PIEXTENSIO-9). + * + * This bootstrap slice sends command-only input: no conversation, no cwd, + * no explicit user text (see docs/research/ai-bash-judge-input-minimality). + * `false` marks a definitively absent field; `null` marks one this slice + * does not capture, so the analyzer never mistakes absence for zero. + */ +function evidenceQuality( + structuredFullInput: boolean, + forwardedProvenance: boolean | null = null, +): Record { + return { + structuredFullInput, + legacyMessage: false, + requesterCwd: null, + explicitUserText: false, + forwardedProvenance, + conversationItems: null, + conversationChars: null, + truncated: false, + latestUserPreserved: null, + }; +} + +/** + * Identity, cohort, and reproducibility fields shared by every result kind. + * End-to-end latency is measured from authorize entry to this call. + */ +function resultBase( + judgeRuntimeId: string, + details: PromptPermissionDetails, + startedAt: number, +): Record { + return { + schemaVersion: REVIEW_SCHEMA_VERSION, + requestId: details.requestId, + judgeRuntimeId, + mode: "shadow", + origin: + details.forwarding !== undefined || + details.payload.kind === "forwarded" + ? "forwarded" + : "local", + promptVersion: PROMPT_VERSION, + toolSchemaVersion: TOOL_SCHEMA_VERSION, + judgeLatencyMs: Date.now() - startedAt, + }; +} + /** Register a Shadow-only structured-output judge for local native Bash asks. */ export default function permissionAiJudge(pi: ExtensionAPI): void { let root: RootSession | undefined; @@ -43,89 +97,131 @@ export default function permissionAiJudge(pi: ExtensionAPI): void { disposeAuthorizer = service.registerAuthorizer( LINK_NAME, async (details, _query, log) => { + const startedAt = Date.now(); try { - // Forwarded asks do not carry a structured child full command in - // permission-system 25.3/25.4. Never parse the legacy prose. - if ( - details.forwarding !== undefined || - details.payload.kind === "forwarded" - ) { - return { kind: "defer" }; - } + // Forwarded asks do not carry a structured child full + // command in permission-system 25.3/25.4. Never parse the + // legacy prose. The deferral is recorded so the request + // stays visible in the offline denominator instead of + // silently vanishing. + if ( + details.forwarding !== undefined || + details.payload.kind === "forwarded" + ) { + log.review("ai_bash_judge.result", { + ...resultBase( + captured.judgeRuntimeId, + details, + startedAt, + ), + resultKind: "preflight_defer", + verdict: null, + effectiveVerdict: "defer", + modelCalled: false, + code: "missing_structured_input", + evidenceQuality: evidenceQuality(false, false), + }); + return { kind: "defer" }; + } - if (captured.getSessionId() !== captured.expectedSessionId) { - log.debug("ai_bash_judge.root_session_mismatch"); - return { kind: "defer" }; - } + // Ignore unrelated permission surfaces without producing a + // Shadow row or invoking the model: the v0.1 cohort selects + // accessSurface = bash only. + if (details.payload.kind !== "bash") { + return { kind: "defer" }; + } - // Ignore unrelated permission surfaces without producing a - // Shadow row or invoking the model. - if (details.payload.kind !== "bash") { - return { kind: "defer" }; - } + if ( + captured.getSessionId() !== captured.expectedSessionId + ) { + log.review("ai_bash_judge.result", { + ...resultBase( + captured.judgeRuntimeId, + details, + startedAt, + ), + resultKind: "preflight_defer", + verdict: null, + effectiveVerdict: "defer", + modelCalled: false, + code: "session_ownership_unproven", + evidenceQuality: evidenceQuality(false), + }); + return { kind: "defer" }; + } - const evidence = buildBashJudgmentEvidence(details); - if (evidence === undefined) { - log.review("ai_bash_judge.result", { - schemaVersion: REVIEW_SCHEMA_VERSION, - requestId: details.requestId, - mode: "shadow", - origin: "local", - resultKind: "preflight_defer", - verdict: null, - effectiveVerdict: "defer", - modelCalled: false, - code: "invalid_evidence", - }); - return { kind: "defer" }; - } + const evidence = buildBashJudgmentEvidence(details); + if (evidence === undefined) { + log.review("ai_bash_judge.result", { + ...resultBase( + captured.judgeRuntimeId, + details, + startedAt, + ), + resultKind: "preflight_defer", + verdict: null, + effectiveVerdict: "defer", + modelCalled: false, + code: "invalid_evidence", + evidenceQuality: evidenceQuality(false), + }); + return { kind: "defer" }; + } - // `captured.model` is the session-start snapshot. Config and - // model-select support are deliberately outside this slice. - const result = await requestStructuredVerdict( - captured.model, - evidence, - captured.shutdown.signal, - ); + // `captured.model` is the session-start snapshot. Config and + // model-select support are deliberately outside this slice. + const result = await requestStructuredVerdict( + captured.model, + evidence, + captured.shutdown.signal, + ); - if (result.kind === "judgment") { - log.review("ai_bash_judge.result", { - schemaVersion: REVIEW_SCHEMA_VERSION, - requestId: details.requestId, - mode: "shadow", - origin: "local", - resultKind: "judgment", - verdict: result.verdict, - effectiveVerdict: "defer", - modelCalled: true, - code: null, - provider: result.metadata.provider, - model: result.metadata.model, - api: result.metadata.api, - // Log key deliberately avoids the substring "token": - // permission-system masks any key matching /token/i - // (structural key-name redaction), which would erase - // this usage telemetry from the review log. - outputUsage: result.outputTokens, - reasonLength: reasonLength(result.reason), - }); - } else { - log.review("ai_bash_judge.result", { - schemaVersion: REVIEW_SCHEMA_VERSION, - requestId: details.requestId, - mode: "shadow", - origin: "local", - resultKind: "infrastructure_failure", - verdict: null, - effectiveVerdict: "defer", - modelCalled: result.modelCalled, - code: result.code, - provider: result.metadata?.provider ?? null, - model: result.metadata?.model ?? null, - api: result.metadata?.api ?? null, - outputUsage: result.outputTokens ?? null, - }); - } + if (result.kind === "judgment") { + log.review("ai_bash_judge.result", { + ...resultBase( + captured.judgeRuntimeId, + details, + startedAt, + ), + resultKind: "judgment", + verdict: result.verdict, + effectiveVerdict: "defer", + modelCalled: true, + code: null, + provider: result.metadata.provider, + model: result.metadata.model, + api: result.metadata.api, + // Log keys deliberately avoid the substring + // "token": permission-system masks any key matching + // /token/i (structural key-name redaction), which + // would erase usage telemetry from the review log. + inputUsage: result.inputTokens, + outputUsage: result.outputTokens, + modelLatencyMs: result.modelLatencyMs, + reasonLength: reasonLength(result.reason), + evidenceQuality: evidenceQuality(true), + }); + } else { + log.review("ai_bash_judge.result", { + ...resultBase( + captured.judgeRuntimeId, + details, + startedAt, + ), + resultKind: "infrastructure_failure", + verdict: null, + effectiveVerdict: "defer", + modelCalled: result.modelCalled, + code: result.code, + provider: result.metadata?.provider ?? null, + model: result.metadata?.model ?? null, + api: result.metadata?.api ?? null, + inputUsage: result.inputTokens ?? null, + outputUsage: result.outputTokens ?? null, + modelLatencyMs: result.modelLatencyMs, + evidenceQuality: evidenceQuality(true), + }); + } // Bootstrap behavior is Shadow-only: the parsed prediction // is recorded but never changes permission authority. @@ -156,6 +252,7 @@ export default function permissionAiJudge(pi: ExtensionAPI): void { expectedSessionId: sessionId, model: createModelAvailability(ctx.model, ctx.modelRegistry), shutdown: new AbortController(), + judgeRuntimeId: crypto.randomUUID(), }; tryRegister(); }); diff --git a/packages/pi-permission-ai-judge/src/model.ts b/packages/pi-permission-ai-judge/src/model.ts index ff17acd..94c4389 100644 --- a/packages/pi-permission-ai-judge/src/model.ts +++ b/packages/pi-permission-ai-judge/src/model.ts @@ -55,7 +55,9 @@ export type ModelAttempt = readonly verdict: SemanticVerdict; readonly reason: string; readonly metadata: ModelMetadata; + readonly inputTokens: number | null; readonly outputTokens: number; + readonly modelLatencyMs: number; } | { readonly kind: "infrastructure_failure"; @@ -64,6 +66,10 @@ export type ModelAttempt = readonly modelCalled: boolean; /** Completion-token usage when a response arrived, else null. */ readonly outputTokens?: number | null; + /** Prompt-token usage when a response arrived, else null. */ + readonly inputTokens?: number | null; + /** Wall-clock model-call latency; null when the model was not called. */ + readonly modelLatencyMs: number | null; }; function forcedToolChoice(api: string): unknown | undefined { @@ -158,17 +164,22 @@ export async function requestStructuredVerdict( ? availability.metadata : undefined, modelCalled: false, + modelLatencyMs: null, }; } let modelCalled = false; let observedOutputTokens: number | null = null; + let observedInputTokens: number | null = null; + let callStartedAt = 0; const failure = (code: InfrastructureCode): ModelAttempt => ({ kind: "infrastructure_failure", code, metadata: availability.metadata, modelCalled, outputTokens: observedOutputTokens, + inputTokens: observedInputTokens, + modelLatencyMs: modelCalled ? Date.now() - callStartedAt : null, }); const timeoutController = new AbortController(); const requestController = new AbortController(); @@ -186,6 +197,7 @@ export async function requestStructuredVerdict( } modelCalled = true; + callStartedAt = Date.now(); const response = await availability.complete( buildJudgeContext(evidence), requestController.signal, @@ -202,6 +214,10 @@ export async function requestStructuredVerdict( } const outputTokens = response.usage.output; + const inputTokens = response.usage.input; + if (Number.isFinite(inputTokens)) { + observedInputTokens = inputTokens; + } if (Number.isFinite(outputTokens)) { observedOutputTokens = outputTokens; } @@ -251,7 +267,9 @@ export async function requestStructuredVerdict( verdict: args.verdict, reason, metadata: availability.metadata, + inputTokens: observedInputTokens, outputTokens, + modelLatencyMs: Date.now() - callStartedAt, }; } catch { const code: InfrastructureCode = shutdownSignal.aborted diff --git a/packages/pi-permission-ai-judge/test/lifecycle.test.ts b/packages/pi-permission-ai-judge/test/lifecycle.test.ts index 2641c88..9d4da3a 100644 --- a/packages/pi-permission-ai-judge/test/lifecycle.test.ts +++ b/packages/pi-permission-ai-judge/test/lifecycle.test.ts @@ -215,10 +215,22 @@ describe("AI judge lifecycle", () => { details: expect.objectContaining({ requestId: "req-1", mode: "shadow", + origin: "local", + judgeRuntimeId: expect.any(String), + promptVersion: "bash-shadow-v1", + toolSchemaVersion: "report-verdict-v1", + judgeLatencyMs: expect.any(Number), + modelLatencyMs: expect.any(Number), + inputUsage: 20, + outputUsage: 10, resultKind: "judgment", verdict: "allow", effectiveVerdict: "defer", modelCalled: true, + evidenceQuality: expect.objectContaining({ + structuredFullInput: true, + explicitUserText: false, + }), }), }, ]); @@ -229,6 +241,80 @@ describe("AI judge lifecycle", () => { expect(dispose).toHaveBeenCalledTimes(1); }); + it("records a forwarded ask as a preflight defer without calling the model", async () => { + let authorize: Authorizer["authorize"] | undefined; + const service = { + registerAuthorizer: vi.fn((_name, callback) => { + authorize = callback; + return vi.fn(); + }), + checkPermission: vi.fn(), + getToolPermission: vi.fn(), + } as unknown as PermissionsService; + publishPermissionsService(service); + publishedService = service; + + const complete = vi.fn(); + const ctx = { + hasUI: true, + sessionManager: { getSessionId: () => "session-root" }, + model: { + id: "test-model", + provider: "test-provider", + api: "openai-codex-responses", + } as Model, + modelRegistry: { complete }, + } as unknown as ExtensionContext; + + const harness = createFakePi(); + extension(harness.pi); + harness.start(ctx); + harness.ready(); + + const reviews: Array<{ + event: string; + details?: Record; + }> = []; + const forwarded = { + ...ask(), + forwarding: { requestId: "fwd-1" } as never, + } as PromptPermissionDetails; + const verdict = await authorize!( + forwarded, + { + checkPermission: vi.fn(), + getToolPermission: vi.fn(), + }, + { + review: (event, details) => reviews.push({ event, details }), + debug: vi.fn(), + }, + ); + + expect(verdict).toEqual({ kind: "defer" }); + expect(complete).not.toHaveBeenCalled(); + expect(reviews).toEqual([ + { + event: "ai_bash_judge.result", + details: expect.objectContaining({ + requestId: "req-1", + origin: "forwarded", + resultKind: "preflight_defer", + verdict: null, + effectiveVerdict: "defer", + modelCalled: false, + code: "missing_structured_input", + evidenceQuality: expect.objectContaining({ + structuredFullInput: false, + forwardedProvenance: false, + }), + }), + }, + ]); + + harness.shutdown(); + }); + it("does not register from a headless child", () => { const service = { registerAuthorizer: vi.fn(), diff --git a/packages/pi-permission-ai-judge/test/model.test.ts b/packages/pi-permission-ai-judge/test/model.test.ts index 495077c..f04482a 100644 --- a/packages/pi-permission-ai-judge/test/model.test.ts +++ b/packages/pi-permission-ai-judge/test/model.test.ts @@ -84,7 +84,9 @@ describe("requestStructuredVerdict", () => { verdict: "defer", reason: "User intent is unavailable.", metadata, + inputTokens: 10, outputTokens: 12, + modelLatencyMs: expect.any(Number), }); }); @@ -264,6 +266,7 @@ describe("requestStructuredVerdict", () => { code: "no_model", metadata: undefined, modelCalled: false, + modelLatencyMs: null, }); await expect( @@ -277,6 +280,7 @@ describe("requestStructuredVerdict", () => { code: "unsupported_api", metadata, modelCalled: false, + modelLatencyMs: null, }); const shutdown = new AbortController(); diff --git a/packages/pi-permission-inner-cmd/src/handlers/timeout.ts b/packages/pi-permission-inner-cmd/src/handlers/timeout.ts index 0fa1b47..f3cbdcd 100644 --- a/packages/pi-permission-inner-cmd/src/handlers/timeout.ts +++ b/packages/pi-permission-inner-cmd/src/handlers/timeout.ts @@ -108,13 +108,17 @@ export const timeoutHandler: CommandHandler = { ); switch (result.state) { case "allow": + // `requestId` joins this link decision to the gate's + // permission_request.* entries for offline analysis. log.review("inner_cmd.allow", { + requestId: details.requestId, command: fullCommand, innerCommand, }); return { kind: "allow" }; case "deny": log.review("inner_cmd.deny", { + requestId: details.requestId, command: fullCommand, innerCommand, }); diff --git a/packages/pi-permission-inner-cmd/test/authorizer.test.ts b/packages/pi-permission-inner-cmd/test/authorizer.test.ts index 19396be..ce6a4dd 100644 --- a/packages/pi-permission-inner-cmd/test/authorizer.test.ts +++ b/packages/pi-permission-inner-cmd/test/authorizer.test.ts @@ -172,6 +172,7 @@ describe("authorizeInnerCommand — recognized wrapper verdicts", () => { level: "review", event: "inner_cmd.allow", details: { + requestId: "req-1", command: "timeout 30s pnpm test", innerCommand: "pnpm test", }, @@ -211,6 +212,7 @@ describe("authorizeInnerCommand — recognized wrapper verdicts", () => { level: "review", event: "inner_cmd.deny", details: { + requestId: "req-1", command: "timeout 30s rm -rf /", innerCommand: "rm -rf /", }, @@ -547,6 +549,7 @@ describe("authorizeInnerCommand — scaffolded commands", () => { level: "review", event: "inner_cmd.allow", details: { + requestId: "req-1", command: full, innerCommand: "pnpm install --frozen-lockfile", },