From 1f0cdbd366021c3eb19eea756c8c2d12b38e48d4 Mon Sep 17 00:00:00 2001 From: Ramnique Singh <30795890+ramnique@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:54:43 +0530 Subject: [PATCH] refactor(x): single live homes for getErrorDetails and isPathInside MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getErrorDetails was the last live import out of runtime/legacy/ that isn't part of the code-mode / mini-apps carve-out — four knowledge pipelines pulled it from legacy/utils.ts, blocking 'legacy deletes as a unit'. Their agents run via the headless runner now, so the RunFailedError unwrap branch was dead for them; the live version in application/lib/errors.ts is the plain Error->message idiom, and the legacy copy (with the RunFailedError coupling) is deleted as consumerless. isPathInside backs path-grant permission decisions, and byte-identical copies lived in filesystem/files.ts, assembly/permission-metadata.ts, and legacy/repo.ts — a place for a future divergence to become a permission bypass in exactly one caller. files.ts now exports the one live copy; permission-metadata imports it (it already imports files.ts, so no new edge). legacy/repo.ts deliberately keeps its frozen private copy: the quarantine must not import live modules. Co-Authored-By: Claude Fable 5 --- apps/x/packages/core/src/application/lib/errors.ts | 7 +++++++ apps/x/packages/core/src/filesystem/files.ts | 6 +++++- apps/x/packages/core/src/knowledge/agent_notes.ts | 2 +- apps/x/packages/core/src/knowledge/build_graph.ts | 2 +- apps/x/packages/core/src/knowledge/label_emails.ts | 2 +- apps/x/packages/core/src/knowledge/tag_notes.ts | 2 +- .../core/src/runtime/assembly/permission-metadata.ts | 7 +------ apps/x/packages/core/src/runtime/legacy/utils.ts | 10 ---------- 8 files changed, 17 insertions(+), 21 deletions(-) create mode 100644 apps/x/packages/core/src/application/lib/errors.ts diff --git a/apps/x/packages/core/src/application/lib/errors.ts b/apps/x/packages/core/src/application/lib/errors.ts new file mode 100644 index 00000000..d5641d07 --- /dev/null +++ b/apps/x/packages/core/src/application/lib/errors.ts @@ -0,0 +1,7 @@ +// One home for the error -> human-readable message idiom. The legacy runs +// engine's getErrorDetails also unwrapped RunFailedError's multi-error list; +// live callers run agents through the headless runner, whose HeadlessRunError +// carries the failure detail in .message — so this is the whole contract. +export function getErrorDetails(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/apps/x/packages/core/src/filesystem/files.ts b/apps/x/packages/core/src/filesystem/files.ts index 85c634c9..3dc2efac 100644 --- a/apps/x/packages/core/src/filesystem/files.ts +++ b/apps/x/packages/core/src/filesystem/files.ts @@ -68,7 +68,11 @@ const MAX_BYTES_LABEL = `${MAX_BYTES / 1024} KB`; let knowledgeCommitTimer: ReturnType | null = null; let canonicalWorkspaceRoot: string | null = null; -function isPathInside(parent: string, child: string): boolean { +// Exported as the one live containment check: permission decisions +// (assembly/permission-metadata) key on it, so a divergent copy is a +// permission-bypass risk, not a style issue. (legacy/repo.ts keeps its own +// frozen copy — the quarantine must not import live modules.) +export function isPathInside(parent: string, child: string): boolean { const relative = path.relative(parent, child); return relative === '' || (!!relative && !relative.startsWith('..') && !path.isAbsolute(relative)); } diff --git a/apps/x/packages/core/src/knowledge/agent_notes.ts b/apps/x/packages/core/src/knowledge/agent_notes.ts index 0666f499..b3a037f0 100644 --- a/apps/x/packages/core/src/knowledge/agent_notes.ts +++ b/apps/x/packages/core/src/knowledge/agent_notes.ts @@ -4,7 +4,7 @@ import { google } from 'googleapis'; import { WorkDir } from '../config/config.js'; import { runWhenPossible } from '../runtime/assembly/headless-app.js'; import { getKgModel } from '../models/defaults.js'; -import { getErrorDetails } from '../runtime/legacy/utils.js'; +import { getErrorDetails } from '../application/lib/errors.js'; import { serviceLogger } from '../services/service_logger.js'; import { loadUserConfig, updateUserEmail } from '../config/user_config.js'; import { GoogleClientFactory } from './google-client-factory.js'; diff --git a/apps/x/packages/core/src/knowledge/build_graph.ts b/apps/x/packages/core/src/knowledge/build_graph.ts index f251f176..98d2c52e 100644 --- a/apps/x/packages/core/src/knowledge/build_graph.ts +++ b/apps/x/packages/core/src/knowledge/build_graph.ts @@ -3,7 +3,7 @@ import path from 'path'; import { WorkDir } from '../config/config.js'; import { getKgModel } from '../models/defaults.js'; import { runWhenPossible, toolInputPaths } from '../runtime/assembly/headless-app.js'; -import { getErrorDetails } from '../runtime/legacy/utils.js'; +import { getErrorDetails } from '../application/lib/errors.js'; import { serviceLogger, type ServiceRunContext } from '../services/service_logger.js'; import { loadState, diff --git a/apps/x/packages/core/src/knowledge/label_emails.ts b/apps/x/packages/core/src/knowledge/label_emails.ts index dfab477c..548c5b52 100644 --- a/apps/x/packages/core/src/knowledge/label_emails.ts +++ b/apps/x/packages/core/src/knowledge/label_emails.ts @@ -3,7 +3,7 @@ import path from 'path'; import { WorkDir } from '../config/config.js'; import { runWhenPossible, toolInputPaths } from '../runtime/assembly/headless-app.js'; import { getKgModel } from '../models/defaults.js'; -import { getErrorDetails } from '../runtime/legacy/utils.js'; +import { getErrorDetails } from '../application/lib/errors.js'; import { serviceLogger } from '../services/service_logger.js'; import { limitEventItems } from './limit_event_items.js'; import { diff --git a/apps/x/packages/core/src/knowledge/tag_notes.ts b/apps/x/packages/core/src/knowledge/tag_notes.ts index 61b18319..b7aefa23 100644 --- a/apps/x/packages/core/src/knowledge/tag_notes.ts +++ b/apps/x/packages/core/src/knowledge/tag_notes.ts @@ -3,7 +3,7 @@ import path from 'path'; import { WorkDir } from '../config/config.js'; import { runWhenPossible, toolInputPaths } from '../runtime/assembly/headless-app.js'; import { getKgModel } from '../models/defaults.js'; -import { getErrorDetails } from '../runtime/legacy/utils.js'; +import { getErrorDetails } from '../application/lib/errors.js'; import { serviceLogger } from '../services/service_logger.js'; import { limitEventItems } from './limit_event_items.js'; import { diff --git a/apps/x/packages/core/src/runtime/assembly/permission-metadata.ts b/apps/x/packages/core/src/runtime/assembly/permission-metadata.ts index 0b33bd15..103b7532 100644 --- a/apps/x/packages/core/src/runtime/assembly/permission-metadata.ts +++ b/apps/x/packages/core/src/runtime/assembly/permission-metadata.ts @@ -11,15 +11,10 @@ import { z } from "zod"; import { ToolPermissionMetadata } from "@x/shared/dist/runs.js"; import { isBlocked, extractCommandNames } from "../../application/lib/command-executor.js"; import { getFileAccessAllowList, type FileAccessGrant, type FileAccessOperation } from "../../config/security.js"; -import { resolveFilePathForPermission } from "../../filesystem/files.js"; +import { isPathInside, resolveFilePathForPermission } from "../../filesystem/files.js"; type ToolPermissionMetadataValue = z.infer; -function isPathInside(parent: string, child: string): boolean { - const relative = path.relative(parent, child); - return relative === '' || (!!relative && !relative.startsWith('..') && !path.isAbsolute(relative)); -} - function fileGrantCoversPath(grant: FileAccessGrant, operation: FileAccessOperation, resolvedPath: string): boolean { return grant.operation === operation && isPathInside(path.resolve(grant.pathPrefix), path.resolve(resolvedPath)); } diff --git a/apps/x/packages/core/src/runtime/legacy/utils.ts b/apps/x/packages/core/src/runtime/legacy/utils.ts index 8d526de1..a4569dc5 100644 --- a/apps/x/packages/core/src/runtime/legacy/utils.ts +++ b/apps/x/packages/core/src/runtime/legacy/utils.ts @@ -20,16 +20,6 @@ export class RunFailedError extends Error { } } -export function getErrorDetails(error: unknown): string { - if (error instanceof RunFailedError) { - return error.errors.join("\n\n"); - } - if (error instanceof Error) { - return error.message; - } - return String(error); -} - /** * Extract the assistant's final text response from a run's log. * @param runId