Przeglądaj źródła

fix(permission): keep the picker retryable after a failed catalog read

Availability now follows the Session projection alone, so a failed catalog
read surfaces through the picker's own retry instead of hiding the command.
Type-guard the persisted denial reason before trimming it, and record in the
Auto review note why the reviewer reads the complete action history.
Chinesezjc 4 tygodni temu
rodzic
commit
e995d71a0e

+ 2 - 2
.agents/notes/implemented/feature/2026-08-28-auto-review.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-08-28-auto-review.md
 #   pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-08-28-auto-review.md
-2026-08-28-auto-review.md: b8308048a0f1b7e435fe1b7d1610d37c5624820f
-2026-08-28-auto-review.zh.md: 1950be4302902820e2428214d16e4e020a1a4fbb
+2026-08-28-auto-review.md: c16b41bbd2f3ab7e6b269011c052240874a75d64
+2026-08-28-auto-review.zh.md: 64579efc523aedf174bb91f6f495ba5bc1f0fdc4

+ 2 - 0
.agents/notes/implemented/feature/2026-08-28-auto-review.md

@@ -40,6 +40,8 @@ The integration uses only the latest `request/header.config` provider/model and
 | `FILTERED_HISTORY` | Current compaction surface's sourced human/direct-parent messages, checkpoints, image/attachment facts, and historical call names with logged arguments |
 | `FILTERED_HISTORY` | Current compaction surface's sourced human/direct-parent messages, checkpoints, image/attachment facts, and historical call names with logged arguments |
 | `PENDING_ACTION` | Tool name, description, parameter schema, and parsed arguments |
 | `PENDING_ACTION` | Tool name, description, parameter schema, and parsed arguments |
 
 
+The reviewer builds those two action sections from the Session's complete action history: an authorization is any matching earlier call, and a duplicate identity has to be visible anywhere in the log rather than inside a recent window, so neither the Session projections nor a bounded read serves the decision. The read is the deprecated synchronous `snapshotEvents()` under a line-scoped `typescript/no-deprecated` waiver that names this note.
+
 The main agent's V3 `system/message` nodes, assistant text/reasoning, and tool results are excluded. The current call must belong to the open step recorded by `step/start`; missing step ownership fails closed. It appears only in `PENDING_ACTION`; an unstarted sibling has no historical call fact. Native schema comes from the latest request header. PTC captures a frozen schema at binding construction and passes it through the scheduler into `ToolExecution`; descriptions and parameter schemas never enter start/settle events or the Session/SDK wire. Missing, inconsistent, or ambiguous action facts reject the call without consulting the live registry. An oversized request fails closed without summarization, truncation, another compaction pass, or a small output-token budget.
 The main agent's V3 `system/message` nodes, assistant text/reasoning, and tool results are excluded. The current call must belong to the open step recorded by `step/start`; missing step ownership fails closed. It appears only in `PENDING_ACTION`; an unstarted sibling has no historical call fact. Native schema comes from the latest request header. PTC captures a frozen schema at binding construction and passes it through the scheduler into `ToolExecution`; descriptions and parameter schemas never enter start/settle events or the Session/SDK wire. Missing, inconsistent, or ambiguous action facts reject the call without consulting the live registry. An oversized request fails closed without summarization, truncation, another compaction pass, or a small output-token budget.
 
 
 ### Result and cancellation
 ### Result and cancellation

+ 2 - 0
.agents/notes/implemented/feature/2026-08-28-auto-review.zh.md

@@ -40,6 +40,8 @@ Integration 只使用最新 `request/header.config` 的 provider/model 与 shi
 | `FILTERED_HISTORY` | 当前 compaction surface 中带来源的 human/直接父级消息、checkpoint、图片/附件事实,以及历史调用名称与日志参数 |
 | `FILTERED_HISTORY` | 当前 compaction surface 中带来源的 human/直接父级消息、checkpoint、图片/附件事实,以及历史调用名称与日志参数 |
 | `PENDING_ACTION` | 工具名称、描述、参数 schema 与解析后的 arguments |
 | `PENDING_ACTION` | 工具名称、描述、参数 schema 与解析后的 arguments |
 
 
