fix: address codex review round 2
Translate ctx.bash.run() REJECTIONS into the SEARCH_* taxonomy. The seam
contract has run() reject for infrastructure failures (a pre-aborted
signal, an unusable/deleted session workdir, a missing shell); the bare
await let those escape as plain Errors, so the tool registry produced
isError results without the structured SearchError { name, code } the
package documents. A pre-aborted spec.signal now maps to SEARCH_ABORTED
and any other start failure to SEARCH_FAILED, original error chained as
cause. Covered by fake-executor tests for both branches plus real-executor
integration tests pinning the exact pre-aborted-signal and deleted-cwd
paths.
This commit is contained in:
@@ -158,7 +158,11 @@ async function completeStdout(toolName: string, result: BashRunResult, rawOutput
|
|||||||
* success with zero results (`noMatches`), anything else throws a
|
* success with zero results (`noMatches`), anything else throws a
|
||||||
* {@link SearchError} (abort/timeout → `SEARCH_ABORTED`, invalid pattern →
|
* {@link SearchError} (abort/timeout → `SEARCH_ABORTED`, invalid pattern →
|
||||||
* `SEARCH_INVALID_PATTERN`, the rest → `SEARCH_FAILED` /
|
* `SEARCH_INVALID_PATTERN`, the rest → `SEARCH_FAILED` /
|
||||||
* `SEARCH_RAW_OUTPUT_OVERFLOW`).
|
* `SEARCH_RAW_OUTPUT_OVERFLOW`). A `run()` REJECTION — the seam's
|
||||||
|
* infrastructure failures (pre-aborted signal, unusable workdir, missing
|
||||||
|
* shell) — is translated into the same taxonomy: a pre-aborted signal becomes
|
||||||
|
* `SEARCH_ABORTED`, everything else `SEARCH_FAILED`, with the original as
|
||||||
|
* `cause`.
|
||||||
*
|
*
|
||||||
* @param ctx - the plugin context; execution uses its `bash` service.
|
* @param ctx - the plugin context; execution uses its `bash` service.
|
||||||
* @param exec - the tool-execution context; supplies the session cwd and the abort signal.
|
* @param exec - the tool-execution context; supplies the session cwd and the abort signal.
|
||||||
@@ -180,7 +184,18 @@ export async function runRipgrep(
|
|||||||
...cwd !== undefined ? { workdir: cwd } : {},
|
...cwd !== undefined ? { workdir: cwd } : {},
|
||||||
...exec.signal ? { signal: exec.signal } : {},
|
...exec.signal ? { signal: exec.signal } : {},
|
||||||
})
|
})
|
||||||
const result = await ctx.bash.run(spec)
|
let result: BashRunResult
|
||||||
|
try {
|
||||||
|
result = await ctx.bash.run(spec)
|
||||||
|
} catch (error: unknown) {
|
||||||
|
// The seam contract: run() REJECTS only for infrastructure failures — a
|
||||||
|
// pre-aborted signal, an unusable workdir, a missing shell. Translate them
|
||||||
|
// so these failures stay machine-routable under the SEARCH_* taxonomy.
|
||||||
|
if (spec.signal?.aborted === true) {
|
||||||
|
throw new SearchError(`${toolName} was aborted before completion (tool timeout or caller cancellation)`, 'SEARCH_ABORTED', { cause: error })
|
||||||
|
}
|
||||||
|
throw new SearchError(`${toolName} could not start its search command (unusable working directory or missing shell)`, 'SEARCH_FAILED', { cause: error })
|
||||||
|
}
|
||||||
if (result.aborted) {
|
if (result.aborted) {
|
||||||
throw new SearchError(`${toolName} was aborted before completion (tool timeout or caller cancellation)`, 'SEARCH_ABORTED')
|
throw new SearchError(`${toolName} was aborted before completion (tool timeout or caller cancellation)`, 'SEARCH_ABORTED')
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -158,4 +158,27 @@ describe.skipIf(!hasRg)('search tools over the real bash executor + real rg', ()
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('bash-start infrastructure failures stay in the SEARCH_* taxonomy', () => {
|
||||||
|
it('a pre-aborted exec.signal (real executor rejects before spawn) is SEARCH_ABORTED', async () => {
|
||||||
|
const controller = new AbortController()
|
||||||
|
controller.abort()
|
||||||
|
const result = await ctx.tools.execute({
|
||||||
|
callId: CallId(`it-${++callCounter}`),
|
||||||
|
name: 'grep',
|
||||||
|
arguments: { pattern: 'x' },
|
||||||
|
signal: controller.signal,
|
||||||
|
})
|
||||||
|
expect(result.isError).toBe(true)
|
||||||
|
expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_ABORTED' })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('an unusable session cwd (spawn failure) is SEARCH_FAILED', async () => {
|
||||||
|
const gone = join(dir, 'deleted-session-dir')
|
||||||
|
const result = await call('glob', { pattern: '*' }, { session: { header: { id: 'session-int', cwd: gone } } })
|
||||||
|
expect(result.isError).toBe(true)
|
||||||
|
expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_FAILED' })
|
||||||
|
expect(text(result)).toContain('could not start')
|
||||||
|
})
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -279,6 +279,27 @@ describe('workdir derivation and signal forwarding', () => {
|
|||||||
expect(result.error).toMatchObject({ code: 'SEARCH_ABORTED' })
|
expect(result.error).toMatchObject({ code: 'SEARCH_ABORTED' })
|
||||||
expect(text(result)).toContain('timed out after 1234ms')
|
expect(text(result)).toContain('timed out after 1234ms')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('translates a run() rejection under a pre-aborted signal into SEARCH_ABORTED', async () => {
|
||||||
|
// The seam contract: run() REJECTS for a pre-aborted signal (it never
|
||||||
|
// spawns). The plain rejection must not escape the SEARCH_* taxonomy.
|
||||||
|
const { ctx, bash } = await setup()
|
||||||
|
const controller = new AbortController()
|
||||||
|
controller.abort()
|
||||||
|
bash.handler = () => { throw new Error('aborted before spawn') }
|
||||||
|
const result = await call(ctx, 'grep', { pattern: 'x' }, { signal: controller.signal })
|
||||||
|
expect(result.isError).toBe(true)
|
||||||
|
expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_ABORTED' })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('translates a run() rejection without an abort (unusable workdir) into SEARCH_FAILED', async () => {
|
||||||
|
const { ctx, bash } = await setup()
|
||||||
|
bash.handler = () => { throw new Error('spawn bash ENOENT') }
|
||||||
|
const result = await call(ctx, 'glob', { pattern: '*' })
|
||||||
|
expect(result.isError).toBe(true)
|
||||||
|
expect(result.error).toMatchObject({ name: 'SearchError', code: 'SEARCH_FAILED' })
|
||||||
|
expect(text(result)).toContain('could not start')
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('exit semantics and failure classification', () => {
|
describe('exit semantics and failure classification', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user