Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/loopover-contract/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// second. Two tools that happen to take the same fields today are usually a coincidence, and
// prematurely coupling them means a later divergence has to be un-shared under pressure.
import { z } from "zod";
import { MCP_TELEMETRY_ERROR_CODES } from "./telemetry.js";

/** The owner/repo pair virtually every repo-scoped tool takes. */
export const ownerRepoInput = z.object({
Expand Down Expand Up @@ -38,7 +39,10 @@ export const freshnessFields = {
export const toolErrorFields = {
error: z
.object({
code: z.string().min(1),
// #9659: the closed set, not free text. The doc above always said "drawn from a closed,
// developer-defined set so telemetry can break failures down by cause" -- while the type said
// `z.string()`, which is what let a server return a code telemetry then re-guessed differently.
code: z.enum(MCP_TELEMETRY_ERROR_CODES),
message: z.string().min(1),
})
.optional(),
Expand Down
20 changes: 20 additions & 0 deletions packages/loopover-contract/src/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ export const MCP_TELEMETRY_ERROR_CODES = [
"upstream_error",
"timeout",
"elicitation_declined",
// #9659: the miner's own envelope has always returned this to callers -- a local SQLite store that
// will not open. It belongs in the closed set so ONE code can serve both the envelope and the
// telemetry event, rather than the event re-deriving a different one from the message.
"store_unavailable",
"unknown_error",
] as const;
export type McpTelemetryErrorCode = (typeof MCP_TELEMETRY_ERROR_CODES)[number];
Expand Down Expand Up @@ -264,6 +268,22 @@ export function mcpToolSpanName(tool: string): string {
* Matches on shape and on the small set of messages the servers actually produce; everything else
* is `unknown_error` rather than a guess. Never reads a caller-supplied string into the code.
*/
/**
* The error envelope a tool's `structuredContent` carries, if it carries one (#9659).
*
* Every server reports failure the same way -- `isError: true` plus `{ error: { code, message } }` -- but
* each was reading that result differently, or not at all: the remote emitted a hardcoded
* `"unknown_error"`, the stdio one passed nothing to the classifier, and the miner passed the raw thrown
* error so the code was re-derived from message regexes and disagreed with the code the caller was given.
* Feed the result of this straight to `resolveErrorCode`, which validates the declared code against the
* closed set and falls back for anything else.
*/
export function toolErrorEnvelope(structuredContent: unknown): { code?: unknown; message?: unknown } | undefined {
if (typeof structuredContent !== "object" || structuredContent === null) return undefined;
const envelope = (structuredContent as { error?: unknown }).error;
return typeof envelope === "object" && envelope !== null ? (envelope as { code?: unknown; message?: unknown }) : undefined;
}

export function resolveErrorCode(error: unknown): McpTelemetryErrorCode {
const envelope = error as { code?: unknown } | null | undefined;
if (envelope && typeof envelope.code === "string") {
Expand Down
29 changes: 26 additions & 3 deletions packages/loopover-contract/src/tools/miner-ops.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// agent-safe control and is here instead.
import { z } from "zod";
import { defineTool } from "../tool-definition.js";
import { toolErrorFields } from "../shared.js";
import { INSTANCE_CHECK_STATUSES } from "../enums.js";

const RepoFullName = z.string().min(3).max(200).describe("owner/repo.");
Expand All @@ -32,6 +33,11 @@ export const MinerDoctorInput = z.object({});
export const MinerDoctorOutput = z.looseObject({
ok: z.boolean().describe("True when no check reported fail. Warnings do not clear it to false."),
checks: z.array(z.looseObject({ name: z.string(), status: z.enum(INSTANCE_CHECK_STATUSES), detail: z.string().optional() })),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerDoctorTool = defineTool({
Expand Down Expand Up @@ -59,6 +65,11 @@ export const MinerMetricsSnapshotOutput = z.looseObject({
samples: z.array(z.looseObject({ value: z.number(), labels: z.record(z.string(), z.string()).optional() })),
}),
),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerMetricsSnapshotTool = defineTool({
Expand Down Expand Up @@ -86,7 +97,11 @@ export const MinerGovernorActionOutput = z.looseObject({
blocked: z.boolean().optional().describe("True when the governor chokepoint refused the action."),
reason: z.string().optional(),
result: z.unknown().optional(),
error: z.string().optional(),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const MinerGovernorPauseInput = z.object({
Expand Down Expand Up @@ -216,7 +231,11 @@ export const MinerRunMigrationsOutput = z.looseObject({
result: z.unknown().optional(),
blocked: z.boolean().optional(),
reason: z.string().optional(),
error: z.string().optional(),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerRunMigrationsTool = defineTool({
Expand Down Expand Up @@ -248,7 +267,11 @@ export const MinerPurgeRepoOutput = z.looseObject({
result: z.unknown().optional(),
blocked: z.boolean().optional(),
reason: z.string().optional(),
error: z.string().optional(),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerPurgeRepoTool = defineTool({
Expand Down
56 changes: 56 additions & 0 deletions packages/loopover-contract/src/tools/miner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// gains is a description of what it already returns, not a new shape.
import { z } from "zod";
import { defineTool } from "../tool-definition.js";
import { toolErrorFields } from "../shared.js";
import { PLAN_STEP_STATUSES } from "../enums.js";

/** Statuses a portfolio-queue entry can hold. */
Expand All @@ -35,6 +36,11 @@ export const MinerPingInput = z.object({});
export const MinerPingOutput = z.looseObject({
status: z.literal("ok"),
tool: z.literal("loopover_miner_ping"),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerPingTool = defineTool({
Expand Down Expand Up @@ -69,6 +75,11 @@ export const MinerPortfolioDashboardOutput = z.looseObject({
}),
),
oldestQueuedAgeMs: z.number().nullable(),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerPortfolioDashboardTool = defineTool({
Expand Down Expand Up @@ -114,6 +125,11 @@ export const MinerManageStatusOutput = z.looseObject({
prs: z.array(manageStatusRowSchema),
}),
),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerManageStatusTool = defineTool({
Expand Down Expand Up @@ -149,6 +165,11 @@ export const MinerListClaimsOutput = z.looseObject({
note: z.string().nullish(),
}),
),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerListClaimsTool = defineTool({
Expand Down Expand Up @@ -187,6 +208,11 @@ export const MinerAuditFeedOutput = z.looseObject({
createdAt: z.string(),
}),
),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerAuditFeedTool = defineTool({
Expand Down Expand Up @@ -219,6 +245,11 @@ export const MinerGetRunStateOutput = z.looseObject({
repoFullName: z.string().optional(),
state: z.enum(MINER_RUN_STATES).nullable().optional(),
states: z.array(z.looseObject({ repoFullName: z.string(), state: z.enum(MINER_RUN_STATES).nullable() })).optional(),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerGetRunStateTool = defineTool({
Expand Down Expand Up @@ -275,6 +306,11 @@ export const MinerListPlansInput = z.object({

export const MinerListPlansOutput = z.looseObject({
plans: z.array(minerPlanRecordSchema),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerListPlansTool = defineTool({
Expand All @@ -300,6 +336,11 @@ export const MinerGetPlanOutput = z.looseObject({
planId: z.string().optional(),
found: z.boolean(),
plan: minerPlanRecordSchema.optional(),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerGetPlanTool = defineTool({
Expand Down Expand Up @@ -335,6 +376,11 @@ export const MinerGovernorDecisionsOutput = z.looseObject({
reason: z.string(),
}),
),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerGovernorDecisionsTool = defineTool({
Expand Down Expand Up @@ -371,6 +417,11 @@ export const MinerStatusOutput = z.looseObject({
}),
}),
doctor: z.array(z.looseObject({ name: z.string(), ok: z.boolean(), detail: z.string() })),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerStatusTool = defineTool({
Expand Down Expand Up @@ -410,6 +461,11 @@ export const MinerCalibrationReportOutput = z.looseObject({
}),
),
hasSignal: z.boolean(),
// #9659: every miner tool answers a store failure with the shared error envelope
// (`withMinerToolErrorHandling`), so the advertised schema declares it rather than describing only
// the success shape. `error.code` is the closed telemetry set, which is what lets the code the caller
// is told and the code telemetry records be the same one.
...toolErrorFields,
});

export const minerCalibrationReportTool = defineTool({
Expand Down
4 changes: 4 additions & 0 deletions packages/loopover-mcp/lib/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
MCP_TOOL_CALL_EVENT,
MCP_USAGE_EVENT,
resolveErrorCode,
toolErrorEnvelope,
toolExcludesPayloads,
UNKNOWN_TOOL_CATEGORY,
type McpTelemetryTransport,
Expand Down Expand Up @@ -131,6 +132,9 @@ export function wrapStdioToolHandler(
transport,
args: args[0],
result: result?.structuredContent,
// #9659: on the failure path this used to pass no error at all, so `resolveErrorCode(undefined)`
// classified every returned failure as `unknown_error` regardless of what the tool told the caller.
...(ok ? {} : { error: toolErrorEnvelope(result?.structuredContent) }),
});
return result;
} catch (error) {
Expand Down
43 changes: 36 additions & 7 deletions packages/loopover-miner/bin/loopover-miner-mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { readFileSync, realpathSync } from "node:fs";
import { fileURLToPath } from "node:url";
import { McpServer, type ToolCallback } from "@modelcontextprotocol/sdk/server/mcp.js";
import type { McpTelemetryErrorCode } from "@loopover/contract";
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
// #9536: every tool's schemas come from the shared contract instead of being declared here. The
// remote and stdio servers already register from the same package (#9517/#9518) -- this closes the
Expand Down Expand Up @@ -53,7 +54,7 @@ import { initGovernorLedger } from "../lib/governor-ledger.js";
import { collectStatus, runDoctorChecks } from "../lib/status.js";
import { collectMinerPredictionMetrics } from "@loopover/engine";
import { collectPredictionMetricRows } from "../lib/metrics-cli.js";
import { dispatchChatAction } from "../lib/chat-action-dispatch.js";
import { dispatchChatAction, type ChatActionRefusalStatus } from "../lib/chat-action-dispatch.js";
import {
MINER_CLAIM_RELEASE_ACTION,
MINER_DENY_HOOKS_DECIDE_ACTION,
Expand Down Expand Up @@ -86,9 +87,21 @@ import { captureMinerPostHogErrorAndFlush, initMinerPostHog } from "../lib/posth
* thrown values to this set rather than passing a caller/store-derived string through, matching the
* `code`/`message` shape @loopover/contract's shared `toolErrorFields` describes.
*/
type MinerToolErrorCode = "store_unavailable" | "unknown_error";

function toolErrorCode(error: unknown): MinerToolErrorCode {
/**
* Each way a dispatch can refuse, as the closed telemetry code for it (#9659).
*
* A `Record` over the status union rather than a lookup with a fallback: adding a dispatch status without
* deciding what it means to a caller then fails the build, which is the only reason a mapping between two
* closed vocabularies is safe to write down at all.
*/
const DISPATCH_REFUSAL_CODES: Record<ChatActionRefusalStatus, McpTelemetryErrorCode> = {
disabled: "not_configured",
unknown_action: "not_found",
invalid_params: "invalid_input",
handler_error: "upstream_error",
};

function toolErrorCode(error: unknown): McpTelemetryErrorCode {
// A local SQLite store failing to open (missing file, corrupted file, permissions) is the one
// failure mode every store-backed tool below can actually hit; anything else is unclassified.
return error instanceof Error && /not found|not a database|permission|ENOENT/i.test(error.message) ? "store_unavailable" : "unknown_error";
Expand Down Expand Up @@ -173,7 +186,10 @@ async function withMinerToolErrorHandling<T extends object>(
return payload;
} catch (error) {
const data = { error: { code: toolErrorCode(error), message: error instanceof Error ? error.message : String(error) } };
recordMinerDispatchTelemetry({ tool: toolName, ok: false, durationMs: Date.now() - startedAt, error });
// #9659: the ENVELOPE, not the raw error. Passing the raw one made telemetry re-derive a code from
// the message regexes, so a store that would not open told the caller `store_unavailable` while the
// event recorded `not_found` -- two classifications of one failure, from adjacent lines.
recordMinerDispatchTelemetry({ tool: toolName, ok: false, durationMs: Date.now() - startedAt, error, errorEnvelope: data.error });
return { ...minerToolResult(data), isError: true };
}
}
Expand Down Expand Up @@ -274,7 +290,11 @@ export interface MinerMcpServerOptions {
export function createMinerMcpServer(options: MinerMcpServerOptions = {}) {
const server = new McpServer({ name: "loopover-miner", version: ownPackageJson.version });

registerMinerTool(server, minerPingTool, async () => minerToolResult(MINER_PING_STATUS),
// #9658: through the wrapper, like the other twenty. Ping is the health check an operator's monitoring
// hits on a loop -- the cheapest signal that this server is alive and being used -- and it was the one
// registration that never reached the dispatch-telemetry chokepoint, so a `usage_event` breakdown by
// tool reported zero pings forever.
registerMinerTool(server, minerPingTool, () => withMinerToolErrorHandling(() => MINER_PING_STATUS, minerPingTool.name),
);

registerMinerTool(server, minerPortfolioDashboardTool, () =>
Expand Down Expand Up @@ -436,7 +456,16 @@ export function createMinerMcpServer(options: MinerMcpServerOptions = {}) {
if (result.ok) return { ok: true, action, result: (result as { result?: unknown }).result };
// A refusal is an ANSWER, not a transport failure: the caller needs to know the governor said no and
// why, which a thrown error would flatten into a generic tool error.
return { ok: false, action, blocked: true, reason: result.status, ...(result.error ? { error: result.error } : {}) };
return {
ok: false,
action,
blocked: true,
reason: result.status,
// #9659: the shared envelope, so one `error` field means one thing everywhere. `blocked` and
// `reason` still carry the refusal's own vocabulary; what used to be a bare string detail is now
// the envelope's `message`, under a code drawn from the same closed set telemetry records.
error: { code: DISPATCH_REFUSAL_CODES[result.status] ?? "unknown_error", message: typeof result.error === "string" ? result.error : result.status },
};
};

registerMinerTool(server, minerGovernorPauseTool, (input) => withMinerToolErrorHandling(() => dispatchResult(GOVERNOR_PAUSE_CHAT_ACTION, input.reason ? { reason: input.reason } : {}), minerGovernorPauseTool.name),
Expand Down
Loading