Skip to content

Commit 20e5f41

Browse files
authored
fix(js/plugins/middleware): propagate ToolInterruptError through filesystem handler (#5283)
1 parent 40c1990 commit 20e5f41

2 files changed

Lines changed: 57 additions & 0 deletions

File tree

js/plugins/middleware/src/filesystem.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ export const filesystem: GenerateMiddleware<typeof FilesystemOptionsSchema> =
110110
try {
111111
return await next(req, ctx);
112112
} catch (e: any) {
113+
// Don't catch ToolInterruptError — let it propagate for interrupt handling
114+
// (e.g. from toolApproval middleware further down the chain).
115+
if (e.name === 'ToolInterruptError') throw e;
116+
113117
if (filesystemToolNames.includes(req.toolRequest.name)) {
114118
const errorPart = {
115119
text: `Tool '${req.toolRequest.name}' failed: ${

js/plugins/middleware/tests/filesystem_test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { afterEach, beforeEach, describe, it } from 'node:test';
2121
import * as os from 'os';
2222
import * as path from 'path';
2323
import { filesystem } from '../src/filesystem.js';
24+
import { toolApproval } from '../src/tool-approval.js';
2425

2526
describe('filesystem middleware', () => {
2627
let tempDir: string;
@@ -619,6 +620,58 @@ D
619620
'Tool message should not be present'
620621
);
621622
});
623+
624+
it('should let ToolInterruptError propagate when toolApproval is after filesystem', async () => {
625+
const ai = genkit({});
626+
// Model requests write_file which is a filesystem tool but NOT in the approved list
627+
const pm = createToolModel(ai, 'write_file', {
628+
filePath: 'test.txt',
629+
content: 'hello',
630+
});
631+
632+
const result = (await ai.generate({
633+
model: pm,
634+
prompt: 'write a file',
635+
use: [
636+
// filesystem is FIRST — its tool hook wraps toolApproval's tool hook.
637+
// Without the fix, filesystem's catch block would swallow the ToolInterruptError.
638+
filesystem({ rootDirectory: tempDir, allowWriteAccess: true }),
639+
toolApproval({ approved: ['read_file', 'list_files'] }),
640+
],
641+
})) as any;
642+
643+
// The ToolInterruptError should propagate through filesystem's error handler
644+
// and result in an interrupted finish reason, NOT a swallowed error message.
645+
assert.strictEqual(
646+
result.finishReason,
647+
'interrupted',
648+
'Should be interrupted, not swallowed by filesystem error handler'
649+
);
650+
651+
// Verify there's no user message with a swallowed error
652+
const swallowedErrorMsg = result.messages.find(
653+
(m: any) =>
654+
m.role === 'user' &&
655+
m.content.some(
656+
(c: any) => c.text && c.text.includes("Tool 'write_file' failed")
657+
)
658+
);
659+
assert.strictEqual(
660+
swallowedErrorMsg,
661+
undefined,
662+
'ToolInterruptError should not be swallowed into a user error message'
663+
);
664+
665+
// Verify the interrupt metadata is present
666+
const interruptPart = result.message?.content.find(
667+
(p: any) => p.metadata?.interrupt
668+
);
669+
assert.ok(interruptPart, 'Should have interrupt metadata');
670+
assert.match(
671+
interruptPart.metadata.interrupt.message,
672+
/Tool not in approved list/
673+
);
674+
});
622675
});
623676
});
624677

0 commit comments

Comments
 (0)