Skip to content

Adopt toolbelt.errors' describeError in factory, fleet, mcp, and run-core #1264

Description

@williamthorsen

Problem

error instanceof Error ? error.message : String(error) appears nine more times across factory, fleet, mcp, and run-core. Five are ad-hoc definitions of it under three names: errorMessage (run-core, twice), getErrorMessage (factory), and readMessage (fleet, twice).

Context

@williamthorsen/toolbelt.errors is published, and both kb (#1261) and agents (#1263) import describeError from its candidate tier rather than defining their own. These four packages adopt the same import, so the repo holds no local definition rather than one per package.

Sites that are supersets keep their extra branch and delegate only the fallback, as kb's extractGitErrorMessage does. Two such sites hold no bare ternary and are easily missed: fleet's readErrorText in packages/fleet/src/forge/github-adapter.ts, and the RunDataParseError branch in packages/run-core/scripts/pick-demo-runs.ts.

Factory holds three near-misses that test instanceof Error but fall back to a domain string ('Failed to load replay') or to an Error object rather than to String(error). They are a different construct and stay as they are.

With these four packages added, @williamthorsen/toolbelt.errors reaches six consumers, more than any other external dependency here, so it becomes the workspace's first catalog: entry rather than a sixth literal version. The other sixteen shared dependencies stay literal. Adopting the catalog on a dependency published in-house also settles cheaply whether nmr upgrade rewrites catalog: entries.

packages/mcp/bin/codeassembly-mcp.js keeps its copy. A top-level import resolves before any statement runs, so importing the helper there would replace the launcher's build-first message with ERR_MODULE_NOT_FOUND whenever the install is incomplete. packages/agents/bin/codeassembly.js is carved out for the same reason in #1263.

describeError diverges from the ternary only for an Error carrying an empty message and for a value String() cannot render, neither of which is reachable at these sites, so no site earns a new test.

Enforcement against reintroduction is tracked separately in #1282.

Acceptance criteria

Must have

  • pnpm-workspace.yaml carries @williamthorsen/toolbelt.errors in its catalog, and all six consumers -- agents, factory, fleet, kb, mcp, and run-core -- declare it as catalog:.
  • factory, fleet, mcp, and run-core define no message-extraction helper of their own.
  • No inline instance of the ternary remains in factory, fleet, mcp, or run-core, apart from packages/mcp/bin/codeassembly-mcp.js, which records the reason for its exemption inline.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactoringImprovement to code without change in functionalityscope:root

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions