fix(agents): align compaction and skill metadata APIs

2026-03-29 19:01:44 +00:00 · 2026-03-27 18:03:13 +00:00
parent 894f57a4ce
commit 52ef2ef790
11 changed files with 43 additions and 93 deletions
--- a/src/agents/compaction.retry.test.ts
+++ b/src/agents/compaction.retry.test.ts
@@ -56,7 +56,7 @@ describe("compaction retry integration", () => {
  } as unknown as NonNullable<ExtensionContext["model"]>;

  const invokeGenerateSummary = (signal = new AbortController().signal) =>
-    mockGenerateSummary(testMessages, testModel, 1000, "test-api-key", undefined, signal);
+    mockGenerateSummary(testMessages, testModel, 1000, "test-api-key", signal);

  const runSummaryRetry = (options: Parameters<typeof retryAsync>[1]) =>
    retryAsync(() => invokeGenerateSummary(), options);
--- a/src/agents/compaction.ts
+++ b/src/agents/compaction.ts
@@ -257,7 +257,6 @@ async function summarizeChunks(params: {
          model,
          params.reserveTokens,
          params.apiKey,
-          model.headers,
          params.signal,
          effectiveInstructions,
          summary,
--- a/src/agents/pi-extensions/compaction-safeguard.test.ts
+++ b/src/agents/pi-extensions/compaction-safeguard.test.ts
@@ -131,13 +131,13 @@ const createCompactionEvent = (params: { messageText: string; tokensBefore: numb

 const createCompactionContext = (params: {
  sessionManager: ExtensionContext["sessionManager"];
-  getApiKeyAndHeadersMock: ReturnType<typeof vi.fn>;
+  getApiKeyMock: ReturnType<typeof vi.fn>;
 }) =>
  ({
    model: undefined,
    sessionManager: params.sessionManager,
    modelRegistry: {
-      getApiKeyAndHeaders: params.getApiKeyAndHeadersMock,
+      getApiKey: params.getApiKeyMock,
    },
  }) as unknown as Partial<ExtensionContext>;

@@ -147,14 +147,10 @@ async function runCompactionScenario(params: {
  apiKey: string | null;
 }) {
  const compactionHandler = createCompactionHandler();
-  const getApiKeyAndHeadersMock = vi
-    .fn()
-    .mockResolvedValue(
-      params.apiKey ? { ok: true, apiKey: params.apiKey } : { ok: false, error: "missing auth" },
-    );
+  const getApiKeyMock = vi.fn().mockResolvedValue(params.apiKey ?? undefined);
  const mockContext = createCompactionContext({
    sessionManager: params.sessionManager,
-    getApiKeyAndHeadersMock,
+    getApiKeyMock,
  });
  const result = (await compactionHandler(params.event, mockContext)) as {
    cancel?: boolean;
@@ -164,7 +160,7 @@ async function runCompactionScenario(params: {
      tokensBefore: number;
    };
  };
-  return { result, getApiKeyAndHeadersMock };
+  return { result, getApiKeyMock };
 }

 function expectCompactionResult(result: {
@@ -1226,10 +1222,10 @@ describe("compaction-safeguard recent-turn preservation", () => {
    });

    const compactionHandler = createCompactionHandler();
-    const getApiKeyAndHeadersMock = vi.fn().mockResolvedValue({ ok: true, apiKey: "test-key" });
+    const getApiKeyMock = vi.fn().mockResolvedValue("test-key");
    const mockContext = createCompactionContext({
      sessionManager,
-      getApiKeyAndHeadersMock,
+      getApiKeyMock,
    });
    const messagesToSummarize: AgentMessage[] = Array.from({ length: 4 }, (_unused, index) => ({
      role: "user",
@@ -1282,10 +1278,10 @@ describe("compaction-safeguard recent-turn preservation", () => {
    });

    const compactionHandler = createCompactionHandler();
-    const getApiKeyAndHeadersMock = vi.fn().mockResolvedValue({ ok: true, apiKey: "test-key" });
+    const getApiKeyMock = vi.fn().mockResolvedValue("test-key");
    const mockContext = createCompactionContext({
      sessionManager,
-      getApiKeyAndHeadersMock,
+      getApiKeyMock,
    });
    const event = {
      preparation: {
@@ -1347,10 +1343,10 @@ describe("compaction-safeguard recent-turn preservation", () => {
    });

    const compactionHandler = createCompactionHandler();
-    const getApiKeyAndHeadersMock = vi.fn().mockResolvedValue({ ok: true, apiKey: "test-key" });
+    const getApiKeyMock = vi.fn().mockResolvedValue("test-key");
    const mockContext = createCompactionContext({
      sessionManager,
-      getApiKeyAndHeadersMock,
+      getApiKeyMock,
    });
    const event = {
      preparation: {
@@ -1450,10 +1446,10 @@ describe("compaction-safeguard recent-turn preservation", () => {
    });

    const compactionHandler = createCompactionHandler();
-    const getApiKeyAndHeadersMock = vi.fn().mockResolvedValue({ ok: true, apiKey: "test-key" });
+    const getApiKeyMock = vi.fn().mockResolvedValue("test-key");
    const mockContext = createCompactionContext({
      sessionManager,
-      getApiKeyAndHeadersMock,
+      getApiKeyMock,
    });
    const event = {
      preparation: {
@@ -1513,10 +1509,10 @@ describe("compaction-safeguard recent-turn preservation", () => {
    });

    const compactionHandler = createCompactionHandler();
-    const getApiKeyAndHeadersMock = vi.fn().mockResolvedValue({ ok: true, apiKey: "test-key" });
+    const getApiKeyMock = vi.fn().mockResolvedValue("test-key");
    const mockContext = createCompactionContext({
      sessionManager,
-      getApiKeyAndHeadersMock,
+      getApiKeyMock,
    });
    const event = {
      preparation: {
@@ -1576,10 +1572,10 @@ describe("compaction-safeguard recent-turn preservation", () => {
    });

    const compactionHandler = createCompactionHandler();
-    const getApiKeyAndHeadersMock = vi.fn().mockResolvedValue({ ok: true, apiKey: "test-key" });
+    const getApiKeyMock = vi.fn().mockResolvedValue("test-key");
    const mockContext = createCompactionContext({
      sessionManager,
-      getApiKeyAndHeadersMock,
+      getApiKeyMock,
    });
    const event = {
      preparation: {
@@ -1638,7 +1634,7 @@ describe("compaction-safeguard extension model fallback", () => {
      messageText: "test message",
      tokensBefore: 1000,
    });
-    const { result, getApiKeyAndHeadersMock } = await runCompactionScenario({
+    const { result, getApiKeyMock } = await runCompactionScenario({
      sessionManager,
      event: mockEvent,
      apiKey: null,
@@ -1649,7 +1645,7 @@ describe("compaction-safeguard extension model fallback", () => {
    // KEY ASSERTION: Prove the fallback path was exercised
    // The handler should have resolved request auth with runtime.model
    // (via ctx.model ?? runtime?.model).
-    expect(getApiKeyAndHeadersMock).toHaveBeenCalledWith(model);
+    expect(getApiKeyMock).toHaveBeenCalledWith(model);

    // Verify runtime.model is still available (for completeness)
    const retrieved = getCompactionSafeguardRuntime(sessionManager);
@@ -1665,7 +1661,7 @@ describe("compaction-safeguard extension model fallback", () => {
      messageText: "test",
      tokensBefore: 500,
    });
-    const { result, getApiKeyAndHeadersMock } = await runCompactionScenario({
+    const { result, getApiKeyMock } = await runCompactionScenario({
      sessionManager,
      event: mockEvent,
      apiKey: null,
@@ -1674,7 +1670,7 @@ describe("compaction-safeguard extension model fallback", () => {
    expect(result).toEqual({ cancel: true });

    // Verify early return: request auth should NOT have been resolved when both models are missing.
-    expect(getApiKeyAndHeadersMock).not.toHaveBeenCalled();
+    expect(getApiKeyMock).not.toHaveBeenCalled();
  });
 });

@@ -1695,7 +1691,7 @@ describe("compaction-safeguard double-compaction guard", () => {
      customInstructions: "",
      signal: new AbortController().signal,
    };
-    const { result, getApiKeyAndHeadersMock } = await runCompactionScenario({
+    const { result, getApiKeyMock } = await runCompactionScenario({
      sessionManager,
      event: mockEvent,
      apiKey: "sk-test", // pragma: allowlist secret
@@ -1709,7 +1705,7 @@ describe("compaction-safeguard double-compaction guard", () => {
    expect(compaction.summary).toContain("## Open TODOs");
    expect(compaction.firstKeptEntryId).toBe("entry-1");
    expect(compaction.tokensBefore).toBe(1500);
-    expect(getApiKeyAndHeadersMock).not.toHaveBeenCalled();
+    expect(getApiKeyMock).not.toHaveBeenCalled();
  });

  it("returns compaction result with structured fallback summary sections", async () => {
@@ -1820,13 +1816,13 @@ describe("compaction-safeguard double-compaction guard", () => {
      messageText: "real message",
      tokensBefore: 1500,
    });
-    const { result, getApiKeyAndHeadersMock } = await runCompactionScenario({
+    const { result, getApiKeyMock } = await runCompactionScenario({
      sessionManager,
      event: mockEvent,
      apiKey: null,
    });
    expect(result).toEqual({ cancel: true });
-    expect(getApiKeyAndHeadersMock).toHaveBeenCalled();
+    expect(getApiKeyMock).toHaveBeenCalled();
  });

  it("treats tool results as real conversation only when linked to a meaningful user ask", async () => {
--- a/src/agents/pi-extensions/compaction-safeguard.ts
+++ b/src/agents/pi-extensions/compaction-safeguard.ts
@@ -614,37 +614,28 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
      return { cancel: true };
    }

-    let requestAuth;
+    let apiKey: string | undefined;
    try {
-      requestAuth = await ctx.modelRegistry.getApiKeyAndHeaders(model);
+      apiKey = await ctx.modelRegistry.getApiKey(model);
    } catch (err) {
      const error = err instanceof Error ? err.message : String(err);
-      log.warn(`Compaction safeguard: request auth unavailable; cancelling compaction. ${error}`);
-      setCompactionSafeguardCancelReason(
-        ctx.sessionManager,
-        `Compaction safeguard could not resolve request auth for ${model.provider}/${model.id}: ${error}`,
-      );
-      return { cancel: true };
-    }
-    if (!requestAuth.ok) {
      log.warn(
-        `Compaction safeguard: request auth resolution failed for ${model.provider}/${model.id}: ${requestAuth.error}`,
+        `Compaction safeguard: request credentials unavailable; cancelling compaction. ${error}`,
      );
      setCompactionSafeguardCancelReason(
        ctx.sessionManager,
-        `Compaction safeguard could not resolve request auth for ${model.provider}/${model.id}: ${requestAuth.error}`,
+        `Compaction safeguard could not resolve request credentials for ${model.provider}/${model.id}: ${error}`,
      );
      return { cancel: true };
    }
-    const apiKey = requestAuth.apiKey ?? "";
-    const headers = requestAuth.headers ?? model.headers;
+    const headers = model.headers;
    if (!apiKey && !headers) {
      log.warn(
-        "Compaction safeguard: no request auth available; cancelling compaction to preserve history.",
+        "Compaction safeguard: no request credentials available; cancelling compaction to preserve history.",
      );
      setCompactionSafeguardCancelReason(
        ctx.sessionManager,
-        `Compaction safeguard could not resolve request auth for ${model.provider}/${model.id}.`,
+        `Compaction safeguard could not resolve request credentials for ${model.provider}/${model.id}.`,
      );
      return { cancel: true };
    }
@@ -710,7 +701,7 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
                droppedSummary = await compactionSafeguardDeps.summarizeInStages({
                  messages: pruned.droppedMessagesList,
                  model,
-                  apiKey,
+                  apiKey: apiKey ?? "",
                  headers,
                  signal,
                  reserveTokens: Math.max(1, Math.floor(preparation.settings.reserveTokens)),
@@ -782,7 +773,7 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
              ? await compactionSafeguardDeps.summarizeInStages({
                  messages: messagesToSummarize,
                  model,
-                  apiKey,
+                  apiKey: apiKey ?? "",
                  headers,
                  signal,
                  reserveTokens,
@@ -799,7 +790,7 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void {
            const prefixSummary = await compactionSafeguardDeps.summarizeInStages({
              messages: turnPrefixMessages,
              model,
-              apiKey,
+              apiKey: apiKey ?? "",
              headers,
              signal,
              reserveTokens,
--- a/src/agents/skills-install.download.test.ts
+++ b/src/agents/skills-install.download.test.ts
@@ -60,13 +60,7 @@ function buildEntry(name: string): SkillEntry {
      description: `${name} test skill`,
      filePath: path.join(skillDir, "SKILL.md"),
      baseDir: skillDir,
-      sourceInfo: {
-        path: path.join(skillDir, "SKILL.md"),
-        source: "openclaw-workspace",
-        scope: "project",
-        origin: "top-level",
-        baseDir: skillDir,
-      },
+      source: "openclaw-workspace",
      disableModelInvocation: false,
    },
    frontmatter: {},
--- a/src/agents/skills-status.test.ts
+++ b/src/agents/skills-status.test.ts
@@ -17,13 +17,7 @@ describe("buildWorkspaceSkillStatus", () => {
        description: "test",
        filePath: "/tmp/os-scoped",
        baseDir: "/tmp",
-        sourceInfo: {
-          path: "/tmp/os-scoped",
-          source: "test",
-          scope: "project",
-          origin: "top-level",
-          baseDir: "/tmp",
-        },
+        source: "test",
        disableModelInvocation: false,
      },
      frontmatter: {},
--- a/src/agents/skills.buildworkspaceskillstatus.test.ts
+++ b/src/agents/skills.buildworkspaceskillstatus.test.ts
@@ -24,13 +24,7 @@ function makeEntry(params: {
      description: `desc:${params.name}`,
      filePath: `/tmp/${params.name}/SKILL.md`,
      baseDir: `/tmp/${params.name}`,
-      sourceInfo: {
-        path: `/tmp/${params.name}/SKILL.md`,
-        source: params.source ?? "openclaw-workspace",
-        scope: "project",
-        origin: "top-level",
-        baseDir: `/tmp/${params.name}`,
-      },
+      source: params.source ?? "openclaw-workspace",
      disableModelInvocation: false,
    },
    frontmatter: {},
--- a/src/agents/skills.resolveskillspromptforrun.test.ts
+++ b/src/agents/skills.resolveskillspromptforrun.test.ts
@@ -17,13 +17,7 @@ describe("resolveSkillsPromptForRun", () => {
        description: "Demo",
        filePath: "/app/skills/demo-skill/SKILL.md",
        baseDir: "/app/skills/demo-skill",
-        sourceInfo: {
-          path: "/app/skills/demo-skill/SKILL.md",
-          source: "openclaw-bundled",
-          scope: "project",
-          origin: "top-level",
-          baseDir: "/app/skills/demo-skill",
-        },
+        source: "openclaw-bundled",
        disableModelInvocation: false,
      },
      frontmatter: {},
--- a/src/agents/skills/compact-format.test.ts
+++ b/src/agents/skills/compact-format.test.ts
@@ -15,13 +15,7 @@ function makeSkill(name: string, desc = "A skill", filePath = `/skills/${name}/S
    description: desc,
    filePath,
    baseDir: `/skills/${name}`,
-    sourceInfo: {
-      path: filePath,
-      source: "workspace",
-      scope: "project",
-      origin: "top-level",
-      baseDir: `/skills/${name}`,
-    },
+    source: "workspace",
    disableModelInvocation: false,
  };
 }
--- a/src/agents/skills/source.ts
+++ b/src/agents/skills/source.ts
@@ -1,5 +1,5 @@
 import type { Skill } from "@mariozechner/pi-coding-agent";

 export function resolveSkillSource(skill: Skill): string {
-  return skill.sourceInfo.source;
+  return skill.source;
 }
--- a/src/cli/skills-cli.formatting.test.ts
+++ b/src/cli/skills-cli.formatting.test.ts
@@ -38,13 +38,7 @@ describe("skills-cli (e2e)", () => {
          description: "Capture UI screenshots",
          filePath: path.join(baseDir, "SKILL.md"),
          baseDir,
-          sourceInfo: {
-            path: path.join(baseDir, "SKILL.md"),
-            source: "openclaw-bundled",
-            scope: "project",
-            origin: "top-level",
-            baseDir,
-          },
+          source: "openclaw-bundled",
          disableModelInvocation: false,
        },
        frontmatter: {},