Files
OmniRoute/tests/unit/skills-executor.test.ts
Bob.Hou d6f315018a fix(chat): continue after a server-owned tool on Chat Completions (#12867)
Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437, ESLint 0 erros nos 152 arquivos alterados, e a suíte vitest:ui completa (2149) verde.

Sobre esta PR especificamente: rodei os **23 arquivos de teste** que ela toca sobre o tip final, depois do merge da base — **392/392**. A migration `174_server_tool_executions.sql` não colide (o tip está em 173, e você já a renumerou em `c35f0fd7`).

O dono foi consultado antes do merge, porque o loop está atrás da flag `SERVER_OWNED_TOOL_LOOP_ENABLED` mas o primeiro send não-streaming mudou de dono sem flag, e a verificação manual em combo com Memory continuava desmarcada. A condição dele foi: entra se os testes focados passarem aqui. Passaram.

O lock de passthrough (`fetchCalls.length === 1`) é a parte que mais me convenceu — o double-dispatch que um `if (stream)` em volta do send existente causaria é exatamente o tipo de regressão que não aparece em teste de comportamento, só em contagem de chamada.

**Três ajustes meus na sua branch:**

1. `tests/unit/chatcore-stream-error-result.test.ts` procurava `"const legResult = await runNonStreamingProviderLeg"`, mas o seu commit final `6077b9dd` passou a reatribuir `legResult` e trocou para `let`. O guard falhava na sua própria branch (confirmei que o arquivo e o `chatCore.ts` eram byte-idênticos ao head da PR, então não era efeito da leva). Passou a aceitar `const|let` — a intenção do guard é o try/catch em volta da chamada, não a palavra-chave.

2. `tests/integration/skills-pipeline.test.ts` foi de 1156 para 1338 linhas e estourou o `testCap` de 1200. Segui o mesmo caminho que você já tinha tomado em `a1d2d20d` para os testes unitários: extraí os três casos do server-owned tool loop para `tests/integration/server-owned-tool-loop-pipeline.test.ts` (259 linhas), com instância própria do harness. O glob `tests/integration/*.test.ts` pega o arquivo novo sem registro adicional. 3/3 verdes isolados.

3. O arquivo novo herdou cinco `any` do original — que só passavam por estarem congelados no `eslint-suppressions.json` sob o nome antigo. Tipei como `Record<string, unknown>`. E `tests/unit/non-streaming-finalization.test.ts` tinha dois argumentos não usados em `trackPendingRequest`, agora prefixados com `_`.

Nada disso toca produção nem enfraquece asserção.
2026-09-07 09:15:00 -03:00

337 lines
11 KiB
TypeScript

import test from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
const TEST_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-skills-executor-"));
const TEST_DATA_DIR = path.join(TEST_ROOT, "data");
const TEST_PLUGINS_DIR = path.join(TEST_ROOT, "plugins");
const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
const ORIGINAL_PLUGINS_DIR = process.env.OMNIROUTE_PLUGINS_DIR;
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
fs.mkdirSync(TEST_PLUGINS_DIR, { recursive: true });
process.env.DATA_DIR = TEST_DATA_DIR;
process.env.OMNIROUTE_PLUGINS_DIR = TEST_PLUGINS_DIR;
const coreDb = await import("../../src/lib/db/core.ts");
const settingsDb = await import("../../src/lib/db/settings.ts");
const { skillRegistry } = await import("../../src/lib/skills/registry.ts");
const { skillExecutor } = await import("../../src/lib/skills/executor.ts");
function resetSkillsRuntime() {
skillRegistry["registeredSkills"].clear();
skillRegistry["versionCache"].clear();
skillExecutor["handlers"].clear();
skillExecutor.setTimeout(50);
skillExecutor.setMaxRetries(3);
}
async function resetStorage() {
resetSkillsRuntime();
coreDb.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
}
async function registerEchoSkill(overrides = {}) {
return skillRegistry.register({
name: "echo",
version: "1.0.0",
description: "echoes input",
schema: { input: { value: "string" }, output: { echoed: "string" } },
handler: "echo-handler",
enabled: true,
apiKeyId: "key-a",
...overrides,
});
}
test.beforeEach(async () => {
await resetStorage();
});
test.after(() => {
resetSkillsRuntime();
coreDb.resetDbInstance();
if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR;
else process.env.DATA_DIR = ORIGINAL_DATA_DIR;
if (ORIGINAL_PLUGINS_DIR === undefined) delete process.env.OMNIROUTE_PLUGINS_DIR;
else process.env.OMNIROUTE_PLUGINS_DIR = ORIGINAL_PLUGINS_DIR;
fs.rmSync(TEST_ROOT, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
});
test("skillExecutor executes a registered handler and persists execution history", async () => {
const skill = await registerEchoSkill();
skillExecutor.registerHandler("echo-handler", async (input, context) => ({
echoed: `${input.value}:${context.apiKeyId}:${context.sessionId}`,
}));
const execution = await skillExecutor.execute(
"echo@1.0.0",
{ value: "hello" },
{ apiKeyId: "key-a", sessionId: "session-1" }
);
assert.equal(execution.skillId, skill.id);
assert.equal(execution.status, "success");
assert.deepEqual(execution.output, { echoed: "hello:key-a:session-1" });
assert.equal(execution.errorMessage, null);
assert.equal(typeof execution.durationMs, "number");
const stored = skillExecutor.getExecution(execution.id);
assert.equal(stored?.status, "success");
assert.deepEqual(stored?.output, { echoed: "hello:key-a:session-1" });
const listed = skillExecutor.listExecutions("key-a");
assert.equal(listed.length, 1);
assert.equal(listed[0].id, execution.id);
});
test("skillExecutor sanitizes failed outputs and nested error subtrees before persistence", async () => {
await registerEchoSkill();
const hostile =
"tool failed access_token=skill-output-secret at /srv/private/skill-output.ts\n" +
" at run (/srv/private/skill-output.ts:8:2)";
skillExecutor.registerHandler("echo-handler", async () => ({
success: false,
status: 502,
statusText: hostile,
headers: { authorization: "Bearer skill-output-secret" },
body: hostile,
stdout: hostile,
stderr: hostile,
}));
const failedOutput = await skillExecutor.execute(
"echo@1.0.0",
{ value: "failure" },
{ apiKeyId: "key-a", sessionId: "session-output" }
);
const storedFailure = skillExecutor.getExecution(failedOutput.id);
const failureSerialized = JSON.stringify({ failedOutput, storedFailure });
assert.equal((failedOutput.output as Record<string, unknown>)?.status, 502);
assert.doesNotMatch(
failureSerialized,
/skill-output-secret|srv\/private|skill-output\.ts|\bat run\b/i
);
skillExecutor.registerHandler("echo-handler", async () => ({
success: true,
payload: {
value: "preserve me",
error: { message: hostile },
},
warning: hostile,
}));
const successfulOutput = await skillExecutor.execute(
"echo@1.0.0",
{ value: "success" },
{ apiKeyId: "key-a", sessionId: "session-success" }
);
const storedSuccess = skillExecutor.getExecution(successfulOutput.id);
const successSerialized = JSON.stringify({ successfulOutput, storedSuccess });
assert.equal(
((successfulOutput.output as Record<string, unknown>)?.payload as Record<string, unknown>)
?.value,
"preserve me"
);
assert.doesNotMatch(
successSerialized,
/skill-output-secret|srv\/private|skill-output\.ts|\bat run\b/i
);
});
test("skillExecutor treats failure discriminators and aliased error objects as boundary failures", async () => {
await registerEchoSkill();
const hostile = "Bearer skill-discriminator-secret at /srv/private/skill-discriminator.ts:8:2";
for (const result of [
{ type: "error", message: hostile },
{ status: "failed", reason: hostile },
]) {
skillExecutor.registerHandler("echo-handler", async () => result);
const execution = await skillExecutor.execute(
"echo@1.0.0",
{ value: "discriminated-failure" },
{ apiKeyId: "key-a", sessionId: "session-discriminated" }
);
const stored = skillExecutor.getExecution(execution.id);
assert.equal(execution.status, "error");
assert.equal(stored?.status, "error");
assert.doesNotMatch(
JSON.stringify({ execution, stored }),
/skill-discriminator-secret|srv\/private|skill-discriminator\.ts/i
);
}
const shared = { message: hostile };
skillExecutor.registerHandler("echo-handler", async () => ({
success: true,
payload: { error: shared },
alias: shared,
}));
const aliased = await skillExecutor.execute(
"echo@1.0.0",
{ value: "alias" },
{ apiKeyId: "key-a", sessionId: "session-alias" }
);
assert.equal(aliased.status, "success");
assert.doesNotMatch(
JSON.stringify({ aliased, stored: skillExecutor.getExecution(aliased.id) }),
/skill-discriminator-secret|srv\/private|skill-discriminator\.ts/i
);
const cyclic: Record<string, unknown> = { success: true, error: shared };
cyclic.self = cyclic;
skillExecutor.registerHandler("echo-handler", async () => cyclic);
const cycleSafe = await skillExecutor.execute(
"echo@1.0.0",
{ value: "cycle" },
{ apiKeyId: "key-a", sessionId: "session-cycle" }
);
assert.equal(cycleSafe.status, "success");
assert.doesNotThrow(() => JSON.stringify(cycleSafe.output));
assert.doesNotMatch(
JSON.stringify({ cycleSafe, stored: skillExecutor.getExecution(cycleSafe.id) }),
/skill-discriminator-secret|srv\/private|skill-discriminator\.ts/i
);
});
test("skillExecutor blocks execution when Skills are disabled in settings", async () => {
await registerEchoSkill();
await settingsDb.updateSettings({ skillsEnabled: false });
await assert.rejects(
skillExecutor.execute("echo@1.0.0", { value: "hello" }, { apiKeyId: "key-a" }),
/Skills execution is disabled/
);
});
test("skillExecutor records handler lookup failures as errored executions", async () => {
await registerEchoSkill();
await assert.rejects(
skillExecutor.execute("echo@1.0.0", { value: "hello" }, { apiKeyId: "key-a" }),
/Handler not found: echo-handler/
);
const executions = skillExecutor.listExecutions("key-a");
assert.equal(executions.length, 1);
assert.equal(executions[0].status, "error");
assert.match(executions[0].errorMessage, /Handler not found/);
assert.equal(executions[0].output, null);
});
test("skillExecutor records disabled skills and missing skills as direct failures", async () => {
await registerEchoSkill({ enabled: false });
await assert.rejects(
skillExecutor.execute("echo@1.0.0", { value: "hello" }, { apiKeyId: "key-a" }),
/Skill is disabled/
);
await assert.rejects(
skillExecutor.execute("missing@1.0.0", { value: "hello" }, { apiKeyId: "key-a" }),
/Skill not found/
);
assert.equal(skillExecutor.listExecutions("key-a").length, 0);
});
test("skillExecutor turns handler errors and timeouts into error executions", async () => {
await registerEchoSkill();
skillExecutor.registerHandler("echo-handler", async () => {
throw new Error(
"handler exploded access_token=skill-db-secret at /srv/private/skill-executor.ts\n" +
" at execute (/srv/private/skill-executor.ts:21:5)"
);
});
const failed = await skillExecutor.execute(
"echo@1.0.0",
{ value: "boom" },
{ apiKeyId: "key-a", sessionId: "session-2" }
);
assert.equal(failed.status, "error");
assert.equal(failed.output, null);
assert.match(failed.errorMessage, /handler exploded/);
assert.doesNotMatch(
String(failed.errorMessage),
/skill-db-secret|srv\/private|skill-executor\.ts|\bat execute\b/i
);
const storedFailure = skillExecutor.getExecution(failed.id);
assert.match(String(storedFailure?.errorMessage), /handler exploded/);
assert.doesNotMatch(
String(storedFailure?.errorMessage),
/skill-db-secret|srv\/private|skill-executor\.ts|\bat execute\b/i
);
skillExecutor.registerHandler(
"echo-handler",
async () =>
new Promise((resolve) => {
setTimeout(() => resolve({ late: true }), 25);
})
);
skillExecutor.setTimeout(5);
skillExecutor.setMaxRetries(7);
const timedOut = await skillExecutor.execute(
"echo@1.0.0",
{ value: "slow" },
{ apiKeyId: "key-a", sessionId: "session-3" }
);
assert.equal(skillExecutor["maxRetries"], 7);
assert.equal(timedOut.status, "error");
assert.equal(timedOut.output, null);
assert.match(timedOut.errorMessage, /timed out/i);
});
// ─── Task 3: executeClaimed separation from execute RED tests ─────────────────
test("executeClaimed executes registered handler and returns SkillExecution without writing skill_executions row", async () => {
await registerEchoSkill();
skillExecutor.registerHandler("echo-handler", async (input, context) => ({
echoed: `${input.value}:${context.apiKeyId}`,
}));
const execution = await skillExecutor.executeClaimed(
"echo@1.0.0",
{ value: "claimed" },
{ apiKeyId: "key-a", sessionId: "session-claimed" },
"test-execution-id"
);
assert.equal(execution.status, "success");
assert.deepEqual(execution.output, { echoed: "claimed:key-a" });
// Must NOT write to skill_executions (only execute() does).
const count = skillExecutor.countExecutions("key-a");
assert.equal(count, 0, "executeClaimed must not write skill_executions row");
});
test("execute still writes exactly 1 skill_executions row (existing contract preserved)", async () => {
await registerEchoSkill();
skillExecutor.registerHandler("echo-handler", async (input) => ({
echoed: input.value,
}));
await skillExecutor.execute(
"echo@1.0.0",
{ value: "persist" },
{ apiKeyId: "key-a", sessionId: "session-persist" }
);
const count = skillExecutor.countExecutions("key-a");
assert.equal(count, 1, "execute must write exactly 1 skill_executions row");
});