mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(model): keep host I/O failures distinct from unavailable model targets (#6175)
Model inspection turned every coded host error into a selected-target refusal, so EMFILE at open or EIO at header read on a valid model came back as target_unavailable with advice to check the path. Callers were told to reselect a model that was fine. Only codes that describe the selected path (missing, not a directory, symlink, name too long, non-regular, changed) keep that refusal, as the Go provider already does. Other host errors reach the existing execution_failure fallback with the path and reason. Fixes #6004
This commit is contained in:
@@ -58,7 +58,10 @@ lengths; affected totals and storage validation remain unknown.
|
||||
Safetensors reports the header extent, exact string `__metadata__` values, dtype,
|
||||
shape and declared data ranges. JSON duplicate keys, invalid UTF-8, out-of-file
|
||||
ranges, overlaps, gaps, trailing unindexed data and known dtype/shape byte-length
|
||||
contradictions are rejected with a typed `file_format_rejected` constraint (public code `target_unavailable`, category `unsupported_target`). Empty
|
||||
contradictions are rejected with a typed `file_format_rejected` constraint (public code `target_unavailable`, category `unsupported_target`).
|
||||
A missing, non-regular or otherwise unusable selected path is also `target_unavailable`;
|
||||
host I/O or resource failures on a valid selection, such as `EMFILE` or `EIO`, are
|
||||
`execution_failure` with the path and reason in `details.diagnostics`. Empty
|
||||
tensors and scalar shapes are supported; packed sub-byte types require whole
|
||||
byte storage. Unknown dtypes retain declared ranges and an explicit limitation
|
||||
that shape-to-storage validation is unavailable. Each tensor records
|
||||
|
||||
@@ -57,6 +57,17 @@ export const MODEL_ARTIFACT_PROVIDER_IDENTITY = {
|
||||
version: "1",
|
||||
} as const;
|
||||
const OPERATION = "inspect_model_artifact";
|
||||
// Codes that describe the selected path itself. Other host errors (EMFILE,
|
||||
// EIO, ...) are execution failures, not a reason to reselect the model.
|
||||
const SELECTED_TARGET_CODES = new Set([
|
||||
"ENOENT",
|
||||
"ENOTDIR",
|
||||
"ELOOP",
|
||||
"ENAMETOOLONG",
|
||||
"EISDIR",
|
||||
"ENOTFILE",
|
||||
"ESTALE",
|
||||
]);
|
||||
|
||||
/** One admitted descriptor and bounded report at a time; retained cleanup blocks new work. */
|
||||
export class ModelArtifactProvider implements ModelArtifactPort {
|
||||
@@ -182,7 +193,11 @@ export class ModelArtifactProvider implements ModelArtifactPort {
|
||||
{},
|
||||
{ cause: primary },
|
||||
);
|
||||
else if (primary instanceof Error && "code" in primary)
|
||||
else if (
|
||||
primary instanceof Error &&
|
||||
"code" in primary &&
|
||||
SELECTED_TARGET_CODES.has(String(primary.code))
|
||||
)
|
||||
error = new BinaryTargetError(input.path, primary.message, { cause });
|
||||
else
|
||||
error = new ProviderAdapterError(this.identity.id, OPERATION, {
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
import { writeFile, type FileHandle } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, expect, vi } from "vitest";
|
||||
|
||||
const fault = vi.hoisted(() => ({
|
||||
path: "",
|
||||
stage: "" as "" | "open" | "read",
|
||||
cause: undefined as Error | undefined,
|
||||
handles: [] as FileHandle[],
|
||||
}));
|
||||
vi.mock("node:fs/promises", async (importOriginal) => {
|
||||
const original = await importOriginal<typeof import("node:fs/promises")>();
|
||||
return {
|
||||
...original,
|
||||
open: async (...args: Parameters<typeof original.open>) => {
|
||||
const selected = String(args[0]) === fault.path;
|
||||
if (selected && fault.stage === "open") throw fault.cause;
|
||||
const handle = await original.open(...args);
|
||||
if (selected) {
|
||||
fault.handles.push(handle);
|
||||
if (fault.stage === "read")
|
||||
handle.read = (async () => {
|
||||
throw fault.cause;
|
||||
}) as FileHandle["read"];
|
||||
}
|
||||
return handle;
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import { ModelArtifactProvider } from "../../src/model/ModelArtifactProvider.js";
|
||||
import { ModelArtifactService } from "../../src/application/model/ModelArtifactService.js";
|
||||
import { EvidenceMcpServer } from "../../src/server/EvidenceMcpServer.js";
|
||||
import { registerModelArtifactTools } from "../../src/server/registerModelArtifactTools.js";
|
||||
import { createToolResultDelivery } from "../../src/server/toolResult.js";
|
||||
import { silentLogger } from "../../src/logger.js";
|
||||
import { ggufFixture, safetensorsFixture } from "../fixtures/model/model.js";
|
||||
import { parseMcpToolError } from "../fixtures/mcpToolError.js";
|
||||
import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";
|
||||
import { mcpTest } from "../support/mcp/mcpFixture.js";
|
||||
|
||||
afterEach(() => {
|
||||
fault.path = "";
|
||||
fault.stage = "";
|
||||
fault.cause = undefined;
|
||||
fault.handles = [];
|
||||
});
|
||||
|
||||
mcpTest.for(["gguf", "safetensors"] as const)(
|
||||
"reports host I/O failures for a valid %s file as execution failures, not unavailable targets",
|
||||
async (format, { mcp }) => {
|
||||
const root = await createTestTempDirectory("rea-model-host-failure-");
|
||||
const path = join(root, `selected.${format}`);
|
||||
await writeFile(
|
||||
path,
|
||||
format === "gguf" ? ggufFixture().bytes : safetensorsFixture().bytes,
|
||||
);
|
||||
const provider = new ModelArtifactProvider();
|
||||
const server = new EvidenceMcpServer(
|
||||
{ name: "model-host-failure-test", version: "1" },
|
||||
{},
|
||||
undefined,
|
||||
createToolResultDelivery(undefined),
|
||||
);
|
||||
registerModelArtifactTools(
|
||||
server,
|
||||
new ModelArtifactService(provider),
|
||||
silentLogger,
|
||||
);
|
||||
const client = await mcp.connect(server);
|
||||
const call = (selected = path) =>
|
||||
client.callTool({
|
||||
name: "inspect_model_artifact",
|
||||
arguments: { path: selected, detail: "complete" },
|
||||
});
|
||||
try {
|
||||
expect((await call()).isError).not.toBe(true);
|
||||
fault.path = path;
|
||||
for (const [stage, code] of [
|
||||
["open", "EMFILE"],
|
||||
["read", "EIO"],
|
||||
] as const) {
|
||||
fault.stage = stage;
|
||||
fault.cause = Object.assign(
|
||||
new Error(`controlled ${stage} failure (${code})`),
|
||||
{ code },
|
||||
);
|
||||
const { error } = parseMcpToolError(await call());
|
||||
expect(error).toMatchObject({
|
||||
code: "execution_failure",
|
||||
category: "execution_failure",
|
||||
retryable: false,
|
||||
details: {
|
||||
provider_id: "model-artifact",
|
||||
diagnostics: {
|
||||
path,
|
||||
reason: `controlled ${stage} failure (${code})`,
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(error.message).not.toContain("Check that the path exists");
|
||||
if (stage === "read") expect(fault.handles.length).toBeGreaterThan(0);
|
||||
expect(fault.handles.every((handle) => handle.fd === -1)).toBe(true);
|
||||
fault.stage = "";
|
||||
expect((await call()).isError).not.toBe(true);
|
||||
}
|
||||
// Errors about the selected path itself remain target refusals.
|
||||
for (const code of [
|
||||
"ENOENT",
|
||||
"ENOTDIR",
|
||||
"ELOOP",
|
||||
"ENAMETOOLONG",
|
||||
"EISDIR",
|
||||
"ENOTFILE",
|
||||
"ESTALE",
|
||||
]) {
|
||||
fault.stage = "open";
|
||||
fault.cause = Object.assign(new Error(`controlled ${code}`), { code });
|
||||
const { error } = parseMcpToolError(await call());
|
||||
expect(error, code).toMatchObject({
|
||||
code: "target_unavailable",
|
||||
category: "unavailable",
|
||||
});
|
||||
}
|
||||
fault.stage = "";
|
||||
const { error } = parseMcpToolError(
|
||||
await call(join(root, `missing.${format}`)),
|
||||
);
|
||||
expect(error).toMatchObject({
|
||||
code: "target_unavailable",
|
||||
category: "unavailable",
|
||||
});
|
||||
} finally {
|
||||
await provider.close();
|
||||
}
|
||||
},
|
||||
);
|
||||
Reference in New Issue
Block a user