diff --git a/apps/desktop/e2e/floating-layers.spec.ts b/apps/desktop/e2e/floating-layers.spec.ts index 55ccfe4b29..1cd45d6daa 100644 --- a/apps/desktop/e2e/floating-layers.spec.ts +++ b/apps/desktop/e2e/floating-layers.spec.ts @@ -1,4 +1,114 @@ -import { expect, test, COMPOSER_INPUT } from './fixtures.js'; +import { expect, test } from './fixtures.js'; + +/** + * #1565 PR 5 — Astryx owns Tooltip and Popover behavior. These journeys lock + * the public user contract: surfaces open, dismiss, and restore focus without + * Maka inspecting Astryx's native layer implementation. + */ + +test('tooltip opens on hover and dismisses on Escape', async ({ + window: page, +}) => { + const trigger = page.getByRole('button', { name: '搜索对话' }); + await expect(trigger).toBeVisible(); + + await trigger.hover(); + const tooltip = page.getByRole('tooltip'); + await expect(tooltip).toBeVisible(); + await expect(tooltip).toContainText('搜索对话'); + + // WCAG 1.4.13: hover content must be dismissible without moving the pointer, + // and an Escape-dismissed tooltip must not reappear until the pointer leaves + // and re-enters. Wait past the 200ms Astryx show delay before concluding — + // an immediate assertion would pass vacuously while a re-show is pending. + await page.keyboard.press('Escape'); + await expect(tooltip).toBeHidden(); + await page.waitForTimeout(350); + await expect(tooltip).toBeHidden(); + + // And it must not linger once the pointer leaves. + await page.mouse.move(10, 300); + await trigger.hover(); + await expect(tooltip).toBeVisible(); + await page.mouse.move(10, 300); + await expect(tooltip).toBeHidden(); +}); + +test('daily review uses the canonical time field and persists its value', async ({ + window: page, +}) => { + await page.getByRole('button', { name: '展开侧边栏' }).click(); + await page.getByRole('button', { name: '设置' }).click(); + const settingsNavigation = page.getByRole('navigation', { name: '设置分组' }); + const settings = page.getByRole('main', { name: '设置内容' }); + await settingsNavigation.getByRole('button', { name: '每日回顾', exact: true }).click(); + + const time = settings.getByRole('textbox', { name: '每日回顾执行时间' }); + await expect(time).toHaveValue('08:00'); + await time.fill('08:05'); + await time.blur(); + await expect(time).toHaveValue('08:05'); + + await settingsNavigation.getByRole('button', { name: '通用', exact: true }).click(); + await settingsNavigation.getByRole('button', { name: '每日回顾', exact: true }).click(); + const persistedTime = settings.getByRole('textbox', { + name: '每日回顾执行时间', + }); + await expect(persistedTime).toHaveValue('08:05'); + + await persistedTime.fill('24:00'); + await persistedTime.blur(); + await expect(persistedTime).toHaveAttribute('aria-invalid', 'true'); + await expect( + settings.getByText('请输入 24 小时制时间,例如 08:00。'), + ).toBeVisible(); + + await settingsNavigation.getByRole('button', { name: '通用', exact: true }).click(); + await settingsNavigation.getByRole('button', { name: '每日回顾', exact: true }).click(); + await expect( + settings.getByRole('textbox', { name: '每日回顾执行时间' }), + ).toHaveValue('08:05'); +}); + +test('model picker only exposes a rendered active descendant', async ({ + window: page, +}) => { + await page.getByRole('button', { name: /选择新对话模型/ }).click(); + + const search = page.getByPlaceholder('搜索模型'); + await expect(search).toBeFocused(); + + await search.fill('no-such-model'); + await expect(page.getByRole('listbox').getByText('No results found')).toBeVisible(); + await expect(search).not.toHaveAttribute('aria-activedescendant'); + + await search.fill('sonnet'); + await search.press('ArrowDown'); + const activeDescendant = await search.getAttribute('aria-activedescendant'); + expect(activeDescendant).not.toBeNull(); + await expect(page.locator(`#${activeDescendant}`)).toHaveRole('option'); +}); + +// Model-picker mark geometry / label ellipsis: CSS contract +// (chat-shell-layout-contract). Listbox scroll-into-view is Astryx-owned. +// Keep focus restore + real session model/thinking persistence below. + +test('model Selector restores focus to its opener on Escape', async ({ + window: page, +}) => { + const trigger = page.getByRole('button', { name: /选择新对话模型/ }); + await trigger.click(); + + const search = page.getByPlaceholder('搜索模型'); + const listbox = page.getByRole('listbox'); + const popup = listbox.locator('xpath=ancestor::*[@popover][1]'); + await expect(search).toBeFocused(); + await expect(listbox.getByRole('option').first()).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(popup).toBeHidden(); + await expect(trigger).toBeFocused(); +}); test('model and adjacent thinking Selectors persist one real Electron journey', async ({ modelPickerLongWindow: page, @@ -8,7 +118,7 @@ test('model and adjacent thinking Selectors persist one real Electron journey', await page.getByRole('option', { name: '关', exact: true }).click(); await expect(thinkingTrigger).toContainText('关'); - const composer = page.locator(COMPOSER_INPUT); + const composer = page.locator('.maka-composer-textarea'); await composer.fill('model selector persistence journey'); await composer.press('Enter'); await expect( @@ -18,7 +128,7 @@ test('model and adjacent thinking Selectors persist one real Electron journey', const activeThinkingTrigger = page.getByRole('combobox', { name: '思考级别' }); await expect(activeThinkingTrigger).toContainText('关'); await page.reload(); - await expect(page.locator(COMPOSER_INPUT)).toBeVisible(); + await expect(page.locator('.maka-composer-textarea')).toBeVisible(); await expect(page.getByRole('combobox', { name: '思考级别' })).toContainText('关'); const modelTrigger = page.getByRole('button', { name: '切换当前会话模型' }); @@ -30,11 +140,89 @@ test('model and adjacent thinking Selectors persist one real Electron journey', await expect(modelTrigger).toContainText('claude-e2e-1'); }); +test('keyboard help keeps its shortcut grid and lets Astryx restore its opener on close', async ({ + window: page, +}) => { + const opener = page.getByRole('button', { name: '搜索对话' }); + await opener.focus(); + await page.keyboard.press('Control+/'); + + const dialog = page.getByRole('dialog', { name: '键盘快捷键' }); + await expect(dialog).toBeVisible(); + + const helpBody = dialog.locator('.maka-help-body'); + const firstSection = helpBody.locator('.maka-help-section').first(); + const firstShortcutList = firstSection.locator('dl'); + await expect(helpBody).toHaveCSS('column-count', '2'); + await expect(firstShortcutList).toHaveCSS('display', 'grid'); + await expect(firstShortcutList).toHaveCSS( + 'grid-template-columns', + /\d+(\.\d+)?px \d+(\.\d+)?px/, + ); + await expect(firstSection.locator('dd').first()).toHaveCSS( + 'justify-self', + 'end', + ); + await expect(dialog).toHaveCSS('border-radius', '16px'); + await expect(dialog).not.toHaveCSS('box-shadow', 'none'); + await expect(firstSection).toHaveCSS('border-radius', '12px'); + await expect( + dialog.getByRole('heading', { name: '键盘快捷键' }), + ).toHaveCSS('box-shadow', 'none'); + await expect( + dialog.getByRole('img', { name: /(Command|Control) \+ K/ }), + ).toBeVisible(); + await expect( + dialog.getByRole('img', { name: 'Left arrow' }).first(), + ).toBeVisible(); + await expect( + dialog.getByRole('img', { name: 'Right arrow' }).first(), + ).toBeVisible(); + await page.setViewportSize({ width: 640, height: 800 }); + await expect(helpBody).toHaveCSS('column-count', '1'); + expect((await dialog.boundingBox())?.width).toBeLessThanOrEqual(608); + + await page.keyboard.press('Escape'); + + await expect(dialog).toBeHidden(); + await expect(opener).toBeFocused(); +}); + +test('a title-only search result restores focus to the opener', async ({ + sidebarLongSessionsWindow: page, +}) => { + const opener = page.getByRole('button', { name: '搜索对话' }); + await opener.click(); + const dialog = page.getByRole('dialog', { name: '搜索' }); + await dialog + .getByRole('combobox', { name: '搜索会话' }) + .fill('会话 01'); + await dialog.getByRole('option', { name: /会话 01/ }).click(); + + await expect(dialog).toBeHidden(); + await expect(page.getByText('示例对话 01')).toBeVisible(); + await expect(opener).toBeFocused(); +}); + +test('search dialog lets Astryx restore its opener on ordinary close', async ({ + window: page, +}) => { + const opener = page.getByRole('button', { name: '搜索对话' }); + await opener.click(); + + const dialog = page.getByRole('dialog', { name: '搜索' }); + await expect(dialog).toBeVisible(); + await page.keyboard.press('Escape'); + + await expect(dialog).toBeHidden(); + await expect(opener).toBeFocused(); +}); + test('search closes before navigating and focusing the matched turn', async ({ window: page, }) => { const needle = 'search ownership needle 7319'; - const composer = page.locator(COMPOSER_INPUT); + const composer = page.locator('.maka-composer-textarea'); await composer.fill(needle); await composer.press('Enter'); await expect(page.getByText(`Fake backend received: ${needle}`)).toBeVisible(); diff --git a/apps/desktop/src/main/__tests__/create-session-input.test.ts b/apps/desktop/src/main/__tests__/create-session-input.test.ts index 60f49c423a..496da9c602 100644 --- a/apps/desktop/src/main/__tests__/create-session-input.test.ts +++ b/apps/desktop/src/main/__tests__/create-session-input.test.ts @@ -10,7 +10,11 @@ import { describe, it } from 'node:test'; import type { AppSettings, ChatDefaultPermissionMode } from '@maka/core'; import { DEEP_RESEARCH_SESSION_LABEL, DEFAULT_SESSION_NAME } from '@maka/core'; -import { type CreateSessionRequest, resolveCreateSessionInput } from '../create-session-input.js'; +import { + type CreateSessionRequest, + resolveCreateSessionInput, + resolveEditingProtocolEnv, +} from '../create-session-input.js'; function settings(permissionMode: ChatDefaultPermissionMode) { return async () => ({ chatDefaults: { permissionMode } }) as AppSettings; @@ -126,4 +130,27 @@ describe('resolveCreateSessionInput', () => { assert.equal(resolved.collaborationMode, 'plan'); assert.equal(resolved.orchestrationMode, 'swarm'); }); + + it('normalizes the editing protocol per session request', async () => { + assert.equal((await resolve({ editingProtocol: 'apply_patch' })).editingProtocol, 'apply_patch'); + assert.equal((await resolve({})).editingProtocol, 'edit_write'); + await assert.rejects(() => resolve({ editingProtocol: 'all' }), TypeError); + }); + + it('uses the process setting only as the default for an individual session', async () => { + const readSettings = settings('ask'); + const configured = await resolveCreateSessionInput(undefined, { + readSettings, + defaultEditingProtocol: 'apply_patch', + }); + const overridden = await resolveCreateSessionInput( + { editingProtocol: 'edit_write' }, + { readSettings, defaultEditingProtocol: 'apply_patch' }, + ); + + assert.equal(configured.editingProtocol, 'apply_patch'); + assert.equal(overridden.editingProtocol, 'edit_write'); + assert.equal(resolveEditingProtocolEnv('apply_patch'), 'apply_patch'); + assert.throws(() => resolveEditingProtocolEnv('all')); + }); }); diff --git a/apps/desktop/src/main/__tests__/desktop-backend-tool-surface.test.ts b/apps/desktop/src/main/__tests__/desktop-backend-tool-surface.test.ts index 41b1a2ba6b..2263a3d6ae 100644 --- a/apps/desktop/src/main/__tests__/desktop-backend-tool-surface.test.ts +++ b/apps/desktop/src/main/__tests__/desktop-backend-tool-surface.test.ts @@ -22,6 +22,7 @@ import { const readTool = tool('Read', 'read'); const writeTool = tool('Write', 'file_write'); +const applyPatchTool = tool('ApplyPatch', 'file_write'); const computerTool = tool('maka_computer', 'computer_use'); const availability: ToolAvailabilityConfig = { economy: true, @@ -135,6 +136,25 @@ describe('Desktop backend tool surface', () => { assert.equal(surface.skillHost.toolNames.has('Write'), true); }); + it('projects ApplyPatch through the standard Desktop backend policy', async () => { + const surface = await resolveDesktopBackendToolSurface( + makeDeps({ + builtinTools: [readTool, writeTool, applyPatchTool], + }), + { + ...inputFor('claude-sonnet-4-5-20250929'), + header: { + ...inputFor('claude-sonnet-4-5-20250929').header, + editingProtocol: 'apply_patch', + }, + }, + ); + + assert.equal(surface.skillHost.toolNames.has('ApplyPatch'), true); + assert.equal(surface.skillHost.toolNames.has('Write'), false); + assert.equal(surface.selectedTools.some((tool) => tool.name === 'Edit'), false); + }); + it('keeps scoped child tools ahead of root-only computer-use and Plan controls', async () => { const deps = makeDeps({ isComputerUseRealModelE2e: true }); const input = inputFor('claude-sonnet-4-5-20250929', 'plan'); diff --git a/apps/desktop/src/main/boot.ts b/apps/desktop/src/main/boot.ts index 49f404d4e5..3595de53f1 100644 --- a/apps/desktop/src/main/boot.ts +++ b/apps/desktop/src/main/boot.ts @@ -49,6 +49,7 @@ import { SessionActivityRegistry, listInvocableSkills, prepareSkillInvocationMessage, + projectEffectiveProductToolSurface, resolveSkillDiscoveryPaths, } from '@maka/runtime'; import type { @@ -85,6 +86,7 @@ import { requireReadyConnection, } from './chat-readiness.js'; import { assertDesktopExecutionBoundary } from './desktop-execution-admission.js'; +import { resolveEditingProtocolEnv } from './create-session-input.js'; import { createFileCredentialStore } from './credential-store.js'; import { bindOnboardingDeps, createOnboardingService } from './onboarding-service.js'; import { createDailyReviewArchiveStore } from './daily-review-archive-store.js'; @@ -886,7 +888,17 @@ const runtime = new SessionManager({ inspectContinuationSafety: createLocalContinuationSafetyInspector({ readSessionCwd: async (sessionId) => (await store.readHeader(sessionId)).cwd, resolveWorkspaceIdentity: async (cwd) => resolveWorkspaceIdentity({ path: cwd }), - listAvailableToolNames: async () => builtinTools.map((tool) => tool.name), + listAvailableToolNames: async (sessionId) => { + const header = await store.readHeader(sessionId); + return projectEffectiveProductToolSurface({ + host: 'desktop', + tools: builtinTools, + policy: { + ...desktopProductToolSurface.identity.policy, + editingProtocol: header.editingProtocol ?? 'edit_write', + }, + }).tools.map((tool) => tool.name); + }, hasPendingBackgroundOperations: async (sessionId) => { const [shellUpdates, runs] = await Promise.all([ shellRuns.listSessionUpdates(sessionId), @@ -1127,6 +1139,7 @@ function registerIpc(): void { streamEvents, getWorkspacePrivacyContext, canCreateFakeSession: canCreateFakeSessionFromRenderer, + defaultEditingProtocol: resolveEditingProtocolEnv(process.env.MAKA_EDITING_PROTOCOL), consumeNativeAudioOperation: (input) => voiceIpcService.consumeNativeAudioOperation(input), }); diff --git a/apps/desktop/src/main/create-session-input.ts b/apps/desktop/src/main/create-session-input.ts index e68a1a74f3..1bf03ff84c 100644 --- a/apps/desktop/src/main/create-session-input.ts +++ b/apps/desktop/src/main/create-session-input.ts @@ -19,6 +19,7 @@ import type { AppSettings, CollaborationMode, + EditingProtocol, OrchestrationMode, PermissionMode, SessionStartMode, @@ -28,7 +29,9 @@ import { DEFAULT_SESSION_NAME, isChatDefaultPermissionMode, isCollaborationMode, + isEditingProtocol, isOrchestrationMode, + resolveEditingProtocolEnv as resolveEditingProtocolEnvValue, } from '@maka/core'; import { resolveDefaultPermissionMode } from './permission-mode-default.js'; @@ -61,6 +64,12 @@ const SESSION_MODE_SEEDS = { }, } satisfies Record; +export function resolveEditingProtocolEnv(value: string | undefined): EditingProtocol { + // Desktop sessions default to Edit/Write; an explicit env override is the + // only way to reach apply_patch. + return resolveEditingProtocolEnvValue(value) ?? 'edit_write'; +} + /** * `unknown`, because this is an IPC boundary and the renderer's type is a * promise, not a guarantee. An unrecognized value confers nothing — it is not @@ -82,6 +91,7 @@ export interface CreateSessionRequest { permissionMode?: PermissionMode; collaborationMode?: CollaborationMode; orchestrationMode?: OrchestrationMode; + editingProtocol?: EditingProtocol; name?: string; labels?: string[]; } @@ -90,13 +100,17 @@ export interface ResolvedCreateSessionInput { permissionMode: PermissionMode; collaborationMode: CollaborationMode; orchestrationMode: OrchestrationMode; + editingProtocol: EditingProtocol; name: string; labels: string[] | undefined; } export async function resolveCreateSessionInput( input: CreateSessionRequest | undefined, - deps: { readSettings: () => Promise }, + deps: { + readSettings: () => Promise; + defaultEditingProtocol?: EditingProtocol; + }, ): Promise { const modeSeed = sessionModeSeed(input?.mode); @@ -108,6 +122,10 @@ export async function resolveCreateSessionInput( if (!isOrchestrationMode(orchestrationMode)) { throw new TypeError('Invalid orchestration mode.'); } + const editingProtocol = input?.editingProtocol ?? deps.defaultEditingProtocol ?? 'edit_write'; + if (!isEditingProtocol(editingProtocol)) { + throw new TypeError('Invalid editing protocol.'); + } // `explore` is a boundary a mode confers, never one a caller may open a // session at — core already spells that out as `ChatDefaultPermissionMode` // (the modes a user can pick). Refusing it here is what makes the seed the @@ -124,6 +142,7 @@ export async function resolveCreateSessionInput( (await resolveDefaultPermissionMode(deps.readSettings)), collaborationMode, orchestrationMode, + editingProtocol, name: modeSeed?.name ?? input?.name ?? DEFAULT_SESSION_NAME, // Merged, not replaced: a mode adds a label, it does not own the set. No // caller sends both today, and silently dropping the caller's would be the diff --git a/apps/desktop/src/main/desktop-backend-tool-surface.ts b/apps/desktop/src/main/desktop-backend-tool-surface.ts index 9b8c2edd0f..df80664d62 100644 --- a/apps/desktop/src/main/desktop-backend-tool-surface.ts +++ b/apps/desktop/src/main/desktop-backend-tool-surface.ts @@ -22,6 +22,7 @@ import { selectCollaborationTools, } from '@maka/runtime'; import type { + EditingProtocol, HostCapabilities, MakaTool, ToolAvailabilityConfig, @@ -79,6 +80,7 @@ export interface DesktopBackendToolSurface { export interface DesktopNewSessionSkillContext { collaborationMode?: CollaborationMode; + editingProtocol?: EditingProtocol; } /** @@ -141,6 +143,7 @@ export async function resolveDesktopNewSessionSkillHost( connectionLocked: false, model: input.readyConnection.model, permissionMode: 'ask', + editingProtocol: input.context?.editingProtocol ?? 'edit_write', collaborationMode: input.context?.collaborationMode ?? 'agent', orchestrationMode: 'default', schemaVersion: 1, @@ -230,7 +233,10 @@ export async function resolveDesktopBackendToolSurface( const productToolSurface = projectEffectiveProductToolSurface({ host: 'desktop', tools: selectedTools, - policy: { economy: toolEconomy }, + policy: { + economy: toolEconomy, + editingProtocol: input.header.editingProtocol ?? 'edit_write', + }, }); return { diff --git a/apps/desktop/src/main/sessions-ipc-main.ts b/apps/desktop/src/main/sessions-ipc-main.ts index e5d194866d..f5902a673d 100644 --- a/apps/desktop/src/main/sessions-ipc-main.ts +++ b/apps/desktop/src/main/sessions-ipc-main.ts @@ -20,6 +20,7 @@ import type { SessionListFilter, StoredMessage, ThinkingLevel, + EditingProtocol, EphemeralVoiceAudio, } from '@maka/core'; import type { ProviderType } from '@maka/core/llm-connections'; @@ -140,6 +141,8 @@ export interface SessionsIpcDeps { ) => Promise<{ turnId: string; ok: boolean; error?: string }>; getWorkspacePrivacyContext: () => Promise; canCreateFakeSession: () => boolean; + /** Default captured for new sessions; an IPC request may override it per session. */ + defaultEditingProtocol?: EditingProtocol; consumeNativeAudioOperation?: (input: { operationId: string; connectionSlug: string; @@ -242,6 +245,7 @@ export function registerSessionsIpc( streamEvents, getWorkspacePrivacyContext, canCreateFakeSession, + defaultEditingProtocol, consumeNativeAudioOperation, } = deps; registerSessionExecutionIpc({ @@ -295,8 +299,17 @@ export function registerSessionsIpc( // what the renderer may ask for directly, and what the configured default // fills in are all resolved in one pure place (create-session-input.ts), // which is also the only place any of it can be tested. - const { permissionMode, collaborationMode, orchestrationMode, name, labels } = - await resolveCreateSessionInput(input, { readSettings: () => settingsStore.get() }); + const { + permissionMode, + collaborationMode, + orchestrationMode, + editingProtocol, + name, + labels, + } = await resolveCreateSessionInput(input, { + readSettings: () => settingsStore.get(), + defaultEditingProtocol, + }); if (input?.backend === 'fake') { if (!canCreateFakeSession()) { throw new Error('FakeBackend sessions are only available in development.'); @@ -308,6 +321,7 @@ export function registerSessionsIpc( llmConnectionSlug: input.llmConnectionSlug ?? 'fake', model: input.model ?? 'fake-model', permissionMode, + editingProtocol, collaborationMode, orchestrationMode, name, @@ -329,6 +343,7 @@ export function registerSessionsIpc( model, ...(thinkingLevel !== undefined ? { thinkingLevel } : {}), permissionMode, + editingProtocol, collaborationMode, orchestrationMode, name, diff --git a/apps/desktop/src/main/tool-assembly.ts b/apps/desktop/src/main/tool-assembly.ts index 54d61a1dd2..7858883968 100644 --- a/apps/desktop/src/main/tool-assembly.ts +++ b/apps/desktop/src/main/tool-assembly.ts @@ -389,7 +389,7 @@ export function assembleDesktopTools(deps: DesktopToolAssemblyDeps) { const desktopProductToolSurface = projectEffectiveProductToolSurface({ host: 'desktop', tools: builtinTools, - policy: { economy: economyEnabled }, + policy: { economy: economyEnabled, editingProtocol: 'edit_write' }, }); // Build the union needed by catalog child profiles. SessionManager applies // each profile's narrower allowlist; parent-facing runtime refs are omitted. @@ -415,7 +415,9 @@ export function assembleDesktopTools(deps: DesktopToolAssemblyDeps) { computerUseScreenLock, computerUseTools, desktopProductToolSurface, - builtinTools: [...desktopProductToolSurface.tools], + // Keep the protocol union bound at process scope. The backend projector + // selects exactly one surface from each persisted session header. + builtinTools, childAgentTools, sandboxDiagnosticsProvider, }; diff --git a/apps/desktop/src/main/workspace-resources-ipc-main.ts b/apps/desktop/src/main/workspace-resources-ipc-main.ts index 46a3bc936e..bfca71b9d5 100644 --- a/apps/desktop/src/main/workspace-resources-ipc-main.ts +++ b/apps/desktop/src/main/workspace-resources-ipc-main.ts @@ -40,6 +40,7 @@ export interface NewSessionSkillContext { llmConnectionSlug?: string; model?: string; collaborationMode?: CollaborationMode; + editingProtocol?: import('@maka/core').EditingProtocol; } interface WorkspaceResourcesIpcDeps { diff --git a/apps/desktop/src/renderer/locales/shell-copy.ts b/apps/desktop/src/renderer/locales/shell-copy.ts index 803d04a9e8..90a799c3f2 100644 --- a/apps/desktop/src/renderer/locales/shell-copy.ts +++ b/apps/desktop/src/renderer/locales/shell-copy.ts @@ -344,7 +344,7 @@ type ShellCopy = { title: string; sections: Array<{ heading: string; - rows: Array<{ keys: string[]; description: string }>; + rows: Array<{ shortcuts: string[]; description: string }>; }>; }; chrome: { @@ -963,52 +963,52 @@ const SHELL_COPY_BY_LOCALE = { heading: '通用', rows: [ { - keys: ['⌘', 'K'], + shortcuts: ['mod+k'], description: '打开命令面板(跳会话 / 设置 / 主题等)', }, - { keys: ['?'], description: '打开 / 关闭此快捷键面板' }, - { keys: ['⌘', 'N'], description: '新建任务' }, - { keys: ['⌘', ','], description: '打开设置' }, - { keys: ['Esc'], description: '关闭当前模态框' }, + { shortcuts: ['?'], description: '打开 / 关闭此快捷键面板' }, + { shortcuts: ['mod+n'], description: '新建任务' }, + { shortcuts: ['mod+,'], description: '打开设置' }, + { shortcuts: ['escape'], description: '关闭当前模态框' }, ], }, { heading: 'Composer 输入', rows: [ - { keys: ['Enter'], description: '发送消息' }, - { keys: ['Shift', 'Enter'], description: '插入换行' }, - { keys: ['Alt', 'Enter'], description: '插入换行(备用)' }, + { shortcuts: ['enter'], description: '发送消息' }, + { shortcuts: ['shift+enter'], description: '插入换行' }, + { shortcuts: ['alt+enter'], description: '插入换行(备用)' }, ], }, { heading: '会话列表', rows: [ - { keys: ['Tab'], description: '在会话与导航之间移动焦点' }, - { keys: ['↑', '↓'], description: '上下移动聚焦的会话' }, - { keys: ['Home', 'End'], description: '跳到列表顶部 / 底部' }, + { shortcuts: ['tab'], description: '在会话与导航之间移动焦点' }, + { shortcuts: ['up', 'down'], description: '上下移动聚焦的会话' }, + { shortcuts: ['home', 'end'], description: '跳到列表顶部 / 底部' }, { - keys: ['←', '→'], + shortcuts: ['left', 'right'], description: '在会话 / 已标记 / 已归档之间循环切换', }, - { keys: ['Enter'], description: '打开聚焦的会话' }, - { keys: ['Delete'], description: '弹出删除确认(永远不静默删除)' }, - { keys: ['F'], description: '聚焦会话列表搜索框(按 Esc 清空)' }, + { shortcuts: ['enter'], description: '打开聚焦的会话' }, + { shortcuts: ['delete'], description: '弹出删除确认(永远不静默删除)' }, + { shortcuts: ['f'], description: '聚焦会话列表搜索框(按 Esc 清空)' }, ], }, { heading: '聊天区', rows: [ - { keys: ['Tab'], description: '聚焦工具活动 / 复制按钮' }, - { keys: ['Space', 'Enter'], description: '展开 / 折叠工具调用' }, + { shortcuts: ['tab'], description: '聚焦工具活动 / 复制按钮' }, + { shortcuts: ['space', 'enter'], description: '展开 / 折叠工具调用' }, ], }, { heading: '面板调整', rows: [ - { keys: ['Tab'], description: '聚焦左右分割条' }, - { keys: ['←', '→'], description: '微调会话列表宽度(±10 px)' }, - { keys: ['Shift', '←', '→'], description: '快速调整(±50 px)' }, - { keys: ['Home', 'End'], description: '直接拉到最小 / 最大宽度' }, + { shortcuts: ['tab'], description: '聚焦左右分割条' }, + { shortcuts: ['left', 'right'], description: '微调会话列表宽度(±10 px)' }, + { shortcuts: ['shift+left', 'shift+right'], description: '快速调整(±50 px)' }, + { shortcuts: ['home', 'end'], description: '直接拉到最小 / 最大宽度' }, ], }, ], @@ -1434,22 +1434,22 @@ const SHELL_COPY_BY_LOCALE = { heading: 'General', rows: [ { - keys: ['⌘', 'K'], + shortcuts: ['mod+k'], description: 'Open the command palette (conversations, Settings, themes, and more)', }, - { keys: ['?'], description: 'Open or close this shortcuts panel' }, - { keys: ['⌘', 'N'], description: 'Create a new task' }, - { keys: ['⌘', ','], description: 'Open Settings' }, - { keys: ['Esc'], description: 'Close the current dialog' }, + { shortcuts: ['?'], description: 'Open or close this shortcuts panel' }, + { shortcuts: ['mod+n'], description: 'Create a new task' }, + { shortcuts: ['mod+,'], description: 'Open Settings' }, + { shortcuts: ['escape'], description: 'Close the current dialog' }, ], }, { heading: 'Composer', rows: [ - { keys: ['Enter'], description: 'Send the message' }, - { keys: ['Shift', 'Enter'], description: 'Insert a line break' }, + { shortcuts: ['enter'], description: 'Send the message' }, + { shortcuts: ['shift+enter'], description: 'Insert a line break' }, { - keys: ['Alt', 'Enter'], + shortcuts: ['alt+enter'], description: 'Insert a line break (alternative)', }, ], @@ -1458,28 +1458,28 @@ const SHELL_COPY_BY_LOCALE = { heading: 'Conversation list', rows: [ { - keys: ['Tab'], + shortcuts: ['tab'], description: 'Move focus between conversations and navigation', }, { - keys: ['↑', '↓'], + shortcuts: ['up', 'down'], description: 'Move through focused conversations', }, { - keys: ['Home', 'End'], + shortcuts: ['home', 'end'], description: 'Jump to the top or bottom of the list', }, { - keys: ['←', '→'], + shortcuts: ['left', 'right'], description: 'Cycle through Conversations, Flagged, and Archived', }, - { keys: ['Enter'], description: 'Open the focused conversation' }, + { shortcuts: ['enter'], description: 'Open the focused conversation' }, { - keys: ['Delete'], + shortcuts: ['delete'], description: 'Open the delete confirmation (never delete silently)', }, { - keys: ['F'], + shortcuts: ['f'], description: 'Focus conversation search (press Esc to clear)', }, ], @@ -1488,11 +1488,11 @@ const SHELL_COPY_BY_LOCALE = { heading: 'Chat', rows: [ { - keys: ['Tab'], + shortcuts: ['tab'], description: 'Focus tool activity and Copy buttons', }, { - keys: ['Space', 'Enter'], + shortcuts: ['space', 'enter'], description: 'Expand or collapse a tool call', }, ], @@ -1500,17 +1500,17 @@ const SHELL_COPY_BY_LOCALE = { { heading: 'Panel sizing', rows: [ - { keys: ['Tab'], description: 'Focus the left or right splitter' }, + { shortcuts: ['tab'], description: 'Focus the left or right splitter' }, { - keys: ['←', '→'], + shortcuts: ['left', 'right'], description: 'Adjust conversation-list width (±10 px)', }, { - keys: ['Shift', '←', '→'], + shortcuts: ['shift+left', 'shift+right'], description: 'Adjust quickly (±50 px)', }, { - keys: ['Home', 'End'], + shortcuts: ['home', 'end'], description: 'Jump directly to minimum or maximum width', }, ], diff --git a/docs/architecture/runtime-core-architecture-draft.md b/docs/architecture/runtime-core-architecture-draft.md index 5e3b6aa45c..548c0766f3 100644 --- a/docs/architecture/runtime-core-architecture-draft.md +++ b/docs/architecture/runtime-core-architecture-draft.md @@ -7,7 +7,7 @@ counterpart: ./runtime-core-architecture-draft.zh-CN.md implementation_status: current document_status: draft translation_status: synced -last_verified: 2026-07-12 +last_verified: 2026-07-30 owners: - maka-backend --- @@ -367,6 +367,19 @@ Startup recovery does not re-execute model requests or tool side effects. It sca This is state repair, not checkpoint resume. The current Runtime can retain partial output, recover a consistent terminal state, and provide the facts needed for future mid-run recovery. It does not automatically continue from the line after an interrupted tool call when the process restarts. +## Editing policy and the ApplyPatch transaction boundary + +Editing-tool selection is part of the normalized per-run product-tool policy, not a host builder switch. Desktop, CLI, and Headless bind the union of editing implementations. `projectEffectiveProductToolSurface()` selects either `Write` plus `Edit`, or `ApplyPatch`, exactly once. Child-agent tool surfaces are then projected inside that effective parent surface, so a child can narrow the parent's capabilities but cannot recover a hidden editing protocol. + +`ApplyPatch` also has one semantic implementation: + +1. Core parses and canonicalizes hunks, then produces a pure mutation plan from an immutable no-follow filesystem snapshot. +2. Runtime acquires stable locks for every referenced path, reads the snapshot, plans every mutation, and preflights every permission before the first side effect. +3. Runtime applies the plan through minimal primitives: no-follow `lstat`, text read, no-clobber create, regular-file replace, recursive parent creation, and directory-entry delete. +4. Desktop's filesystem worker and Headless isolation provide only those primitives. They do not reimplement patch planning. + +Create operations must not overwrite an existing destination. Replace and Move sources must be regular files. Delete operates on the named directory entry, while Move deletes that named regular-file source only after creating its destination; neither operation substitutes or deletes a symlink target. If an apply step fails after earlier mutations completed, Runtime returns an explicit partial result with the completed paths; it does not claim rollback that the underlying filesystem cannot guarantee. + ## What this design buys—and what it costs ### Capabilities gained diff --git a/docs/architecture/runtime-core-architecture-draft.zh-CN.md b/docs/architecture/runtime-core-architecture-draft.zh-CN.md index 327af0794f..78ac0120ba 100644 --- a/docs/architecture/runtime-core-architecture-draft.zh-CN.md +++ b/docs/architecture/runtime-core-architecture-draft.zh-CN.md @@ -7,7 +7,7 @@ counterpart: ./runtime-core-architecture-draft.md implementation_status: current document_status: draft translation_status: synced -last_verified: 2026-07-12 +last_verified: 2026-07-30 owners: - maka-backend --- @@ -367,6 +367,19 @@ Maka 当前保护的核心不变量是: 这是“状态修复”,不是 checkpoint resume。当前 Runtime 可以保留部分输出、恢复一致的最终状态,并为以后真正的中点恢复提供事实基础;它不会在进程重启后自动从某个工具调用的下一行继续执行。 +## 编辑策略与 ApplyPatch 事务边界 + +编辑工具的选择属于规范化后的每次运行产品工具策略,而不是 Host builder 的第二个开关。Desktop、CLI 和 Headless 都先绑定全部编辑实现,再由 `projectEffectiveProductToolSurface()` 唯一一次选择 `Write` 加 `Edit`,或 `ApplyPatch`。子 Agent 的工具面随后只能在父运行的有效工具面内继续投影,因此可以进一步收窄能力,但不能重新获得父运行已隐藏的编辑协议。 + +`ApplyPatch` 也只有一份语义实现: + +1. Core 解析并规范化 hunk,再根据一份不可变、且不跟随最终符号链接的文件系统快照生成纯 mutation plan。 +2. Runtime 为所有相关路径获取稳定锁,读取快照,规划全部 mutation,并在第一个副作用发生前完成全部权限预检。 +3. Runtime 只通过最小原语执行计划:不跟随链接的 `lstat`、文本读取、禁止覆盖的创建、仅限普通文件的替换、递归创建父目录,以及删除指定目录项。 +4. Desktop 文件系统 worker 和 Headless 隔离层只提供这些原语,不再各自实现 patch 规划。 + +创建操作不得覆盖已有目标;替换操作和 Move 源都必须是普通文件。Delete 操作用户指定的目录项;Move 也只会在创建目标后删除指定的普通文件源;两者都不会把符号链接替换成其目标或删除链接目标。如果某一步执行失败而此前 mutation 已经完成,Runtime 会返回明确的 partial 结果及已完成路径;底层文件系统不能保证回滚时,Runtime 不会声称存在回滚。 + ## 这套设计换来了什么,又付出了什么 ### 得到的能力 diff --git a/packages/cli/src/__tests__/runtime-bootstrap.test.ts b/packages/cli/src/__tests__/runtime-bootstrap.test.ts index 3d07c5e81b..a9767d9296 100644 --- a/packages/cli/src/__tests__/runtime-bootstrap.test.ts +++ b/packages/cli/src/__tests__/runtime-bootstrap.test.ts @@ -423,6 +423,41 @@ describe('Maka CLI runtime bootstrap', () => { }); }); + test('projects ApplyPatch through the standard CLI runtime input', async () => { + await withWorkspace(async (workspaceRoot) => { + const connectionStore = createConnectionStore(workspaceRoot); + await connectionStore.create({ + slug: 'local', + name: 'Local Ollama', + providerType: 'ollama', + defaultModel: 'llama3.2', + }); + + const context = await createMakaCliRuntimeContext({ + surface: 'run', + workspaceRoot, + cwd: '/repo', + editingProtocol: 'apply_patch', + }); + try { + assert.equal( + context.tools.some((tool) => tool.name === 'ApplyPatch'), + true, + ); + assert.equal( + context.tools.some((tool) => tool.name === 'Write'), + false, + ); + assert.equal( + context.tools.some((tool) => tool.name === 'Edit'), + false, + ); + } finally { + await context.close(); + } + }); + }); + test('registers interactive-only tools exclusively on the TUI surface', async () => { await withWorkspace(async (workspaceRoot) => { const connectionStore = createConnectionStore(workspaceRoot); @@ -594,7 +629,7 @@ describe('Maka CLI runtime bootstrap', () => { }); assert.deepEqual( runtimeDeps.childTools?.map((tool) => tool.name), - ['Read', 'Glob', 'Grep', 'Write', 'Edit', 'Bash'], + ['Read', 'Glob', 'Grep', 'Write', 'Edit', 'Bash', 'ApplyPatch'], ); assert.equal( runtimeDeps.childTools?.some((tool) => diff --git a/packages/cli/src/__tests__/session-driver.test.ts b/packages/cli/src/__tests__/session-driver.test.ts index 92028e8571..829884052a 100644 --- a/packages/cli/src/__tests__/session-driver.test.ts +++ b/packages/cli/src/__tests__/session-driver.test.ts @@ -48,6 +48,7 @@ describe('Maka session driver', () => { llmConnectionSlug: 'anthropic', model: 'claude-sonnet-4-5', permissionMode: 'ask', + editingProtocol: 'edit_write', }, ]); assert.deepEqual(runtime.sent, [ diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index e796f35a54..245e9a5120 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -244,6 +244,7 @@ export async function runMakaCli(argv: string[] = process.argv.slice(2)): Promis llmConnectionSlug: context.target.connection.slug, model: context.target.model, permissionMode: 'ask', + editingProtocol: context.editingProtocol, }); await runMakaPiTui({ driver, diff --git a/packages/cli/src/run-command.ts b/packages/cli/src/run-command.ts index 99aedc6a9e..4bce96b32b 100644 --- a/packages/cli/src/run-command.ts +++ b/packages/cli/src/run-command.ts @@ -13,6 +13,7 @@ import { type CreateMakaCliRuntimeContextInput, } from './runtime-bootstrap.js'; import type { ReadySessionTarget } from './connection-target.js'; +import type { EditingProtocol } from '@maka/core/apply-patch'; import { selectMakaRunSession } from './run-session-selection.js'; import { invocationHasSandboxBoundaryFailure, @@ -57,6 +58,7 @@ export interface MakaRunRuntime { export interface MakaRunContext { runtime: MakaRunRuntime; target: ReadySessionTarget; + editingProtocol?: EditingProtocol; agentGraph?: { reserveActivity(sessionId: string): { release(): void }; waitForCompletion(sessionId: string): Promise; @@ -277,6 +279,7 @@ export async function runMakaTextCli( llmConnectionSlug: context.target.connection.slug, model: context.target.model, permissionMode: parsed.options.yolo ? 'bypass' : 'ask', + editingProtocol: context.editingProtocol ?? 'edit_write', ...(parsed.options.thinking !== undefined ? { thinkingLevel: parsed.options.thinking } : {}), diff --git a/packages/cli/src/runtime-bootstrap.ts b/packages/cli/src/runtime-bootstrap.ts index 9de962fccb..a4e126cf44 100644 --- a/packages/cli/src/runtime-bootstrap.ts +++ b/packages/cli/src/runtime-bootstrap.ts @@ -83,6 +83,7 @@ import { resolveWorkspaceIdentity } from '@maka/storage/workspace-identity'; import { fetchProviderModels } from '@maka/runtime'; import { createApiKeyOnboardingSurface, type MakaOnboardingSurface } from './onboarding.js'; import { isActiveShellRunStatus, resolveModelVisionSupport } from '@maka/core'; +import { resolveEditingProtocolEnv, type EditingProtocol } from '@maka/core/apply-patch'; import type { ModelChoice, ReadySessionTarget } from './connection-target.js'; import { listReadyModelChoices, @@ -101,6 +102,8 @@ export interface MakaCliRuntimeContext { /** Host-injected configuration root used by connections, credentials, and settings. */ configRoot: string; cwd: string; + /** Editing surface captured for sessions created by this runtime context. */ + editingProtocol: EditingProtocol; runtime: SessionManager; target: ReadySessionTarget; /** Selectable models across every ready connection, for the `/model` picker. */ @@ -164,6 +167,9 @@ export interface SessionRecapGenerator { export interface CreateMakaCliRuntimeContextInput { surface: 'tui' | 'run' | 'activation'; + /** Explicit per-run editing surface override; defaults to Edit/Write. */ + editingProtocol?: EditingProtocol; + /** Legacy root; both new roots default to this path. */ workspaceRoot: string; /** Optional portable session-owned state root. */ @@ -219,6 +225,8 @@ export async function createMakaCliRuntimeContext( const stateRoot = input.stateRoot ?? input.workspaceRoot; const configRoot = input.configRoot ?? input.workspaceRoot; const agentGraphEnabled = input.surface === 'tui' || input.enableAgentGraph === true; + const editingProtocol = + input.editingProtocol ?? editingProtocolFromEnv(process.env.MAKA_EDITING_PROTOCOL); if (input.stateRoot !== undefined || input.configRoot !== undefined) { await assertSessionBundleRootLayout({ stateRoot, @@ -604,6 +612,7 @@ export async function createMakaCliRuntimeContext( tools: boundTools, policy: { economy: input.surface === 'tui' && !process.env.MAKA_DISABLE_DEFERRED_TOOLS, + editingProtocol, }, }); const allTools = [...cliProductToolSurface.tools]; @@ -646,8 +655,11 @@ export async function createMakaCliRuntimeContext( : []; const productToolSurface = projectEffectiveProductToolSurface({ host: 'cli', - tools: ctx.tools ? ctx.tools : [...allTools, ...agentGraphSupervisorTools], - policy: cliProductToolSurface.identity.policy, + tools: ctx.tools ? ctx.tools : [...boundTools, ...agentGraphSupervisorTools], + policy: { + ...cliProductToolSurface.identity.policy, + editingProtocol: header.editingProtocol ?? editingProtocol, + }, }); const backendTools = [...productToolSurface.tools]; const admitsAgentChildren = productToolSurface.boundSurfaceIds.includes(AGENT_TOOL_GROUP_ID); @@ -807,7 +819,17 @@ export async function createMakaCliRuntimeContext( inspectContinuationSafety: createLocalContinuationSafetyInspector({ readSessionCwd: async (sessionId) => (await store.readHeader(sessionId)).cwd, resolveWorkspaceIdentity: async (cwd) => resolveWorkspaceIdentity({ path: cwd }), - listAvailableToolNames: async () => allTools.map((tool) => tool.name), + listAvailableToolNames: async (sessionId) => { + const header = await store.readHeader(sessionId); + return projectEffectiveProductToolSurface({ + host: 'cli', + tools: boundTools, + policy: { + ...cliProductToolSurface.identity.policy, + editingProtocol: header.editingProtocol ?? editingProtocol, + }, + }).tools.map((tool) => tool.name); + }, hasPendingBackgroundOperations: async (sessionId) => { const [shellUpdates, runs] = await Promise.all([ shellRuns.listSessionUpdates(sessionId), @@ -1023,6 +1045,7 @@ export async function createMakaCliRuntimeContext( stateRoot, configRoot, cwd: input.cwd, + editingProtocol, runtime, target, modelChoices, @@ -1092,6 +1115,12 @@ export async function createMakaCliRuntimeContext( }; } +function editingProtocolFromEnv(value: string | undefined): EditingProtocol { + // CLI sessions default to Edit/Write; an explicit env override is the only + // way to reach apply_patch. + return resolveEditingProtocolEnv(value) ?? 'edit_write'; +} + export async function getOrCreateCliClaudeDeviceId( workspaceRoot: string, deps: GetOrCreateCliClaudeDeviceIdDeps = {}, diff --git a/packages/cli/src/session-driver.ts b/packages/cli/src/session-driver.ts index 485c131b1e..0ded391f10 100644 --- a/packages/cli/src/session-driver.ts +++ b/packages/cli/src/session-driver.ts @@ -19,6 +19,7 @@ import type { OrchestrationMode } from '@maka/core/orchestration'; import type { SessionSummary, StoredMessage } from '@maka/core/session'; import { userFacingText } from '@maka/core/session'; import type { ThinkingLevel } from '@maka/core/model-thinking'; +import type { EditingProtocol } from '@maka/core/apply-patch'; import type { ContextDiagnostics, RuntimeContinuation, @@ -97,6 +98,7 @@ export interface MakaSessionDriverInput { llmConnectionSlug: string; model: string; permissionMode?: PermissionMode; + editingProtocol?: EditingProtocol; orchestrationMode?: OrchestrationMode; newId?: () => string; inspectCwdChanges?: InspectCwdChanges; @@ -529,6 +531,7 @@ class RuntimeMakaSessionDriver implements MakaSessionDriver { llmConnectionSlug: this.llmConnectionSlug, model: this.model, permissionMode: this.permissionMode, + editingProtocol: this.input.editingProtocol ?? 'edit_write', ...(this.orchestrationMode !== 'default' ? { orchestrationMode: this.orchestrationMode } : {}), diff --git a/packages/core/package.json b/packages/core/package.json index 1b1ee5748b..4f6c410b59 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -95,7 +95,8 @@ "./usage-record-schema": "./dist/usage-record-schema.js", "./session-send-projection": "./dist/session-send-projection.js", "./session-name": "./dist/session-name.js", - "./tool-catalog": "./dist/tool-catalog.js" + "./tool-catalog": "./dist/tool-catalog.js", + "./apply-patch": "./dist/apply-patch.js" }, "scripts": { "clean": "node ../../scripts/clean-paths.mjs dist tsconfig.tsbuildinfo", diff --git a/packages/core/src/__tests__/apply-patch.test.ts b/packages/core/src/__tests__/apply-patch.test.ts new file mode 100644 index 0000000000..8fd706b57a --- /dev/null +++ b/packages/core/src/__tests__/apply-patch.test.ts @@ -0,0 +1,333 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + applyUpdateChunksToContent, + assertSafePatchPath, + canonicalizeApplyPatchHunks, + parseApplyPatch, + planApplyPatchMutations, +} from '../apply-patch.js'; + +function envelope(body: string): string { + return `*** Begin Patch\n${body}*** End Patch\n`; +} + +describe('parseApplyPatch', () => { + test('parses add, update, move, and delete operations', () => { + const patch = envelope( + [ + '*** Add File: hello.txt', + '+Hello', + '*** Update File: src/app.py', + '*** Move to: src/main.py', + '@@ def greet():', + '-print("Hi")', + '+print("Hello")', + '*** Delete File: obsolete.txt', + '', + ].join('\n'), + ); + const parsed = parseApplyPatch(patch); + assert.equal(parsed.ok, true); + if (!parsed.ok) return; + assert.equal(parsed.value.hunks.length, 3); + assert.deepEqual(parsed.value.hunks[0], { + kind: 'add', + path: 'hello.txt', + contents: 'Hello\n', + }); + assert.equal(parsed.value.hunks[1]?.kind, 'update'); + if (parsed.value.hunks[1]?.kind === 'update') { + assert.equal(parsed.value.hunks[1].path, 'src/app.py'); + assert.equal(parsed.value.hunks[1].movePath, 'src/main.py'); + assert.equal(parsed.value.hunks[1].chunks.length, 1); + } + assert.deepEqual(parsed.value.hunks[2], { kind: 'delete', path: 'obsolete.txt' }); + }); + + test('strips heredoc wrappers in lenient mode', () => { + const patch = [ + "<<'EOF'", + '*** Begin Patch', + '*** Add File: a.txt', + '+x', + '*** End Patch', + 'EOF', + ].join('\n'); + const parsed = parseApplyPatch(patch); + assert.equal(parsed.ok, true); + if (!parsed.ok) return; + assert.equal(parsed.value.hunks[0]?.kind, 'add'); + }); + + test('rejects absolute paths at the tool safety check', () => { + assert.ok(assertSafePatchPath('/etc/passwd')); + assert.ok(assertSafePatchPath('C:/Windows/system32')); + assert.ok(assertSafePatchPath('../escape')); + assert.equal(assertSafePatchPath('src/ok.ts'), null); + }); +}); + +describe('planApplyPatchMutations', () => { + test('plans aliases and sequential updates against one immutable snapshot', () => { + const parsed = parseApplyPatch( + envelope( + [ + '*** Update File: ./src/a.txt', + '@@', + '-one', + '+two', + '*** Update File: src//a.txt', + '@@', + '-two', + '+three', + '', + ].join('\n'), + ), + ); + assert.equal(parsed.ok, true); + if (!parsed.ok) return; + const hunks = canonicalizeApplyPatchHunks(parsed.value.hunks); + const plan = planApplyPatchMutations( + hunks, + new Map([['src/a.txt', { kind: 'file' as const, content: 'one\n' }]]), + ); + assert.deepEqual(plan, [ + { operation: 'update', path: 'src/a.txt', content: 'two\n' }, + { operation: 'update', path: 'src/a.txt', content: 'three\n' }, + ]); + }); + + test('allows deleting a symlink entry but never treats it as update content', () => { + assert.deepEqual( + planApplyPatchMutations( + [{ kind: 'delete', path: 'link.txt' }], + new Map([['link.txt', { kind: 'symlink' as const }]]), + ), + [{ operation: 'delete', path: 'link.txt' }], + ); + assert.throws( + () => + planApplyPatchMutations( + [ + { + kind: 'update', + path: 'link.txt', + chunks: [{ oldLines: ['a'], newLines: ['b'], isEndOfFile: false }], + }, + ], + new Map([['link.txt', { kind: 'symlink' as const }]]), + ), + /regular file/, + ); + }); +}); + +describe('applyUpdateChunksToContent', () => { + test('applies a unique hunk and preserves CRLF', () => { + const original = 'line1\r\nline2\r\nline3\r\n'; + const result = applyUpdateChunksToContent( + original, + [ + { + oldLines: ['line2'], + newLines: ['line2-updated'], + isEndOfFile: false, + }, + ], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'line1\r\nline2-updated\r\nline3\r\n'); + }); + + test('fails when the hunk is not unique', () => { + const original = 'a\nx\nb\nx\nc\n'; + const result = applyUpdateChunksToContent( + original, + [{ oldLines: ['x'], newLines: ['y'], isEndOfFile: false }], + 'f.txt', + ); + assert.equal(result.ok, false); + }); + + test('pure insertion without EOF marker appends at EOF', () => { + const original = 'line1\nline2\n'; + const result = applyUpdateChunksToContent( + original, + [{ oldLines: [], newLines: ['appended'], isEndOfFile: false }], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'line1\nline2\nappended\n'); + }); + + test('preserves CR-only line endings outside the edit', () => { + const original = 'a\rb\rc\r'; + const result = applyUpdateChunksToContent( + original, + [{ oldLines: ['b'], newLines: ['B'], isEndOfFile: false }], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'a\rB\rc\r'); + }); + + test('preserves mixed endings outside the edited region', () => { + const original = 'a\r\nb\nc\r'; + const result = applyUpdateChunksToContent( + original, + [{ oldLines: ['b'], newLines: ['B'], isEndOfFile: false }], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + // Untouched lines keep their original terminators; the replaced line uses + // the file's dominant default ending (CRLF here because the file has CRLF). + assert.equal(result.content, 'a\r\nB\r\nc\r'); + }); + + test('deleting a final line preserves the preceding untouched newline', () => { + const result = applyUpdateChunksToContent( + 'first\nlast', + [{ oldLines: ['last'], newLines: [], isEndOfFile: true }], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'first\n'); + }); + + test('Codex context match: a substring-only context does not anchor the hunk', () => { + // `foo` appears inside `prefix foo suffix` but names no whole line, so the + // matcher must not use the first substring hit as the context line. + const original = 'prefix foo suffix\nfoo\ntail\n'; + const result = applyUpdateChunksToContent( + original, + [ + { + changeContext: 'foo', + oldLines: ['tail'], + newLines: ['INSERT'], + isEndOfFile: false, + }, + ], + 'f.txt', + ); + // The context line is the second line (`foo`), so the old-lines search + // starts there and finds `tail` immediately after it. + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'prefix foo suffix\nfoo\nINSERT\n'); + }); + + test('Codex pure insertion with a context header anchors after the context line', () => { + const original = 'first\ncontext line\nlast\n'; + const result = applyUpdateChunksToContent( + original, + [ + { + changeContext: 'context line', + oldLines: [], + newLines: ['INSERT'], + isEndOfFile: false, + }, + ], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'first\ncontext line\nINSERT\nlast\n'); + }); + + test('Codex pure insertion with a substring-only context falls back to EOF', () => { + // The context names no whole line (only a substring of `prefix foo suffix`), + // so a pure addition is placed at EOF instead of an arbitrary position. + const original = 'prefix foo suffix\nlast\n'; + const result = applyUpdateChunksToContent( + original, + [ + { + changeContext: 'foo', + oldLines: [], + newLines: ['INSERT'], + isEndOfFile: false, + }, + ], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'prefix foo suffix\nlast\nINSERT\n'); + }); + + test('Codex ambiguous context prefers the first whole-line match', () => { + const original = 'a\ncontext\nb\ncontext\nc\n'; + const result = applyUpdateChunksToContent( + original, + [ + { + changeContext: 'context', + oldLines: [], + newLines: ['INSERT'], + isEndOfFile: false, + }, + ], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'a\ncontext\nINSERT\nb\ncontext\nc\n'); + }); + + test('Codex repeated old-lines still requires a unique match', () => { + const original = 'x\nx\n'; + const result = applyUpdateChunksToContent( + original, + [{ oldLines: ['x'], newLines: ['y'], isEndOfFile: false }], + 'f.txt', + ); + assert.equal(result.ok, false); + }); + + test('Codex EOF marker inserts at the end regardless of context', () => { + const original = 'context line\nlast\n'; + const result = applyUpdateChunksToContent( + original, + [ + { + changeContext: 'context line', + oldLines: [], + newLines: ['INSERT'], + isEndOfFile: true, + }, + ], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'context line\nlast\nINSERT\n'); + }); + + test('Codex context matching tolerates trailing-whitespace drift', () => { + const original = 'context line \nlast\n'; + const result = applyUpdateChunksToContent( + original, + [ + { + changeContext: 'context line', + oldLines: [], + newLines: ['INSERT'], + isEndOfFile: false, + }, + ], + 'f.txt', + ); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.content, 'context line \nINSERT\nlast\n'); + }); +}); diff --git a/packages/core/src/__tests__/permission-compatibility.test.ts b/packages/core/src/__tests__/permission-compatibility.test.ts index d1ecf67e66..311e1844d9 100644 --- a/packages/core/src/__tests__/permission-compatibility.test.ts +++ b/packages/core/src/__tests__/permission-compatibility.test.ts @@ -18,6 +18,7 @@ describe('legacy permission payload classification', () => { test('keeps plan-mode tool availability classification independent of authorization', () => { expect(classifyToolUse({ toolName: 'Read', args: {} })).toBe('read'); expect(classifyToolUse({ toolName: 'Write', args: {} })).toBe('file_write'); + expect(classifyToolUse({ toolName: 'ApplyPatch', args: {} })).toBe('file_write'); expect(classifyToolUse({ toolName: 'ExploreAgent', args: {}, categoryHint: 'subagent' })).toBe( 'subagent', ); diff --git a/packages/core/src/__tests__/tool-catalog-contract.test.ts b/packages/core/src/__tests__/tool-catalog-contract.test.ts index f9a7613838..dea7abcd92 100644 --- a/packages/core/src/__tests__/tool-catalog-contract.test.ts +++ b/packages/core/src/__tests__/tool-catalog-contract.test.ts @@ -117,6 +117,7 @@ describe('tool catalog contract', () => { 'Bash', 'Read', 'Write', + 'ApplyPatch', 'Glob', 'Grep', 'FormatJson', @@ -155,6 +156,7 @@ describe('tool catalog contract', () => { 'Read', 'Write', 'Edit', + 'ApplyPatch', 'Glob', 'Grep', 'FormatJson', @@ -179,6 +181,7 @@ describe('tool catalog contract', () => { 'Read', 'Write', 'Edit', + 'ApplyPatch', 'Glob', 'Grep', 'agent_spawn', diff --git a/packages/core/src/apply-patch.ts b/packages/core/src/apply-patch.ts new file mode 100644 index 0000000000..b1cd352984 --- /dev/null +++ b/packages/core/src/apply-patch.ts @@ -0,0 +1,666 @@ +/** + * Codex-compatible ApplyPatch parse + in-memory apply (#1552 / #1383). + * + * Per-run editing protocol projection belongs to the normalized product-tool + * policy. Tool builders bind implementations; the projector selects exactly + * one of Edit/Write or ApplyPatch for a run. + */ +export type EditingProtocol = 'edit_write' | 'apply_patch'; + +export function isEditingProtocol(value: unknown): value is EditingProtocol { + return value === 'edit_write' || value === 'apply_patch'; +} + +/** + * Parse the MAKA_EDITING_PROTOCOL environment value. Undefined/empty means + * "no override" (callers apply their own default); any other value must be a + * valid protocol or the process is misconfigured. One parser for CLI, Desktop, + * and Headless so the three hosts cannot drift. + */ +export function resolveEditingProtocolEnv( + value: string | undefined, + label = 'MAKA_EDITING_PROTOCOL', +): EditingProtocol | undefined { + if (value === undefined || value === '') return undefined; + if (isEditingProtocol(value)) return value; + throw new Error(`${label} must be "edit_write" or "apply_patch"`); +} + +/** + * Grammar (lenient around heredoc wrappers): + * *** Begin Patch + * *** Add File: + * +lines... + * *** Delete File: + * *** Update File: + * *** Move to: (optional) + * @@ [optional context] + * context / -old / +new lines + * *** End of File (optional, anchors match at EOF) + * *** End Patch + * + * This module never touches the filesystem. Callers preflight and mutate + * through WorkspaceExecutor / filesystem worker + permission boundaries. + */ + +export const BEGIN_PATCH_MARKER = '*** Begin Patch'; +export const END_PATCH_MARKER = '*** End Patch'; +export const ADD_FILE_MARKER = '*** Add File: '; +export const DELETE_FILE_MARKER = '*** Delete File: '; +export const UPDATE_FILE_MARKER = '*** Update File: '; +export const MOVE_TO_MARKER = '*** Move to: '; +export const EOF_MARKER = '*** End of File'; +export const CHANGE_CONTEXT_MARKER = '@@'; + +export type ApplyPatchHunk = + | { kind: 'add'; path: string; contents: string } + | { kind: 'delete'; path: string } + | { + kind: 'update'; + path: string; + movePath?: string; + chunks: ApplyPatchUpdateChunk[]; + }; + +export interface ApplyPatchUpdateChunk { + /** Optional @@ context header used to narrow the search window. */ + changeContext?: string; + oldLines: string[]; + newLines: string[]; + isEndOfFile: boolean; +} + +export interface ApplyPatchParseResult { + hunks: ApplyPatchHunk[]; +} + +export type ApplyPatchParseError = + | { code: 'invalid_patch'; message: string } + | { code: 'invalid_hunk'; message: string; lineNumber: number }; + +export type ApplyPatchParseOutcome = + | { ok: true; value: ApplyPatchParseResult } + | { ok: false; error: ApplyPatchParseError }; + +export type ApplyContentError = + | { code: 'hunk_mismatch'; message: string; path: string } + | { code: 'empty_update'; message: string; path: string }; + +export type ApplyContentOutcome = + | { ok: true; content: string } + | { ok: false; error: ApplyContentError }; + +export type PlannedPatchMutation = + | { operation: 'add'; path: string; content: string } + | { operation: 'update'; path: string; content: string } + | { operation: 'delete'; path: string } + | { operation: 'move'; path: string; fromPath: string; content: string }; + +export type ApplyPatchPathState = + | { readonly kind: 'missing' } + | { readonly kind: 'file'; readonly content: string } + | { readonly kind: 'symlink' } + | { readonly kind: 'other' }; + +/** + * Canonicalize lexical aliases before locking, permission planning, or state + * lookup. Filesystem containment remains the host adapter's responsibility. + */ +export function canonicalizeApplyPatchHunks(hunks: readonly ApplyPatchHunk[]): ApplyPatchHunk[] { + return hunks.map((hunk) => { + const path = canonicalApplyPatchPath(hunk.path); + if (hunk.kind === 'add' || hunk.kind === 'delete') return { ...hunk, path }; + return { + ...hunk, + path, + ...(hunk.movePath ? { movePath: canonicalApplyPatchPath(hunk.movePath) } : {}), + }; + }); +} + +export function canonicalApplyPatchPath(path: string): string { + const normalized = path.replaceAll('\\', '/').trim(); + const segments: string[] = []; + for (const segment of normalized.split('/')) { + if (!segment || segment === '.') continue; + if (segment === '..') { + segments.pop(); + continue; + } + segments.push(segment); + } + return segments.join('/') || '.'; +} + +/** + * Pure ApplyPatch planner. + * + * The caller snapshots every referenced directory entry under the transaction + * locks and passes that immutable state here. Planning never touches the + * filesystem, so Runtime and Headless cannot acquire different overwrite, + * alias, or partial-order semantics. + */ +export function planApplyPatchMutations( + hunks: readonly ApplyPatchHunk[], + initialState: ReadonlyMap, +): PlannedPatchMutation[] { + const state = new Map(initialState); + const mutations: PlannedPatchMutation[] = []; + + const current = (path: string): ApplyPatchPathState => state.get(path) ?? { kind: 'missing' }; + const requireRegularFile = (path: string, operation: string): string => { + const value = current(path); + if (value.kind === 'missing') { + throw new Error(`ApplyPatch ${operation} target missing: ${path}`); + } + if (value.kind !== 'file') { + throw new Error(`ApplyPatch ${operation} target must be a regular file: ${path}`); + } + return value.content; + }; + + for (const hunk of hunks) { + if (hunk.kind === 'add') { + if (current(hunk.path).kind !== 'missing') { + throw new Error(`ApplyPatch Add File target already exists: ${hunk.path}`); + } + mutations.push({ operation: 'add', path: hunk.path, content: hunk.contents }); + state.set(hunk.path, { kind: 'file', content: hunk.contents }); + continue; + } + + if (hunk.kind === 'delete') { + const value = current(hunk.path); + if (value.kind === 'missing') { + throw new Error(`ApplyPatch Delete File target missing: ${hunk.path}`); + } + if (value.kind !== 'file' && value.kind !== 'symlink') { + throw new Error(`ApplyPatch Delete File target must be a file or symlink: ${hunk.path}`); + } + mutations.push({ operation: 'delete', path: hunk.path }); + state.set(hunk.path, { kind: 'missing' }); + continue; + } + + const original = requireRegularFile(hunk.path, 'Update File'); + const applied = applyUpdateChunksToContent(original, hunk.chunks, hunk.path); + if (!applied.ok) throw new Error(applied.error.message); + + if (hunk.movePath) { + if (current(hunk.movePath).kind !== 'missing') { + throw new Error(`ApplyPatch Move destination already exists: ${hunk.movePath}`); + } + mutations.push({ + operation: 'move', + path: hunk.movePath, + fromPath: hunk.path, + content: applied.content, + }); + state.set(hunk.path, { kind: 'missing' }); + state.set(hunk.movePath, { kind: 'file', content: applied.content }); + continue; + } + + mutations.push({ operation: 'update', path: hunk.path, content: applied.content }); + state.set(hunk.path, { kind: 'file', content: applied.content }); + } + + return mutations; +} + +export function parseApplyPatch(input: string): ApplyPatchParseOutcome { + const trimmed = input.replace(/^\uFEFF/, '').trimEnd(); + const rawLines = trimmed.split(/\r?\n/); + const boundary = extractPatchLines(rawLines); + if (!boundary.ok) return boundary; + + const lines = boundary.lines; + const hunks: ApplyPatchHunk[] = []; + let i = 1; // skip Begin Patch + + while (i < lines.length - 1) { + const line = lines[i]!; + const lineNumber = i + 1; + + if (line.startsWith(ADD_FILE_MARKER)) { + const path = line.slice(ADD_FILE_MARKER.length).trim(); + if (!path) { + return invalidHunk('Add File header is missing a path', lineNumber); + } + i += 1; + const contentLines: string[] = []; + while (i < lines.length - 1 && lines[i]!.startsWith('+')) { + contentLines.push(lines[i]!.slice(1)); + i += 1; + } + if (contentLines.length === 0) { + return invalidHunk('Add File must include at least one + content line', lineNumber); + } + hunks.push({ kind: 'add', path, contents: contentLines.join('\n') + '\n' }); + continue; + } + + if (line.startsWith(DELETE_FILE_MARKER)) { + const path = line.slice(DELETE_FILE_MARKER.length).trim(); + if (!path) { + return invalidHunk('Delete File header is missing a path', lineNumber); + } + hunks.push({ kind: 'delete', path }); + i += 1; + continue; + } + + if (line.startsWith(UPDATE_FILE_MARKER)) { + const path = line.slice(UPDATE_FILE_MARKER.length).trim(); + if (!path) { + return invalidHunk('Update File header is missing a path', lineNumber); + } + i += 1; + let movePath: string | undefined; + if (i < lines.length - 1 && lines[i]!.startsWith(MOVE_TO_MARKER)) { + movePath = lines[i]!.slice(MOVE_TO_MARKER.length).trim(); + if (!movePath) { + return invalidHunk('Move to header is missing a path', i + 1); + } + i += 1; + } + const chunks: ApplyPatchUpdateChunk[] = []; + while (i < lines.length - 1 && !isFileOpHeader(lines[i]!)) { + if ( + lines[i] === CHANGE_CONTEXT_MARKER || + lines[i]!.startsWith(`${CHANGE_CONTEXT_MARKER} `) + ) { + const changeContext = + lines[i] === CHANGE_CONTEXT_MARKER + ? undefined + : lines[i]!.slice(CHANGE_CONTEXT_MARKER.length + 1); + i += 1; + const chunkResult = readChunkBody(lines, i, lines.length - 1); + if (!chunkResult.ok) return chunkResult; + i = chunkResult.nextIndex; + chunks.push({ + ...(changeContext !== undefined && changeContext.length > 0 ? { changeContext } : {}), + oldLines: chunkResult.oldLines, + newLines: chunkResult.newLines, + isEndOfFile: chunkResult.isEndOfFile, + }); + continue; + } + // Bare chunk without @@ — tolerate as a single chunk body. + if ( + lines[i]!.startsWith(' ') || + lines[i]!.startsWith('-') || + lines[i]!.startsWith('+') || + lines[i] === EOF_MARKER + ) { + const chunkResult = readChunkBody(lines, i, lines.length - 1); + if (!chunkResult.ok) return chunkResult; + i = chunkResult.nextIndex; + chunks.push({ + oldLines: chunkResult.oldLines, + newLines: chunkResult.newLines, + isEndOfFile: chunkResult.isEndOfFile, + }); + continue; + } + return invalidHunk(`Unexpected line in Update File: ${lines[i]}`, i + 1); + } + if (chunks.length === 0 && !movePath) { + return invalidHunk('Update File must include at least one hunk or Move to', lineNumber); + } + hunks.push({ + kind: 'update', + path, + ...(movePath ? { movePath } : {}), + chunks, + }); + continue; + } + + return invalidHunk(`Expected a file operation header, got: ${line}`, lineNumber); + } + + if (hunks.length === 0) { + return { + ok: false, + error: { code: 'invalid_patch', message: 'Patch contains no file operations' }, + }; + } + + return { + ok: true, + value: { + hunks, + }, + }; +} + +/** + * Reject absolute paths and `..` escapes in patch path strings (lexical check). + * Resolved containment still happens at the tool/worker layer. + */ +export function assertSafePatchPath(path: string): string | null { + const normalized = path.replaceAll('\\', '/').trim(); + if (!normalized) return 'path is empty'; + if (normalized.startsWith('/') || /^[A-Za-z]:\//.test(normalized)) { + return 'path must be relative (absolute paths are rejected)'; + } + const segments = normalized.split('/'); + if (segments.some((segment) => segment === '..')) { + return 'path must not contain parent-directory segments (..)'; + } + return null; +} + +/** Apply update chunks to file text, preserving the original newline style. */ +export function applyUpdateChunksToContent( + original: string, + chunks: readonly ApplyPatchUpdateChunk[], + path: string, +): ApplyContentOutcome { + if (chunks.length === 0) { + return { ok: true, content: original }; + } + + // Preserve CR-only, CRLF, LF, and mixed endings outside the edited region by + // carrying each original line's terminator through untouched lines. + const originalLines = splitContentLinesWithEnds(original); + let bodies = originalLines.map((line) => line.body); + let endings = originalLines.map((line) => line.ending); + const defaultEnding = detectDefaultLineEnding(original); + + for (const chunk of chunks) { + const match = findChunkMatch(bodies, chunk); + if (!match) { + return { + ok: false, + error: { + code: 'hunk_mismatch', + path, + message: `ApplyPatch hunk did not match in ${path}${ + chunk.changeContext ? ` (context ${JSON.stringify(chunk.changeContext)})` : '' + }`, + }, + }; + } + const removedEnds = endings.slice(match.start, match.end); + const insertedEnds = chunk.newLines.map((_, index) => { + if (index < chunk.newLines.length - 1) return defaultEnding; + if (removedEnds.length > 0) return removedEnds.at(-1) === '' ? '' : defaultEnding; + if (match.start < bodies.length) return defaultEnding; + return endings.at(-1) ? defaultEnding : ''; + }); + // Appending to a file without an EOF newline still needs a separator + // between the former last line and the first inserted line. The inserted + // last line itself keeps the file's no-EOF-newline state. + if ( + match.start === bodies.length && + match.end === bodies.length && + chunk.newLines.length > 0 && + bodies.length > 0 && + endings.at(-1) === '' + ) { + endings[endings.length - 1] = defaultEnding; + } + bodies = [...bodies.slice(0, match.start), ...chunk.newLines, ...bodies.slice(match.end)]; + endings = [...endings.slice(0, match.start), ...insertedEnds, ...endings.slice(match.end)]; + } + + return { + ok: true, + content: bodies.map((body, index) => body + (endings[index] ?? '')).join(''), + }; +} + +export function collectPatchPaths(hunks: readonly ApplyPatchHunk[]): string[] { + const paths: string[] = []; + for (const hunk of hunks) { + paths.push(hunk.path); + if (hunk.kind === 'update' && hunk.movePath) paths.push(hunk.movePath); + } + return paths; +} + +// ── internals ────────────────────────────────────────────────────────────── + +function extractPatchLines( + lines: string[], +): { ok: true; lines: string[] } | { ok: false; error: ApplyPatchParseError } { + const strict = checkBoundariesStrict(lines); + if (strict.ok) return strict; + + // Lenient: strip heredoc wrappers (<= 4) { + const first = lines[0]!.trim(); + const last = lines[lines.length - 1]!.trim(); + if ((first === '< { + if (content.length === 0) return []; + const lines: Array<{ body: string; ending: string }> = []; + let body = ''; + for (let i = 0; i < content.length; i += 1) { + const ch = content[i]!; + if (ch === '\r') { + if (content[i + 1] === '\n') { + lines.push({ body, ending: '\r\n' }); + body = ''; + i += 1; + } else { + lines.push({ body, ending: '\r' }); + body = ''; + } + continue; + } + if (ch === '\n') { + lines.push({ body, ending: '\n' }); + body = ''; + continue; + } + body += ch; + } + if (body.length > 0) { + lines.push({ body, ending: '' }); + } + return lines; +} + +function findChunkMatch( + lines: readonly string[], + chunk: ApplyPatchUpdateChunk, +): { start: number; end: number } | null { + let searchFrom = 0; + if (chunk.changeContext) { + const ctxIndex = findContextLine(lines, chunk.changeContext); + if (ctxIndex < 0) { + // A substring-only (or absent) context never anchors a pure addition: + // Codex places it at EOF instead of an arbitrary substring position. + if (chunk.oldLines.length === 0) { + return { start: lines.length, end: lines.length }; + } + return null; + } + searchFrom = ctxIndex; + } + + const old = chunk.oldLines; + if (old.length === 0) { + // Codex pure-insertion: EOF marker or no context → insert at EOF. + // With @@ context only, insert immediately after the context line — but + // only when the context is a real line match. A substring-only context + // must never anchor the insertion at an arbitrary position, so it falls + // back to EOF exactly like Codex's pure-addition behavior. + if (chunk.isEndOfFile || !chunk.changeContext) { + return { start: lines.length, end: lines.length }; + } + return { start: searchFrom + 1, end: searchFrom + 1 }; + } + + const candidates: number[] = []; + for (let i = searchFrom; i <= lines.length - old.length; i += 1) { + let matched = true; + for (let j = 0; j < old.length; j += 1) { + if (lines[i + j] !== old[j]) { + matched = false; + break; + } + } + if (matched) candidates.push(i); + } + + if (candidates.length === 0) { + // Line-trimmed fallback (indentation drift), only when unique. + const trimmedOld = old.map((line) => line.trimEnd()); + for (let i = searchFrom; i <= lines.length - old.length; i += 1) { + let matched = true; + for (let j = 0; j < old.length; j += 1) { + if (lines[i + j]!.trimEnd() !== trimmedOld[j]) { + matched = false; + break; + } + } + if (matched) candidates.push(i); + } + } + + if (candidates.length !== 1) return null; + + const start = candidates[0]!; + const end = start + old.length; + if (chunk.isEndOfFile && end !== lines.length) return null; + return { start, end }; +} + +/** + * Locate the `@@` context as a whole line (Codex semantics): the header names + * a line, not a substring, so `foo` must not anchor inside `prefix foo suffix`. + * Exact match after trimming trailing whitespace first, then a controlled + * full-trim fallback for indentation drift. Returns -1 when no line matches. + */ +function findContextLine(lines: readonly string[], context: string): number { + const trimmed = context.trimEnd(); + const exact = lines.findIndex((line) => line.trimEnd() === trimmed); + if (exact >= 0) return exact; + const fullyTrimmed = context.trim(); + return lines.findIndex((line) => line.trim() === fullyTrimmed); +} + +function invalidHunk( + message: string, + lineNumber: number, +): { ok: false; error: ApplyPatchParseError } { + return { ok: false, error: { code: 'invalid_hunk', message, lineNumber } }; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6fd1f9a36b..5efdc33caf 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -8,6 +8,7 @@ */ export * from './mcp.js'; +export * from './apply-patch.js'; export * from './collaboration.js'; export * from './orchestration.js'; export * from './swarm-command.js'; @@ -319,16 +320,10 @@ export type { RuntimePrefixSegmentV1, RuntimeBoundaryDigest, } from './runtime-boundary.js'; -export { - buildImmutableRuntimePrefix, - createRuntimeBoundaryCursor, - decodeContinuationClaim, - decodeRuntimeBoundaryCursor, - decodeRuntimePrefixSegment, - digestRuntimeBoundaryManifest, - digestRuntimePrefix, - runtimePrefixSegment, -} from './runtime-boundary.js'; +// runtime-boundary.ts is intentionally type-only in this browser-consumed +// barrel: its digest implementation depends on node:crypto. Runtime code must +// import its values from `@maka/core/runtime-boundary` so renderer imports of +// `@maka/core` do not evaluate Node-only modules before React can mount. // session.ts export type { diff --git a/packages/core/src/permission.ts b/packages/core/src/permission.ts index ae84456ebb..d89c5d348f 100644 --- a/packages/core/src/permission.ts +++ b/packages/core/src/permission.ts @@ -102,6 +102,7 @@ export const BUILTIN_TOOL_CATEGORY: Record = { Write: 'file_write', Edit: 'file_write', patch: 'file_write', + ApplyPatch: 'file_write', // shell — default unsafe; categorizeBash() may downgrade or upgrade Bash: 'shell_unsafe', WriteStdin: 'shell_unsafe', diff --git a/packages/core/src/runtime-inputs.ts b/packages/core/src/runtime-inputs.ts index 37b3e71b34..08b1ddd31d 100644 --- a/packages/core/src/runtime-inputs.ts +++ b/packages/core/src/runtime-inputs.ts @@ -17,6 +17,7 @@ import type { CollaborationMode } from './collaboration.js'; import type { OrchestrationMode, TurnOrchestration } from './orchestration.js'; import type { SessionStartMode } from './explore-agent.js'; import type { SubagentWorkspaceBinding } from './subagent-workspace.js'; +import type { EditingProtocol } from './apply-patch.js'; import type { EphemeralVoiceAudio } from './voice.js'; export type { TurnOrchestration } from './orchestration.js'; @@ -38,6 +39,8 @@ export interface CreateSessionInput { /** Per-model reasoning-depth variant; `undefined` = model default. */ thinkingLevel?: ThinkingLevel; permissionMode: PermissionMode; + /** Per-session editing surface; defaults to Edit/Write for compatibility. */ + editingProtocol?: EditingProtocol; /** Defaults to `agent`. */ collaborationMode?: CollaborationMode; /** Defaults to `default`. Orthogonal to Agent/Plan collaboration mode. */ diff --git a/packages/core/src/session.ts b/packages/core/src/session.ts index 42a272aab1..cf6b6e6805 100644 --- a/packages/core/src/session.ts +++ b/packages/core/src/session.ts @@ -42,6 +42,7 @@ import { normalizeToolResultContentForRead, } from './tool-result-record-schema.js'; import type { SubagentWorkspaceBinding } from './subagent-workspace.js'; +import type { EditingProtocol } from './apply-patch.js'; export { DEEP_RESEARCH_SESSION_LABEL, isDeepResearchSession } from './explore-agent.js'; @@ -242,6 +243,8 @@ export interface SessionHeader { /** Per-model reasoning-depth variant; `undefined` = model default. Cleared on model switch. */ thinkingLevel?: import('./model-thinking.js').ThinkingLevel; permissionMode: PermissionMode; + /** Editing surface captured when the session is created. Legacy records default to Edit/Write. */ + editingProtocol?: EditingProtocol; /** Defaults to `agent` when absent on legacy session records. */ collaborationMode?: CollaborationMode; /** Defaults to `default` when absent on legacy session records. */ @@ -311,6 +314,8 @@ export interface SessionSummary { /** Per-model reasoning-depth variant; `undefined` = model default. Cleared on model switch. */ thinkingLevel?: import('./model-thinking.js').ThinkingLevel; permissionMode: PermissionMode; + /** Editing surface captured for this session. Legacy summaries default to Edit/Write. */ + editingProtocol?: EditingProtocol; /** Defaults to `agent` when absent on legacy summaries. */ collaborationMode?: CollaborationMode; /** Defaults to `default` when absent on legacy summaries. */ diff --git a/packages/core/src/tool-catalog.ts b/packages/core/src/tool-catalog.ts index 914b1a1f4d..90e0e26628 100644 --- a/packages/core/src/tool-catalog.ts +++ b/packages/core/src/tool-catalog.ts @@ -86,6 +86,8 @@ export const MAKA_CATALOG_TOOLS: readonly CatalogToolDef[] = Object.freeze( { name: 'ArchiveRead' }, { name: 'Write' }, { name: 'Edit' }, + /** GPT/Codex-compatible editing projection (#1552); mutually exclusive with Write/Edit per run. */ + { name: 'ApplyPatch' }, { name: 'FormatJson' }, { name: 'Glob' }, { name: 'Grep' }, diff --git a/packages/headless/harbor/maka_agent.py b/packages/headless/harbor/maka_agent.py index 0fa61203e9..9ccf10d3b3 100644 --- a/packages/headless/harbor/maka_agent.py +++ b/packages/headless/harbor/maka_agent.py @@ -619,6 +619,7 @@ def _cell_env(self, instruction_path: Any) -> dict[str, str]: "MAKA_TRIAL_PRICING_SOURCE", "MAKA_REASONING_EFFORT", "MAKA_AGENT_TOOLS", + "MAKA_EDITING_PROTOCOL", "MAKA_MODEL_API_PROTOCOL", # Default per-command timeout floor for the in-container Bash tool, so # long builds/tests do not hit a hard-coded 2-minute ceiling. diff --git a/packages/headless/src/__tests__/cell-output.test.ts b/packages/headless/src/__tests__/cell-output.test.ts index 35535db220..274de088c1 100644 --- a/packages/headless/src/__tests__/cell-output.test.ts +++ b/packages/headless/src/__tests__/cell-output.test.ts @@ -224,7 +224,11 @@ describe('Harbor cell output contract', () => { pricingProfile: 'deepseek-v4-flash-tbench-v1', agentTools: true, productToolSurface: { - policy: { economy: true, disabledSurfaceIds: [] }, + policy: { + economy: true, + disabledSurfaceIds: [], + editingProtocol: 'edit_write', + }, productToolNames: ['Bash', 'agent_spawn'], }, supplementalToolSets: [ diff --git a/packages/headless/src/__tests__/harbor-adapter.test.ts b/packages/headless/src/__tests__/harbor-adapter.test.ts index 6181536158..be3f29ea37 100644 --- a/packages/headless/src/__tests__/harbor-adapter.test.ts +++ b/packages/headless/src/__tests__/harbor-adapter.test.ts @@ -29,6 +29,10 @@ function validContextEnvValue(key: string): string { return '1'; } +async function readRepoFile(relativePath: string): Promise { + return readFile(resolve(repoRoot, relativePath), 'utf8'); +} + describe('Harbor adapter contract', () => { test('Maka trajectory builder preserves multi-step tool pairing and fails closed to a summary', (t: TestContext) => { const providerOptionsEvent = decodeRuntimeEvent({ @@ -97,6 +101,75 @@ describe('Harbor adapter contract', () => { assert.match(result.stdout, /trajectory-harbor-contract ok/); }); + test('run-cell.mjs delegates to the shared env entrypoint', async () => { + const source = await readRepoFile('packages/headless/harbor/run-cell.mjs'); + const cellSource = await readRepoFile('packages/headless/src/harbor-cell.ts'); + + assert.match(source, /^#!\/usr\/bin\/env node/); + assert.match(source, /runHarborCellFromEnv/); + assert.match(source, /process\.env/); + assert.match(cellSource, /resolvedEnv\.MAKA_WORKDIR \?\? process\.cwd\(\)/); + }); + + test('maka_agent.py invokes run-cell with the shared artifact contract', async () => { + const source = await readRepoFile('packages/headless/harbor/maka_agent.py'); + const processScopeSource = await readRepoFile('packages/headless/harbor/process_scope.py'); + + assert.match(source, /class MakaAgent\(BaseInstalledAgent\):/); + assert.match(source, /async def install\(self, environment: BaseEnvironment\) -> None:/); + assert.match(source, /MAKA_INSTRUCTION_FILE/); + assert.match(source, /MAKA_SYSTEM_PROMPT/); + assert.match(source, /MAKA_OUTPUT_DIR/); + assert.match(source, /MAKA_STORAGE_ROOT/); + assert.match(source, /maka-cell-output\.json/); + assert.match(source, /run-cell\.mjs/); + assert.match(source, /run-host-cell\.mjs/); + assert.match(source, /MAKA_HOST_API_KEY_FILE/); + assert.match(source, /MAKA_HARBOR_TOOL_EXECUTOR_URL/); + assert.match(source, /_run_host_cell/); + assert.match(source, /_ToolExecutorServer/); + assert.match(source, /from process_scope import/); + assert.match(source, /_scoped_process_cleanup_command/); + assert.match(source, /_scoped_command_cleanup_command/); + assert.match(processScopeSource, /MAKA_HARBOR_COMMAND_SCOPE/); + assert.match(processScopeSource, /MAKA_HARBOR_COMMAND_ID/); + assert.match(processScopeSource, /def scoped_process_cleanup_command/); + assert.match(processScopeSource, /def scoped_command_cleanup_command/); + assert.match(processScopeSource, /\/proc\/\[0-9\]\*\/environ/); + assert.doesNotMatch(processScopeSource, /grep -Fqx/); + assert.match(processScopeSource, /grep -Fx -- .* > \/dev\/null/); + assert.match(source, /upload_file\(local_instruction_path, instruction_path\.as_posix\(\)\)/); + assert.match(source, /bash -lc/); + assert.match(source, /set -o pipefail/); + assert.doesNotMatch(source, /cd \/app/); + assert.doesNotMatch(source, /"MAKA_WORKDIR": "\/app"/); + assert.match(source, /Maka cell did not write/); + assert.match(source, /Maka cell output is not valid JSON/); + assert.match(source, /_read_cell_output\(required=True\)/); + assert.match(source, /MAKA_CELL_TIMEOUT_SEC/); + assert.match(source, /MAKA_CELL_SETTLEMENT_GRACE_SEC/); + assert.match(source, /MAKA_CELL_SOFT_TIMEOUT_MS/); + assert.match(source, /timeout_sec=self\._cell_timeout_sec\(\)/); + assert.match(source, /process\.returncode == 124/); + assert.match(source, /deepseek\/deepseek-v4-flash/); + assert.doesNotMatch(source, /deepseek\/deepseek-chat/); + assert.match(source, /economy_task_flag = self\._resolved_flags\.get\("economy_task_mode"\)/); + assert.match(source, /economy_task_env = self\._get_env\("MAKA_ECONOMY_TASK_MODE"\)/); + assert.match( + source, + /economy_task_mode = True if economy_task_flag is True else economy_task_env == "true"/, + ); + assert.match(source, /"MAKA_ECONOMY_TASK_MODE": "true" if economy_task_mode else "false"/); + assert.match(source, /"MAKA_AGENT_TOOLS"/); + assert.match(source, /"MAKA_EDITING_PROTOCOL"/); + assert.doesNotMatch( + source, + /"MAKA_ECONOMY_TASK_MODE": "true" if self\._resolved_flags\.get\("economy_task_mode"\) else "false"/, + ); + assert.doesNotMatch(source, /and raw else None/); + assert.doesNotMatch(source, /MAKA_INSTRUCTION_EOF/); + }); + test('maka_agent.py emits a host-cell timeout recognized as budget exhaustion', async () => { const source = await readFile( resolve(repoRoot, 'packages/headless/harbor/maka_agent.py'), @@ -2300,6 +2373,12 @@ with tempfile.TemporaryDirectory() as tmp: })._cell_env(Path("/logs/agent/instruction.txt")) assert agent_tools_env["MAKA_AGENT_TOOLS"] == "true", agent_tools_env + editing_protocol_env = MakaAgent(Path(tmp), extra_env={ + "MAKA_BACKEND": "fake", + "MAKA_EDITING_PROTOCOL": "apply_patch", + })._cell_env(Path("/logs/agent/instruction.txt")) + assert editing_protocol_env["MAKA_EDITING_PROTOCOL"] == "apply_patch", editing_protocol_env + # Host-side LLM mode must not forward provider secrets into the task-cell env. host_agent = MakaAgent(Path(tmp), extra_env={ "MAKA_HOST_API_KEY_FILE": "/host/secrets/deepseek-key", diff --git a/packages/headless/src/__tests__/harbor-cell.test.ts b/packages/headless/src/__tests__/harbor-cell.test.ts index 79e7ddca9a..84af5d3bb2 100644 --- a/packages/headless/src/__tests__/harbor-cell.test.ts +++ b/packages/headless/src/__tests__/harbor-cell.test.ts @@ -1071,6 +1071,31 @@ describe('runHarborCell', () => { }); }); + test('persists the apply_patch editing protocol on the durable session header', async () => { + await withDirs(async ({ workspaceDir, outputDir, storageRoot }) => { + const result = await runHarborCell({ + config: { ...config, editingProtocol: 'apply_patch' }, + instruction: 'write the answer in-place', + cwd: workspaceDir, + outputDir, + storageRoot, + registerBackends: registerCellBackend, + }); + + assert.equal(result.output.status, 'completed'); + // The product surface is projected from the config, but the durable + // header is what a spawned implementation child reads to derive its own + // tool surface; without persistence the child loses ApplyPatch (#1556). + const sessions = createSessionStore(storageRoot); + try { + const header = await sessions.readHeader(result.invocation.sessionId); + assert.equal(header.editingProtocol, 'apply_patch'); + } finally { + await sessions.close?.(); + } + }); + }); + test('does not provision child tools when Agent tools are disabled by default', async () => { await withDirs(async ({ workspaceDir, outputDir, storageRoot }) => { const observed: { spawned?: boolean; error?: string } = {}; @@ -1100,7 +1125,11 @@ describe('runHarborCell', () => { assert.equal(observed.spawned, false); assert.match(observed.error ?? '', /missing tools/i); assert.deepEqual(result.output.executionIdentity?.productToolSurface, { - policy: { economy: true, disabledSurfaceIds: ['agent'] }, + policy: { + economy: true, + disabledSurfaceIds: ['agent'], + editingProtocol: 'edit_write', + }, productToolNames: ['Bash', 'Edit', 'Glob', 'Grep', 'Read', 'Write'], }); }); diff --git a/packages/headless/src/__tests__/runner.test.ts b/packages/headless/src/__tests__/runner.test.ts index 3096eebdca..4d0ffbf74a 100644 --- a/packages/headless/src/__tests__/runner.test.ts +++ b/packages/headless/src/__tests__/runner.test.ts @@ -397,6 +397,7 @@ describe('fail-closed (a model-backed backend does not run without isolation)', backend: 'ai-sdk', llmConnectionSlug: 'deepseek', model: 'deepseek-chat', + editingProtocol: 'apply_patch', }; const task: Task = { id: 'real-task', @@ -409,7 +410,15 @@ describe('fail-closed (a model-backed backend does not run without isolation)', const result = await runExperiment(realConfig, task, { storageRoot, registerBackends: registerIsolatedRealBackend(contexts), - realBackendIsolation: { kind: 'external', label: 'unit-test isolated backend' }, + realBackendIsolation: { + kind: 'external', + label: 'unit-test isolated backend', + toolExecutor: { + async exec() { + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }, + }, }); assert.equal(result.status, 'completed'); @@ -418,6 +427,10 @@ describe('fail-closed (a model-backed backend does not run without isolation)', assert.equal(contexts[0]?.realBackendIsolation?.label, 'unit-test isolated backend'); assert.equal(contexts[0]?.config.id, 'real-cfg'); assert.equal(contexts[0]?.task.id, 'real-task'); + assert.equal(contexts[0]?.productToolSurface?.identity.policy.editingProtocol, 'apply_patch'); + assert.equal(contexts[0]?.productToolSurface?.toolNames.has('ApplyPatch'), true); + assert.equal(contexts[0]?.productToolSurface?.toolNames.has('Write'), false); + assert.equal(contexts[0]?.productToolSurface?.toolNames.has('Edit'), false); assert.equal(typeof contexts[0]?.spawnChildAgent, 'function'); assert.equal(typeof contexts[0]?.spawnChildSession, 'function'); assert.equal(typeof contexts[0]?.retryChildAgent, 'function'); @@ -430,6 +443,12 @@ describe('fail-closed (a model-backed backend does not run without isolation)', kind: 'external', revision: 0, }); + // The editing protocol must be persisted onto the session header, not + // only projected onto the product tool surface: an implementation + // child later derives its tool surface from the durable header, so a + // missing field there would filter ApplyPatch out of the child (#1556). + const header = await sessions.readHeader(result.sessionId); + assert.equal(header.editingProtocol, 'apply_patch'); } finally { await sessions.close?.(); } diff --git a/packages/headless/src/__tests__/task-agent-controller.test.ts b/packages/headless/src/__tests__/task-agent-controller.test.ts index 12435666d4..deb0add811 100644 --- a/packages/headless/src/__tests__/task-agent-controller.test.ts +++ b/packages/headless/src/__tests__/task-agent-controller.test.ts @@ -1311,7 +1311,11 @@ describe('runTaskOnce', () => { !result.projection.toolExecutors[0]?.toolNames.some((name) => name.startsWith('agent_')), ); assert.deepEqual(result.projection.toolExecutors[0]?.productToolSurface, { - policy: { economy: true, disabledSurfaceIds: ['agent'] }, + policy: { + economy: true, + disabledSurfaceIds: ['agent'], + editingProtocol: 'edit_write', + }, productToolNames: ['Bash', 'Edit', 'Glob', 'Grep', 'Read', 'Write'], }); assert.equal( @@ -1380,7 +1384,11 @@ describe('runTaskOnce', () => { assert.ok(result.projection.toolExecutors[0]?.toolNames.includes(toolName)); } assert.deepEqual(result.projection.toolExecutors[0]?.productToolSurface, { - policy: { economy: true, disabledSurfaceIds: [] }, + policy: { + economy: true, + disabledSurfaceIds: [], + editingProtocol: 'edit_write', + }, productToolNames: [ 'Bash', 'Edit', diff --git a/packages/headless/src/__tests__/tools.test.ts b/packages/headless/src/__tests__/tools.test.ts index 333cc9c172..b9ddf734e8 100644 --- a/packages/headless/src/__tests__/tools.test.ts +++ b/packages/headless/src/__tests__/tools.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { exec as childExec } from 'node:child_process'; import { createHash } from 'node:crypto'; -import { chmod, mkdir, mkdtemp, readFile, stat, symlink, writeFile } from 'node:fs/promises'; +import { chmod, mkdir, mkdtemp, readFile, rm, stat, symlink, writeFile } from 'node:fs/promises'; import { describe, test } from 'node:test'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -223,6 +223,25 @@ describe('isolated headless tools', () => { ); }); + test('nested writable targets emit real shell parameter expansion', async () => { + let captured = ''; + const tools = buildIsolatedHeadlessTools({ + async exec(input) { + captured = input.command; + return { exitCode: 0, stdout: fileToolOutputForCommand(input.command), stderr: '' }; + }, + }); + + await tool(tools, 'Write').impl( + { path: 'generated/deep/file.txt', content: 'nested\n' }, + toolCtx('/workspace'), + ); + + assert.equal(captured.includes('segment=${remaining%%/*}'), true); + assert.equal(captured.includes('remaining=${remaining#*/}'), true); + assert.equal(captured.includes(String.raw`segment=\${remaining%%/*}`), false); + }); + test('command-backed file tools forward active-turn cancellation to the isolated executor', async () => { const seenSignals: Array = []; const tools = buildIsolatedHeadlessTools({ @@ -1662,6 +1681,119 @@ describe('isolated headless tools', () => { assert.equal(await readFile(join(outside, 'secret.txt'), 'utf8'), 'outside needle\n'); }); + test('ApplyPatch creates destination parents and rejects canonical duplicate targets', async (t) => { + if (process.platform === 'win32') { + t.skip('the isolated command-backed executor requires a POSIX shell'); + return; + } + const cwd = await mkdtemp(join(tmpdir(), 'maka-headless-apply-patch-')); + try { + const tools = execBackedTools(process.env, 'apply_patch'); + const apply = tool(tools, 'ApplyPatch'); + const nested = await apply.impl( + { + patch: + '*** Begin Patch\n' + + '*** Add File: generated/deep/file.txt\n' + + '+nested\n' + + '*** End Patch\n', + }, + toolCtx(cwd), + ); + assert.equal((nested as { ok: boolean }).ok, true); + assert.equal(await readFile(join(cwd, 'generated', 'deep', 'file.txt'), 'utf8'), 'nested\n'); + + await assert.rejects( + async () => + await apply.impl( + { + patch: + '*** Begin Patch\n' + + '*** Add File: duplicate.txt\n' + + '+first\n' + + '*** Add File: ./duplicate.txt\n' + + '+second\n' + + '*** End Patch\n', + }, + toolCtx(cwd), + ), + /already exists/i, + ); + await assert.rejects(() => readFile(join(cwd, 'duplicate.txt'), 'utf8')); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test('ApplyPatch treats a symlink entry as an existing Add target before any mutation', async (t) => { + if (process.platform === 'win32') { + t.skip('file symlink creation is not reliably available on Windows CI'); + return; + } + const cwd = await mkdtemp(join(tmpdir(), 'maka-headless-apply-patch-symlink-')); + const outside = await mkdtemp(join(tmpdir(), 'maka-headless-apply-patch-outside-')); + try { + await writeFile(join(cwd, 'first.txt'), 'before\n', 'utf8'); + await writeFile(join(outside, 'secret.txt'), 'secret\n', 'utf8'); + await symlink(join(outside, 'secret.txt'), join(cwd, 'escape.txt')); + const apply = tool(execBackedTools(process.env, 'apply_patch'), 'ApplyPatch'); + + await assert.rejects( + async () => + await apply.impl( + { + patch: + '*** Begin Patch\n' + + '*** Update File: first.txt\n' + + '@@\n' + + '-before\n' + + '+after\n' + + '*** Add File: escape.txt\n' + + '+overwrite\n' + + '*** End Patch\n', + }, + toolCtx(cwd), + ), + /already exists/i, + ); + assert.equal(await readFile(join(cwd, 'first.txt'), 'utf8'), 'before\n'); + assert.equal(await readFile(join(outside, 'secret.txt'), 'utf8'), 'secret\n'); + } finally { + await rm(cwd, { recursive: true, force: true }); + await rm(outside, { recursive: true, force: true }); + } + }); + + test('ApplyPatch Delete removes the symlink entry and leaves its target intact', async (t) => { + if (process.platform === 'win32') { + t.skip('file symlink creation is not reliably available on Windows CI'); + return; + } + const cwd = await mkdtemp(join(tmpdir(), 'maka-headless-apply-patch-delete-link-')); + const outside = await mkdtemp(join(tmpdir(), 'maka-headless-apply-patch-delete-outside-')); + try { + await writeFile(join(outside, 'secret.txt'), 'secret\n', 'utf8'); + await symlink(join(outside, 'secret.txt'), join(cwd, 'escape.txt')); + const apply = tool(execBackedTools(process.env, 'apply_patch'), 'ApplyPatch'); + + const result = await apply.impl( + { + patch: '*** Begin Patch\n' + '*** Delete File: escape.txt\n' + '*** End Patch\n', + }, + toolCtx(cwd), + ); + // Entry-delete semantics: the link itself is removed, matching the + // shared engine's lstat/delete contract, without following it to the + // file outside the workspace. + assert.equal((result as { ok: boolean }).ok, true); + await assert.rejects(() => stat(join(cwd, 'escape.txt')), { code: 'ENOENT' }); + assert.equal(await readFile(join(outside, 'secret.txt'), 'utf8'), 'secret\n'); + } finally { + await rm(cwd, { recursive: true, force: true }); + await rm(outside, { recursive: true, force: true }); + } + }); + test('isolated file tools reject path escapes before executor invocation', async () => { let calls = 0; const tools = buildIsolatedHeadlessTools({ @@ -1806,25 +1938,31 @@ describe('isolated headless tools', () => { }); }); -function execBackedTools(env: NodeJS.ProcessEnv = process.env) { - return buildIsolatedHeadlessTools({ - async exec(input) { - try { - const { stdout, stderr } = await execAsync(input.command, { - cwd: input.cwd, - env, - maxBuffer: 1024 * 1024, - }); - return { exitCode: 0, stdout, stderr }; - } catch (error: any) { - return { - exitCode: typeof error?.code === 'number' ? error.code : 1, - stdout: typeof error?.stdout === 'string' ? error.stdout : '', - stderr: typeof error?.stderr === 'string' ? error.stderr : String(error), - }; - } +function execBackedTools( + env: NodeJS.ProcessEnv = process.env, + editingProtocol: 'edit_write' | 'apply_patch' = 'edit_write', +) { + return buildIsolatedHeadlessTools( + { + async exec(input) { + try { + const { stdout, stderr } = await execAsync(input.command, { + cwd: input.cwd, + env, + maxBuffer: 1024 * 1024, + }); + return { exitCode: 0, stdout, stderr }; + } catch (error: any) { + return { + exitCode: typeof error?.code === 'number' ? error.code : 1, + stdout: typeof error?.stdout === 'string' ? error.stdout : '', + stderr: typeof error?.stderr === 'string' ? error.stderr : String(error), + }; + } + }, }, - }); + { editingProtocol }, + ); } function tool(tools: ReturnType, name: string) { diff --git a/packages/headless/src/cell-output.ts b/packages/headless/src/cell-output.ts index 4f5fe05702..e00742c7ac 100644 --- a/packages/headless/src/cell-output.ts +++ b/packages/headless/src/cell-output.ts @@ -479,6 +479,17 @@ function validateProductToolSurfaceIdentity(value: unknown): ProductToolSurfaceI ); } } + const editingProtocolRaw = value.policy.editingProtocol; + const editingProtocol = + editingProtocolRaw === undefined + ? 'edit_write' + : editingProtocolRaw === 'edit_write' || editingProtocolRaw === 'apply_patch' + ? editingProtocolRaw + : (() => { + throw new Error( + 'executionIdentity.productToolSurface.policy.editingProtocol must be "edit_write" or "apply_patch"', + ); + })(); return { policy: { economy: requireBoolean( @@ -486,6 +497,7 @@ function validateProductToolSurfaceIdentity(value: unknown): ProductToolSurfaceI 'executionIdentity.productToolSurface.policy.economy', ), disabledSurfaceIds, + editingProtocol, }, productToolNames, }; diff --git a/packages/headless/src/contracts.ts b/packages/headless/src/contracts.ts index c453d4db08..c15e464823 100644 --- a/packages/headless/src/contracts.ts +++ b/packages/headless/src/contracts.ts @@ -19,6 +19,7 @@ import type { OrchestrationMode, ThinkingLevel, } from '@maka/core'; +import { resolveEditingProtocolEnv, type EditingProtocol } from '@maka/core/apply-patch'; /** * A unit of work the lab runs a Config against. Field names lean toward @@ -167,6 +168,15 @@ export interface Config { * deferred `load_tools` activation. */ agentTools?: boolean; + /** Explicit per-run editing surface override; current Edit/Write remains the default. */ + editingProtocol?: EditingProtocol; +} + +export function editingProtocolFromValue( + value: string | undefined, + label = 'MAKA_EDITING_PROTOCOL', +): EditingProtocol | undefined { + return resolveEditingProtocolEnv(value, label); } export interface HeavyTaskModeConfig { diff --git a/packages/headless/src/fixed-prompt-controller.ts b/packages/headless/src/fixed-prompt-controller.ts index f9bafe794b..d487fd0552 100644 --- a/packages/headless/src/fixed-prompt-controller.ts +++ b/packages/headless/src/fixed-prompt-controller.ts @@ -944,6 +944,8 @@ function classifyExecutionIdentityFailure( ? !identity.productToolSurface.policy.disabledSurfaceIds.includes('agent') : identity.agentTools) !== (expectedConfig.agentTools === true) || + (identity.productToolSurface?.policy.editingProtocol ?? 'edit_write') !== + (expectedConfig.editingProtocol ?? 'edit_write') || identity.systemPromptHash !== expectedPromptHash || (expectedPricingProfile !== undefined && identity.pricingProfile !== expectedPricingProfile) ) { diff --git a/packages/headless/src/harbor-cell.ts b/packages/headless/src/harbor-cell.ts index 507d484629..16fd942f79 100644 --- a/packages/headless/src/harbor-cell.ts +++ b/packages/headless/src/harbor-cell.ts @@ -54,7 +54,7 @@ import { type HarborCellExecutionIdentity, type HarborCellOutput, } from './cell-output.js'; -import type { Config, Task } from './contracts.js'; +import { editingProtocolFromValue, type Config, type Task } from './contracts.js'; import { resolveHeavyTaskMode } from './heavy-task-policy.js'; import { resolveEconomyTaskMode } from './economy-task-policy.js'; import { @@ -358,6 +358,7 @@ export async function runHarborCellWithStorage( input.realBackendIsolation?.toolExecutor, { agentTools: config.agentTools, + ...(config.editingProtocol ? { editingProtocol: config.editingProtocol } : {}), snapshotImage: createReadImageSnapshotter(storage.artifactStore), ...(input.archiveResources ? { archiveResources: input.archiveResources } : {}), }, @@ -421,6 +422,7 @@ export async function runHarborCellWithStorage( model: config.model, ...(config.thinkingLevel ? { thinkingLevel: config.thinkingLevel } : {}), permissionMode: 'ask', + ...(config.editingProtocol ? { editingProtocol: config.editingProtocol } : {}), name: `harbor-cell:${input.config.id}`, }, { initialBoundary: { kind: 'external', revision: 0 } }, @@ -772,10 +774,15 @@ export async function runHarborCellFromEnv( const settleAfterMs = harborCellSoftTimeoutMsFromEnv(resolvedEnv); const reasoningEffort = reasoningEffortFromEnv(resolvedEnv.MAKA_REASONING_EFFORT); const agentTools = booleanEnv(resolvedEnv.MAKA_AGENT_TOOLS, 'MAKA_AGENT_TOOLS') ?? false; + const editingProtocol = editingProtocolFromValue( + resolvedEnv.MAKA_EDITING_PROTOCOL, + 'MAKA_EDITING_PROTOCOL', + ); const baseConfig = { id: resolvedEnv.MAKA_CONFIG_ID ?? 'harbor-cell', backend, agentTools, + ...(editingProtocol ? { editingProtocol } : {}), ...(reasoningEffort ? { thinkingLevel: reasoningEffort } : {}), ...(resolvedEnv.MAKA_SYSTEM_PROMPT !== undefined ? { systemPrompt: resolvedEnv.MAKA_SYSTEM_PROMPT } diff --git a/packages/headless/src/harbor-cli.ts b/packages/headless/src/harbor-cli.ts index 24dce36e11..325220664e 100644 --- a/packages/headless/src/harbor-cli.ts +++ b/packages/headless/src/harbor-cli.ts @@ -5,7 +5,7 @@ import { homedir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import type { BackendKind, ProviderType } from '@maka/core'; import { PROVIDER_DEFAULTS, normalizeProviderType } from '@maka/core'; -import type { Config, Task } from './contracts.js'; +import { editingProtocolFromValue, type Config, type Task } from './contracts.js'; import { type HarborCellExecutionIdentity, combineInvocations, @@ -454,6 +454,7 @@ export async function resolveHarborRunOptions( ? true : booleanEnv(env.MAKA_ECONOMY_TASK_MODE, 'MAKA_ECONOMY_TASK_MODE'), agentTools: booleanEnv(env.MAKA_AGENT_TOOLS, 'MAKA_AGENT_TOOLS') ?? false, + editingProtocol: editingProtocolFromValue(env.MAKA_EDITING_PROTOCOL, 'MAKA_EDITING_PROTOCOL'), }); const realBackendIsolation = buildIsolation(isolation, env, workdir, outDir); const providerEnvFetch = backend === 'ai-sdk' ? createProviderEnvFetch(env) : undefined; @@ -582,6 +583,7 @@ function buildConfig(input: { heavyTask: boolean; economyTask: boolean | undefined; agentTools: boolean; + editingProtocol: Config['editingProtocol']; }): Config { const thinkingLevel = reasoningEffortFromEnv(input.env.MAKA_REASONING_EFFORT); const economyTaskMode = @@ -603,6 +605,7 @@ function buildConfig(input: { model: input.env.MAKA_MODEL ?? input.env.HARBOR_MODEL ?? 'fake', ...(thinkingLevel ? { thinkingLevel } : {}), agentTools: input.agentTools, + ...(input.editingProtocol ? { editingProtocol: input.editingProtocol } : {}), ...(input.heavyTask ? { heavyTaskMode: { enabled: true, reason: 'maka eval harbor run --heavy-task' } } : {}), @@ -626,6 +629,7 @@ function buildConfig(input: { model: modelSpec.model, ...(thinkingLevel ? { thinkingLevel } : {}), agentTools: input.agentTools, + ...(input.editingProtocol ? { editingProtocol: input.editingProtocol } : {}), ...(input.env.MAKA_SYSTEM_PROMPT !== undefined ? { systemPrompt: input.env.MAKA_SYSTEM_PROMPT } : {}), diff --git a/packages/headless/src/harbor-task-runner.ts b/packages/headless/src/harbor-task-runner.ts index e3beeca02d..a40b92441b 100644 --- a/packages/headless/src/harbor-task-runner.ts +++ b/packages/headless/src/harbor-task-runner.ts @@ -1124,6 +1124,9 @@ export function buildHarborJobConfig( // Verbatim — the controller hashes exactly these bytes and verifies the round-trip. MAKA_SYSTEM_PROMPT: input.systemPrompt, }; + if (input.config.editingProtocol) { + agentEnv.MAKA_EDITING_PROTOCOL = input.config.editingProtocol; + } if (options.reasoningEffort) { agentEnv.MAKA_REASONING_EFFORT = options.reasoningEffort; if (adapter === 'opencode') agentEnv.MAKA_OPENCODE_VARIANT = options.reasoningEffort; diff --git a/packages/headless/src/heavy-task-evidence.ts b/packages/headless/src/heavy-task-evidence.ts index 28ae265ade..f867dc4254 100644 --- a/packages/headless/src/heavy-task-evidence.ts +++ b/packages/headless/src/heavy-task-evidence.ts @@ -80,6 +80,24 @@ export type HeavyTaskToolEvidenceInput = input: IsolatedEditFileInput; result: IsolatedEditFileResult; } + | { + name: 'ApplyPatch'; + input: { cwd: string; patch: string }; + result: { + ok: boolean; + operations: Array<{ + operation: string; + path: string; + fromPath?: string; + status: string; + bytes?: number; + }>; + completed: string[]; + uncompleted: string[]; + partial?: boolean; + error?: string; + }; + } | { name: 'Glob'; input: IsolatedGlobInput; @@ -245,6 +263,30 @@ export function compactToolEvidence( diff: notCapturedDiff(input.result.path), }, }; + case 'ApplyPatch': { + const completedPaths = input.result.completed; + const primaryPath = completedPaths[0] ?? input.result.operations[0]?.path; + return { + ...base, + tool: { + name: 'ApplyPatch', + inputSummary: { + cwd: compactPublicString(input.input.cwd, 500), + patchBytes: Buffer.byteLength(input.input.patch, 'utf8'), + patchOmitted: true, + operationCount: input.result.operations.length, + completedPaths: completedPaths.map((path) => compactPublicString(path, 500)), + uncompletedPaths: input.result.uncompleted.map((path) => + compactPublicString(path, 500), + ), + ...(input.result.partial !== undefined ? { partial: input.result.partial } : {}), + }, + ok: input.result.ok, + outputs: [omittedOutput('content', Buffer.byteLength(input.input.patch, 'utf8'))], + diff: primaryPath ? notCapturedDiff(primaryPath) : { status: 'not_captured' }, + }, + }; + } case 'Glob': return { ...base, diff --git a/packages/headless/src/isolation.ts b/packages/headless/src/isolation.ts index 6af54d955f..c8e76f99d4 100644 --- a/packages/headless/src/isolation.ts +++ b/packages/headless/src/isolation.ts @@ -116,6 +116,7 @@ export const ISOLATED_HEADLESS_TOOL_NAMES = [ 'Read', 'Write', 'Edit', + 'ApplyPatch', 'Glob', 'Grep', ] as const; diff --git a/packages/headless/src/pier-task-runner.ts b/packages/headless/src/pier-task-runner.ts index 950c3f574f..5f79afb16c 100644 --- a/packages/headless/src/pier-task-runner.ts +++ b/packages/headless/src/pier-task-runner.ts @@ -773,6 +773,7 @@ function buildPierAgentEnv( // whitespace from --ae values, and a stripped copy in the adapter's // extra_env would shadow the byte-exact os.environ value. See processEnv. }; + if (input.config.editingProtocol) env.MAKA_EDITING_PROTOCOL = input.config.editingProtocol; if (options.reasoningEffort) env.MAKA_REASONING_EFFORT = options.reasoningEffort; if (agent === 'maka' && options.makaNodeToolchainPath) { env.MAKA_NODE_TOOLCHAIN_FINGERPRINT = MAKA_NODE_TOOLCHAIN_FINGERPRINT; diff --git a/packages/headless/src/runner.ts b/packages/headless/src/runner.ts index f69337d37b..01433e1466 100644 --- a/packages/headless/src/runner.ts +++ b/packages/headless/src/runner.ts @@ -135,6 +135,9 @@ export async function runExperimentWithStorage( deps.realBackendIsolation?.toolExecutor, { agentTools: effectiveConfig.agentTools, + ...(effectiveConfig.editingProtocol + ? { editingProtocol: effectiveConfig.editingProtocol } + : {}), snapshotImage: createReadImageSnapshotter(storage.artifactStore), }, ); @@ -185,6 +188,9 @@ export async function runExperimentWithStorage( llmConnectionSlug: config.llmConnectionSlug, model: config.model, permissionMode: 'ask', + ...(effectiveConfig.editingProtocol + ? { editingProtocol: effectiveConfig.editingProtocol } + : {}), ...(deps.orchestrationMode ? { orchestrationMode: deps.orchestrationMode } : {}), name: `lab:${config.id}:${task.id}`, }, diff --git a/packages/headless/src/task-agent-controller.ts b/packages/headless/src/task-agent-controller.ts index b8e769f061..aa1c789bb3 100644 --- a/packages/headless/src/task-agent-controller.ts +++ b/packages/headless/src/task-agent-controller.ts @@ -255,6 +255,9 @@ export async function runTaskOnceWithStorage( deps.realBackendIsolation?.toolExecutor, { agentTools: effectiveConfig.agentTools, + ...(effectiveConfig.editingProtocol + ? { editingProtocol: effectiveConfig.editingProtocol } + : {}), ...(heavyTaskEvidence ? { heavyTaskEvidence } : {}), snapshotImage: createReadImageSnapshotter(storage.artifactStore), }, @@ -359,6 +362,9 @@ export async function runTaskOnceWithStorage( ? { thinkingLevel: effectiveConfig.thinkingLevel } : {}), permissionMode: 'ask', + ...(effectiveConfig.editingProtocol + ? { editingProtocol: effectiveConfig.editingProtocol } + : {}), ...(deps.orchestrationMode ? { orchestrationMode: deps.orchestrationMode } : {}), name: `task:${config.id}:${task.id}`, }, diff --git a/packages/headless/src/task-contracts.ts b/packages/headless/src/task-contracts.ts index 95e9da8e00..d5c31ede98 100644 --- a/packages/headless/src/task-contracts.ts +++ b/packages/headless/src/task-contracts.ts @@ -588,6 +588,7 @@ export type HeavyTaskToolEvidenceName = | 'Grep' | 'Write' | 'Edit' + | 'ApplyPatch' | 'Glob' | string; diff --git a/packages/headless/src/tools.ts b/packages/headless/src/tools.ts index f054e8f38e..c805915681 100644 --- a/packages/headless/src/tools.ts +++ b/packages/headless/src/tools.ts @@ -4,6 +4,7 @@ import type { MakaTool, ToolResultArchiveResourceReader, } from '@maka/runtime'; +import { type EditingProtocol } from '@maka/core/apply-patch'; import { assertProductBindingCatalogClean, bashToolShellGuidance, @@ -11,9 +12,11 @@ import { buildForegroundBashTool, buildParentAgentTools, computeEditedSource, + executeApplyPatchWithAdapter, isSupportedImagePath, projectEffectiveProductToolSurface, validateImageBytes, + type ApplyPatchFsAdapter, } from '@maka/runtime'; import { withFileWriteLock } from '@maka/runtime/file-write-lock'; import { createHash } from 'node:crypto'; @@ -36,6 +39,8 @@ import { export interface BuildIsolatedHeadlessToolsOptions { agentTools?: boolean; + /** Per-run editing protocol consumed only by the product-tool projector. */ + editingProtocol?: EditingProtocol; /** * Ref-addressed archive reader. Supplying it binds `ArchiveRead`, which pruned * tool results name explicitly in their placeholders. Pass it whenever the host @@ -99,14 +104,18 @@ export function buildIsolatedHeadlessProductToolSurface( executor: IsolatedToolExecutor, options: Pick< BuildIsolatedHeadlessToolsOptions, - 'agentTools' | 'archiveResources' | 'heavyTaskEvidence' | 'snapshotImage' + 'agentTools' | 'archiveResources' | 'heavyTaskEvidence' | 'snapshotImage' | 'editingProtocol' > = {}, ): EffectiveProductToolSurface { + const editingProtocol: EditingProtocol = options.editingProtocol ?? 'edit_write'; + // Bind every editing protocol implementation; the product-tool projector + // selects exactly one via policy.editingProtocol (#1493 / #1552). const productTools = [ buildIsolatedBashTool(executor, options), buildIsolatedReadTool(executor, options), buildIsolatedWriteTool(executor, options), buildIsolatedEditTool(executor, options), + buildIsolatedApplyPatchTool(executor, options), buildIsolatedGlobTool(executor, options), buildIsolatedGrepTool(executor, options), ...buildParentAgentTools(), @@ -121,6 +130,7 @@ export function buildIsolatedHeadlessProductToolSurface( tools: productTools, policy: { economy: true, + editingProtocol, ...(options.agentTools ? {} : { disabledSurfaceIds: ['agent'] }), }, }); @@ -135,7 +145,7 @@ export function buildHeadlessProductToolSurfaceForBackend( executor: IsolatedToolExecutor | undefined, options: Pick< BuildIsolatedHeadlessToolsOptions, - 'agentTools' | 'archiveResources' | 'heavyTaskEvidence' | 'snapshotImage' + 'agentTools' | 'archiveResources' | 'heavyTaskEvidence' | 'snapshotImage' | 'editingProtocol' > = {}, ): EffectiveProductToolSurface | undefined { if (!headlessBackendBindsMakaProductTools(backend) || !executor) return undefined; @@ -305,6 +315,115 @@ export function buildIsolatedWriteTool( }; } +export function buildIsolatedApplyPatchTool( + executor: IsolatedToolExecutor, + options: Pick = {}, +): MakaTool { + return { + name: 'ApplyPatch', + activityKind: 'edit', + categoryHint: 'file_write', + description: + 'Apply a Codex-compatible multi-file patch in the isolated headless task workspace. ' + + 'Pass the full *** Begin Patch … *** End Patch envelope. Parsed and preflighted ' + + 'before any mutation; absolute paths and parent escapes are rejected.', + parameters: z.object({ + patch: z.string().describe('Full *** Begin Patch … *** End Patch text.'), + }), + impl: async ({ patch }, ctx) => { + const { cwd } = ctx; + const fs = createIsolatedApplyPatchFs(executor, cwd, ctx.abortSignal); + const result = await executeApplyPatchWithAdapter(patch, fs, withFileWriteLock); + await options.heavyTaskEvidence?.recordToolEvidence( + { + name: 'ApplyPatch', + input: { cwd, patch }, + result: { + ok: result.ok, + operations: result.operations.map((operation) => ({ + operation: operation.operation, + path: operation.path, + ...(operation.fromPath ? { fromPath: operation.fromPath } : {}), + status: operation.status, + ...(operation.bytes !== undefined ? { bytes: operation.bytes } : {}), + })), + completed: result.completed, + uncompleted: result.uncompleted, + ...(result.partial !== undefined ? { partial: result.partial } : {}), + ...(result.error ? { error: result.error } : {}), + }, + }, + ctx, + ); + return result; + }, + }; +} + +function createIsolatedApplyPatchFs( + executor: IsolatedToolExecutor, + cwd: string, + abortSignal: AbortSignal, +): ApplyPatchFsAdapter { + return { + async lockKey(path) { + const normalized = normalizeWorkspacePath(path, cwd, 'ApplyPatch path'); + return fileWriteKey(cwd, normalized); + }, + async lstat(path) { + const normalized = normalizeWorkspacePath(path, cwd, 'ApplyPatch exists path'); + const kind = ( + await execFileCommand( + executor, + cwd, + shellFileCommand(LSTAT_SCRIPT, [normalized]), + abortSignal, + ) + ).trim(); + if (kind === 'missing' || kind === 'file' || kind === 'symlink' || kind === 'other') { + return kind; + } + throw new Error(`ApplyPatch lstat returned an invalid entry type for ${normalized}`); + }, + async readText(path, label) { + const normalized = normalizeWorkspacePath(path, cwd, label); + const raw = await readIsolatedFileBytes( + executor, + cwd, + normalized, + abortSignal, + EDIT_READ_BYTES_SCRIPT, + label, + ); + const source = raw.toString('utf8'); + if (Buffer.compare(Buffer.from(source, 'utf8'), raw) !== 0) { + throw new Error(`${label} does not support non-UTF-8 files: ${normalized}`); + } + return source; + }, + async writeText(path, content, mode) { + const normalized = normalizeWorkspacePath(path, cwd, 'ApplyPatch write path'); + const bytes = Buffer.from(content, 'utf8'); + // Create-capable write: Add/Move destinations must not require an existing target. + // Edit's atomic script uses existing_target and fails new files with "does not exist". + await writeIsolatedFileBytes( + executor, + cwd, + normalized, + bytes, + abortSignal, + mode === 'create' ? 'create' : 'edit', + ); + return { path: normalized, bytes: bytes.length }; + }, + async deletePath(path) { + const normalized = normalizeWorkspacePath(path, cwd, 'ApplyPatch delete path'); + await deleteIsolatedPath(executor, cwd, normalized, abortSignal); + return { path: normalized }; + }, + }; +} + export function buildIsolatedEditTool( executor: IsolatedToolExecutor, options: Pick = {}, @@ -584,15 +703,26 @@ async function writeIsolatedFileBytes( path: string, content: Buffer, abortSignal: AbortSignal, + mode: 'edit' | 'create' = 'edit', ): Promise { + const script = mode === 'create' ? CREATE_WRITE_BYTES_SCRIPT : EDIT_WRITE_BYTES_SCRIPT; await execFileCommand( executor, cwd, - shellFileCommand(EDIT_WRITE_BYTES_SCRIPT, [path, content.toString('base64')]), + shellFileCommand(script, [path, content.toString('base64')]), abortSignal, ); } +async function deleteIsolatedPath( + executor: IsolatedToolExecutor, + cwd: string, + path: string, + abortSignal: AbortSignal, +): Promise { + await execFileCommand(executor, cwd, shellFileCommand(DELETE_SCRIPT, [path]), abortSignal); +} + function computeEditBytes( raw: Buffer, oldString: string, @@ -735,15 +865,16 @@ function assertNoDriveOrParentSegment(inputPath: string, label: string): void { } function assertNormalizedRelativePath(inputPath: string, label: string): string { + const normalized = pathPosix.normalize(inputPath.replaceAll('\\', '/')); if ( - inputPath.length === 0 || - inputPath.startsWith('/') || - /^[A-Za-z]:[\\/]/.test(inputPath) || - inputPath.split(/[\\/]+/).includes('..') + normalized.length === 0 || + normalized.startsWith('/') || + /^[A-Za-z]:[\\/]/.test(normalized) || + normalized.split(/[\\/]+/).includes('..') ) { throw new Error(`${label} must stay inside the isolated workspace`); } - return inputPath; + return normalized; } const COMMON_SHELL_HELPERS = String.raw` @@ -782,7 +913,11 @@ existing_target() { printf '%s\n' "$real" } -writable_target() { +# Delete/Move operands address a directory entry, never the link target +# (matches the shared ApplyPatch engine's lstat/delete semantics): canonicalize +# the parent, check containment, then hand back the entry itself without +# following a final symlink. +delete_operand() { input_path=$1 label=$2 target=$root/$input_path @@ -790,6 +925,41 @@ writable_target() { base=$(basename "$target") parent_real=$(cd -P "$parent" 2>/dev/null && pwd -P) || fail "$label must stay inside workspace" inside_workspace "$parent_real" || fail "$label must stay inside workspace" + entry=$parent_real/$base + [ -e "$entry" ] || [ -L "$entry" ] || fail "$label does not exist: $input_path" + printf '%s\n' "$entry" +} + +writable_target() { + input_path=$1 + label=$2 + parent_rel=$(dirname "$input_path") + parent_real=$root + remaining=$parent_rel + while [ "$remaining" != "." ] && [ -n "$remaining" ]; do + case "$remaining" in + */*) + segment=${'${remaining%%/*}'} + remaining=${'${remaining#*/}'} + ;; + *) + segment=$remaining + remaining=. + ;; + esac + [ "$segment" = "." ] && continue + next=$parent_real/$segment + [ -L "$next" ] && fail "$label must stay inside workspace" + if [ -e "$next" ]; then + [ -d "$next" ] || fail "$label parent is not a directory" + else + mkdir "$next" || fail "$label parent could not be created" + fi + parent_real=$(cd -P "$next" 2>/dev/null && pwd -P) || + fail "$label must stay inside workspace" + inside_workspace "$parent_real" || fail "$label must stay inside workspace" + done + base=$(basename "$input_path") real=$parent_real/$base [ -L "$real" ] && fail "$label must stay inside workspace" printf '%s\n' "$real" @@ -854,12 +1024,60 @@ LC_ALL=C awk -v start="$offset" -v limit="$limit" ' ' "$target" `; +const LSTAT_SCRIPT = `${COMMON_SHELL_HELPERS} +root=$(pwd -P) || exit 1 +input_path=$1 +parent_rel=$(dirname "$input_path") +parent=$root +remaining=$parent_rel +while [ "$remaining" != "." ] && [ -n "$remaining" ]; do + case "$remaining" in + */*) + segment=${'${remaining%%/*}'} + remaining=${'${remaining#*/}'} + ;; + *) + segment=$remaining + remaining=. + ;; + esac + [ "$segment" = "." ] && continue + next=$parent/$segment + [ -L "$next" ] && fail 'ApplyPatch lstat parent must stay inside workspace' + if [ ! -e "$next" ]; then + printf 'missing\n' + exit 0 + fi + [ -d "$next" ] || fail 'ApplyPatch lstat parent is not a directory' + parent=$(cd -P "$next" 2>/dev/null && pwd -P) || + fail 'ApplyPatch lstat parent must stay inside workspace' + inside_workspace "$parent" || fail 'ApplyPatch lstat parent must stay inside workspace' +done +target=$parent/$(basename "$input_path") +if [ -L "$target" ]; then + printf 'symlink\n' +elif [ -f "$target" ]; then + printf 'file\n' +elif [ -e "$target" ]; then + printf 'other\n' +else + printf 'missing\n' +fi +`; + const WRITE_SCRIPT = `${COMMON_SHELL_HELPERS} root=$(pwd -P) || exit 1 target=$(writable_target "$1" 'Write path') || exit 1 printf '%s' "$2" > "$target" `; +const DELETE_SCRIPT = `${COMMON_SHELL_HELPERS} +root=$(pwd -P) || exit 1 +target=$(delete_operand "$1" 'Delete path') || exit 1 +[ -f "$target" ] || [ -L "$target" ] || fail 'Delete path must be a regular file' +rm -f "$target" +`; + const EDIT_READ_BYTES_SCRIPT = `${COMMON_SHELL_HELPERS} root=$(pwd -P) || exit 1 target=$(existing_target "$1" 'Edit path') || exit 1 @@ -919,6 +1137,35 @@ created= trap - EXIT HUP INT TERM `; +// No-clobber create for ApplyPatch Add/Move destinations. The temporary file +// is linked into place atomically so a destination that appears after planning +// cannot be overwritten. +const CREATE_WRITE_BYTES_SCRIPT = `${COMMON_SHELL_HELPERS} +root=$(pwd -P) || exit 1 +target=$(writable_target "$1" 'Write path') || exit 1 +payload=$2 +tmp= +created= +cleanup() { + [ -n "$created" ] && [ -n "$tmp" ] && rm -f "$tmp" +} +trap cleanup EXIT HUP INT TERM +tmp=$(mktemp "$target.maka-write.XXXXXX") || fail 'Write temp file creation failed' +created=1 +if printf '%s' "$payload" | base64 -d > "$tmp" 2>/dev/null; then + : +elif printf '%s' "$payload" | base64 -D > "$tmp" 2>/dev/null; then + : +else + fail 'base64 decode failed for Write payload' +fi +[ ! -e "$target" ] && [ ! -L "$target" ] || fail 'Write target already exists' +ln "$tmp" "$target" || fail 'Write target already exists' +rm -f "$tmp" +created= +trap - EXIT HUP INT TERM +`; + const GLOB_SCRIPT = `${COMMON_SHELL_HELPERS} root=$(pwd -P) || exit 1 pattern=$1 diff --git a/packages/runtime/src/__tests__/apply-patch-tool.test.ts b/packages/runtime/src/__tests__/apply-patch-tool.test.ts new file mode 100644 index 0000000000..82235b985d --- /dev/null +++ b/packages/runtime/src/__tests__/apply-patch-tool.test.ts @@ -0,0 +1,646 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { lstat, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { buildBuiltinTools } from '../builtin-tools.js'; +import { createWorkspaceWritePermissionProfile } from '@maka/core/permission-profile'; +import { executeApplyPatchWithAdapter, type ApplyPatchFsAdapter } from '../apply-patch-engine.js'; +import { projectEffectiveProductToolSurface } from '../tool-catalog-derive.js'; +import { executeFilesystemWorkerRequest } from '../filesystem-worker/operations.js'; +import { + FILESYSTEM_WORKER_PROTOCOL_VERSION, + type FilesystemWorkerOperation, + type FilesystemWorkerRequest, + type FilesystemWorkerTarget, +} from '../filesystem-worker/protocol.js'; +import type { MakaToolContext } from '../tool-runtime.js'; + +function toolCtx(cwd: string): MakaToolContext { + return { + sessionId: 's', + turnId: 't', + cwd, + toolCallId: 'c', + abortSignal: new AbortController().signal, + emitOutput: () => {}, + }; +} + +function envelope(body: string): string { + return `*** Begin Patch\n${body}*** End Patch\n`; +} + +describe('editingProtocol projection', () => { + test('edit_write is the default and omits ApplyPatch', () => { + const surface = projectEffectiveProductToolSurface({ + host: 'cli', + tools: buildBuiltinTools(), + policy: { economy: false }, + }); + assert.ok(surface.toolNames.has('Write')); + assert.ok(surface.toolNames.has('Edit')); + assert.equal(surface.toolNames.has('ApplyPatch'), false); + }); + + test('apply_patch exposes ApplyPatch and omits Write/Edit', () => { + const surface = projectEffectiveProductToolSurface({ + host: 'cli', + tools: buildBuiltinTools(), + policy: { economy: false, editingProtocol: 'apply_patch' }, + }); + assert.ok(surface.toolNames.has('ApplyPatch')); + assert.equal(surface.toolNames.has('Write'), false); + assert.equal(surface.toolNames.has('Edit'), false); + }); + + test('never advertises both editing protocols', () => { + for (const protocol of ['edit_write', 'apply_patch'] as const) { + const names = projectEffectiveProductToolSurface({ + host: 'cli', + tools: buildBuiltinTools({ includeEdit: true }), + policy: { economy: false, editingProtocol: protocol }, + }).toolNames; + const hasClassic = names.has('Write') || names.has('Edit'); + const hasPatch = names.has('ApplyPatch'); + assert.equal(hasClassic && hasPatch, false); + assert.ok(hasClassic || hasPatch); + } + }); +}); + +describe('ApplyPatch tool integration', () => { + test('rejects Add and Move when the destination already exists', async () => { + const root = await mkdtemp(join(tmpdir(), 'maka-apply-patch-exists-')); + try { + await writeFile(join(root, 'existing.txt'), 'keep\n', 'utf8'); + await writeFile(join(root, 'source.txt'), 'src\n', 'utf8'); + const tools = buildBuiltinTools(); + const apply = tools.find((tool) => tool.name === 'ApplyPatch'); + assert.ok(apply); + + await assert.rejects(async () => { + await apply.impl( + { + patch: envelope(['*** Add File: existing.txt', '+nope', ''].join('\n')), + }, + toolCtx(root), + ); + }, /already exists/i); + assert.equal(await readFile(join(root, 'existing.txt'), 'utf8'), 'keep\n'); + + await assert.rejects(async () => { + await apply.impl( + { + patch: envelope( + [ + '*** Update File: source.txt', + '*** Move to: existing.txt', + '@@', + '-src', + '+moved', + '', + ].join('\n'), + ), + }, + toolCtx(root), + ); + }, /already exists/i); + assert.equal(await readFile(join(root, 'source.txt'), 'utf8'), 'src\n'); + assert.equal(await readFile(join(root, 'existing.txt'), 'utf8'), 'keep\n'); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + test('add, update, delete, multi-file, mismatch, and absolute path', async () => { + const root = await mkdtemp(join(tmpdir(), 'maka-apply-patch-')); + try { + await writeFile(join(root, 'existing.txt'), 'hello world\n', 'utf8'); + await writeFile(join(root, 'to-delete.txt'), 'bye\n', 'utf8'); + await mkdir(join(root, 'src'), { recursive: true }); + await writeFile(join(root, 'src', 'a.ts'), 'const x = 1;\n', 'utf8'); + + const tools = buildBuiltinTools(); + const apply = tools.find((tool) => tool.name === 'ApplyPatch'); + assert.ok(apply); + + const multi = await apply.impl( + { + patch: envelope( + [ + '*** Add File: new.txt', + '+created', + '*** Update File: existing.txt', + '@@', + '-hello world', + '+hello maka', + '*** Delete File: to-delete.txt', + '', + ].join('\n'), + ), + }, + toolCtx(root), + ); + assert.equal((multi as { ok: boolean }).ok, true); + assert.equal(await readFile(join(root, 'new.txt'), 'utf8'), 'created\n'); + assert.equal(await readFile(join(root, 'existing.txt'), 'utf8'), 'hello maka\n'); + await assert.rejects(() => readFile(join(root, 'to-delete.txt'), 'utf8')); + + const nested = await apply.impl( + { + patch: envelope( + ['*** Add File: generated/deep/nested.txt', '+created with parents', ''].join('\n'), + ), + }, + toolCtx(root), + ); + assert.equal((nested as { ok: boolean }).ok, true); + assert.equal( + await readFile(join(root, 'generated', 'deep', 'nested.txt'), 'utf8'), + 'created with parents\n', + ); + + const before = await readFile(join(root, 'src', 'a.ts'), 'utf8'); + await assert.rejects(async () => { + await apply.impl( + { + patch: envelope( + ['*** Update File: src/a.ts', '@@', '-const y = 2;', '+const y = 3;', ''].join('\n'), + ), + }, + toolCtx(root), + ); + }, /hunk did not match|ApplyPatch/); + assert.equal(await readFile(join(root, 'src', 'a.ts'), 'utf8'), before); + + await assert.rejects(async () => { + await apply.impl( + { + patch: envelope(['*** Add File: /tmp/evil.txt', '+nope', ''].join('\n')), + }, + toolCtx(root), + ); + }, /absolute|relative/i); + + const moved = await apply.impl( + { + patch: envelope( + [ + '*** Update File: src/a.ts', + '*** Move to: src/b.ts', + '@@', + '-const x = 1;', + '+const x = 2;', + '', + ].join('\n'), + ), + }, + toolCtx(root), + ); + assert.equal((moved as { ok: boolean }).ok, true); + assert.equal(await readFile(join(root, 'src', 'b.ts'), 'utf8'), 'const x = 2;\n'); + await assert.rejects(() => readFile(join(root, 'src', 'a.ts'), 'utf8')); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + test('Delete unlinks a symlink while Update plus Move rejects one without side effects', async (t) => { + if (process.platform === 'win32') { + t.skip('file symlink creation is not reliably available on Windows CI'); + return; + } + const root = await mkdtemp(join(tmpdir(), 'maka-apply-patch-symlink-')); + try { + await writeFile(join(root, 'target.txt'), 'target\n', 'utf8'); + await symlink('target.txt', join(root, 'delete-link.txt')); + await symlink('target.txt', join(root, 'move-link.txt')); + const apply = buildBuiltinTools().find((tool) => tool.name === 'ApplyPatch'); + assert.ok(apply); + + await apply.impl( + { + patch: envelope(['*** Delete File: delete-link.txt', ''].join('\n')), + }, + toolCtx(root), + ); + await assert.rejects(() => lstat(join(root, 'delete-link.txt'))); + assert.equal(await readFile(join(root, 'target.txt'), 'utf8'), 'target\n'); + + await assert.rejects( + async () => + await apply.impl( + { + patch: envelope( + [ + '*** Update File: move-link.txt', + '*** Move to: moved.txt', + '@@', + '-target', + '+moved', + '', + ].join('\n'), + ), + }, + toolCtx(root), + ), + /regular file/i, + ); + assert.equal((await lstat(join(root, 'move-link.txt'))).isSymbolicLink(), true); + assert.equal(await readFile(join(root, 'target.txt'), 'utf8'), 'target\n'); + await assert.rejects(() => lstat(join(root, 'moved.txt'))); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + test('managed-worker Add and Move to missing destinations preflight cleanly', async (t) => { + if (process.platform === 'win32') { + // canWritePath matches workspace roots with POSIX separators; the + // preflight path space on Windows is a pre-existing gap, not part of + // the managed-worker behavior under test (CI runs POSIX). + t.skip('managed permission matching requires POSIX host paths'); + return; + } + const root = await mkdtemp(join(tmpdir(), 'maka-apply-patch-managed-')); + try { + await writeFile(join(root, 'source.txt'), 'src\n', 'utf8'); + const worker = { + async execute(input: { + operation: FilesystemWorkerOperation; + cwd: string; + }): Promise { + const expectedTarget = await workerExpectedTarget(root, input.operation); + // The real client sends the absolute display/enforcement path as the + // operand; the worker revalidates it against expectedTarget. + const operation: FilesystemWorkerOperation = { + ...input.operation, + cwd: input.cwd, + path: expectedTarget.displayPath, + }; + const request: FilesystemWorkerRequest = { + version: FILESYSTEM_WORKER_PROTOCOL_VERSION, + requestId: 'request-1', + operation, + operationBoundary: { + filesystem: { + entries: [ + { + path: expectedTarget.enforcementPath, + access: expectedTarget.access, + scope: expectedTarget.scope, + }, + ], + }, + }, + expectedTarget, + }; + const response = await executeFilesystemWorkerRequest(request); + if (!response.ok) throw new Error(response.error.message); + return response.result; + }, + }; + const apply = buildBuiltinTools({ + filesystemWorker: worker as never, + permissionProfile: createWorkspaceWritePermissionProfile(), + }).find((tool) => tool.name === 'ApplyPatch'); + assert.ok(apply); + + // Add a brand-new file: the preflight lstat must classify the missing + // destination as `missing`, not not_found. + const added = await apply.impl( + { + patch: envelope(['*** Add File: generated/deep/new.txt', '+created', ''].join('\n')), + }, + toolCtx(root), + ); + assert.equal((added as { ok: boolean }).ok, true); + assert.equal(await readFile(join(root, 'generated', 'deep', 'new.txt'), 'utf8'), 'created\n'); + + // Move to a missing destination: the destination lstat is missing and + // the source delete lands on the source entry. + const moved = await apply.impl( + { + patch: envelope( + [ + '*** Update File: source.txt', + '*** Move to: moved-away.txt', + '@@', + '-src', + '+moved', + '', + ].join('\n'), + ), + }, + toolCtx(root), + ); + assert.equal((moved as { ok: boolean }).ok, true); + assert.equal(await readFile(join(root, 'moved-away.txt'), 'utf8'), 'moved\n'); + await assert.rejects(() => readFile(join(root, 'source.txt'), 'utf8')); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); + +describe('product-tool surface editingProtocol', () => { + test('projector selects exactly one editing protocol from bound tools', () => { + const tools = buildBuiltinTools({ includeEdit: true }); + const editWrite = projectEffectiveProductToolSurface({ + host: 'cli', + tools, + policy: { economy: false, editingProtocol: 'edit_write' }, + }); + assert.ok(editWrite.toolNames.has('Write')); + assert.ok(editWrite.toolNames.has('Edit')); + assert.equal(editWrite.toolNames.has('ApplyPatch'), false); + assert.equal(editWrite.identity.policy.editingProtocol, 'edit_write'); + + const applyPatch = projectEffectiveProductToolSurface({ + host: 'cli', + tools, + policy: { economy: false, editingProtocol: 'apply_patch' }, + }); + assert.ok(applyPatch.toolNames.has('ApplyPatch')); + assert.equal(applyPatch.toolNames.has('Write'), false); + assert.equal(applyPatch.toolNames.has('Edit'), false); + assert.equal(applyPatch.identity.policy.editingProtocol, 'apply_patch'); + }); + + test('builder binds the union and the default projector selects edit_write once', () => { + const tools = buildBuiltinTools({ includeEdit: true }); + const surface = projectEffectiveProductToolSurface({ + host: 'cli', + tools, + policy: { economy: false }, + }); + assert.equal(surface.toolNames.has('ApplyPatch'), false); + assert.ok(surface.toolNames.has('Write')); + assert.ok(surface.toolNames.has('Edit')); + assert.equal(surface.identity.policy.editingProtocol, 'edit_write'); + }); +}); + +describe('shared ApplyPatch engine partial move failure', () => { + test('reports partial=true and completed destination when source delete fails', async () => { + const files = new Map([['src.txt', 'hello\n']]); + const fs: ApplyPatchFsAdapter = { + async lockKey(path) { + return path; + }, + async lstat(path) { + return files.has(path) ? 'file' : 'missing'; + }, + async readText(path) { + const content = files.get(path); + if (content === undefined) throw new Error(`missing ${path}`); + return content; + }, + async writeText(path, content) { + files.set(path, content); + return { path, bytes: Buffer.byteLength(content, 'utf8') }; + }, + async deletePath(path) { + if (path === 'src.txt') throw new Error('delete failed'); + files.delete(path); + return { path }; + }, + }; + + const result = await executeApplyPatchWithAdapter( + envelope( + ['*** Update File: src.txt', '*** Move to: dest.txt', '@@', '-hello', '+hello', ''].join( + '\n', + ), + ), + fs, + async (_key, run) => run(), + ); + + assert.equal(result.ok, false); + assert.equal(result.partial, true); + assert.deepEqual(result.completed, ['dest.txt']); + assert.deepEqual(result.uncompleted, ['src.txt']); + assert.ok(files.has('dest.txt')); + assert.ok(files.has('src.txt')); + }); + + test('preflightPermissions runs before mutation and rethrows structured boundary errors', async () => { + const files = new Map([ + ['a.txt', 'a\n'], + ['b.txt', 'b\n'], + ]); + let writes = 0; + let preflighted: string[] = []; + const boundaryError = Object.assign(new Error('sandbox boundary required'), { + domain: 'filesystem', + reason: 'sandbox_boundary_required', + requiredExpansion: { + filesystem: { + entries: [{ path: '/tmp/workspace/b.txt', access: 'write', scope: 'exact' }], + }, + }, + }); + const fs: ApplyPatchFsAdapter = { + async lockKey(path) { + return path; + }, + async lstat(path) { + return files.has(path) ? 'file' : 'missing'; + }, + async readText(path) { + const content = files.get(path); + if (content === undefined) throw new Error(`missing ${path}`); + return content; + }, + async writeText(path, content) { + writes += 1; + files.set(path, content); + return { path, bytes: Buffer.byteLength(content, 'utf8') }; + }, + async deletePath(path) { + files.delete(path); + return { path }; + }, + async preflightPermissions(accesses) { + preflighted = accesses.map((access) => access.path).sort(); + throw boundaryError; + }, + }; + + await assert.rejects( + () => + executeApplyPatchWithAdapter( + envelope( + [ + '*** Update File: a.txt', + '@@', + '-a', + '+aa', + '*** Update File: b.txt', + '@@', + '-b', + '+bb', + '', + ].join('\n'), + ), + fs, + async (_key, run) => run(), + ), + (error: unknown) => { + assert.equal(error, boundaryError); + return true; + }, + ); + assert.equal(writes, 0); + assert.deepEqual(preflighted, ['a.txt', 'b.txt']); + assert.equal(files.get('a.txt'), 'a\n'); + assert.equal(files.get('b.txt'), 'b\n'); + }); + + test('rethrows structured boundary errors from the first mutation without partial result', async () => { + const files = new Map([['a.txt', 'a\n']]); + const boundaryError = Object.assign(new Error('sandbox boundary required'), { + domain: 'filesystem', + reason: 'sandbox_boundary_required', + requiredExpansion: { + filesystem: { + entries: [{ path: '/tmp/workspace/a.txt', access: 'write', scope: 'exact' }], + }, + }, + }); + const fs: ApplyPatchFsAdapter = { + async lockKey(path) { + return path; + }, + async lstat(path) { + return files.has(path) ? 'file' : 'missing'; + }, + async readText(path) { + const content = files.get(path); + if (content === undefined) throw new Error(`missing ${path}`); + return content; + }, + async writeText() { + throw boundaryError; + }, + async deletePath(path) { + files.delete(path); + return { path }; + }, + }; + + await assert.rejects( + () => + executeApplyPatchWithAdapter( + envelope(['*** Update File: a.txt', '@@', '-a', '+aa', ''].join('\n')), + fs, + async (_key, run) => run(), + ), + (error: unknown) => { + assert.equal(error, boundaryError); + return true; + }, + ); + assert.equal(files.get('a.txt'), 'a\n'); + }); + + test('canonical path aliases cannot bypass duplicate Add tracking', async () => { + const files = new Map(); + const fs: ApplyPatchFsAdapter = { + async lockKey(path) { + return path; + }, + async lstat(path) { + return files.has(path) ? 'file' : 'missing'; + }, + async readText(path) { + const content = files.get(path); + if (content === undefined) throw new Error(`missing ${path}`); + return content; + }, + async writeText(path, content) { + files.set(path, content); + return { path, bytes: Buffer.byteLength(content, 'utf8') }; + }, + async deletePath(path) { + files.delete(path); + return { path }; + }, + }; + + await assert.rejects( + () => + executeApplyPatchWithAdapter( + envelope( + ['*** Add File: x.txt', '+first', '*** Add File: ./x.txt', '+second', ''].join('\n'), + ), + fs, + async (_key, run) => run(), + ), + /already exists/i, + ); + assert.equal(files.size, 0); + }); +}); + +/** + * Build the worker `expectedTarget` the same way the real client does: the + * entry-mode path (lstat/delete/create) resolves the parent chain and appends + * the leaf, so a missing destination still has a valid enforcement path. + */ +async function workerExpectedTarget( + root: string, + operation: FilesystemWorkerOperation, +): Promise { + const { realpathAllowMissing } = await import('../path-containment.js'); + const { dirname, basename, resolve } = await import('node:path'); + const entryMode = + operation.kind === 'delete' || + operation.kind === 'lstat' || + (operation.kind === 'write' && operation.mode === 'create'); + const canonicalCwd = await import('node:fs/promises').then((m) => m.realpath(root)); + const displayPath = resolve(canonicalCwd, operation.path); + const access = operation.kind === 'write' || operation.kind === 'delete' ? 'write' : 'read'; + if (!entryMode) { + const enforcementPath = await realpathAllowMissing(displayPath); + return { + displayPath, + enforcementPath, + access, + scope: 'exact', + targetType: await targetTypeOf(enforcementPath), + }; + } + const parentReal = await realpathAllowMissing(dirname(displayPath)); + const enforcementPath = resolve(parentReal, basename(displayPath)); + return { + displayPath, + enforcementPath, + access, + scope: 'exact', + targetType: await targetTypeOf(enforcementPath), + }; +} + +async function targetTypeOf(path: string): Promise { + const { lstat: lstatFn } = await import('node:fs/promises'); + try { + const metadata = await lstatFn(path); + if (metadata.isSymbolicLink()) return 'symlink'; + if (metadata.isFile()) return 'file'; + if (metadata.isDirectory()) return 'directory'; + return 'other'; + } catch (error) { + if ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error.code === 'ENOENT' || error.code === 'ENOTDIR') + ) { + return 'missing'; + } + throw error; + } +} diff --git a/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts b/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts index 3c6ae6b671..ccf8e6062e 100644 --- a/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts +++ b/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts @@ -81,6 +81,8 @@ describe('builtin file tools use the sandboxed worker', () => { execute: async (input) => { calls.push(input); switch (input.operation.kind) { + case 'lstat': + return { kind: 'lstat', targetType: 'file' }; case 'read': return { kind: 'read', content: 'worker-content' }; case 'write': @@ -106,6 +108,8 @@ describe('builtin file tools use the sandboxed worker', () => { byteDelta: 1, changed: true, }; + case 'delete': + return { kind: 'delete', ok: true, path: input.operation.path }; case 'glob': return { kind: 'glob', files: ['worker.ts'] }; case 'grep': diff --git a/packages/runtime/src/__tests__/builtin-tools.test.ts b/packages/runtime/src/__tests__/builtin-tools.test.ts index 166e5ba7f3..3bde789b98 100644 --- a/packages/runtime/src/__tests__/builtin-tools.test.ts +++ b/packages/runtime/src/__tests__/builtin-tools.test.ts @@ -23,6 +23,7 @@ import { } from '@maka/core/permission-profile'; import { expect } from '../test-helpers.js'; import { buildBuiltinTools } from '../builtin-tools.js'; +import { projectEffectiveProductToolSurface } from '../tool-catalog-derive.js'; import { SandboxManager } from '../sandbox/sandbox-manager.js'; import { LinuxBubblewrapBackend } from '../sandbox/linux-sandbox.js'; import { MacosSeatbeltBackend } from '../sandbox/macos-seatbelt.js'; @@ -51,7 +52,11 @@ const ONE_PIXEL_PNG = Buffer.from( describe('builtin tool activity kinds', () => { test('declares stable semantic categories independently of tool names', () => { const kinds = Object.fromEntries( - buildBuiltinTools().map((tool) => [tool.name, tool.activityKind]), + projectEffectiveProductToolSurface({ + host: 'cli', + tools: buildBuiltinTools(), + policy: { economy: false }, + }).tools.map((tool) => [tool.name, tool.activityKind]), ); expect(kinds).toEqual({ @@ -63,6 +68,16 @@ describe('builtin tool activity kinds', () => { Glob: 'search', Grep: 'search', }); + const patchKinds = Object.fromEntries( + projectEffectiveProductToolSurface({ + host: 'cli', + tools: buildBuiltinTools(), + policy: { economy: false, editingProtocol: 'apply_patch' }, + }).tools.map((tool) => [tool.name, tool.activityKind]), + ); + expect(patchKinds.ApplyPatch).toBe('edit'); + assert.equal('Write' in patchKinds, false); + assert.equal('Edit' in patchKinds, false); }); test('categorizes background task controls as command activity', () => { diff --git a/packages/runtime/src/__tests__/continuation-replay.test.ts b/packages/runtime/src/__tests__/continuation-replay.test.ts index 6686ca8be5..2fb0077143 100644 --- a/packages/runtime/src/__tests__/continuation-replay.test.ts +++ b/packages/runtime/src/__tests__/continuation-replay.test.ts @@ -1,10 +1,10 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; +import { type RuntimeEvent } from '@maka/core'; import { buildImmutableRuntimePrefix, - type RuntimeEvent, type RuntimePrefixIdentityV1, -} from '@maka/core'; +} from '@maka/core/runtime-boundary'; import { buildContinuationReplayPlan, buildContinuationReplaySegment, diff --git a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts index 1ccad44731..1f5f11e2f1 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts @@ -1,6 +1,15 @@ import assert from 'node:assert/strict'; -import { rmSync } from 'node:fs'; -import { mkdtemp, mkdir, realpath, rm, writeFile } from 'node:fs/promises'; +import { rmSync, realpathSync } from 'node:fs'; +import { + lstat, + mkdtemp, + mkdir, + readFile, + realpath, + rm, + symlink, + writeFile, +} from 'node:fs/promises'; import { homedir, tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, test } from 'node:test'; @@ -31,6 +40,7 @@ import { type FilesystemWorkerRequest, type FilesystemWorkerResult, } from '../filesystem-worker/protocol.js'; +import { executeFilesystemWorkerRequest } from '../filesystem-worker/operations.js'; import { LinuxBubblewrapBackend } from '../sandbox/linux-sandbox.js'; import { MacosSeatbeltBackend } from '../sandbox/macos-seatbelt.js'; import { SandboxManager } from '../sandbox/sandbox-manager.js'; @@ -52,6 +62,29 @@ test('Read image payloads fit within the filesystem worker response limit', () = }); describe('filesystem worker client permission snapshots', () => { + test('deletes a symlink operand without deleting its canonical target', { + skip: process.platform === 'win32' ? 'POSIX filesystem worker path policy required' : false, + }, async () => { + const workspace = await temporaryDirectory('maka-worker-client-symlink-delete-'); + const target = join(workspace, 'target.txt'); + const link = join(workspace, 'link.txt'); + await writeFile(target, 'keep me', 'utf8'); + await symlink(target, link); + const { client, requests } = fakeClient({ executeRequest: true }); + + await client.execute({ + operation: { kind: 'delete', path: link }, + cwd: workspace, + mode: 'execute', + }); + + await assert.rejects(lstat(link), { code: 'ENOENT' }); + assert.equal(await readFile(target, 'utf8'), 'keep me'); + assert.equal(requests[0]?.operation.path, link); + assert.equal(requests[0]?.expectedTarget.enforcementPath, link); + assert.equal(requests[0]?.expectedTarget.targetType, 'symlink'); + }); + for (const kind of ['bypass', 'external'] as const) { test(`rejects an authoritative ${kind} boundary instead of falling back to legacy mode`, async () => { const workspace = await temporaryDirectory(`maka-worker-client-${kind}-`); @@ -358,7 +391,11 @@ describe('filesystem worker Linux path context', () => { assert.ok(hasArgTriple(processInput.argv, '--ro-bind', `/proc/self/fd/${pinned.fd}`, target)); }); - test('pins the writable parent of a missing exact target', async () => { + test('pins the missing target existing parent so a create can build the chain', async (t) => { + if (process.platform === 'win32') { + t.skip('Linux sandbox path matching requires POSIX host paths'); + return; + } const workspace = await temporaryDirectory('maka-linux-worker-parent-pin-'); const parent = join(workspace, 'output'); const target = join(parent, 'new.txt'); @@ -378,11 +415,13 @@ describe('filesystem worker Linux path context', () => { ); assert.ok(pinned); assert.ok(hasArgTriple(processInput.argv, '--bind', `/proc/self/fd/${pinned.fd}`, parent)); - assert.equal(hasArgTriple(processInput.argv, '--bind', parent, parent), false); + assert.equal(hasArgTriple(processInput.argv, '--bind', workspace, workspace), false); }); test('requests a trusted parent mount only for a missing write target', () => { const target = join(tmpdir(), 'maka-linux-worker-parent', 'new.txt'); + // The parent does not exist yet: the mount is the deepest existing + // ancestor (tmpdir), which is enough for the worker to mkdir the chain. assert.deepEqual( filesystemWorkerRuntimeWritableRoots({ platform: 'linux', @@ -390,7 +429,7 @@ describe('filesystem worker Linux path context', () => { enforcementPath: target, targetType: 'missing', }), - [join(tmpdir(), 'maka-linux-worker-parent')], + [realpathSync(tmpdir())], ); assert.equal( filesystemWorkerRuntimeWritableRoots({ @@ -410,6 +449,18 @@ describe('filesystem worker Linux path context', () => { }), undefined, ); + // Entry-mode deletes address an existing entry: the mount is the entry's + // parent, never the entry itself (which may be a symlink). + assert.deepEqual( + filesystemWorkerRuntimeWritableRoots({ + platform: 'linux', + access: 'write', + enforcementPath: target, + targetType: 'file', + entryMode: true, + }), + [realpathSync(tmpdir())], + ); }); }); @@ -418,6 +469,7 @@ function fakeClient( operationErrorCode?: FilesystemWorkerErrorCode; platform?: SandboxPlatform; beforeLaunchSpecReturn?: () => void; + executeRequest?: boolean; } = {}, ): { client: FilesystemWorkerClient; @@ -463,26 +515,27 @@ function fakeClient( processInputs.push(input); const request = FilesystemWorkerRequestSchema.parse(JSON.parse(input.stdin)); requests.push(request); + const response = options.executeRequest + ? await executeFilesystemWorkerRequest(request) + : options.operationErrorCode + ? { + version: FILESYSTEM_WORKER_PROTOCOL_VERSION, + requestId: request.requestId, + ok: false as const, + error: { + code: options.operationErrorCode, + message: 'Sandbox denied the filesystem operation.', + }, + } + : { + version: FILESYSTEM_WORKER_PROTOCOL_VERSION, + requestId: request.requestId, + ok: true as const, + result: fakeResult(request), + }; return { exitCode: 0, - stdout: JSON.stringify( - options.operationErrorCode - ? { - version: FILESYSTEM_WORKER_PROTOCOL_VERSION, - requestId: request.requestId, - ok: false, - error: { - code: options.operationErrorCode, - message: 'Sandbox denied the filesystem operation.', - }, - } - : { - version: FILESYSTEM_WORKER_PROTOCOL_VERSION, - requestId: request.requestId, - ok: true, - result: fakeResult(request), - }, - ), + stdout: JSON.stringify(response), stderrTail: '', timedOut: false, aborted: false, diff --git a/packages/runtime/src/__tests__/filesystem-worker.test.ts b/packages/runtime/src/__tests__/filesystem-worker.test.ts index 229992fe62..52ba83bf91 100644 --- a/packages/runtime/src/__tests__/filesystem-worker.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker.test.ts @@ -195,6 +195,65 @@ describe('filesystem worker operations', () => { await assert.rejects(readFile(outsidePath, 'utf8'), { code: 'ENOENT' }); }); + test('creates missing destination parents for an approved create', async () => { + const root = await temporaryDirectory('maka-worker-write-parent-'); + const target = join(root, 'generated', 'deep', 'file.txt'); + + // #2059 follow-final containment: a plain write needs its parent in + // realpath space, while create-mode (ApplyPatch Add) may build the + // missing parent chain through the canonical entry. + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'write', cwd: root, path: target, content: 'nested', mode: 'create' }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'missing' }, + ), + ); + + assert.equal(response.ok, true); + assert.equal(await readFile(target, 'utf8'), 'nested'); + }); + + test('create-mode writes never clobber an entry that appeared after planning', async () => { + const root = await temporaryDirectory('maka-worker-write-no-clobber-'); + const target = join(root, 'target.txt'); + await writeFile(target, 'winner', 'utf8'); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'write', cwd: root, path: target, content: 'stale', mode: 'create' }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + ), + ); + + assert.equal(response.ok, false); + assert.equal(await readFile(target, 'utf8'), 'winner'); + }); + + test('replace-mode writes land on the canonical target and never follow a final symlink', async (t) => { + if (process.platform === 'win32') { + t.skip('file symlink creation is not reliably available on Windows CI'); + return; + } + const root = await temporaryDirectory('maka-worker-write-replace-link-'); + const target = join(root, 'target.txt'); + const link = join(root, 'link.txt'); + await writeFile(target, 'target', 'utf8'); + await symlink(target, link); + + // #2059 follow-final semantics: replace-mode is authorised against the + // canonical (followed) path and the O_NOFOLLOW open stays pinned there. + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'write', cwd: root, path: link, content: 'stale', mode: 'replace' }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + link, + ), + ); + + assert.equal(response.ok, true); + assert.equal(await readFile(target, 'utf8'), 'stale'); + }); + test('denies a write through a dangling symlink the boundary does not cover', async () => { // The worker enforces its own boundary rather than trusting the caller to // have canonicalised the path: a link inside the root whose target does not @@ -219,6 +278,120 @@ describe('filesystem worker operations', () => { await assert.rejects(readFile(target, 'utf8'), { code: 'ENOENT' }); }); + test('lstat reports the directory entry without following its final symlink', async (t) => { + if (process.platform === 'win32') { + t.skip('file symlink creation is not reliably available on Windows CI'); + return; + } + const root = await temporaryDirectory('maka-worker-lstat-link-'); + const target = join(root, 'target.txt'); + const link = join(root, 'link.txt'); + await writeFile(target, 'target', 'utf8'); + await symlink(target, link); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'lstat', cwd: root, path: link }, + { enforcementPath: link, access: 'read', scope: 'exact', targetType: 'symlink' }, + link, + ), + ); + + assert.equal(response.ok, true); + if (response.ok) { + assert.deepEqual(response.result, { kind: 'lstat', targetType: 'symlink' }); + } + }); + + test('lstat classifies a missing entry as missing instead of not_found', async () => { + const root = await temporaryDirectory('maka-worker-lstat-missing-'); + const target = join(root, 'new-file.txt'); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'lstat', cwd: root, path: target }, + { enforcementPath: target, access: 'read', scope: 'exact', targetType: 'missing' }, + target, + ), + ); + + // An ApplyPatch Add/Move preflight probes the destination before it + // exists; the worker must report `missing`, not a not_found error. + assert.equal(response.ok, true); + if (response.ok) { + assert.deepEqual(response.result, { kind: 'lstat', targetType: 'missing' }); + } + }); + + test('lstat classifies a missing nested entry as missing instead of not_found', async () => { + const root = await temporaryDirectory('maka-worker-lstat-nested-'); + const target = join(root, 'generated', 'deep', 'nested.txt'); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'lstat', cwd: root, path: target }, + { enforcementPath: target, access: 'read', scope: 'exact', targetType: 'missing' }, + target, + ), + ); + + // Neither the entry nor its parents exist; this is a legal preflight + // result for Add File: generated/deep/nested.txt. + assert.equal(response.ok, true); + if (response.ok) { + assert.deepEqual(response.result, { kind: 'lstat', targetType: 'missing' }); + } + }); + + test('deletes an in-workspace symlink operand without deleting its target', async (t) => { + if (process.platform === 'win32') { + t.skip('file symlink creation is not reliably available on Windows CI'); + return; + } + const root = await temporaryDirectory('maka-worker-delete-link-'); + const target = join(root, 'target.txt'); + const link = join(root, 'link.txt'); + await writeFile(target, 'keep', 'utf8'); + await symlink(target, link); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'delete', cwd: root, path: link }, + { enforcementPath: link, access: 'write', scope: 'exact', targetType: 'symlink' }, + link, + ), + ); + + assert.equal(response.ok, true); + assert.equal(await readFile(target, 'utf8'), 'keep'); + await assert.rejects(readFile(link, 'utf8'), { code: 'ENOENT' }); + }); + + test('deletes an escaping symlink entry without touching its target', async (t) => { + if (process.platform === 'win32') { + t.skip('file symlink creation is not reliably available on Windows CI'); + return; + } + const root = await temporaryDirectory('maka-worker-delete-link-root-'); + const outside = await temporaryDirectory('maka-worker-delete-link-outside-'); + const target = join(outside, 'target.txt'); + const link = join(root, 'link.txt'); + await writeFile(target, 'keep', 'utf8'); + await symlink(target, link); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'delete', cwd: root, path: link }, + { enforcementPath: link, access: 'write', scope: 'exact', targetType: 'symlink' }, + link, + ), + ); + + assert.equal(response.ok, true); + await assert.rejects(readFile(link, 'utf8'), { code: 'ENOENT' }); + assert.equal(await readFile(target, 'utf8'), 'keep'); + }); + test('fails when an approved target changes type before execution', async () => { const root = await temporaryDirectory('maka-worker-type-'); const target = join(root, 'target'); diff --git a/packages/runtime/src/__tests__/session-manager.test.ts b/packages/runtime/src/__tests__/session-manager.test.ts index 1941a9d6df..1b8c0a80f5 100644 --- a/packages/runtime/src/__tests__/session-manager.test.ts +++ b/packages/runtime/src/__tests__/session-manager.test.ts @@ -8,13 +8,12 @@ import { createWorkspaceWritePermissionProfile, DEEP_RESEARCH_SESSION_LABEL, RUNTIME_CONTINUATION_AUTHORITY_V1, - buildImmutableRuntimePrefix, - decodeContinuationClaim, deriveTurnRecords, isSandboxBoundaryRestartClosure, isSessionInlineRun, isTerminalRuntimeEvent, } from '@maka/core'; +import { buildImmutableRuntimePrefix, decodeContinuationClaim } from '@maka/core/runtime-boundary'; import type { CreateSandboxBoundaryRequest, SandboxBoundaryRequest, diff --git a/packages/runtime/src/__tests__/subagent-tools.test.ts b/packages/runtime/src/__tests__/subagent-tools.test.ts index 8ffcabbfd6..a313501410 100644 --- a/packages/runtime/src/__tests__/subagent-tools.test.ts +++ b/packages/runtime/src/__tests__/subagent-tools.test.ts @@ -31,9 +31,11 @@ import { WEB_RESEARCH_AGENT_DEFINITION, WEB_RESEARCH_AGENT_PROFILE, assertAgentDefinitionRunnable, + buildToolsForAgentDefinition, evaluateAgentDefinitionAvailability, evaluateAgentDefinitionToolAccess, listBuiltinAgentDefinitions, + projectDefinitionToolNames, requireBuiltinAgentDefinitionByProfile, } from '../agent-catalog.js'; import { AGENT_SWARM_TOOL_NAME } from '../agent-swarm-tools.js'; @@ -43,6 +45,7 @@ import { AGENT_SPAWN_TOOL_NAME, CHILD_AGENT_TOOL_NAMES, buildChildAgentTools, + childAgentToolsWithinEditingProtocol, buildParentAgentTools, buildSubagentListTool, buildSubagentOutputTool, @@ -331,6 +334,62 @@ describe('subagent tools', () => { }); }); + test('ApplyPatch-only parents cannot elevate implementation children to Write or Edit', () => { + const parentScopedTools = childAgentToolsWithinEditingProtocol( + buildChildAgentTools(buildBuiltinTools()), + 'apply_patch', + ); + + expect(parentScopedTools.some((tool) => tool.name === 'Write')).toBe(false); + expect(parentScopedTools.some((tool) => tool.name === 'Edit')).toBe(false); + expect( + listBuiltinAgentDefinitions({ + tools: parentScopedTools, + worktreeChildExecutorAvailable: true, + }).find((definition) => definition.id === IMPLEMENTATION_AGENT_ID)?.availability, + ).toEqual({ status: 'available' }); + expect( + buildToolsForAgentDefinition(parentScopedTools, IMPLEMENTATION_AGENT_DEFINITION).map( + (tool) => tool.name, + ), + ).toEqual(['Read', 'Glob', 'Grep', 'ApplyPatch', 'Bash']); + }); + + test('durable child snapshots store the projected surface, not raw definition tools', () => { + const parentScopedTools = childAgentToolsWithinEditingProtocol( + buildChildAgentTools(buildBuiltinTools()), + 'apply_patch', + ); + const projected = projectDefinitionToolNames( + IMPLEMENTATION_AGENT_DEFINITION, + parentScopedTools, + ); + + // The snapshot must equal what resume/retry/restore rebuilds from the + // snapshot: buildToolsForAgentDefinition against the same host tools. + expect(projected).toEqual(['Read', 'Glob', 'Grep', 'ApplyPatch', 'Bash']); + expect(projected).toEqual( + buildToolsForAgentDefinition(parentScopedTools, IMPLEMENTATION_AGENT_DEFINITION).map( + (tool) => tool.name, + ), + ); + + // Projecting an already-projected snapshot (the resume path re-projects + // snapshot.toolNames) must be a no-op, or the strict length invariant + // fails on the second round trip. + const snapshotTools = projected.map((name) => testCatalogTool(name, 'file_write')); + expect(projectDefinitionToolNames(IMPLEMENTATION_AGENT_DEFINITION, snapshotTools)).toEqual( + projected, + ); + expect( + buildToolsForAgentDefinition(snapshotTools, { + id: IMPLEMENTATION_AGENT_ID, + permissionMode: 'ask', + tools: projected, + }).length, + ).toBe(projected.length); + }); + test('agent definition availability depends on exposed tools, not legacy parent modes', () => { expect( evaluateAgentDefinitionAvailability({ @@ -442,6 +501,7 @@ describe('subagent tools', () => { 'Write', 'Edit', 'Bash', + 'ApplyPatch', ]); expect([...CHILD_AGENT_TOOL_NAMES]).toEqual([ 'Read', @@ -451,6 +511,7 @@ describe('subagent tools', () => { 'Write', 'Edit', 'Bash', + 'ApplyPatch', ]); }); diff --git a/packages/runtime/src/__tests__/tool-artifacts.test.ts b/packages/runtime/src/__tests__/tool-artifacts.test.ts index e38677f56c..887db05933 100644 --- a/packages/runtime/src/__tests__/tool-artifacts.test.ts +++ b/packages/runtime/src/__tests__/tool-artifacts.test.ts @@ -48,6 +48,74 @@ describe('deriveToolArtifactCandidates', () => { ).toBe(true); }); + test('ApplyPatch derives file candidates from completed operations', () => { + const candidates = deriveToolArtifactCandidates({ + toolName: 'ApplyPatch', + cwd: '/workspace/maka', + args: { patch: '*** Begin Patch\n*** End Patch\n' }, + result: { + ok: true, + operations: [ + { + operation: 'add', + path: '/workspace/maka/docs/report.html', + status: 'completed', + bytes: 12, + }, + { + operation: 'update', + path: '/workspace/maka/src/main.ts', + status: 'completed', + bytes: 20, + }, + { operation: 'delete', path: '/workspace/maka/tmp/old.txt', status: 'failed' }, + ], + completed: ['/workspace/maka/docs/report.html', '/workspace/maka/src/main.ts'], + uncompleted: ['/workspace/maka/tmp/old.txt'], + }, + }); + + expect(candidates.length).toBe(2); + expect(candidates[0]).toEqual({ + kind: 'html', + name: 'report.html', + mimeType: 'text/html', + source: 'tool_result', + summary: 'ApplyPatch tool output', + sourcePath: '/workspace/maka/docs/report.html', + }); + expect(candidates[1]?.name).toBe('main.ts'); + expect(candidates[1]?.sourcePath).toBe('/workspace/maka/src/main.ts'); + }); + + test('ApplyPatch derives the written destination from a partially failed Move', () => { + const candidates = deriveToolArtifactCandidates({ + toolName: 'ApplyPatch', + cwd: '/workspace/maka', + args: { patch: '*** Begin Patch\n*** End Patch\n' }, + result: { + ok: false, + partial: true, + operations: [ + { + operation: 'move', + path: '/workspace/maka/dest.txt', + fromPath: '/workspace/maka/source.txt', + status: 'failed', + bytes: 12, + error: 'delete failed', + }, + ], + completed: ['/workspace/maka/dest.txt'], + uncompleted: ['/workspace/maka/source.txt'], + }, + }); + + expect(candidates.map((candidate) => candidate.sourcePath)).toEqual([ + '/workspace/maka/dest.txt', + ]); + }); + test('Bash derives only explicit stdout redirects and does not scan stdout/stderr text', () => { const [candidate] = deriveToolArtifactCandidates({ toolName: 'Bash', @@ -56,7 +124,10 @@ describe('deriveToolArtifactCandidates', () => { result: { stdout: 'wrote /tmp/guessed.html', stderr: 'see report.pdf' }, }); - expect(candidate?.sourcePath).toBe('/workspace/maka/reports/build.log'); + // resolve() is platform-native; only assert the relative suffix contract. + expect( + candidate?.sourcePath?.replaceAll('\\', '/').endsWith('/workspace/maka/reports/build.log'), + ).toBe(true); expect(candidate?.kind).toBe('file'); expect( diff --git a/packages/runtime/src/__tests__/tool-catalog-derive.test.ts b/packages/runtime/src/__tests__/tool-catalog-derive.test.ts index f5b99bc499..deb08a5611 100644 --- a/packages/runtime/src/__tests__/tool-catalog-derive.test.ts +++ b/packages/runtime/src/__tests__/tool-catalog-derive.test.ts @@ -94,6 +94,7 @@ describe('projectEffectiveProductToolSurface', () => { policy: { economy: true, disabledSurfaceIds: ['agent'], + editingProtocol: 'edit_write', }, productToolNames: ['Read', 'browser_click', 'browser_navigate'], }); diff --git a/packages/runtime/src/agent-catalog.ts b/packages/runtime/src/agent-catalog.ts index a5818f0a2c..c9b7b9ee5b 100644 --- a/packages/runtime/src/agent-catalog.ts +++ b/packages/runtime/src/agent-catalog.ts @@ -260,7 +260,11 @@ export function evaluateAgentDefinitionToolAccess( tool: Pick, ): { category: ToolCategory; decision: PolicyDecision } { const category = categoryForTool(tool); - return { category, decision: definition.tools.includes(tool.name) ? 'allow' : 'block' }; + const admitted = + definition.tools.includes(tool.name) || + (tool.name === 'ApplyPatch' && + (definition.tools.includes('Write') || definition.tools.includes('Edit'))); + return { category, decision: admitted ? 'allow' : 'block' }; } export function evaluateAgentDefinitionAvailability(input: { @@ -282,7 +286,8 @@ export function evaluateAgentDefinitionAvailability(input: { } const byName = new Map(tools.map((tool) => [tool.name, tool])); - const missingTools = definition.tools.filter((name) => !byName.has(name)); + const requiredToolNames = projectDefinitionToolNamesToAvailableSurface(definition, byName); + const missingTools = requiredToolNames.filter((name) => !byName.has(name)); if (missingTools.length > 0) { return { status: 'unavailable', reason: 'missing_tools', missingTools }; } @@ -295,15 +300,69 @@ export function buildToolsForAgentDefinition( definition: AgentRuntimeDefinition = LOCAL_READ_AGENT_DEFINITION, ): MakaTool[] { const byName = new Map(tools.map((tool) => [tool.name, tool])); - const out: MakaTool[] = []; - for (const name of definition.tools) { + return buildToolNamesForAgentDefinition(definition, byName).map((name) => byName.get(name)!); +} + +/** + * The exact tool-name list `buildToolsForAgentDefinition` will emit for this + * definition against these host tools. Durable child snapshots must store this + * list (never the raw definition tools) so resume/retry/restore length checks + * compare like with like; the projection is idempotent, so re-projecting an + * already-projected snapshot is a no-op. + */ +export function projectDefinitionToolNames( + definition: AgentRuntimeDefinition, + tools: readonly MakaTool[], +): string[] { + return buildToolNamesForAgentDefinition( + definition, + new Map(tools.map((tool) => [tool.name, tool])), + ); +} + +function buildToolNamesForAgentDefinition( + definition: AgentRuntimeDefinition, + byName: ReadonlyMap, +): string[] { + const out: string[] = []; + for (const name of projectDefinitionToolNamesToAvailableSurface(definition, byName)) { const tool = byName.get(name); if (!tool) continue; - out.push(tool); + out.push(name); + } + const applyPatch = byName.get('ApplyPatch'); + if ( + applyPatch && + !out.includes('ApplyPatch') && + evaluateAgentDefinitionToolAccess(definition, applyPatch).decision === 'allow' + ) { + out.push('ApplyPatch'); } return out; } +function projectDefinitionToolNamesToAvailableSurface( + definition: AgentRuntimeDefinition, + byName: ReadonlyMap, +): string[] { + const applyPatchOnly = byName.has('ApplyPatch') && !byName.has('Write') && !byName.has('Edit'); + if ( + !applyPatchOnly || + (!definition.tools.includes('Write') && !definition.tools.includes('Edit')) + ) { + return [...definition.tools]; + } + const projected: string[] = []; + for (const name of definition.tools) { + if (name === 'Write' || name === 'Edit') { + if (!projected.includes('ApplyPatch')) projected.push('ApplyPatch'); + continue; + } + projected.push(name); + } + return projected; +} + export function assertAgentDefinitionRunnable(input: { definition: AgentDefinition; tools: readonly MakaTool[]; diff --git a/packages/runtime/src/apply-patch-engine.ts b/packages/runtime/src/apply-patch-engine.ts new file mode 100644 index 0000000000..4b29372478 --- /dev/null +++ b/packages/runtime/src/apply-patch-engine.ts @@ -0,0 +1,275 @@ +/** + * Shared ApplyPatch planner + settlement (#1552). + * + * Hosts supply a filesystem adapter; this module owns path safety, locking + * order, preflight-under-lock, mutation, and result projection so Runtime and + * Headless cannot drift. + */ +import { + assertSafePatchPath, + canonicalizeApplyPatchHunks, + collectPatchPaths, + parseApplyPatch, + planApplyPatchMutations, + type ApplyPatchPathState, + type PlannedPatchMutation, +} from '@maka/core/apply-patch'; + +/** Write/delete intent used for permission preflight before any mutation. */ +export type ApplyPatchAccessIntent = + | { access: 'write'; path: string } + | { access: 'delete'; path: string }; + +export interface ApplyPatchFsAdapter { + /** Stable exclusive lock key for a relative path (may not exist yet). */ + lockKey(path: string): Promise; + /** Inspect the directory entry without following its final symlink. */ + lstat(path: string): Promise<'missing' | 'file' | 'directory' | 'symlink' | 'other'>; + readText(path: string, label: string): Promise; + /** + * Atomically create or replace a regular file. `create` must fail rather + * than clobber an entry that appeared after planning. + */ + writeText( + path: string, + content: string, + mode: 'create' | 'replace', + ): Promise<{ path: string; bytes: number }>; + deletePath(path: string): Promise<{ path: string }>; + /** + * Optional: assert every planned mutation path is currently permitted. + * Must not mutate. Throws structured permission/sandbox errors (including + * `requiredExpansion`) so ToolRuntime can offer boundary retry before any write. + */ + preflightPermissions?(accesses: readonly ApplyPatchAccessIntent[]): Promise; +} + +export interface ApplyPatchOperationResult { + operation: 'add' | 'update' | 'delete' | 'move'; + path: string; + fromPath?: string; + status: 'completed' | 'failed' | 'skipped'; + error?: string; + bytes?: number; +} + +export interface ApplyPatchEngineResult { + ok: boolean; + operations: ApplyPatchOperationResult[]; + completed: string[]; + uncompleted: string[]; + error?: string; + partial?: boolean; +} + +/** + * Parse, plan, lock, revalidate, and settle a Codex ApplyPatch envelope. + * All filesystem reads used for matching and existence checks run under the + * acquired path locks so concurrent writers cannot race the plan. + * + * Permission coverage for every mutation path is checked before the first + * write/delete when the adapter implements `preflightPermissions`. Structured + * sandbox/boundary errors rethrow so hosts keep `requiredExpansion`. + */ +export async function executeApplyPatchWithAdapter( + patchText: string, + fs: ApplyPatchFsAdapter, + withLock: (key: string, run: () => Promise) => Promise, +): Promise { + const parsed = parseApplyPatch(patchText); + if (!parsed.ok) { + const message = + parsed.error.code === 'invalid_hunk' + ? `ApplyPatch parse error at line ${parsed.error.lineNumber}: ${parsed.error.message}` + : `ApplyPatch parse error: ${parsed.error.message}`; + throw new Error(message); + } + + for (const path of collectPatchPaths(parsed.value.hunks)) { + const pathError = assertSafePatchPath(path); + if (pathError) { + throw new Error(`ApplyPatch rejected path ${JSON.stringify(path)}: ${pathError}`); + } + } + const hunks = canonicalizeApplyPatchHunks(parsed.value.hunks); + + const lockKeySet = new Set(); + for (const path of collectPatchPaths(hunks)) { + lockKeySet.add(await fs.lockKey(path)); + } + const orderedKeys = [...lockKeySet].sort(); + + const run = async (): Promise => { + const state = new Map(); + for (const path of collectPatchPaths(hunks)) { + if (state.has(path)) continue; + const kind = await fs.lstat(path); + state.set( + path, + kind === 'file' + ? { kind, content: await fs.readText(path, 'ApplyPatch preflight') } + : { kind: kind === 'directory' ? 'other' : kind }, + ); + } + const prepared = planApplyPatchMutations(hunks, state); + if (fs.preflightPermissions) { + await fs.preflightPermissions(collectAccessIntents(prepared)); + } + return settlePrepared(prepared, fs); + }; + + return withNestedLocks(orderedKeys, withLock, run); +} + +function collectAccessIntents(prepared: readonly PlannedPatchMutation[]): ApplyPatchAccessIntent[] { + const byKey = new Map(); + for (const step of prepared) { + if (step.operation === 'add' || step.operation === 'update') { + byKey.set(`write:${step.path}`, { access: 'write', path: step.path }); + continue; + } + if (step.operation === 'delete') { + byKey.set(`delete:${step.path}`, { access: 'delete', path: step.path }); + continue; + } + // move: destination write + source delete + byKey.set(`write:${step.path}`, { access: 'write', path: step.path }); + byKey.set(`delete:${step.fromPath}`, { access: 'delete', path: step.fromPath }); + } + return [...byKey.values()]; +} + +async function settlePrepared( + prepared: readonly PlannedPatchMutation[], + fs: ApplyPatchFsAdapter, +): Promise { + const operations: ApplyPatchOperationResult[] = []; + const completed: string[] = []; + let failure: string | undefined; + + for (const step of prepared) { + if (failure) { + operations.push({ + operation: step.operation, + path: step.path, + ...(step.operation === 'move' ? { fromPath: step.fromPath } : {}), + status: 'skipped', + }); + continue; + } + + try { + if (step.operation === 'add' || step.operation === 'update') { + const written = await fs.writeText( + step.path, + step.content, + step.operation === 'add' ? 'create' : 'replace', + ); + operations.push({ + operation: step.operation, + path: written.path, + status: 'completed', + bytes: written.bytes, + }); + completed.push(written.path); + continue; + } + + if (step.operation === 'delete') { + const deleted = await fs.deletePath(step.path); + operations.push({ operation: 'delete', path: deleted.path, status: 'completed' }); + completed.push(deleted.path); + continue; + } + + // move: write destination first, then delete source. A failed source + // delete after a successful write is an explicit partial failure. + const written = await fs.writeText(step.path, step.content, 'create'); + completed.push(written.path); + try { + await fs.deletePath(step.fromPath); + operations.push({ + operation: 'move', + path: written.path, + fromPath: step.fromPath, + status: 'completed', + bytes: written.bytes, + }); + } catch (error) { + // Destination already written — treat as partial, not a clean rethrow: + // the error is captured below instead of propagating to ToolRuntime. + failure = error instanceof Error ? error.message : String(error); + operations.push({ + operation: 'move', + path: written.path, + fromPath: step.fromPath, + status: 'failed', + error: failure, + bytes: written.bytes, + }); + } + } catch (error) { + // Before any successful mutation, preserve structured sandbox errors so + // ToolRuntime can surface requiredExpansion for boundary retry. + if (shouldRethrowBoundaryError(error, completed.length)) { + throw error; + } + failure = error instanceof Error ? error.message : String(error); + operations.push({ + operation: step.operation, + path: step.path, + ...(step.operation === 'move' ? { fromPath: step.fromPath } : {}), + status: 'failed', + error: failure, + }); + } + } + + const uncompleted = operations + .filter((op) => op.status !== 'completed') + .map((op) => + op.operation === 'move' && op.status === 'failed' && op.bytes !== undefined && op.fromPath + ? op.fromPath + : op.path, + ); + + if (!failure) { + return { ok: true, operations, completed, uncompleted: [] }; + } + return { + ok: false, + partial: completed.length > 0, + error: failure, + operations, + completed, + uncompleted, + }; +} + +/** + * Boundary / permission errors must reach ToolRuntime when the workspace is + * still clean (no completed mutations). Once a mutation has landed we keep the + * partial-failure result shape instead. + */ +function shouldRethrowBoundaryError(error: unknown, completedCount: number): boolean { + if (completedCount > 0) return false; + if (!error || typeof error !== 'object') return false; + const value = error as { + requiredExpansion?: unknown; + reason?: unknown; + domain?: unknown; + }; + if (value.requiredExpansion !== undefined) return true; + if (value.domain === 'filesystem' && value.reason === 'sandbox_boundary_required') return true; + return false; +} + +async function withNestedLocks( + keys: readonly string[], + withLock: (key: string, run: () => Promise) => Promise, + run: () => Promise, +): Promise { + if (keys.length === 0) return run(); + const [head, ...rest] = keys; + return withLock(head!, () => withNestedLocks(rest, withLock, run)); +} diff --git a/packages/runtime/src/builtin-tools.ts b/packages/runtime/src/builtin-tools.ts index af919c01a6..1444c6ab21 100644 --- a/packages/runtime/src/builtin-tools.ts +++ b/packages/runtime/src/builtin-tools.ts @@ -16,7 +16,7 @@ import { unlinkSync, } from 'node:fs'; import { tmpdir } from 'node:os'; -import { basename, dirname, isAbsolute } from 'node:path'; +import { basename, dirname, isAbsolute, resolve } from 'node:path'; import { compilePermissionProfile, type SandboxBoundaryExpansion, @@ -24,6 +24,12 @@ import { type PermissionProfile, } from '@maka/core'; import { bashToolResultToModelOutput } from './bash-model-output.js'; +export type { EditingProtocol } from '@maka/core/apply-patch'; +import { + executeApplyPatchWithAdapter, + type ApplyPatchAccessIntent, + type ApplyPatchFsAdapter, +} from './apply-patch-engine.js'; import { buildManagedBashTool, buildStopBackgroundTaskTool, @@ -43,11 +49,14 @@ import { type WorkspaceExecResult, type WorkspaceExecutor, } from './workspace-executor.js'; +import { lstat, mkdir, open, realpath, unlink, writeFile } from 'node:fs/promises'; +import { isPathInside, realpathAllowMissing } from './path-containment.js'; + import { createBoundaryFilesystemExecutor, type FilesystemExecuteInput, } from './filesystem-executor.js'; - +import { withFileWriteLock } from './file-write-lock.js'; // tool-runtime.ts is the single source of truth for the tool shape; this // re-export only keeps back-compat for callers that imported from // builtin-tools directly. @@ -63,7 +72,10 @@ import type { ChildFdInput } from './child-fd-input.js'; import { buildArchiveReadTool } from './archive-read-tool.js'; import type { ToolResultArchiveResourceReader } from './tool-result-archive-resource.js'; import { normalizeSandboxBoundaryPath } from './sandbox-boundary-path.js'; -import type { FilesystemWorkerClient } from './filesystem-worker/client.js'; +import { + preflightApplyPatchWrites, + type FilesystemWorkerClient, +} from './filesystem-worker/client.js'; import { preflightDeclaredSandboxBoundary, sandboxBoundaryExpansionSchema, @@ -399,6 +411,33 @@ export function buildBuiltinTools(options: BuildBuiltinToolsOptions = {}): MakaT }; }, }, + { + name: 'ApplyPatch', + activityKind: 'edit', + categoryHint: 'file_write', + description: + 'Apply a Codex-compatible multi-file patch in one call. Pass the full patch ' + + 'envelope (*** Begin Patch … *** End Patch) with Add File / Update File ' + + '(optional Move to) / Delete File operations. Paths must be relative to the ' + + 'session cwd. The complete patch is parsed and preflighted before any mutation; ' + + 'syntax errors, path escapes, missing targets, and hunk mismatches leave the ' + + 'workspace unchanged. Subject to permission policy.', + parameters: z.object({ + patch: z + .string() + .describe('Full *** Begin Patch … *** End Patch text (Codex apply_patch envelope).'), + }), + executionFacts, + impl: async ({ patch }, ctx) => { + const fs = createRuntimeApplyPatchFs({ + ctx, + executor, + filesystemWorker: options.filesystemWorker, + permissionProfile: options.permissionProfile, + }); + return await executeApplyPatchWithAdapter(patch, fs, withFileWriteLock); + }, + }, { name: 'FormatJson', activityKind: 'edit', @@ -505,7 +544,9 @@ export function buildBuiltinTools(options: BuildBuiltinToolsOptions = {}): MakaT }, }, ]; - return tools.filter((tool) => options.includeEdit !== false || tool.name !== 'Edit'); + // Builders bind implementations only. The normalized per-run product-tool + // policy is the single authority that selects the editing protocol. + return tools.filter((tool) => tool.name !== 'Edit' || options.includeEdit !== false); } /** The per-call context every file tool hands to the filesystem authority. */ @@ -985,6 +1026,270 @@ function effectivePermissionProfile( return { profile: compiled.profile, workspaceRoots: compiled.workspaceRoots }; } +function filesystemWorkerForExecution( + worker: Pick | undefined, + ctx: MakaToolContext, +): Pick | undefined { + if (ctx.executionBoundary?.kind === 'bypass' || ctx.executionBoundary?.kind === 'external') { + return undefined; + } + if (worker) return worker; + if (ctx.executionBoundary?.kind !== 'managed') return undefined; + throw new SandboxCommandError({ + domain: 'filesystem', + stage: 'capability', + reason: 'requires_bypass', + recoverable: false, + profileName: ctx.executionBoundary.profile.name ?? ctx.executionBoundary.profile.type, + message: + 'Managed filesystem execution is unavailable because the sandboxed worker cannot be enforced.', + }); +} + +async function fileToolWriteLockKey(cwd: string, path: string): Promise { + const target = await normalizeSandboxBoundaryPath({ + path, + access: 'write', + scope: 'exact', + cwd, + }); + return target.enforcementPath; +} + +interface RuntimeApplyPatchDeps { + ctx: MakaToolContext; + executor: WorkspaceExecutor; + filesystemWorker?: Pick; + permissionProfile?: PermissionProfile; +} + +function createRuntimeApplyPatchFs(input: RuntimeApplyPatchDeps): ApplyPatchFsAdapter { + const { cwd } = input.ctx; + const filesystemWorker = filesystemWorkerForExecution(input.filesystemWorker, input.ctx); + const canonicalCwd = filesystemWorker ? canonicalExistingPath(cwd) : cwd; + + const workerExecute = async (operation: { + kind: 'lstat' | 'read' | 'write' | 'delete'; + path: string; + content?: string; + mode?: 'create' | 'replace'; + }) => { + if (!filesystemWorker) throw new Error('Filesystem worker is unavailable.'); + return filesystemWorker.execute({ + operation: + operation.kind === 'write' + ? { + kind: 'write', + path: operation.path, + content: operation.content ?? '', + ...(operation.mode ? { mode: operation.mode } : {}), + } + : operation.kind === 'delete' + ? { kind: 'delete', path: operation.path } + : operation.kind === 'lstat' + ? { kind: 'lstat', path: operation.path } + : { kind: 'read', path: operation.path }, + cwd: canonicalCwd, + ...(input.ctx.executionBoundary ? { executionBoundary: input.ctx.executionBoundary } : {}), + mode: input.ctx.permissionMode ?? 'ask', + ...(input.permissionProfile ? { permissionProfile: input.permissionProfile } : {}), + ...(input.ctx.abortSignal ? { abortSignal: input.ctx.abortSignal } : {}), + }); + }; + + return { + async lockKey(path) { + if (filesystemWorker) { + // Delete/Move address a directory entry, never the link target, so + // the lock key is the canonical entry path: concurrent ops through + // the same entry serialize on one key without rejecting an escaping + // link before its op runs. + const target = await assertApplyPatchEntryContained(canonicalCwd, path); + return target.entryPath; + } + return (await input.executor.writeLockKey({ cwd, path })).key; + }, + async lstat(path) { + if (filesystemWorker) { + const result = await workerExecute({ kind: 'lstat', path }); + if (result.kind !== 'lstat') + throw new Error('Filesystem worker returned mismatched lstat.'); + return result.targetType; + } + const target = await assertApplyPatchEntryContained(cwd, path); + try { + const metadata = await lstat(target.entryPath); + if (metadata.isSymbolicLink()) return 'symlink'; + if (metadata.isFile()) return 'file'; + return 'other'; + } catch (error) { + if ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error.code === 'ENOENT' || error.code === 'ENOTDIR') + ) { + return 'missing'; + } + throw error; + } + }, + async readText(path, label) { + if (filesystemWorker) { + const result = await workerExecute({ kind: 'read', path }); + if (result.kind === 'read_image') throw new Error(`${label} does not support image files.`); + if (result.kind !== 'read') throw new Error(`${label}: unexpected read result for ${path}`); + return result.content; + } + const { path: resolvedPath } = await input.executor.resolveExistingPath({ + cwd, + path, + label, + scope: 'workspace', + }); + const read = await input.executor.readFile({ cwd, path: resolvedPath }); + if ('bytes' in read) throw new Error(`${label} does not support image files.`); + return read.content; + }, + async writeText(path, content, mode) { + if (filesystemWorker) { + const result = await workerExecute({ kind: 'write', path, content, mode }); + if (result.kind !== 'write') { + throw new Error('Filesystem worker returned a mismatched write.'); + } + return { path: result.path, bytes: result.bytes }; + } + const target = await assertApplyPatchPathContained(cwd, path); + await mkdir(dirname(target.enforcementPath), { recursive: true }); + if (mode === 'create') { + await writeFile(target.enforcementPath, content, { encoding: 'utf8', flag: 'wx' }); + return { path: target.enforcementPath, bytes: Buffer.byteLength(content, 'utf8') }; + } + // Follow-final containment (#2059): an Update lands on the canonical + // target, never through a link, and O_NOFOLLOW keeps it there. + const metadata = await lstat(target.enforcementPath); + if (!metadata.isFile() || metadata.isSymbolicLink()) { + throw new Error(`ApplyPatch Update target must be a regular file: ${path}`); + } + const handle = await open( + target.enforcementPath, + constants.O_NOFOLLOW === undefined ? 'r+' : constants.O_WRONLY | constants.O_NOFOLLOW, + ); + try { + await handle.truncate(0); + await handle.writeFile(content, 'utf8'); + } finally { + await handle.close(); + } + return { path: target.enforcementPath, bytes: Buffer.byteLength(content, 'utf8') }; + }, + async deletePath(path) { + if (filesystemWorker) { + const result = await workerExecute({ kind: 'delete', path }); + if (result.kind !== 'delete') { + throw new Error('Filesystem worker returned a mismatched delete.'); + } + return { path: result.path }; + } + const target = await assertApplyPatchEntryContained(cwd, path); + await unlink(target.entryPath); + return { path: target.entryPath }; + }, + async preflightPermissions(accesses: readonly ApplyPatchAccessIntent[]) { + await preflightApplyPatchPermissions({ + accesses, + cwd, + canonicalCwd, + ctx: input.ctx, + executor: input.executor, + filesystemWorker, + permissionProfile: input.permissionProfile, + }); + }, + }; +} + +async function assertApplyPatchPathContained( + cwd: string, + path: string, +): Promise>> { + const root = await realpath(cwd); + const target = await normalizeSandboxBoundaryPath({ + path, + access: 'write', + scope: 'exact', + cwd: root, + }); + if (!isPathInside(root, target.displayPath) || !isPathInside(root, target.enforcementPath)) { + throw new Error(`ApplyPatch path must stay inside session cwd: ${path}`); + } + return target; +} + +/** + * Canonicalise a directory entry (Delete/Move source, lstat operand) without + * following its final symlink: the parent chain resolves in realpath space + * (#2059 containment authority) and the leaf name is appended. A link inside + * the root whose parent chain resolves outside still fails containment; the + * entry itself (file or link) stays addressable for lstat/unlink. + */ +async function assertApplyPatchEntryContained( + cwd: string, + path: string, +): Promise<{ displayPath: string; entryPath: string }> { + const root = await realpath(cwd); + const displayPath = resolve(root, path); + if (!isPathInside(root, displayPath)) { + throw new Error(`ApplyPatch path must stay inside session cwd: ${path}`); + } + const parentReal = await realpathAllowMissing(dirname(displayPath)); + const entryPath = resolve(parentReal, basename(displayPath)); + if (!isPathInside(root, entryPath)) { + throw new Error(`ApplyPatch path must stay inside session cwd: ${path}`); + } + return { displayPath, entryPath }; +} + +async function preflightApplyPatchPermissions(input: { + accesses: readonly ApplyPatchAccessIntent[]; + cwd: string; + canonicalCwd: string; + ctx: MakaToolContext; + executor: WorkspaceExecutor; + filesystemWorker?: Pick; + permissionProfile?: PermissionProfile; +}): Promise { + if (input.accesses.length === 0) return; + + // Without a managed filesystem worker, path resolve is the host safety check. + if (!input.filesystemWorker) { + for (const intent of input.accesses) { + if (intent.access === 'write') { + await assertApplyPatchPathContained(input.cwd, intent.path); + } else { + await assertApplyPatchEntryContained(input.cwd, intent.path); + } + } + return; + } + + // With a managed worker, the worker client owns normalization, profile + // compilation, writable-root widening, and the structured boundary error + // (with requiredExpansion for every missing path). Keeping this planning in + // one authority means the preflight and the mutation that follows can never + // disagree about a path's classification. + await preflightApplyPatchWrites({ + intents: input.accesses.map((intent) => ({ + path: intent.path, + access: intent.access, + })), + cwd: input.canonicalCwd, + ...(input.ctx.executionBoundary ? { executionBoundary: input.ctx.executionBoundary } : {}), + mode: input.ctx.permissionMode ?? 'ask', + ...(input.permissionProfile ? { permissionProfile: input.permissionProfile } : {}), + }); +} + function terminalError( message: string, result: Pick & { diff --git a/packages/runtime/src/continuation-replay.ts b/packages/runtime/src/continuation-replay.ts index 8bc894e121..3aafdc56ae 100644 --- a/packages/runtime/src/continuation-replay.ts +++ b/packages/runtime/src/continuation-replay.ts @@ -1,14 +1,13 @@ import { createHash } from 'node:crypto'; +import { stableJsonStringify, type RuntimeEvent } from '@maka/core'; import { createRuntimeBoundaryCursor, runtimePrefixSegment, - stableJsonStringify, type ImmutableRuntimePrefixV1, type RuntimeBoundaryCursorV1, type RuntimeBoundaryDigest, - type RuntimeEvent, type RuntimePrefixSegmentV1, -} from '@maka/core'; +} from '@maka/core/runtime-boundary'; import type { RuntimeEventModelReplayItem, RuntimeEventReplayDiagnostic } from './model-history.js'; import { buildRuntimeEventModelReplayPlan, diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index abe6ce1704..b60d8f87fa 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -10,8 +10,8 @@ // "full access" stricter than ask mode, which grants :slash_tmp outright (#2083). import { Buffer } from 'node:buffer'; -import { realpath } from 'node:fs/promises'; -import { isAbsolute } from 'node:path'; +import { lstat, realpath, unlink } from 'node:fs/promises'; +import { isAbsolute, resolve } from 'node:path'; import type { ExecutionBoundary, PermissionMode, PermissionProfile } from '@maka/core'; import { computeEditedSource } from './edit-replace.js'; import { withFileWriteLock } from './file-write-lock.js'; @@ -20,7 +20,10 @@ import type { FilesystemWorkerClientOperation, } from './filesystem-worker/client.js'; import type { ImageMimeType } from './image-file.js'; -import type { FilesystemWorkerResult } from './filesystem-worker/protocol.js'; +import type { + FilesystemWorkerResult, + FilesystemWorkerTarget, +} from './filesystem-worker/protocol.js'; import { normalizeSandboxBoundaryPath } from './sandbox-boundary-path.js'; import { SandboxCommandError } from './sandbox/errors.js'; import type { @@ -320,6 +323,17 @@ function createWorkspaceFilesystemExecutor( }); return { kind: 'grep', matches }; } + default: { + // lstat/delete are entry-semantics operations owned by the + // ApplyPatch engine adapter (builtin-tools) and the filesystem + // worker protocol. The host-local backend deliberately does not + // carry a second copy of entry normalization — keeping one avoids + // the drift that produced path-classification failures (#1556). + const kind = (operation as { kind: string }).kind; + throw new Error( + `Filesystem operation ${kind} is not supported by the host-local backend.`, + ); + } } }, }; @@ -337,6 +351,11 @@ function assertGlobPatternInScope(pattern: string, scope: WorkspacePathScope): v } } +function nodeErrorCode(error: unknown): string | undefined { + if (!error || typeof error !== 'object' || !('code' in error)) return undefined; + return typeof error.code === 'string' ? error.code : undefined; +} + /** The canonical spelling of an existing directory, or the input when it is not resolvable here. */ async function canonicalExistingPath(path: string): Promise { return await realpath(path).catch(() => path); diff --git a/packages/runtime/src/filesystem-worker/client.ts b/packages/runtime/src/filesystem-worker/client.ts index b1c18be422..84573b36e0 100644 --- a/packages/runtime/src/filesystem-worker/client.ts +++ b/packages/runtime/src/filesystem-worker/client.ts @@ -1,7 +1,8 @@ import { randomUUID } from 'node:crypto'; -import { realpath } from 'node:fs/promises'; +import { realpathSync } from 'node:fs'; +import { lstat, realpath } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { dirname } from 'node:path'; +import { basename, dirname, resolve } from 'node:path'; import { canReadPath, canWritePath, @@ -9,10 +10,16 @@ import { type ExecutionBoundary, type PermissionMode, type PermissionProfile, + type SandboxBoundaryAccess, type SandboxBoundaryExpansion, + type SandboxBoundaryScope, } from '@maka/core'; -import { normalizeSandboxBoundaryPath } from '../sandbox-boundary-path.js'; +import { realpathAllowMissing } from '../path-containment.js'; +import { + normalizeSandboxBoundaryPath, + type NormalizedSandboxBoundaryPath, +} from '../sandbox-boundary-path.js'; import { pinExistingLinuxProfilePath } from '../sandbox/linux-profile-path.js'; import type { SandboxManager } from '../sandbox/sandbox-manager.js'; import type { SandboxPlatform } from '../sandbox/types.js'; @@ -59,6 +66,104 @@ export interface FilesystemWorkerExecuteInput { abortSignal?: AbortSignal; } +/** One planned ApplyPatch mutation path, for batch permission preflight. */ +export interface ApplyPatchWriteIntent { + path: string; + /** Delete/Move sources address the directory entry; writes follow the canonical target. */ + access: 'write' | 'delete'; +} + +/** + * Batch permission preflight for a planned ApplyPatch: every mutation path is + * checked under the same authority as a real worker call (same normalization, + * profile compilation, and writable-root widening), before the first mutation + * lands. Throws one structured error carrying `requiredExpansion` covering all + * missing paths, so ToolRuntime can offer the normal boundary retry once. + */ +export async function preflightApplyPatchWrites(input: { + intents: readonly ApplyPatchWriteIntent[]; + cwd: string; + executionBoundary?: ExecutionBoundary; + mode?: PermissionMode; + permissionProfile?: PermissionProfile; + platform?: SandboxPlatform; +}): Promise { + if (input.intents.length === 0) return; + const canonicalCwd = await realpath(input.cwd).catch(() => { + throw clientError( + 'invalid_operation', + 'validation', + 'preflight', + 'Session cwd is unavailable.', + ); + }); + const compiled = + input.executionBoundary?.kind === 'managed' + ? { + profile: input.executionBoundary.profile, + workspaceRoots: [canonicalCwd], + } + : input.permissionProfile + ? { + profile: input.permissionProfile, + workspaceRoots: [canonicalCwd], + } + : compilePermissionProfile({ mode: input.mode ?? 'ask', cwd: canonicalCwd }); + const platform = input.platform ?? process.platform; + const tmpCanonical = await canonicalPath(tmpdir()); + const slashTmpCanonical = await canonicalPath('/tmp'); + const missingEntries: Array<{ path: string; access: 'write'; scope: 'exact' }> = []; + + for (const intent of input.intents) { + const entryMode = intent.access === 'delete'; + const target = entryMode + ? await normalizeDirectoryEntryTarget(canonicalCwd, intent.path, 'write', 'exact') + : await normalizeSandboxBoundaryPath({ + path: intent.path, + access: 'write', + scope: 'exact', + cwd: canonicalCwd, + }); + const runtimeWritableRoots = filesystemWorkerRuntimeWritableRoots({ + platform, + access: 'write', + enforcementPath: target.enforcementPath, + targetType: target.targetType, + entryMode, + }); + const pathContext = { + workspaceRoots: compiled.workspaceRoots, + tmpdir: tmpCanonical, + slashTmp: slashTmpCanonical, + ...(runtimeWritableRoots ? { runtimeWritableRoots } : {}), + }; + if (!canWritePath(compiled.profile, target.enforcementPath, pathContext)) { + missingEntries.push({ path: target.enforcementPath, access: 'write', scope: 'exact' }); + } + } + + if (missingEntries.length === 0) return; + const managed = input.executionBoundary?.kind === 'managed'; + throw clientError( + managed ? 'sandbox_boundary_required' : 'path_denied', + 'validation', + 'preflight', + managed + ? 'ApplyPatch requires sandbox boundary expansion for one or more paths before mutation.' + : 'ApplyPatch path is not writable under the current permission profile.', + true, + managed + ? { + requiredExpansion: { + filesystem: { + entries: missingEntries, + }, + }, + } + : {}, + ); +} + export type FilesystemWorkerClientErrorReason = | 'invalid_operation' | 'invalid_request' @@ -150,12 +255,28 @@ export class FilesystemWorkerClient { if (!parsedOperation.success) throw clientError('invalid_operation', 'validation', requestId); const access = operationAccess(parsedOperation.data.kind); - const target = await normalizeSandboxBoundaryPath({ - path: parsedOperation.data.path, - access, - scope: operationScope(parsedOperation.data.kind), - cwd: canonicalCwd, - }).catch(() => { + // delete/lstat address the directory entry; create-mode (ApplyPatch Add) + // must not clobber an existing entry (including a link). replace-mode + // follows the canonical target like a plain write, so the client and + // worker resolve the same path. + const entryMode = + parsedOperation.data.kind === 'delete' || + parsedOperation.data.kind === 'lstat' || + (parsedOperation.data.kind === 'write' && parsedOperation.data.mode === 'create'); + const target = await (entryMode + ? normalizeDirectoryEntryTarget( + canonicalCwd, + parsedOperation.data.path, + access, + operationScope(parsedOperation.data.kind), + ) + : normalizeSandboxBoundaryPath({ + path: parsedOperation.data.path, + access, + scope: operationScope(parsedOperation.data.kind), + cwd: canonicalCwd, + }) + ).catch(() => { throw clientError('invalid_operation', 'validation', requestId); }); const compiled = @@ -177,12 +298,23 @@ export class FilesystemWorkerClient { access, enforcementPath: target.enforcementPath, targetType: target.targetType, + entryMode, + }); + // Entry-mode read probes (lstat) need the entry's parent reachable too; + // the read-only mount is derived from the same deepest-existing-ancestor + // rule so a missing entry still classifies correctly on Linux. + const runtimeReadableRoots = filesystemWorkerRuntimeReadableRoots({ + platform, + access, + enforcementPath: target.enforcementPath, + entryMode, }); const pathContext = { workspaceRoots: compiled.workspaceRoots, tmpdir: await canonicalPath(tmpdir()), slashTmp: await canonicalPath('/tmp'), ...(runtimeWritableRoots ? { runtimeWritableRoots } : {}), + ...(runtimeReadableRoots ? { runtimeReadableRoots } : {}), }; const allowed = access === 'write' @@ -220,7 +352,14 @@ export class FilesystemWorkerClient { } as const; const operation = FilesystemWorkerOperationSchema.parse({ ...parsedOperation.data, - path: target.enforcementPath, + // Delete must keep the original directory entry as its operand. The + // canonical enforcement path remains pinned separately in + // expectedTarget and operationBoundary so the worker can reject a + // changed target without replacing a symlink operand with its target. + path: + parsedOperation.data.kind === 'delete' || parsedOperation.data.kind === 'lstat' + ? target.displayPath + : target.enforcementPath, }); const request = { version: FILESYSTEM_WORKER_PROTOCOL_VERSION, @@ -243,13 +382,13 @@ export class FilesystemWorkerClient { if (!launch.ok) throw clientError(launch.reason, 'launch', requestId, launch.message); const workerProfile = deriveWorkerProfile(effectiveProfile, operationBoundary); const pinnedTarget = - platform === 'linux' && target.targetType !== 'missing' + platform === 'linux' && !entryMode && target.targetType !== 'missing' ? (() => { try { return pinExistingLinuxProfilePath({ path: target.enforcementPath, access, - targetType: target.targetType, + targetType: target.targetType as 'file' | 'directory' | 'other', childFd: 4, }); } catch { @@ -262,7 +401,7 @@ export class FilesystemWorkerClient { } })() : undefined; - if (platform === 'linux' && target.targetType !== 'missing' && !pinnedTarget) { + if (platform === 'linux' && !entryMode && target.targetType !== 'missing' && !pinnedTarget) { throw clientError( 'path_changed', 'validation', @@ -271,7 +410,9 @@ export class FilesystemWorkerClient { ); } const pinnedRuntimeWritableRoot = - platform === 'linux' && target.targetType === 'missing' && runtimeWritableRoots?.[0] + platform === 'linux' && + runtimeWritableRoots?.[0] && + (entryMode || target.targetType === 'missing') ? (() => { try { return pinExistingLinuxProfilePath({ @@ -292,8 +433,8 @@ export class FilesystemWorkerClient { : undefined; if ( platform === 'linux' && - target.targetType === 'missing' && runtimeWritableRoots && + (entryMode || target.targetType === 'missing') && !pinnedRuntimeWritableRoot ) { throw clientError( @@ -315,7 +456,12 @@ export class FilesystemWorkerClient { profile: workerProfile, pathContext: { ...pathContext, - runtimeReadableRoots: launch.spec.runtimeReadableRoots, + // Keep the entry-probe read-only mount alongside the launch + // spec's own runtime roots (worker executable, Grep runtime). + runtimeReadableRoots: uniqueRoots([ + ...(pathContext.runtimeReadableRoots ?? []), + ...launch.spec.runtimeReadableRoots, + ]), executableRoots: launch.spec.executableRoots, ...(pinnedTarget ? { @@ -425,11 +571,119 @@ export function filesystemWorkerRuntimeWritableRoots(input: { access: 'read' | 'write'; enforcementPath: string; targetType: FilesystemWorkerTarget['targetType']; + entryMode?: boolean; +}): readonly string[] | undefined { + if (input.platform !== 'linux' || input.access !== 'write') { + return undefined; + } + // Entry-mode operations (Delete, lstat probe, ApplyPatch create) address + // the directory entry itself: the worker needs the deepest existing + // ancestor of the entry's parent mounted so it can probe, unlink, or mkdir + // the entry. The entry may not exist yet or may be a symlink that cannot be + // pinned, so the mount target is always the ancestor directory. Replace- + // mode writes pin the exact file instead and need no parent root. + if (input.entryMode || input.targetType === 'missing') { + const ancestor = deepestExistingAncestor(dirname(input.enforcementPath)); + return ancestor ? [ancestor] : undefined; + } + return undefined; +} + +/** + * Read-only parent mount for an entry-mode lstat probe on Linux: the worker + * must be able to reach the entry's parent to classify the entry itself. + */ +export function filesystemWorkerRuntimeReadableRoots(input: { + platform: SandboxPlatform; + access: 'read' | 'write'; + enforcementPath: string; + entryMode?: boolean; }): readonly string[] | undefined { - if (input.platform !== 'linux' || input.access !== 'write' || input.targetType !== 'missing') { + if (input.platform !== 'linux' || input.access !== 'read' || !input.entryMode) { return undefined; } - return [dirname(input.enforcementPath)]; + const ancestor = deepestExistingAncestor(dirname(input.enforcementPath)); + return ancestor ? [ancestor] : undefined; +} + +/** + * The deepest directory on `path`'s chain that exists on disk, so a mount + * (and its pin) always targets a real directory even when the entry itself + * or several parents are missing. + */ +function deepestExistingAncestor(path: string): string | undefined { + let cursor = path; + while (true) { + try { + return realpathSync(cursor); + } catch (error) { + if (!isMissingPathError(error)) throw error; + const parent = dirname(cursor); + if (parent === cursor) return undefined; + cursor = parent; + } + } +} + +function isMissingPathError(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error.code === 'ENOENT' || error.code === 'ENOTDIR') + ); +} + +function uniqueRoots(values: readonly string[]): readonly string[] { + return [...new Set(values)]; +} + +/** + * Canonicalise a directory entry (Delete/Move operand, lstat probe, + * create-mode write) without following its final symlink: the parent chain + * resolves in realpath space and the leaf name is appended, matching the + * worker-side resolveDeleteOperandAllowed semantics. A link inside the root + * whose parent chain resolves outside still fails containment; the entry + * itself (file or link) stays addressable for lstat/unlink/create checks. + */ +async function normalizeDirectoryEntryTarget( + cwd: string, + path: string, + access: SandboxBoundaryAccess, + scope: SandboxBoundaryScope | 'auto', +): Promise< + Omit & { + targetType: FilesystemWorkerTarget['targetType']; + } +> { + const canonicalCwd = await realpath(cwd); + const displayPath = resolve(canonicalCwd, path); + const parentReal = await realpathAllowMissing(dirname(displayPath)); + const enforcementPath = resolve(parentReal, basename(displayPath)); + const targetType = await entryTargetTypeOf(enforcementPath); + const effectiveScope = + scope === 'auto' ? (targetType === 'directory' ? 'subtree' : 'exact') : scope; + return { displayPath, enforcementPath, access, scope: effectiveScope, targetType }; +} + +async function entryTargetTypeOf(path: string): Promise { + try { + const metadata = await lstat(path); + if (metadata.isSymbolicLink()) return 'symlink'; + if (metadata.isFile()) return 'file'; + if (metadata.isDirectory()) return 'directory'; + return 'other'; + } catch (error) { + if ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error.code === 'ENOENT' || error.code === 'ENOTDIR') + ) { + return 'missing'; + } + throw error; + } } function deriveWorkerProfile( @@ -467,7 +721,9 @@ function deriveWorkerProfile( } function operationAccess(kind: FilesystemWorkerOperation['kind']): 'read' | 'write' { - return kind === 'write' || kind === 'edit' || kind === 'format_json' ? 'write' : 'read'; + return kind === 'write' || kind === 'edit' || kind === 'format_json' || kind === 'delete' + ? 'write' + : 'read'; } function operationScope(kind: FilesystemWorkerOperation['kind']): 'exact' | 'subtree' | 'auto' { diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 800f35336f..58425ab18b 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -1,7 +1,7 @@ import { spawn } from 'node:child_process'; -import { promises as fs } from 'node:fs'; +import { constants as fsConstants, promises as fs } from 'node:fs'; import { glob as nodeGlob } from 'node:fs/promises'; -import { dirname, isAbsolute, parse, resolve } from 'node:path'; +import { basename, dirname, isAbsolute, parse, resolve } from 'node:path'; import { isPathInside, realpathAllowMissing } from '../path-containment.js'; import { sandboxBoundaryExpansionAllowsPath } from '@maka/core'; @@ -49,7 +49,13 @@ export async function executeFilesystemWorkerRequest( dependencies: FilesystemWorkerOperationDependencies = {}, ): Promise { try { - await assertTargetUnchanged(request.operation.path, request.expectedTarget); + await assertTargetUnchanged( + request.operation.path, + request.expectedTarget, + request.operation.kind === 'delete' || + request.operation.kind === 'lstat' || + (request.operation.kind === 'write' && request.operation.mode === 'create'), + ); return { version: FILESYSTEM_WORKER_PROTOCOL_VERSION, requestId: request.requestId, @@ -77,6 +83,20 @@ export async function executeFilesystemOperation( dependencies: FilesystemWorkerOperationDependencies = {}, ): Promise { switch (operation.kind) { + case 'lstat': { + // Address the directory entry (never the followed target) with the + // same containment as Delete, so a plan-time symlink probe is + // consistent with the mutation that follows. Missing entries are a + // legitimate probe result (Add/Move preflight), not an error: the + // target-type classification below reports them as `missing`. + const path = await resolveLstatOperandAllowed( + operation.cwd, + operation.path, + 'ApplyPatch lstat', + operationBoundary, + ); + return { kind: 'lstat', targetType: await lstatTargetTypeOf(path) }; + } case 'read': { const path = await resolveExistingAllowed( operation.cwd, @@ -109,12 +129,57 @@ export async function executeFilesystemOperation( return { kind: 'read', content: lines.slice(start, end).join('\n') }; } case 'write': { - const path = await resolveWritableAllowed( - operation.cwd, - operation.path, - 'Write', - operationBoundary, - ); + // create-mode (ApplyPatch Add) addresses the directory entry so an + // existing link blocks the add; replace-mode follows the canonical + // target like a plain write (#2059). + const path = + operation.mode === 'create' + ? await resolveDirectoryEntryWritableAllowed( + operation.cwd, + operation.path, + 'Write', + operationBoundary, + ) + : await resolveWritableAllowed(operation.cwd, operation.path, 'Write', operationBoundary); + await fs.mkdir(dirname(path), { recursive: true }); + if (operation.mode === 'create') { + const handle = await fs.open(path, 'wx'); + try { + await handle.writeFile(operation.content, 'utf8'); + } finally { + await handle.close(); + } + return { + kind: 'write', + ok: true, + path, + bytes: Buffer.byteLength(operation.content, 'utf8'), + }; + } + if (operation.mode === 'replace') { + const metadata = await fs.lstat(path); + if (!metadata.isFile() || metadata.isSymbolicLink()) { + throw operationError('filesystem_error', 'Write replacement requires a regular file.'); + } + const handle = await fs.open( + path, + fsConstants.O_NOFOLLOW === undefined + ? 'r+' + : fsConstants.O_WRONLY | fsConstants.O_NOFOLLOW, + ); + try { + await handle.truncate(0); + await handle.writeFile(operation.content, 'utf8'); + } finally { + await handle.close(); + } + return { + kind: 'write', + ok: true, + path, + bytes: Buffer.byteLength(operation.content, 'utf8'), + }; + } await fs.writeFile(path, operation.content, 'utf8'); return { kind: 'write', ok: true, path, bytes: Buffer.byteLength(operation.content, 'utf8') }; } @@ -191,6 +256,16 @@ export async function executeFilesystemOperation( changed: formatted !== original, }; } + case 'delete': { + const path = await resolveDeleteOperandAllowed( + operation.cwd, + operation.path, + 'Delete', + operationBoundary, + ); + await fs.unlink(path); + return { kind: 'delete', ok: true, path }; + } case 'glob': { assertContainedGlobPattern(operation.pattern); const path = await resolveExistingAllowed( @@ -291,9 +366,14 @@ function normalizeOperationError(error: unknown): FilesystemOperationError { async function assertTargetUnchanged( path: string, expected: FilesystemWorkerTarget, + noFollowFinalSymlink = false, ): Promise { - const enforcementPath = await realpathAllowMissing(path); - const targetType = await targetTypeOf(enforcementPath); + const enforcementPath = noFollowFinalSymlink + ? await canonicalDirectoryEntryPath(path) + : await realpathAllowMissing(path); + const targetType = noFollowFinalSymlink + ? await lstatTargetTypeOf(enforcementPath) + : await targetTypeOf(enforcementPath); if (enforcementPath !== expected.enforcementPath || targetType !== expected.targetType) { throw operationError( 'path_changed', @@ -302,6 +382,11 @@ async function assertTargetUnchanged( } } +async function canonicalDirectoryEntryPath(path: string): Promise { + const parent = dirname(path); + return resolve(await realpathAllowMissing(parent), basename(path)); +} + async function resolveWritableAllowed( cwd: string, inputPath: string, @@ -333,6 +418,73 @@ async function resolveWritableAllowed( return followed; } +async function resolveDirectoryEntryWritableAllowed( + cwd: string, + inputPath: string, + label: string, + permission: FilesystemWorkerRequest['operationBoundary'], +): Promise { + const { root, candidate } = await resolveCandidate(cwd, inputPath, label, 'write', permission); + const operand = await canonicalDirectoryEntryPath(candidate); + assertAllowed(root, operand, label, 'write', permission); + return operand; +} + +async function resolveLstatOperandAllowed( + cwd: string, + inputPath: string, + label: string, + permission: FilesystemWorkerRequest['operationBoundary'], +): Promise { + const { root, candidate } = await resolveCandidate(cwd, inputPath, label, 'read', permission); + // The parent may itself be missing (nested Add/Move destinations). Resolve + // the deepest existing ancestor and keep the missing segments so a missing + // entry anywhere below the root classifies as `missing`, not not_found. + const parent = await realpathAllowMissing(dirname(candidate)); + if (!isPathInside(root, parent)) { + throw operationError('path_denied', `${label} path escaped its approved target.`); + } + const operand = resolve(parent, basename(candidate)); + const metadata = await fs.lstat(operand).catch((error) => { + // A missing entry is a valid probe result (Add/Move destinations). + if (nodeErrorCode(error) === 'ENOENT') return undefined; + throw error; + }); + if (metadata === undefined) { + // The caller classifies the operand with lstatTargetTypeOf, which maps + // ENOENT to `missing`. The parent containment above still holds, so a + // missing entry inside the root is a legal answer. + return operand; + } + if (metadata.isSymbolicLink()) { + return operand; + } + const target = await fs.realpath(operand); + assertAllowed(root, target, label, 'read', permission); + return operand; +} + +async function resolveDeleteOperandAllowed( + cwd: string, + inputPath: string, + label: string, + permission: FilesystemWorkerRequest['operationBoundary'], +): Promise { + const { root, candidate } = await resolveCandidate(cwd, inputPath, label, 'write', permission); + const parent = await fs.realpath(dirname(candidate)); + if (!isPathInside(root, parent)) { + throw operationError('path_denied', `${label} path escaped its approved target.`); + } + const operand = resolve(parent, basename(candidate)); + const metadata = await fs.lstat(operand); + if (metadata.isSymbolicLink()) { + return operand; + } + const target = await fs.realpath(operand); + assertAllowed(root, target, label, 'write', permission); + return operand; +} + async function resolveExistingAllowed( cwd: string, inputPath: string, @@ -410,6 +562,19 @@ async function targetTypeOf(path: string): Promise { + try { + const metadata = await fs.lstat(path); + if (metadata.isSymbolicLink()) return 'symlink'; + if (metadata.isFile()) return 'file'; + if (metadata.isDirectory()) return 'directory'; + return 'other'; + } catch (error) { + if (nodeErrorCode(error) === 'ENOENT') return 'missing'; + throw error; + } +} + function nodeErrorCode(error: unknown): string | undefined { if (!error || typeof error !== 'object' || !('code' in error)) return undefined; return typeof error.code === 'string' ? error.code : undefined; diff --git a/packages/runtime/src/filesystem-worker/protocol.ts b/packages/runtime/src/filesystem-worker/protocol.ts index 84f666e8fe..8e720ff13e 100644 --- a/packages/runtime/src/filesystem-worker/protocol.ts +++ b/packages/runtime/src/filesystem-worker/protocol.ts @@ -1,7 +1,9 @@ import { z } from 'zod'; import { validateSandboxBoundaryExpansion } from '@maka/core'; -export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 4 as const; +// v6: directory-entry lstat plus create-vs-replace write semantics for the +// transactional ApplyPatch applier (#1552). +export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 6 as const; const path = z.string().min(1).max(4096); const cwd = z.string().min(1).max(4096); @@ -40,7 +42,7 @@ export const FilesystemWorkerTargetSchema = z enforcementPath: path, access: z.enum(['read', 'write']), scope: z.enum(['exact', 'subtree']), - targetType: z.enum(['file', 'directory', 'other', 'missing']), + targetType: z.enum(['file', 'directory', 'symlink', 'other', 'missing']), }) .strict(); @@ -54,7 +56,16 @@ export const FilesystemWorkerOperationSchema = z.discriminatedUnion('kind', [ limit: z.number().int().positive().optional(), }) .strict(), - z.object({ kind: z.literal('write'), cwd, path, content: z.string() }).strict(), + z.object({ kind: z.literal('lstat'), cwd, path }).strict(), + z + .object({ + kind: z.literal('write'), + cwd, + path, + content: z.string(), + mode: z.enum(['create', 'replace']).optional(), + }) + .strict(), z .object({ kind: z.literal('edit'), @@ -72,6 +83,7 @@ export const FilesystemWorkerOperationSchema = z.discriminatedUnion('kind', [ sortKeys: z.boolean(), }) .strict(), + z.object({ kind: z.literal('delete'), cwd, path }).strict(), z .object({ kind: z.literal('glob'), @@ -106,6 +118,12 @@ export const FilesystemWorkerRequestSchema = z .strict(); export const FilesystemWorkerResultSchema = z.discriminatedUnion('kind', [ + z + .object({ + kind: z.literal('lstat'), + targetType: z.enum(['file', 'directory', 'symlink', 'other', 'missing']), + }) + .strict(), z.object({ kind: z.literal('read'), content: z.string() }).strict(), z .object({ @@ -146,6 +164,13 @@ export const FilesystemWorkerResultSchema = z.discriminatedUnion('kind', [ changed: z.boolean(), }) .strict(), + z + .object({ + kind: z.literal('delete'), + ok: z.literal(true), + path: z.string(), + }) + .strict(), z.object({ kind: z.literal('glob'), files: z.array(z.string()) }).strict(), z.object({ kind: z.literal('grep'), matches: z.array(z.string()) }).strict(), ]); diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index 07762c6459..9c98ba3424 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -421,9 +421,28 @@ export { queryTavily } from './tavily-search.js'; export { buildWebSearchTool } from './web-search-tool.js'; export type { BuildBuiltinToolsOptions, + EditingProtocol, MakaTool as BuiltinMakaTool, MakaToolContext as BuiltinMakaToolContext, } from './builtin-tools.js'; +export { + parseApplyPatch, + applyUpdateChunksToContent, + assertSafePatchPath, + collectPatchPaths, +} from '@maka/core/apply-patch'; +export type { + ApplyPatchHunk, + ApplyPatchParseOutcome, + ApplyPatchUpdateChunk, +} from '@maka/core/apply-patch'; +export { executeApplyPatchWithAdapter } from './apply-patch-engine.js'; +export type { + ApplyPatchAccessIntent, + ApplyPatchEngineResult, + ApplyPatchFsAdapter, + ApplyPatchOperationResult, +} from './apply-patch-engine.js'; export { buildToolResultArchiveResourceRef, parseToolResultArchiveResourceRef, diff --git a/packages/runtime/src/runtime-kernel.ts b/packages/runtime/src/runtime-kernel.ts index 9bf7232165..de2cf8e8cd 100644 --- a/packages/runtime/src/runtime-kernel.ts +++ b/packages/runtime/src/runtime-kernel.ts @@ -76,6 +76,7 @@ import { buildToolsForAgentDefinition, requireBuiltinAgentDefinition, } from './agent-catalog.js'; +import { childAgentToolsWithinEditingProtocol } from './subagent-tools.js'; import { loadLatestHistoryCompactCheckpointFromRunLedger } from './history-compact-ledger.js'; import { canReplaceHistoryCompactCheckpoint, @@ -1154,7 +1155,10 @@ export class RuntimeKernel implements RuntimeKernelLike { await this.enterExecutionClaim(execution); const parentHeader = await this.deps.store.readHeader(sessionId); const definition = requireBuiltinAgentDefinition(input.spec.id); - const availableChildTools = this.deps.childTools ?? []; + const availableChildTools = childAgentToolsWithinEditingProtocol( + this.deps.childTools ?? [], + parentHeader.editingProtocol, + ); assertAgentDefinitionRunnable({ definition, tools: availableChildTools, @@ -1256,7 +1260,10 @@ export class RuntimeKernel implements RuntimeKernelLike { tools: linkedSnapshot.toolNames, } : requireBuiltinAgentDefinition(input.spec.id); - const availableChildTools = this.deps.childTools ?? []; + const availableChildTools = childAgentToolsWithinEditingProtocol( + this.deps.childTools ?? [], + parentHeader.editingProtocol, + ); if (!linkedSnapshot) { assertAgentDefinitionRunnable({ definition: requireBuiltinAgentDefinition(input.spec.id), @@ -2932,7 +2939,10 @@ export class RuntimeKernel implements RuntimeKernelLike { permissionMode: header.permissionMode, tools: snapshot.toolNames, }; - const availableTools = this.deps.childTools ?? []; + const availableTools = childAgentToolsWithinEditingProtocol( + this.deps.childTools ?? [], + header.editingProtocol, + ); const tools = buildToolsForAgentDefinition(availableTools, snapshotDefinition); if (tools.length !== snapshot.toolNames.length) { throw new Error('Subagent runtime tool snapshot is unavailable'); diff --git a/packages/runtime/src/session-manager.ts b/packages/runtime/src/session-manager.ts index c141840369..6e7829d4b7 100644 --- a/packages/runtime/src/session-manager.ts +++ b/packages/runtime/src/session-manager.ts @@ -192,6 +192,7 @@ import { buildToolsForAgentDefinition, getBuiltinAgentDefinition, listBuiltinAgentDefinitions, + projectDefinitionToolNames, requireBuiltinAgentDefinition, requireBuiltinAgentDefinitionByProfile, AGENT_WORKSPACE_WORKTREE, @@ -200,6 +201,7 @@ import { type AgentDefinitionListItem, type SubagentPresetListItem, } from './agent-catalog.js'; +import { childAgentToolsWithinEditingProtocol } from './subagent-tools.js'; import { buildRuntimeEventModelReplayPlan } from './model-history.js'; import { stableHash } from './request-shape.js'; import type { SubagentExecutionRef } from './subagent-execution.js'; @@ -2398,9 +2400,13 @@ export class SessionManager { } const definition = requireBuiltinAgentDefinition(input.agentId); + const availableChildTools = childAgentToolsWithinEditingProtocol( + this.deps.childTools ?? [], + parentHeader.editingProtocol, + ); assertAgentDefinitionRunnable({ definition, - tools: this.deps.childTools ?? [], + tools: availableChildTools, worktreeChildExecutorAvailable: this.hasWorktreeChildExecutor(), }); const childPermissionMode = @@ -2427,7 +2433,8 @@ export class SessionManager { profile: definition.profile, workspace: definition.contract.workspace, permissionMode: childPermissionMode, - toolNames: [...definition.tools], + editingProtocol: parentHeader.editingProtocol ?? 'edit_write', + toolNames: projectDefinitionToolNames(definition, availableChildTools), categoryPolicy: {}, systemPrompt: definition.systemPrompt, }, @@ -2486,7 +2493,7 @@ export class SessionManager { agentName: definition.name, profile: definition.profile, systemPrompt: definition.systemPrompt, - toolNames: [...definition.tools], + toolNames: projectDefinitionToolNames(definition, availableChildTools), categoryPolicy: {}, }, subagentSpawn: { @@ -2967,7 +2974,10 @@ export class SessionManager { this.assertActiveParentRun(parentSessionId, parentRun, input.spawnedBy.parentTurnId); const definition = requireBuiltinAgentDefinitionByProfile(input.agentProfile); - const availableChildTools = this.deps.childTools ?? []; + const availableChildTools = childAgentToolsWithinEditingProtocol( + this.deps.childTools ?? [], + parentHeader.editingProtocol, + ); assertAgentDefinitionRunnable({ definition, tools: availableChildTools, @@ -2997,6 +3007,7 @@ export class SessionManager { ? { thinkingLevel: parentHeader.thinkingLevel } : {}), permissionMode: definition.permissionMode, + editingProtocol: parentHeader.editingProtocol ?? 'edit_write', collaborationMode: 'agent', orchestrationMode: 'default', subagentParent: { @@ -3014,7 +3025,7 @@ export class SessionManager { profile: definition.profile, ...(input.resolvedPreset ? { presetId: input.resolvedPreset.id } : {}), systemPrompt: definition.systemPrompt, - toolNames: [...definition.tools], + toolNames: projectDefinitionToolNames(definition, availableChildTools), categoryPolicy: {}, }, subagentSpawn: { @@ -3304,9 +3315,13 @@ export class SessionManager { const sessionHeader = await this.deps.store.readHeader(sessionId); await this.ensureChildWorkspace(sessionHeader); + const availableChildTools = childAgentToolsWithinEditingProtocol( + this.deps.childTools ?? [], + sessionHeader.editingProtocol, + ); assertAgentDefinitionRunnable({ definition, - tools: this.deps.childTools ?? [], + tools: availableChildTools, worktreeChildExecutorAvailable: this.hasWorktreeChildExecutor(), }); const visited = new Set(); @@ -3432,11 +3447,14 @@ export class SessionManager { } await this.ensureChildWorkspace(child); await this.assertLinkedChildBoundaryMatchesParent(parentSessionId, child.id); - const runnableTools = buildToolsForAgentDefinition(this.deps.childTools ?? [], { - id: snapshot.agentId, - permissionMode: child.permissionMode, - tools: snapshot.toolNames, - }); + const runnableTools = buildToolsForAgentDefinition( + childAgentToolsWithinEditingProtocol(this.deps.childTools ?? [], child.editingProtocol), + { + id: snapshot.agentId, + permissionMode: child.permissionMode, + tools: snapshot.toolNames, + }, + ); if (runnableTools.length !== snapshot.toolNames.length) { throw new Error('Child Session durable runtime tool snapshot is unavailable'); } @@ -4281,8 +4299,12 @@ export class SessionManager { } async listChildAgents(sessionId: string): Promise { + const header = await this.deps.store.readHeader(sessionId); const definitions = listBuiltinAgentDefinitions({ - tools: this.deps.childTools ?? [], + tools: childAgentToolsWithinEditingProtocol( + this.deps.childTools ?? [], + header.editingProtocol, + ), worktreeChildExecutorAvailable: this.hasWorktreeChildExecutor(), }); const presets = this.deps.subagentCatalog ? await this.deps.subagentCatalog.list() : []; @@ -4919,6 +4941,7 @@ export class SessionManager { model: header.model, thinkingLevel: header.thinkingLevel, permissionMode: header.permissionMode, + editingProtocol: header.editingProtocol ?? 'edit_write', collaborationMode: header.collaborationMode, orchestrationMode: header.orchestrationMode ?? 'default', name: header.name, @@ -4987,6 +5010,7 @@ export class SessionManager { model: header.model, thinkingLevel: header.thinkingLevel, permissionMode: header.permissionMode, + editingProtocol: header.editingProtocol ?? 'edit_write', collaborationMode: header.collaborationMode, orchestrationMode: header.orchestrationMode ?? 'default', name: input.name ?? `${header.name} · 分支`, @@ -6119,6 +6143,7 @@ export function headerToSummary(h: SessionHeader): SessionSummary { connectionLocked: h.connectionLocked, model: h.model, permissionMode: h.permissionMode ?? 'ask', + editingProtocol: h.editingProtocol ?? 'edit_write', collaborationMode: h.collaborationMode ?? 'agent', orchestrationMode: h.orchestrationMode ?? 'default', }; diff --git a/packages/runtime/src/subagent-tools.ts b/packages/runtime/src/subagent-tools.ts index a1af1d0d93..0a932a49d7 100644 --- a/packages/runtime/src/subagent-tools.ts +++ b/packages/runtime/src/subagent-tools.ts @@ -3,6 +3,7 @@ import { TASK_ID_MAX_CHARS, decodeCanonicalToolResultContent, isSafeTaskId, + type EditingProtocol, isSafeSubagentPresetId, type TaskLedgerStore, type ToolResultContent, @@ -34,7 +35,10 @@ export const AGENT_TOOL_NAMES = [ ] as const; const CHILD_RECOVERY_TOOL_NAMES = ['ArchiveRead'] as const; export const CHILD_AGENT_TOOL_NAMES = [ - ...new Set(BUILTIN_AGENT_DEFINITIONS.flatMap((definition) => definition.tools)), + ...new Set([ + ...BUILTIN_AGENT_DEFINITIONS.flatMap((definition) => definition.tools), + 'ApplyPatch', + ]), ] as readonly string[]; const AGENT_SPAWN_WRITE_BACK_MODES = [AGENT_WRITE_BACK_SUMMARY, AGENT_WRITE_BACK_PATCH] as const; const AGENT_SPAWN_ISOLATION_MODES = [ @@ -79,6 +83,24 @@ export function buildChildAgentTools(tools: readonly MakaTool[]): MakaTool[] { return out; } +/** + * Enforce the parent's editing surface as a hard capability ceiling. + * + * Hosts bind a union so different sessions can choose different protocols. + * A child may narrow that union for its profile, but it must never regain the + * editing protocol hidden from the parent session. + */ +export function childAgentToolsWithinEditingProtocol( + tools: readonly MakaTool[], + editingProtocol: EditingProtocol = 'edit_write', +): MakaTool[] { + return tools.filter((tool) => + editingProtocol === 'apply_patch' + ? tool.name !== 'Edit' && tool.name !== 'Write' + : tool.name !== 'ApplyPatch', + ); +} + export function buildSubagentSpawnTool( deps: { taskLedger?: TaskLedgerStore; definitions?: readonly AgentDefinition[] } = {}, ): MakaTool< diff --git a/packages/runtime/src/tool-artifacts.ts b/packages/runtime/src/tool-artifacts.ts index 1303cb763b..83e5b6d81b 100644 --- a/packages/runtime/src/tool-artifacts.ts +++ b/packages/runtime/src/tool-artifacts.ts @@ -40,6 +40,8 @@ export function deriveToolArtifactCandidates( return deriveWriteArtifacts(args, result, input.cwd); case 'Edit': return deriveEditArtifacts(args); + case 'ApplyPatch': + return deriveApplyPatchArtifacts(result, input.cwd); case 'Bash': return deriveBashArtifacts(args, input.cwd); default: @@ -100,6 +102,35 @@ function deriveEditArtifacts(args: Record | null): ToolArtifact ]; } +function deriveApplyPatchArtifacts( + result: Record | null, + cwd: string, +): ToolArtifactCandidate[] { + const operations = Array.isArray(result?.operations) ? result.operations : []; + const candidates: ToolArtifactCandidate[] = []; + const seen = new Set(); + for (const operation of operations) { + if (!operation || typeof operation !== 'object' || Array.isArray(operation)) continue; + const record = operation as Record; + const completedMoveDestination = + record.operation === 'move' && record.status === 'failed' && typeof record.bytes === 'number'; + if (record.status !== 'completed' && !completedMoveDestination) continue; + const rawPath = typeof record.path === 'string' ? record.path : undefined; + if (!rawPath || seen.has(rawPath)) continue; + seen.add(rawPath); + const path = isAbsolute(rawPath) ? rawPath : resolve(cwd, rawPath); + candidates.push({ + kind: kindForPath(path), + name: basename(path), + mimeType: mimeForPath(path), + source: 'tool_result', + summary: 'ApplyPatch tool output', + sourcePath: path, + }); + } + return candidates; +} + function deriveBashArtifacts( args: Record | null, cwd: string, diff --git a/packages/runtime/src/tool-catalog-derive.ts b/packages/runtime/src/tool-catalog-derive.ts index 8820c26f35..48533c96ed 100644 --- a/packages/runtime/src/tool-catalog-derive.ts +++ b/packages/runtime/src/tool-catalog-derive.ts @@ -11,6 +11,7 @@ import { unknownBoundToolNames, type ToolHostId, } from '@maka/core/tool-catalog'; +import type { EditingProtocol } from '@maka/core/apply-patch'; import type { HostCapabilities } from './skills-context.js'; import type { ToolGroup } from './tool-availability.js'; import type { MakaTool } from './tool-runtime.js'; @@ -18,11 +19,18 @@ import type { MakaTool } from './tool-runtime.js'; export interface ProductToolSurfacePolicy { readonly economy: boolean; readonly disabledSurfaceIds?: Iterable; + /** + * Editing protocol projection (#1552). Default `edit_write` keeps Write/Edit + * and drops ApplyPatch; `apply_patch` does the inverse. Exactly one editing + * protocol remains on the effective surface. + */ + readonly editingProtocol?: EditingProtocol; } export interface NormalizedProductToolSurfacePolicy { readonly economy: boolean; readonly disabledSurfaceIds: readonly string[]; + readonly editingProtocol: EditingProtocol; } export interface ProductToolSurfaceIdentity { @@ -92,6 +100,7 @@ export function projectEffectiveProductToolSurface(input: { policy: ProductToolSurfacePolicy; }): EffectiveProductToolSurface { const disabledSurfaceIds = [...new Set(input.policy.disabledSurfaceIds ?? [])].sort(); + const editingProtocol: EditingProtocol = input.policy.editingProtocol ?? 'edit_write'; const excludedToolNames = new Set(); for (const surfaceId of disabledSurfaceIds) { const surface = catalogSurfaceById(surfaceId); @@ -102,6 +111,15 @@ export function projectEffectiveProductToolSurface(input: { if (surface.hosts[input.host] === 'supported') continue; for (const name of surface.toolNames) excludedToolNames.add(name); } + // Exactly one editing protocol on the effective surface (#1552). Builders + // bind the union; this normalized policy is the only selection point. + // policy defaults to edit_write — that double-filter left zero editing tools. + if (editingProtocol === 'apply_patch') { + excludedToolNames.add('Write'); + excludedToolNames.add('Edit'); + } else { + excludedToolNames.add('ApplyPatch'); + } const tools = input.tools.filter((tool) => !excludedToolNames.has(tool.name)); const boundToolNames = new Set(tools.map((tool) => tool.name)); const toolNames = readonlySetSnapshot(boundToolNames); @@ -116,6 +134,7 @@ export function projectEffectiveProductToolSurface(input: { const policy = Object.freeze({ economy: input.policy.economy, disabledSurfaceIds: Object.freeze(disabledSurfaceIds), + editingProtocol, }); return Object.freeze({ tools: Object.freeze(tools), diff --git a/packages/storage/src/__tests__/session-store.test.ts b/packages/storage/src/__tests__/session-store.test.ts index b902a51d91..a25436163e 100644 --- a/packages/storage/src/__tests__/session-store.test.ts +++ b/packages/storage/src/__tests__/session-store.test.ts @@ -99,6 +99,19 @@ describe('SQLite SessionStore', () => { await rm(root, { recursive: true, force: true }); } }); + + test('persists the editing protocol on the session header', async () => { + const root = await mkdtemp(join(tmpdir(), 'maka-session-editing-protocol-')); + const store = createSessionStore(root); + try { + const session = await store.create(makeInput({ editingProtocol: 'apply_patch' })); + assert.equal(session.editingProtocol, 'apply_patch'); + assert.equal((await store.readHeader(session.id)).editingProtocol, 'apply_patch'); + } finally { + await store.close?.(); + await rm(root, { recursive: true, force: true }); + } + }); }); function makeInput(overrides: Partial = {}): CreateSessionInput { diff --git a/packages/storage/src/__tests__/sqlite-runtime-store.test.ts b/packages/storage/src/__tests__/sqlite-runtime-store.test.ts index ae17789182..80a8547df2 100644 --- a/packages/storage/src/__tests__/sqlite-runtime-store.test.ts +++ b/packages/storage/src/__tests__/sqlite-runtime-store.test.ts @@ -4,15 +4,14 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { DatabaseSync } from 'node:sqlite'; import { describe, it } from 'node:test'; +import { canonicalToolArgsHash, type RuntimeEvent } from '@maka/core'; import { buildImmutableRuntimePrefix, - canonicalToolArgsHash, createRuntimeBoundaryCursor, runtimePrefixSegment, type ContinuationClaimV1, type ImmutableRuntimePrefixV1, - type RuntimeEvent, -} from '@maka/core'; +} from '@maka/core/runtime-boundary'; import { SQLITE_RUNTIME_SCHEMA_VERSION, createSqliteRuntimeStore, diff --git a/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts b/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts index 5f5938badf..4f16e78319 100644 --- a/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts +++ b/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts @@ -2596,6 +2596,7 @@ function fullHeader(overrides: Partial = {}): SessionHeader { model: 'gpt-5', thinkingLevel: 'high', permissionMode: 'ask', + editingProtocol: 'edit_write', collaborationMode: 'agent', orchestrationMode: 'swarm', schemaVersion: 1, diff --git a/packages/storage/src/session-store.ts b/packages/storage/src/session-store.ts index cd6e7eaf1b..a81cee4d1a 100644 --- a/packages/storage/src/session-store.ts +++ b/packages/storage/src/session-store.ts @@ -30,6 +30,7 @@ import { isSubagentSessionSpawn, isSubagentWorkspaceBinding, isSessionStatus, + isEditingProtocol, normalizeUserSessionName, subagentSessionRuntimeSummary, WORKSPACE_AUTHORITY_SESSION_ID, @@ -839,6 +840,7 @@ function buildSessionHeader( model: input.model ?? 'default', permissionMode: input.permissionMode, collaborationMode: input.collaborationMode ?? 'agent', + editingProtocol: input.editingProtocol ?? 'edit_write', orchestrationMode: input.orchestrationMode ?? 'default', ...(input.thinkingLevel !== undefined ? { thinkingLevel: input.thinkingLevel } : {}), schemaVersion: 1, @@ -892,6 +894,7 @@ export function normalizeSessionHeader( isPermissionMode(header.permissionMode) && isCollaborationMode(header.collaborationMode) && isOrchestrationMode(header.orchestrationMode) && + (header.editingProtocol === undefined || isEditingProtocol(header.editingProtocol)) && header.schemaVersion === 1; if (!valid) { throw new Error(`Invalid session header for session ${sessionId}: malformed fields`);