fix(fs): resolve paths against the caller's session cwd
The ACP bridge gives each session its own workspace (SessionHeader.cwd), and
dsh-tool-bash already resolves a bash workdir against it. But ctx.fs.resolve(path)
took no caller context and dsh-fs-local resolved every relative path against a
fixed config.cwd (process.cwd() at plugin load) — so in the ACP demo `write
foo.txt` and `bash cat foo.txt` hit different directories the moment an editor
opens any project other than the server's launch dir.
Thread the session cwd into resolution, mirroring dsh-tool-bash: widen
FileSystem.resolve to resolve(path, opts?: { cwd?: string }); dsh-fs-local bases
a relative path on opts.cwd ?? config.cwd (absolute paths ignore it); the
read/write/edit tools derive it via a shared sessionCwd(exec) helper
(exec.agent?.session.header.cwd). The provider stays free of dsh-agent/dsh-session
— the tool projects exec → cwd and hands over a plain string, per the
explicit-at-seams convention. Backward compatible (the arg is optional).
Tests: fs-local resolve(path,{cwd}) bases relative on the passed cwd / ignores it
for absolute; tool integration writes/reads/edits in a session cwd != config.cwd
and verifies the file on disk (proven to fail on the pre-fix no-cwd path). Fakes
that stood in a bare {session:{}} now carry a header so sessionCwd doesn't throw.
RFC in docs/rfc/implemented/architecture/2026-07-02-fs-per-session-cwd.md.
This commit is contained in:
@@ -12,7 +12,7 @@ await ctx.plugin(LocalFileSystem, { cwd: process.cwd() })
|
||||
|
||||
## Behavior
|
||||
|
||||
- **`resolve(path)`** — relative paths resolve from `config.cwd` (default `process.cwd()`). The `targetKey` is the file's `realpath`, so two input paths reaching the same file through symlinks share one identity, and writes/edits land on the link target (preserving the link). A not-yet-existing path uses the realpathed parent directory plus basename when the parent exists; only an unresolvable parent falls back to the absolute path. `displayPath` is the absolute (un-resolved) path.
|
||||
- **`resolve(path, opts?)`** — a relative `path` resolves against `opts.cwd` when the caller supplies one (the model-facing tools pass the calling agent's session cwd — see [the per-session cwd RFC](../../../docs/rfc/implemented/architecture/2026-07-02-fs-per-session-cwd.md)), else `config.cwd` (default `process.cwd()`); an absolute `path` ignores both. The `targetKey` is the file's `realpath`, so two input paths reaching the same file through symlinks share one identity, and writes/edits land on the link target (preserving the link). A not-yet-existing path uses the realpathed parent directory plus basename when the parent exists; only an unresolvable parent falls back to the absolute path. `displayPath` is the absolute (un-resolved) path.
|
||||
- **`stat`** — returns `FsInfo` (`version` = `mtimeMs:size`, `type` of `file`/`directory`/`other`, byte `size`) or `undefined` when the target is absent.
|
||||
- **`readText` / `streamText`** — UTF-8 only. `readText` reads the whole file; `streamText` streams it in chunks (cross-chunk decoding) so a huge file never has to be held whole in memory. Both reject invalid UTF-8 and NUL-byte binary samples (`FS_NOT_TEXT`) and non-regular targets. The `read` tool (`@deepseek-ai/dsh-tool-fs`) decides which to call by size and owns the line windowing.
|
||||
- **`writeText`** — atomic: writes to a temp file opened exclusively (`wx`, `0o600`) inside a randomly-named private staging dir (`0o700`) next to the target, fsyncs, then renames over the target. An existing file's mode is preserved, while new files default to `0o600`. The `expected` guard is OPTIONAL: omitting it unconditionally creates-or-overwrites; `createIfAbsent` creates a missing target and rejects an existing one (`FS_NOT_OBSERVED`); `replaceIfVersion` replaces only at the observed version (a missing target or mismatch is `FS_STALE_VERSION`).
|
||||
|
||||
@@ -97,8 +97,8 @@ export class LocalFileSystem extends FileSystem {
|
||||
}
|
||||
}
|
||||
|
||||
override async resolve(path: string): Promise<FsTarget> {
|
||||
const local = await resolveLocalTarget(this.config.cwd, path)
|
||||
override async resolve(path: string, opts?: { cwd?: string }): Promise<FsTarget> {
|
||||
const local = await resolveLocalTarget(opts?.cwd ?? this.config.cwd, path)
|
||||
return { inputPath: path, targetKey: local.targetKey, displayPath: local.displayPath }
|
||||
}
|
||||
|
||||
|
||||
@@ -51,6 +51,30 @@ describe('registration', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolve', () => {
|
||||
it('resolves a relative path against opts.cwd, not config.cwd', async () => {
|
||||
// config.cwd is `dir`; a call supplying a DIFFERENT cwd bases the relative
|
||||
// path there (the per-session-workspace seam — mirrors tool-bash workdir).
|
||||
const other = await mkdtemp(join(tmpdir(), 'dsh-fs-other-'))
|
||||
try {
|
||||
await writeFile(join(other, 'x.txt'), 'in other')
|
||||
const viaOther = await fs.resolve('x.txt', { cwd: other })
|
||||
expect(await fs.readText(viaOther)).toBe('in other')
|
||||
// Same relative path with no opts falls back to config.cwd (= dir), where
|
||||
// x.txt does not exist.
|
||||
await expect(fs.readText(await fs.resolve('x.txt'))).rejects.toMatchObject({ code: 'FS_NOT_FOUND' })
|
||||
} finally {
|
||||
await rm(other, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('ignores opts.cwd for an ABSOLUTE path', async () => {
|
||||
await writeFile(join(dir, 'abs.txt'), 'absolute')
|
||||
const target = await fs.resolve(join(dir, 'abs.txt'), { cwd: '/nonexistent-base' })
|
||||
expect(await fs.readText(target)).toBe('absolute')
|
||||
})
|
||||
})
|
||||
|
||||
describe('stat', () => {
|
||||
it('returns file metadata, directory type, and undefined for absent', async () => {
|
||||
await writeFile(join(dir, 'a.txt'), 'hello')
|
||||
|
||||
@@ -19,7 +19,7 @@ A backend subclasses `FileSystem` and implements six primitives.
|
||||
|
||||
| Member | Semantics |
|
||||
|---|---|
|
||||
| `resolve(path)` | Resolve a path into a stable `FsTarget` (`inputPath`, opaque `targetKey`, `displayPath`). Async — a remote backend may need I/O. The same file via different paths must yield the same `targetKey`. |
|
||||
| `resolve(path, opts?)` | Resolve a path into a stable `FsTarget` (`inputPath`, opaque `targetKey`, `displayPath`). `opts.cwd` is the base a relative `path` resolves against (a caller supplies its session workspace; absolute paths ignore it; omitted ⇒ the backend default). Async — a remote backend may need I/O. The same file via different paths must yield the same `targetKey`. |
|
||||
| `stat(target, signal?)` | Return `FsInfo` metadata (`version`, `type`, optional `size`), or `undefined` when the target is absent. Never content. |
|
||||
| `readText(target, signal?)` | Read the whole regular text file as one decoded string. Owns regular-file checks, UTF-8 decoding, binary/NUL rejection (`FS_NOT_TEXT`). |
|
||||
| `streamText(target, signal?)` | Stream the same text as decoded chunks for large files (cross-chunk UTF-8 decoding stays here). |
|
||||
|
||||
@@ -165,8 +165,16 @@ export abstract class FileSystem extends Service {
|
||||
* perform I/O (a remote/sandboxed backend may need a round-trip to map a path
|
||||
* to a stable identity), hence async even though the local backend only
|
||||
* normalizes + realpaths.
|
||||
*
|
||||
* `opts.cwd` is the base directory a RELATIVE `path` resolves against; an
|
||||
* absolute `path` ignores it. Omitted ⇒ the backend's own default base (the
|
||||
* local backend uses its configured `cwd`). The CALLER supplies this — the
|
||||
* seam does not read a session or agent — so a tool can resolve against the
|
||||
* caller's per-session workspace (`exec.agent.session.header.cwd`) without the
|
||||
* provider depending on `dsh-agent`/`dsh-session`. Mirrors how `dsh-tool-bash`
|
||||
* defaults a bash `workdir` to the session cwd.
|
||||
*/
|
||||
abstract resolve(path: string): Promise<FsTarget>
|
||||
abstract resolve(path: string, opts?: { cwd?: string }): Promise<FsTarget>
|
||||
|
||||
/** Return target metadata, or `undefined` when the target does not exist. */
|
||||
abstract stat(target: FsTarget, signal?: AbortSignal): Promise<FsInfo | undefined>
|
||||
|
||||
@@ -23,9 +23,9 @@ Field names are snake_case to match Claude Code and existing harness tool schema
|
||||
|
||||
## The tool is the executor; policy is an event gate
|
||||
|
||||
The tools do **not** inject a policy service or inspect any cache. Each tool resolves the path via `ctx.fs.resolve()`, then:
|
||||
The tools do **not** inject a policy service or inspect any cache. Each tool resolves the path via `ctx.fs.resolve(path, { cwd })` — passing the calling agent's session cwd (`exec.agent.session.header.cwd`) so a relative path resolves against the session's workspace, matching `dsh-tool-bash` (see [the per-session cwd RFC](../../../docs/rfc/implemented/architecture/2026-07-02-fs-per-session-cwd.md)) — then:
|
||||
|
||||
- **read** — one `ctx.fs.stat` (type + size routing + version), then `readText`/`streamText`, then builds the line window, then emits a contained `fs/observed`. (1 stat.)
|
||||
- **read** — one `ctx.fs.stat` (type + size routing + version), then `readText`/`streamText`, then builds the line window, then emits `fs/observed` with a plain `ctx.emit`. (1 stat.)
|
||||
- **write** — `ctx.waterfall('fs/write-intent', target, exec, () => undefined)` for the optional guard, then `ctx.fs.writeText(target, content, intent)`, then `fs/observed`. (0 stat.)
|
||||
- **edit** — `ctx.waterfall('fs/edit-intent', target, exec, () => undefined)` for the optional guard, then `ctx.fs.editText(target, edit, intent)`, then `fs/observed`. (0 stat.)
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||
import type { FsEditOutcome } from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { sessionCwd } from './session-cwd.ts'
|
||||
|
||||
/** Validated `edit` arguments after defaulting. */
|
||||
interface EditInput {
|
||||
@@ -66,7 +67,8 @@ export function applyEditTool(ctx: Context): void {
|
||||
},
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
const input = parseEditArgs(args)
|
||||
const target = await ctx.fs.resolve(input.filePath)
|
||||
const cwd = sessionCwd(exec)
|
||||
const target = await ctx.fs.resolve(input.filePath, cwd !== undefined ? { cwd } : undefined)
|
||||
// Single-slot decision: the policy plugin returns { version: vObserved } or
|
||||
// throws FS_NOT_OBSERVED; the bare default is undefined (unconditional edit).
|
||||
// No stat — the bare default never manufactures a version basis.
|
||||
|
||||
@@ -20,6 +20,7 @@ import type {} from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { buildWindow, formatReadOutput } from './read-render.ts'
|
||||
import type { FileReadOutcome } from './read-render.ts'
|
||||
import { sessionCwd } from './session-cwd.ts'
|
||||
|
||||
/** Default and maximum number of lines returned by one `read` call. */
|
||||
export const READ_LIMIT = 2000
|
||||
@@ -68,7 +69,8 @@ export function applyReadTool(ctx: Context): void {
|
||||
},
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
const input = parseReadArgs(args)
|
||||
const target = await ctx.fs.resolve(input.filePath)
|
||||
const cwd = sessionCwd(exec)
|
||||
const target = await ctx.fs.resolve(input.filePath, cwd !== undefined ? { cwd } : undefined)
|
||||
|
||||
// One stat: type check + size routing + the version recorded as observed.
|
||||
// A writer racing between this stat and the read can at worst make a LATER
|
||||
|
||||
24
packages/fs/tool-fs/src/session-cwd.ts
Normal file
24
packages/fs/tool-fs/src/session-cwd.ts
Normal file
@@ -0,0 +1,24 @@
|
||||
/**
|
||||
* Derive the working directory a filesystem tool resolves relative paths
|
||||
* against: the calling agent's per-session workspace
|
||||
* (`exec.agent.session.header.cwd`), so each ACP session's `read`/`write`/`edit`
|
||||
* act on ITS workspace, not the server's launch dir — mirroring how
|
||||
* `dsh-tool-bash` defaults a bash `workdir` to the session cwd.
|
||||
*
|
||||
* The `agent` is optional-chained — a non-agent caller yields `undefined`, and
|
||||
* the tool then calls `ctx.fs.resolve(path)` with no base so the backend applies
|
||||
* its own configured default (preserving the non-ACP / no-session behavior).
|
||||
* `session`/`header` are non-optional on a real `Agent`, so only `agent` needs
|
||||
* the guard (mirroring `dsh-tool-bash`'s `resolveWorkdir`). Returning `undefined`
|
||||
* rather than reading `process.cwd()` here keeps the default in ONE place (the
|
||||
* provider), per the "explicit > implicit at seams" convention.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-tool-fs/session-cwd
|
||||
*/
|
||||
|
||||
import type { ToolExecution } from '@deepseek-ai/dsh-tools'
|
||||
|
||||
/** The session workspace cwd for this call, or `undefined` when none applies. */
|
||||
export function sessionCwd(exec: ToolExecution): string | undefined {
|
||||
return exec.agent?.session.header.cwd
|
||||
}
|
||||
@@ -17,6 +17,7 @@ import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||
import type { FsWriteOutcome } from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-fs'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
import { sessionCwd } from './session-cwd.ts'
|
||||
|
||||
/** Validate value constraints the schema DSL can't express. */
|
||||
export function parseWriteArgs(args: { file_path: string; content: string }): { filePath: string; content: string } {
|
||||
@@ -51,7 +52,8 @@ export function applyWriteTool(ctx: Context): void {
|
||||
},
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
const input = parseWriteArgs(args)
|
||||
const target = await ctx.fs.resolve(input.filePath)
|
||||
const cwd = sessionCwd(exec)
|
||||
const target = await ctx.fs.resolve(input.filePath, cwd !== undefined ? { cwd } : undefined)
|
||||
// Single-slot decision: the policy plugin produces createIfAbsent/
|
||||
// replaceIfVersion; the bare default is undefined (unconditional). No stat.
|
||||
const intent = await ctx.waterfall('fs/write-intent', target, exec, () => undefined)
|
||||
|
||||
@@ -28,8 +28,10 @@ import * as ToolFs from '@deepseek-ai/dsh-tool-fs'
|
||||
let dir: string
|
||||
let ctx: Context
|
||||
let fiber: Awaited<ReturnType<Context['plugin']>>
|
||||
// A stable session object stands in for an agent session (the file-state owner).
|
||||
const session = {}
|
||||
// A stable session object stands in for an agent session (the file-state
|
||||
// owner). It carries a `header` (no `cwd`) so `sessionCwd(exec)` resolves to
|
||||
// `undefined` and the backend falls back to its configured cwd (= `dir`).
|
||||
const session = { header: {} }
|
||||
|
||||
let callCounter = 0
|
||||
function call(name: string, args: unknown) {
|
||||
@@ -287,3 +289,52 @@ describe('bare provider (no dsh-fs-policy)', () => {
|
||||
statSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
// --------------------------------------------------------------------------
|
||||
// Per-session cwd: a relative file_path resolves against the CALLING session's
|
||||
// workspace (`exec.agent.session.header.cwd`), NOT the backend's config.cwd —
|
||||
// so an ACP editor's per-session dir wins, matching dsh-tool-bash. The regression
|
||||
// this guards: before the seam fix the tool passed no cwd, so a relative write
|
||||
// landed in config.cwd instead of the session dir.
|
||||
// --------------------------------------------------------------------------
|
||||
describe('per-session cwd', () => {
|
||||
let sessionDir: string
|
||||
beforeEach(async () => {
|
||||
dir = await mkdtemp(join(tmpdir(), 'dsh-tool-fs-cfg-'))
|
||||
sessionDir = await mkdtemp(join(tmpdir(), 'dsh-tool-fs-session-'))
|
||||
ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(LocalFileSystem, { cwd: dir }) // config.cwd = dir, NOT sessionDir
|
||||
await ctx.plugin(FsPolicy)
|
||||
fiber = await ctx.plugin(ToolFs)
|
||||
})
|
||||
afterEach(async () => { await rm(sessionDir, { recursive: true, force: true }) })
|
||||
|
||||
const callIn = (sessionObj: object, name: string, args: unknown) =>
|
||||
ctx.tools.execute({
|
||||
callId: CallId(`call-${++callCounter}`),
|
||||
name,
|
||||
arguments: args,
|
||||
agent: { session: sessionObj } as never,
|
||||
})
|
||||
|
||||
it('writes a relative path into the SESSION cwd, not config.cwd', async () => {
|
||||
const result = await callIn({ header: { cwd: sessionDir } }, 'write', { file_path: 'note.txt', content: 'hi' })
|
||||
expect(result.isError).toBe(false)
|
||||
// Verify the WORLD: the file is in the session dir, and NOT in config.cwd.
|
||||
expect(await readFile(join(sessionDir, 'note.txt'), 'utf8')).toBe('hi')
|
||||
await expect(readFile(join(dir, 'note.txt'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' })
|
||||
})
|
||||
|
||||
it('read + edit both resolve against the session cwd (end-to-end)', async () => {
|
||||
// ONE session object across both calls — observed-state keys by owner
|
||||
// identity, so read must record under the same owner the edit reads.
|
||||
const session = { header: { cwd: sessionDir } }
|
||||
await writeFile(join(sessionDir, 'code.txt'), 'alpha')
|
||||
expect((await callIn(session, 'read', { file_path: 'code.txt' })).isError).toBe(false)
|
||||
const edited = await callIn(session, 'edit', { file_path: 'code.txt', old_string: 'alpha', new_string: 'beta' })
|
||||
expect(edited.isError).toBe(false)
|
||||
expect(await readFile(join(sessionDir, 'code.txt'), 'utf8')).toBe('beta')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -174,7 +174,7 @@ describe('read tool', () => {
|
||||
|
||||
it('records observed state so a follow-up edit by the same session is authorized', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
const session = {}
|
||||
const session = { header: {} }
|
||||
fs.files.set('key:a.txt', 'hello')
|
||||
expect((await call(ctx, 'read', { file_path: 'a.txt' }, { session })).isError).toBe(false)
|
||||
const edited = await call(ctx, 'edit', { file_path: 'a.txt', old_string: 'hello', new_string: 'bye' }, { session })
|
||||
@@ -259,7 +259,7 @@ describe('formatReadOutput footer variants', () => {
|
||||
describe('write tool', () => {
|
||||
it('formats a create result and uses createIfAbsent (unobserved, with the gate)', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
const result = await call(ctx, 'write', { file_path: 'a.txt', content: 'hi' }, { session: {} })
|
||||
const result = await call(ctx, 'write', { file_path: 'a.txt', content: 'hi' }, { session: { header: {} } })
|
||||
expect(result.isError).toBe(false)
|
||||
expect(text(result)).toContain('Created file')
|
||||
expect(fs.writeIntents).toEqual([{ kind: 'createIfAbsent' }])
|
||||
@@ -284,7 +284,7 @@ describe('write tool', () => {
|
||||
describe('edit tool', () => {
|
||||
it('formats a single-replacement success after a read', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
const session = {}
|
||||
const session = { header: {} }
|
||||
fs.files.set('key:a.txt', 'a')
|
||||
await call(ctx, 'read', { file_path: 'a.txt' }, { session })
|
||||
const result = await call(ctx, 'edit', { file_path: 'a.txt', old_string: 'a', new_string: 'b' }, { session })
|
||||
@@ -315,7 +315,7 @@ describe('edit tool', () => {
|
||||
it('propagates FS_NOT_OBSERVED when the file was never read (the gate decides)', async () => {
|
||||
const { ctx, fs } = await setup()
|
||||
fs.files.set('key:a.txt', 'hello')
|
||||
const result = await call(ctx, 'edit', { file_path: 'a.txt', old_string: 'a', new_string: 'b' }, { session: {} })
|
||||
const result = await call(ctx, 'edit', { file_path: 'a.txt', old_string: 'a', new_string: 'b' }, { session: { header: {} } })
|
||||
expect(result.isError).toBe(true)
|
||||
expect(result.error).toMatchObject({ code: 'FS_NOT_OBSERVED' })
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user