diff --git a/src/agent-opens.ts b/src/agent-opens.ts index fdaf737..f0af56d 100644 --- a/src/agent-opens.ts +++ b/src/agent-opens.ts @@ -188,14 +188,17 @@ function isWindowsDrivePrefix(raw: string): boolean { * own routes do. * @param ctx - host plugin context (carries the tools service). * @param registry - the open-request registry (per-session queue + views). - * @param resolveCwd - live cwd resolver for one session id. + * @param resolveCwd - async cwd resolver for one session id. Resolves through + * the session header, the client-supplied cwd, and the persistence index + * before falling back to the host process cwd (production always provides + * persistence, so the fallback is reached only in tests / stripped-down hosts). * @param readPrefs - live resolved side card prefs (for tab enable gates). * @returns a disposer that unregisters the tool. */ export function registerOpenTool( ctx: Context, registry: AgentOpenRegistry, - resolveCwd: (sessionId: string) => string, + resolveCwd: (sessionId: string) => Promise, readPrefs: () => SidebarPrefs, ): () => void { return ctx.tools.register(defineTool({ @@ -245,7 +248,7 @@ export function registerOpenTool( execute: async (args: { target: string; title?: string }, exec: ToolRunContext) => { exec.signal.throwIfAborted() const sessionId = sessionIdOf(exec) - const cwd = resolveCwd(sessionId) + const cwd = await resolveCwd(sessionId) const { kind, target, title: defaultTitle } = await classifyTarget(args.target, cwd) // A disabled target tab type would make the client no-op the open: // report the real cause to the model instead of a silent success. diff --git a/src/index.ts b/src/index.ts index 3852518..46c9481 100644 --- a/src/index.ts +++ b/src/index.ts @@ -105,12 +105,17 @@ export function mediaTypeForPath(path: string): string { * header wins; while the session is still hydrating from persistence (the * web client attaches the current conversation a moment after page load, so * the very first sidebar requests can arrive detached) the caller's own - * list-summary cwd is used; the process cwd is the last resort (blank - * sessions have no cwd anywhere yet). Never throws for a missing cwd, so - * explorer/git/terminal work from first paint instead of surfacing - * "session ... has no working directory". + * list-summary cwd is used; the session-persistence index is queried as a + * last resort for cold (not-yet-attached) sessions so a detached first + * request still resolves the correct project instead of the host process + * cwd (which on Windows is the DSH source root after `dsh.cmd`'s `pushd`, + * causing every user-project path to be misclassified as "outside + * workspace"). The host process cwd is the FINAL fallback for deployments + * without persistence (tests / stripped-down hosts); production always + * provides persistence, so the bug-fix path (header → client → persistence) + * always resolves the real session cwd before reaching it. */ -function sessionCwdOf(ctx: Context, sessionId: string, clientCwd?: string): string { +async function sessionCwdOf(ctx: Context, sessionId: string, clientCwd?: string): Promise { const session = ctx.sessions.get(sessionId) const headerCwd = session?.header.cwd if (headerCwd !== undefined && headerCwd !== '') return headerCwd @@ -121,6 +126,18 @@ function sessionCwdOf(ctx: Context, sessionId: string, clientCwd?: string): stri throw new SidebarError('bad-request', `invalid working directory "${clientCwd}"`) } } + const persistence = ctx.get('sessionPersistence') + if (persistence !== undefined) { + const inspected = await persistence.inspect(sessionId) + const metaCwd = inspected.meta.cwd + if (metaCwd !== undefined && metaCwd !== '') { + try { + return requireAbsolute(metaCwd) + } catch { + throw new SidebarError('bad-request', `invalid working directory "${metaCwd}"`) + } + } + } return process.cwd() } @@ -268,16 +285,16 @@ function buildApi( terminalShell: string, getSettings: () => SidebarSettingsFace | undefined, ): Record { - const cwdOf = (payload: unknown): { sessionId: string; cwd: string } => { + const cwdOf = async (payload: unknown): Promise<{ sessionId: string; cwd: string }> => { const sessionId = requireString(payload, 'sessionId') const record = payload as { cwd?: unknown } | null const clientCwd = typeof record?.cwd === 'string' && record.cwd !== '' ? record.cwd : undefined - return { sessionId, cwd: sessionCwdOf(ctx, sessionId, clientCwd) } + return { sessionId, cwd: await sessionCwdOf(ctx, sessionId, clientCwd) } } /** Resolve the optional Git-panel checkout selector against the authoritative * session repository. Unlike `cwd`, `worktree` is never trusted directly. */ const gitCwdOf = async (payload: unknown): Promise<{ sessionId: string; cwd: string }> => { - const base = cwdOf(payload) + const base = await cwdOf(payload) const record = payload as { worktree?: unknown } | null const requested = typeof record?.worktree === 'string' && record.worktree !== '' ? record.worktree : undefined return { sessionId: base.sessionId, cwd: await git.resolveWorktree(base.cwd, requested) } @@ -293,12 +310,12 @@ function buildApi( // subagent runtime is absent (the page has no topology to show anyway). const subagentLiveApi: SidebarSubagentLiveRoutes = buildSubagentLiveApi(ctx) return { - 'session.cwd': (payload) => { - const { sessionId, cwd } = cwdOf(payload) + 'session.cwd': async (payload) => { + const { sessionId, cwd } = await cwdOf(payload) return { sessionId, cwd, root: rootLabel(cwd), parent: parentOf(cwd) ?? null } }, 'fs.tree': async (payload) => { - const { cwd } = cwdOf(payload) + const { cwd } = await cwdOf(payload) const record = payload as { path?: unknown } const target = record.path === undefined ? cwd : await ensureWorkspacePath(cwd, requireString(payload, 'path')) return listDirectory(target, resolved.listLimit) @@ -307,12 +324,12 @@ function buildApi( // The editor side panel's global name search: rooted at the session // cwd (not caller-targetable — the walk is unbounded by design and // must never escape the workspace), budgeted inside searchFiles. - const { cwd } = cwdOf(payload) + const { cwd } = await cwdOf(payload) const query = requireString(payload, 'query') return searchFiles(cwd, query) }, 'fs.read': async (payload) => { - const { cwd } = cwdOf(payload) + const { cwd } = await cwdOf(payload) // Relative paths are git-derived (status/diff report repo-root-relative // names; the untracked diff view reads the file through this route). A // child-repo path is relative to the selected repoRoot, not the session @@ -324,7 +341,7 @@ function buildApi( return { kind: 'text', content, truncated } }, 'fs.write': async (payload) => { - const { cwd } = cwdOf(payload) + const { cwd } = await cwdOf(payload) const path = await ensureWorkspaceWritePath(cwd, requireString(payload, 'path')) const content = requireString(payload, 'content') const tmp = `${path}.dsh-sidebar-tmp-${process.pid}` @@ -814,7 +831,7 @@ export function apply(ctx: Context, config?: SidebarConfig): void { if (sessionId === null || dir === null || relativePath === null || relativePath.trim() === '') { throw new SidebarError('bad-request', 'sessionId, dir, and relativePath are required') } - const cwd = sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined) + const cwd = await sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined) const { path, size } = await writeWorkspaceUpload({ cwd, dir, @@ -855,7 +872,7 @@ export function apply(ctx: Context, config?: SidebarConfig): void { const sessionId = url.searchParams.get('sessionId') const raw = url.searchParams.get('path') if (sessionId === null || raw === null) throw new SidebarError('bad-request', 'sessionId and path are required') - const cwd = sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined) + const cwd = await sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined) const path = await ensureWorkspacePath(cwd, raw) const info = await stat(path) if (!info.isFile() || info.size > resolved.mediaLimit) { @@ -914,7 +931,7 @@ export function apply(ctx: Context, config?: SidebarConfig): void { // back to the process cwd and is normally refused by the workspace // real-path guard, with the same semantics as the media route's // fallback. - const cwd = sessionCwdOf(ctx, sessionId) + const cwd = await sessionCwdOf(ctx, sessionId) const absolute = await ensureWorkspacePath(cwd, path) const info = await stat(absolute) if (!info.isFile() || info.size > resolved.mediaLimit) { @@ -1131,7 +1148,7 @@ async function attachTerminal( ws.close(1011, PTY_DEPS_MISSING) return } - const cwd = sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined) + const cwd = await sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined) // Settings-page shell overrides win over the yaml/auto shell for // terminals opened from now on (existing pty handles keep their shell). const overrides = shellOverridesOf(getSettings) diff --git a/src/tools.ts b/src/tools.ts index a23467e..d16bb38 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -73,14 +73,17 @@ function sessionIdOf(exec: ToolRunContext): string { * session's terminals. * @param ctx - host plugin context (carries the tools service). * @param registry - the agent-owned terminal registry. - * @param resolveCwd - live cwd resolver for one session id. + * @param resolveCwd - async cwd resolver for one session id. Resolves through + * the session header, the client-supplied cwd, and the persistence index + * before falling back to the host process cwd (production always provides + * persistence, so the fallback is reached only in tests / stripped-down hosts). * @returns a disposer that unregisters all eight tools (the caller gates * registration on the side-card setting and calls this to turn them off). */ export function registerTools( ctx: Context, registry: AgentPtyRegistry, - resolveCwd: (sessionId: string) => string, + resolveCwd: (sessionId: string) => Promise, readShellOverrides: () => { shell?: string; shellArgs?: string[] }, ): () => void { const disposers: Array<() => void> = [] @@ -124,13 +127,13 @@ export function registerTools( `Opened terminal "${v.title}" (uuid: ${v.uuid}). The sidebar tab appears automatically; use terminal_read to see output and terminal_send (with submit=true) to run more commands.`, ), }, - execute: (args: { title: string; command: string }, exec) => { + execute: async (args: { title: string; command: string }, exec) => { exec.signal.throwIfAborted() const sessionId = sessionIdOf(exec) - const cwd = resolveCwd(sessionId) + const cwd = await resolveCwd(sessionId) const { shell, shellArgs } = readShellOverrides() const uuid = registry.create(sessionId, args.title, args.command, cwd, 80, 24, shell, shellArgs) - return Promise.resolve({ uuid, title: args.title }) + return { uuid, title: args.title } }, })) diff --git a/tests/agent-opens.spec.ts b/tests/agent-opens.spec.ts index 1d31365..9663f7d 100644 --- a/tests/agent-opens.spec.ts +++ b/tests/agent-opens.spec.ts @@ -28,7 +28,7 @@ function exec(sessionId: string): ToolRunContext { } /** Register the tool against a fake service + real registry. */ -function mount(overrides: { prefs?: Partial; resolveCwd?: (sessionId: string) => string } = {}) { +function mount(overrides: { prefs?: Partial; resolveCwd?: (sessionId: string) => Promise } = {}) { const captured: CapturedTool[] = [] let disposeCount = 0 const ctx = { @@ -44,7 +44,7 @@ function mount(overrides: { prefs?: Partial; resolveCwd?: (session const dispose = registerOpenTool( ctx, registry, - overrides.resolveCwd ?? (() => '/cwd'), + overrides.resolveCwd ?? (async () => '/cwd'), () => prefs, ) return { captured, registry, prefs, dispose: () => { dispose(); return disposeCount } } @@ -135,7 +135,7 @@ describe('sidebar_open tool', () => { try { const file = join(dir, 'a.txt') writeFileSync(file, 'hello') - const { captured, registry } = mount({ resolveCwd: () => dir }) + const { captured, registry } = mount({ resolveCwd: async () => dir }) const received: unknown[] = [] const detach = registry.attach('s1', (request) => { received.push(request) }) const tool = toolOf(captured, 'sidebar_open') @@ -153,7 +153,7 @@ describe('sidebar_open tool', () => { const dir = mkdtempSync(join(tmpdir(), 'sidebar-open-')) try { writeFileSync(join(dir, 'note.md'), '# hi') - const { captured, registry } = mount({ resolveCwd: () => dir }) + const { captured, registry } = mount({ resolveCwd: async () => dir }) const tool = toolOf(captured, 'sidebar_open') const value = await tool.execute({ target: 'note.md' }, exec('s1')) expect(value).toMatchObject({ kind: 'file', target: join(dir, 'note.md'), title: 'note.md', delivered: false }) @@ -172,7 +172,7 @@ describe('sidebar_open tool', () => { try { const sub = join(dir, 'src') mkdirSync(sub) - const { captured } = mount({ resolveCwd: () => dir }) + const { captured } = mount({ resolveCwd: async () => dir }) const tool = toolOf(captured, 'sidebar_open') const value = await tool.execute({ target: sub, title: 'Sources' }, exec('s1')) expect(value).toEqual({ kind: 'folder', target: sub, title: 'Sources', delivered: false }) @@ -198,7 +198,7 @@ describe('sidebar_open tool', () => { }) it('reports a missing local path instead of opening it', async () => { - const { captured } = mount({ resolveCwd: () => '/definitely/missing' }) + const { captured } = mount({ resolveCwd: async () => '/definitely/missing' }) const tool = toolOf(captured, 'sidebar_open') await expect(tool.execute({ target: '/definitely/missing/nope.txt' }, exec('s1'))) .rejects.toThrow(/does not exist/) @@ -213,7 +213,7 @@ describe('sidebar_open tool', () => { const dir = mkdtempSync(join(tmpdir(), 'sidebar-open-')) try { writeFileSync(join(dir, 'f.txt'), 'x') - const { captured: fileCaptured } = mount({ resolveCwd: () => dir, prefs: { tabsEnabled: { editor: false } } }) + const { captured: fileCaptured } = mount({ resolveCwd: async () => dir, prefs: { tabsEnabled: { editor: false } } }) const fileTool = toolOf(fileCaptured, 'sidebar_open') // A URL is unaffected by the editor gate; a path is refused with cause. await expect(fileTool.execute({ target: 'https://example.com/' }, exec('s1'))).resolves.toMatchObject({ kind: 'url' }) diff --git a/tests/smoke.spec.ts b/tests/smoke.spec.ts index dd98d33..ad2f267 100644 --- a/tests/smoke.spec.ts +++ b/tests/smoke.spec.ts @@ -485,6 +485,7 @@ describe('git destructive operations (scratch repository)', () => { describe('session cwd resolution over the API route', () => { interface CtxOverrides { sessions?: { get: (id: string) => { header: { cwd?: string } } | undefined } + sessionPersistence?: { inspect: (id: string) => Promise<{ meta: { cwd?: string } }> } } const mountAll = (overrides: CtxOverrides = {}): SidebarWebRoute[] => { @@ -502,7 +503,7 @@ describe('session cwd resolution over the API route', () => { // No settings service: the namespace registration never runs. inject: () => () => {}, // No jobs/agents services in the smoke context: the routes degrade. - get: () => undefined, + get: (key: string) => key === 'sessionPersistence' ? overrides.sessionPersistence : undefined, } apply(ctx as never) return routes @@ -558,6 +559,51 @@ describe('session cwd resolution over the API route', () => { expect(result.value?.cwd).toBe(process.cwd()) }) + it('resolves a cold (detached) session cwd through the persistence index', async () => { + // Regression: a detached first request (session not yet attached, no + // client cwd) must resolve the cwd from the session-persistence index + // instead of the host process cwd. On Windows the host process cwd is + // the DSH source root (dsh.cmd's `pushd`), so every user-project path + // was misclassified as "outside workspace" by the realpath guard. + const coldCwd = resolvePath('/cold-project-cwd') + const route = mount({ + sessionPersistence: { + inspect: async (id) => ({ + meta: id === 's-cold' ? { cwd: coldCwd } : {}, + }), + }, + }) + const result = await invoke(route, 'session.cwd', { sessionId: 's-cold' }) + expect(result.ok).toBe(true) + expect(result.value?.cwd).toBe(coldCwd) + }) + + it('rejects a relative cwd from the persistence index', async () => { + // A buggy / corrupt persistence layer that stored a relative cwd must + // be rejected by requireAbsolute instead of flowing into the workspace + // guard, where it would be resolved against the host process cwd and + // potentially recreate the original "outside workspace" misclassification. + const route = mount({ + sessionPersistence: { + inspect: async () => ({ meta: { cwd: 'relative/path' } }), + }, + }) + const result = await invoke(route, 'session.cwd', { sessionId: 's-bad' }) + expect(result.ok).toBe(false) + expect(result.error?.message).toMatch(/invalid working directory/) + }) + + it('falls back to the process cwd when persistence has no cwd for the session', async () => { + const route = mount({ + sessionPersistence: { + inspect: async () => ({ meta: {} }), + }, + }) + const result = await invoke(route, 'session.cwd', { sessionId: 's-blank' }) + expect(result.ok).toBe(true) + expect(result.value?.cwd).toBe(process.cwd()) + }) + it('prefers the attached session header over the client summary', async () => { const route = mount({ sessions: { diff --git a/tests/tools.spec.ts b/tests/tools.spec.ts index b1c1ed8..865235e 100644 --- a/tests/tools.spec.ts +++ b/tests/tools.spec.ts @@ -87,7 +87,7 @@ function mount(): { captured: CapturedTool[]; registry: FakeRegistry; dispose: ( }, } as unknown as Context const registry = new FakeRegistry() - const dispose = registerTools(ctx, registry as unknown as AgentPtyRegistry, () => '/cwd', () => ({})) + const dispose = registerTools(ctx, registry as unknown as AgentPtyRegistry, async () => '/cwd', () => ({})) return { captured, registry, dispose: () => { dispose(); return disposeCount } } }