fix(ci): stabilize module-bound exact regressions (#56085)

* Adjust compaction identifier test for summary args

* Harden exec completion after child exit

* Handle SDK compaction and skill shape drift

* Stabilize Synology Chat module-bound tests

* Restore skill source compatibility shims

* Restore self-hosted provider discovery mocks
This commit is contained in:
Tak Hoffman 2026-03-27 19:44:15 -05:00 committed by GitHub
parent ce21ef641a
commit 262e5c57c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 267 additions and 36 deletions

View File

@ -92,6 +92,7 @@ vi.mock("./runtime.js", () => ({
},
},
})),
setSynologyRuntime: vi.fn(),
}));
export function makeSecurityAccount(

View File

@ -34,7 +34,8 @@ vi.mock("./webhook-handler.js", () => ({
createWebhookHandler: vi.fn(() => vi.fn()),
}));
const { createSynologyChatPlugin } = await import("./channel.js");
const freshChannelModulePath = "./channel.js?channel-test";
const { createSynologyChatPlugin } = await import(freshChannelModulePath);
describe("createSynologyChatPlugin", () => {
beforeEach(() => {
@ -548,19 +549,15 @@ describe("createSynologyChatPlugin", () => {
abortSignal: abortCtrl.signal,
});
// Start first account (returns a pending promise)
const firstPromise = plugin.gateway.startAccount(makeCtx(abortFirst));
// Start second account on same path — should deregister the first route
const secondPromise = plugin.gateway.startAccount(makeCtx(abortSecond));
// Give microtasks time to settle
await new Promise((r) => setTimeout(r, 10));
expect(registerMock).toHaveBeenCalledTimes(2);
expect(unregisterFirst).not.toHaveBeenCalled();
expect(unregisterSecond).not.toHaveBeenCalled();
// Clean up: abort both to resolve promises and prevent test leak
abortFirst.abort();
abortSecond.abort();
await Promise.allSettled([firstPromise, secondPromise]);

View File

@ -10,7 +10,7 @@ import {
readRequestBodyWithLimit,
requestBodyErrorToText,
} from "openclaw/plugin-sdk/webhook-ingress";
import { sendMessage, resolveLegacyWebhookNameToChatUserId } from "./client.js";
import * as synologyClient from "./client.js";
import { validateToken, authorizeUserForDm, sanitizeInput, RateLimiter } from "./security.js";
import type { SynologyWebhookPayload, ResolvedSynologyChatAccount } from "./types.js";
@ -481,7 +481,7 @@ async function resolveSynologyReplyDeliveryUserId(params: {
return params.payload.user_id;
}
const resolvedChatApiUserId = await resolveLegacyWebhookNameToChatUserId({
const resolvedChatApiUserId = await synologyClient.resolveLegacyWebhookNameToChatUserId({
incomingUrl: params.account.incomingUrl,
mutableWebhookUsername: params.payload.username,
allowInsecureSsl: params.account.allowInsecureSsl,
@ -529,7 +529,7 @@ async function processAuthorizedSynologyWebhook(params: {
return;
}
await sendMessage(
await synologyClient.sendMessage(
params.account.incomingUrl,
reply,
deliveryUserId,
@ -544,7 +544,7 @@ async function processAuthorizedSynologyWebhook(params: {
params.log?.error?.(
`Failed to process message from ${params.message.payload.username}: ${errMsg}`,
);
await sendMessage(
await synologyClient.sendMessage(
params.account.incomingUrl,
"Sorry, an error occurred while processing your message.",
deliveryUserId,

View File

@ -65,7 +65,7 @@ describe("compaction identifier-preservation instructions", () => {
}
function firstSummaryInstructions() {
return mockGenerateSummary.mock.calls[0]?.[6];
return extractSummaryInstructions(mockGenerateSummary.mock.calls[0]);
}
it("injects identifier-preservation guidance even without custom instructions", async () => {
@ -101,7 +101,9 @@ describe("compaction identifier-preservation instructions", () => {
expect(mockGenerateSummary.mock.calls.length).toBeGreaterThan(1);
for (const call of mockGenerateSummary.mock.calls) {
expect(call[6]).toContain("Preserve all opaque identifiers exactly as written");
expect(extractSummaryInstructions(call)).toContain(
"Preserve all opaque identifiers exactly as written",
);
}
});
@ -114,13 +116,31 @@ describe("compaction identifier-preservation instructions", () => {
});
const mergedCall = mockGenerateSummary.mock.calls.at(-1);
const instructions = mergedCall?.[6] ?? "";
const instructions = extractSummaryInstructions(mergedCall);
expect(instructions).toContain("Merge these partial summaries into a single cohesive summary.");
expect(instructions).toContain("Prioritize customer-visible regressions.");
expect((instructions.match(/Additional focus:/g) ?? []).length).toBe(1);
});
});
function extractSummaryInstructions(call: unknown[] | undefined): string {
if (!call) {
return "";
}
for (let index = call.length - 1; index >= 4; index -= 1) {
const arg = call[index];
if (
typeof arg === "string" &&
(arg.includes("Preserve all opaque identifiers exactly as written") ||
arg.includes("Merge these partial summaries into a single cohesive summary.") ||
arg.includes("Additional focus:"))
) {
return arg;
}
}
return "";
}
describe("buildCompactionSummarizationInstructions", () => {
it("returns base instructions when no custom text is provided", () => {
const result = buildCompactionSummarizationInstructions();

View File

@ -15,6 +15,17 @@ vi.mock("@mariozechner/pi-coding-agent", async (importOriginal) => {
});
const mockGenerateSummary = vi.mocked(piCodingAgent.generateSummary);
type MockGenerateSummaryCompat = (
currentMessages: AgentMessage[],
model: NonNullable<ExtensionContext["model"]>,
reserveTokens: number,
apiKey: string,
headers: Record<string, string> | undefined,
signal?: AbortSignal,
customInstructions?: string,
previousSummary?: string,
) => Promise<string>;
const mockGenerateSummaryCompat = mockGenerateSummary as unknown as MockGenerateSummaryCompat;
describe("compaction retry integration", () => {
beforeEach(() => {
@ -56,7 +67,7 @@ describe("compaction retry integration", () => {
} as unknown as NonNullable<ExtensionContext["model"]>;
const invokeGenerateSummary = (signal = new AbortController().signal) =>
mockGenerateSummary(testMessages, testModel, 1000, "test-api-key", signal);
mockGenerateSummaryCompat(testMessages, testModel, 1000, "test-api-key", undefined, signal);
const runSummaryRetry = (options: Parameters<typeof retryAsync>[1]) =>
retryAsync(() => invokeGenerateSummary(), options);

View File

@ -1,6 +1,9 @@
import type { AgentMessage } from "@mariozechner/pi-agent-core";
import type { ExtensionContext } from "@mariozechner/pi-coding-agent";
import { estimateTokens, generateSummary } from "@mariozechner/pi-coding-agent";
import {
estimateTokens,
generateSummary as piGenerateSummary,
} from "@mariozechner/pi-coding-agent";
import type { AgentCompactionIdentifierPolicy } from "../config/types.agent-defaults.js";
import { retryAsync } from "../infra/retry.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
@ -37,6 +40,30 @@ export type CompactionSummarizationInstructions = {
identifierInstructions?: string;
};
type GenerateSummaryCompat = {
(
currentMessages: AgentMessage[],
model: NonNullable<ExtensionContext["model"]>,
reserveTokens: number,
apiKey: string,
signal?: AbortSignal,
customInstructions?: string,
previousSummary?: string,
): Promise<string>;
(
currentMessages: AgentMessage[],
model: NonNullable<ExtensionContext["model"]>,
reserveTokens: number,
apiKey: string,
headers: Record<string, string> | undefined,
signal?: AbortSignal,
customInstructions?: string,
previousSummary?: string,
): Promise<string>;
};
const generateSummaryCompat = piGenerateSummary as unknown as GenerateSummaryCompat;
function resolveIdentifierPreservationInstructions(
instructions?: CompactionSummarizationInstructions,
): string | undefined {
@ -240,6 +267,7 @@ async function summarizeChunks(params: {
params.model,
params.reserveTokens,
params.apiKey,
undefined,
params.signal,
effectiveInstructions,
summary,
@ -258,6 +286,39 @@ async function summarizeChunks(params: {
return summary ?? DEFAULT_SUMMARY_FALLBACK;
}
function generateSummary(
currentMessages: AgentMessage[],
model: NonNullable<ExtensionContext["model"]>,
reserveTokens: number,
apiKey: string,
headers: Record<string, string> | undefined,
signal: AbortSignal,
customInstructions?: string,
previousSummary?: string,
): Promise<string> {
if (piGenerateSummary.length >= 8) {
return generateSummaryCompat(
currentMessages,
model,
reserveTokens,
apiKey,
headers,
signal,
customInstructions,
previousSummary,
);
}
return generateSummaryCompat(
currentMessages,
model,
reserveTokens,
apiKey,
signal,
customInstructions,
previousSummary,
);
}
/**
* Summarize with progressive fallback for handling oversized messages.
* If full summarization fails, tries partial summarization excluding oversized messages.

View File

@ -55,18 +55,36 @@ const TAR_GZ_TRAVERSAL_BUFFER = Buffer.from(
function buildEntry(name: string): SkillEntry {
const skillDir = path.join(workspaceDir, "skills", name);
return {
skill: {
skill: createFixtureSkill({
name,
description: `${name} test skill`,
filePath: path.join(skillDir, "SKILL.md"),
baseDir: skillDir,
source: "openclaw-workspace",
disableModelInvocation: false,
},
}),
frontmatter: {},
};
}
function createFixtureSkill(params: {
name: string;
description: string;
filePath: string;
baseDir: string;
source: string;
}): SkillEntry["skill"] {
const skill = {
name: params.name,
description: params.description,
filePath: params.filePath,
baseDir: params.baseDir,
source: params.source,
sourceInfo: { source: params.source },
disableModelInvocation: false,
};
return skill as unknown as SkillEntry["skill"];
}
function buildDownloadSpec(params: {
url: string;
archive: "tar.gz" | "tar.bz2" | "zip";

View File

@ -12,14 +12,13 @@ describe("buildWorkspaceSkillStatus", () => {
const mismatchedOs = process.platform === "darwin" ? "linux" : "darwin";
const entry: SkillEntry = {
skill: {
skill: createFixtureSkill({
name: "os-scoped",
description: "test",
filePath: "/tmp/os-scoped",
baseDir: "/tmp",
source: "test",
disableModelInvocation: false,
},
}),
frontmatter: {},
metadata: {
os: [mismatchedOs],
@ -41,3 +40,22 @@ describe("buildWorkspaceSkillStatus", () => {
expect(report.skills[0]?.install).toEqual([]);
});
});
function createFixtureSkill(params: {
name: string;
description: string;
filePath: string;
baseDir: string;
source: string;
}): SkillEntry["skill"] {
const skill = {
name: params.name,
description: params.description,
filePath: params.filePath,
baseDir: params.baseDir,
source: params.source,
sourceInfo: { source: params.source },
disableModelInvocation: false,
};
return skill as unknown as SkillEntry["skill"];
}

View File

@ -19,14 +19,13 @@ function makeEntry(params: {
}>;
}): SkillEntry {
return {
skill: {
skill: createFixtureSkill({
name: params.name,
description: `desc:${params.name}`,
filePath: `/tmp/${params.name}/SKILL.md`,
baseDir: `/tmp/${params.name}`,
source: params.source ?? "openclaw-workspace",
disableModelInvocation: false,
},
}),
frontmatter: {},
metadata: {
...(params.os ? { os: params.os } : {}),
@ -37,6 +36,25 @@ function makeEntry(params: {
};
}
function createFixtureSkill(params: {
name: string;
description: string;
filePath: string;
baseDir: string;
source: string;
}): SkillEntry["skill"] {
const skill = {
name: params.name,
description: params.description,
filePath: params.filePath,
baseDir: params.baseDir,
source: params.source,
sourceInfo: { source: params.source },
disableModelInvocation: false,
};
return skill as unknown as SkillEntry["skill"];
}
describe("buildWorkspaceSkillStatus", () => {
it("reports missing requirements and install options", async () => {
const entry = makeEntry({

View File

@ -12,14 +12,13 @@ describe("resolveSkillsPromptForRun", () => {
});
it("builds prompt from entries when snapshot is missing", () => {
const entry: SkillEntry = {
skill: {
skill: createFixtureSkill({
name: "demo-skill",
description: "Demo",
filePath: "/app/skills/demo-skill/SKILL.md",
baseDir: "/app/skills/demo-skill",
source: "openclaw-bundled",
disableModelInvocation: false,
},
}),
frontmatter: {},
};
const prompt = resolveSkillsPromptForRun({
@ -30,3 +29,22 @@ describe("resolveSkillsPromptForRun", () => {
expect(prompt).toContain("/app/skills/demo-skill/SKILL.md");
});
});
function createFixtureSkill(params: {
name: string;
description: string;
filePath: string;
baseDir: string;
source: string;
}): SkillEntry["skill"] {
const skill = {
name: params.name,
description: params.description,
filePath: params.filePath,
baseDir: params.baseDir,
source: params.source,
sourceInfo: { source: params.source },
disableModelInvocation: false,
};
return skill as unknown as SkillEntry["skill"];
}

View File

@ -10,14 +10,16 @@ import {
} from "./workspace.js";
function makeSkill(name: string, desc = "A skill", filePath = `/skills/${name}/SKILL.md`): Skill {
return {
const skill = {
name,
description: desc,
filePath,
baseDir: `/skills/${name}`,
source: "workspace",
sourceInfo: { source: "workspace" },
disableModelInvocation: false,
};
return skill as unknown as Skill;
}
function makeEntry(skill: Skill): SkillEntry {

View File

@ -1,5 +1,13 @@
import type { Skill } from "@mariozechner/pi-coding-agent";
type SkillSourceShapeCompat = Skill & {
source?: string;
sourceInfo?: {
source?: string;
};
};
export function resolveSkillSource(skill: Skill): string {
return skill.source;
const compatSkill = skill as SkillSourceShapeCompat;
return compatSkill.source ?? compatSkill.sourceInfo?.source ?? "unknown";
}

View File

@ -33,14 +33,13 @@ describe("skills-cli (e2e)", () => {
const baseDir = path.join(tempWorkspaceDir, "peekaboo");
return [
{
skill: {
skill: createFixtureSkill({
name: "peekaboo",
description: "Capture UI screenshots",
filePath: path.join(baseDir, "SKILL.md"),
baseDir,
source: "openclaw-bundled",
disableModelInvocation: false,
},
}),
frontmatter: {},
metadata: { emoji: "📸" },
},
@ -84,3 +83,22 @@ describe("skills-cli (e2e)", () => {
expect(output).toContain("Details:");
});
});
function createFixtureSkill(params: {
name: string;
description: string;
filePath: string;
baseDir: string;
source: string;
}): SkillEntry["skill"] {
const skill = {
name: params.name,
description: params.description,
filePath: params.filePath,
baseDir: params.baseDir,
source: params.source,
sourceInfo: { source: params.source },
disableModelInvocation: false,
};
return skill as unknown as SkillEntry["skill"];
}

View File

@ -246,6 +246,8 @@ export async function runCommandWithTimeout(
let settled = false;
let timedOut = false;
let noOutputTimedOut = false;
let childExitState: { code: number | null; signal: NodeJS.Signals | null } | null = null;
let closeFallbackTimer: NodeJS.Timeout | null = null;
let noOutputTimer: NodeJS.Timeout | null = null;
const shouldTrackOutputTimeout =
typeof noOutputTimeoutMs === "number" &&
@ -260,6 +262,14 @@ export async function runCommandWithTimeout(
noOutputTimer = null;
};
const clearCloseFallbackTimer = () => {
if (!closeFallbackTimer) {
return;
}
clearTimeout(closeFallbackTimer);
closeFallbackTimer = null;
};
const armNoOutputTimer = () => {
if (!shouldTrackOutputTimeout || settled) {
return;
@ -304,8 +314,22 @@ export async function runCommandWithTimeout(
settled = true;
clearTimeout(timer);
clearNoOutputTimer();
clearCloseFallbackTimer();
reject(err);
});
child.on("exit", (code, signal) => {
childExitState = { code, signal };
if (settled || closeFallbackTimer) {
return;
}
closeFallbackTimer = setTimeout(() => {
if (settled) {
return;
}
child.stdout?.destroy();
child.stderr?.destroy();
}, 250);
});
child.on("close", (code, signal) => {
if (settled) {
return;
@ -313,25 +337,28 @@ export async function runCommandWithTimeout(
settled = true;
clearTimeout(timer);
clearNoOutputTimer();
clearCloseFallbackTimer();
const resolvedCode = childExitState?.code ?? code;
const resolvedSignal = childExitState?.signal ?? signal;
const termination = noOutputTimedOut
? "no-output-timeout"
: timedOut
? "timeout"
: signal != null
: resolvedSignal != null
? "signal"
: "exit";
const normalizedCode =
termination === "timeout" || termination === "no-output-timeout"
? code === 0
? resolvedCode === 0
? 124
: code
: code;
: resolvedCode
: resolvedCode;
resolve({
pid: child.pid ?? undefined,
stdout,
stderr,
code: normalizedCode,
signal,
signal: resolvedSignal,
killed: child.killed,
termination,
noOutputTimedOut,

View File

@ -149,6 +149,20 @@ function installDiscoveryHooks(state: DiscoveryState) {
buildSglangProvider: (...args: unknown[]) => buildSglangProviderMock(...args),
};
});
vi.doMock("../../../extensions/vllm/api.js", async () => {
const actual = await vi.importActual<object>("../../../extensions/vllm/api.js");
return {
...actual,
buildVllmProvider: (...args: unknown[]) => buildVllmProviderMock(...args),
};
});
vi.doMock("../../../extensions/sglang/api.js", async () => {
const actual = await vi.importActual<object>("../../../extensions/sglang/api.js");
return {
...actual,
buildSglangProvider: (...args: unknown[]) => buildSglangProviderMock(...args),
};
});
({ runProviderCatalog: state.runProviderCatalog } =
await import("../../../src/plugins/provider-discovery.js"));
const [