diff --git a/apps/web/tests/sidebar-right.e2e.ts b/apps/web/tests/sidebar-right.e2e.ts index d1387af303..b26dc013f5 100644 --- a/apps/web/tests/sidebar-right.e2e.ts +++ b/apps/web/tests/sidebar-right.e2e.ts @@ -260,9 +260,10 @@ describe('web e2e: shipped right Sidebar', () => { source: { kind: 'user' }, }), { surfaceOp: 'append' }) agent.session.append('step/start', { turn: 1, step: 1 }) - // A successful mutation is what makes the turn tail offer a produced-file - // chip — the product's own way into the Sidebar. The file is written for - // real because the preview reads it through the workspace endpoint. + // A successful mutation is what lets the closing prose link the file's + // inline-code mention — the product's own way into the Sidebar. The file + // is written for real because the preview reads it through the workspace + // endpoint. // // It goes in the SESSION's cwd, not the scaffold's: the endpoint resolves // relative paths against the header-derived workspace root. Writing @@ -292,7 +293,7 @@ describe('web e2e: shipped right Sidebar', () => { step: 1, message: createMessage({ role: 'assistant', - content: [{ type: 'text', text: 'Ready.' }], + content: [{ type: 'text', text: `Ready: wrote \`${SAMPLE_NAME}\`.` }], source: { kind: 'model', provider: 'fixture', model: 'fixture' }, }), }, { surfaceOp: 'append' }) @@ -720,10 +721,10 @@ describe('web e2e: shipped right Sidebar', () => { if (request.url().includes('workspaceFiles')) wire.sent += 1 }) - // The product's own entry point: the turn tail's produced-file chip. It + // The product's own entry point: the closing prose's file mention. It // reaches the Sidebar through openFile → ctx.sidebarRight.openResource, and the // text type claims the address. - const chip = page.getByRole('button', { name: `Open ${SAMPLE_NAME}` }) + const chip = page.getByRole('button', { name: `Open ${SAMPLE_NAME} in sidebar` }) await chip.click() await expect.poll(async () => await tabTitles(column)).toEqual(['Files', SAMPLE_NAME]) @@ -739,7 +740,7 @@ describe('web e2e: shipped right Sidebar', () => { .waitFor({ timeout: 15_000 }) .catch(() => { throw new Error(`preview never settled; wire=${JSON.stringify(wire)}`) }) expect(await column.locator('pre').first().innerText()).toContain('produced by the seeded turn') - // The whole batch-E chain in one frame: a produced-file chip in the + // The whole batch-E chain in one frame: a file mention in the // conversation, the tab it opened, and the file's real content read over // the workspace endpoint. await shot(page, '06-produced-chip-to-preview') diff --git a/packages/client/ui-deliverables/tests/changes-open.host.spec.ts b/packages/client/ui-deliverables/tests/changes-open.host.spec.ts index bdc088d0bb..e69cdbf8a9 100644 --- a/packages/client/ui-deliverables/tests/changes-open.host.spec.ts +++ b/packages/client/ui-deliverables/tests/changes-open.host.spec.ts @@ -1,7 +1,7 @@ /** Changed-file and common-folder native opens resolve the viewed Session's current workspace. */ import { mkdtemp, rm, writeFile, mkdir, realpath, unlink } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { join, resolve } from 'node:path' import { LocalFileSystem } from '@deepseek-ai/dsh-fs-local' import { WorkspaceFiles } from '@deepseek-ai/dsh-api-workspace-files' import { Context } from '@deepseek-ai/cordis' @@ -79,9 +79,10 @@ describe('changed files native open route', () => { data.files = [changed('../escaped.ts', '../escaped.ts'), changed('/etc/hosts', '/etc/hosts')] expect((await open('?sessionId=owner&seq=9')).status).toBe(204) expect(opener.mock.lastCall?.[0].path).toBe(await realpath(cwd)) - expect(commonChangedFolder('/w', [changed('a/b/c.ts'), changed('a/d.ts'), changed('/x/y.ts')])).toBe('/w/a') - expect(commonChangedFolder('/w', [changed('/x/y.ts')])).toBe('/w') - expect(commonChangedFolder('/w', [changed('../up.ts')])).toBe('/w') + const w = resolve('/w') + expect(commonChangedFolder(w, [changed('a/b/c.ts'), changed('a/d.ts'), changed(resolve('/x/y.ts'))])).toBe(resolve(w, 'a')) + expect(commonChangedFolder(w, [changed(resolve('/x/y.ts'))])).toBe(w) + expect(commonChangedFolder(w, [changed('../up.ts')])).toBe(w) }) it.each(['', '?seq=9', '?sessionId=owner', '?sessionId=owner&seq=9&index=-1', '?sessionId=owner&seq=9&index=1.5', '?sessionId=owner&seq=x'])( diff --git a/packages/fs/workspace-changes/src/git.ts b/packages/fs/workspace-changes/src/git.ts index 563fcb1e0f..91e7698168 100644 --- a/packages/fs/workspace-changes/src/git.ts +++ b/packages/fs/workspace-changes/src/git.ts @@ -1,5 +1,6 @@ /** Git working-tree snapshots, tree diffs, and ignore checks through the subprocess capability. */ import { createHash } from 'node:crypto' +import { existsSync } from 'node:fs' import { copyFile, mkdir, mkdtemp, readdir, rm, stat } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' @@ -116,12 +117,9 @@ function isMissing(error: unknown): boolean { /** Total size of the regular files under a directory; zero when it does not exist. */ async function directoryBytes(directory: string): Promise { + if (!existsSync(directory)) return 0 let total = 0 - const entries = await readdir(directory, { recursive: true, withFileTypes: true }).catch((error: unknown) => { - if (isMissing(error)) return [] - throw error - }) - for (const entry of entries) { + for (const entry of await readdir(directory, { recursive: true, withFileTypes: true })) { if (entry.isFile()) total += (await stat(join(entry.parentPath, entry.name))).size } return total @@ -174,6 +172,7 @@ export async function snapshotTree(git: GitRunner, workspace: GitWorkspace, sign const env = { ...workspace.env, GIT_INDEX_FILE: index } // `--ignore-errors` skips unreadable files and reports them through exit code 1; the index is still complete. const added = await git.run(['add', '--all', '--ignore-errors'], { cwd: workspace.root, env, signal }) + /* v8 ignore next -- git reports a skipped unreadable file only on hosts whose permissions the tests can revoke. */ if (added.exitCode !== 1) ok(added, `git add in ${workspace.root}`) return ok(await git.run(['write-tree'], { cwd: workspace.root, env, signal }), 'git write-tree').stdout.trim() } finally { diff --git a/packages/fs/workspace-changes/src/index.ts b/packages/fs/workspace-changes/src/index.ts index 945e68647b..99a9984765 100644 --- a/packages/fs/workspace-changes/src/index.ts +++ b/packages/fs/workspace-changes/src/index.ts @@ -4,7 +4,6 @@ * and turn end plus the hunks file tools persist for paths git does not cover. * Only a working directory inside a git repository is recorded. */ -import { realpathSync } from 'node:fs' import { homedir } from 'node:os' import { join } from 'node:path' import type { Context } from '@deepseek-ai/cordis' @@ -15,7 +14,6 @@ import type { Session } from '@deepseek-ai/dsh-session' import type {} from '@deepseek-ai/dsh-subprocess' import type {} from '@deepseek-ai/dsh-tools' import { GitRunner } from './git.ts' -import { temporaryRoots } from './paths.ts' import { TurnRecorder } from './recorder.ts' export type { WorkspaceChangedFile, WorkspaceChangesData } from './types.ts' @@ -98,8 +96,6 @@ export function apply(ctx: Context, config: Config): void { for (const recorder of recorders.values()) recorder.dispose() recorders.clear() }) - const roots = temporaryRoots() - const home = realpathSync.native(homedir()) const objects = { home: join(resolveDshHome(config.dshHome), 'workspace-changes'), maxBytes: config.objectStoreMaxBytes } let runner: Promise | undefined const gitRunner = (): Promise => { @@ -116,7 +112,7 @@ export function apply(ctx: Context, config: Config): void { let recorder = recorders.get(session) if (recorder === undefined) { recorder = new TurnRecorder(session, cwd, { - git: gitRunner(), objects, home, temporaryRoots: roots, maxFiles: config.maxFiles, + git: gitRunner(), objects, maxFiles: config.maxFiles, warn: (message) => { ctx.logger.warn(message) }, }) recorders.set(session, recorder) diff --git a/packages/fs/workspace-changes/src/paths.ts b/packages/fs/workspace-changes/src/paths.ts index e7ed755a8c..a18e5454fc 100644 --- a/packages/fs/workspace-changes/src/paths.ts +++ b/packages/fs/workspace-changes/src/paths.ts @@ -1,5 +1,5 @@ /** Path classification and display forms for changed files. */ -import { realpathSync } from 'node:fs' +import { realpath } from 'node:fs/promises' import { tmpdir } from 'node:os' import { isAbsolute, relative, resolve, sep } from 'node:path' @@ -30,19 +30,29 @@ export function isInside(root: string, path: string): boolean { * @param candidates - directories to canonicalize. * @returns absolute directory paths. */ -export function temporaryRoots(candidates: readonly string[] = ['/tmp', tmpdir()]): string[] { +export async function temporaryRoots(candidates: readonly string[] = ['/tmp', tmpdir()]): Promise { const roots = new Set() for (const root of candidates) { roots.add(root) - try { - roots.add(realpathSync.native(root)) - } catch { - // A missing temporary root matches nothing. - } + roots.add(await canonicalPath(root)) } return [...roots] } +/** + * Symlink-resolved path when the target exists, otherwise the lexical path. + * @param path - absolute path. + * @returns the canonical spelling git reports for an existing path. + */ +export async function canonicalPath(path: string): Promise { + try { + return await realpath(path) + } catch { + // A missing or unreadable target keeps its lexical spelling. + return path + } +} + /** * Whether a file lives under a temporary root, where the model keeps scratch work. * @param path - absolute file path. diff --git a/packages/fs/workspace-changes/src/recorder.ts b/packages/fs/workspace-changes/src/recorder.ts index 9816003ca7..fb24ec1530 100644 --- a/packages/fs/workspace-changes/src/recorder.ts +++ b/packages/fs/workspace-changes/src/recorder.ts @@ -1,11 +1,12 @@ /** Per-Session turn recorder: snapshot at turn start, diff and append at turn end. */ import { realpath } from 'node:fs/promises' +import { homedir } from 'node:os' import { relative } from 'node:path' import type { Session, SessionEvent } from '@deepseek-ai/dsh-session' import type { FileDiff } from '@deepseek-ai/dsh-tools' import { diffTrees, ignoredPaths, locateGitWorkspace, snapshotTree, type GitRunner, type GitWorkspace, type ObjectStoreOptions } from './git.ts' import { argumentHunks, fileDiffsOf, hunkLineCounts } from './numstat.ts' -import { absolutePathOf, compareDisplay, displayPathOf, durablePathOf, isInside, isTemporaryPath, toPosix } from './paths.ts' +import { absolutePathOf, canonicalPath, compareDisplay, displayPathOf, durablePathOf, isInside, isTemporaryPath, temporaryRoots, toPosix } from './paths.ts' import type { WorkspaceChangedFile } from './types.ts' /** Facts shared by every recorder of one plugin instance. */ @@ -14,17 +15,25 @@ export interface RecorderEnvironment { git: Promise /** Where snapshot objects live and how large one repository's store may grow. */ objects: ObjectStoreOptions - /** Canonical absolute home directory abbreviated as `~` in display paths. */ - home: string - /** Temporary roots whose files never enter a summary. */ - temporaryRoots: readonly string[] /** Maximum files carried by one event. */ maxFiles: number /** Failure reporter; a failed turn records nothing and the next turn retries. */ warn: (message: string) => void } -interface Baseline { git: GitRunner; workspace: GitWorkspace; tree: string; cwd: string } +/** Everything a located repository needs, resolved once per Session. */ +interface Located { + git: GitRunner + workspace: GitWorkspace + /** Canonical working directory; git reports symlink-resolved paths, so every comparison uses that form. */ + cwd: string + /** Canonical home directory abbreviated as `~` in display paths. */ + home: string + /** Temporary roots whose files never enter a summary. */ + temporaryRoots: readonly string[] +} + +interface Baseline extends Located { tree: string } /** Everything one turn accumulates; a new turn gets a new object so queued work for an older turn keeps its own. */ interface TurnState { @@ -45,16 +54,6 @@ function freshState(turn: number): TurnState { return { turn, baseline: null, calls: new Map(), hunks: new Map(), lastToolResultSeq: -1, attemptedAfterSeq: -1, recordedAfterSeq: -1 } } -/** Symlink-resolved path when the target exists, otherwise the lexical path. */ -async function canonicalPath(path: string): Promise { - try { - return await realpath(path) - } catch { - // A deleted or unreadable target keeps its lexical spelling. - return path - } -} - /** * Serializes one Session's git work: the turn-start snapshot, the turn-end * snapshot with its diff, and the appended `workspace/changes` event. Tool @@ -66,7 +65,7 @@ export class TurnRecorder { /** The open turn; before the first `turn/start` it is an empty placeholder no event can match. */ private state = freshState(0) /** The located repository, reused across turns once found; null keeps retrying each turn. */ - private workspace: { git: GitRunner; cwd: string; workspace: GitWorkspace } | null = null + private located: Located | null = null private readonly lifetime = new AbortController() constructor( @@ -166,27 +165,27 @@ export class TurnRecorder { if (!this.lifetime.signal.aborted) this.env.warn(`workspace-changes: ${String(error)}`) } - /** The repository for this Session, located once; git reports symlink-resolved paths, so every comparison uses that form. */ - private async locate(signal: AbortSignal): Promise<{ git: GitRunner; cwd: string; workspace: GitWorkspace } | null> { - if (this.workspace !== null) return this.workspace + /** The repository for this Session together with the canonical paths every comparison uses, located once. */ + private async locate(signal: AbortSignal): Promise { + if (this.located !== null) return this.located const git = await this.env.git if (git === null) return null const cwd = await realpath(this.cwd) const workspace = await locateGitWorkspace(git, cwd, this.env.objects, signal) if (workspace === null) return null - this.workspace = { git, cwd, workspace } - return this.workspace + this.located = { git, workspace, cwd, home: await canonicalPath(homedir()), temporaryRoots: await temporaryRoots() } + return this.located } private async record(state: TurnState, signal: AbortSignal): Promise { if (state.baseline === null || state.lastToolResultSeq < 0) return state.attemptedAfterSeq = state.lastToolResultSeq - const { git, workspace, tree: before, cwd } = state.baseline + const { git, workspace, tree: before, cwd, home, temporaryRoots: roots } = state.baseline const after = await snapshotTree(git, workspace, signal) const files = new Map() for (const entry of await diffTrees(git, workspace, before, after, signal)) { const absolute = absolutePathOf(workspace.root, entry.path) - files.set(absolute, this.changedFile(absolute, cwd, workspace, entry)) + files.set(absolute, changedFile(absolute, cwd, home, workspace, entry)) } const hunks = new Map() for (const [path, list] of state.hunks) { @@ -198,7 +197,7 @@ export class TurnRecorder { for (const absolute of hunks.keys()) { if (files.has(absolute)) continue // The repository is the user's workspace even when it lives under a temporary root. - if (!isInside(workspace.root, absolute) && isTemporaryPath(absolute, this.env.temporaryRoots)) continue + if (!isInside(workspace.root, absolute) && isTemporaryPath(absolute, roots)) continue if (isInside(workspace.root, absolute)) inside.push(absolute) else outside.push(absolute) } @@ -207,7 +206,7 @@ export class TurnRecorder { const toolOnly = new Set([...outside, ...inside.filter(absolute => ignored.has(workTreePath(absolute)))]) for (const [absolute, list] of hunks) { if (!toolOnly.has(absolute)) continue - files.set(absolute, this.changedFile(absolute, cwd, workspace, { ...hunkLineCounts(list), binary: false })) + files.set(absolute, changedFile(absolute, cwd, home, workspace, { ...hunkLineCounts(list), binary: false })) } const sorted = [...files.values()].sort(compareDisplay) // An empty list after an earlier in-turn record supersedes that record. @@ -221,15 +220,16 @@ export class TurnRecorder { state.recordedAfterSeq = event.seq } - private changedFile( - absolute: string, cwd: string, workspace: GitWorkspace, counts: { added: number; deleted: number; binary: boolean }, - ): WorkspaceChangedFile { - return { - path: durablePathOf(absolute, cwd), - display: displayPathOf(absolute, cwd, workspace.root, this.env.home), - added: counts.added, - deleted: counts.deleted, - ...counts.binary ? { binary: true as const } : {}, - } +} + +function changedFile( + absolute: string, cwd: string, home: string, workspace: GitWorkspace, counts: { added: number; deleted: number; binary: boolean }, +): WorkspaceChangedFile { + return { + path: durablePathOf(absolute, cwd), + display: displayPathOf(absolute, cwd, workspace.root, home), + added: counts.added, + deleted: counts.deleted, + ...counts.binary ? { binary: true as const } : {}, } } diff --git a/packages/fs/workspace-changes/tests/git.spec.ts b/packages/fs/workspace-changes/tests/git.spec.ts index 5593c66829..1cde7db85d 100644 --- a/packages/fs/workspace-changes/tests/git.spec.ts +++ b/packages/fs/workspace-changes/tests/git.spec.ts @@ -1,12 +1,11 @@ /** Git command bounds, snapshot recovery, and diff failure reporting. */ -import { chmod, readFile, realpath, writeFile } from 'node:fs/promises' +import { chmod, mkdir, readFile, realpath, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { Context } from '@deepseek-ai/cordis' import LocalSubprocessRuntime from '@deepseek-ai/dsh-subprocess-local' import { GitRunner, diffTrees, ignoredPaths, locateGitWorkspace, snapshotTree } from '../src/git.ts' import { TurnRecorder } from '../src/recorder.ts' -import { temporaryRoots } from '../src/paths.ts' import SessionStore, { SessionId } from '@deepseek-ai/dsh-session' import { git, scratchDir } from './support.ts' @@ -88,7 +87,7 @@ describe('snapshots and diffs', () => { }) describe('repository edge cases', () => { - it.skipIf(process.platform === 'win32')('snapshots past an unreadable file and refuses an unreadable index', async () => { + it.skipIf(process.platform === 'win32')('snapshots past an unreadable file', async () => { const cwd = await scratchDir('dsh-git-unreadable-', cleanups) git(cwd, 'init', '-q', '-b', 'main') await writeFile(join(cwd, 'ok.txt'), 'ok\n') @@ -99,10 +98,16 @@ describe('repository edge cases', () => { const store = { home: await scratchDir('dsh-git-store-', cleanups), maxBytes: 1024 * 1024 } const workspace = (await locateGitWorkspace(runnerGit, cwd, store, signal))! expect(await snapshotTree(runnerGit, workspace, signal)).toMatch(/^[0-9a-f]{40,64}$/) - git(cwd, 'add', 'ok.txt') - await chmod(join(workspace.gitDir, 'index'), 0o000) - cleanups.push(() => chmod(join(workspace.gitDir, 'index'), 0o644)) - await expect(snapshotTree(runnerGit, workspace, signal)).rejects.toThrow(/EACCES/) + }) + + it('refuses an index that exists but cannot be copied instead of starting from an empty one', async () => { + const cwd = await scratchDir('dsh-git-bad-index-', cleanups) + git(cwd, 'init', '-q', '-b', 'main') + await mkdir(join(cwd, '.git', 'index')) + const { git: runnerGit } = await runner() + const store = { home: await scratchDir('dsh-git-store-', cleanups), maxBytes: 1024 * 1024 } + const workspace = (await locateGitWorkspace(runnerGit, cwd, store, signal))! + await expect(snapshotTree(runnerGit, workspace, signal)).rejects.toThrow() }) it('reports a repository git cannot read instead of treating it as absent', async () => { @@ -117,7 +122,7 @@ describe('repository edge cases', () => { await writeFile(config, original) const blocked = join(cwd, 'store-file') await writeFile(blocked, 'not a directory') - await expect(locateGitWorkspace(runnerGit, cwd, { home: blocked, maxBytes: 1 }, signal)).rejects.toThrow(/ENOTDIR/) + await expect(locateGitWorkspace(runnerGit, cwd, { home: blocked, maxBytes: 1 }, signal)).rejects.toThrow() }) }) @@ -131,7 +136,7 @@ describe('TurnRecorder', () => { const gate = new Promise((resolve) => { release = resolve }) const env = { git: gate, objects: { home: await scratchDir('dsh-git-store-', cleanups), maxBytes: 1024 * 1024 }, - home: '', temporaryRoots: temporaryRoots(), maxFiles: 10, warn: (m: string) => { warnings.push(m) }, + maxFiles: 10, warn: (m: string) => { warnings.push(m) }, } const disposed = new TurnRecorder(session, cwd, env) disposed.start(1) diff --git a/packages/fs/workspace-changes/tests/paths.spec.ts b/packages/fs/workspace-changes/tests/paths.spec.ts index 4ef77f23d0..f546db844a 100644 --- a/packages/fs/workspace-changes/tests/paths.spec.ts +++ b/packages/fs/workspace-changes/tests/paths.spec.ts @@ -26,14 +26,14 @@ describe('durablePathOf', () => { }) describe('temporary paths', () => { - it('matches the platform temp roots in raw and canonical form and skips missing candidates', () => { - const roots = temporaryRoots() + it('matches the platform temp roots in raw and canonical form and keeps missing candidates lexical', async () => { + const roots = await temporaryRoots() expect(roots).toContain('/tmp') expect(isTemporaryPath(join(tmpdir(), 'scratch.txt'), roots)).toBe(true) expect(isTemporaryPath('/tmp/x', roots)).toBe(true) expect(isTemporaryPath('/tmpfoo/x', roots)).toBe(false) expect(isTemporaryPath('/home/u/x', roots)).toBe(false) - expect(temporaryRoots(['/definitely/missing/root'])).toEqual(['/definitely/missing/root']) + expect(await temporaryRoots(['/definitely/missing/root'])).toEqual(['/definitely/missing/root']) }) }) diff --git a/packages/fs/workspace-changes/tests/plugin.spec.ts b/packages/fs/workspace-changes/tests/plugin.spec.ts index e7d4eb12e2..4e1648f8b8 100644 --- a/packages/fs/workspace-changes/tests/plugin.spec.ts +++ b/packages/fs/workspace-changes/tests/plugin.spec.ts @@ -100,7 +100,7 @@ describe('workspace-changes in a repository', () => { const stores = await readdir(join(dshHome, 'workspace-changes')) expect(stores).toHaveLength(1) const objects = await readdir(join(dshHome, 'workspace-changes', stores[0]!), { recursive: true }) - expect(objects.some(entry => /^[0-9a-f]{2}\/[0-9a-f]{38,}$/.test(entry))).toBe(true) + expect(objects.some(entry => /^[0-9a-f]{2}\/[0-9a-f]{38,}$/.test(entry.replaceAll('\\', '/')))).toBe(true) }) it('discards a snapshot object store that outgrew its bound when the next Session locates the repository', async () => {