fix(workspace-context): skip blocked touches and disable in Code Mode
Address the two remaining review warnings on PR #106. - tools/post-execute: when a downstream listener/policy returns `block`, return early without loading or attaching workspace instructions. The registry turns a block into a final isError result, so reconciling off the original successful result leaked instructions from a rejected call and advanced nested/baseline tracking off a touch that never happened. - Disable workspaceContext in the Code Mode examples: fs tools run as run_code sub-dispatches and code-mode.ts drops sub-call additionalContext, so dynamic AGENTS.md updates are silently discarded there. Update the block regression test to assert no context is attached, and add a waterfall case proving accept still surfaces the discovered instructions.
This commit is contained in:
@@ -17,8 +17,14 @@
|
|||||||
config:
|
config:
|
||||||
model: deepseek-v4-flash
|
model: deepseek-v4-flash
|
||||||
persistenceRoot: !!js process.env.DSH_SNAPSHOT_SESSIONS_ROOT ?? './.sessions'
|
persistenceRoot: !!js process.env.DSH_SNAPSHOT_SESSIONS_ROOT ?? './.sessions'
|
||||||
workspaceContext:
|
# Disabled in Code Mode: fs tools run as run_code sub-dispatches and
|
||||||
maxBytes: 65536
|
# code-mode.ts deliberately drops sub-call `additionalContext`, so the
|
||||||
|
# nested/changed/removed AGENTS.md notices this feature emits after
|
||||||
|
# read/write/edit are discarded before the loop can append them.
|
||||||
|
# Enabling it would only ship the baseline prefix while silently
|
||||||
|
# dropping the dynamic updates, so keep it off until sub-dispatch
|
||||||
|
# context propagation lands.
|
||||||
|
workspaceContext: false
|
||||||
tools:
|
tools:
|
||||||
mode: code
|
mode: code
|
||||||
persona: |
|
persona: |
|
||||||
|
|||||||
@@ -17,8 +17,14 @@
|
|||||||
config:
|
config:
|
||||||
model: deepseek-v4-flash
|
model: deepseek-v4-flash
|
||||||
persistenceRoot: !!js process.env.DSH_SNAPSHOT_SESSIONS_ROOT ?? './.sessions'
|
persistenceRoot: !!js process.env.DSH_SNAPSHOT_SESSIONS_ROOT ?? './.sessions'
|
||||||
workspaceContext:
|
# Disabled in Code Mode: fs tools run as run_code sub-dispatches and
|
||||||
maxBytes: 65536
|
# code-mode.ts deliberately drops sub-call `additionalContext`, so the
|
||||||
|
# nested/changed/removed AGENTS.md notices this feature emits after
|
||||||
|
# read/write/edit are discarded before the loop can append them.
|
||||||
|
# Enabling it would only ship the baseline prefix while silently
|
||||||
|
# dropping the dynamic updates, so keep it off until sub-dispatch
|
||||||
|
# context propagation lands.
|
||||||
|
workspaceContext: false
|
||||||
tools:
|
tools:
|
||||||
mode: code
|
mode: code
|
||||||
persona: |
|
persona: |
|
||||||
|
|||||||
@@ -19,8 +19,14 @@
|
|||||||
model: deepseek-v4-flash
|
model: deepseek-v4-flash
|
||||||
resumeSessionId: !!js process.env.RESUME_SESSION_ID
|
resumeSessionId: !!js process.env.RESUME_SESSION_ID
|
||||||
persistenceRoot: './.sessions'
|
persistenceRoot: './.sessions'
|
||||||
workspaceContext:
|
# Disabled in Code Mode: fs tools run as run_code sub-dispatches and
|
||||||
maxBytes: 65536
|
# code-mode.ts deliberately drops sub-call `additionalContext`, so the
|
||||||
|
# nested/changed/removed AGENTS.md notices this feature emits after
|
||||||
|
# read/write/edit are discarded before the loop can append them.
|
||||||
|
# Enabling it would only ship the baseline prefix while silently
|
||||||
|
# dropping the dynamic updates, so keep it off until sub-dispatch
|
||||||
|
# context propagation lands.
|
||||||
|
workspaceContext: false
|
||||||
tools:
|
tools:
|
||||||
mode: code
|
mode: code
|
||||||
welcome: 'code-mode agent ready. Give it a multi-tool task.'
|
welcome: 'code-mode agent ready. Give it a multi-tool task.'
|
||||||
|
|||||||
@@ -91,6 +91,13 @@ export function apply(ctx: Context, config: Config): void {
|
|||||||
next,
|
next,
|
||||||
): Promise<PostToolDecision> => {
|
): Promise<PostToolDecision> => {
|
||||||
const downstream = await next()
|
const downstream = await next()
|
||||||
|
// A downstream listener/policy blocked this call: the registry turns it
|
||||||
|
// into a final `isError` result, so treat it like a failed fs touch and
|
||||||
|
// load nothing. Reconciling here would surface workspace instructions from
|
||||||
|
// a call the pipeline rejected, violating the "successful fs tool touches"
|
||||||
|
// contract, and would advance the nested/baseline tracking state off a
|
||||||
|
// touch that never really happened.
|
||||||
|
if (downstream.kind === 'block') return downstream
|
||||||
const fileSystem = ctx.get('fs')
|
const fileSystem = ctx.get('fs')
|
||||||
if (fileSystem === undefined) return downstream
|
if (fileSystem === undefined) return downstream
|
||||||
const context = await dynamicInstructionContext(
|
const context = await dynamicInstructionContext(
|
||||||
@@ -104,14 +111,10 @@ export function apply(ctx: Context, config: Config): void {
|
|||||||
fileSystem,
|
fileSystem,
|
||||||
)
|
)
|
||||||
if (context === undefined) return downstream
|
if (context === undefined) return downstream
|
||||||
const additionalContext = concatContext(context, downstream.additionalContext)
|
|
||||||
if (downstream.kind === 'block') {
|
|
||||||
return { kind: 'block', feedback: downstream.feedback, additionalContext }
|
|
||||||
}
|
|
||||||
return {
|
return {
|
||||||
kind: 'accept',
|
kind: 'accept',
|
||||||
...downstream.content !== undefined ? { content: downstream.content } : {},
|
...downstream.content !== undefined ? { content: downstream.content } : {},
|
||||||
additionalContext,
|
additionalContext: concatContext(context, downstream.additionalContext),
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -725,6 +725,62 @@ describe('workspace context request injection', () => {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('does not load workspace instructions when a downstream listener blocks the tool call', async () => {
|
||||||
|
const root = await tempRepo()
|
||||||
|
const home = await tempRepo()
|
||||||
|
const ctx = new Context()
|
||||||
|
try {
|
||||||
|
await ctx.plugin(SystemPrompt)
|
||||||
|
await ctx.plugin(ToolRegistry)
|
||||||
|
await ctx.plugin(RecordingFileSystem)
|
||||||
|
const fs = ctx.fs as RecordingFileSystem
|
||||||
|
fs.entries.set(join(root, '.git'), { type: 'directory' })
|
||||||
|
fs.entries.set(join(root, 'pkg/AGENTS.md'), { type: 'file', content: 'nested package rule' })
|
||||||
|
fs.entries.set(join(root, 'pkg/file.txt'), { type: 'file', content: 'hello' })
|
||||||
|
await ctx.plugin(ToolFs)
|
||||||
|
await ctx.plugin(workspaceContext, { dshHome: home, maxBytes: 65536 })
|
||||||
|
const agent = stubAgent(root)
|
||||||
|
|
||||||
|
const exec = {
|
||||||
|
callId: CallId('read-blocked-post-execute'),
|
||||||
|
name: 'read',
|
||||||
|
arguments: { file_path: 'pkg/file.txt' },
|
||||||
|
agent,
|
||||||
|
}
|
||||||
|
const result = {
|
||||||
|
callId: CallId('read-blocked-post-execute'),
|
||||||
|
isError: false,
|
||||||
|
content: [{ type: 'text' as const, text: 'hello' }],
|
||||||
|
}
|
||||||
|
|
||||||
|
// A later PostToolUse-style policy blocks this otherwise-successful read.
|
||||||
|
const blocked = await ctx.waterfall('tools/post-execute', exec, result, async () => ({
|
||||||
|
kind: 'block' as const,
|
||||||
|
feedback: [{ type: 'text' as const, text: 'blocked by policy' }],
|
||||||
|
}))
|
||||||
|
|
||||||
|
expect(blocked).toEqual({
|
||||||
|
kind: 'block',
|
||||||
|
feedback: [{ type: 'text', text: 'blocked by policy' }],
|
||||||
|
})
|
||||||
|
expect(blocked.additionalContext).toBeUndefined()
|
||||||
|
|
||||||
|
// The same read, when the downstream accepts, DOES surface the nested
|
||||||
|
// instructions — proving the block branch above is what suppressed them,
|
||||||
|
// and that the block did not consume the pending nested change.
|
||||||
|
const accepted = await ctx.waterfall('tools/post-execute', exec, result, async () => ({
|
||||||
|
kind: 'accept' as const,
|
||||||
|
}))
|
||||||
|
expect(accepted.kind).toBe('accept')
|
||||||
|
expect(accepted.additionalContext?.source).toEqual({ kind: 'plugin', plugin: 'workspace-context' })
|
||||||
|
expect(blocksText(accepted.additionalContext?.content)).toContain('nested package rule')
|
||||||
|
} finally {
|
||||||
|
await ctx.fiber.dispose()
|
||||||
|
await rm(root, { recursive: true, force: true })
|
||||||
|
await rm(home, { recursive: true, force: true })
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
it('contributes baseline instructions through the frozen session prefix instead of durable history', async () => {
|
it('contributes baseline instructions through the frozen session prefix instead of durable history', async () => {
|
||||||
const root = await tempRepo()
|
const root = await tempRepo()
|
||||||
const home = await tempRepo()
|
const home = await tempRepo()
|
||||||
@@ -1977,7 +2033,7 @@ describe('dynamic nested workspace context injection', () => {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
it('keeps downstream post-execute blocks while still attaching discovered instructions', async () => {
|
it('does not attach discovered instructions when a downstream listener blocks the tool call', async () => {
|
||||||
const root = await tempRepo()
|
const root = await tempRepo()
|
||||||
const home = await tempRepo()
|
const home = await tempRepo()
|
||||||
try {
|
try {
|
||||||
@@ -1998,12 +2054,11 @@ describe('dynamic nested workspace context injection', () => {
|
|||||||
agent: stubAgent(root),
|
agent: stubAgent(root),
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// The pipeline rejected this touch, so no workspace instructions from it
|
||||||
|
// should reach the model, and the block feedback must survive unchanged.
|
||||||
expect(result.isError).toBe(true)
|
expect(result.isError).toBe(true)
|
||||||
expect(blocksText(result.content)).toBe('blocked downstream')
|
expect(blocksText(result.content)).toBe('blocked downstream')
|
||||||
expect(blocksText(result.additionalContext?.content)).toContain('nested package rule')
|
expect(result.additionalContext).toBeUndefined()
|
||||||
expect(result.additionalContext?.meta).toMatchObject({
|
|
||||||
changes: [{ action: 'set', scope: 'pkg', path: 'pkg/AGENTS.md' }],
|
|
||||||
})
|
|
||||||
} finally {
|
} finally {
|
||||||
await rm(root, { recursive: true, force: true })
|
await rm(root, { recursive: true, force: true })
|
||||||
await rm(home, { recursive: true, force: true })
|
await rm(home, { recursive: true, force: true })
|
||||||
|
|||||||
Reference in New Issue
Block a user