From 4ae9ae12b62dbb94cd07df1fb56e501fc3565202 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 2 May 2026 18:39:26 -0700 Subject: [PATCH] test(e2e): fix kitchen sink crabbox coverage (#76287) * test(e2e): fix kitchen sink crabbox coverage * test(e2e): update kitchen sink expected diagnostics * fix(plugins): harden registry and package gates * fix(plugins): load lazy tool middleware snapshots * fix(ci): satisfy crabbox branch gates * fix(plugins): await guarded fetch cleanup --- .github/workflows/crabbox-hydrate.yml | 50 ++++++- extensions/chutes/models.ts | 135 ++++++++++-------- extensions/huggingface/models.ts | 119 ++++++++------- .../lib/kitchen-sink-plugin/assertions.mjs | 7 + .../agent-tool-result-middleware-loader.ts | 12 +- ...sion-runtime-dependencies.contract.test.ts | 6 + src/plugins/plugin-registry-snapshot.test.ts | 9 +- 7 files changed, 214 insertions(+), 124 deletions(-) diff --git a/.github/workflows/crabbox-hydrate.yml b/.github/workflows/crabbox-hydrate.yml index 35da86a752f..34fec362801 100644 --- a/.github/workflows/crabbox-hydrate.yml +++ b/.github/workflows/crabbox-hydrate.yml @@ -62,6 +62,26 @@ jobs: sudo ln -sf "$node_bin/corepack" /usr/local/bin/corepack sudo ln -sf "$pnpm_bin" /usr/local/bin/pnpm + - name: Ensure Docker is available + shell: bash + run: | + set -euo pipefail + + if ! command -v docker >/dev/null 2>&1; then + curl -fsSL https://get.docker.com | sudo sh + fi + + if command -v systemctl >/dev/null 2>&1; then + sudo systemctl start docker + fi + + if [ -S /var/run/docker.sock ]; then + sudo usermod -aG docker "$USER" || true + # The runner process keeps its original groups; grant this + # ephemeral runner session access without requiring a relogin. + sudo chmod 666 /var/run/docker.sock + fi + - name: Hydrate provider env helper shell: bash env: @@ -90,14 +110,23 @@ jobs: - name: Mark Crabbox ready shell: bash + env: + CRABBOX_ID: ${{ inputs.crabbox_id }} + CRABBOX_JOB: ${{ inputs.crabbox_job }} run: | set -euo pipefail - job="${{ inputs.crabbox_job }}" + job="${CRABBOX_JOB}" if [ -z "$job" ]; then job=hydrate; fi + case "$CRABBOX_ID" in + ''|*[!A-Za-z0-9._-]*) + echo "Invalid crabbox_id" >&2 + exit 2 + ;; + esac mkdir -p "$HOME/.crabbox/actions" - state="$HOME/.crabbox/actions/${{ inputs.crabbox_id }}.env" - env_file="$HOME/.crabbox/actions/${{ inputs.crabbox_id }}.env.sh" - services_file="$HOME/.crabbox/actions/${{ inputs.crabbox_id }}.services" + state="$HOME/.crabbox/actions/${CRABBOX_ID}.env" + env_file="$HOME/.crabbox/actions/${CRABBOX_ID}.env.sh" + services_file="$HOME/.crabbox/actions/${CRABBOX_ID}.services" write_export() { key="$1" value="${!key-}" @@ -129,13 +158,22 @@ jobs: - name: Keep Crabbox job alive shell: bash + env: + CRABBOX_ID: ${{ inputs.crabbox_id }} + CRABBOX_KEEP_ALIVE_MINUTES: ${{ inputs.crabbox_keep_alive_minutes }} run: | set -euo pipefail - minutes="${{ inputs.crabbox_keep_alive_minutes }}" + case "$CRABBOX_ID" in + ''|*[!A-Za-z0-9._-]*) + echo "Invalid crabbox_id" >&2 + exit 2 + ;; + esac + minutes="${CRABBOX_KEEP_ALIVE_MINUTES}" case "$minutes" in ''|*[!0-9]*) minutes=90 ;; esac - stop="$HOME/.crabbox/actions/${{ inputs.crabbox_id }}.stop" + stop="$HOME/.crabbox/actions/${CRABBOX_ID}.stop" deadline=$(( $(date +%s) + minutes * 60 )) while [ "$(date +%s)" -lt "$deadline" ]; do if [ -f "$stop" ]; then diff --git a/extensions/chutes/models.ts b/extensions/chutes/models.ts index 249923128a0..dff1e9c0e6e 100644 --- a/extensions/chutes/models.ts +++ b/extensions/chutes/models.ts @@ -1,5 +1,9 @@ import type { ModelDefinitionConfig } from "openclaw/plugin-sdk/provider-model-shared"; import { createSubsystemLogger } from "openclaw/plugin-sdk/runtime-env"; +import { + fetchWithSsrFGuard, + ssrfPolicyFromHttpBaseUrlAllowedHostname, +} from "openclaw/plugin-sdk/ssrf-runtime"; import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, @@ -533,77 +537,94 @@ export async function discoverChutesModels(accessToken?: string): Promise(); - const models: ModelDefinitionConfig[] = []; - - for (const entry of data) { - const id = normalizeOptionalString(entry?.id) ?? ""; - if (!id || seen.has(id)) { - continue; + const body = (await response.json()) as OpenAIListModelsResponse; + const data = body?.data; + if (!Array.isArray(data) || data.length === 0) { + log.warn("No models in response, using static catalog"); + return staticCatalog(); } - seen.add(id); - const lowerId = normalizeLowercaseStringOrEmpty(id); - const isReasoning = - entry.supported_features?.includes("reasoning") || - lowerId.includes("r1") || - lowerId.includes("thinking") || - lowerId.includes("reason") || - lowerId.includes("tee"); + const seen = new Set(); + const models: ModelDefinitionConfig[] = []; - const input: Array<"text" | "image"> = (entry.input_modalities || ["text"]).filter( - (i): i is "text" | "image" => i === "text" || i === "image", + for (const entry of data) { + const id = normalizeOptionalString(entry?.id) ?? ""; + if (!id || seen.has(id)) { + continue; + } + seen.add(id); + + const lowerId = normalizeLowercaseStringOrEmpty(id); + const isReasoning = + entry.supported_features?.includes("reasoning") || + lowerId.includes("r1") || + lowerId.includes("thinking") || + lowerId.includes("reason") || + lowerId.includes("tee"); + + const input: Array<"text" | "image"> = (entry.input_modalities || ["text"]).filter( + (i): i is "text" | "image" => i === "text" || i === "image", + ); + + models.push({ + id, + name: id, + reasoning: isReasoning, + input, + cost: { + input: entry.pricing?.prompt || 0, + output: entry.pricing?.completion || 0, + cacheRead: 0, + cacheWrite: 0, + }, + contextWindow: entry.context_length || CHUTES_DEFAULT_CONTEXT_WINDOW, + maxTokens: entry.max_output_length || CHUTES_DEFAULT_MAX_TOKENS, + compat: { + supportsUsageInStreaming: false, + }, + }); + } + + return cacheAndReturn( + effectiveKey, + models.length > 0 ? models : CHUTES_MODEL_CATALOG.map(buildChutesModelDefinition), ); - - models.push({ - id, - name: id, - reasoning: isReasoning, - input, - cost: { - input: entry.pricing?.prompt || 0, - output: entry.pricing?.completion || 0, - cacheRead: 0, - cacheWrite: 0, - }, - contextWindow: entry.context_length || CHUTES_DEFAULT_CONTEXT_WINDOW, - maxTokens: entry.max_output_length || CHUTES_DEFAULT_MAX_TOKENS, - compat: { - supportsUsageInStreaming: false, - }, - }); + } finally { + await guardedFetch.release(); } - - return cacheAndReturn( - effectiveKey, - models.length > 0 ? models : CHUTES_MODEL_CATALOG.map(buildChutesModelDefinition), - ); } catch (error) { log.warn(`Discovery failed: ${String(error)}, using static catalog`); return staticCatalog(); diff --git a/extensions/huggingface/models.ts b/extensions/huggingface/models.ts index fe1d69f7329..80a823fdd3e 100644 --- a/extensions/huggingface/models.ts +++ b/extensions/huggingface/models.ts @@ -1,4 +1,8 @@ import type { ModelDefinitionConfig } from "openclaw/plugin-sdk/provider-model-types"; +import { + fetchWithSsrFGuard, + ssrfPolicyFromHttpBaseUrlAllowedHostname, +} from "openclaw/plugin-sdk/ssrf-runtime"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime"; import { isHuggingfaceModelDiscoveryTestEnvironment } from "./model-discovery-env.js"; @@ -140,65 +144,74 @@ export async function discoverHuggingfaceModels( } try { - const response = await fetch(`${HUGGINGFACE_BASE_URL}/models`, { - signal: AbortSignal.timeout(timeoutMs), - headers: { - Authorization: `Bearer ${trimmedKey}`, - "Content-Type": "application/json", + const { response, release } = await fetchWithSsrFGuard({ + url: `${HUGGINGFACE_BASE_URL}/models`, + init: { + signal: AbortSignal.timeout(timeoutMs), + headers: { + Authorization: `Bearer ${trimmedKey}`, + "Content-Type": "application/json", + }, }, + policy: ssrfPolicyFromHttpBaseUrlAllowedHostname(HUGGINGFACE_BASE_URL), + auditContext: "huggingface-model-discovery", }); - if (!response.ok) { - return HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition); - } - - const body = (await response.json()) as OpenAIListModelsResponse; - const data = body?.data; - if (!Array.isArray(data) || data.length === 0) { - return HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition); - } - - const catalogById = new Map( - HUGGINGFACE_MODEL_CATALOG.map((model) => [model.id, model] as const), - ); - const seen = new Set(); - const models: ModelDefinitionConfig[] = []; - - for (const entry of data) { - const id = typeof entry?.id === "string" ? entry.id.trim() : ""; - if (!id || seen.has(id)) { - continue; - } - seen.add(id); - - const catalogEntry = catalogById.get(id); - if (catalogEntry) { - models.push(buildHuggingfaceModelDefinition(catalogEntry)); - continue; + try { + if (!response.ok) { + return HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition); } - const inferred = inferredMetaFromModelId(id); - const name = displayNameFromApiEntry(entry, inferred.name); - const modalities = entry.architecture?.input_modalities; - const input: Array<"text" | "image"> = - Array.isArray(modalities) && modalities.includes("image") ? ["text", "image"] : ["text"]; - const providers = Array.isArray(entry.providers) ? entry.providers : []; - const providerWithContext = providers.find( - (provider) => typeof provider?.context_length === "number" && provider.context_length > 0, + const body = (await response.json()) as OpenAIListModelsResponse; + const data = body?.data; + if (!Array.isArray(data) || data.length === 0) { + return HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition); + } + + const catalogById = new Map( + HUGGINGFACE_MODEL_CATALOG.map((model) => [model.id, model] as const), ); - models.push({ - id, - name, - reasoning: inferred.reasoning, - input, - cost: HUGGINGFACE_DEFAULT_COST, - contextWindow: providerWithContext?.context_length ?? HUGGINGFACE_DEFAULT_CONTEXT_WINDOW, - maxTokens: HUGGINGFACE_DEFAULT_MAX_TOKENS, - }); - } + const seen = new Set(); + const models: ModelDefinitionConfig[] = []; - return models.length > 0 - ? models - : HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition); + for (const entry of data) { + const id = typeof entry?.id === "string" ? entry.id.trim() : ""; + if (!id || seen.has(id)) { + continue; + } + seen.add(id); + + const catalogEntry = catalogById.get(id); + if (catalogEntry) { + models.push(buildHuggingfaceModelDefinition(catalogEntry)); + continue; + } + + const inferred = inferredMetaFromModelId(id); + const name = displayNameFromApiEntry(entry, inferred.name); + const modalities = entry.architecture?.input_modalities; + const input: Array<"text" | "image"> = + Array.isArray(modalities) && modalities.includes("image") ? ["text", "image"] : ["text"]; + const providers = Array.isArray(entry.providers) ? entry.providers : []; + const providerWithContext = providers.find( + (provider) => typeof provider?.context_length === "number" && provider.context_length > 0, + ); + models.push({ + id, + name, + reasoning: inferred.reasoning, + input, + cost: HUGGINGFACE_DEFAULT_COST, + contextWindow: providerWithContext?.context_length ?? HUGGINGFACE_DEFAULT_CONTEXT_WINDOW, + maxTokens: HUGGINGFACE_DEFAULT_MAX_TOKENS, + }); + } + + return models.length > 0 + ? models + : HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition); + } finally { + await release(); + } } catch { return HUGGINGFACE_MODEL_CATALOG.map(buildHuggingfaceModelDefinition); } diff --git a/scripts/e2e/lib/kitchen-sink-plugin/assertions.mjs b/scripts/e2e/lib/kitchen-sink-plugin/assertions.mjs index 0f3d299d8d5..3b8fdae450d 100644 --- a/scripts/e2e/lib/kitchen-sink-plugin/assertions.mjs +++ b/scripts/e2e/lib/kitchen-sink-plugin/assertions.mjs @@ -142,14 +142,21 @@ function assertExpectedDiagnostics(surfaceMode, errorMessages) { "cli registration missing explicit commands metadata", "only bundled plugins can register Codex app-server extension factories", "only bundled plugins can register agent tool result middleware", + "agent event subscription registration requires id and handle", 'compaction provider "kitchen-sink-compaction-provider" registration missing summarize', "context engine registration missing id", + "control UI descriptor registration requires id, surface, label, and valid optional fields", "http route registration missing or invalid auth: /kitchen-sink/http-route", + "node invoke policy registration missing commands", + "only bundled plugins can register trusted tool policies", "plugin must own memory slot or declare contracts.memoryEmbeddingProviders for adapter: kitchen-sink-memory-embedding-provider", "plugin must declare contracts.tools for: kitchen-sink-tool", 'channel "kitchen-sink-channel-probe" registration missing required config helpers', 'agent harness "kitchen-sink-agent-harness" registration missing required runtime methods', "memory prompt supplement registration missing builder", + "session extension registration requires namespace and description", + "session scheduler job registration requires unique id, sessionKey, and kind", + "tool metadata registration missing toolName", ]); if (!INVALID_PROBE_DIAGNOSTIC_SURFACE_MODES.has(surfaceMode)) { if (errorMessages.size > 0) { diff --git a/src/plugins/agent-tool-result-middleware-loader.ts b/src/plugins/agent-tool-result-middleware-loader.ts index 3dca9dd28a6..11d403981aa 100644 --- a/src/plugins/agent-tool-result-middleware-loader.ts +++ b/src/plugins/agent-tool-result-middleware-loader.ts @@ -68,18 +68,18 @@ export async function loadAgentToolResultMiddlewaresForRuntime(params: { return []; } - const registry = getLoadedRuntimePluginRegistry({ - workspaceDir: params.workspaceDir, - env, - requiredPluginIds: pluginIds, - }); const runtimeRegistry = - registry ?? + getLoadedRuntimePluginRegistry({ + workspaceDir: params.workspaceDir, + env, + requiredPluginIds: pluginIds, + }) ?? loadOpenClawPlugins({ config, workspaceDir: params.workspaceDir, env, onlyPluginIds: pluginIds, + manifestRegistry, activate: false, }); diff --git a/src/plugins/contracts/extension-runtime-dependencies.contract.test.ts b/src/plugins/contracts/extension-runtime-dependencies.contract.test.ts index b7d63c03652..a9fa978604d 100644 --- a/src/plugins/contracts/extension-runtime-dependencies.contract.test.ts +++ b/src/plugins/contracts/extension-runtime-dependencies.contract.test.ts @@ -101,6 +101,11 @@ function listRuntimeFiles(root: string): string[] { return files.toSorted(); } +function readManifestText(root: string): string { + const manifestPath = path.join(root, "openclaw.plugin.json"); + return fs.existsSync(manifestPath) ? fs.readFileSync(manifestPath, "utf8") : ""; +} + function packageNameForSpecifier(specifier: string): string | null { if ( specifier.startsWith("$") || @@ -244,6 +249,7 @@ describe("extension runtime dependency manifests", () => { const allowedIndirect = INDIRECT_RUNTIME_DEPENDENCIES.get(extensionDir) ?? new Set(); const runtimeText = listRuntimeFiles(extensionDir) .map((filePath) => fs.readFileSync(filePath, "utf8")) + .concat(readManifestText(extensionDir)) .join("\n"); const unused = declared.filter( diff --git a/src/plugins/plugin-registry-snapshot.test.ts b/src/plugins/plugin-registry-snapshot.test.ts index 9d6ff5a4f97..f0800d2633d 100644 --- a/src/plugins/plugin-registry-snapshot.test.ts +++ b/src/plugins/plugin-registry-snapshot.test.ts @@ -96,7 +96,7 @@ describe("loadPluginRegistrySnapshotWithMetadata", () => { expect(result.diagnostics).toEqual([]); }); - it("keeps persisted package plugins when metadata still matches", () => { + it("keeps persisted package plugins on the fast path when file signatures match", () => { const tempRoot = makeTempDir(); const rootDir = path.join(tempRoot, "workspace"); const stateDir = path.join(tempRoot, "state"); @@ -113,14 +113,19 @@ describe("loadPluginRegistrySnapshotWithMetadata", () => { expect(record?.packageJson?.fileSignature).toBeDefined(); writePersistedInstalledPluginIndexSync(index, { stateDir }); + const readFileSyncSpy = vi.spyOn(fs, "readFileSync"); const result = loadPluginRegistrySnapshotWithMetadata({ config, env, stateDir, }); + const pluginManifestFileReads = readFileSyncSpy.mock.calls.filter((call) => { + const filePath = String(call[0]); + return filePath === path.join(rootDir, "openclaw.plugin.json"); + }); expect(result.source).toBe("persisted"); - expect(result.diagnostics).toEqual([]); + expect(pluginManifestFileReads).toEqual([]); }); it("detects same-size same-mtime manifest replacements", () => {