+reviewer 从 Session 的完整动作历史构建这两个动作分节:授权是任何匹配的更早调用,而重复身份必须在整份日志的任何位置都可见,而不是只在近期窗口内可见,因此 Session 投影与有界读取都无法支撑该判定。这次读取使用已废弃的同步 `snapshotEvents()`,并带有指认本 note 的行级 `typescript/no-deprecated` 豁免。
+
 主 agent 的 V3 `system/message` 节点、assistant 正文/reasoning 与 tool results 全部排除。当前调用必须属于 `step/start` 记录的开放 step;缺少 step 归属时拒绝执行。该调用只在 `PENDING_ACTION` 出现;尚未开始的 sibling 没有历史调用事实。原生 schema 来自最新 request header。PTC 在 binding 构造时捕获冻结 schema,经由调度器传入 `ToolExecution`;描述与参数 schema 不进入开始/结算事件或 Session/SDK wire。动作事实缺失、不一致或有歧义时拒绝调用,不查询 live registry。超窗请求直接拒绝,不做摘要、截断、额外 compaction 或设置小型输出 token 预算。
 主 agent 的 V3 `system/message` 节点、assistant 正文/reasoning 与 tool results 全部排除。当前调用必须属于 `step/start` 记录的开放 step;缺少 step 归属时拒绝执行。该调用只在 `PENDING_ACTION` 出现;尚未开始的 sibling 没有历史调用事实。原生 schema 来自最新 request header。PTC 在 binding 构造时捕获冻结 schema,经由调度器传入 `ToolExecution`;描述与参数 schema 不进入开始/结算事件或 Session/SDK wire。动作事实缺失、不一致或有歧义时拒绝调用,不查询 live registry。超窗请求直接拒绝,不做摘要、截断、额外 compaction 或设置小型输出 token 预算。
 
 
 ### 结果与取消
 ### 结果与取消

+ 4 - 4
packages/client/ui-permission-presets/src/client/index.ts

