Просмотр исходного кода

fix(workspace-changes): resolve canonical paths lazily and make the tests portable

The packed worker deployment offers no realpathSync.native at plugin load, so
the home and temporary roots are now canonicalized asynchronously when a
Session first locates its repository. Windows-neutral tests replace chmod
and POSIX-only path expectations, and the right-Sidebar scenario enters
through the closing prose's file mention.
creatixchu 2 недель назад
Родитель
Сommit
86a1bf1e80

+ 8 - 7
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')

+ 5 - 4
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'])(

+ 4 - 5
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<number> {
+  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 {

+ 1 - 5
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<GitRunner | null> | undefined
   const gitRunner = (): Promise<GitRunner | null> => {
@@ -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)

+ 17 - 7
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<string[]> {
   const roots = new Set<string>()
   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<string> {
+  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.

+ 36 - 36
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<GitRunner | null>
   /** 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<string> {
-  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<Located | null> {
+    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<void> {
     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<string, WorkspaceChangedFile>()
     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<string, FileDiff[]>()
     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 } : {},
   }
 }

+ 14 - 9
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<GitRunner | null>((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)

+ 3 - 3
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'])
   })
 })
 

+ 1 - 1
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 () => {