diff --git a/common/src/types/session-state.ts b/common/src/types/session-state.ts index f17d7d6795..3d0259d350 100644 --- a/common/src/types/session-state.ts +++ b/common/src/types/session-state.ts @@ -42,6 +42,24 @@ export type EditRereadRequirement = { sourceTool?: string } +/** + * Whole-file post-edit anchor: hash+bounds are durable; cap.v3 is reminted only + * when the stored issuer matches the current project+run or the stored token + * still authenticates for that scope. Optional projectId/runId stamp the minting + * issuer so process restart can remint without a live HMAC (legacy anchors + * without these fields still parse). + */ +export type ConfirmedPostEditAnchor = { + startLine: number + endLine: number + contentHash: string + readCapability: string + /** Issuing project identity; optional for legacy session parse. */ + projectId?: string + /** Issuing run identity; optional for legacy session parse. */ + runId?: string +} + /** * One recorded injected-block measurement in the per-turn context budget. * Canonical declaration: `packages/agent-runtime/src/util/context-budget.ts` @@ -199,6 +217,15 @@ export type AgentState = { * content the agent read or most recently wrote successfully. */ readAuthorizationHashesByPath?: Record + /** + * Durable confirmed whole-file post-edit anchors (hash + bounds + remintable + * cap.v3). Hydrated into per-turn FileProcessingState the same way sticky + * hashes are; tokens are reminted only when the stored issuer matches the + * current project+run or the stored token authenticates for that scope. + * Cross-project/run restore drops anchors. Optional so old sessions parse + * cleanly. + */ + confirmedPostEditAnchorsByPath?: Record /** Why a path must be read again after a failed edit, persisted across turns. */ editRereadRequirementsByPath?: Record /** Runtime-owned orchestrator state that must survive message compaction. */ @@ -387,6 +414,7 @@ export function getInitialAgentState(): AgentState { contextWindowTokens: undefined, readAuthorizationsByPath: {}, readAuthorizationHashesByPath: {}, + confirmedPostEditAnchorsByPath: {}, editRereadRequirementsByPath: {}, taskMemory: undefined, workspaceState: createInitialWorkspaceState(), diff --git a/docs/deterministic-edit-system.md b/docs/deterministic-edit-system.md index 0ebdd934b6..f44a4b4e90 100644 --- a/docs/deterministic-edit-system.md +++ b/docs/deterministic-edit-system.md @@ -97,6 +97,16 @@ common cause of "I already read this file, why is my edit blocked?": a follow-up edit, avoiding a redundant re-read. The part appears only when at least one anchor was granted; it is omitted when no anchor could be minted (for example, no runtime-known content or no authoritative scope). +- `coordinateEditApplication` confirms only `confirmationPaths` (default: all + coordinated `paths`). `wholeFileContentByPath` may still include excluded + no-op snapshots for sticky/anchor minting, but the afterHash / covering-action + check is restricted to `confirmationPaths`. An extra snapshot with no applied + action must not undo confirmation of the applied subset. + Client-output inspection (`hasExplicitError`, envelope collection, structured + `stale_snapshot`/`stale_state` classification, and unconfirmed-application + detection) is an iterative heap walk with a depth bound of 6. Recursion is + not used, so deeply nested or cyclic tool outputs cannot overflow the stack; + nodes past the bound are ignored (fail closed). - The strict read-before-edit blocked-recovery message distinguishes a file that was created or edited earlier in the session (it has a confirmed post-edit anchor). Instead of the generic "no fresh read authorization @@ -105,26 +115,39 @@ common cause of "I already read this file, why is my edit blocked?": confirmed post-edit anchor, and the structured `recovery.basedOnRead` echoes that token. The other blocked-recovery causes (`stale_snapshot`, a prior failed edit, stale-revoked, compacted, never-read) are unchanged. + `stale_snapshot` is classified from structured `errorCode` (on the output + object or `failures[]` entries) and from filesystem `code: 'stale_state'` + on mutation errors/actions (SDK CAS), not only `errorCode: 'stale_snapshot'`. - A confirmed `create` (or any confirmed whole-file write) grants sticky - whole-file authorization straight from the runtime-known post-edit bytes — - the bytes a `create` supplies are exact, so the runtime does not need the - client to echo a whole-file-covering anchor. When no usable client anchor is - present, the runtime mints its own `cap.v3` anchor from those known bytes - (scope-bound to project, path, and run) and records it as the confirmed - post-edit anchor. A follow-up `delete` (or `move`) on that path is then - authorized when the anchor's content hash still matches the transaction's - snapshotted current content; an external modification (hash mismatch) fails - closed and requires a fresh read. A `move`'s destination path needs no read - authorization — its safety is enforced by the lifecycle preflight, which - blocks `Move destination already exists`. The client-echoed anchor is - preferred when valid but is never itself trusted to authorize — it is only - reused after passing the 7-point verification. + whole-file authorization **iff** a whole-file post-edit cap can be minted + (client-verified 7-point anchor or a synthesized `cap.v3` from known bytes). + Empty or non-authoritative project/run scope does **not** grant sticky-from- + apply and does **not** store an anchor or surface `postEditCapabilities`. + When minting succeeds, the runtime records the confirmed post-edit anchor + (hash + bounds + issuer projectId/runId) on durable `agentState` and remints + `cap.v3` on hydrate only when the stored issuer matches the current + project+run (or the stored token still authenticates for that scope). + Cross-project/run restore drops anchors rather than rebinding them. + A follow-up `delete` (or `move`) on that path is then authorized when the + anchor's content hash still matches the transaction's snapshotted current + content; an external modification (hash mismatch) fails closed and requires + a fresh read. A `move`'s destination path needs no read authorization — its + safety is enforced by the lifecycle preflight, which blocks `Move destination + already exists`. The client-echoed anchor is preferred when valid but is + never itself trusted to authorize — it is only reused after passing the + 7-point verification. - A `ConfirmedPostEditAnchor` (recorded in `confirmedPostEditAnchorsByPath`) is definitionally whole-file-verified: it is only minted when the 7-point check confirms whole-file coverage (`startLine === 1` and `endLine === totalLines`) with a hash matching the runtime-known post-edit - bytes. There is no scoped/partial confirmed post-edit anchor — a confirmed - apply re-anchors to whole-file because the full post-edit content is always + bytes. Confirmed post-edit anchors are durable on `agentState` across turns; + hash+bounds are the source of truth and `cap.v3` is reminted on hydrate only + when the stored issuer matches the current project+run (or the stored token + authenticates for that scope). Cross-project/run restore drops anchors. + Compaction keeps them (same as sticky hashes); revoke, journal revision + bump, and unknown-tool wipe clear them. + There is no scoped/partial confirmed post-edit anchor — a confirmed apply + re-anchors to whole-file because the full post-edit content is always runtime-known (`processEditTransaction` computes it from gate-verified initial bytes). This is why a confirmed apply may grant whole-file sticky authorization even for a localized edit: the granted hash pins the exact @@ -152,9 +175,9 @@ common cause of "I already read this file, why is my edit blocked?": - Semantic compaction and emergency mechanical trimming **no longer wipe** sticky whole-file authorizations/hashes. They record a `context_compacted` reread reason. For `str_replace`, hash-fresh unique edits may still proceed - and clear the marker only after a successful unique apply (unique `oldString` - is the safety bound); failed/no-match `str_replace` and proper-subset/scoped - reads do **not** clear it. For **`write_file`**, `context_compacted` **blocks** + (unique `oldString` is the safety bound) but a successful unique apply does + **not** clear `context_compacted`; failed/no-match `str_replace` and + proper-subset/scoped reads also do **not** clear it. For **`write_file`**, `context_compacted` **blocks** a whole-file overwrite even when the sticky hash still matches disk — only a complete whole-file `read_files` grant (paths whole-file content, or full-file range `1..totalLines` with `sourceContent`) **or** an explicit whole-file-covering @@ -194,9 +217,10 @@ common cause of "I already read this file, why is my edit blocked?": not an exploratory `read_files` first. The same preference applies to strict auth-miss and residual process-failure recovery messages: mint/`basedOnRead` first when content is known; `read_files` remains the fallback only when no - capability can be minted. Auto-reread for transaction `str_replace` does **not** - clear `context_compacted`/failed-edit markers pre-apply — only successful unique - apply (or a real whole-file basedOnRead/read) clears them. + capability can be minted. Auto-reread and a successful unique `str_replace` + apply do **not** clear `context_compacted` — only a complete whole-file + `read_files` grant or an explicit whole-file-covering `basedOnRead` does. + Other failed-edit markers may still clear after a confirmed unique apply. - Input-only and preflight failures that never reached the client preserve a still-current whole-file authorization. Failures that make filesystem state uncertain revoke it and persist a typed reread reason across turns; the next diff --git a/packages/agent-runtime/src/__tests__/read-files-edit-state.test.ts b/packages/agent-runtime/src/__tests__/read-files-edit-state.test.ts index d97131d58b..91f0790d95 100644 --- a/packages/agent-runtime/src/__tests__/read-files-edit-state.test.ts +++ b/packages/agent-runtime/src/__tests__/read-files-edit-state.test.ts @@ -2,7 +2,10 @@ import { TEST_AGENT_RUNTIME_IMPL } from '@codebuff/common/testing/impl/agent-run import { editTransactionParams } from '@codebuff/common/tools/params/tool/edit-transaction' import { buildReadFilesResultV1 } from '@codebuff/common/tools/results/filesystem' import { getInitialSessionState } from '@codebuff/common/types/session-state' -import { getExactContentHash } from '@codebuff/common/util/content-hash' +import { + decodeReadCapabilityToken, + getExactContentHash, +} from '@codebuff/common/util/content-hash' import { describe, expect, it } from 'bun:test' import { handleEditTransaction } from '../tools/handlers/tool/edit-transaction' @@ -19,6 +22,10 @@ import { getContentHash, } from '../process-str-replace' import { processStream } from '../tools/stream-parser' +import { + remintConfirmedPostEditAnchors, + revokeImplicitReadAuthorizationsAfterCompaction, +} from '../util/read-authorization' import { createMockStreamWithToolCalls, mockFileContext } from './test-utils' import type { FileProcessingState } from '../tools/handlers/tool/write-file' @@ -4861,6 +4868,7 @@ describe('read_files edit-state recovery', () => { type: 'json' as const, value: { file: toolCall.input.path, + errorCode: 'stale_snapshot', errorMessage: 'replace_range rejected: stale range', }, }, @@ -5287,7 +5295,8 @@ describe('read_files edit-state recovery', () => { fileProcessingState.editRereadRequirementsByPath?.[path]?.reason, ).toBe('context_compacted') - // str_replace unique may still apply on hash-fresh and clear the marker. + // Unique str_replace may still apply on hash-fresh, but must NOT clear + // context_compacted — only a whole-file read or basedOnRead may. let replaceApplied = false const replaceResult = await handleStrReplace({ ...defaultTestHandlerAuthority, previousToolCallFinished: Promise.resolve(), @@ -5323,8 +5332,46 @@ describe('read_files edit-state recovery', () => { expect(replaceResult.output[0].value).not.toHaveProperty('errorMessage') } expect( - fileProcessingState.editRereadRequirementsByPath?.[path], - ).toBeUndefined() + fileProcessingState.editRereadRequirementsByPath?.[path]?.reason, + ).toBe('context_compacted') + + // Hash-fresh write_file without basedOnRead stays blocked after unique apply. + const postReplaceContent = 'export const value = 3\n' + let followUpWriteApplied = false + const followUpWrite = await handleWriteFile({ ...defaultTestHandlerAuthority, + previousToolCallFinished: Promise.resolve(), + toolCall: { + toolCallId: 'write-after-unique-replace-compaction', + toolName: 'write_file', + input: { path, content: 'export const value = 4\n' }, + }, + agentState: { messageHistory: [] }, + clientSessionId: 'test-session', + fileProcessingState, + fingerprintId: 'test-fingerprint', + logger, + prompt: undefined, + userId: undefined, + userInputId: 'test-input', + requestOptionalFile: async () => postReplaceContent, + requestClientToolCall: async () => { + followUpWriteApplied = true + return [] + }, + writeToClient: () => {}, + } as any) + + expect(followUpWriteApplied).toBe(false) + expect(followUpWrite.output[0]?.type).toBe('json') + if (followUpWrite.output[0]?.type === 'json') { + const msg = String((followUpWrite.output[0].value as any).errorMessage) + expect(msg).toMatch(/context compaction|read_files/i) + expect(msg).not.toContain('cap.v3.') + expect(msg).not.toContain('basedOnRead=') + } + expect( + fileProcessingState.editRereadRequirementsByPath?.[path]?.reason, + ).toBe('context_compacted') }) it('failed str_replace after compaction revokes authorization so write_file stays blocked', async () => { @@ -5424,7 +5471,7 @@ describe('read_files edit-state recovery', () => { // COMPACTION-ALLOWMULTIPLE-NO-CLEAR: a blind replace-all apply is not // evidence the model knows the file content, so it must NOT clear the // context_compacted marker. A subsequent whole-file overwrite stays - // blocked (a unique str_replace apply WOULD have cleared it). + // blocked. A unique str_replace apply also must not clear it. const path = 'src/compacted.ts' const initialContent = 'const x = 1\nconst y = 2\n' const replacedContent = 'const x = 10\nconst y = 2\n' @@ -5525,10 +5572,9 @@ describe('read_files edit-state recovery', () => { ).toBe('context_compacted') }) - it('unique str_replace apply clears context_compacted so write_file can proceed', async () => { - // Contrast: a unique-anchor str_replace apply IS evidence the model knows - // the file content, so it clears the context_compacted marker and a later - // whole-file overwrite is authorized. + it('unique str_replace apply keeps context_compacted so write_file stays blocked', async () => { + // Unique-anchor apply may refresh sticky hashes but must not clear + // context_compacted. Hash-fresh write_file without basedOnRead stays blocked. const path = 'src/compacted.ts' const initialContent = 'const x = 1\nconst y = 2\n' const replacedContent = 'const x = 10\nconst y = 2\n' @@ -5586,10 +5632,9 @@ describe('read_files edit-state recovery', () => { if (replaceResult.output[0]?.type === 'json') { expect(replaceResult.output[0].value).not.toHaveProperty('errorMessage') } - // The unique-anchor apply clears the marker. expect( - fileProcessingState.editRereadRequirementsByPath?.[path], - ).toBeUndefined() + fileProcessingState.editRereadRequirementsByPath?.[path]?.reason, + ).toBe('context_compacted') let writeApplied = false const writeResult = await handleWriteFile({ ...defaultTestHandlerAuthority, @@ -5608,22 +5653,26 @@ describe('read_files edit-state recovery', () => { userId: undefined, userInputId: 'test-input', requestOptionalFile: async () => replacedContent, - requestClientToolCall: async (toolCall: any) => { + requestClientToolCall: async () => { writeApplied = true - return confirmedMutationOutput( - toolCall, - { [path]: 'const x = 100\nconst y = 2\n' }, - { projectId: mockFileContext.projectRoot, runId }, - ) + return [] }, writeToClient: () => {}, } as any) - expect(writeApplied).toBe(true) + expect(writeApplied).toBe(false) expect(writeResult.output[0]?.type).toBe('json') if (writeResult.output[0]?.type === 'json') { - expect(writeResult.output[0].value).not.toHaveProperty('errorMessage') + expect( + String((writeResult.output[0].value as any).errorMessage), + ).toMatch(/compaction|read_files/i) + expect(String((writeResult.output[0].value as any).errorMessage)).not.toContain( + 'basedOnRead=', + ) } + expect( + fileProcessingState.editRereadRequirementsByPath?.[path]?.reason, + ).toBe('context_compacted') }) it('proper-subset range read after compaction does not clear context_compacted', async () => { @@ -6817,7 +6866,7 @@ describe('read_files edit-state recovery', () => { ).toBe(getContentHash(createdContent)) }) - it('cross-turn hydration keeps sticky read authorization but not the confirmed post-edit anchor', async () => { + it('cross-turn hydration remints the confirmed post-edit anchor with sticky auth', async () => { const path = 'src/cross-turn.ts' const createdContent = 'export const c = 1\n' const runId = 'cross-turn-hydration-strict-run' @@ -6857,22 +6906,14 @@ describe('read_files edit-state recovery', () => { if (createOutput.type === 'json') { expect(createOutput.value).not.toHaveProperty('errorMessage') } - // After the confirmed create, state holds BOTH a sticky authorization and - // a confirmed post-edit anchor for the path. expect(fileProcessingState.readAuthorizationsByPath?.[path]).toBe(true) expect(fileProcessingState.readAuthorizationHashesByPath?.[path]).toBe( getContentHash(createdContent), ) - expect( - fileProcessingState.confirmedPostEditAnchorsByPath?.[path], - ).toBeDefined() + const storedAnchor = + fileProcessingState.confirmedPostEditAnchorsByPath?.[path] + expect(storedAnchor).toBeDefined() - // Simulate the per-turn hydration exactly as stream-parser.ts:222-250 - // does: build a FRESH fileProcessingState copying ONLY the durable - // registry fields (readAuthorizationsByPath / - // readAuthorizationHashesByPath / editRereadRequirementsByPath). The - // confirmed post-edit anchor is turn-local and is intentionally NOT - // hydrated. const nextTurnState = createFileProcessingState() nextTurnState.strictReadBeforeEdit = true nextTurnState.readAuthorizationsByPath = { @@ -6884,17 +6925,31 @@ describe('read_files edit-state recovery', () => { nextTurnState.editRereadRequirementsByPath = { ...(fileProcessingState.editRereadRequirementsByPath ?? {}), } + nextTurnState.confirmedPostEditAnchorsByPath = + remintConfirmedPostEditAnchors({ + anchors: fileProcessingState.confirmedPostEditAnchorsByPath, + projectId: mockFileContext.projectRoot, + runId, + }) - // Sticky auth + hash survive the turn boundary. expect(nextTurnState.readAuthorizationsByPath?.[path]).toBe(true) expect(nextTurnState.readAuthorizationHashesByPath?.[path]).toBe( getContentHash(createdContent), ) - // The confirmed post-edit anchor does NOT survive: it is turn-local and - // the durable hydration set omits it. - expect( - nextTurnState.confirmedPostEditAnchorsByPath?.[path], - ).toBeUndefined() + const reminted = nextTurnState.confirmedPostEditAnchorsByPath?.[path] + expect(reminted).toBeDefined() + expect(storedAnchor).toBeDefined() + if (!reminted || !storedAnchor) return + expect(reminted.contentHash).toBe(storedAnchor.contentHash) + expect(reminted.startLine).toBe(storedAnchor.startLine) + expect(reminted.endLine).toBe(storedAnchor.endLine) + const decoded = decodeReadCapabilityToken(reminted.readCapability) + expect(typeof decoded).not.toBe('string') + if (typeof decoded !== 'string') { + expect(decoded.hash).toBe(storedAnchor.contentHash) + expect(decoded.startLine).toBe(storedAnchor.startLine) + expect(decoded.endLine).toBe(storedAnchor.endLine) + } }) it('move on an externally-modified created file still fails closed', async () => { @@ -7691,7 +7746,7 @@ describe('processStream cross-turn read-before-edit', () => { includeMessageHistory: true, inheritParentSystemPrompt: false, mcpServers: {}, - toolNames: ['read_files', 'str_replace', 'end_turn'], + toolNames: ['read_files', 'str_replace', 'write_file', 'end_turn'], spawnableAgents: [], systemPrompt: 'Test system prompt', instructionsPrompt: 'Test instructions', @@ -7879,4 +7934,109 @@ describe('processStream cross-turn read-before-edit', () => { agentState.readAuthorizationHashesByPath?.[targetPath], ).toBeUndefined() }) + + it('keeps context_compacted after unique str_replace so processStream write_file stays blocked', async () => { + const sessionState = getInitialSessionState(mockFileContext) + const agentState = sessionState.mainAgentState + const targetPath = 'src/compacted-stream.ts' + const diskContent = 'export const value = 1\n' + const replacedContent = 'export const value = 2\n' + agentState.readAuthorizationsByPath = { [targetPath]: true } + agentState.readAuthorizationHashesByPath = { + [targetPath]: getContentHash(diskContent), + } + revokeImplicitReadAuthorizationsAfterCompaction(agentState) + expect(agentState.editRereadRequirementsByPath?.[targetPath]?.reason).toBe( + 'context_compacted', + ) + + let writeFileInvoked = false + let currentContent = diskContent + + const agentRuntimeImpl = { + ...TEST_AGENT_RUNTIME_IMPL, + sendAction: () => {}, + requestFiles: async () => + buildWholeFileReadResultV1([targetPath], () => currentContent), + requestOptionalFile: async ({ filePath }: { filePath: string }) => + filePath === targetPath ? currentContent : null, + requestToolCall: async (params: any) => { + if (params.toolName === 'str_replace') { + currentContent = replacedContent + const output = confirmedMutationOutput( + { + toolCallId: params.callId, + input: params.input, + }, + { [targetPath]: replacedContent }, + { + projectId: mockFileContext.projectRoot, + runId: 'test-run-id', + }, + ) + const canonicalReceipt: CommitReceiptV1 = + output[0].value.authorityReceipt + return { output, canonicalReceipt } + } + if (params.toolName === 'write_file') { + writeFileInvoked = true + return { output: [] } + } + return { output: [] } + }, + } as AgentRuntimeDeps & AgentRuntimeScopedDeps + + const stream = createMockStreamWithToolCalls([ + { + toolName: 'str_replace', + input: { + path: targetPath, + replacements: [ + { + oldString: 'export const value = 1', + newString: 'export const value = 2', + allowMultiple: false, + }, + ], + }, + }, + { + toolName: 'write_file', + input: { path: targetPath, content: 'export const value = 3\n' }, + }, + { toolName: 'end_turn', input: {} }, + ]) + + await processStream({ + ...agentRuntimeImpl, + agentContext: {}, + agentState, + agentStepId: 'compaction-turn', + agentTemplate: testAgentTemplate, + ancestorRunIds: [], + clientSessionId: 'test-session', + fileContext: mockFileContext, + fingerprintId: 'test-fingerprint', + fullResponse: '', + localAgentTemplates: { 'test-agent': testAgentTemplate }, + messages: [], + prompt: 'test prompt', + repoId: undefined, + repoUrl: undefined, + runId: 'test-run-id', + signal: new AbortController().signal, + stream, + system: 'test system', + tools: {}, + userId: 'test-user', + userInputId: 'test-input-id', + onCostCalculated: async () => {}, + onResponseChunk: () => {}, + }) + + expect(writeFileInvoked).toBe(false) + expect(agentState.editRereadRequirementsByPath?.[targetPath]?.reason).toBe( + 'context_compacted', + ) + }) }) diff --git a/packages/agent-runtime/src/run-programmatic-step.ts b/packages/agent-runtime/src/run-programmatic-step.ts index 1aeff1ec91..6897c60b60 100644 --- a/packages/agent-runtime/src/run-programmatic-step.ts +++ b/packages/agent-runtime/src/run-programmatic-step.ts @@ -13,6 +13,7 @@ import { getModelContextMessageLimit, getSemanticCompactionBudget, } from './util/context-pruning' +import { remintConfirmedPostEditAnchors } from './util/read-authorization' import type { FileProcessingState } from './tools/handlers/tool/write-file' import type { ExecuteToolCallParams } from './tools/tool-executor' @@ -401,6 +402,13 @@ export async function runProgrammaticStep( readAuthorizationHashesByPath: { ...(agentState.readAuthorizationHashesByPath ?? {}), }, + confirmedPostEditAnchorsByPath: remintConfirmedPostEditAnchors({ + anchors: agentState.confirmedPostEditAnchorsByPath, + projectId: + (params as { fileContext?: { projectRoot?: string } }).fileContext + ?.projectRoot ?? '', + runId: agentState.runId ?? '', + }), editRereadRequirementsByPath: { ...(agentState.editRereadRequirementsByPath ?? {}), }, @@ -624,6 +632,9 @@ export async function runProgrammaticStep( agentState.readAuthorizationHashesByPath = { ...(fileProcessingState.readAuthorizationHashesByPath ?? {}), } + agentState.confirmedPostEditAnchorsByPath = { + ...(fileProcessingState.confirmedPostEditAnchorsByPath ?? {}), + } agentState.editRereadRequirementsByPath = { ...(fileProcessingState.editRereadRequirementsByPath ?? {}), } diff --git a/packages/agent-runtime/src/tools/handlers/tool/__tests__/edit-application-coordinator.test.ts b/packages/agent-runtime/src/tools/handlers/tool/__tests__/edit-application-coordinator.test.ts index 81f756ec9f..f675eb49ec 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/__tests__/edit-application-coordinator.test.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/__tests__/edit-application-coordinator.test.ts @@ -8,6 +8,7 @@ import { } from '@codebuff/common/util/content-hash' import { + commitAppliedEditPaths, coordinateEditApplication, editOutputHasError, } from '../edit-application-coordinator' @@ -104,6 +105,100 @@ describe('edit application coordinator', () => { ).toBe(false) }) + it('walks deeply nested and cyclic tool output iteratively without overflowing', async () => { + const nest = (value: unknown, depth: number): unknown => { + let current = value + for (let i = 0; i < depth; i++) { + current = { wrap: current } + } + return current + } + + // 200 object wrappers would blow a recursive walker; the iterative walk + // must return (and still honor the depth bound: a hit past depth 6 is + // ignored, same as the previous helpers). + expect(() => + editOutputHasError([ + { type: 'json', value: nest({ applied: false }, 200) }, + ] as any), + ).not.toThrow() + expect( + editOutputHasError([ + { type: 'json', value: nest({ applied: false }, 200) }, + ] as any), + ).toBe(false) + expect( + editOutputHasError([ + { type: 'json', value: nest({ applied: false }, 3) }, + ] as any), + ).toBe(true) + + const cyclic: { type: string; value: { self?: unknown } } = { + type: 'json', + value: {}, + } + cyclic.value.self = cyclic + expect(() => editOutputHasError([cyclic] as any)).not.toThrow() + expect(editOutputHasError([cyclic] as any)).toBe(false) + + const deepUnconfirmedState = getFileProcessingValues({ + promisesByPath: { 'a.ts': [] }, + }) + const deepUnconfirmed = await coordinateEditApplication({ + toolName: 'str_replace', + fileProcessingState: deepUnconfirmedState, + ...applicationScope, + paths: ['a.ts'], + apply: async () => + [ + { + type: 'json', + value: nest( + canonicalAppliedOutput('a.ts', 'new content')[0]!.value, + 200, + ), + }, + ] as any, + }) + expect(deepUnconfirmed.status).toBe('rejected') + expect(deepUnconfirmedState.failedEditRequiresReadByPath['a.ts']).toBe(true) + + const deepStaleState = getFileProcessingValues({ + promisesByPath: { 'a.ts': [] }, + readAuthorizationsByPath: { 'a.ts': true }, + readAuthorizationHashesByPath: { 'a.ts': getContentHash('current') }, + }) + const deepStale = await coordinateEditApplication({ + toolName: 'str_replace', + fileProcessingState: deepStaleState, + ...applicationScope, + paths: ['a.ts'], + rejectionRequiresRead: false, + apply: async () => + [ + { + type: 'json', + value: { + errorMessage: 'client rejected', + deep: nest( + { + path: 'a.ts', + errorCode: 'stale_snapshot', + errorMessage: 'stale snapshot', + }, + 200, + ), + }, + }, + ] as any, + }) + expect(deepStale.status).toBe('rejected') + // Structured stale past the walk bound is ignored; this is a generic + // rejection, so rejectionRequiresRead:false keeps authorization. + expect(deepStaleState.failedEditRequiresReadByPath['a.ts']).toBeUndefined() + expect(deepStaleState.readAuthorizationsByPath?.['a.ts']).toBe(true) + }) + it('invalidates every path and authorization when client application rejects', async () => { const state = getFileProcessingValues({ promisesByPath: { 'a.ts': [], 'b.ts': [] }, @@ -266,6 +361,36 @@ describe('edit application coordinator', () => { expect(defaultResult.status).toBe('rejected') }) + it('confirms confirmationPaths when wholeFileContentByPath includes an excluded no-op snapshot', async () => { + // b.ts is a no-op excluded from confirmationPaths, but the runtime still + // snapshots it in wholeFileContentByPath. The afterHash / covering-action + // loop must skip that extra snapshot so a.ts's applied envelope can + // confirm the transaction. + const clientOutput = canonicalAppliedOutput('a.ts', 'new content') as any + const state = getFileProcessingValues({ + promisesByPath: { 'a.ts': [], 'b.ts': [] }, + }) + let committed = false + const result = await coordinateEditApplication({ + toolName: 'edit_transaction', + fileProcessingState: state, + ...applicationScope, + paths: ['a.ts', 'b.ts'], + confirmationPaths: ['a.ts'], + wholeFileContentByPath: new Map([ + ['a.ts', 'new content'], + ['b.ts', 'unchanged no-op'], + ]), + apply: async () => clientOutput, + onApplied: () => { + committed = true + }, + }) + + expect(result.status).toBe('applied') + expect(committed).toBe(true) + }) + it('rejects forged applied evidence before invoking onApplied or granting state', async () => { const state = getFileProcessingValues({ promisesByPath: { 'a.ts': [] } }) let committed = false @@ -775,6 +900,242 @@ describe('edit application coordinator', () => { ) }) + it('does not classify unstructured expected-hash text as stale_snapshot when rejectionRequiresRead is false', async () => { + const state = getFileProcessingValues({ + promisesByPath: { 'a.ts': [] }, + readAuthorizationsByPath: { 'a.ts': true }, + readAuthorizationHashesByPath: { 'a.ts': getContentHash('current') }, + }) + + const result = await coordinateEditApplication({ + toolName: 'str_replace', + fileProcessingState: state, + ...applicationScope, + paths: ['a.ts'], + rejectionRequiresRead: false, + apply: async () => + [ + { + type: 'json', + value: { + errorMessage: + 'client rejected: expected hash / content changed', + }, + }, + ] as any, + }) + + expect(result.status).toBe('rejected') + expect(state.promisesByPath['a.ts']).toBeUndefined() + expect(state.failedEditRequiresReadByPath['a.ts']).toBeUndefined() + expect(state.editRereadRequirementsByPath?.['a.ts']).toBeUndefined() + expect(state.readAuthorizationsByPath?.['a.ts']).toBe(true) + expect(state.readAuthorizationHashesByPath?.['a.ts']).toBe( + getContentHash('current'), + ) + }) + + it('revokes only the structured stale path in a two-path batch while clearing every promisesByPath entry', async () => { + const state = getFileProcessingValues({ + promisesByPath: { 'a.ts': [], 'b.ts': [] }, + readAuthorizationsByPath: { 'a.ts': true, 'b.ts': true }, + readAuthorizationHashesByPath: { + 'a.ts': getContentHash('old a'), + 'b.ts': getContentHash('old b'), + }, + }) + + const result = await coordinateEditApplication({ + toolName: 'edit_transaction', + fileProcessingState: state, + ...applicationScope, + paths: ['a.ts', 'b.ts'], + rejectionRequiresRead: false, + apply: async () => + [ + { + type: 'json', + value: { + errorMessage: 'client rejected batch', + failures: [ + { + path: 'a.ts', + errorCode: 'stale_snapshot', + errorMessage: 'stale snapshot', + }, + ], + }, + }, + ] as any, + }) + + expect(result.status).toBe('rejected') + expect(state.promisesByPath['a.ts']).toBeUndefined() + expect(state.promisesByPath['b.ts']).toBeUndefined() + expect(state.failedEditRequiresReadByPath['a.ts']).toBe(true) + expect(state.failedEditRequiresReadByPath['b.ts']).toBeUndefined() + expect(state.editRereadRequirementsByPath?.['a.ts']).toMatchObject({ + reason: 'stale_snapshot', + }) + expect(state.editRereadRequirementsByPath?.['b.ts']).toBeUndefined() + expect(state.readAuthorizationsByPath?.['a.ts']).toBeUndefined() + expect(state.readAuthorizationsByPath?.['b.ts']).toBe(true) + expect(state.readAuthorizationHashesByPath?.['b.ts']).toBe( + getContentHash('old b'), + ) + }) + + it('still classifies structured stale hits inside failures[] after the iterative walk', async () => { + const state = getFileProcessingValues({ + promisesByPath: { 'a.ts': [] }, + readAuthorizationsByPath: { 'a.ts': true }, + readAuthorizationHashesByPath: { 'a.ts': getContentHash('old a') }, + }) + + const result = await coordinateEditApplication({ + toolName: 'str_replace', + fileProcessingState: state, + ...applicationScope, + paths: ['a.ts'], + rejectionRequiresRead: false, + apply: async () => + [ + { + type: 'json', + value: { + errorMessage: 'client rejected', + failures: [ + { + path: 'a.ts', + errorCode: 'stale_snapshot', + errorMessage: 'stale snapshot', + }, + ], + }, + }, + ] as any, + }) + + expect(result.status).toBe('rejected') + expect(state.failedEditRequiresReadByPath['a.ts']).toBe(true) + expect(state.editRereadRequirementsByPath?.['a.ts']).toMatchObject({ + reason: 'stale_snapshot', + }) + expect(state.readAuthorizationsByPath?.['a.ts']).toBeUndefined() + }) + + it('revokes every coordinated path when a nameless structured stale_snapshot has no path or file', async () => { + const state = getFileProcessingValues({ + promisesByPath: { 'a.ts': [], 'b.ts': [] }, + readAuthorizationsByPath: { 'a.ts': true, 'b.ts': true }, + readAuthorizationHashesByPath: { + 'a.ts': getContentHash('old a'), + 'b.ts': getContentHash('old b'), + }, + }) + + const result = await coordinateEditApplication({ + toolName: 'edit_transaction', + fileProcessingState: state, + ...applicationScope, + paths: ['a.ts', 'b.ts'], + rejectionRequiresRead: false, + apply: async () => + [ + { + type: 'json', + value: { + errorCode: 'stale_snapshot', + errorMessage: 'stale snapshot', + }, + }, + ] as any, + }) + + expect(result.status).toBe('rejected') + expect(state.promisesByPath['a.ts']).toBeUndefined() + expect(state.promisesByPath['b.ts']).toBeUndefined() + expect(state.failedEditRequiresReadByPath).toEqual({ + 'a.ts': true, + 'b.ts': true, + }) + expect(state.editRereadRequirementsByPath?.['a.ts']).toMatchObject({ + reason: 'stale_snapshot', + }) + expect(state.editRereadRequirementsByPath?.['b.ts']).toMatchObject({ + reason: 'stale_snapshot', + }) + expect(state.readAuthorizationsByPath?.['a.ts']).toBeUndefined() + expect(state.readAuthorizationsByPath?.['b.ts']).toBeUndefined() + expect(state.readAuthorizationHashesByPath?.['a.ts']).toBeUndefined() + expect(state.readAuthorizationHashesByPath?.['b.ts']).toBeUndefined() + }) + + it('revokes only the named stale_state action in a file_mutation_result envelope', async () => { + const state = getFileProcessingValues({ + promisesByPath: { 'a.ts': [], 'b.ts': [] }, + readAuthorizationsByPath: { 'a.ts': true, 'b.ts': true }, + readAuthorizationHashesByPath: { + 'a.ts': getContentHash('old a'), + 'b.ts': getContentHash('old b'), + }, + }) + + const result = await coordinateEditApplication({ + toolName: 'edit_transaction', + fileProcessingState: state, + ...applicationScope, + paths: ['a.ts', 'b.ts'], + rejectionRequiresRead: false, + apply: async () => + [ + { + type: 'json', + value: { + kind: 'file_mutation_result', + version: 1, + outcome: 'not_applied', + errors: [ + { + code: 'stale_state', + message: 'file changed since last read', + }, + ], + actions: [ + { + path: 'a.ts', + outcome: 'not_applied', + error: { + code: 'stale_state', + message: 'file changed since last read', + }, + }, + { + path: 'b.ts', + outcome: 'not_applied', + }, + ], + }, + }, + ] as any, + }) + + expect(result.status).toBe('rejected') + expect(state.promisesByPath['a.ts']).toBeUndefined() + expect(state.promisesByPath['b.ts']).toBeUndefined() + expect(state.failedEditRequiresReadByPath['a.ts']).toBe(true) + expect(state.failedEditRequiresReadByPath['b.ts']).toBeUndefined() + expect(state.editRereadRequirementsByPath?.['a.ts']).toMatchObject({ + reason: 'stale_snapshot', + }) + expect(state.editRereadRequirementsByPath?.['b.ts']).toBeUndefined() + expect(state.readAuthorizationsByPath?.['a.ts']).toBeUndefined() + expect(state.readAuthorizationsByPath?.['b.ts']).toBe(true) + expect(state.readAuthorizationHashesByPath?.['b.ts']).toBe( + getContentHash('old b'), + ) + }) + it('threads confirmationPaths through handleEditTransaction so no-op content edits are excluded from positive-evidence confirmation', async () => { // b.ts was a no-op and is excluded from confirmationPaths, so the // transaction is confirmed by a.ts's positive evidence alone. @@ -800,10 +1161,11 @@ describe('edit application coordinator', () => { expect(committed).toBe(true) }) - it('keeps the context_compacted reread marker on a blind allowMultiple str_replace apply while a unique str_replace apply clears it and mints an anchor', async () => { - // allowMultiple (replace-all) str_replace must NOT clear the - // context_compacted reread requirement, so a subsequent write_file stays - // blocked. The reread marker is represented by failedEditRequiresReadByPath. + it('keeps a failed-edit reread marker on a blind allowMultiple apply while a unique apply may clear that marker and mint an anchor', async () => { + // allowMultiple (replace-all) str_replace must NOT clear a failed-edit + // reread marker (failedEditRequiresReadByPath), so a subsequent write_file + // stays blocked. This case does not set context_compacted; that reason is + // independently preserved even on unique apply (see the next test). const blindState = getFileProcessingValues({ promisesByPath: { 'a.ts': [] }, failedEditRequiresReadByPath: { 'a.ts': true }, @@ -829,8 +1191,9 @@ describe('edit application coordinator', () => { readCapability: expect.stringMatching(/^cap\.v3\./), }) - // A unique str_replace apply clears the reread marker post-apply and mints - // an anchor into confirmedPostEditAnchorsByPath. + // A unique str_replace apply may clear a generic failed-edit marker and + // mint an anchor. context_compacted is not set here and must not be + // inferred from this case. const uniqueState = getFileProcessingValues({ promisesByPath: { 'a.ts': [] }, failedEditRequiresReadByPath: { 'a.ts': true }, @@ -854,26 +1217,79 @@ describe('edit application coordinator', () => { }) }) - it('rejects when two committed envelopes for the same path carry conflicting anchors', async () => { - // Two committed applied envelopes for the SAME path but DIFFERENT content, - // so their editAnchor contentHash/readCapability differ. wholeFileContent - // matches only the first envelope. Because wholeFileContentByPath pins the - // known content to 'content one', the second envelope's anchor cannot pass - // the content-pinned 7-point anchor check, so it never becomes a merged - // candidate — the fail-closed rejection here is driven by the union - // afterHash disagreement (the second covering action's afterHash !== - // getExactContentHash('content one')), which returns null and rejects. + it('commitAppliedEditPaths keeps context_compacted when preserveRereadRequirementsForPaths is omitted', async () => { + // Unlike the allowMultiple case (failedEditRequiresReadByPath + preserve set), + // context_compacted is authoritative on its own: a unique apply must keep it + // even when the caller does not pass preserveRereadRequirementsForPaths. + const path = 'a.ts' + const content = 'replaced once' + const state = getFileProcessingValues({ + promisesByPath: { [path]: [] }, + failedEditRequiresReadByPath: { [path]: true }, + editRereadRequirementsByPath: { + [path]: { + reason: 'context_compacted', + sourceTool: 'context compaction', + }, + }, + }) + + const granted = commitAppliedEditPaths({ + fileProcessingState: state, + paths: [path], + wholeFileContentByPath: new Map([[path, content]]), + projectId: applicationScope.projectId, + runId: applicationScope.runId, + }) + + expect(state.editRereadRequirementsByPath?.[path]?.reason).toBe( + 'context_compacted', + ) + expect(state.editRereadRequirementsByPath?.[path]?.sourceTool).toBe( + 'context compaction', + ) + expect(state.failedEditRequiresReadByPath[path]).toBe(true) + expect(granted.get(path)).toMatchObject({ + contentHash: getContentHash(content), + readCapability: expect.stringMatching(/^cap\.v3\./), + }) + + const coordinated = await coordinateEditApplication({ + toolName: 'str_replace', + fileProcessingState: state, + ...applicationScope, + paths: [path], + wholeFileContentByPath: new Map([[path, content]]), + apply: async () => canonicalAppliedOutput(path, content) as any, + }) + + expect(coordinated.status).toBe('applied') + expect(state.editRereadRequirementsByPath?.[path]?.reason).toBe( + 'context_compacted', + ) + }) + + it('rejects when two same-content committed envelopes for the same path carry differing readCapability tokens', async () => { + // Same afterContent/afterHash so both anchors pass the content-pinned + // 7-point check. The second token is a quoted copy of the first: decode + // strips the wrapper so both authenticate, then merge fails closed on + // existing.readCapability !== candidate.readCapability. const state = getFileProcessingValues({ promisesByPath: { 'a.ts': [] } }) let committed = false - const envelopeA = canonicalAppliedOutput('a.ts', 'content one') as any - const envelopeB = canonicalAppliedOutput('a.ts', 'content two') as any + const envelopeA = canonicalAppliedOutput('a.ts', 'same content') as any + const envelopeB = canonicalAppliedOutput('a.ts', 'same content') as any + const token = envelopeB[0].value.actions[0].editAnchor.readCapability + const quotedToken = `"${token}"` + envelopeB[0].value.actions[0].editAnchor.readCapability = quotedToken + envelopeB[0].value.authorityReceipt.actions[0].editAnchor.readCapability = + quotedToken const result = await coordinateEditApplication({ toolName: 'edit_transaction', fileProcessingState: state, ...applicationScope, paths: ['a.ts'], - wholeFileContentByPath: new Map([['a.ts', 'content one']]), + wholeFileContentByPath: new Map([['a.ts', 'same content']]), apply: async () => [...envelopeA, ...envelopeB] as any, onApplied: () => { committed = true @@ -915,16 +1331,15 @@ describe('edit application coordinator', () => { expect(state.readAuthorizationsByPath?.['a.ts']).toBeUndefined() }) - it('grants sticky authorization but mints no anchor when the authoritative scope is empty', async () => { + it('does not grant sticky or store an anchor when the authoritative scope is empty', async () => { // projectId '' and runId '' are NOT authoritative. canonicalAppliedOutput // builds its editAnchor with the default scope { '/project', path, 'run' }, // which does NOT match the empty runtime scope, so the client anchor is // rejected by the 7-point scope check. synthesizePostEditAnchor also // returns null for an empty scope (hasAuthoritativeReadCapabilityScope - // fails), so NO anchor is minted. The apply still confirms (the union - // afterHash check passes since content matches), so sticky authorization - // IS granted straight from the runtime-known bytes — but no - // postEditCapabilities part is surfaced because no anchor was granted. + // fails), so NO whole-file cap can be minted. The apply still confirms + // (the union afterHash check passes since content matches), but sticky + // maps, stored anchors, and postEditCapabilities stay empty. const state = getFileProcessingValues({ promisesByPath: { 'a.ts': [] } }) const result = await coordinateEditApplication({ @@ -937,16 +1352,10 @@ describe('edit application coordinator', () => { apply: async () => canonicalAppliedOutput('a.ts', 'new content') as any, }) - // The confirmed apply succeeds (receipt committed) even with empty scope. expect(result.status).toBe('applied') - // Sticky authorization IS granted from the runtime-known content. - expect(state.readAuthorizationsByPath?.['a.ts']).toBe(true) - expect(state.readAuthorizationHashesByPath?.['a.ts']).toBe( - getContentHash('new content'), - ) - // ...but NO anchor is minted without an authoritative scope. + expect(state.readAuthorizationsByPath?.['a.ts']).toBeUndefined() + expect(state.readAuthorizationHashesByPath?.['a.ts']).toBeUndefined() expect(state.confirmedPostEditAnchorsByPath?.['a.ts']).toBeUndefined() - // And no postEditCapabilities part is appended to the output. const output = (result.status === 'applied' ? result.output : []) as any[] for (const part of output) { expect(part.value).not.toHaveProperty('postEditCapabilities') diff --git a/packages/agent-runtime/src/tools/handlers/tool/edit-application-coordinator.ts b/packages/agent-runtime/src/tools/handlers/tool/edit-application-coordinator.ts index 5e7e338b27..7b5e9bac4b 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/edit-application-coordinator.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/edit-application-coordinator.ts @@ -20,13 +20,7 @@ import { import type { FileProcessingState } from './write-file' import type { CodebuffToolOutput } from '@codebuff/common/tools/list' import type { ToolName } from '@codebuff/common/tools/constants' - -type ConfirmedPostEditAnchor = { - startLine: number - endLine: number - contentHash: string - readCapability: string -} +import type { ConfirmedPostEditAnchor } from '@codebuff/common/types/session-state' type CoordinatedApplication = | { @@ -37,31 +31,71 @@ type CoordinatedApplication = | { status: 'rejected'; output: CodebuffToolOutput } | { status: 'threw'; error: unknown } -function hasExplicitError(value: unknown, depth = 0): boolean { - if (depth > 6 || value === null || value === undefined) return false - if (Array.isArray(value)) { - return value.some((item) => hasExplicitError(item, depth + 1)) - } - if (typeof value !== 'object') return false +const MAX_TOOL_OUTPUT_WALK_DEPTH = 6 - const record = value as Record - if ( - // Only a non-empty errorMessage string signals an error; null/'' are - // benign diagnostic placeholders (symmetric with error: null). - (typeof record.errorMessage === 'string' && - record.errorMessage.length > 0) || - (record.error !== undefined && record.error !== null) || - record.success === false || - record.applied === false || - record.status === 'failed' || - record.status === 'error' || - record.status === 'blocked' - ) { - return true +/** + * Iterative walk of untrusted tool-output trees. Recursion is avoided so + * adversarial nesting cannot overflow the stack. The depth bound keeps + * cyclic or huge objects from running forever; nodes past the bound are + * ignored, matching the previous recursive helpers. + */ +function walkToolOutput( + root: unknown, + visit: (value: unknown, depth: number) => 'descend' | 'skip' | 'stop', + children?: (value: unknown) => unknown[] | undefined, +): boolean { + const stack: Array<{ value: unknown; depth: number }> = [ + { value: root, depth: 0 }, + ] + while (stack.length > 0) { + const frame = stack.pop() + if (frame === undefined) return false + const { value, depth } = frame + if ( + depth > MAX_TOOL_OUTPUT_WALK_DEPTH || + value === null || + value === undefined + ) { + continue + } + if (Array.isArray(value)) { + for (let i = value.length - 1; i >= 0; i--) { + stack.push({ value: value[i], depth: depth + 1 }) + } + continue + } + const decision = visit(value, depth) + if (decision === 'stop') return true + if (decision === 'skip' || typeof value !== 'object') continue + const nested = + children?.(value) ?? Object.values(value as Record) + for (let i = nested.length - 1; i >= 0; i--) { + stack.push({ value: nested[i], depth: depth + 1 }) + } } - return Object.values(record).some((nested) => - hasExplicitError(nested, depth + 1), - ) + return false +} + +function hasExplicitError(value: unknown): boolean { + return walkToolOutput(value, (node) => { + if (typeof node !== 'object') return 'skip' + const record = node as Record + if ( + // Only a non-empty errorMessage string signals an error; null/'' are + // benign diagnostic placeholders (symmetric with error: null). + (typeof record.errorMessage === 'string' && + record.errorMessage.length > 0) || + (record.error !== undefined && record.error !== null) || + record.success === false || + record.applied === false || + record.status === 'failed' || + record.status === 'error' || + record.status === 'blocked' + ) { + return 'stop' + } + return 'descend' + }) } type FileMutationResultV1 = Parameters[0] @@ -72,33 +106,25 @@ type ConfirmedAppliedActionV1 = ReturnType< function collectEnvelopes( value: unknown, - depth: number, out: FileMutationResultV1[], ): void { - if (depth > 6 || value === null || value === undefined) return - if (Array.isArray(value)) { - for (const item of value) { - collectEnvelopes(item, depth + 1, out) - } - return - } - if (typeof value !== 'object') return - const parsed = fileMutationResultV1Schema.safeParse(value) - if (parsed.success) { - // Envelope nodes are atomic: never recurse into a parsed envelope's - // internals. A non-applied or non-committed envelope simply contributes - // nothing (explicit rejections are handled by hasExplicitError earlier). - if ( - parsed.data.outcome === 'applied' && - parsed.data.authorityReceipt?.status === 'committed' - ) { - out.push(parsed.data) + walkToolOutput(value, (node) => { + if (typeof node !== 'object') return 'skip' + const parsed = fileMutationResultV1Schema.safeParse(node) + if (parsed.success) { + // Envelope nodes are atomic: never walk a parsed envelope's internals. + // A non-applied or non-committed envelope simply contributes nothing + // (explicit rejections are handled by hasExplicitError earlier). + if ( + parsed.data.outcome === 'applied' && + parsed.data.authorityReceipt?.status === 'committed' + ) { + out.push(parsed.data) + } + return 'skip' } - return - } - for (const nested of Object.values(value as Record)) { - collectEnvelopes(nested, depth + 1, out) - } + return 'descend' + }) } function getPositiveApplicationEvidence( @@ -109,7 +135,7 @@ function getPositiveApplicationEvidence( wholeFileContentByPath?: ReadonlyMap, ): ReadonlyMap | null { const envelopes: FileMutationResultV1[] = [] - collectEnvelopes(value, 0, envelopes) + collectEnvelopes(value, envelopes) if (envelopes.length === 0) return null const confirmedPaths = new Set() @@ -151,6 +177,8 @@ function getPositiveApplicationEvidence( endLine: record.endLine, contentHash: record.contentHash, readCapability, + projectId, + runId, } const existing = mergedAnchors.get(targetPath) if (!existing) { @@ -172,6 +200,11 @@ function getPositiveApplicationEvidence( if (!confirmedPaths.has(path)) return null } for (const [path, content] of wholeFileContentByPath ?? []) { + // Snapshots outside confirmationPaths are excluded no-ops: they may be + // present for sticky/anchor minting but must not demand a covering + // applied action. Requiring one would return null and undo confirmed + // paths that already have evidence. + if (!paths.has(path)) continue const expected = getExactContentHash(content) const covering = confirmedActions.filter( (action) => (action.destinationPath ?? action.path) === path, @@ -218,39 +251,76 @@ export function editOutputHasError( return hasExplicitError(output) } -function outputIndicatesStaleSnapshot(value: unknown, depth = 0): boolean { - if (depth > 6 || value === null || value === undefined) return false - if (Array.isArray(value)) { - return value.some((item) => outputIndicatesStaleSnapshot(item, depth + 1)) - } - if (typeof value === 'string') { - return /(?:stale\s+(?:snapshot|hash|range)|content\s+(?:changed|mismatch)|expected\s+hash)/i.test( - value, - ) - } - if (typeof value !== 'object') return false - return Object.values(value as Record).some((nested) => - outputIndicatesStaleSnapshot(nested, depth + 1), - ) +function isStructuredStaleCode(value: unknown): boolean { + return value === 'stale_snapshot' || value === 'stale_state' } -function outputIndicatesUnconfirmedApplication( - value: unknown, - depth = 0, -): boolean { - if (depth > 6 || value === null || value === undefined) return false - if (Array.isArray(value)) { - return value.some((item) => - outputIndicatesUnconfirmedApplication(item, depth + 1), - ) - } - if (typeof value === 'string') { - return /could not confirm/i.test(value) +function collectStaleSnapshotPaths(value: unknown): { + paths: Set + sawStructuredStale: boolean +} { + const out = { + paths: new Set(), + sawStructuredStale: false, } - if (typeof value !== 'object') return false - return Object.values(value as Record).some((nested) => - outputIndicatesUnconfirmedApplication(nested, depth + 1), + walkToolOutput( + value, + (node) => { + if (typeof node !== 'object') return 'skip' + const record = node as Record + const nestedError = + record.error !== null && + typeof record.error === 'object' && + !Array.isArray(record.error) + ? (record.error as Record) + : null + // Classify on this record when it carries a structured stale code, or when + // it owns `error: { code: 'stale_state' | 'stale_snapshot' }`, so a named + // action is never treated as nameless just because the nested error object + // has no path/file of its own. + if ( + isStructuredStaleCode(record.errorCode) || + isStructuredStaleCode(record.code) || + (nestedError !== null && isStructuredStaleCode(nestedError.code)) + ) { + out.sawStructuredStale = true + if (typeof record.path === 'string' && record.path) { + out.paths.add(record.path) + } else if (typeof record.file === 'string' && record.file) { + out.paths.add(record.file) + } + } + return 'descend' + }, + (node) => { + if (typeof node !== 'object' || node === null || Array.isArray(node)) { + return undefined + } + const record = node as Record + const next: unknown[] = [] + // failures[] items stay at the same depth as sibling fields (not one + // level deeper via the array node) so a shallow failures wrapper cannot + // push a structured hit past the walk bound. + if (Array.isArray(record.failures)) { + next.push(...record.failures) + } + for (const [key, nested] of Object.entries(record)) { + if (key === 'failures') continue + next.push(nested) + } + return next + }, ) + return out +} + +function outputIndicatesUnconfirmedApplication(value: unknown): boolean { + return walkToolOutput(value, (node) => { + if (typeof node === 'string' && /could not confirm/i.test(node)) { + return 'stop' + } + return typeof node === 'object' ? 'descend' : 'skip' + }) } export function invalidatePreparedEditPaths(params: { @@ -306,6 +376,8 @@ function synthesizePostEditAnchor(params: { hash: contentHash, scope, }), + projectId, + runId, } } @@ -357,22 +429,11 @@ export function commitAppliedEditPaths(params: { typeof wholeFileContent === 'string' && fileProcessingState.strictReadBeforeEdit ) { - // A confirmed apply with runtime-known content (e.g. a `create`, whose - // bytes are exactly edit.content, already stored in - // wholeFileContentByPath) grants sticky whole-file authorization - // straight from those bytes — no client anchor evidence required. The - // confirmedAnchor 7-point check still governs whether a client-echoed - // anchor is trusted. + // Sticky-from-confirmed-apply is granted iff a whole-file post-edit + // anchor can be minted (client-verified 7-point anchor or + // synthesizePostEditAnchor). Empty/non-authoritative scope does not + // grant or overwrite sticky maps and does not store an anchor. const contentHash = getContentHash(wholeFileContent) - fileProcessingState.readAuthorizationsByPath ??= {} - fileProcessingState.readAuthorizationHashesByPath ??= {} - fileProcessingState.readAuthorizationsByPath[path] = true - fileProcessingState.readAuthorizationHashesByPath[path] = contentHash - // Prefer the verified client-echoed anchor; otherwise mint one from the - // known content so a follow-up delete can be authorized and the - // capability can be surfaced for reuse. Skip minting when no - // authoritative scope is available (the sticky grant above still - // stands). const anchor = confirmedAnchorsByPath?.get(path) ?? synthesizePostEditAnchor({ @@ -382,6 +443,10 @@ export function commitAppliedEditPaths(params: { content: wholeFileContent, }) if (anchor) { + fileProcessingState.readAuthorizationsByPath ??= {} + fileProcessingState.readAuthorizationHashesByPath ??= {} + fileProcessingState.readAuthorizationsByPath[path] = true + fileProcessingState.readAuthorizationHashesByPath[path] = contentHash fileProcessingState.confirmedPostEditAnchorsByPath ??= {} fileProcessingState.confirmedPostEditAnchorsByPath[path] = anchor grantedAnchorsByPath.set(path, anchor) @@ -436,10 +501,23 @@ export async function coordinateEditApplication(params: { } if (editOutputHasError(output)) { - if (outputIndicatesStaleSnapshot(output)) { + const staleSnapshot = collectStaleSnapshotPaths(output) + if (staleSnapshot.sawStructuredStale) { + // Coordinated batches are all-or-nothing: drop prepared state for every + // path even when only a subset of structured failures is stale. invalidatePreparedEditPaths({ fileProcessingState: params.fileProcessingState, paths, + requiresFreshRead: false, + }) + // Revoke only paths named by structured stale hits. A top-level + // stale_snapshot with no per-path file/path and no stale failures[] + // fails closed onto every coordinated path. + const stalePaths = + staleSnapshot.paths.size > 0 ? [...staleSnapshot.paths] : paths + invalidatePreparedEditPaths({ + fileProcessingState: params.fileProcessingState, + paths: stalePaths, reason: 'stale_snapshot', sourceTool: params.toolName, }) diff --git a/packages/agent-runtime/src/tools/handlers/tool/edit-read-state.ts b/packages/agent-runtime/src/tools/handlers/tool/edit-read-state.ts index 92e8b7e9d8..74b73a1f77 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/edit-read-state.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/edit-read-state.ts @@ -38,14 +38,20 @@ export function markEditRequiresFreshRead(params: { delete fileProcessingState.readAuthorizationsByPath?.[path] delete fileProcessingState.readAuthorizationHashesByPath?.[path] delete fileProcessingState.modelVisibleReadAuthorizationHashesByPath?.[path] + delete fileProcessingState.confirmedPostEditAnchorsByPath?.[path] } } export function clearEditRereadRequirement( fileProcessingState: FileProcessingState, path: string, + options?: { clearContextCompacted?: boolean }, ): void { + const existing = fileProcessingState.editRereadRequirementsByPath?.[path] delete fileProcessingState.failedEditRequiresReadByPath[path] + if (existing?.reason === 'context_compacted' && !options?.clearContextCompacted) { + return + } delete fileProcessingState.editRereadRequirementsByPath?.[path] } @@ -133,11 +139,15 @@ export function strictEditAuthorizationError(params: { const scopeNote = wholeFileRequired ? ' A prior range-anchored edit or scoped range capability cannot authorize a whole-file overwrite.' : ' A scoped edit may instead provide the fresh capability/hash returned by read_files.' - // Fall back to the confirmed post-edit anchor's capability (from a create - // or edit earlier this session) only when the caller did not supply one. - const effectiveFreshReadCapability = - freshReadCapability ?? - fileProcessingState.confirmedPostEditAnchorsByPath?.[path]?.readCapability + // A post-edit cap minted under context_compacted must not be echoed as + // basedOnRead: that would let write_file clear compaction without a real + // whole-file read. Only a complete read_files grant (or an explicit + // basedOnRead the caller already holds from that grant) may clear it. + const echoPostEditCapability = prior?.reason !== 'context_compacted' + const effectiveFreshReadCapability = echoPostEditCapability + ? (freshReadCapability ?? + fileProcessingState.confirmedPostEditAnchorsByPath?.[path]?.readCapability) + : undefined // Prefer capability-retry when a whole-file token is already available; keep // read_files only as the secondary path when no capability can be echoed. // recovery.preferredStrategy is the machine-readable primary signal so diff --git a/packages/agent-runtime/src/tools/handlers/tool/edit-transaction.ts b/packages/agent-runtime/src/tools/handlers/tool/edit-transaction.ts index d5c04d9094..8ea2ef882d 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/edit-transaction.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/edit-transaction.ts @@ -533,7 +533,9 @@ export const handleEditTransaction = (async ( } freshWholeFileAuthorizationPaths.add(path) grantWholeFileReadAuthorization(fileProcessingState, path, content) - clearEditRereadRequirement(fileProcessingState, path) + clearEditRereadRequirement(fileProcessingState, path, { + clearContextCompacted: true, + }) const result: { ok: true } = { ok: true } authorizeWholeFileFromCapabilityCache.set(cacheKey, result) return result @@ -557,8 +559,8 @@ export const handleEditTransaction = (async ( if (isFresh) { freshWholeFileAuthorizationPaths.add(path) // Do not clear context_compacted on mere hash-fresh: write_file must stay - // blocked until a real re-read or explicit whole-file basedOnRead. str_replace - // clears the marker when it proceeds below (unique oldString is the safety bound). + // blocked until a complete whole-file read_files grant or explicit + // whole-file basedOnRead. Unique str_replace apply also must not drop it. const rereadReq = getEditRereadRequirement(fileProcessingState, path) if (rereadReq?.reason !== 'context_compacted') { clearEditRereadRequirement(fileProcessingState, path) @@ -645,10 +647,9 @@ export const handleEditTransaction = (async ( // Transaction-local only: do not call grantWholeFileReadAuthorization and // do not add to freshWholeFileAuthorizationPaths (would authorize write_file). // Do not clear reread markers here — auto-reread only authorizes this - // transaction's str_replace preflight. Markers (context_compacted / - // failed-edit) remain until successful non-allowMultiple apply - // (onApplied) so a failed unique/no-match still keeps write_file - // blocked under context_compacted. + // transaction's str_replace preflight. Failed-edit markers may drop on + // successful unique apply; context_compacted stays until a whole-file + // read_files grant or explicit whole-file basedOnRead. autoRereadAuthorizedPaths.add(path) } } @@ -741,9 +742,8 @@ export const handleEditTransaction = (async ( } if (freshWholeFileAuthorizationPaths.has(edit.path)) { // Hash-fresh sticky authorizes this edit, but do not clear - // context_compacted here for str_replace — only after successful apply - // in onApplied. write_file stays blocked above while the marker remains - // unless basedOnRead already cleared it. + // context_compacted here. write_file stays blocked while the marker + // remains unless basedOnRead already cleared it. if ( edit.type !== 'write_file' && edit.type !== 'str_replace' && @@ -1232,13 +1232,10 @@ export const handleEditTransaction = (async ( input: clientChanges.map(({ change }) => change), }), onApplied: () => { - // Paths that successfully applied a str_replace in this transaction may - // clear context_compacted (unique oldString safety bound, post-apply only). - // allowMultiple (replace-all) applies do NOT clear context_compacted: a - // blind global replace is not evidence the model knows the file content; - // only a unique-anchor apply or a fresh read clears the marker. - // write_file authorized via whole-file basedOnRead already cleared markers - // pre-apply and refreshes sticky from post-edit content below. + // Unique (non-allowMultiple) str_replace apply may drop failed-edit + // markers, but the helper preserves context_compacted. Only a complete + // whole-file read_files grant or explicit whole-file basedOnRead may + // clear that reason (write_file basedOnRead already did so pre-apply). const appliedNonAllowMultipleStrReplacePaths = new Set( edits .filter( diff --git a/packages/agent-runtime/src/tools/handlers/tool/read-files.ts b/packages/agent-runtime/src/tools/handlers/tool/read-files.ts index 1805a6f2a7..fccc8bf2e1 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/read-files.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/read-files.ts @@ -354,7 +354,9 @@ export const handleReadFiles = (async ( delete fileProcessingState.promisesByPath[path] continue } - clearEditRereadRequirement(fileProcessingState, path) + clearEditRereadRequirement(fileProcessingState, path, { + clearContextCompacted: true, + }) delete fileProcessingState.promisesByPath[path] } @@ -548,7 +550,9 @@ export const handleReadFiles = (async ( delete fileProcessingState.promisesByPath[path] continue } - clearEditRereadRequirement(fileProcessingState, path) + clearEditRereadRequirement(fileProcessingState, path, { + clearContextCompacted: true, + }) delete fileProcessingState.promisesByPath[path] } @@ -648,7 +652,9 @@ export const handleReadFiles = (async ( request.path, ) if (symbolRereadReq?.reason !== 'context_compacted') { - clearEditRereadRequirement(fileProcessingState, request.path) + clearEditRereadRequirement(fileProcessingState, request.path, { + clearContextCompacted: true, + }) } delete fileProcessingState.promisesByPath[request.path] const slicesTooLarge = diff --git a/packages/agent-runtime/src/tools/handlers/tool/str-replace.ts b/packages/agent-runtime/src/tools/handlers/tool/str-replace.ts index a6362da045..6c8c21703a 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/str-replace.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/str-replace.ts @@ -200,8 +200,7 @@ export const handleStrReplace = (async ( } if (typeof latestContent === 'string') { // In-process only: authorize this str_replace call; no durable sticky mint. - // Auto-reread runs only when there is no stored sticky, so clearing - // context_compacted here cannot open a hash-fresh write_file chain. + // The helper may drop failed-edit markers but keeps context_compacted. clearEditRereadRequirement(fileProcessingState, path) hadFreshWholeFileAuthorization = true } else { @@ -243,9 +242,8 @@ export const handleStrReplace = (async ( } // Hash-fresh authorization may clear prior reread markers for UX, but must - // preserve context_compacted until a successful unique apply (or a complete - // whole-file read_files grant). Otherwise a failed no-match attempt would - // drop the marker and let a later write_file overwrite on sticky alone. + // preserve context_compacted. Only a complete whole-file read_files grant + // or explicit whole-file basedOnRead may drop that marker. if (hadFreshWholeFileAuthorization) { const rereadReq = getEditRereadRequirement(fileProcessingState, path) if (rereadReq?.reason !== 'context_compacted') { @@ -433,8 +431,9 @@ export const handleStrReplace = (async ( ) { delete fileProcessingState.consecutiveStrReplaceFailuresByPath[path] } - // Clear context_compacted only after a successful unique apply. Unique - // oldString is the safety bound that authorizes dropping the marker. + // Unique apply may drop failed-edit markers via the helper, but must + // not clear context_compacted. Only a whole-file read_files grant or + // explicit whole-file basedOnRead may drop that reason. if ( !hadAutoCorrect && 'content' in strReplaceResult && diff --git a/packages/agent-runtime/src/tools/handlers/tool/write-file.ts b/packages/agent-runtime/src/tools/handlers/tool/write-file.ts index 64578b8b97..af45f665cf 100644 --- a/packages/agent-runtime/src/tools/handlers/tool/write-file.ts +++ b/packages/agent-runtime/src/tools/handlers/tool/write-file.ts @@ -30,6 +30,7 @@ import type { Logger } from '@codebuff/common/types/contracts/logger' import type { ParamsExcluding } from '@codebuff/common/types/function-params' import type { AgentState, + ConfirmedPostEditAnchor, EditRereadRequirement, } from '@codebuff/common/types/session-state' @@ -96,15 +97,7 @@ export type FileProcessingState = { // similar per-turn bounds. readAuthorizationsByPath?: Record readAuthorizationHashesByPath?: Record - confirmedPostEditAnchorsByPath?: Record< - string, - { - startLine: number - endLine: number - contentHash: string - readCapability: string - } - > + confirmedPostEditAnchorsByPath?: Record /** * Whole-file authorizations that were visible before the current model * generation started. When present, strict edit checks use this snapshot @@ -124,18 +117,22 @@ function getUsableWholeFileAuthorizationHash( >, path: string, ): string | undefined { + const stickyHash = + state.readAuthorizationsByPath?.[path] === true + ? state.readAuthorizationHashesByPath?.[path] + : undefined + // Prefer a confirmed post-edit hash only when it agrees with the sticky hash + // map. An unsigned/injected confirmed hash must not override sticky authority. if ( - state.readAuthorizationsByPath?.[path] === true && - state.confirmedPostEditAnchorsByPath?.[path] + typeof stickyHash === 'string' && + state.confirmedPostEditAnchorsByPath?.[path]?.contentHash === stickyHash ) { - return state.confirmedPostEditAnchorsByPath[path].contentHash + return stickyHash } if (state.modelVisibleReadAuthorizationHashesByPath !== undefined) { return state.modelVisibleReadAuthorizationHashesByPath[path] } - return state.readAuthorizationsByPath?.[path] === true - ? state.readAuthorizationHashesByPath?.[path] - : undefined + return stickyHash } export function hasWholeFileReadAuthorization( @@ -413,7 +410,9 @@ export const handleWriteFile = (async ( // Explicit whole-file capability is the real re-read substitute, including // clearing context_compacted without an exploratory auto-reread. grantWholeFileReadAuthorization(fileProcessingState, path, fileContent) - clearEditRereadRequirement(fileProcessingState, path) + clearEditRereadRequirement(fileProcessingState, path, { + clearContextCompacted: true, + }) return { ok: true } } diff --git a/packages/agent-runtime/src/tools/stream-parser.ts b/packages/agent-runtime/src/tools/stream-parser.ts index da6e6a382d..40442705f9 100644 --- a/packages/agent-runtime/src/tools/stream-parser.ts +++ b/packages/agent-runtime/src/tools/stream-parser.ts @@ -13,6 +13,7 @@ import { executeToolCall, tryTransformAgentToolCall, } from './tool-executor' +import { remintConfirmedPostEditAnchors } from '../util/read-authorization' import { withSystemTags } from '../util/messages' import { normalizeToolPath } from './handlers/tool/write-file' @@ -234,6 +235,11 @@ export async function processStream( readAuthorizationHashesByPath: { ...(agentState.readAuthorizationHashesByPath ?? {}), }, + confirmedPostEditAnchorsByPath: remintConfirmedPostEditAnchors({ + anchors: agentState.confirmedPostEditAnchorsByPath, + projectId: fileContext.projectRoot ?? '', + runId, + }), // Only authorizations already present before this provider generation are // epistemically usable by edit arguments authored in this response. A // read_files call emitted earlier in the same response still executes @@ -628,6 +634,9 @@ export async function processStream( agentState.readAuthorizationHashesByPath = { ...(fileProcessingState.readAuthorizationHashesByPath ?? {}), } + agentState.confirmedPostEditAnchorsByPath = { + ...(fileProcessingState.confirmedPostEditAnchorsByPath ?? {}), + } agentState.editRereadRequirementsByPath = { ...(fileProcessingState.editRereadRequirementsByPath ?? {}), } @@ -638,6 +647,7 @@ export async function processStream( // reads/index evidence as belonging to the current workspace state. agentState.readAuthorizationsByPath = {} agentState.readAuthorizationHashesByPath = {} + agentState.confirmedPostEditAnchorsByPath = {} agentState.workspaceState = advanceWorkspaceState( agentState.workspaceState, { diff --git a/packages/agent-runtime/src/util/__tests__/read-authorization.test.ts b/packages/agent-runtime/src/util/__tests__/read-authorization.test.ts index f9bfb814b5..82b5cf6d93 100644 --- a/packages/agent-runtime/src/util/__tests__/read-authorization.test.ts +++ b/packages/agent-runtime/src/util/__tests__/read-authorization.test.ts @@ -1,17 +1,41 @@ import { describe, expect, it } from 'bun:test' -import { revokeImplicitReadAuthorizationsAfterCompaction } from '../read-authorization' +import { + remintConfirmedPostEditAnchors, + revokeImplicitReadAuthorizationsAfterCompaction, +} from '../read-authorization' + +import { + decodeReadCapabilityToken, + encodeReadCapabilityToken, + getContentHash, +} from '@codebuff/common/util/content-hash' import type { AgentState } from '@codebuff/common/types/session-state' describe('revokeImplicitReadAuthorizationsAfterCompaction', () => { it('keeps sticky whole-file authority and records a typed reread reason', () => { + const contentHash = getContentHash('export const a = 1\n') + const readCapability = encodeReadCapabilityToken({ + startLine: 1, + endLine: 1, + hash: contentHash, + scope: { projectId: '/project', path: 'src/a.ts', runId: 'run' }, + }) const state = { readAuthorizationsByPath: { 'src/a.ts': true }, readAuthorizationHashesByPath: { 'src/a.ts': 'sha256:a', 'src/hash-only.ts': 'sha256:b', }, + confirmedPostEditAnchorsByPath: { + 'src/a.ts': { + startLine: 1, + endLine: 1, + contentHash, + readCapability, + }, + }, editRereadRequirementsByPath: { 'src/existing.ts': { reason: 'stale_snapshot', @@ -28,6 +52,14 @@ describe('revokeImplicitReadAuthorizationsAfterCompaction', () => { 'src/a.ts': 'sha256:a', 'src/hash-only.ts': 'sha256:b', }) + expect(state.confirmedPostEditAnchorsByPath).toEqual({ + 'src/a.ts': { + startLine: 1, + endLine: 1, + contentHash, + readCapability, + }, + }) expect(state.editRereadRequirementsByPath).toEqual({ 'src/a.ts': { reason: 'context_compacted', @@ -45,3 +77,185 @@ describe('revokeImplicitReadAuthorizationsAfterCompaction', () => { }) }) }) + +describe('remintConfirmedPostEditAnchors', () => { + const contentHash = getContentHash('export const value = 1\n') + const sameScopeCapability = encodeReadCapabilityToken({ + startLine: 1, + endLine: 2, + hash: contentHash, + scope: { projectId: '/project', path: 'src/a.ts', runId: 'run' }, + }) + const crossScopeCapability = encodeReadCapabilityToken({ + startLine: 1, + endLine: 2, + hash: contentHash, + scope: { projectId: '/old-project', path: 'src/a.ts', runId: 'old-run' }, + }) + const wellFormedSameScope = { + startLine: 1, + endLine: 2, + contentHash, + readCapability: sameScopeCapability, + } + const wellFormedOkPathScope = { + startLine: 1, + endLine: 2, + contentHash, + readCapability: encodeReadCapabilityToken({ + startLine: 1, + endLine: 2, + hash: contentHash, + scope: { projectId: '/project', path: 'src/ok.ts', runId: 'run' }, + }), + } + const wellFormedCrossScope = { + startLine: 1, + endLine: 2, + contentHash, + readCapability: crossScopeCapability, + } + + it('remints cap.v3 when stored token authenticates for the same project/run', () => { + const reminted = remintConfirmedPostEditAnchors({ + anchors: { 'src/a.ts': wellFormedSameScope }, + projectId: '/project', + runId: 'run', + }) + + expect(reminted['src/a.ts']?.startLine).toBe(1) + expect(reminted['src/a.ts']?.endLine).toBe(2) + expect(reminted['src/a.ts']?.contentHash).toBe(contentHash) + expect(reminted['src/a.ts']?.projectId).toBe('/project') + expect(reminted['src/a.ts']?.runId).toBe('run') + const decoded = decodeReadCapabilityToken( + reminted['src/a.ts']!.readCapability, + ) + expect(typeof decoded).not.toBe('string') + if (typeof decoded !== 'string') { + expect(decoded.hash).toBe(contentHash) + expect(decoded.startLine).toBe(1) + expect(decoded.endLine).toBe(2) + } + }) + + it('remints from stamped issuer when stored token no longer authenticates (restart path)', () => { + const reminted = remintConfirmedPostEditAnchors({ + anchors: { + 'src/a.ts': { + startLine: 1, + endLine: 2, + contentHash, + // Unauthenticated after process restart (HMAC key rotated). + readCapability: 'cap.v3.1.2.invalid-token-payload-for-restart', + projectId: '/project', + runId: 'run', + }, + }, + projectId: '/project', + runId: 'run', + }) + + expect(reminted['src/a.ts']?.contentHash).toBe(contentHash) + expect(reminted['src/a.ts']?.projectId).toBe('/project') + expect(reminted['src/a.ts']?.runId).toBe('run') + const decoded = decodeReadCapabilityToken( + reminted['src/a.ts']!.readCapability, + ) + expect(typeof decoded).not.toBe('string') + }) + + it('drops cross-project/cross-run remint even with well-formed hash/bounds', () => { + const reminted = remintConfirmedPostEditAnchors({ + anchors: { + 'src/a.ts': wellFormedCrossScope, + 'src/stamped.ts': { + ...wellFormedCrossScope, + projectId: '/old-project', + runId: 'old-run', + }, + }, + projectId: '/project', + runId: 'run', + }) + + expect(reminted).toEqual({}) + }) + + it('drops malformed entries without throwing', () => { + const reminted = remintConfirmedPostEditAnchors({ + anchors: { + '': wellFormedSameScope, + 'src/bad-start.ts': { ...wellFormedSameScope, startLine: 2 }, + 'src/bad-end.ts': { ...wellFormedSameScope, endLine: 0 }, + 'src/bad-hash.ts': { ...wellFormedSameScope, contentHash: 'not-a-hash' }, + 'src/missing-cap.ts': { + startLine: 1, + endLine: 2, + contentHash, + readCapability: '', + }, + 'src/ok.ts': wellFormedOkPathScope, + }, + projectId: '/project', + runId: 'run', + }) + + expect(Object.keys(reminted)).toEqual(['src/ok.ts']) + }) + + it('drops well-formed entries when scope is empty (no unauthenticated keep)', () => { + const reminted = remintConfirmedPostEditAnchors({ + anchors: { 'src/a.ts': wellFormedSameScope }, + projectId: '', + runId: '', + }) + + expect(reminted).toEqual({}) + }) + + it('drops empty-scope entries that are not well-formed objects', () => { + const reminted = remintConfirmedPostEditAnchors({ + anchors: { + 'src/a.ts': { + startLine: 1, + endLine: 2, + contentHash, + readCapability: '', + }, + }, + projectId: '', + runId: '', + }) + + expect(reminted).toEqual({}) + }) + + it('drops hostile and non-canonical path keys without assigning them', () => { + const anchors: Record = { + constructor: wellFormedSameScope, + prototype: wellFormedSameScope, + '../escape.ts': wellFormedSameScope, + '/abs.ts': wellFormedSameScope, + 'src/ok.ts': wellFormedOkPathScope, + } + // Object-literal `__proto__` sets the prototype; define an own key instead. + Object.defineProperty(anchors, '__proto__', { + value: wellFormedSameScope, + enumerable: true, + configurable: true, + writable: true, + }) + + const reminted = remintConfirmedPostEditAnchors({ + anchors, + projectId: '/project', + runId: 'run', + }) + + expect(Object.keys(reminted)).toEqual(['src/ok.ts']) + expect(Object.prototype.hasOwnProperty.call(reminted, '__proto__')).toBe( + false, + ) + }) +}) diff --git a/packages/agent-runtime/src/util/read-authorization.ts b/packages/agent-runtime/src/util/read-authorization.ts index 09d02e8efa..c59e734091 100644 --- a/packages/agent-runtime/src/util/read-authorization.ts +++ b/packages/agent-runtime/src/util/read-authorization.ts @@ -1,10 +1,112 @@ -import type { AgentState } from '@codebuff/common/types/session-state' +import { + decodeReadCapabilityToken, + encodeReadCapabilityToken, + hasAuthoritativeReadCapabilityScope, + readCapabilityMatchesScope, +} from '@codebuff/common/util/content-hash' + +import { normalizeToolPath } from '../tools/handlers/tool/write-file' + +import type { + AgentState, + ConfirmedPostEditAnchor, +} from '@codebuff/common/types/session-state' + +const CONTENT_HASH_PATTERN = /^sha256:[a-f0-9]{64}$/ +const HOSTILE_ANCHOR_KEYS = new Set(['__proto__', 'constructor', 'prototype']) + +function isPositiveInteger(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0 +} + +function isWellFormedStoredAnchor( + value: unknown, +): value is ConfirmedPostEditAnchor { + if (value === null || typeof value !== 'object') return false + const anchor = value as Partial + return ( + anchor.startLine === 1 && + isPositiveInteger(anchor.endLine) && + typeof anchor.contentHash === 'string' && + CONTENT_HASH_PATTERN.test(anchor.contentHash) && + typeof anchor.readCapability === 'string' && + anchor.readCapability.length > 0 + ) +} + +function isSafeAnchorPath(path: string): boolean { + if (typeof path !== 'string' || path.length === 0) return false + if (HOSTILE_ANCHOR_KEYS.has(path)) return false + // Reject traversal / absolute / non-canonical keys; never assign hostile keys. + return normalizeToolPath(path) === path +} + +/** + * Remint durable confirmed post-edit anchors only when issuer-bound to the + * current project/run. cap.v3 is reminted when the stored token authenticates + * for that scope, or when stamped projectId+runId match (process-restart path). + * Unauthenticated / cross-scope / malformed / hostile entries are dropped. + */ +export function remintConfirmedPostEditAnchors(params: { + anchors: Record | undefined + projectId: string + runId: string +}): Record { + const { anchors, projectId, runId } = params + const result: Record = {} + if (!anchors) return result + + for (const [path, stored] of Object.entries(anchors)) { + if (!isSafeAnchorPath(path)) continue + if (!isWellFormedStoredAnchor(stored)) continue + + const scope = { projectId, path, runId } + if (!hasAuthoritativeReadCapabilityScope(scope)) continue + + const decoded = decodeReadCapabilityToken(stored.readCapability) + const tokenAuthenticatesForScope = + typeof decoded !== 'string' && + readCapabilityMatchesScope(decoded, scope) && + decoded.startLine === stored.startLine && + decoded.endLine === stored.endLine && + decoded.hash === stored.contentHash + + const issuerMatchesCurrent = + typeof stored.projectId === 'string' && + stored.projectId.length > 0 && + typeof stored.runId === 'string' && + stored.runId.length > 0 && + stored.projectId === projectId && + stored.runId === runId + + // Path 3: live HMAC authenticates for current scope. + // Path 4: process restart — in-process HMAC dies, but stamped issuer matches. + if (!tokenAuthenticatesForScope && !issuerMatchesCurrent) continue + + result[path] = { + startLine: stored.startLine, + endLine: stored.endLine, + contentHash: stored.contentHash, + readCapability: encodeReadCapabilityToken({ + startLine: stored.startLine, + endLine: stored.endLine, + hash: stored.contentHash, + scope, + }), + projectId, + runId, + } + } + + return result +} /** * After context compaction removes exact read bodies from model-visible context, * record a typed reread reason for telemetry/guidance — but keep sticky whole-file * authorizations and hashes. Edit-time `isWholeFileReadAuthorizationFresh` still * fails closed when disk content has drifted from the stored hash. + * Confirmed post-edit anchors are also kept (same durability as sticky hashes). */ export function revokeImplicitReadAuthorizationsAfterCompaction( agentState: AgentState, @@ -22,5 +124,6 @@ export function revokeImplicitReadAuthorizationsAfterCompaction( sourceTool: 'context compaction', } } - // Sticky maps intentionally preserved: hash freshness is enforced at edit time. + // Sticky maps and confirmed post-edit anchors intentionally preserved: + // hash freshness is enforced at edit time. } diff --git a/sdk/src/run.ts b/sdk/src/run.ts index 85b10ebf57..1f3e7c74a6 100644 --- a/sdk/src/run.ts +++ b/sdk/src/run.ts @@ -567,6 +567,7 @@ async function runOnce({ sessionState.mainAgentState.workspaceState = persistedWorkspace sessionState.mainAgentState.readAuthorizationsByPath = {} sessionState.mainAgentState.readAuthorizationHashesByPath = {} + sessionState.mainAgentState.confirmedPostEditAnchorsByPath = {} } } catch (error) { logger?.warn(