@@ -172,10 +172,10 @@ export function apply(ctx: ClientContext): void {
 
 
   ctx.effect(() => command.decorate({
   ctx.effect(() => command.decorate({
     name: 'permission',
     name: 'permission',
-    // Both halves must exist: the Session exposes its current value and the
-    // active Host generation has supplied a complete selectable catalog.
-    available: session => selectionOf(sessionFor(session)) !== undefined
-      && catalog.store.getSnapshot().value !== null,
+    // The Session's current value alone decides availability. A missing catalog
+    // surfaces through `options()`, which keeps the picker's own retry entry
+    // reachable after a failed read instead of hiding the command.
+    available: session => selectionOf(sessionFor(session)) !== undefined,
     ui: {
     ui: {
       kind: 'popupSelect',
       kind: 'popupSelect',
       options: async (session) => {
       options: async (session) => {

+ 28 - 0
packages/client/ui-permission-presets/tests/browser-plugin.client.spec.ts

@@ -53,9 +53,16 @@ async function bench() {
   onTestFinished(() => { mock.assertNoUnmatched() })
   onTestFinished(() => { mock.assertNoUnmatched() })
   let catalog = CATALOG
   let catalog = CATALOG
   let catalogCalls = 0
   let catalogCalls = 0
+  let catalogFailure: string | undefined
   const permissionPresets = {
   const permissionPresets = {
     catalog: () => {
     catalog: () => {
       catalogCalls += 1
       catalogCalls += 1
+      if (catalogFailure !== undefined) {
+        return Promise.resolve({
+          ok: false as const,
+          error: { code: 'gateway/internal', message: catalogFailure },
+        })
+      }
       return Promise.resolve({ ok: true as const, value: catalog })
       return Promise.resolve({ ok: true as const, value: catalog })
     },
     },
   }
   }
@@ -113,6 +120,7 @@ async function bench() {
       catalog = value
       catalog = value
       remote.emit('permission-presets/catalog-changed', [])
       remote.emit('permission-presets/catalog-changed', [])
     },
     },
+    setCatalogFailure: (message: string | undefined) => { catalogFailure = message },
     setResult: (r: { ok: boolean; matched?: boolean }) => { commandResult = r },
     setResult: (r: { ok: boolean; matched?: boolean }) => { commandResult = r },
     decoration: () => decoration,
     decoration: () => decoration,
     popup: (): PopupSelectSpec => {
     popup: (): PopupSelectSpec => {
@@ -227,6 +235,26 @@ describe('ui-permission browser plugin', () => {
       .rejects.toThrow(/not available on this host/)
       .rejects.toThrow(/not available on this host/)
   })
   })
 
 
+  it('keeps the picker available after a failed catalog read and recovers on the next open', async () => {
+    const b = await bench()
+    const c = b.decoration()!
+    const proj = { sessionId: sid('s1') }
+    b.values.set(sid('s1'), { currentValue: 'workspace-write' })
+    b.setCatalogFailure('catalog read failed')
+    b.setCatalog(CATALOG)
+
+    await expect(b.popup().options(proj, new AbortController().signal))
+      .rejects.toThrow('catalog read failed')
+    // The failed read clears the catalog. The command stays available so the
+    // picker keeps its own retry entry, and a later open re-reads the catalog.
+    expect(c.available(proj)).toBe(true)
+
+    b.setCatalogFailure(undefined)
+    const recovered = await b.popup().options(proj, new AbortController().signal)
+    expect(recovered.map(option => option.id))
+      .toEqual(['read-only', 'workspace-write', 'danger-full-access', 'auto'])
+  })
+
   it('localizes the Auto description instead of displaying host English copy', async () => {
   it('localizes the Auto description instead of displaying host English copy', async () => {
     const b = await bench()
     const b = await bench()
     b.ctx.locale.setLocale('zh')
     b.ctx.locale.setLocale('zh')

+ 3 - 1
packages/client/ui-tool/src/client/tool/models/tool-call-model.ts

@@ -119,7 +119,9 @@ function deriveAutoReviewDenial(block: ToolCallBlock): AutoReviewDenial | null {
   if (!('kind' in block) || !block.isError) return null
   if (!('kind' in block) || !block.isError) return null
   const error = block.error
   const error = block.error
   if (error?.name !== 'AutoReviewDeniedError' || error.code !== 'AUTO_REVIEW_DENIED') return null
   if (error?.name !== 'AutoReviewDeniedError' || error.code !== 'AUTO_REVIEW_DENIED') return null
-  return { reason: error.reason ?? null }
+  // A durable record reaches this renderer without a type check on `reason`, so
+  // a non-string value degrades to the no-reason copy exactly as a missing one.
+  return { reason: typeof error.reason === 'string' ? error.reason : null }
 }
 }
 
 
 /**
 /**

+ 4 - 0
packages/client/ui-tool/tests/tool-row.client.spec.tsx

@@ -215,6 +215,10 @@ describe('tool-call-model', () => {
       isError: true,
       isError: true,
       error: { name: 'AutoReviewDeniedError', code: 'AUTO_REVIEW_DENIED' },
       error: { name: 'AutoReviewDeniedError', code: 'AUTO_REVIEW_DENIED' },
     })).autoReviewDenial).toEqual({ reason: null })
     })).autoReviewDenial).toEqual({ reason: null })
+    expect(toolRowModel('bash', result({
+      isError: true,
+      error: { name: 'AutoReviewDeniedError', code: 'AUTO_REVIEW_DENIED', reason: 42 },
+    } as never)).autoReviewDenial).toEqual({ reason: null })
     expect(toolRowModel('bash', result({
     expect(toolRowModel('bash', result({
       isError: true,
       isError: true,
       error: { name: 'AutoReviewDeniedError', code: 'OTHER' },
       error: { name: 'AutoReviewDeniedError', code: 'OTHER' },

+ 1 - 1
packages/experimental/auto-review/src/index.ts

@@ -362,7 +362,7 @@ function snapshotAutoReview(agent: Agent, exec: ToolExecution): ReviewSnapshot {
   // and PTC starts carry the authorizations and duplicate identities this call is
   // and PTC starts carry the authorizations and duplicate identities this call is
   // compared against, and the direct parent's initial prompt sets the delegated
   // compared against, and the direct parent's initial prompt sets the delegated
   // scope. No projection or paged reader exposes those records yet.
   // scope. No projection or paged reader exposes those records yet.
-  // oxlint-disable-next-line typescript/no-deprecated -- Existing Session history read; migration deferred.
+  // oxlint-disable-next-line typescript/no-deprecated -- Reviewer needs the whole action history; no projection or paged reader exists yet.
   const events = session.snapshotEvents()
   const events = session.snapshotEvents()
   const nodes = [...session.surface.nodes]
   const nodes = [...session.surface.nodes]
   const header = session.requestHeader()
   const header = session.requestHeader()

+ 1 - 1
packages/experimental/auto-review/tests/auto-review.spec.ts

@@ -1431,7 +1431,7 @@ describe('cancellation and integration teardown', () => {
     await reinstalled.dispose()
     await reinstalled.dispose()
   })
   })
 
 
-  it('loads without a configured read-only preset', async () => {
+  it('publishes Auto without validating the preset table at load', async () => {
     const invalid = new Context()
     const invalid = new Context()
     contexts.push(invalid)
     contexts.push(invalid)
     await invalid.plugin(LlmRuntime)
     await invalid.plugin(LlmRuntime)