mirror of
https://bitbucket.org/siakitem/my-pi.git
synced 2026-10-26 10:45:22 +00:00
feat: 移除 Codex Fast mode 与 AutoReview 后台分析,模型改为配置驱动
移除 pi-extension-codex-fast-mode:
- 删除整个扩展目录;同步清理根 package.json 的 files / pi.extensions /
dependencies 条目、package-lock.json、tests/extension-load.test.ts 断言
与文档章节。
- pi-minimal-footer 的 Fast on 渲染是该扩展的唯一消费者。扩展移除后 accent
分支不再有生产者,collectFooterStatusItems 简化为直接返回非空状态字符串,
去掉 FooterStatusItem 与 accent 标志,避免留下死代码。
移除 500 次裁决后的后台分析体系:
- 删除 analysis-agent.ts(192 行)与 ledger.ts(296 行),以及对应的
auto-review-runtime / auto-review-ledger 测试。
- runtime.ts 由 177 行收敛到 85 行:安装选项缩为 { service, config };删除
handleAgentSettled 与 agent_settled 注册;保留 session_start / turn_start
(denial circuit breaker 属安全语义)/ session_shutdown。
- 删除 analysis 配置键:config、extension-config、config-schema、生成的
permissions.schema.json 与种子配置。
- 删除 reviewer 的 onDecision 回调(唯一消费者是 ledger)。
- 删除迁移机制 scripts/migrate-permission-policy.mjs、其测试与 update.sh 中
46 行 migrate_permission_policy 接线。种子部署保持“目标缺失才写入”,已存在
的 policy.jsonc 永不被覆盖。
AutoReview 模型改为纯配置驱动:
- 删除 DEFAULT_PROVIDER / DEFAULT_MODEL 常量与 findCodexTemplate 合成逻辑;
model.ts 由 65 行收敛到 48 行。
- provider / model 任一缺失时返回新增失败类别 model-unconfigured 并 defer 到
人工审批;provider 未注册或模型未发布同样 defer。
- reasoning 不再有默认常量,完全由配置决定。
- session_start 在 auto-review 已列入 authorizerChain 且 provider/model 未配齐
时 ui.notify 提示一次,避免不使用 AutoReview 的用户被打扰。
- 种子 autoReview 只保留 enabled,provider/model/reasoning 由用户按本机实际
可用的模型填写。
破坏性变更:analysis 键已从 strictObject schema 删除,任何仍带该键的
policy.jsonc 会使整份配置被拒(fail-closed 到全 ask)。这是刻意的变更,已由
config-loader 测试固定,防止将来无声放宽。
测试:
- 新增 auto-review-model.test.ts(7 例),覆盖此前完全缺失的模型解析:真实
命中、未配置、provider 未知、模型未发布,以及不再合成替代模型。
- 重写 auto-review-runtime.test.ts(6 例):authorizer 注册、未配置时提示
一次、已配置 / disabled / 无 UI / 不在 chain 时静默。
- config-loader.test.ts 新增 2 例:带 analysis 的旧配置被拒且 fail-closed、
autoReview 逐字段合并。
- composition-root.test.ts 的 EXPECTED_HANDLERS 移除 agent_settled。
文档:AGENTS.md、README.md、pi-permission-system 的 README / CHANGELOG /
configuration.md / architecture.md / auto-review UPSTREAM.md。
验证:pi-permission-system 的 typecheck、2577 个测试与 build 全部通过;根 50
个测试通过;schema 重新生成无漂移;新种子通过 schema 校验;5 个脚本 sh -n
通过;git diff --check 无空白问题。未安装 shellcheck;未执行真实 pi install /
update(属外部写操作)。
This commit is contained in:
@@ -9,7 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Changed
|
||||
|
||||
- Integrate the former standalone AutoReview authorizer into the permission-system composition root; split user-editable ordinary policy and global-only hard guards into independent JSONC files; add sanitized 500-verdict checkpoints and isolated same-session background policy proposals without automatic rule edits.
|
||||
- Make the AutoReview model configuration-only: drop the built-in `openai-codex` / `codex-auto-review` default pair and the synthesized Codex stand-in, so automatic review runs exactly the provider/model named in `policy.jsonc` and defers to the human prompt (with a one-time startup notice) when either is unset, unknown, or unpublished.
|
||||
- Remove AutoReview's 500-verdict count ledger, isolated background analysis AgentSession, and same-session proposal delivery; the reviewer now only decides each ask and writes its verdict to the permission review log.
|
||||
- Integrate the former standalone AutoReview authorizer into the permission-system composition root; split user-editable ordinary policy and global-only hard guards into independent JSONC files.
|
||||
- Simplify ordinary policy to one global `policy.jsonc`: stop loading project policy files, rank matching patterns by specificity instead of declaration order, remove the `external_directory` gate and delegation envelope, let configured authorizers approve any ordinary-policy ask including `path`, and leave all active-tool visibility changes exclusively to Tool Search.
|
||||
|
||||
- Preserve a registered shell-alias input preview as authorizer evidence and permission review context, so wrappers such as `ssh_bash` can disclose their remote execution target while retaining full Bash policy enforcement.
|
||||
|
||||
@@ -20,7 +20,7 @@ Permission enforcement extension for the [Pi](https://pi.mariozechner.at/) codin
|
||||
- **Chooses the most specific matching pattern**, so a specific deny wins over a broad `"*": "allow"` regardless of declaration order
|
||||
- **Applies a separate hard-guard policy first** so session grants, per-agent rules, and model review cannot override high-risk denies
|
||||
- **Controls Bash, paths, MCP, skills, and tool surfaces** while leaving active-tool ownership entirely to Tool Search
|
||||
- **Reviews ordinary `ask` requests automatically** with integrated AutoReview; its `allow` / `deny` verdict is final for ordinary tools and `path`, while `defer` reaches the next authorizer or human terminal
|
||||
- **Reviews ordinary `ask` requests automatically** with integrated AutoReview once you name a review model in config; its `allow` / `deny` verdict is final for ordinary tools and `path`, while `defer` reaches the next authorizer or human terminal
|
||||
- **Forwards subagent `ask` decisions** to the parent authority and keeps structured audit logs
|
||||
|
||||
## Install
|
||||
|
||||
@@ -496,7 +496,9 @@ Nothing constrains an `Authorizer` to be deterministic. `LocalUserAuthorizer` is
|
||||
|
||||
This package now owns the default model reviewer as the named `auto-review` authorizer. It is still downstream of deterministic policy: only an ordinary `ask` enters the authorizer chain, and a hard-guard `deny` never reaches it. The reviewer returns `allow`, `deny`, or `defer`; only `defer` falls through to the terminal human authority. Its transcript boundary admits direct user messages and recognized non-cancelled structured answers, never assistant/tool/compaction content.
|
||||
|
||||
The composition root registers the authorizer on the same `LocalPermissionsService` instance it publishes. `installBuiltinAutoReview()` returns lifecycle methods instead of registering a second set of Pi session handlers. The root's single `session_start` and `session_shutdown` handlers coordinate service publication and reviewer state; `turn_start` resets the denial circuit breaker, and `agent_settled` is the safe boundary for optional count-based analysis.
|
||||
The composition root registers the authorizer on the same `LocalPermissionsService` instance it publishes. `installBuiltinAutoReview()` returns lifecycle methods instead of registering a second set of Pi session handlers. The root's single `session_start` and `session_shutdown` handlers coordinate service publication and reviewer state, and `turn_start` resets the denial circuit breaker.
|
||||
|
||||
The review model is configuration-only. `resolveReviewModel()` reads `autoReview.provider` / `autoReview.model` and looks the pair up in the live registry; it neither defaults the pair nor synthesizes a stand-in. An unconfigured, unknown-provider, or unpublished-model pair resolves to a failure category, which the reviewer records and turns into `defer`, so the ask reaches the human prompt instead of being decided by an unintended model.
|
||||
|
||||
A separate `HardGuardPolicyLoader` supplies a global-only manager to `PermissionResolver`. Resolution is deliberately asymmetric:
|
||||
|
||||
@@ -511,8 +513,6 @@ ordinaryManager.check(intent, session rules)
|
||||
|
||||
Thus hard-file `allow` means “not blocked by hard guards,” not “approved overall.” Ordinary policy, per-agent restrictions, session grants, yolo mode, and model authority cannot loosen a hard deny.
|
||||
|
||||
Completed AutoReview verdicts flow to an owner-only count ledger. The reviewer callback strips rationale/transcript/raw payload and persists bounded fingerprints. Reaching the configured checkpoint marks analysis pending. At `agent_settled`, a tool-free, extension-free, in-memory AgentSession receives only aggregated records. Its strict JSON proposal is validated against the evidence, persisted, and injected into the same session as a follow-up. The checkpoint advances only after a valid proposal is stored; shutdown leaves pending work durable. No analysis path mutates either policy file.
|
||||
|
||||
The public authorizer registry remains available for other named authorities. Configuration order determines chain order, unregistered names are skipped fail-safe, and registration alone grants no authority.
|
||||
|
||||
Authorizers are named capabilities registered on `permissions:ready`; the integrated `auto-review` authorizer uses the same chain contract as third-party links. Once configured, a link's `allow` or `deny` is final for any ordinary-policy `ask`, including `path`; `defer` advances to the next link or terminal.
|
||||
@@ -588,7 +588,7 @@ src/
|
||||
├── authority/
|
||||
│ ├── authorizer-selection.ts Configured authorizer resolution
|
||||
│ └── authorizer-chain.ts Named authorizer ordering and terminal fallback
|
||||
├── auto-review/ Integrated reviewer and count-based analysis
|
||||
├── auto-review/ Integrated reviewer
|
||||
├── service.ts Cross-extension permission service
|
||||
└── types.ts Shared domain types
|
||||
```
|
||||
|
||||
@@ -12,5 +12,5 @@
|
||||
|
||||
- AutoReview 是 `pi-permission-system` 的内置 authorizer,而不是单独加载的 Pi 扩展。
|
||||
- 配置从统一的 `policy.jsonc` 读取。
|
||||
- 完成的自动裁决进入基于计数的 ledger 和一个隔离的、无工具的后台分析 Agent。
|
||||
- 分析建议回传到当前活跃的 Pi Session,且在用户明确确认前绝不修改策略。
|
||||
- 评审模型完全由配置指定:不保留 provider/model 默认常量,也不合成替代模型;未配置或无法解析时 `defer` 到人工审批。
|
||||
- 已移除上游的裁决计数 ledger、后台分析 Agent 与建议回传流程;扩展不再汇总历史裁决,也不自动修改策略。
|
||||
|
||||
@@ -64,17 +64,39 @@ There is no separate `external_directory` decision. A path outside the current w
|
||||
```
|
||||
|
||||
> **Note:** Trailing commas are not supported. If parsing fails, ordinary policy fails safely to `ask`.
|
||||
## Built-in AutoReview and Count-Based Analysis
|
||||
## Built-in AutoReview
|
||||
|
||||
`autoReview.enabled` controls the built-in authorizer named `auto-review`. It only receives requests that ordinary policy resolved to `ask`; it cannot bypass a hard deny. Add `"auto-review"` to `authorizerChain` to make it the primary case-by-case authority. An `allow` or `deny` verdict is final for that request. `defer` continues to the next authorizer and ultimately the human permission dialog.
|
||||
|
||||
The reviewer uses only trusted direct-user transcript evidence and recognized structured answers. Assistant, tool, compaction, and cancelled-answer content cannot authorize an operation. Model lookup, authentication, timeout, malformed output, or internal errors all return `defer`.
|
||||
|
||||
`analysis.threshold` counts completed automatic verdicts, not Pi turns and not deferrals. Each `allow` or `deny` appends one sanitized record under the global extension state directory. Records contain bounded surface/pattern fingerprints and risk/authorization classifications; they omit rationale, raw payload, transcript, and credentials.
|
||||
The review model comes entirely from configuration. There is no default provider/model pair and no synthesized stand-in, so automatic review stays inactive until both `autoReview.provider` and `autoReview.model` name a model your Pi registry actually publishes:
|
||||
|
||||
When the count since the last accepted checkpoint reaches the threshold (500 by default), the ledger marks analysis pending. At the next `agent_settled` boundary the extension starts an isolated AgentSession with no tools, extensions, skills, prompt templates, themes, or project context. Its JSON proposal is schema-validated and filtered so authorization-dependent, denied, or high-risk evidence cannot become an unconditional allow candidate.
|
||||
```jsonc
|
||||
{
|
||||
"authorizerChain": ["auto-review"],
|
||||
"autoReview": {
|
||||
"enabled": true,
|
||||
"provider": "deepseek",
|
||||
"model": "deepseek-flash",
|
||||
"reasoning": "off"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
A successful proposal advances the checkpoint, is retained according to `retainAnalyzedBatches`, and is delivered with `deliverAs: "followUp"` and `triggerTurn: true` to this conversation. The proposal is advisory only. Neither `policy.jsonc` nor `hard-guards.jsonc` is modified until the user explicitly chooses and confirms rules. Shutdown aborts the running worker but leaves the pending checkpoint durable for the next session.
|
||||
| Key | Default | Description |
|
||||
| ---------------------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `autoReview.enabled` | `true` | Registers the `auto-review` authorizer. It decides nothing until named in `authorizerChain`. |
|
||||
| `autoReview.provider` | — | Pi provider id of the review model. No default; when absent the reviewer defers every ask. |
|
||||
| `autoReview.model` | — | Model id within that provider. No default; when absent the reviewer defers every ask. |
|
||||
| `autoReview.reasoning` | — | Thinking level for a reasoning-capable review model. No default; omit to send no reasoning option. |
|
||||
| `autoReview.timeoutMs` | `90000` | Total retry budget for one review call. |
|
||||
| `autoReview.includeBaselinePolicy` | `true` | Include the built-in Codex Guardian–style policy text. |
|
||||
| `autoReview.additionalPolicy` | — | Operator policy appended to the baseline. Required when `includeBaselinePolicy` is `false`. |
|
||||
|
||||
When either `provider` or `model` is missing and `auto-review` is named in `authorizerChain`, the extension notifies once at `session_start` that automatic review cannot run and every ask will reach the human prompt. An unknown provider or an unpublished model id also defers, with the reason recorded in the permission review log.
|
||||
|
||||
Each `allow` / `deny` is written to the permission review log; the extension keeps no aggregate history, runs no background analysis agent, and never edits either policy file.
|
||||
|
||||
## Runtime Knobs
|
||||
|
||||
|
||||
@@ -180,31 +180,6 @@
|
||||
},
|
||||
"additionalProperties": false
|
||||
},
|
||||
"analysis": {
|
||||
"description": "Background policy analysis triggered after a configured number of completed automatic reviews.",
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"enabled": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"threshold": {
|
||||
"type": "integer",
|
||||
"minimum": 1,
|
||||
"maximum": 100000
|
||||
},
|
||||
"retainAnalyzedBatches": {
|
||||
"type": "integer",
|
||||
"minimum": 0,
|
||||
"maximum": 20
|
||||
},
|
||||
"timeoutMs": {
|
||||
"type": "integer",
|
||||
"exclusiveMinimum": 0,
|
||||
"maximum": 300000
|
||||
}
|
||||
},
|
||||
"additionalProperties": false
|
||||
},
|
||||
"shellTools": {
|
||||
"type": "object",
|
||||
"propertyNames": {
|
||||
|
||||
@@ -1,192 +0,0 @@
|
||||
import {
|
||||
createAgentSession,
|
||||
DefaultResourceLoader,
|
||||
SessionManager,
|
||||
SettingsManager,
|
||||
type ModelRegistry,
|
||||
} from "@earendil-works/pi-coding-agent";
|
||||
import { z } from "zod";
|
||||
import type { AutoReviewAnalysisConfig, AutoReviewConfig } from "./config.js";
|
||||
import type { AutoReviewAnalysisBatch, AutoReviewLedgerRecord } from "./ledger.js";
|
||||
import { resolveReviewModel } from "./model.js";
|
||||
|
||||
const candidateSchema = z.strictObject({
|
||||
surface: z.string().min(1).max(100),
|
||||
toolName: z.string().min(1).max(100).optional(),
|
||||
pattern: z.string().min(1).max(240),
|
||||
reason: z.string().min(1).max(1_000),
|
||||
evidenceCount: z.number().int().positive(),
|
||||
});
|
||||
|
||||
const proposalSchema = z.strictObject({
|
||||
summary: z.string().min(1).max(4_000),
|
||||
allowCandidates: z.array(candidateSchema).max(50),
|
||||
denyCandidates: z.array(candidateSchema).max(50),
|
||||
keepReview: z.array(candidateSchema).max(100),
|
||||
hardGuardCandidates: z.array(candidateSchema).max(25),
|
||||
});
|
||||
|
||||
export type AutoReviewAnalysisProposal = z.infer<typeof proposalSchema>;
|
||||
|
||||
interface Aggregate {
|
||||
surface: string;
|
||||
toolName?: string;
|
||||
pattern: string;
|
||||
total: number;
|
||||
allow: number;
|
||||
deny: number;
|
||||
authorizationDependent: number;
|
||||
low: number;
|
||||
medium: number;
|
||||
high: number;
|
||||
critical: number;
|
||||
}
|
||||
|
||||
function aggregateRecords(records: AutoReviewLedgerRecord[]): Aggregate[] {
|
||||
const groups = new Map<string, Aggregate>();
|
||||
for (const record of records) {
|
||||
const key = record.fingerprint;
|
||||
const current = groups.get(key) ?? {
|
||||
surface: record.surface,
|
||||
...(record.toolName ? { toolName: record.toolName } : {}),
|
||||
pattern: record.normalizedPattern,
|
||||
total: 0,
|
||||
allow: 0,
|
||||
deny: 0,
|
||||
authorizationDependent: 0,
|
||||
low: 0,
|
||||
medium: 0,
|
||||
high: 0,
|
||||
critical: 0,
|
||||
};
|
||||
current.total += 1;
|
||||
current[record.outcome] += 1;
|
||||
current[record.riskLevel] += 1;
|
||||
if (record.authorizationDependent) current.authorizationDependent += 1;
|
||||
groups.set(key, current);
|
||||
}
|
||||
return [...groups.values()]
|
||||
.sort((left, right) => right.total - left.total)
|
||||
.slice(0, 250);
|
||||
}
|
||||
|
||||
function parseJsonObject(text: string): unknown {
|
||||
try { return JSON.parse(text); }
|
||||
catch {
|
||||
const start = text.indexOf("{");
|
||||
const end = text.lastIndexOf("}");
|
||||
if (start < 0 || end <= start) throw new Error("analysis response was not JSON");
|
||||
return JSON.parse(text.slice(start, end + 1));
|
||||
}
|
||||
}
|
||||
|
||||
function assistantText(messages: readonly unknown[]): string {
|
||||
for (let index = messages.length - 1; index >= 0; index -= 1) {
|
||||
const message = messages[index] as { role?: string; content?: unknown };
|
||||
if (message?.role !== "assistant") continue;
|
||||
if (typeof message.content === "string") return message.content;
|
||||
if (Array.isArray(message.content)) {
|
||||
return message.content
|
||||
.flatMap((block) => {
|
||||
if (typeof block === "string") return [block];
|
||||
if (typeof block === "object" && block !== null && "text" in block) {
|
||||
const text = (block as { text?: unknown }).text;
|
||||
return typeof text === "string" ? [text] : [];
|
||||
}
|
||||
return [];
|
||||
})
|
||||
.join("\n");
|
||||
}
|
||||
}
|
||||
throw new Error("analysis agent returned no assistant text");
|
||||
}
|
||||
|
||||
function keyOf(value: { surface: string; toolName?: string; pattern: string }): string {
|
||||
return `${value.surface}\u0000${value.toolName ?? ""}\u0000${value.pattern}`;
|
||||
}
|
||||
|
||||
function validateProposal(
|
||||
proposal: AutoReviewAnalysisProposal,
|
||||
aggregates: Aggregate[],
|
||||
): AutoReviewAnalysisProposal {
|
||||
const evidence = new Map(aggregates.map((item) => [keyOf(item), item]));
|
||||
const validateList = (
|
||||
values: AutoReviewAnalysisProposal["allowCandidates"],
|
||||
kind: "allow" | "deny" | "review" | "hard",
|
||||
) => values.filter((candidate) => {
|
||||
const item = evidence.get(keyOf(candidate));
|
||||
if (!item || candidate.evidenceCount > item.total) return false;
|
||||
if (kind === "allow") {
|
||||
return item.deny === 0 && item.authorizationDependent === 0 && item.high === 0 && item.critical === 0;
|
||||
}
|
||||
if (kind === "deny") return item.allow === 0;
|
||||
if (kind === "hard") return item.allow === 0 && item.high + item.critical > 0;
|
||||
return true;
|
||||
});
|
||||
return {
|
||||
...proposal,
|
||||
allowCandidates: validateList(proposal.allowCandidates, "allow"),
|
||||
denyCandidates: validateList(proposal.denyCandidates, "deny"),
|
||||
keepReview: validateList(proposal.keepReview, "review"),
|
||||
hardGuardCandidates: validateList(proposal.hardGuardCandidates, "hard"),
|
||||
};
|
||||
}
|
||||
|
||||
export async function runAutoReviewAnalysisAgent(options: {
|
||||
batch: AutoReviewAnalysisBatch;
|
||||
registry: Pick<ModelRegistry, "find" | "getAll" | "getApiKeyAndHeaders" | "getProvider">;
|
||||
autoReview: AutoReviewConfig;
|
||||
analysis: AutoReviewAnalysisConfig;
|
||||
stateDir: string;
|
||||
agentDir: string;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<AutoReviewAnalysisProposal> {
|
||||
const resolved = resolveReviewModel(options.registry, options.autoReview);
|
||||
if (!resolved.ok) throw new Error(`analysis model unavailable: ${resolved.category}`);
|
||||
const aggregates = aggregateRecords(options.batch.records);
|
||||
const systemPrompt = `You are a read-only permission-policy analyst. Analyze aggregated automatic-review decisions and return one JSON object only. Never suggest an unconditional allow when authorizationDependent is non-zero, any denial exists, or high/critical risk exists. Do not modify files or execute tools. Candidate keys must exactly copy surface, toolName (when present), and pattern from the evidence. The schema is: {summary:string,allowCandidates:Candidate[],denyCandidates:Candidate[],keepReview:Candidate[],hardGuardCandidates:Candidate[]}; Candidate is {surface:string,toolName?:string,pattern:string,reason:string,evidenceCount:integer}.`;
|
||||
|
||||
const loader = new DefaultResourceLoader({
|
||||
cwd: options.stateDir,
|
||||
agentDir: options.agentDir,
|
||||
noExtensions: true,
|
||||
noSkills: true,
|
||||
noPromptTemplates: true,
|
||||
noThemes: true,
|
||||
noContextFiles: true,
|
||||
systemPrompt,
|
||||
appendSystemPrompt: [],
|
||||
});
|
||||
await loader.reload();
|
||||
|
||||
const { session } = await createAgentSession({
|
||||
cwd: options.stateDir,
|
||||
agentDir: options.agentDir,
|
||||
model: resolved.value.model,
|
||||
thinkingLevel: options.autoReview.reasoning,
|
||||
noTools: "all",
|
||||
resourceLoader: loader,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settingsManager: SettingsManager.inMemory(),
|
||||
});
|
||||
|
||||
const timeout = setTimeout(() => session.abort(), options.analysis.timeoutMs);
|
||||
const abort = () => session.abort();
|
||||
options.signal?.addEventListener("abort", abort, { once: true });
|
||||
try {
|
||||
await session.prompt(JSON.stringify({
|
||||
batch: {
|
||||
fromSequence: options.batch.fromSequence,
|
||||
toSequence: options.batch.toSequence,
|
||||
policyHash: options.batch.policyHash,
|
||||
},
|
||||
aggregates,
|
||||
}));
|
||||
const parsed = proposalSchema.parse(parseJsonObject(assistantText(session.state.messages)));
|
||||
return validateProposal(parsed, aggregates);
|
||||
} finally {
|
||||
clearTimeout(timeout);
|
||||
options.signal?.removeEventListener("abort", abort);
|
||||
session.dispose();
|
||||
}
|
||||
}
|
||||
@@ -1,10 +1,7 @@
|
||||
import { z } from "zod";
|
||||
|
||||
export const DEFAULT_PROVIDER = "openai-codex";
|
||||
export const DEFAULT_MODEL = "codex-auto-review";
|
||||
/** Total retry budget for one review call. */
|
||||
export const DEFAULT_TIMEOUT_MS = 90_000;
|
||||
export const DEFAULT_ANALYSIS_THRESHOLD = 500;
|
||||
export const DEFAULT_RETAIN_ANALYZED_BATCHES = 2;
|
||||
|
||||
export const REASONING_LEVELS = [
|
||||
"off",
|
||||
@@ -36,49 +33,31 @@ export const autoReviewConfigFileSchema = z
|
||||
}
|
||||
});
|
||||
|
||||
export const analysisConfigFileSchema = z.strictObject({
|
||||
enabled: z.boolean().optional(),
|
||||
threshold: z.number().int().min(1).max(100_000).optional(),
|
||||
retainAnalyzedBatches: z.number().int().min(0).max(20).optional(),
|
||||
timeoutMs: z.number().int().positive().max(300_000).optional(),
|
||||
});
|
||||
|
||||
export type AutoReviewConfigFile = z.infer<typeof autoReviewConfigFileSchema>;
|
||||
export type AutoReviewAnalysisConfigFile = z.infer<typeof analysisConfigFileSchema>;
|
||||
|
||||
/**
|
||||
* Effective reviewer configuration.
|
||||
*
|
||||
* `provider` and `model` have no built-in default and no synthesized fallback:
|
||||
* the review model is whatever the operator names in the config file. When
|
||||
* either is absent the reviewer cannot run and defers to the human prompt.
|
||||
*/
|
||||
export interface AutoReviewConfig {
|
||||
enabled: boolean;
|
||||
provider: string;
|
||||
model: string;
|
||||
reasoning: (typeof REASONING_LEVELS)[number];
|
||||
provider?: string;
|
||||
model?: string;
|
||||
reasoning?: (typeof REASONING_LEVELS)[number];
|
||||
timeoutMs: number;
|
||||
includeBaselinePolicy: boolean;
|
||||
additionalPolicy?: string;
|
||||
}
|
||||
|
||||
export interface AutoReviewAnalysisConfig {
|
||||
enabled: boolean;
|
||||
threshold: number;
|
||||
retainAnalyzedBatches: number;
|
||||
timeoutMs: number;
|
||||
}
|
||||
|
||||
export const DEFAULT_AUTO_REVIEW_CONFIG: AutoReviewConfig = {
|
||||
enabled: true,
|
||||
provider: DEFAULT_PROVIDER,
|
||||
model: DEFAULT_MODEL,
|
||||
reasoning: "low",
|
||||
timeoutMs: DEFAULT_TIMEOUT_MS,
|
||||
includeBaselinePolicy: true,
|
||||
};
|
||||
|
||||
export const DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG: AutoReviewAnalysisConfig = {
|
||||
enabled: true,
|
||||
threshold: DEFAULT_ANALYSIS_THRESHOLD,
|
||||
retainAnalyzedBatches: DEFAULT_RETAIN_ANALYZED_BATCHES,
|
||||
timeoutMs: DEFAULT_TIMEOUT_MS,
|
||||
};
|
||||
|
||||
export function normalizeAutoReviewConfig(
|
||||
value?: AutoReviewConfigFile,
|
||||
): AutoReviewConfig {
|
||||
@@ -87,12 +66,3 @@ export function normalizeAutoReviewConfig(
|
||||
...value,
|
||||
};
|
||||
}
|
||||
|
||||
export function normalizeAutoReviewAnalysisConfig(
|
||||
value?: AutoReviewAnalysisConfigFile,
|
||||
): AutoReviewAnalysisConfig {
|
||||
return {
|
||||
...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG,
|
||||
...value,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,296 +0,0 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import {
|
||||
appendFileSync,
|
||||
chmodSync,
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
readFileSync,
|
||||
renameSync,
|
||||
unlinkSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import type { PromptPermissionDetails } from "../service.js";
|
||||
import type { AutoReviewAnalysisConfig, AutoReviewConfig } from "./config.js";
|
||||
import type { ReviewAssessment } from "./verdict.js";
|
||||
|
||||
const DIRECTORY_MODE = 0o700;
|
||||
const FILE_MODE = 0o600;
|
||||
const MAX_PATTERN_LENGTH = 240;
|
||||
|
||||
interface LedgerState {
|
||||
version: 1;
|
||||
completedSequence: number;
|
||||
lastAnalyzedSequence: number;
|
||||
proposalSequence: number;
|
||||
analysisPending: boolean;
|
||||
}
|
||||
|
||||
export interface AutoReviewLedgerRecord {
|
||||
version: 1;
|
||||
sequence: number;
|
||||
timestamp: string;
|
||||
requestId: string;
|
||||
source: string;
|
||||
agentName: string;
|
||||
surface: string;
|
||||
toolName?: string;
|
||||
normalizedPattern: string;
|
||||
fingerprint: string;
|
||||
outcome: "allow" | "deny";
|
||||
riskLevel: ReviewAssessment["riskLevel"];
|
||||
userAuthorization: ReviewAssessment["userAuthorization"];
|
||||
authorizationDependent: boolean;
|
||||
policyRevision: string;
|
||||
policyHash: string;
|
||||
provider: string;
|
||||
model: string;
|
||||
}
|
||||
|
||||
export interface AutoReviewAnalysisBatch {
|
||||
fromSequence: number;
|
||||
toSequence: number;
|
||||
records: AutoReviewLedgerRecord[];
|
||||
policyHash: string;
|
||||
}
|
||||
|
||||
export interface StoredAnalysisProposal {
|
||||
version: 1;
|
||||
proposalId: string;
|
||||
createdAt: string;
|
||||
fromSequence: number;
|
||||
toSequence: number;
|
||||
policyHash: string;
|
||||
proposal: unknown;
|
||||
}
|
||||
|
||||
function initialState(): LedgerState {
|
||||
return {
|
||||
version: 1,
|
||||
completedSequence: 0,
|
||||
lastAnalyzedSequence: 0,
|
||||
proposalSequence: 0,
|
||||
analysisPending: false,
|
||||
};
|
||||
}
|
||||
|
||||
function hash(value: unknown): string {
|
||||
return createHash("sha256").update(JSON.stringify(value)).digest("hex");
|
||||
}
|
||||
|
||||
function bounded(value: string): string {
|
||||
return value.length <= MAX_PATTERN_LENGTH
|
||||
? value
|
||||
: `${value.slice(0, MAX_PATTERN_LENGTH - 1)}…`;
|
||||
}
|
||||
|
||||
const SAFE_SUBCOMMAND_COMMANDS = new Set([
|
||||
"bun", "cargo", "docker", "git", "go", "kubectl", "npm", "pnpm",
|
||||
"sudo", "systemctl", "yarn",
|
||||
]);
|
||||
const SAFE_PATH_PREFIXES = new Set([
|
||||
"app", "apps", "config", "docs", "etc", "lib", "opt", "packages",
|
||||
"private", "src", "test", "tests", "tmp", "Users", "var", "workspace",
|
||||
"workspaces",
|
||||
]);
|
||||
|
||||
function normalizedExecutable(value: string): string {
|
||||
const executable = value.replace(/\\/g, "/").split("/").filter(Boolean).at(-1) ?? "";
|
||||
return /^[A-Za-z0-9._+-]{1,64}$/u.test(executable) ? executable : "<command>";
|
||||
}
|
||||
|
||||
function normalizeCommand(command: string): string {
|
||||
const words = command.trim().replace(/\s+/g, " ").split(" ").filter(Boolean);
|
||||
if (words.length === 0) return "<empty-command>";
|
||||
const executable = normalizedExecutable(words[0]!);
|
||||
const subcommand = words[1];
|
||||
if (
|
||||
subcommand !== undefined &&
|
||||
SAFE_SUBCOMMAND_COMMANDS.has(executable) &&
|
||||
/^[A-Za-z0-9._+-]{1,64}$/u.test(subcommand)
|
||||
) {
|
||||
return bounded(`${executable} ${subcommand} *`);
|
||||
}
|
||||
return bounded(words.length === 1 ? executable : `${executable} *`);
|
||||
}
|
||||
|
||||
function normalizePath(value: string): string {
|
||||
const normalized = value.trim().replace(/\\/g, "/");
|
||||
if (normalized.length === 0) return "<empty-path>";
|
||||
if (normalized === "~" || normalized.startsWith("~/")) return "~/*";
|
||||
const absolute = normalized.startsWith("/");
|
||||
const segments = normalized.split("/").filter(Boolean);
|
||||
const first = segments[0];
|
||||
if (first && SAFE_PATH_PREFIXES.has(first)) {
|
||||
return bounded(`${absolute ? "/" : ""}${first}/*`);
|
||||
}
|
||||
if (!absolute && segments.length === 1) {
|
||||
const extension = normalized.match(/\.[A-Za-z0-9]{1,10}$/u)?.[0];
|
||||
if (extension) return `*${extension.toLowerCase()}`;
|
||||
}
|
||||
return absolute ? "/<path>/*" : "<relative>/*";
|
||||
}
|
||||
|
||||
function derivePattern(details: PromptPermissionDetails): string {
|
||||
if (typeof details.command === "string") return normalizeCommand(details.command);
|
||||
if (typeof details.path === "string") return normalizePath(details.path);
|
||||
const values = details.accessIntent?.matchValues;
|
||||
if (Array.isArray(values) && typeof values[0] === "string") return normalizePath(values[0]);
|
||||
if (details.toolName) return bounded(details.toolName);
|
||||
return bounded(details.surface ?? "unknown");
|
||||
}
|
||||
|
||||
function readJson<T>(path: string): T | undefined {
|
||||
if (!existsSync(path)) return undefined;
|
||||
try {
|
||||
return JSON.parse(readFileSync(path, "utf8")) as T;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
export class AutoReviewLedger {
|
||||
private readonly ledgerDir: string;
|
||||
private readonly proposalsDir: string;
|
||||
private readonly statePath: string;
|
||||
private readonly decisionsPath: string;
|
||||
private state: LedgerState;
|
||||
|
||||
constructor(rootDir: string) {
|
||||
this.ledgerDir = join(rootDir, "ledger");
|
||||
this.proposalsDir = join(rootDir, "proposals");
|
||||
this.statePath = join(rootDir, "state.json");
|
||||
this.decisionsPath = join(this.ledgerDir, "decisions.jsonl");
|
||||
for (const dir of [rootDir, this.ledgerDir, this.proposalsDir]) {
|
||||
mkdirSync(dir, { recursive: true, mode: DIRECTORY_MODE });
|
||||
try { chmodSync(dir, DIRECTORY_MODE); } catch { /* best effort on non-POSIX */ }
|
||||
}
|
||||
this.state = readJson<LedgerState>(this.statePath) ?? initialState();
|
||||
this.persistState();
|
||||
}
|
||||
|
||||
recordDecision(event: {
|
||||
details: PromptPermissionDetails;
|
||||
assessment: ReviewAssessment;
|
||||
policyRevision: string;
|
||||
config: AutoReviewConfig;
|
||||
analysis: AutoReviewAnalysisConfig;
|
||||
}): AutoReviewLedgerRecord {
|
||||
const sequence = this.state.completedSequence + 1;
|
||||
const pattern = derivePattern(event.details);
|
||||
const surface = bounded(event.details.accessIntent?.surface ?? event.details.surface ?? "unknown");
|
||||
const toolName = event.details.toolName ? bounded(event.details.toolName) : undefined;
|
||||
const record: AutoReviewLedgerRecord = {
|
||||
version: 1,
|
||||
sequence,
|
||||
timestamp: new Date().toISOString(),
|
||||
requestId: bounded(event.details.requestId),
|
||||
source: bounded(event.details.source),
|
||||
agentName: bounded(event.details.agentName ?? "main"),
|
||||
surface,
|
||||
...(toolName ? { toolName } : {}),
|
||||
normalizedPattern: pattern,
|
||||
fingerprint: hash({ surface, toolName, pattern }),
|
||||
outcome: event.assessment.outcome,
|
||||
riskLevel: event.assessment.riskLevel,
|
||||
userAuthorization: event.assessment.userAuthorization,
|
||||
authorizationDependent: event.assessment.userAuthorization !== "unknown",
|
||||
policyRevision: bounded(event.policyRevision),
|
||||
policyHash: hash(event.config),
|
||||
provider: bounded(event.config.provider),
|
||||
model: bounded(event.config.model),
|
||||
};
|
||||
appendFileSync(this.decisionsPath, `${JSON.stringify(record)}\n`, {
|
||||
encoding: "utf8",
|
||||
mode: FILE_MODE,
|
||||
});
|
||||
try { chmodSync(this.decisionsPath, FILE_MODE); } catch { /* best effort */ }
|
||||
|
||||
this.state.completedSequence = sequence;
|
||||
if (
|
||||
event.analysis.enabled &&
|
||||
sequence - this.state.lastAnalyzedSequence >= event.analysis.threshold
|
||||
) {
|
||||
this.state.analysisPending = true;
|
||||
}
|
||||
this.persistState();
|
||||
return record;
|
||||
}
|
||||
|
||||
hasPendingAnalysis(): boolean {
|
||||
return this.state.analysisPending;
|
||||
}
|
||||
|
||||
prepareBatch(config: AutoReviewAnalysisConfig): AutoReviewAnalysisBatch | undefined {
|
||||
const records = this.readRecords();
|
||||
const fromSequence = this.state.lastAnalyzedSequence + 1;
|
||||
const eligible = records.filter((record) => record.sequence >= fromSequence);
|
||||
if (eligible.length < config.threshold) return undefined;
|
||||
const selected = eligible.slice(0, config.threshold);
|
||||
const policyHash = selected[0]?.policyHash ?? "";
|
||||
return {
|
||||
fromSequence,
|
||||
toSequence: selected.at(-1)!.sequence,
|
||||
records: selected,
|
||||
policyHash,
|
||||
};
|
||||
}
|
||||
|
||||
saveProposal(batch: AutoReviewAnalysisBatch, proposal: unknown): StoredAnalysisProposal {
|
||||
const next = this.state.proposalSequence + 1;
|
||||
const proposalId = `proposal-${String(next).padStart(6, "0")}`;
|
||||
const stored: StoredAnalysisProposal = {
|
||||
version: 1,
|
||||
proposalId,
|
||||
createdAt: new Date().toISOString(),
|
||||
fromSequence: batch.fromSequence,
|
||||
toSequence: batch.toSequence,
|
||||
policyHash: batch.policyHash,
|
||||
proposal,
|
||||
};
|
||||
this.writeAtomic(join(this.proposalsDir, `${proposalId}.json`), stored);
|
||||
this.state.proposalSequence = next;
|
||||
this.state.lastAnalyzedSequence = batch.toSequence;
|
||||
this.state.analysisPending =
|
||||
this.state.completedSequence - this.state.lastAnalyzedSequence > 0;
|
||||
this.persistState();
|
||||
return stored;
|
||||
}
|
||||
|
||||
compact(config: AutoReviewAnalysisConfig): void {
|
||||
const keep = Math.max(config.threshold, config.threshold * config.retainAnalyzedBatches);
|
||||
const records = this.readRecords();
|
||||
const retained = records.slice(-keep);
|
||||
const content = retained.map((record) => JSON.stringify(record)).join("\n");
|
||||
this.writeAtomic(this.decisionsPath, content ? `${content}\n` : "");
|
||||
}
|
||||
|
||||
private readRecords(): AutoReviewLedgerRecord[] {
|
||||
if (!existsSync(this.decisionsPath)) return [];
|
||||
return readFileSync(this.decisionsPath, "utf8")
|
||||
.split("\n")
|
||||
.filter(Boolean)
|
||||
.flatMap((line) => {
|
||||
try { return [JSON.parse(line) as AutoReviewLedgerRecord]; }
|
||||
catch { return []; }
|
||||
})
|
||||
.sort((left, right) => left.sequence - right.sequence);
|
||||
}
|
||||
|
||||
private persistState(): void {
|
||||
this.writeAtomic(this.statePath, this.state);
|
||||
}
|
||||
|
||||
private writeAtomic(path: string, value: unknown): void {
|
||||
const temporaryPath = `${path}.${process.pid}.${Date.now()}.tmp`;
|
||||
const content = typeof value === "string" ? value : `${JSON.stringify(value, null, 2)}\n`;
|
||||
writeFileSync(temporaryPath, content, { encoding: "utf8", mode: FILE_MODE, flag: "wx" });
|
||||
try {
|
||||
renameSync(temporaryPath, path);
|
||||
try { chmodSync(path, FILE_MODE); } catch { /* best effort */ }
|
||||
} catch (error) {
|
||||
try { unlinkSync(temporaryPath); } catch { /* best effort */ }
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,65 +1,48 @@
|
||||
import type { AutoReviewConfig } from './config.js'
|
||||
import type { Api, Model, Provider } from '@earendil-works/pi-ai'
|
||||
import type { ModelRegistry } from '@earendil-works/pi-coding-agent'
|
||||
import { DEFAULT_MODEL, DEFAULT_PROVIDER } from './config.js'
|
||||
|
||||
export type ReviewModelRegistry = Pick<ModelRegistry, 'find' | 'getAll' | 'getApiKeyAndHeaders' | 'getProvider'>
|
||||
|
||||
interface ResolvedReviewModel {
|
||||
export interface ResolvedReviewModel {
|
||||
model: Model<Api>
|
||||
provider: Provider<Api>
|
||||
synthesized: boolean
|
||||
}
|
||||
|
||||
export type ResolveReviewModelFailure =
|
||||
| 'model-unconfigured'
|
||||
| 'provider-unresolved'
|
||||
| 'model-unresolved'
|
||||
|
||||
export type ResolveReviewModelResult =
|
||||
| { ok: true; value: ResolvedReviewModel }
|
||||
| {
|
||||
ok: false
|
||||
category: 'provider-unresolved' | 'model-unresolved'
|
||||
}
|
||||
|
||||
function findCodexTemplate(registry: ReviewModelRegistry, provider: Provider<Api>): Model<Api> | undefined {
|
||||
return (
|
||||
registry.getAll().find(model => model.provider === DEFAULT_PROVIDER && model.api === 'openai-codex-responses') ??
|
||||
provider.getModels().find(model => model.api === 'openai-codex-responses')
|
||||
)
|
||||
}
|
||||
| { ok: false; category: ResolveReviewModelFailure }
|
||||
|
||||
/**
|
||||
* Resolve the reviewer model from operator configuration alone.
|
||||
*
|
||||
* There is no built-in provider/model pair and no synthesized stand-in: the
|
||||
* reviewer runs exactly the model named in `autoReview.provider` /
|
||||
* `autoReview.model`. A configuration that names neither, names an unknown
|
||||
* provider, or names a model the provider does not publish resolves to a
|
||||
* failure, and the reviewer defers the ask to the human prompt.
|
||||
*/
|
||||
export function resolveReviewModel(registry: ReviewModelRegistry, config: AutoReviewConfig): ResolveReviewModelResult {
|
||||
const provider = registry.getProvider(config.provider)
|
||||
const providerId = config.provider
|
||||
const modelId = config.model
|
||||
if (providerId === undefined || modelId === undefined) {
|
||||
return { ok: false, category: 'model-unconfigured' }
|
||||
}
|
||||
|
||||
const provider = registry.getProvider(providerId)
|
||||
if (provider === undefined) {
|
||||
return { ok: false, category: 'provider-unresolved' }
|
||||
}
|
||||
|
||||
const registeredModel = registry.find(config.provider, config.model)
|
||||
if (registeredModel !== undefined) {
|
||||
return {
|
||||
ok: true,
|
||||
value: { model: registeredModel, provider, synthesized: false },
|
||||
}
|
||||
}
|
||||
|
||||
if (config.provider !== DEFAULT_PROVIDER || config.model !== DEFAULT_MODEL) {
|
||||
const model = registry.find(providerId, modelId)
|
||||
if (model === undefined) {
|
||||
return { ok: false, category: 'model-unresolved' }
|
||||
}
|
||||
|
||||
const template = findCodexTemplate(registry, provider)
|
||||
if (template === undefined) {
|
||||
return { ok: false, category: 'model-unresolved' }
|
||||
}
|
||||
|
||||
return {
|
||||
ok: true,
|
||||
value: {
|
||||
model: {
|
||||
...template,
|
||||
id: DEFAULT_MODEL,
|
||||
name: 'Codex Auto Review',
|
||||
reasoning: true,
|
||||
input: ['text'],
|
||||
},
|
||||
provider,
|
||||
synthesized: true,
|
||||
},
|
||||
}
|
||||
return { ok: true, value: { model, provider } }
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ const FAILURE_EVENT = 'auto_review.failure'
|
||||
const CIRCUIT_OPEN_EVENT = 'auto_review.circuit_open'
|
||||
|
||||
type FailureCategory =
|
||||
| 'model-unconfigured'
|
||||
| 'provider-unresolved'
|
||||
| 'model-unresolved'
|
||||
| 'auth-unresolved'
|
||||
@@ -35,12 +36,6 @@ export interface ReviewerRuntime {
|
||||
sessionManager: Pick<SessionManager, 'getBranch'>
|
||||
circuitBreaker: DenialCircuitBreaker
|
||||
sessionSignal?: AbortSignal
|
||||
onDecision?: (event: {
|
||||
details: PromptPermissionDetails
|
||||
assessment: ReviewAssessment
|
||||
durationMs: number
|
||||
policyRevision: string
|
||||
}) => void
|
||||
}
|
||||
|
||||
export interface ReviewerDependencies {
|
||||
@@ -154,7 +149,7 @@ function buildStreamOptions(
|
||||
if (auth.env !== undefined) {
|
||||
options.env = auth.env
|
||||
}
|
||||
if (reasoning && runtime.config.reasoning !== 'off') {
|
||||
if (reasoning && runtime.config.reasoning !== undefined && runtime.config.reasoning !== 'off') {
|
||||
options.reasoning = runtime.config.reasoning
|
||||
}
|
||||
return options
|
||||
@@ -358,16 +353,6 @@ export function createPermissionReviewer(
|
||||
durationMs,
|
||||
...contextDiagnostics,
|
||||
})
|
||||
try {
|
||||
runtime.onDecision?.({
|
||||
details,
|
||||
assessment,
|
||||
durationMs,
|
||||
policyRevision: POLICY_REVISION,
|
||||
})
|
||||
} catch {
|
||||
// Audit/analysis persistence must never change the permission verdict.
|
||||
}
|
||||
|
||||
if (assessment.outcome === 'allow') {
|
||||
runtime.circuitBreaker.recordNonDenial()
|
||||
|
||||
@@ -1,23 +1,15 @@
|
||||
import type {
|
||||
ExtensionAPI,
|
||||
ExtensionContext,
|
||||
ModelRegistry,
|
||||
SessionManager,
|
||||
} from "@earendil-works/pi-coding-agent";
|
||||
import type { ConfigReader } from "../config-store";
|
||||
import { getAnalysisStateDir } from "../config-paths";
|
||||
import type { PermissionsService } from "../service";
|
||||
import type { ReviewLogger } from "../session-logger";
|
||||
import { runAutoReviewAnalysisAgent } from "./analysis-agent.js";
|
||||
import {
|
||||
DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG,
|
||||
DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
} from "./config.js";
|
||||
import { DEFAULT_AUTO_REVIEW_CONFIG } from "./config.js";
|
||||
import { DenialCircuitBreaker } from "./circuit-breaker.js";
|
||||
import { AutoReviewLedger } from "./ledger.js";
|
||||
import { createPermissionReviewer } from "./reviewer.js";
|
||||
|
||||
const AUTHORIZER_NAME = "auto-review";
|
||||
const ANALYSIS_RETRY_BACKOFF_MS = 60_000;
|
||||
|
||||
interface SessionRuntime {
|
||||
registry: ModelRegistry;
|
||||
@@ -26,40 +18,38 @@ interface SessionRuntime {
|
||||
}
|
||||
|
||||
export interface AutoReviewLifecycleController {
|
||||
handleSessionStart(context: {
|
||||
modelRegistry: ModelRegistry;
|
||||
sessionManager: Pick<SessionManager, "getBranch">;
|
||||
}): void;
|
||||
handleSessionStart(ctx: ExtensionContext): void;
|
||||
handleTurnStart(): void;
|
||||
handleAgentSettled(): Promise<void>;
|
||||
handleSessionShutdown(): void;
|
||||
}
|
||||
|
||||
/** Warn once per session when AutoReview is active but names no usable model. */
|
||||
function warnWhenUnconfigured(config: ConfigReader, ctx: ExtensionContext): void {
|
||||
const active = config.current();
|
||||
const autoReview = active.autoReview ?? DEFAULT_AUTO_REVIEW_CONFIG;
|
||||
if (!autoReview.enabled) return;
|
||||
if (!(active.authorizerChain ?? []).includes(AUTHORIZER_NAME)) return;
|
||||
if (autoReview.provider !== undefined && autoReview.model !== undefined) return;
|
||||
if (!ctx.hasUI) return;
|
||||
ctx.ui.notify(
|
||||
"AutoReview is enabled but autoReview.provider / autoReview.model are not both set, so automatic review cannot run. Every ask will go to the human permission prompt. Set both in the global policy.jsonc to enable automatic review.",
|
||||
"warning",
|
||||
);
|
||||
}
|
||||
|
||||
export function installBuiltinAutoReview(options: {
|
||||
pi: ExtensionAPI;
|
||||
service: PermissionsService;
|
||||
config: ConfigReader;
|
||||
logger: ReviewLogger;
|
||||
agentDir: string;
|
||||
/** Test seam; production always uses the isolated AgentSession implementation. */
|
||||
runAnalysis?: typeof runAutoReviewAnalysisAgent;
|
||||
}): AutoReviewLifecycleController {
|
||||
const { pi, service, config, logger, agentDir } = options;
|
||||
const analyze = options.runAnalysis ?? runAutoReviewAnalysisAgent;
|
||||
const { service, config } = options;
|
||||
const circuitBreaker = new DenialCircuitBreaker();
|
||||
const stateDir = getAnalysisStateDir(agentDir);
|
||||
const ledger = new AutoReviewLedger(stateDir);
|
||||
let runtime: SessionRuntime | undefined;
|
||||
let analysisRunning = false;
|
||||
let analysisGeneration = 0;
|
||||
let nextAnalysisAttemptAt = 0;
|
||||
|
||||
service.registerAuthorizer(
|
||||
AUTHORIZER_NAME,
|
||||
async (details, query, log) => {
|
||||
const active = config.current();
|
||||
const autoReview = active.autoReview ?? DEFAULT_AUTO_REVIEW_CONFIG;
|
||||
const analysis = active.analysis ?? DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG;
|
||||
if (!autoReview.enabled || runtime === undefined) return { kind: "defer" };
|
||||
const reviewer = createPermissionReviewer({
|
||||
config: autoReview,
|
||||
@@ -67,110 +57,28 @@ export function installBuiltinAutoReview(options: {
|
||||
sessionManager: runtime.sessionManager,
|
||||
circuitBreaker,
|
||||
sessionSignal: runtime.controller.signal,
|
||||
onDecision: (event) => {
|
||||
ledger.recordDecision({
|
||||
details: event.details,
|
||||
assessment: event.assessment,
|
||||
policyRevision: event.policyRevision,
|
||||
config: autoReview,
|
||||
analysis,
|
||||
});
|
||||
},
|
||||
});
|
||||
return reviewer(details, query, log);
|
||||
},
|
||||
);
|
||||
|
||||
const runPendingAnalysis = async (): Promise<void> => {
|
||||
if (analysisRunning || runtime === undefined || !ledger.hasPendingAnalysis()) return;
|
||||
if (Date.now() < nextAnalysisAttemptAt) return;
|
||||
const active = config.current();
|
||||
const autoReview = active.autoReview ?? DEFAULT_AUTO_REVIEW_CONFIG;
|
||||
const analysis = active.analysis ?? DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG;
|
||||
if (!analysis.enabled) return;
|
||||
|
||||
const batch = ledger.prepareBatch(analysis);
|
||||
if (!batch) return;
|
||||
const activeRuntime = runtime;
|
||||
const generation = ++analysisGeneration;
|
||||
const isCurrent = (): boolean =>
|
||||
runtime === activeRuntime &&
|
||||
analysisGeneration === generation &&
|
||||
!activeRuntime.controller.signal.aborted;
|
||||
analysisRunning = true;
|
||||
try {
|
||||
const proposal = await analyze({
|
||||
batch,
|
||||
registry: activeRuntime.registry,
|
||||
autoReview,
|
||||
analysis,
|
||||
stateDir,
|
||||
agentDir,
|
||||
signal: activeRuntime.controller.signal,
|
||||
});
|
||||
if (!isCurrent()) return;
|
||||
pi.sendMessage(
|
||||
{
|
||||
customType: "permission-policy-analysis-ready",
|
||||
content: [
|
||||
`Permission policy analysis is ready for automatic review decisions ${batch.fromSequence}-${batch.toSequence}.`,
|
||||
"Present these suggestions to the user in this conversation. Do not edit either policy file until the user explicitly selects and confirms rules.",
|
||||
JSON.stringify(proposal),
|
||||
].join("\n\n"),
|
||||
display: true,
|
||||
details: {
|
||||
fromSequence: batch.fromSequence,
|
||||
toSequence: batch.toSequence,
|
||||
},
|
||||
},
|
||||
{ deliverAs: "followUp", triggerTurn: true },
|
||||
);
|
||||
if (!isCurrent()) return;
|
||||
const stored = ledger.saveProposal(batch, proposal);
|
||||
ledger.compact(analysis);
|
||||
logger.review("auto_review.analysis_completed", {
|
||||
proposalId: stored.proposalId,
|
||||
fromSequence: stored.fromSequence,
|
||||
toSequence: stored.toSequence,
|
||||
allowCandidates: proposal.allowCandidates.length,
|
||||
denyCandidates: proposal.denyCandidates.length,
|
||||
hardGuardCandidates: proposal.hardGuardCandidates.length,
|
||||
});
|
||||
nextAnalysisAttemptAt = 0;
|
||||
} catch (error) {
|
||||
if (!isCurrent()) return;
|
||||
nextAnalysisAttemptAt = Date.now() + ANALYSIS_RETRY_BACKOFF_MS;
|
||||
logger.review("auto_review.analysis_failed", {
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
} finally {
|
||||
if (analysisGeneration === generation) analysisRunning = false;
|
||||
}
|
||||
};
|
||||
|
||||
return {
|
||||
handleSessionStart(context): void {
|
||||
handleSessionStart(ctx): void {
|
||||
runtime?.controller.abort();
|
||||
analysisGeneration += 1;
|
||||
analysisRunning = false;
|
||||
runtime = {
|
||||
registry: context.modelRegistry,
|
||||
sessionManager: context.sessionManager,
|
||||
registry: ctx.modelRegistry,
|
||||
sessionManager: ctx.sessionManager,
|
||||
controller: new AbortController(),
|
||||
};
|
||||
circuitBreaker.resetTurn();
|
||||
warnWhenUnconfigured(config, ctx);
|
||||
},
|
||||
handleTurnStart(): void {
|
||||
circuitBreaker.resetTurn();
|
||||
},
|
||||
async handleAgentSettled(): Promise<void> {
|
||||
await runPendingAnalysis();
|
||||
},
|
||||
handleSessionShutdown(): void {
|
||||
runtime?.controller.abort();
|
||||
runtime = undefined;
|
||||
analysisGeneration += 1;
|
||||
analysisRunning = false;
|
||||
circuitBreaker.resetTurn();
|
||||
},
|
||||
};
|
||||
|
||||
@@ -254,13 +254,13 @@ export function mergeUnifiedConfigs(
|
||||
) as ShellToolsConfig;
|
||||
}
|
||||
|
||||
// Built-in AutoReview and analyzer objects merge field-by-field so a project
|
||||
// may narrow runtime settings without erasing global defaults.
|
||||
for (const key of ["autoReview", "analysis"] as const) {
|
||||
const baseValue = base[key];
|
||||
const overrideValue = override[key];
|
||||
// The built-in AutoReview object merges field-by-field so a project may
|
||||
// narrow runtime settings without erasing global defaults.
|
||||
{
|
||||
const baseValue = base.autoReview;
|
||||
const overrideValue = override.autoReview;
|
||||
if (baseValue !== undefined || overrideValue !== undefined) {
|
||||
merged[key] = { ...baseValue, ...overrideValue };
|
||||
merged.autoReview = { ...baseValue, ...overrideValue };
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -17,10 +17,6 @@ export function getHardGuardConfigPath(agentDir: string): string {
|
||||
return join(getGlobalConfigDir(agentDir), "hard-guards.jsonc");
|
||||
}
|
||||
|
||||
export function getAnalysisStateDir(agentDir: string): string {
|
||||
return join(getGlobalConfigDir(agentDir), "analysis");
|
||||
}
|
||||
|
||||
|
||||
export function getGlobalLogsDir(agentDir: string): string {
|
||||
return join(getGlobalConfigDir(agentDir), "logs");
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
import { z } from "zod";
|
||||
import {
|
||||
analysisConfigFileSchema,
|
||||
autoReviewConfigFileSchema,
|
||||
} from "./auto-review/config.ts";
|
||||
import { autoReviewConfigFileSchema } from "./auto-review/config.ts";
|
||||
|
||||
/**
|
||||
* Single source of truth for the permission-system config file shape.
|
||||
@@ -251,10 +248,6 @@ export const unifiedConfigSchema = z
|
||||
description:
|
||||
"Built-in model-assisted reviewer used for static-policy review decisions.",
|
||||
}),
|
||||
analysis: analysisConfigFileSchema.optional().meta({
|
||||
description:
|
||||
"Background policy analysis triggered after a configured number of completed automatic reviews.",
|
||||
}),
|
||||
shellTools: shellToolsSchema.optional(),
|
||||
})
|
||||
.meta({
|
||||
|
||||
@@ -2,9 +2,7 @@ import { mkdirSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import {
|
||||
normalizeAutoReviewAnalysisConfig,
|
||||
normalizeAutoReviewConfig,
|
||||
type AutoReviewAnalysisConfig,
|
||||
type AutoReviewConfig,
|
||||
} from "./auto-review/config.js";
|
||||
import type {
|
||||
@@ -38,8 +36,6 @@ export interface PermissionSystemExtensionConfig {
|
||||
authorizerChain?: string[];
|
||||
/** Built-in model reviewer configuration. */
|
||||
autoReview?: AutoReviewConfig;
|
||||
/** Count-based background policy analysis configuration. */
|
||||
analysis?: AutoReviewAnalysisConfig;
|
||||
}
|
||||
|
||||
export const DEFAULT_EXTENSION_CONFIG: PermissionSystemExtensionConfig = {
|
||||
@@ -82,9 +78,6 @@ export function normalizePermissionSystemConfig(
|
||||
if (raw.autoReview !== undefined) {
|
||||
result.autoReview = normalizeAutoReviewConfig(raw.autoReview);
|
||||
}
|
||||
if (raw.analysis !== undefined) {
|
||||
result.analysis = normalizeAutoReviewAnalysisConfig(raw.analysis);
|
||||
}
|
||||
if (raw.forwardingTimeoutMs !== undefined) {
|
||||
result.forwardingTimeoutMs = raw.forwardingTimeoutMs;
|
||||
}
|
||||
|
||||
@@ -243,11 +243,8 @@ export default function piPermissionSystemExtension(pi: ExtensionAPI): void {
|
||||
);
|
||||
|
||||
const autoReview = installBuiltinAutoReview({
|
||||
pi,
|
||||
service: permissionsService,
|
||||
config: configStore,
|
||||
logger,
|
||||
agentDir,
|
||||
});
|
||||
|
||||
// Subscribe to @gotgenes/pi-subagents' child lifecycle events so child
|
||||
@@ -316,9 +313,6 @@ export default function piPermissionSystemExtension(pi: ExtensionAPI): void {
|
||||
lifecycle.handleResourcesDiscover(event, ctx),
|
||||
);
|
||||
pi.on("turn_start", () => autoReview.handleTurnStart());
|
||||
pi.on("agent_settled", () => {
|
||||
void autoReview.handleAgentSettled();
|
||||
});
|
||||
pi.on("session_shutdown", async () => {
|
||||
autoReview.handleSessionShutdown();
|
||||
await lifecycle.handleSessionShutdown();
|
||||
|
||||
@@ -1,141 +0,0 @@
|
||||
import { mkdtempSync, readFileSync, rmSync, statSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG,
|
||||
DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
} from "#src/auto-review/config";
|
||||
import { AutoReviewLedger } from "#src/auto-review/ledger";
|
||||
import type { PromptPermissionDetails } from "#src/service";
|
||||
|
||||
const roots: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function details(requestId: string): PromptPermissionDetails {
|
||||
return {
|
||||
requestId,
|
||||
source: "tool",
|
||||
surface: "bash",
|
||||
toolName: "bash",
|
||||
command: "npm install example",
|
||||
agentName: "main",
|
||||
} as unknown as PromptPermissionDetails;
|
||||
}
|
||||
|
||||
describe("AutoReviewLedger", () => {
|
||||
it("marks analysis pending only after the configured number of completed verdicts", () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "auto-review-ledger-"));
|
||||
roots.push(root);
|
||||
const ledger = new AutoReviewLedger(root);
|
||||
const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 2 };
|
||||
const assessment = {
|
||||
outcome: "allow" as const,
|
||||
riskLevel: "low" as const,
|
||||
userAuthorization: "high" as const,
|
||||
rationale: "directly authorized",
|
||||
};
|
||||
|
||||
ledger.recordDecision({
|
||||
details: details("request-1"),
|
||||
assessment,
|
||||
policyRevision: "policy-a",
|
||||
config: DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
analysis,
|
||||
});
|
||||
expect(ledger.hasPendingAnalysis()).toBe(false);
|
||||
expect(ledger.prepareBatch(analysis)).toBeUndefined();
|
||||
|
||||
ledger.recordDecision({
|
||||
details: details("request-2"),
|
||||
assessment,
|
||||
policyRevision: "policy-a",
|
||||
config: DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
analysis,
|
||||
});
|
||||
expect(ledger.hasPendingAnalysis()).toBe(true);
|
||||
const batch = ledger.prepareBatch(analysis);
|
||||
expect(batch?.fromSequence).toBe(1);
|
||||
expect(batch?.toSequence).toBe(2);
|
||||
expect(batch?.records).toHaveLength(2);
|
||||
expect(batch?.records[0]).not.toHaveProperty("rationale");
|
||||
expect(batch?.records[0].normalizedPattern).toBe("npm install *");
|
||||
});
|
||||
|
||||
it("persists owner-only state and resumes a pending batch", () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "auto-review-ledger-"));
|
||||
roots.push(root);
|
||||
const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 1 };
|
||||
const ledger = new AutoReviewLedger(root);
|
||||
ledger.recordDecision({
|
||||
details: details("request-1"),
|
||||
assessment: {
|
||||
outcome: "deny",
|
||||
riskLevel: "high",
|
||||
userAuthorization: "unknown",
|
||||
rationale: "unsafe",
|
||||
},
|
||||
policyRevision: "policy-b",
|
||||
config: DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
analysis,
|
||||
});
|
||||
|
||||
const resumed = new AutoReviewLedger(root);
|
||||
expect(resumed.hasPendingAnalysis()).toBe(true);
|
||||
expect(resumed.prepareBatch(analysis)?.records[0].outcome).toBe("deny");
|
||||
expect(statSync(join(root, "state.json")).mode & 0o777).toBe(0o600);
|
||||
expect(readFileSync(join(root, "ledger", "decisions.jsonl"), "utf8")).not.toContain("unsafe");
|
||||
});
|
||||
|
||||
it("does not retain command arguments, secret path segments, or rationale", () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "auto-review-ledger-"));
|
||||
roots.push(root);
|
||||
const ledger = new AutoReviewLedger(root);
|
||||
const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 10 };
|
||||
const assessment = {
|
||||
outcome: "allow" as const,
|
||||
riskLevel: "medium" as const,
|
||||
userAuthorization: "high" as const,
|
||||
rationale: "secret-rationale",
|
||||
};
|
||||
const commandRecord = ledger.recordDecision({
|
||||
details: {
|
||||
requestId: "request-command",
|
||||
source: "tool",
|
||||
agentName: "main",
|
||||
surface: "bash",
|
||||
toolName: "bash",
|
||||
command: "curl https://token.example/?api_key=secret-value",
|
||||
} as unknown as PromptPermissionDetails,
|
||||
assessment,
|
||||
policyRevision: "policy-c",
|
||||
config: DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
analysis,
|
||||
});
|
||||
const pathRecord = ledger.recordDecision({
|
||||
details: {
|
||||
requestId: "request-path",
|
||||
source: "tool",
|
||||
agentName: "main",
|
||||
surface: "path",
|
||||
toolName: "read",
|
||||
path: "clients/acme-secret/token.txt",
|
||||
} as unknown as PromptPermissionDetails,
|
||||
assessment,
|
||||
policyRevision: "policy-c",
|
||||
config: DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
analysis,
|
||||
});
|
||||
|
||||
expect(commandRecord.normalizedPattern).toBe("curl *");
|
||||
expect(pathRecord.normalizedPattern).toBe("<relative>/*");
|
||||
const persisted = readFileSync(join(root, "ledger", "decisions.jsonl"), "utf8");
|
||||
expect(persisted).not.toContain("secret-value");
|
||||
expect(persisted).not.toContain("acme-secret");
|
||||
expect(persisted).not.toContain("secret-rationale");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,94 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import type { Api, Model, Provider } from "@earendil-works/pi-ai";
|
||||
import { resolveReviewModel, type ReviewModelRegistry } from "#src/auto-review/model";
|
||||
import { DEFAULT_AUTO_REVIEW_CONFIG, type AutoReviewConfig } from "#src/auto-review/config";
|
||||
|
||||
const model: Model<Api> = {
|
||||
id: "reviewer-model",
|
||||
name: "Reviewer Model",
|
||||
api: "openai-completions",
|
||||
provider: "vendor",
|
||||
baseUrl: "https://example.invalid",
|
||||
reasoning: true,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 1000,
|
||||
maxTokens: 100,
|
||||
};
|
||||
|
||||
function registryWith(providerId: string | undefined): ReviewModelRegistry {
|
||||
const provider = { id: providerId ?? "vendor", getModels: () => [model] } as unknown as Provider<Api>;
|
||||
return {
|
||||
find: (candidateProvider, modelId) =>
|
||||
candidateProvider === providerId && modelId === model.id ? model : undefined,
|
||||
getAll: () => [model],
|
||||
getProvider: (candidate) => (candidate === providerId ? provider : undefined),
|
||||
getApiKeyAndHeaders: async () => ({ ok: true, apiKey: "key" }),
|
||||
};
|
||||
}
|
||||
|
||||
function config(overrides: Partial<AutoReviewConfig>): AutoReviewConfig {
|
||||
return { ...DEFAULT_AUTO_REVIEW_CONFIG, ...overrides };
|
||||
}
|
||||
|
||||
describe("resolveReviewModel", () => {
|
||||
it("resolves the operator-named model from the registry", () => {
|
||||
const result = resolveReviewModel(
|
||||
registryWith("vendor"),
|
||||
config({ provider: "vendor", model: "reviewer-model" }),
|
||||
);
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
if (!result.ok) throw new Error("expected success");
|
||||
expect(result.value.model.id).toBe("reviewer-model");
|
||||
expect(result.value.provider.id).toBe("vendor");
|
||||
});
|
||||
|
||||
it("reports model-unconfigured when no provider is named", () => {
|
||||
const result = resolveReviewModel(registryWith("vendor"), config({ model: "reviewer-model" }));
|
||||
|
||||
expect(result).toEqual({ ok: false, category: "model-unconfigured" });
|
||||
});
|
||||
|
||||
it("reports model-unconfigured when no model is named", () => {
|
||||
const result = resolveReviewModel(registryWith("vendor"), config({ provider: "vendor" }));
|
||||
|
||||
expect(result).toEqual({ ok: false, category: "model-unconfigured" });
|
||||
});
|
||||
|
||||
it("reports model-unconfigured for the default config with no named pair", () => {
|
||||
const result = resolveReviewModel(registryWith("vendor"), DEFAULT_AUTO_REVIEW_CONFIG);
|
||||
|
||||
expect(result).toEqual({ ok: false, category: "model-unconfigured" });
|
||||
});
|
||||
|
||||
it("reports provider-unresolved for an unknown provider", () => {
|
||||
const result = resolveReviewModel(
|
||||
registryWith("vendor"),
|
||||
config({ provider: "missing", model: "reviewer-model" }),
|
||||
);
|
||||
|
||||
expect(result).toEqual({ ok: false, category: "provider-unresolved" });
|
||||
});
|
||||
|
||||
it("reports model-unresolved for a model the provider does not publish", () => {
|
||||
const result = resolveReviewModel(
|
||||
registryWith("vendor"),
|
||||
config({ provider: "vendor", model: "missing-model" }),
|
||||
);
|
||||
|
||||
expect(result).toEqual({ ok: false, category: "model-unresolved" });
|
||||
});
|
||||
|
||||
it("never synthesizes a model the provider does not publish", () => {
|
||||
// The reviewer used to fall back to a synthesized Codex stand-in. It must
|
||||
// now refuse instead, so the ask defers to the human prompt.
|
||||
const result = resolveReviewModel(
|
||||
registryWith("vendor"),
|
||||
config({ provider: "vendor", model: "codex-auto-review" }),
|
||||
);
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -1,61 +1,15 @@
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import type { ExtensionAPI, ModelRegistry, SessionManager } from "@earendil-works/pi-coding-agent";
|
||||
import {
|
||||
DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG,
|
||||
DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
} from "#src/auto-review/config";
|
||||
import { AutoReviewLedger } from "#src/auto-review/ledger";
|
||||
import type { ExtensionContext, ModelRegistry, SessionManager } from "@earendil-works/pi-coding-agent";
|
||||
import { DEFAULT_AUTO_REVIEW_CONFIG, type AutoReviewConfig } from "#src/auto-review/config";
|
||||
import { installBuiltinAutoReview } from "#src/auto-review/runtime";
|
||||
import { getAnalysisStateDir } from "#src/config-paths";
|
||||
import type { ConfigReader } from "#src/config-store";
|
||||
import type { PermissionsService, PromptPermissionDetails } from "#src/service";
|
||||
import type { ReviewLogger } from "#src/session-logger";
|
||||
import type { PermissionsService } from "#src/service";
|
||||
|
||||
type AnalysisRunner = NonNullable<
|
||||
Parameters<typeof installBuiltinAutoReview>[0]["runAnalysis"]
|
||||
>;
|
||||
|
||||
const roots: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function seedPending(agentDir: string) {
|
||||
const analysis = { ...DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, threshold: 1 };
|
||||
const ledger = new AutoReviewLedger(getAnalysisStateDir(agentDir));
|
||||
ledger.recordDecision({
|
||||
details: {
|
||||
requestId: "request-1",
|
||||
source: "tool",
|
||||
agentName: "main",
|
||||
surface: "bash",
|
||||
toolName: "bash",
|
||||
command: "npm install example",
|
||||
} as unknown as PromptPermissionDetails,
|
||||
assessment: {
|
||||
outcome: "allow",
|
||||
riskLevel: "low",
|
||||
userAuthorization: "high",
|
||||
rationale: "directly authorized",
|
||||
},
|
||||
policyRevision: "policy-a",
|
||||
config: DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
analysis,
|
||||
});
|
||||
return { analysis, ledger };
|
||||
}
|
||||
|
||||
function harness(agentDir: string, analysis: typeof DEFAULT_AUTO_REVIEW_ANALYSIS_CONFIG, runAnalysis: Parameters<typeof installBuiltinAutoReview>[0]["runAnalysis"]) {
|
||||
const sendMessage = vi.fn();
|
||||
function harness(autoReview: AutoReviewConfig, hasUI = true) {
|
||||
const notify = vi.fn();
|
||||
const registerAuthorizer = vi.fn(() => () => {});
|
||||
const review = vi.fn();
|
||||
const controller = installBuiltinAutoReview({
|
||||
pi: { sendMessage } as unknown as ExtensionAPI,
|
||||
service: { registerAuthorizer } as unknown as PermissionsService,
|
||||
config: {
|
||||
current: () => ({
|
||||
@@ -63,118 +17,75 @@ function harness(agentDir: string, analysis: typeof DEFAULT_AUTO_REVIEW_ANALYSIS
|
||||
permissionReviewLog: true,
|
||||
yoloMode: false,
|
||||
doublePressToConfirm: true,
|
||||
autoReview: DEFAULT_AUTO_REVIEW_CONFIG,
|
||||
analysis,
|
||||
authorizerChain: ["auto-review"],
|
||||
autoReview,
|
||||
}),
|
||||
} as ConfigReader,
|
||||
logger: { review } as unknown as ReviewLogger,
|
||||
agentDir,
|
||||
runAnalysis,
|
||||
});
|
||||
controller.handleSessionStart({
|
||||
const ctx = {
|
||||
hasUI,
|
||||
ui: { notify },
|
||||
modelRegistry: {} as ModelRegistry,
|
||||
sessionManager: { getBranch: () => [] } as unknown as Pick<SessionManager, "getBranch">,
|
||||
});
|
||||
return { controller, sendMessage, registerAuthorizer, review };
|
||||
} as unknown as ExtensionContext;
|
||||
|
||||
controller.handleSessionStart(ctx);
|
||||
return { controller, notify, registerAuthorizer };
|
||||
}
|
||||
|
||||
describe("built-in AutoReview lifecycle", () => {
|
||||
it("analyzes a persisted pending checkpoint at agent_settled and follows up in the same session", async () => {
|
||||
const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
|
||||
roots.push(agentDir);
|
||||
const { analysis } = seedPending(agentDir);
|
||||
const runAnalysis = vi.fn<AnalysisRunner>(async () => ({
|
||||
summary: "No unconditional policy changes recommended.",
|
||||
allowCandidates: [],
|
||||
denyCandidates: [],
|
||||
keepReview: [],
|
||||
hardGuardCandidates: [],
|
||||
}));
|
||||
const { controller, sendMessage, registerAuthorizer } = harness(agentDir, analysis, runAnalysis);
|
||||
|
||||
await controller.handleAgentSettled();
|
||||
it("registers the auto-review authorizer", () => {
|
||||
const { registerAuthorizer } = harness({ ...DEFAULT_AUTO_REVIEW_CONFIG });
|
||||
|
||||
expect(registerAuthorizer).toHaveBeenCalledWith("auto-review", expect.any(Function));
|
||||
expect(runAnalysis).toHaveBeenCalledTimes(1);
|
||||
expect(runAnalysis.mock.calls[0]?.[0].batch).toMatchObject({ fromSequence: 1, toSequence: 1 });
|
||||
expect(sendMessage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ customType: "permission-policy-analysis-ready", display: true }),
|
||||
{ deliverAs: "followUp", triggerTurn: true },
|
||||
);
|
||||
expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(false);
|
||||
|
||||
await controller.handleAgentSettled();
|
||||
expect(runAnalysis).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("keeps the checkpoint pending when background analysis fails", async () => {
|
||||
const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
|
||||
roots.push(agentDir);
|
||||
const { analysis } = seedPending(agentDir);
|
||||
const runAnalysis = vi.fn<AnalysisRunner>(async () => {
|
||||
throw new Error("analysis unavailable");
|
||||
it("warns once at session start when provider and model are unset", () => {
|
||||
const { notify } = harness({ ...DEFAULT_AUTO_REVIEW_CONFIG });
|
||||
|
||||
expect(notify).toHaveBeenCalledTimes(1);
|
||||
expect(notify.mock.calls[0]?.[1]).toBe("warning");
|
||||
expect(notify.mock.calls[0]?.[0]).toMatch(/provider.*model/iu);
|
||||
});
|
||||
|
||||
it("stays silent when both provider and model are configured", () => {
|
||||
const { notify } = harness({ ...DEFAULT_AUTO_REVIEW_CONFIG, provider: "vendor", model: "m" });
|
||||
|
||||
expect(notify).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("stays silent when AutoReview is disabled", () => {
|
||||
const { notify } = harness({ ...DEFAULT_AUTO_REVIEW_CONFIG, enabled: false });
|
||||
|
||||
expect(notify).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not notify without a UI", () => {
|
||||
const { notify } = harness({ ...DEFAULT_AUTO_REVIEW_CONFIG }, false);
|
||||
|
||||
expect(notify).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("stays silent when auto-review is not in the authorizer chain", () => {
|
||||
const notify = vi.fn();
|
||||
const controller = installBuiltinAutoReview({
|
||||
service: { registerAuthorizer: vi.fn(() => () => {}) } as unknown as PermissionsService,
|
||||
config: {
|
||||
current: () => ({
|
||||
debugLog: false,
|
||||
permissionReviewLog: true,
|
||||
yoloMode: false,
|
||||
doublePressToConfirm: true,
|
||||
authorizerChain: [],
|
||||
autoReview: { ...DEFAULT_AUTO_REVIEW_CONFIG },
|
||||
}),
|
||||
} as ConfigReader,
|
||||
});
|
||||
const { controller, sendMessage, review } = harness(agentDir, analysis, runAnalysis);
|
||||
|
||||
await controller.handleAgentSettled();
|
||||
|
||||
expect(sendMessage).not.toHaveBeenCalled();
|
||||
expect(review).toHaveBeenCalledWith("auto_review.analysis_failed", { error: "analysis unavailable" });
|
||||
expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(true);
|
||||
});
|
||||
|
||||
it("discards a stale result after session replacement without advancing the checkpoint", async () => {
|
||||
const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
|
||||
roots.push(agentDir);
|
||||
const { analysis } = seedPending(agentDir);
|
||||
let resolveAnalysis!: (proposal: Awaited<ReturnType<AnalysisRunner>>) => void;
|
||||
const runAnalysis = vi.fn<AnalysisRunner>(() => new Promise((resolve) => {
|
||||
resolveAnalysis = resolve;
|
||||
}));
|
||||
const { controller, sendMessage, review } = harness(agentDir, analysis, runAnalysis);
|
||||
|
||||
const pending = controller.handleAgentSettled();
|
||||
expect(runAnalysis).toHaveBeenCalledTimes(1);
|
||||
controller.handleSessionShutdown();
|
||||
controller.handleSessionStart({
|
||||
modelRegistry: {} as ModelRegistry,
|
||||
sessionManager: { getBranch: () => [] } as unknown as Pick<SessionManager, "getBranch">,
|
||||
});
|
||||
resolveAnalysis({
|
||||
summary: "stale proposal",
|
||||
allowCandidates: [],
|
||||
denyCandidates: [],
|
||||
keepReview: [],
|
||||
hardGuardCandidates: [],
|
||||
});
|
||||
await pending;
|
||||
hasUI: true,
|
||||
ui: { notify },
|
||||
} as unknown as ExtensionContext);
|
||||
|
||||
expect(sendMessage).not.toHaveBeenCalled();
|
||||
expect(review).not.toHaveBeenCalledWith("auto_review.analysis_completed", expect.anything());
|
||||
expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(true);
|
||||
});
|
||||
|
||||
it("does not advance the checkpoint when same-session delivery fails", async () => {
|
||||
const agentDir = mkdtempSync(join(tmpdir(), "auto-review-runtime-"));
|
||||
roots.push(agentDir);
|
||||
const { analysis } = seedPending(agentDir);
|
||||
const runAnalysis = vi.fn<AnalysisRunner>(async () => ({
|
||||
summary: "proposal",
|
||||
allowCandidates: [],
|
||||
denyCandidates: [],
|
||||
keepReview: [],
|
||||
hardGuardCandidates: [],
|
||||
}));
|
||||
const { controller, sendMessage, review } = harness(agentDir, analysis, runAnalysis);
|
||||
sendMessage.mockImplementation(() => {
|
||||
throw new Error("session delivery unavailable");
|
||||
});
|
||||
|
||||
await controller.handleAgentSettled();
|
||||
|
||||
expect(review).toHaveBeenCalledWith("auto_review.analysis_failed", {
|
||||
error: "session delivery unavailable",
|
||||
});
|
||||
expect(new AutoReviewLedger(getAnalysisStateDir(agentDir)).hasPendingAnalysis()).toBe(true);
|
||||
expect(notify).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -45,7 +45,6 @@ const SERVING_REGISTRY_KEY = Symbol.for(
|
||||
|
||||
/** Events the factory must register a handler for. */
|
||||
const EXPECTED_HANDLERS = [
|
||||
"agent_settled",
|
||||
"before_agent_start",
|
||||
"input",
|
||||
"resources_discover",
|
||||
|
||||
@@ -131,6 +131,34 @@ describe("loadUnifiedConfig", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects the removed analysis key so a stale config cannot silently widen", () => {
|
||||
const configPath = join(tempDir, "config.json");
|
||||
writeFileSync(
|
||||
configPath,
|
||||
JSON.stringify({ analysis: { threshold: 500 }, permission: { "*": "ask" } }),
|
||||
);
|
||||
|
||||
const result = loadUnifiedConfig(configPath);
|
||||
expect(result.issues.some((issue) => issue.includes("analysis"))).toBe(true);
|
||||
// Fail-closed: the whole scope config is dropped, so `ask` is the effective
|
||||
// default again rather than any rule the stale file meant to contribute.
|
||||
expect(result.config).toEqual({});
|
||||
});
|
||||
|
||||
it("merges the autoReview object field-by-field", () => {
|
||||
const merged = mergeUnifiedConfigs(
|
||||
{ autoReview: { enabled: true, provider: "vendor", model: "m" } },
|
||||
{ autoReview: { reasoning: "low" } },
|
||||
);
|
||||
|
||||
expect(merged.autoReview).toEqual({
|
||||
enabled: true,
|
||||
provider: "vendor",
|
||||
model: "m",
|
||||
reasoning: "low",
|
||||
});
|
||||
});
|
||||
|
||||
it("strips JSONC comments before parsing", () => {
|
||||
const configPath = join(tempDir, "config.json");
|
||||
writeFileSync(
|
||||
|
||||
Reference in New Issue
Block a user