diff --git a/docs/context.md b/docs/context.md index 00abf2f00..aa22ef501 100644 --- a/docs/context.md +++ b/docs/context.md @@ -321,8 +321,14 @@ above 16 MiB is not copied: the snapshot keeps only its metadata, so a change by the removed turns makes the whole restore unavailable and nothing is restored; an unchanged one does not get in the way. Old turns without backups, overlapping turns in the same workspace, a cut inside a running turn and -incomplete snapshots cannot restore code. The rewind -answers `files: { status, count, reason? }`; `status` is `restored`, `unchanged` +incomplete snapshots cannot restore code. Two folders overlap when one holds +the other and the outer snapshot would list a file of the inner one: git +lists a repository or worktree of its own (`.boite/worktrees/*`) as one +directory entry, unless the outer index tracks its files; a plain walk lists +all but the skipped folders. An edit made while another turn still works in an +overlapping folder, such as a sub-agent winding down after a Stop, rewinds the +messages and leaves every file as it is, with `busy: true`. The rewind +answers `files: { status, count, reason?, busy? }`; `status` is `restored`, `unchanged` or `unavailable`. The client shows a warning for unavailable backups rather than claiming that the code was restored. diff --git a/packages/contracts/src/threads.ts b/packages/contracts/src/threads.ts index 6b2616078..55d77c645 100644 --- a/packages/contracts/src/threads.ts +++ b/packages/contracts/src/threads.ts @@ -484,8 +484,12 @@ export interface ThreadRewind { * carrying the kept history, as a change of account does. */ session: 'native' | 'seeded'; - /** File changes restored before the cut. Missing on older cores. */ - files?: { status: 'restored' | 'unchanged' | 'unavailable'; count: number; reason?: string }; + /** + * File changes restored before the cut. Missing on older cores. `busy`: + * another turn was working in the same folder, so the files were left as + * they are and only the messages were rewound. + */ + files?: { status: 'restored' | 'unchanged' | 'unavailable'; count: number; reason?: string; busy?: true }; } export interface ThreadsRpcMethods { diff --git a/packages/core/src/threads/checkpoint-files.ts b/packages/core/src/threads/checkpoint-files.ts index 0bdbaf997..4a8bc27e9 100644 --- a/packages/core/src/threads/checkpoint-files.ts +++ b/packages/core/src/threads/checkpoint-files.ts @@ -33,6 +33,28 @@ export function contains(root: string, path: string): boolean { return child === '' || (!isAbsolute(child) && child !== '..' && !child.startsWith(`..${sep}`)); } +/** + * Whether turns in these two folders can change the same snapshot file. A + * folder inside another shares its files when the outer snapshot would list + * one of them: git decides what it lists (a repository or worktree of its own + * shows as one directory entry, files the outer index tracks still count), and + * a plain walk lists all but the skipped folders. + */ +export async function overlap(core: Core, threadId: string, a: string, b: string): Promise { + if (contains(a, b)) return listsAny(core, threadId, resolve(a), resolve(b)); + if (contains(b, a)) return listsAny(core, threadId, resolve(b), resolve(a)); + return false; +} + +async function listsAny(core: Core, threadId: string, root: string, path: string): Promise { + const inner = relative(root, path); + if (!inner) return true; + const listed = await git(core, threadId, root, ['ls-files', '-z', '--cached', '--others', '--exclude-standard', '--', inner.split(sep).join('/')], 5000); + // Same fallback as `snapshot`: without git, every folder but the skipped ones is walked. + if (listed.code !== 0) return !inner.split(sep).some(part => SKIP.has(part)); + return listed.stdout.split('\0').some(name => name && !name.endsWith('/')); +} + export function same(a: FileEntry | undefined, b: FileEntry | undefined): boolean { return a?.hash === b?.hash && a?.mode === b?.mode && a?.link === b?.link && a?.unbacked === b?.unbacked; } diff --git a/packages/core/src/threads/code-checkpoints.ts b/packages/core/src/threads/code-checkpoints.ts index d20b5c00c..ba70bf834 100644 --- a/packages/core/src/threads/code-checkpoints.ts +++ b/packages/core/src/threads/code-checkpoints.ts @@ -3,7 +3,7 @@ import { basename, dirname, join } from 'node:path'; import type { ThreadRewind, ThreadSummary } from '@boite/contracts'; import type { Core } from '../core.ts'; import { messageOf, refused } from '../errors.ts'; -import { collectSharedBlobs, contains, restoreFiles, same, snapshot, workspaceRoot, type FileChange, type FileSnapshot, type SnapshotCache } from './checkpoint-files.ts'; +import { collectSharedBlobs, contains, overlap, restoreFiles, same, snapshot, workspaceRoot, type FileChange, type FileSnapshot, type SnapshotCache } from './checkpoint-files.ts'; import { decodeManifest, encodeManifest, StoreMigration, trimFolder } from './checkpoint-store.ts'; type FileResult = NonNullable; @@ -119,13 +119,13 @@ export class CodeCheckpoints { try { root = await workspaceRoot(thread.cwd); } catch { return; } const checkpoint: Checkpoint = { root, messages: [...this.core.journal.walkTurnMessages(thread.id, turnId, { blobs: false })].filter(message => message.role === 'user').map(message => message.id) }; this.assertAvailable(root); - for (const other of this.active.values()) { - if (contains(root, other.checkpoint.root) || contains(other.checkpoint.root, root)) { - checkpoint.reason = other.checkpoint.reason = 'turns ran concurrently in the same workspace'; - } - } + // Registered first: a turn that starts while this one compares sees it. + const others = [...this.active.values()]; this.active.set(turnId, { threadId: thread.id, turnId, checkpoint }); try { + for (const other of others) { + if (await overlap(this.core, thread.id, root, other.checkpoint.root)) checkpoint.reason = other.checkpoint.reason = 'turns ran concurrently in the same workspace'; + } this.pruned ??= this.prune(); await this.pruned; await this.trims.get(thread.id); @@ -168,11 +168,18 @@ export class CodeCheckpoints { try { root = await workspaceRoot(thread.cwd); } catch (error) { return cut({ status: 'unavailable', count: 0, reason: `the working directory is unavailable: ${messageOf(error)}` }); } this.assertAvailable(root); - if ([...this.active.values()].some(active => contains(root, active.checkpoint.root) || contains(active.checkpoint.root, root))) { - throw refused('another turn is working in this project; stop it before restoring files', { field: 'cwd', path: root, reason: 'turn-in-flight' }); - } + // Held before comparing, so no turn starts here while the files are restored. this.restoring.add(root); try { + // A turn still working here (a sub-agent winding down after a Stop, a + // second conversation) ran beside the removed ones, so their checkpoint + // is not exact, and restoring would write under that agent. The edit + // still goes through; only the files stay as they are. + for (const active of [...this.active.values()]) { + if (await overlap(this.core, thread.id, root, active.checkpoint.root)) { + return cut({ status: 'unavailable', count: 0, reason: 'another turn is working in this folder; files were left as they are', busy: true }); + } + } const { files, changes } = await this.restore(thread, root, turnIds, messageId); try { return cut(files); } catch (error) { diff --git a/packages/core/test/rewind-files.test.ts b/packages/core/test/rewind-files.test.ts index 463c83b63..f28812af0 100644 --- a/packages/core/test/rewind-files.test.ts +++ b/packages/core/test/rewind-files.test.ts @@ -5,7 +5,7 @@ import { afterEach, beforeEach, expect, spyOn, test } from 'bun:test'; import type { CoreClient } from '../src/client.ts'; import { setDriver } from '../src/drivers/index.ts'; import { CodeCheckpoints, IDLE_THREAD_MS, KEEP_TURNS } from '../src/threads/code-checkpoints.ts'; -import { restoreFiles, sharedBlob, snapshot } from '../src/threads/checkpoint-files.ts'; +import { overlap, restoreFiles, sharedBlob, snapshot } from '../src/threads/checkpoint-files.ts'; import { BLOB_BYTE_LIMIT, decodeBlob, decodeManifest } from '../src/threads/checkpoint-store.ts'; import { git } from '../src/git/read.ts'; import { startTestCore, waitFor, type TestCore } from './harness.ts'; @@ -48,6 +48,16 @@ async function run(prompt: string): Promise { return h.core.threads.get(threadId).messages.find(message => message.turnId === turn.id && message.role === 'user')!.id; } +/** The fixture must not depend on the runner having a configured Git identity. */ +function commitFixture(folder = cwd): void { + const committed = Bun.spawnSync({ + cmd: ['git', 'commit', '--quiet', '-m', 'fixture'], cwd: folder, + env: { ...process.env, GIT_AUTHOR_NAME: 'boite test', GIT_AUTHOR_EMAIL: 'test@boite.invalid', GIT_COMMITTER_NAME: 'boite test', GIT_COMMITTER_EMAIL: 'test@boite.invalid' }, + stdout: 'pipe', stderr: 'pipe', windowsHide: true, + }); + if (!committed.success) throw new Error(committed.stderr.toString()); +} + test('editing restores changed, deleted, created and binary files, then runs the replacement from that state', async () => { await writeFile(join(cwd, 'app.ts'), 'initial'); await writeFile(join(cwd, 'image.bin'), new Uint8Array([0, 255, 2])); @@ -91,13 +101,7 @@ test('git checkpoints restore the dirty starting state without changing HEAD, th await writeFile(join(cwd, '.gitignore'), 'ignored.txt\n'); await writeFile(join(cwd, 'app.ts'), 'staged'); expect((await git(h.core, threadId, cwd, ['add', '.'])).code).toBe(0); - // The fixture must not depend on the runner having a configured Git identity. - const committed = Bun.spawnSync({ - cmd: ['git', 'commit', '--quiet', '-m', 'fixture'], cwd, - env: { ...process.env, GIT_AUTHOR_NAME: 'boite test', GIT_AUTHOR_EMAIL: 'test@boite.invalid', GIT_COMMITTER_NAME: 'boite test', GIT_COMMITTER_EMAIL: 'test@boite.invalid' }, - stdout: 'pipe', stderr: 'pipe', windowsHide: true, - }); - if (!committed.success) throw new Error(committed.stderr.toString()); + commitFixture(); await writeFile(join(cwd, 'app.ts'), 'dirty staged'); expect((await git(h.core, threadId, cwd, ['add', 'app.ts'])).code).toBe(0); await writeFile(join(cwd, 'app.ts'), 'dirty unstaged'); @@ -205,7 +209,7 @@ test('a failure while cutting messages rolls the restored files back', async () expect(h.core.threads.get(threadId).messages.filter(message => message.role === 'user')).toHaveLength(1); }); -test('overlapping turns cannot restore each other\'s files, and an active workspace refuses a rewind', async () => { +test('overlapping turns cannot restore each other\'s files, and an edit beside a working turn leaves the files alone', async () => { await writeFile(join(cwd, 'app.ts'), 'initial'); const thread = h.core.threads.require(threadId); const other = await client.call('threads.create', { projectId: thread.projectId!, providerId: 'echo', accountId: thread.accountId }); @@ -215,12 +219,16 @@ test('overlapping turns cannot restore each other\'s files, and an active worksp if (prompt.includes('held')) { entered = true; await held.promise; } else await writeFile(join(cwd, 'app.ts'), 'parallel'); }; - const first = await run('initial'); + await run('initial'); + const second = await run('second'); const pending = await client.call('turns.start', { threadId: other.id, prompt: 'held' }); try { await waitFor(() => entered); - await expect(client.call('threads.rewind', { threadId, messageId: first })).rejects.toThrow('another turn'); - expect(h.core.threads.get(threadId).messages.some(message => message.id === first)).toBe(true); + // A sub-agent still stopping, or another conversation, works in this folder. + const beside = await client.call('threads.rewind', { threadId, messageId: second }); + expect(beside.files).toMatchObject({ status: 'unavailable', busy: true }); + expect(h.core.threads.get(threadId).messages.some(message => message.id === second)).toBe(false); + expect(await readFile(join(cwd, 'app.ts'), 'utf8')).toBe('parallel'); const overlap = await run('parallel'); held.resolve(); await waitFor(() => h.core.journal.getTurn(pending.id)?.finishedAt != null); @@ -231,6 +239,42 @@ test('overlapping turns cannot restore each other\'s files, and an active worksp } finally { held.resolve(); } }); +test('a turn in a worktree nested in the project neither blocks nor spoils a rewind of the main checkout', async () => { + expect((await git(h.core, threadId, cwd, ['init', '--quiet'])).code).toBe(0); + await writeFile(join(cwd, '.gitignore'), '.boite/\n'); + await writeFile(join(cwd, 'app.ts'), 'initial'); + expect((await git(h.core, threadId, cwd, ['add', '.'])).code).toBe(0); + commitFixture(); + const nested = join(cwd, '.boite', 'worktrees', 'wt-child'); + expect((await git(h.core, threadId, cwd, ['worktree', 'add', '--quiet', '-b', 'child', nested])).code).toBe(0); + const thread = h.core.threads.require(threadId); + const child = await client.call('threads.create', { projectId: thread.projectId!, providerId: 'echo', accountId: thread.accountId }); + h.core.journal.putThread({ ...h.core.threads.require(child.id), cwd: nested }); + const held = Promise.withResolvers(); + let entered = false; + change = async prompt => { + if (prompt.includes('held')) { entered = true; await held.promise; } + else await writeFile(join(cwd, 'app.ts'), prompt); + }; + await run('kept'); + const pending = await client.call('turns.start', { threadId: child.id, prompt: 'held' }); + try { + await waitFor(() => entered); + const edited = await run('removed'); + const rewound = await client.call('threads.rewind', { threadId, messageId: edited }); + expect(rewound.files).toEqual({ status: 'restored', count: 1 }); + expect(await readFile(join(cwd, 'app.ts'), 'utf8')).toBe('kept'); + } finally { held.resolve(); } + await waitFor(() => h.core.journal.getTurn(pending.id)?.finishedAt != null); + // A repository of its own whose files the outer index still tracks shares them. + await mkdir(join(cwd, 'vendor')); + await writeFile(join(cwd, 'vendor', 'lib.ts'), 'tracked'); + expect((await git(h.core, threadId, cwd, ['add', 'vendor'])).code).toBe(0); + expect((await git(h.core, threadId, join(cwd, 'vendor'), ['init', '--quiet'])).code).toBe(0); + expect(await overlap(h.core, threadId, cwd, join(cwd, 'vendor'))).toBe(true); + expect(await overlap(h.core, threadId, nested, cwd)).toBe(false); +}); + test('permanently removing a project removes its private file checkpoints', async () => { const message = await run('save'); expect(message).toBeTruthy(); diff --git a/packages/ui/src/lib/store-rewind.test.ts b/packages/ui/src/lib/store-rewind.test.ts index 467326ba0..59b56a470 100644 --- a/packages/ui/src/lib/store-rewind.test.ts +++ b/packages/ui/src/lib/store-rewind.test.ts @@ -2,6 +2,7 @@ import { expect, vi } from 'vitest'; import { test } from '../test/fake-client'; import { RpcErrorCode } from '@boite/contracts'; import type { FakeClient } from './fake-client'; +import { strings } from './strings'; /** Two finished turns on the open thread, and the ids of their user messages. */ async function twoTurns(client: FakeClient, threadId: string): Promise<[string, string]> { @@ -55,6 +56,20 @@ test('a rewind without file backups visibly explains that the code could not be expect(store.error).toContain('code'); }); +test('a rewind beside another working agent says the files were left as they are', async ({ ready }) => { + const { store, client } = await ready(); + const [, edited] = await twoTurns(client, 't-trace'); + await store.open('t-trace'); + const call = client.call.bind(client); + vi.spyOn(client, 'call').mockImplementation(async (method, params) => { + const result = await call(method, params); + if (method === 'threads.rewind') return { ...result as object, files: { status: 'unavailable', count: 0, busy: true } } as typeof result; + return result; + }); + await store.rewind(edited); + expect(store.error).toBe(strings.composer.filesBusy); +}); + test('the fake forks a copy that leaves the source whole', async ({ createClient }) => { const client = await createClient({ delayMs: 0 }); const [kept] = await twoTurns(client, 't-trace'); diff --git a/packages/ui/src/lib/store/threads.svelte.ts b/packages/ui/src/lib/store/threads.svelte.ts index 1bda2dbed..7963354ce 100644 --- a/packages/ui/src/lib/store/threads.svelte.ts +++ b/packages/ui/src/lib/store/threads.svelte.ts @@ -169,7 +169,7 @@ export class Threads { const { thread, ...rewound } = await client.call('threads.rewind', { threadId, messageId }); if (!this.ctx.currentClient(client, clientGeneration)) return null; this.applyRewound(thread); - if (rewound.files?.status === 'unavailable') this.ctx.store.error = strings.composer.filesUnavailable; + if (rewound.files?.status === 'unavailable') this.ctx.store.error = rewound.files.busy ? strings.composer.filesBusy : strings.composer.filesUnavailable; return rewound; } catch (error) { if (this.ctx.currentClient(client, clientGeneration)) this.ctx.fail(error); diff --git a/packages/ui/src/lib/strings/en/composer.ts b/packages/ui/src/lib/strings/en/composer.ts index a7fbdae95..fcff6725e 100644 --- a/packages/ui/src/lib/strings/en/composer.ts +++ b/packages/ui/src/lib/strings/en/composer.ts @@ -5,6 +5,7 @@ export const strings = { editing: 'Editing a sent message: sending replaces it and everything after it', editingQueued: 'Send or remove the queued messages before replacing a sent message.', filesUnavailable: 'The messages were rewound, but this edit has no complete file backup. The code could not be restored.', + filesBusy: 'The messages were rewound. Another agent is still working in this folder, so the files were left as they are.', editingCancel: 'Stop editing (Esc)', /** A question waits for a written answer: the box writes it, the row above names the question. */ replying: 'Answering: {question}', diff --git a/packages/ui/src/lib/strings/fr/composer.ts b/packages/ui/src/lib/strings/fr/composer.ts index d6402b980..7e4602779 100644 --- a/packages/ui/src/lib/strings/fr/composer.ts +++ b/packages/ui/src/lib/strings/fr/composer.ts @@ -6,6 +6,7 @@ export const fr: Translation = { editing: 'Modification d’un message envoyé : l’envoi le remplace, avec tout ce qui suit', editingQueued: 'Envoyez ou retirez les messages en attente avant de remplacer un message envoyé.', filesUnavailable: "Les messages ont été rembobinés, mais cette modification n'a pas de sauvegarde complète des fichiers. Le code n'a pas pu être restauré.", + filesBusy: "Les messages ont été rembobinés. Un autre agent travaille encore dans ce dossier, les fichiers sont donc restés tels quels.", editingCancel: 'Arrêter la modification (Échap)', replying: 'Réponse à : {question}', replyPlaceholder: 'Écrivez votre réponse',