CL-6471: never leak a run id as a participant's name or greeting

packages/agent-directory/src/client.ts

@@ -7,87 +7,33 @@
 // to identify that plumbing; a host injects only its raw definition/
 // instance lists, already fetched from wherever it gets them.
 
-import { type } from "arktype";
 import { isWorkbenchHostDefinitionName } from "@corbits/chat/workbench-host-naming";
+import {
+  deriveDisplayName,
+  humanizeSlug,
+  withDisplayName,
+  withDisplayNames,
+  type WithDisplayName,
+} from "@corbits/chat/display-name";
+
+// `deriveDisplayName`/`humanizeSlug` (CL-6413) live in `@corbits/chat`
+// itself, not here: this package already depends on `@corbits/chat` (for
+// `isWorkbenchHostDefinitionName` below), so a copy defined here could
+// never be imported back by `@corbits/chat`'s own call sites without a
+// circular dependency — exactly the gap CL-6471 traces the "Run
+// 737a058d…" leak to (the chat participant invite/greeting path never
+// migrated onto this derivation because it couldn't). Re-exported here so
+// every existing caller of `@corbits/agent-directory/client`'s
+// `deriveDisplayName`/`humanizeSlug` keeps working unchanged.
+export { deriveDisplayName, humanizeSlug, withDisplayName, withDisplayNames };
+export type { WithDisplayName };
 
 export type UserFacingAgentDefinition = {
   readonly id: string;
   readonly name: string;
   readonly description?: string | null;
 };
 
