From 290e1acc45ba7e9710eb507b999eb84ba8fb100d Mon Sep 17 00:00:00 2001 From: kingwl Date: Sun, 26 Jul 2026 23:56:41 +0800 Subject: [PATCH] policy: guard adoption baselines, resolve-time validation, and inherited-delta narration Review fixes (ds-review-bot on #623): - Persistence adoption compares the immutable policy baselines: onCreated's ownerless claim and adoptLivePrefix retain the STORED header, so a same-id live session with a conflicting baseline now rejects as a collision instead of appending under read-only and resuming under the stored danger-full-access. - resolve() resolves the session override BEFORE applying an explicit approved mode: the one-shot grant no longer bypasses the unconditional durable-header validation. - The approval narrator attributes positionally over the session's OWN events (past the seed boundary): a fork child whose baseline delta has no own override narrates 'inherited from the delegating session' instead of misattributing a stale seed-carried switch to the user or the operator. Red-first: baseline-conflict adoption in the shared coordinator contract (both backends), resolve-with-explicit-mode validation, and the fork-child narration attribution case. --- packages/sandbox/sandbox-policy/src/index.ts | 6 +++- .../sandbox-policy/tests/policy.spec.ts | 9 ++++++ .../session-persistence/src/coordinator.ts | 18 +++++++++++ .../tests/coordinator-contract.ts | 21 +++++++++++++ packages/ui/user-approval/README.i18n.yaml | 4 +-- packages/ui/user-approval/README.md | 4 +-- packages/ui/user-approval/README.zh.md | 4 +-- packages/ui/user-approval/src/index.ts | 24 ++++++++++----- .../ui/user-approval/tests/approval.spec.ts | 30 +++++++++++++++++++ 9 files changed, 105 insertions(+), 15 deletions(-) diff --git a/packages/sandbox/sandbox-policy/src/index.ts b/packages/sandbox/sandbox-policy/src/index.ts index 2dba47a643..dd4f2d6465 100644 --- a/packages/sandbox/sandbox-policy/src/index.ts +++ b/packages/sandbox/sandbox-policy/src/index.ts @@ -99,8 +99,12 @@ export class SandboxPolicyService extends Service { */ resolve(request: SandboxPolicyRequest = {}): SandboxExecutionPolicy { const { session } = request + // Resolve the session override FIRST even when an explicit approved mode + // outranks it: the unconditional durable-header validation must hold on + // every resolution — a one-shot grant is not a validation bypass. + const override = session === undefined ? undefined : this.overrideOf(session) return { - mode: request.mode ?? (session === undefined ? undefined : this.overrideOf(session)) ?? this.defaultMode, + mode: request.mode ?? override ?? this.defaultMode, workspaceRoot: resolveWorkspaceRoot(session?.header.cwd ?? this.workspaceRoot), } } diff --git a/packages/sandbox/sandbox-policy/tests/policy.spec.ts b/packages/sandbox/sandbox-policy/tests/policy.spec.ts index d69b5b0e2b..b0362b6592 100644 --- a/packages/sandbox/sandbox-policy/tests/policy.spec.ts +++ b/packages/sandbox/sandbox-policy/tests/policy.spec.ts @@ -229,4 +229,13 @@ describe('delegation inheritance (overrideOf over the header baseline)', () => { expect(() => ctx.sandboxPolicy.overrideOf(child)).toThrow(/seedLength/) }) + + it('resolve() validates the durable header even when an explicit approved mode is supplied', async () => { + const ctx = await mounted() + const child = inheritedSession('sess-resolve-invalid', { sandboxMode: 'yolo' }) + + // The explicit one-shot grant must not become a validation bypass: the + // unconditional durable-header contract holds on EVERY resolution. + expect(() => ctx.sandboxPolicy.resolve({ session: child, mode: 'workspace-write' })).toThrow(/sandboxMode/) + }) }) diff --git a/packages/session-persistence/session-persistence/src/coordinator.ts b/packages/session-persistence/session-persistence/src/coordinator.ts index c252957db7..25ffad5f93 100644 --- a/packages/session-persistence/session-persistence/src/coordinator.ts +++ b/packages/session-persistence/session-persistence/src/coordinator.ts @@ -113,6 +113,22 @@ async function settledErrors(promises: Iterable>): Promise { if (tracked.meta.cwd !== session.header.cwd) { throw new Error(`session "${id}" is already persisted at a different cwd (persisted: ${String(tracked.meta.cwd)}, live: ${String(session.header.cwd)}) (id collision)`) } + assertSamePolicyBaselines(id, tracked.meta, session.header) if (!await this.seedMatchesPersisted(id, seed, tracked.cursor)) { throw new Error(`session "${id}" is already persisted with ${tracked.cursor} event(s) that do not match this live session (id collision)`) } @@ -587,6 +604,7 @@ export class PersistenceCoordinator { if (meta.cwd !== session.header.cwd) { throw new Error(`session "${session.header.id}" is already persisted at a different cwd (persisted: ${String(meta.cwd)}, live: ${String(session.header.cwd)}) (id collision)`) } + assertSamePolicyBaselines(session.header.id, meta, session.header) this.assertVersion(meta) assertSupportedEvents(events, session.header.id) if (!seedCoversPrefix(seed, events)) { diff --git a/packages/session-persistence/session-persistence/tests/coordinator-contract.ts b/packages/session-persistence/session-persistence/tests/coordinator-contract.ts index 83ee8d774c..4bbb8aa471 100644 --- a/packages/session-persistence/session-persistence/tests/coordinator-contract.ts +++ b/packages/session-persistence/session-persistence/tests/coordinator-contract.ts @@ -679,6 +679,27 @@ export function runCoordinatorContract(name: string, makeFixture: () => Promise< } }) + it('a live session with a CONFLICTING policy baseline cannot adopt a stored prefix', async () => { + const fix = await makeFixture() + const { ctx, fiber } = await freshCtx(fix) + try { + // A stored artifact carrying a WIDE baseline. A same-id live session + // claiming a NARROW baseline must be rejected: adoption retains the + // stored header, so accepting the pair would let the session append + // under read-only now but resume under danger-full-access later. + await ctx.sessionPersistence.create({ ...meta('baseline-conflict', WORK), sandboxMode: 'danger-full-access' }) + await ctx.sessionPersistence.append(SessionId('baseline-conflict'), oneTurnLog()) + const live = ctx.sessions.create(SessionId('baseline-conflict'), { + seed: oneTurnLog(), + meta: { cwd: WORK, sandboxMode: 'read-only' }, + }) + await expect(ctx.sessions.flush(live)).rejects.toThrow(/policy baseline|id collision/) + } finally { + await fiber.dispose() + await fix.cleanup() + } + }) + it('a no-cwd ownerless state cannot be claimed by a live session WITH a cwd (cwd scope, undefined side)', async () => { const fix = await makeFixture() const { ctx, fiber } = await freshCtx(fix) diff --git a/packages/ui/user-approval/README.i18n.yaml b/packages/ui/user-approval/README.i18n.yaml index 0d7efac766..eff3ad6622 100644 --- a/packages/ui/user-approval/README.i18n.yaml +++ b/packages/ui/user-approval/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -README.md: 978a397f3f48b4e20461c2486e0611313526abf9 -README.zh.md: 96bbb13c0fbe555faa3dcca44caf31227c505a41 +README.md: 251782ac19de54413e6e318d141f39f964900a32 +README.zh.md: 9fab4f5cb73bb893d7c55a2e65c43c96eb9dfa4c diff --git a/packages/ui/user-approval/README.md b/packages/ui/user-approval/README.md index 978a397f3f..251782ac19 100644 --- a/packages/ui/user-approval/README.md +++ b/packages/ui/user-approval/README.md @@ -8,7 +8,7 @@ Each request must belong to an open agent turn. The service appends a paired `ap Answerers are `approval/request` waterfall listeners. Return an outcome to answer for an owned agent or call `next()` to delegate. Agent-scoped listeners receive only that agent's requests; compose one terminal answerer per deployment because sibling listener order is not a policy priority mechanism. The ACP automation bridge supplies one-shot machine decisions for sessions it owns. -`ApprovalPolicy` is `'ask'` or `'never'`. The effective value is the last `approval/policy` event, falling back to config; `setApprovalPolicy()` is the write path. `'never'` rejects before interactive dispatch and is the only policy stated in the prompt. Switches produce at most one coalesced notice, attributed to the user when the override follows the last `request/header` and to operator/config otherwise. `ctx.approval.overrideOf(session)` (the pure `approvalOverrideOf` export, also consumed by the permission presets) resolves the session's override chain, never the configured default: with an inherited `approvalPolicy` header baseline (a delegation child), the fold of the session's OWN switches past `SessionHeader.seedLength`, else the baseline, validated against the closed vocabulary on read; without one (a top-level session or a generic `SessionStore.fork` child), the whole-log fold, so a seed-carried `'never'` survives; the in-process subagent driver captures this at delegation and writes it into each child's creation-time header, so a `'never'` parent cannot mint prompting children, with no first-turn timing window ([rationale](../../../.agents/notes/implemented/feature/2026-07-25-subagent-policy-inheritance.md)). +`ApprovalPolicy` is `'ask'` or `'never'`. The effective value is the last `approval/policy` event, falling back to config; `setApprovalPolicy()` is the write path. `'never'` rejects before interactive dispatch and is the only policy stated in the prompt. Switches produce at most one coalesced notice, attributed positionally over the session's OWN events: to the user when an own override follows the last own `request/header`, to the delegating session when no own override exists and the delta matches the inherited header baseline, and to operator/config otherwise. `ctx.approval.overrideOf(session)` (the pure `approvalOverrideOf` export, also consumed by the permission presets) resolves the session's override chain, never the configured default: with an inherited `approvalPolicy` header baseline (a delegation child), the fold of the session's OWN switches past `SessionHeader.seedLength`, else the baseline, validated against the closed vocabulary on read; without one (a top-level session or a generic `SessionStore.fork` child), the whole-log fold, so a seed-carried `'never'` survives; the in-process subagent driver captures this at delegation and writes it into each child's creation-time header, so a `'never'` parent cannot mint prompting children, with no first-turn timing window ([rationale](../../../.agents/notes/implemented/feature/2026-07-25-subagent-policy-inheritance.md)). The tools pipeline routes `ask` decisions through this seam and fails closed when it is absent; the sandboxed bash tool also uses it for escalated retries. The ACP automation bridge answers calls for its own agents through the client's machine policy. Audit events remain log-only, so the model sees only the asking consumer's result. See the [approval-seam Agent Note](../../../.agents/notes/implemented/feature/2026-07-06-approval-seam.md) and [sandbox Agent Note](../../../.agents/notes/implemented/feature/2026-07-06-sandbox.md). @@ -18,7 +18,7 @@ The tools pipeline routes `ask` decisions through this seam and fails closed whe #### What the model sees -Under `ask`, every agent request carries the ask-policy prompt section below. Under `never`, it carries the never-policy prompt section below. A policy switch injects exactly `The approval policy changed from "" to "" (changed by the user).` or `The approval policy changed from "" to "" (changed by the operator/config).` before the next step. +Under `ask`, every agent request carries the ask-policy prompt section below. Under `never`, it carries the never-policy prompt section below. A policy switch injects exactly `The approval policy changed from "" to "" (changed by the user).`, `The approval policy changed from "" to "" (inherited from the delegating session).`, or `The approval policy changed from "" to "" (changed by the operator/config).` before the next step. ##### Ask-policy prompt section diff --git a/packages/ui/user-approval/README.zh.md b/packages/ui/user-approval/README.zh.md index 96bbb13c0f..9fab4f5cb7 100644 --- a/packages/ui/user-approval/README.zh.md +++ b/packages/ui/user-approval/README.zh.md @@ -8,7 +8,7 @@ 应答者是 `approval/request` waterfall（瀑布式事件）监听器。要回答所拥有 agent 的请求，请返回一个结果；否则调用 `next()` 委托。限定到 agent 的监听器只接收该 agent 的请求；每项部署应当组合一个终端应答者，因为同级监听器的顺序不是策略优先级机制。ACP（Agent Client Protocol）自动化桥接层为其拥有的会话提供一次性机器决定。 -`ApprovalPolicy` 为 `'ask'` 或 `'never'`。实际值取最后一条 `approval/policy` 事件，并回退到配置；`setApprovalPolicy()` 是写入路径。`'never'` 会在交互式分发之前拒绝请求，也是提示词中唯一声明的策略。切换最多产生一条合并通知：如果覆盖发生在最后一个 `request/header` 之后，则归因于用户；否则归因于操作方／配置。`ctx.approval.overrideOf(session)`（即纯函数导出 `approvalOverrideOf`，也供权限 preset 消费）解析会话的覆盖链，绝不包含配置默认值：当存在继承的 `approvalPolicy` 会话头基线时（即委派子 agent），先折叠会话自己在 `SessionHeader.seedLength` 之后的切换，否则取该基线，读取时按封闭词汇校验；没有基线时（顶层会话或通用的 `SessionStore.fork` 子会话），折叠覆盖完整日志，因此种子携带的 `'never'` 得以存续；进程内 subagent 驱动器在委派时捕获该值，并写入每个子 agent 创建时的会话头，使 `'never'` 父级无法造出会弹出提示的子 agent，且不存在任何第一轮次的时序窗口（参见[设计原理](../../../.agents/notes/implemented/feature/2026-07-25-subagent-policy-inheritance.md)）。 +`ApprovalPolicy` 为 `'ask'` 或 `'never'`。实际值取最后一条 `approval/policy` 事件，并回退到配置；`setApprovalPolicy()` 是写入路径。`'never'` 会在交互式分发之前拒绝请求，也是提示词中唯一声明的策略。切换最多产生一条合并通知，并按其在会话自己的事件中的位置归因：如果会话自己的覆盖出现在自己最后一个 `request/header` 之后，则归因于用户；如果不存在自己的覆盖且该变化与继承的会话头基线相符，则归因于发起委派的会话；否则归因于操作方／配置。`ctx.approval.overrideOf(session)`（即纯函数导出 `approvalOverrideOf`，也供权限 preset 消费）解析会话的覆盖链，绝不包含配置默认值：当存在继承的 `approvalPolicy` 会话头基线时（即委派子 agent），先折叠会话自己在 `SessionHeader.seedLength` 之后的切换，否则取该基线，读取时按封闭词汇校验；没有基线时（顶层会话或通用的 `SessionStore.fork` 子会话），折叠覆盖完整日志，因此种子携带的 `'never'` 得以存续；进程内 subagent 驱动器在委派时捕获该值，并写入每个子 agent 创建时的会话头，使 `'never'` 父级无法造出会弹出提示的子 agent，且不存在任何第一轮次的时序窗口（参见[设计原理](../../../.agents/notes/implemented/feature/2026-07-25-subagent-policy-inheritance.md)）。 工具流水线通过此 seam 路由 `ask` 决定，并在该 seam 缺失时以拒绝方式关闭；沙箱 bash 工具也会将它用于升权重试。ACP 自动化桥接层根据客户端的机器策略，回答其自有 agent 的调用。审计事件仍只写入日志，因此模型只会看到发起请求的消费方所返回的结果。详见[审批 seam Agent Note（agent 决策记录）](../../../.agents/notes/implemented/feature/2026-07-06-approval-seam.md)和[沙箱 Agent Note](../../../.agents/notes/implemented/feature/2026-07-06-sandbox.md)。 @@ -18,7 +18,7 @@ #### 模型看到的内容 -在 `ask` 下，每个 agent 请求都会携带下方的 ask 策略提示词段。在 `never` 下，请求会携带下方的 never 策略提示词段。策略切换会在下一步骤前精确注入 `The approval policy changed from "" to "" (changed by the user).` 或 `The approval policy changed from "" to "" (changed by the operator/config).`。 +在 `ask` 下，每个 agent 请求都会携带下方的 ask 策略提示词段。在 `never` 下，请求会携带下方的 never 策略提示词段。策略切换会在下一步骤前精确注入 `The approval policy changed from "" to "" (changed by the user).`、`The approval policy changed from "" to "" (inherited from the delegating session).` 或 `The approval policy changed from "" to "" (changed by the operator/config).`。 ##### Ask 策略提示词段 diff --git a/packages/ui/user-approval/src/index.ts b/packages/ui/user-approval/src/index.ts index 06ea9ae9ef..3d4ed57416 100644 --- a/packages/ui/user-approval/src/index.ts +++ b/packages/ui/user-approval/src/index.ts @@ -276,17 +276,21 @@ export class ApprovalService extends Service { // turn's first step (net-zero → nothing), and a mid-turn switch is // narrated no later than the next step. What each session was last told // is in-memory with a log-derived fallback (the folded header's system - // text), so restarts lose nothing. Attribution is positional: an - // override event after the log's last `request/header` was a runtime - // switch by the user; otherwise the configured default moved under the - // session (operator/config). + // text), so restarts lose nothing. Attribution is positional over the + // session's OWN events (past the seed boundary — a seed-carried switch is + // stale parent history, never this session's runtime action): an own + // override after the last own `request/header` was a runtime switch by + // the user; no own override with the delta matching the inherited header + // baseline came from the delegating session; otherwise the configured + // default moved under the session (operator/config). const narrated = new WeakMap() ctx.on('agent/pre-step', (agent) => { const session = agent.session const events = session.events + const seedStart = Math.min(session.header.seedLength ?? 0, events.length) let overrideIndex = -1 let headerIndex = -1 - for (let index = events.length - 1; index >= 0 && (overrideIndex < 0 || headerIndex < 0); index -= 1) { + for (let index = events.length - 1; index >= seedStart && (overrideIndex < 0 || headerIndex < 0); index -= 1) { const event = events[index] as (typeof events)[number] if (overrideIndex < 0 && event.type === 'approval/policy') { overrideIndex = index @@ -294,8 +298,8 @@ export class ApprovalService extends Service { headerIndex = index } } - // Same fold effectivePolicy performs — override is scanned here anyway - // for POSITIONAL attribution; the default lives once, in the method. + // Same fold effectivePolicy performs — the own override is scanned here + // anyway for POSITIONAL attribution; the default lives once, in the method. const current = this.effectivePolicy(session) const header = session.requestHeader() const told = narrated.get(session) ?? toldApprovalPolicy(header?.system) @@ -303,7 +307,11 @@ export class ApprovalService extends Service { // Cold start (nothing ever told) narrates nothing — the section about // to go out states the truth, and there is no delta to explain. if (told === undefined || told === current) return - const cause = overrideIndex > headerIndex ? 'changed by the user' : 'changed by the operator/config' + const cause = overrideIndex > headerIndex + ? 'changed by the user' + : overrideIndex < 0 && session.header.approvalPolicy === current + ? 'inherited from the delegating session' + : 'changed by the operator/config' agent.inject( [{ type: 'text', text: `The approval policy changed from "${told}" to "${current}" (${cause}).` }], { source: { kind: 'plugin', plugin: 'user-approval' } }, diff --git a/packages/ui/user-approval/tests/approval.spec.ts b/packages/ui/user-approval/tests/approval.spec.ts index 431423f959..c3707b8121 100644 --- a/packages/ui/user-approval/tests/approval.spec.ts +++ b/packages/ui/user-approval/tests/approval.spec.ts @@ -517,6 +517,36 @@ describe('approval policy (the approval/policy fold)', () => { expect(injected).toEqual(['The approval policy changed from "ask" to "never" (changed by the operator/config).']) }) + it('does not attribute a fork child\'s baseline delta to a stale seed-carried user switch', async () => { + // A fork child: the seed carries the parent's OLD 'ask' switch (event 0, + // inside seedLength) and the last request header told 'ask'; the header + // baseline captured at delegation is 'never'. The delta must not be + // attributed to "the user" — the seed switch is stale parent history, not + // this session's runtime action. + const ctx = new Context() + await ctx.plugin(ApprovalService) + const id = SessionId('sess-narr-fork-baseline') + const session = new Session(id, undefined, { + version: 0, + id, + createdAt: 0, + approvalPolicy: 'never', + seedLength: 2, + }) + setApprovalPolicy(session, 'ask') + session.append('request/header', { header: { config: { provider: 'mock', model: 'mock' }, system: `persona\n${ASK_MARKER}` }, reason: 'initial' }) + session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + const injected: string[] = [] + const agent = { + id, + session, + inject: (content: { type: string; text: string }[]) => { injected.push(content[0]?.text ?? '') }, + } as unknown as Agent + + await preStep(ctx, agent) + expect(injected).toEqual(['The approval policy changed from "ask" to "never" (inherited from the delegating session).']) + }) + it('a pinned override survives a default change silently', async () => { const ctx = new Context() await ctx.plugin(ApprovalService, { policy: 'never' })