Fix project instructions fs provider seam
This commit is contained in:
@@ -56,6 +56,19 @@ async function waitForStdout(running: RunningBash, expected: string, timeoutMs =
|
||||
throw new Error(`stdout did not include ${JSON.stringify(expected)} after ${timeoutMs}ms`)
|
||||
}
|
||||
|
||||
async function waitForFileText(path: string, timeoutMs = 5_000): Promise<string> {
|
||||
const deadline = Date.now() + timeoutMs
|
||||
while (Date.now() < deadline) {
|
||||
try {
|
||||
return readFileSync(path, 'utf8')
|
||||
} catch (error: unknown) {
|
||||
if (typeof error !== 'object' || error === null || !('code' in error) || error.code !== 'ENOENT') throw error
|
||||
await new Promise(resolve => setTimeout(resolve, 20))
|
||||
}
|
||||
}
|
||||
throw new Error(`file ${path} did not appear after ${timeoutMs}ms`)
|
||||
}
|
||||
|
||||
describe('runBash', () => {
|
||||
it('captures stdout on success', async () => {
|
||||
const result = await runBash(spec('echo hello')).done
|
||||
@@ -119,8 +132,7 @@ describe('runBash', () => {
|
||||
// group must take the sleep down with bash.
|
||||
const pidFile = join(spillDir, `grandchild-${Date.now()}.pid`)
|
||||
const running = runBash(spec(`sleep 60 & echo $! > ${pidFile}; wait`))
|
||||
await new Promise(resolve => setTimeout(resolve, 300))
|
||||
const grandchild = Number(readFileSync(pidFile, 'utf8').trim())
|
||||
const grandchild = Number((await waitForFileText(pidFile)).trim())
|
||||
expect(grandchild).toBeGreaterThan(0)
|
||||
|
||||
running.kill()
|
||||
|
||||
@@ -4,7 +4,7 @@ Project instruction file loader for the harness. It discovers the configured per
|
||||
|
||||
## Behavior
|
||||
|
||||
The plugin listens on the `agent/request` waterfall and depends on the `ctx.fs` provider seam to read instruction file content. For each request it derives the workspace from `agent.session.header.cwd`; if the session has no cwd, it falls back to `process.cwd()` for single-session local/stdio runs. It then finds the project root by walking upward until it sees `.git` as either a directory or a file, considers the ancestor chain from project root to cwd, and loads at most one instruction file per directory by checking `instructionFileCandidates` in order. With the default candidates, `AGENTS.md` wins and `CLAUDE.md` is a compatibility fallback.
|
||||
The plugin listens on the `agent/request` waterfall and reads instruction file content through the `ctx.fs` provider seam. It deliberately does not declare `fs` as a static dependency: `agent-core` can load the plugin in providerless app trees, and the plugin simply does nothing until a filesystem provider is present at request/tool time. For each request it derives the workspace from `agent.session.header.cwd`; if the session has no cwd, it falls back to `process.cwd()` for single-session local/stdio runs. It then finds the project root by walking upward until it sees `.git` as either a directory or a file, considers the ancestor chain from project root to cwd, and loads at most one instruction file per directory by checking `instructionFileCandidates` in order. With the default candidates, `AGENTS.md` wins and `CLAUDE.md` is a compatibility fallback.
|
||||
|
||||
The plugin also listens on `tools/post-execute` for successful structured filesystem touches from the first-party `read`, `write`, and `edit` tools. When one of those tools touches a descendant of the session cwd, the plugin checks the directories between the session cwd and the touched file for instruction files that are not already visible in session context, then attaches them as `additionalContext` so the loop records a durable `context/message` for the next model request. This intentionally follows file-tool touches, not shell `cd`: `dsh-bash-local` uses fresh shells per call, and parsing arbitrary shell commands for reached paths would be brittle.
|
||||
|
||||
|
||||
@@ -17,7 +17,6 @@ import { DEFAULT_DSH_HOME_DISPLAY, defaultDshHome, resolveDshHome } from '@deeps
|
||||
import type { PostToolDecision, ToolExecution, ToolExecutionResult } from '@deepseek-ai/dsh-tools'
|
||||
|
||||
export const name = 'project-instructions'
|
||||
export const inject = ['fs']
|
||||
|
||||
const DEFAULT_BASELINE_MAX_BYTES = 64 * 1024
|
||||
const DEFAULT_PROJECT_ROOT_MARKERS = ['.git'] as const
|
||||
@@ -154,16 +153,14 @@ async function nodeStatFile(path: string): Promise<FileSignature | undefined> {
|
||||
}
|
||||
|
||||
async function fsStatFile(path: string, fileSystem: FileSystem): Promise<DiscoveredInstructionFile['signature'] & { target: FsTarget } | undefined> {
|
||||
const noFollow = await nodeStatFile(path)
|
||||
if (noFollow === undefined) return undefined
|
||||
try {
|
||||
const target = await fileSystem.resolve(path)
|
||||
const info = await fileSystem.stat(target)
|
||||
if (info?.type !== 'file') return undefined
|
||||
return { version: info.version, size: info.size ?? noFollow.size, target }
|
||||
return { version: info.version, size: info.size, target }
|
||||
} catch {
|
||||
// Expected race/absence: the no-follow check passed, but the backing fs
|
||||
// provider could no longer resolve/stat the target. Treat it as not loadable.
|
||||
// Expected race/absence: a candidate file may not exist, or may disappear
|
||||
// between directory discovery and provider stat. Treat it as not loadable.
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
@@ -594,6 +591,8 @@ export function apply(ctx: Context, config: Config): void {
|
||||
const pendingNestedDisplayPaths = new WeakMap<object, Set<string>>()
|
||||
ctx.on('agent/request', async (agent: Agent, _turn: number, _step: number, request: GenerateOptions, next) => {
|
||||
if (resolved.baselineMaxBytes <= 0 || !Number.isFinite(resolved.baselineMaxBytes)) return next()
|
||||
const fileSystem = ctx.get('fs')
|
||||
if (fileSystem === undefined) return next()
|
||||
/* v8 ignore next -- stdio compatibility fallback; tests avoid process.chdir() because cwd is process-global. */
|
||||
const cwd = agent.session.header.cwd ?? process.cwd()
|
||||
const instructions = await loadBaselineInstructions({
|
||||
@@ -603,7 +602,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
baselineMaxBytes: resolved.baselineMaxBytes,
|
||||
instructionFileCandidates: resolved.instructionFileCandidates,
|
||||
cache,
|
||||
}, ctx.fs)
|
||||
}, fileSystem)
|
||||
if (instructions !== undefined) {
|
||||
request.messages = [workspaceContextMessage(instructions.text), ...request.messages]
|
||||
}
|
||||
@@ -612,7 +611,9 @@ export function apply(ctx: Context, config: Config): void {
|
||||
ctx.on('tools/post-execute', async (exec: ToolExecution, result: ToolExecutionResult, next): Promise<PostToolDecision> => {
|
||||
const downstream = await next()
|
||||
if (downstream.kind === 'block') return downstream
|
||||
const context = await dynamicInstructionContext(exec.agent, exec, result, resolved, cache, pendingNestedDisplayPaths, ctx.fs)
|
||||
const fileSystem = ctx.get('fs')
|
||||
if (fileSystem === undefined) return downstream
|
||||
const context = await dynamicInstructionContext(exec.agent, exec, result, resolved, cache, pendingNestedDisplayPaths, fileSystem)
|
||||
if (context === undefined) return downstream
|
||||
return {
|
||||
kind: 'accept',
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { chmod, mkdtemp, mkdir, rm, symlink, writeFile } from 'node:fs/promises'
|
||||
import { join } from 'node:path'
|
||||
import { dirname, join } from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
@@ -567,6 +567,72 @@ describe('project instruction rendering', () => {
|
||||
})
|
||||
|
||||
describe('project instruction request injection', () => {
|
||||
it('mounts without requiring a filesystem provider', async () => {
|
||||
const ctx = new Context()
|
||||
try {
|
||||
const outcome = await Promise.race([
|
||||
ctx.plugin(projectInstructions, {}).then(() => {
|
||||
return 'settled' as const
|
||||
}),
|
||||
new Promise<'pending'>((resolve) => {
|
||||
setTimeout(() => {
|
||||
resolve('pending')
|
||||
}, 50)
|
||||
}),
|
||||
])
|
||||
|
||||
expect(outcome).toBe('settled')
|
||||
} finally {
|
||||
await ctx.fiber.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not declare fs as a static inject dependency', () => {
|
||||
expect('inject' in projectInstructions).toBe(false)
|
||||
})
|
||||
|
||||
it('leaves requests unchanged when no filesystem provider is present', async () => {
|
||||
const ctx = new Context()
|
||||
try {
|
||||
await ctx.plugin(projectInstructions, {})
|
||||
const request: GenerateOptions = {
|
||||
model: 'mock',
|
||||
messages: [{ role: 'user', content: [{ type: 'text', text: 'actual prompt' }] }],
|
||||
}
|
||||
|
||||
const result = await ctx.waterfall('agent/request', stubAgent('/virtual/repo'), 1, 1, request, async () => request)
|
||||
|
||||
expect(result.messages).toEqual([{ role: 'user', content: [{ type: 'text', text: 'actual prompt' }] }])
|
||||
} finally {
|
||||
await ctx.fiber.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
it('leaves post-execute decisions unchanged when no filesystem provider is present', async () => {
|
||||
const ctx = new Context()
|
||||
try {
|
||||
await ctx.plugin(projectInstructions, {})
|
||||
|
||||
const decision = await ctx.waterfall('tools/post-execute', {
|
||||
callId: CallId('no-fs-post-execute'),
|
||||
name: 'read',
|
||||
arguments: { file_path: 'pkg/file.txt' },
|
||||
agent: stubAgent('/virtual/repo'),
|
||||
}, {
|
||||
callId: CallId('no-fs-post-execute'),
|
||||
isError: false,
|
||||
content: [{ type: 'text', text: 'file content' }],
|
||||
}, async () => ({
|
||||
kind: 'accept',
|
||||
content: [{ type: 'text', text: 'downstream content' }],
|
||||
}))
|
||||
|
||||
expect(decision).toEqual({ kind: 'accept', content: [{ type: 'text', text: 'downstream content' }] })
|
||||
} finally {
|
||||
await ctx.fiber.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
it('prepends a synthetic user workspace-context message without mutating the system prompt', async () => {
|
||||
const root = await tempRepo()
|
||||
const home = await tempRepo()
|
||||
@@ -623,6 +689,32 @@ describe('project instruction request injection', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('loads provider-visible instruction files that do not exist on the host filesystem', async () => {
|
||||
const root = join(await tempRepo(), 'virtual-repo')
|
||||
const home = join(await tempRepo(), 'virtual-home')
|
||||
const ctx = new Context()
|
||||
try {
|
||||
await ctx.plugin(RecordingFileSystem)
|
||||
const fs = ctx.fs as RecordingFileSystem
|
||||
fs.entries.set(join(root, '.git'), { type: 'directory' })
|
||||
fs.entries.set(join(root, 'AGENTS.md'), { type: 'file', content: 'provider-only rule' })
|
||||
await ctx.plugin(projectInstructions, { dshHome: home })
|
||||
|
||||
const request: GenerateOptions = {
|
||||
model: 'mock',
|
||||
messages: [{ role: 'user', content: [{ type: 'text', text: 'actual prompt' }] }],
|
||||
}
|
||||
const result = await ctx.waterfall('agent/request', stubAgent(root), 1, 1, request, async () => request)
|
||||
|
||||
expect(firstText(result.messages[0])).toContain('provider-only rule')
|
||||
expect(fs.readTargets).toEqual([join(root, 'AGENTS.md')])
|
||||
} finally {
|
||||
await ctx.fiber.dispose()
|
||||
await rm(dirname(root), { recursive: true, force: true })
|
||||
await rm(dirname(home), { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('loads user-global and CLAUDE fallback content through ctx.fs', async () => {
|
||||
const root = await tempRepo()
|
||||
const home = await tempRepo()
|
||||
@@ -654,7 +746,7 @@ describe('project instruction request injection', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('skips lstat-visible instruction files when ctx.fs reports a non-file target', async () => {
|
||||
it('skips provider-visible instruction candidates when ctx.fs reports a non-file target', async () => {
|
||||
const root = await tempRepo()
|
||||
const home = await tempRepo()
|
||||
try {
|
||||
@@ -706,7 +798,7 @@ describe('project instruction request injection', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('skips lstat-visible instruction files when ctx.fs cannot stat them', async () => {
|
||||
it('skips provider-visible instruction candidates when ctx.fs cannot stat them', async () => {
|
||||
const root = await tempRepo()
|
||||
const home = await tempRepo()
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user