From 5b6caf97badd035a3ac113beee1b32d7361727fd Mon Sep 17 00:00:00 2001 From: hopeatina Date: Fri, 15 May 2026 17:08:00 -0500 Subject: [PATCH] fix: harden OrgX MCP tool compatibility --- src/tools/core-tools.ts | 115 +++++++++++++++++- .../http/onboarding-pairing-timeout.test.mjs | 66 ++++++---- tests/tools/core-tools-api-compat.test.mjs | 30 ++++- tests/tools/core-tools-proof-status.test.mjs | 42 +++++++ .../core-tools-register-artifact.test.mjs | 30 +++++ 5 files changed, 258 insertions(+), 25 deletions(-) diff --git a/src/tools/core-tools.ts b/src/tools/core-tools.ts index 377a226f..cf3fecdd 100644 --- a/src/tools/core-tools.ts +++ b/src/tools/core-tools.ts @@ -1,7 +1,7 @@ import { randomUUID as randomUuidFn } from "node:crypto"; import type { OrgXClient } from "../api.js"; -import { registerArtifact } from "../artifacts/register-artifact.js"; +import { registerArtifact, type ArtifactEntityType } from "../artifacts/register-artifact.js"; import { validateOpenClawSkillPackManifest } from "../contracts/skill-pack-schema.js"; import type { AutoAssignedAgent } from "../entities/auto-assignment.js"; import { listBuiltInSentinels } from "../http/helpers/sentinel-catalog.js"; @@ -140,6 +140,46 @@ export function registerCoreTools(deps: RegisterCoreToolsDeps): Map + ): Record => { + const normalized = { ...updates }; + if (entityType === "workstream" && normalized.status === "in_progress") { + normalized.status = "active"; + } + return normalized; + }; + + const resolveArtifactAssociation = ( + params: Record + ): + | { ok: true; entityType: ArtifactEntityType; entityId: string } + | { ok: false; error: string } => { + const entityType = pickNonEmptyString(params.entity_type) as ArtifactEntityType | undefined; + const entityId = pickNonEmptyString( + params.entity_id, + params.task_id, + params.workstream_id, + params.milestone_id, + params.decision_id + ); + if (entityType && entityId) { + return { ok: true, entityType, entityId }; + } + + const initiativeId = pickNonEmptyString(params.initiative_id); + if (initiativeId) { + return { ok: true, entityType: "initiative", entityId: initiativeId }; + } + + return { + ok: false, + error: + "artifact creation requires entity_type and entity_id, or initiative_id as the association target", + }; + }; + // --- orgx_status --- registerMcpTool( @@ -879,6 +919,21 @@ export function registerCoreTools(deps: RegisterCoreToolsDeps): Map = {}) { try { const { type, ...data } = params; + if (type === "artifact") { + const association = resolveArtifactAssociation(data); + if (!association.ok) { + return text(`❌ Creation failed: ${association.error}`); + } + + const name = pickNonEmptyString(data.name, data.title); + const artifactType = pickNonEmptyString(data.artifact_type); + const externalUrl = pickNonEmptyString( + data.external_url, + data.url, + data.artifact_url + ); + if (!name || !artifactType || !externalUrl) { + return text( + "❌ Creation failed: artifact creation requires name/title, artifact_type, and external_url/url/artifact_url." + ); + } + + const result = await registerArtifact(client, client.getBaseUrl(), { + entity_type: association.entityType, + entity_id: association.entityId, + name, + artifact_type: artifactType, + external_url: externalUrl, + description: pickNonEmptyString(data.description, data.summary) ?? null, + preview_markdown: + pickNonEmptyString(data.preview_markdown, data.content) ?? null, + status: pickNonEmptyString(data.status), + metadata: + data.metadata && typeof data.metadata === "object" && !Array.isArray(data.metadata) + ? { + ...(data.metadata as Record), + compatibility_alias: "orgx_create_entity type=artifact", + preferred_tool: "orgx_register_artifact", + } + : { + compatibility_alias: "orgx_create_entity type=artifact", + preferred_tool: "orgx_register_artifact", + }, + }); + + if (!result.ok) { + return text( + `❌ Creation failed: ${result.persistence.last_error ?? "artifact registration failed"}` + ); + } + + return json(`✅ Created artifact: ${name}`, { + artifact: result, + compatibility_alias: "orgx_create_entity type=artifact", + preferred_tool: "orgx_register_artifact", + }); + } + let entity = await client.createEntity(type as string, data); let assignmentSummary: { assignment_source: "orchestrator" | "fallback" | "manual"; @@ -1275,10 +1385,11 @@ export function registerCoreTools(deps: RegisterCoreToolsDeps): Map = {}) { try { const { type, id, ...updates } = params; + const normalizedUpdates = normalizeEntityUpdates(String(type), updates); const result = await client.updateEntityDetailed( type as string, id as string, - updates + normalizedUpdates ); const payload = result.reassignment || result.initiative_reassignment diff --git a/tests/http/onboarding-pairing-timeout.test.mjs b/tests/http/onboarding-pairing-timeout.test.mjs index 5c21146e..1959f9b5 100644 --- a/tests/http/onboarding-pairing-timeout.test.mjs +++ b/tests/http/onboarding-pairing-timeout.test.mjs @@ -57,10 +57,48 @@ function createApiStub(configOverrides = {}) { return stub; } +function isolateOnboardingConfig(dir) { + const previous = { + pluginDir: process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR, + openclawHome: process.env.OPENCLAW_HOME, + home: process.env.HOME, + orgxApiKey: process.env.ORGX_API_KEY, + }; + process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = dir; + process.env.OPENCLAW_HOME = dir; + process.env.HOME = dir; + delete process.env.ORGX_API_KEY; + + return () => { + if (previous.pluginDir === undefined) { + delete process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR; + } else { + process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = previous.pluginDir; + } + + if (previous.openclawHome === undefined) { + delete process.env.OPENCLAW_HOME; + } else { + process.env.OPENCLAW_HOME = previous.openclawHome; + } + + if (previous.home === undefined) { + delete process.env.HOME; + } else { + process.env.HOME = previous.home; + } + + if (previous.orgxApiKey === undefined) { + delete process.env.ORGX_API_KEY; + } else { + process.env.ORGX_API_KEY = previous.orgxApiKey; + } + }; +} + test("Onboarding pairing start uses an extended timeout for /api/plugin/openclaw/pairings", async () => { const dir = mkdtempSync(join(tmpdir(), "orgx-openclaw-pairing-")); - const prevPluginDir = process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR; - process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = dir; + const restoreConfig = isolateOnboardingConfig(dir); const prevFetch = globalThis.fetch; const prevSetTimeout = globalThis.setTimeout; @@ -145,18 +183,13 @@ test("Onboarding pairing start uses an extended timeout for /api/plugin/openclaw globalThis.fetch = prevFetch; globalThis.setTimeout = prevSetTimeout; globalThis.clearTimeout = prevClearTimeout; - if (prevPluginDir === undefined) { - delete process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR; - } else { - process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = prevPluginDir; - } + restoreConfig(); } }); test("Onboarding pairing start surfaces request tracing on failure", async () => { const dir = mkdtempSync(join(tmpdir(), "orgx-openclaw-pairing-")); - const prevPluginDir = process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR; - process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = dir; + const restoreConfig = isolateOnboardingConfig(dir); const prevFetch = globalThis.fetch; @@ -207,18 +240,13 @@ test("Onboarding pairing start surfaces request tracing on failure", async () => assert.ok(String(payload.error).includes("clerk=signed-out")); } finally { globalThis.fetch = prevFetch; - if (prevPluginDir === undefined) { - delete process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR; - } else { - process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = prevPluginDir; - } + restoreConfig(); } }); test("Onboarding pairing start retries against canonical OrgX URL when configured base URL is unreachable", async () => { const dir = mkdtempSync(join(tmpdir(), "orgx-openclaw-pairing-retry-canonical-")); - const prevPluginDir = process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR; - process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = dir; + const restoreConfig = isolateOnboardingConfig(dir); const prevFetch = globalThis.fetch; const calls = []; @@ -285,10 +313,6 @@ test("Onboarding pairing start retries against canonical OrgX URL when configure ); } finally { globalThis.fetch = prevFetch; - if (prevPluginDir === undefined) { - delete process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR; - } else { - process.env.ORGX_OPENCLAW_PLUGIN_CONFIG_DIR = prevPluginDir; - } + restoreConfig(); } }); diff --git a/tests/tools/core-tools-api-compat.test.mjs b/tests/tools/core-tools-api-compat.test.mjs index cb74e34f..eb0068b3 100644 --- a/tests/tools/core-tools-api-compat.test.mjs +++ b/tests/tools/core-tools-api-compat.test.mjs @@ -11,6 +11,7 @@ async function importMcpHandler() { function createDeps(overrides = {}) { let recordedQuality = null; + let updatedEntity = null; const deps = { registerTool: () => {}, @@ -22,7 +23,10 @@ function createDeps(overrides = {}) { checkSpawnGuard: async () => ({ ok: true, allowed: true, modelTier: "sonnet", checks: {} }), createEntity: async () => ({}), updateEntity: async () => ({}), - updateEntityDetailed: async () => ({ entity: {} }), + updateEntityDetailed: async (type, id, updates) => { + updatedEntity = { type, id, updates }; + return { entity: { id, ...updates } }; + }, listEntities: async () => ({ data: [] }), emitActivity: async () => ({}), applyChangeset: async () => ({ applied_count: 1, replayed: false, run_id: "run" }), @@ -54,7 +58,11 @@ function createDeps(overrides = {}) { ...overrides, }; - return { deps, getRecordedQuality: () => recordedQuality }; + return { + deps, + getRecordedQuality: () => recordedQuality, + getUpdatedEntity: () => updatedEntity, + }; } test("compatibility tools are registered with strict schemas", () => { @@ -93,6 +101,24 @@ test("orgx_quality_score accepts agentDomain-only requests", async () => { }); }); +test("orgx_update_entity normalizes legacy workstream in_progress status", async () => { + const { deps, getUpdatedEntity } = createDeps(); + const tool = registerCoreTools(deps).get("orgx_update_entity"); + + const result = await tool.execute("call-update-workstream", { + type: "workstream", + id: "workstream-1", + status: "in_progress", + }); + + assert.match(result.content[0].text, /Updated workstream/); + assert.deepEqual(getUpdatedEntity(), { + type: "workstream", + id: "workstream-1", + updates: { status: "active" }, + }); +}); + test("compatibility tools appear in all scoped MCP domains", async () => { const mod = await importMcpHandler(); diff --git a/tests/tools/core-tools-proof-status.test.mjs b/tests/tools/core-tools-proof-status.test.mjs index 36a2c5f0..e541e132 100644 --- a/tests/tools/core-tools-proof-status.test.mjs +++ b/tests/tools/core-tools-proof-status.test.mjs @@ -62,6 +62,48 @@ test("proof ladder tools are registered by registerCoreTools", async () => { } }); +test("orgx_proof_status returns auth guidance for unauthorized responses", async () => { + const { registerCoreTools } = await import("../../dist/tools/core-tools.js"); + + const deps = { + registerTool: () => {}, + client: { + syncMemory: async () => ({}), + checkSpawnGuard: async () => ({ ok: true, allowed: true, modelTier: "sonnet", checks: {} }), + createEntity: async () => ({}), + updateEntity: async () => ({}), + updateEntityDetailed: async () => ({ entity: {} }), + listEntities: async () => ({ data: [] }), + emitActivity: async () => ({}), + applyChangeset: async () => ({ applied_count: 1, replayed: false, run_id: "run" }), + rawRequest: async () => { + throw new Error("401 Unauthorized"); + }, + }, + config: { syncIntervalMs: 10_000, pluginVersion: "test" }, + getCachedSnapshot: () => null, + getLastSnapshotAt: () => 0, + doSync: async () => {}, + text: (v) => ({ content: [{ type: "text", text: v }] }), + json: (l, d) => ({ content: [{ type: "text", text: `${l}\n${JSON.stringify(d)}` }] }), + formatSnapshot: () => "snapshot", + autoAssignEntityForCreate: async () => ({ assignmentSource: "manual", assignedAgents: [], warnings: [] }), + toReportingPhase: () => "execution", + inferReportingInitiativeId: () => undefined, + isUuid: () => true, + pickNonEmptyString: (...vs) => vs.find((v) => typeof v === "string" && v.trim())?.trim(), + resolveReportingContext: () => ({ ok: false, error: "unused" }), + readSkillPackState: () => ({}), + randomUUID: () => "uuid-test", + }; + + const tool = registerCoreTools(deps).get("orgx_proof_status"); + const result = await tool.execute("call-proof-auth", { task_id: "task-1" }); + + assert.match(result.content[0].text, /Proof status requires authentication/); + assert.match(result.content[0].text, /auth_required/); +}); + // --------------------------------------------------------------------------- // 2. MCP scope inclusion – proof tools in all 7 domain scopes // --------------------------------------------------------------------------- diff --git a/tests/tools/core-tools-register-artifact.test.mjs b/tests/tools/core-tools-register-artifact.test.mjs index cd5f770e..101c4e6f 100644 --- a/tests/tools/core-tools-register-artifact.test.mjs +++ b/tests/tools/core-tools-register-artifact.test.mjs @@ -155,3 +155,33 @@ test("orgx_register_artifact forwards caller proof metadata to durable artifacts }); assert.ok(createRequest.body.metadata.artifact_hash, "expected artifact hash to be generated"); }); + +test("orgx_create_entity routes artifact payloads to durable artifact registration", async () => { + const { deps, requests } = createDeps(); + const tool = registerCoreTools(deps).get("orgx_create_entity"); + + const result = await tool.execute("call-create-artifact", { + type: "artifact", + title: "OrgX MCP error audit", + artifact_type: "engineering.report", + initiative_id: INITIATIVE_ID, + external_url: "https://github.com/useorgx/orgx-mcp/pull/184", + summary: "Audit and fix notes for MCP tool-call failures.", + }); + + assert.match(result.content[0].text, /Created artifact: OrgX MCP error audit/); + + const createRequest = requests.find( + (request) => request.method === "POST" && request.path === "/api/client/artifacts" + ); + assert.ok(createRequest, "expected canonical artifact create request"); + assert.equal(createRequest.body.entity_type, "initiative"); + assert.equal(createRequest.body.entity_id, INITIATIVE_ID); + assert.equal(createRequest.body.name, "OrgX MCP error audit"); + assert.equal(createRequest.body.artifact_type, "engineering.report"); + assert.equal( + createRequest.body.metadata.compatibility_alias, + "orgx_create_entity type=artifact" + ); + assert.equal(createRequest.body.metadata.preferred_tool, "orgx_register_artifact"); +});