-/**
- * A definition's kebab `name` is its immutable, URL-facing identifier
- * (CL-6413) — the mail handle `createAgentDefinitionCore` binds it to
- * (`@corbits/agent-directory/agent-workflow`'s `input.handle`). The
- * person-facing display name it was created with lands one hop away, on
- * `description`: that same handler seeds `workflowDefinition.description`
- * from the asset's own `displayName`
- * (`vendor/intx/hub-sessions/src/workflow-definition-ensure.ts`), so
- * `deriveDisplayName` reads it from there. A definition created before
- * that seeding existed, or with no description ever set, has no display
- * name to read — `humanizeSlug` backfills one from the identifier itself
- * rather than showing the raw slug as if it were a name.
- */
-const DisplayNameSource = type({
-  name: "string",
-  "description?": "string | null",
-});
-
-/** kebab-case identifier -> Title Case words: `"research-analyst"` ->
- * `"Research Analyst"`. Words that aren't hyphen-separated (a name that
- * already reads as prose) pass through with only their case fixed up, so
- * this is safe to run over a definition's raw `name` unconditionally. */
-export function humanizeSlug(slug: string): string {
-  return slug
-    .split(/[-_\s]+/)
-    .filter((word) => word.length > 0)
-    .map((word) => word.slice(0, 1).toUpperCase() + word.slice(1))
-    .join(" ");
-}
-
-/**
- * The display name a definition should render as: its own description
- * when one was set at creation, otherwise a humanized reading of its
- * immutable slug. A whitespace-only description reads as absent — never
- * a blank display name — since it carries nothing a person actually
- * typed. Throws on a shape that isn't at least `{ name }` — this is a
- * trust boundary, not a formatting helper, so a malformed record fails
- * loudly rather than rendering "undefined".
- */
-export function deriveDisplayName(definition: {
-  readonly name: string;
-  readonly description?: string | null;
-}): string {
-  const parsed = DisplayNameSource(definition);
-  if (parsed instanceof type.errors) {
-    throw new Error(
-      `deriveDisplayName: invalid agent definition: ${parsed.summary}`,
-    );
-  }
-  const description = parsed.description?.trim();
-  return description !== undefined && description !== ""
-    ? description
-    : humanizeSlug(parsed.name);
-}
-
-export type WithDisplayName<T> = T & { readonly displayName: string };
-
-/** Projects `deriveDisplayName` onto a definition, keeping every other
- * field untouched — the read-boundary derivation the ticket calls for,
- * done once here rather than as scattered `??` fallbacks in UI code. */
-export function withDisplayName<T extends UserFacingAgentDefinition>(
-  definition: T,
-): WithDisplayName<T> {
-  return { ...definition, displayName: deriveDisplayName(definition) };
-}
-
-export function withDisplayNames<T extends UserFacingAgentDefinition>(
-  definitions: readonly T[],
-): readonly WithDisplayName<T>[] {
-  return definitions.map(withDisplayName);
-}
-
 export type UserFacingAgentInstance = {
   readonly id: string;
   readonly definitionId: string;

packages/chat/package.json

@@ -15,7 +15,9 @@
     "./stream-events": "./src/stream-events.ts",
     "./blocks": "./src/blocks.ts",
     "./agent-address": "./src/agent-address.ts",
-    "./workbench-host-naming": "./src/workbench-host-naming.ts"
+    "./workbench-host-naming": "./src/workbench-host-naming.ts",
+    "./display-name": "./src/display-name.ts",
+    "./id-leak-guard": "./src/id-leak-guard.ts"
   },
   "scripts": {
     "typecheck": "tsc --noEmit",

packages/chat/src/display-name.test.ts

@@ -0,0 +1,55 @@
+import { describe, expect, test } from "bun:test";
+import { deriveDisplayName, humanizeSlug } from "./display-name";
+
+describe("humanizeSlug", () => {
+  test("title-cases a hyphenated slug", () => {
+    expect(humanizeSlug("research-analyst")).toBe("Research Analyst");
+  });
+
+  test("title-cases an underscored slug", () => {
+    expect(humanizeSlug("architecture_reviewer")).toBe("Architecture Reviewer");
+  });
+
+  test("passes prose through with only case fixed up", () => {
+    expect(humanizeSlug("myra")).toBe("Myra");
+  });
+});
+
+describe("deriveDisplayName", () => {
+  test("prefers a non-blank description over the slug", () => {
+    expect(
+      deriveDisplayName({
+        name: "architecture-reviewer",
+        description: "Architecture reviewer",
+      }),
+    ).toBe("Architecture reviewer");
+  });
+
+  test("humanizes the slug when description is absent or blank", () => {
+    expect(deriveDisplayName({ name: "architecture-reviewer" })).toBe(
+      "Architecture Reviewer",
+    );
+    expect(
+      deriveDisplayName({ name: "architecture-reviewer", description: "   " }),
+    ).toBe("Architecture Reviewer");
+  });
+
+  test("throws rather than humanizing an internal run id into a fake name (CL-6471)", () => {
+    expect(() =>
+      deriveDisplayName({ name: "run_737a058d48006e2bde12559576f422e0" }),
+    ).toThrow(/internal identifier/);
+  });
+
+  test("throws when the description itself is an internal id", () => {
+    expect(() =>
+      deriveDisplayName({
+        name: "architecture-reviewer",
+        description: "run_737a058d48006e2bde12559576f422e0",
+      }),
+    ).toThrow(/internal identifier/);
+  });
+
+  test('throws on a malformed shape rather than rendering "undefined"', () => {
+    expect(() => deriveDisplayName({} as { name: string })).toThrow();
+  });
+});

packages/chat/src/display-name.ts

@@ -0,0 +1,102 @@
+// A workflow definition's person-facing display name, derived once here
+// so every caller that decides "what does this agent look like to a
+// person" reads it the same way — never a scattered `description ?? name`
+// (or worse, a raw address/run id) reimplemented per call site.
+//
+// Lives in `@corbits/chat` rather than `@corbits/agent-directory` (which
+// originated this logic, CL-6413) because `@corbits/agent-directory`
+// itself depends on `@corbits/chat` (`workbench-host-naming`); a reverse
+// dependency would be circular. `@corbits/agent-directory/client.ts`
+// re-exports `deriveDisplayName`/`humanizeSlug` from here for its existing
+// callers rather than keeping a second copy that could drift.
+import { type } from "arktype";
+import { ID_LEAK_PATTERN } from "./id-leak-guard";
+
+const DisplayNameSource = type({
+  name: "string",
+  "description?": "string | null",
+});
+
+/** kebab-case identifier -> Title Case words: `"research-analyst"` ->
+ * `"Research Analyst"`. Words that aren't hyphen-separated (a name that
+ * already reads as prose) pass through with only their case fixed up, so
+ * this is safe to run over a definition's raw `name` unconditionally —
+ * PROVIDED that `name` is never an internal id in disguise; see
+ * `deriveDisplayName`'s own guard for why the raw run-id case never
+ * reaches this function at all. */
+export function humanizeSlug(slug: string): string {
+  return slug
+    .split(/[-_\s]+/)
+    .filter((word) => word.length > 0)
+    .map((word) => word.slice(0, 1).toUpperCase() + word.slice(1))
+    .join(" ");
+}
+
+/**
+ * The display name a definition should render as: its own description
+ * when one was set at creation, otherwise a humanized reading of its
+ * immutable slug. A whitespace-only description reads as absent — never
+ * a blank display name — since it carries nothing a person actually
+ * typed. Throws on a shape that isn't at least `{ name }` — this is a
+ * trust boundary, not a formatting helper, so a malformed record fails
+ * loudly rather than rendering "undefined".
+ *
+ * Also throws when `name` is itself an internal-id shape (`run_…`,
+ * `wfd_…`, …, see `./id-leak-guard`) — the product rule is that a person
+ * never sees an internal identifier, so a caller that reaches this
+ * function with a run id where a definition's slug belongs gets a loud
+ * failure instead of a Title-Cased leak like "Run 737a058d…" (CL-6471).
+ */
+export function deriveDisplayName(definition: {
+  readonly name: string;
+  readonly description?: string | null;
+}): string {
+  const parsed = DisplayNameSource(definition);
+  if (parsed instanceof type.errors) {
+    throw new Error(
+      `deriveDisplayName: invalid agent definition: ${parsed.summary}`,
+    );
+  }
+  const description = parsed.description?.trim();
+  if (description !== undefined && description !== "") {
+    if (ID_LEAK_PATTERN.test(description)) {
+      throw new Error(
+        `deriveDisplayName: description "${description}" for definition ` +
+          `"${parsed.name}" carries an internal identifier; refusing to ` +
+          "render it as a display name",
+      );
+    }
+    return description;
+  }
+  if (ID_LEAK_PATTERN.test(parsed.name)) {
+    throw new Error(
+      `deriveDisplayName: definition name "${parsed.name}" is an internal ` +
+        "identifier, not a slug; refusing to humanize it into a fake " +
+        "display name",
+    );
+  }
+  return humanizeSlug(parsed.name);
+}
+
+export type UserFacingAgentDefinition = {
+  readonly id: string;
+  readonly name: string;
+  readonly description?: string | null;
+};
+
+export type WithDisplayName<T> = T & { readonly displayName: string };
+
+/** Projects `deriveDisplayName` onto a definition, keeping every other
+ * field untouched — the read-boundary derivation done once here rather
+ * than as scattered `??` fallbacks in UI code. */
+export function withDisplayName<T extends UserFacingAgentDefinition>(
+  definition: T,
+): WithDisplayName<T> {
+  return { ...definition, displayName: deriveDisplayName(definition) };
+}
+
+export function withDisplayNames<T extends UserFacingAgentDefinition>(
+  definitions: readonly T[],
+): readonly WithDisplayName<T>[] {
+  return definitions.map(withDisplayName);
+}

packages/chat/src/id-leak-guard.test.ts

@@ -0,0 +1,60 @@
+// CL-6471's systemic guard: no user-visible string may carry an internal
+// identifier, in raw or humanized ("Run 737a058d…") form.
+import { describe, expect, test } from "bun:test";
+import { assertNoLeakedInternalId } from "./id-leak-guard";
+
+describe("assertNoLeakedInternalId", () => {
+  test("passes real, human-authored text through untouched", () => {
+    expect(() =>
+      assertNoLeakedInternalId("Architecture reviewer", "a display name"),
+    ).not.toThrow();
+    expect(() =>
+      assertNoLeakedInternalId(
+        "Hi Alice, I'm Myra — your teammate here.",
+        "a greeting",
+      ),
+    ).not.toThrow();
+  });
+
+  test("catches a raw internal id for every named prefix", () => {
+    const ids = [
+      "run_737a058d48006e2bde12559576f422e0",
+      "wfd_737a058d48006e2bde12559576f422e0",
+      "tnt_737a058d48006e2bde12559576f422e0",
+      "prn_737a058d48006e2bde12559576f422e0",
+      "ast_737a058d48006e2bde12559576f422e0",
+      "gtk_737a058d48006e2bde12559576f422e0",
+    ];
+    for (const id of ids) {
+      expect(() => assertNoLeakedInternalId(id, "a name")).toThrow(
+        /internal identifier/,
+      );
+    }
+  });
+
+  test("catches the humanized (Title Cased) form the same id renders as once split", () => {
+    // `humanizeSlug`'s exact transform: "run_737a058d..." -> "Run 737a058d..."
+    expect(() =>
+      assertNoLeakedInternalId(
+        "Run 737a058d48006e2bde12559576f422e0",
+        "a participant name",
+      ),
+    ).toThrow(/internal identifier/);
+  });
+
+  test("catches an id leaked mid-sentence, as a greeting would carry it", () => {
+    expect(() =>
+      assertNoLeakedInternalId(
+        "Hi Alice, I'm run_737a058d48006e2bde12559576f422e0. Three reviewers read every pull request.",
+        "a greeting",
+      ),
+    ).toThrow(/internal identifier/);
+  });
+
+  test("never flags a short, coincidental substring match", () => {
+    // "runner" starts with "run" but not the "run_" id prefix + hex tail.
+    expect(() =>
+      assertNoLeakedInternalId("Runner McRunface", "a display name"),
+    ).not.toThrow();
+  });
+});

packages/chat/src/id-leak-guard.ts

@@ -0,0 +1,44 @@
+// The systemic guard CL-6471 calls for: a person never sees an internal
+// identifier — not as a participant's name, not in a join/system line,
+// not in a greeting, not in a chat title. This has recurred repeatedly
+// (a raw definitionId/runId in insights, a `writing_systems` id in a
+// Myra reply, and CL-6471's own "Run 737a058d…" / "I'm run_737a…") each
+// time as a one-off display-time patch; this module is the one place
+// every id-generating prefix is named, so a new leak is a missed test
+// run rather than a missed grep.
+//
+// Prefix words mirror `@intx/hub-common`'s `generateId` (vendored;
+// upstream source: `packages/hub-common/src/ids.ts`'s `PREFIXES`) for
+// the six kinds CL-6471 names explicitly — a workflow run, a workflow
+// definition, a tenant, a principal, an asset, and a git token — each
+// normally followed by `_` and 32 lowercase hex characters (`run_737a…`).
+//
+// Matched with either `_`, a space, or `-` as the separator, not just
+// `_`: `humanizeSlug`'s Title Case reading of a leaked id (CL-6471's own
+// "Run 737a058d48006e2bde12559576f422e0") replaces the underscore with a
+// space, so the raw and the humanized-leak forms both need to trip this.
+const ID_PREFIX_WORDS = ["run", "wfd", "tnt", "prn", "ast", "gtk"] as const;
+
+export const ID_LEAK_PATTERN = new RegExp(
+  `\\b(?:${ID_PREFIX_WORDS.join("|")})[_\\s-][0-9a-f]{16,}\\b`,
+  "i",
+);
+
+/**
+ * Throws when `value` carries an internal identifier — a raw id, or a
+ * humanized reading of one (`"Run 737a058d…"`, produced by title-casing
+ * `run_737a058d…`'s underscore split). The humanized case is why this
+ * checks the ORIGINAL prefixes case-insensitively rather than requiring
+ * the literal lowercase prefix: `humanizeSlug` capitalizes the first
+ * letter of the leading word, so a leaked run id renders as "Run …", not
+ * "run …", by the time it would reach a person.
+ */
+export function assertNoLeakedInternalId(value: string, context: string): void {
+  if (ID_LEAK_PATTERN.test(value)) {
+    throw new Error(
+      `${context} carries an internal identifier ("${value}"); a person ` +
+        "must never see one — resolve the real name at the source instead " +
+        "of rendering the id (CL-6471)",
+    );
+  }
+}

packages/chat/src/platform-adapter.ts

@@ -666,6 +666,16 @@ export function createHubChatPlatform(
       return row?.assetId ?? undefined;
     },
 
+    async resolveDefinitionNameSource(definitionId) {
+      const row = await deps.db.query.workflowDefinition.findFirst({
+        where: eq(workflowDefinition.id, definitionId),
+      });
+      if (row === undefined) return undefined;
+      return typeof row.description === "string" && row.description !== ""
+        ? { name: row.name, description: row.description }
+        : { name: row.name };
+    },
+
     async refreshAgentInstanceFromDefinition(
       tenantId,
       _workbenchId,