feat(desktop): guard the single definition of agent identity

desktop/package.json

@@ -11,8 +11,9 @@
     "check:file-sizes": "node ./scripts/check-file-sizes.mjs",
     "check:px-text": "node ./scripts/check-px-text.mjs",
     "check:pubkey-truncation": "node ./scripts/check-pubkey-truncation.mjs",
+    "check:agent-identity": "node ./scripts/check-agent-identity.mjs",
     "lint": "biome lint .",
-    "check": "biome check . && pnpm check:file-sizes && pnpm check:px-text && pnpm check:pubkey-truncation",
+    "check": "biome check . && pnpm check:file-sizes && pnpm check:px-text && pnpm check:pubkey-truncation && pnpm check:agent-identity",
     "format": "biome format --write .",
     "test": "node --import ./test-loader.mjs --experimental-strip-types --test \"src/**/*.test.mjs\"",
     "preview": "vite preview",

desktop/scripts/check-agent-identity.mjs

@@ -0,0 +1,158 @@
+import { promises as fs } from "node:fs";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+
+/**
+ * Guards the single definition of agent identity.
+ *
+ * Two surfaces answer "which agents exist" — @-mention autocomplete and the
+ * Agents library — and each once hand-rolled its own key. They drifted, and the
+ * drift was invisible until an agent became unreachable on one of them:
+ * autocomplete keyed on the pubkey (#5202) while the library still grouped by
+ * `personaId`, so renaming an instance made it vanish from the library
+ * entirely. Nothing failed loudly; a card simply stopped existing.
+ *
+ * The invariant that prevents a third answer appearing: an agent identity or
+ * display-group key is minted in exactly ONE module,
+ * `src/features/agents/lib/agentIdentity.ts`. Everywhere else imports it.
+ *
+ * So this flags a string or template literal that *begins* an identity
+ * namespace — `pubkey:` or `persona:` — anywhere outside that module. Those
+ * two prefixes are the wire format `agentIdentityKey` and
+ * `agentDisplayGroupKey` produce; a literal starting with one is either a
+ * second implementation or something close enough to become one.
+ *
+ * Deliberately narrow. A guard that cries wolf gets disabled, so it does not
+ * try to catch every possible way of grouping agents — only the shape that
+ * actually caused the outage.
+ *
+ * It has no exceptions. The two legitimate non-identity uses of a similar
+ * prefix carry namespaces of their own instead — `catalog-persona:` for the
+ * persona catalog dialog's selection token, `profile:` for the profile panel's
+ * render key — so neither looks like an agent identity to this guard or to a
+ * reader. Prefer that route over an allowlist entry; see `overrides`.
+ */
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const projectRoot = path.resolve(__dirname, "..");
+
+/** The one module allowed to mint these keys. */
+const CANONICAL_MODULE = "src/features/agents/lib/agentIdentity.ts";
+
+const SCAN_ROOT = "src";
+const EXTENSIONS = new Set([".ts", ".tsx"]);
+
+// A quote or backtick immediately followed by an identity namespace, plus the
+// first interpolation or word after it. Matching the opening delimiter keeps
+// `builtin:fizz`, `"persona"`, and prose containing "persona:" from tripping
+// it; capturing what FOLLOWS the namespace is what makes an allowlist entry
+// specific to one key rather than to the whole file (see `overrides`).
+const IDENTITY_KEY_RE = /[`"'](?:pubkey|persona):(?:\$\{[^}]*\}|[\w.-]*)/g;
+
+/**
+ * Allowlisted `relativePath:matchedLiteral` pairs.
+ *
+ * **Empty on purpose, and worth keeping that way.** The first version of this
+ * guard carried four entries and matched only the bare `persona:` prefix, so
+ * each entry exempted *every* occurrence of that prefix in the file — and the
+ * exempted files were the agent-adjacent ones, i.e. exactly the code most
+ * likely to drift. The guard was theatre over its highest-risk surface.
+ *
+ * The fix was not a tighter allowlist but removing the need for one: the two
+ * legitimate non-identity uses now carry their own namespaces
+ * (`catalog-persona:` for the persona catalog dialog's selection token,
+ * `profile:` for the profile panel's render key), so neither looks like an
+ * agent identity to this guard or to a reader.
+ *
+ * Prefer that route. If an entry is ever genuinely unavoidable, note that the
+ * key now includes the text after the namespace, so it scopes to one literal —
+ * but a namespace of its own is still the better answer.
+ */
+const overrides = new Set([]);
+
+/**
+ * Whether a line is wholly a comment. Deliberately a heuristic over the common
+ * shapes (`//`, `/*`, and the `*` continuation of a block comment) rather than
+ * a parser: a false negative here just means the allowlist earns an entry,
+ * while a parser would be far more machinery than this guard is worth.
+ */
+function isCommentLine(line) {
+  const trimmed = line.trimStart();
+  return (
+    trimmed.startsWith("//") ||
+    trimmed.startsWith("/*") ||
+    trimmed.startsWith("*")
+  );
+}
+
+async function walkFiles(directory) {
+  const entries = await fs.readdir(directory, { withFileTypes: true });
+  const files = await Promise.all(
+    entries.map(async (entry) => {
+      const fullPath = path.join(directory, entry.name);
+      return entry.isDirectory() ? walkFiles(fullPath) : [fullPath];
+    }),
+  );
+  return files.flat();
+}
+
+const scanDirectory = path.join(projectRoot, SCAN_ROOT);
+const candidateFiles = await fs
+  .access(scanDirectory)
+  .then(() => walkFiles(scanDirectory))
+  .catch(() => []);
+
+const violations = [];
+
+for (const filePath of candidateFiles) {
+  // Override keys and the canonical path are authored with `/`, but
+  // path.relative yields `\` on Windows — compare in posix form or this
+  // silently matches nothing.
+  const relativePath = path
+    .relative(projectRoot, filePath)
+    .split(path.sep)
+    .join("/");
+
+  if (!EXTENSIONS.has(path.extname(relativePath))) {
+    continue;
+  }
+  if (relativePath === CANONICAL_MODULE) {
+    continue;
+  }
+
+  const content = await fs.readFile(filePath, "utf8");
+  content.split(/\r?\n/).forEach((line, index) => {
+    // Prose describing a key format is not a second implementation of one.
+    // Unrelated subsystems document their own scope keys (e.g. the channel
+    // storage scope `"pubkey:normalizedRelayUrl"`), and flagging a comment
+    // teaches people to silence the guard rather than read it.
+    if (isCommentLine(line)) {
+      return;
+    }
+    for (const match of line.match(IDENTITY_KEY_RE) ?? []) {
+      if (!overrides.has(`${relativePath}:${match}`)) {
+        violations.push({ relativePath, lineNumber: index + 1, match });
+      }
+    }
+  });
+}
+
+if (violations.length > 0) {
+  console.error("Desktop agent-identity check failed:");
+  for (const violation of violations) {
+    console.error(
+      `- ${violation.relativePath}:${violation.lineNumber}: ${violation.match}`,
+    );
+  }
+  console.error(
+    `Agent identity is minted in one place: \`${CANONICAL_MODULE}\`. ` +
+      "Import `agentIdentityKey` (identity — the pubkey) or " +
+      "`agentDisplayGroupKey` (presentation — which agents may share one card) " +
+      "instead of building the key here. Two surfaces that answer " +
+      '"which agents exist" differently is how a renamed agent silently ' +
+      "disappeared from the Agents library. If this literal is genuinely not " +
+      "an agent identity, add a narrowly scoped `relativePath:matchedLiteral` " +
+      "exception, with a reason, in `desktop/scripts/check-agent-identity.mjs`.",
+  );
+  process.exit(1);
+}

desktop/src/features/agents/lib/agentAutocompleteEligibility.ts

@@ -1,3 +1,4 @@
+import { agentIdentityKey } from "@/features/agents/lib/agentIdentity";
 import type { Channel, RelayAgent } from "@/shared/api/types";
 import { normalizePubkey } from "@/shared/lib/pubkey";
 
@@ -285,16 +286,13 @@ type AgentAutocompleteCandidate = {
   personaId?: string | null;
 };
 
-function agentIdentityKey<T extends AgentAutocompleteCandidate>(candidate: T) {
-  if (candidate.isAgent !== true || !candidate.pubkey) {
-    return null;
-  }
-
-  // Pubkeys—not persona metadata or a display name—are agent identities.
-  // A persona may be installed more than once, and an owner may intentionally
-  // create multiple same-named agents. Collapsing either case makes one agent
-  // impossible to choose from autocomplete.
-  return `pubkey:${normalizePubkey(candidate.pubkey)}`;
+function agentAutocompleteIdentityKey<T extends AgentAutocompleteCandidate>(
+  candidate: T,
+) {
+  // Only agents coalesce; two humans may legitimately share every other field.
+  // The identity itself comes from `agentIdentityKey` so this surface and the
+  // Agents library cannot drift into two different answers for "same agent?".
+  return candidate.isAgent === true ? agentIdentityKey(candidate) : null;
 }
 
 function agentCandidateRank<T extends AgentAutocompleteCandidate>(
@@ -369,7 +367,7 @@ export function coalesceAgentAutocompleteCandidates<
   const indexesByKey = new Map<string, number>();
 
   for (const candidate of candidates) {
-    const key = agentIdentityKey(candidate);
+    const key = agentAutocompleteIdentityKey(candidate);
     if (!key) {
       output.push(candidate);
       continue;

desktop/src/features/agents/lib/agentIdentity.test.mjs

@@ -0,0 +1,138 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import {
+  agentDisplayGroupKey,
+  agentIdentityKey,
+  groupAgentsForDisplay,
+} from "./agentIdentity.ts";
+
+const PUBKEY_A = "a".repeat(64);
+const PUBKEY_B = "b".repeat(64);
+
+test("agent identity is the pubkey, never persona metadata or a name", () => {
+  const left = {
+    pubkey: PUBKEY_A,
+    name: "Bumble",
+    personaId: "builtin:bumble",
+  };
+  const right = {
+    pubkey: PUBKEY_B,
+    name: "Bumble",
+    personaId: "builtin:bumble",
+  };
+
+  assert.notEqual(agentIdentityKey(left), agentIdentityKey(right));
+  assert.equal(
+    agentIdentityKey({ pubkey: ` ${PUBKEY_A.toUpperCase()} ` }),
+    agentIdentityKey({ pubkey: PUBKEY_A, personaId: "something-else" }),
+  );
+  assert.equal(agentIdentityKey({ pubkey: null }), null);
+  assert.equal(agentIdentityKey({}), null);
+});
+
+test("the display group key separates renamed instances of one persona", () => {
+  const claude = {
+    pubkey: PUBKEY_A,
+    name: "Claude",
+    personaId: "builtin:fizz",
+  };
+  const fizz = { pubkey: PUBKEY_B, name: "Fizz", personaId: "builtin:fizz" };
+
+  assert.notEqual(agentDisplayGroupKey(claude), agentDisplayGroupKey(fizz));
+  assert.equal(
+    agentDisplayGroupKey(claude),
+    agentDisplayGroupKey({ ...claude, pubkey: PUBKEY_B, name: " claude " }),
+  );
+  assert.notEqual(
+    agentDisplayGroupKey(claude),
+    agentDisplayGroupKey({ ...claude, personaId: "builtin:honey" }),
+  );
+});
+
+test("a name is folded to NFC, so one fleet does not split on encoding", () => {
+  // macOS input methods and file systems commonly emit NFD, Windows emits NFC.
+  // The same name typed on two machines must land on one card.
+  const precomposed = "José"; // é as U+00E9
+  const decomposed = "José"; // e + U+0301 combining acute
+
+  assert.notEqual(precomposed, decomposed, "the inputs really do differ");
+  assert.equal(
+    agentDisplayGroupKey({ personaId: "builtin:fizz", name: precomposed }),
+    agentDisplayGroupKey({ personaId: "builtin:fizz", name: decomposed }),
+  );
+
+  const groups = groupAgentsForDisplay([
+    { pubkey: PUBKEY_A, name: precomposed, personaId: "builtin:fizz" },
+    { pubkey: PUBKEY_B, name: decomposed, personaId: "builtin:fizz" },
+  ]);
+
+  assert.equal(
+    groups.length,
+    1,
+    "two encodings of one name must not render two identical-looking cards",
+  );
+  assert.equal(groups[0].agents.length, 2);
+});
+
+test("the group key cannot be forged by a name containing a separator", () => {
+  // Segments are length-prefixed. With a plain `|` join both of these render
+  // `persona:a|name:x|name:y`, silently merging two different agents onto one
+  // card and leaving one of them unopenable.
+  assert.notEqual(
+    agentDisplayGroupKey({ personaId: "a", name: "x|name:y" }),
+    agentDisplayGroupKey({ personaId: "a|name:x", name: "y" }),
+  );
+  assert.notEqual(
+    agentDisplayGroupKey({ personaId: "builtin:fizz", name: "a:b" }),
+    agentDisplayGroupKey({ personaId: "builtin:fizz:a", name: "b" }),
+  );
+  // A separator in a name is still just a name — same input, same key.
+  assert.equal(
+    agentDisplayGroupKey({ personaId: "a", name: "x|name:y" }),
+    agentDisplayGroupKey({ personaId: "a", name: " X|NAME:Y " }),
+  );
+});
+
+test("unnamed instances of one persona share a card — documented, not accidental", () => {
+  // "", "   ", null and undefined all fold to the same empty name, so several
+  // unnamed instances of one persona collapse onto the persona's card. They
+  // stay reachable through that card's profile panel, which lists every
+  // instance behind it. Asserted so a future change to the fold has to decide
+  // this deliberately rather than discover it.
+  const groups = groupAgentsForDisplay([
+    { pubkey: PUBKEY_A, name: "", personaId: "builtin:fizz" },
+    { pubkey: PUBKEY_B, name: "   ", personaId: "builtin:fizz" },
+    { pubkey: "c".repeat(64), name: null, personaId: "builtin:fizz" },
+    { pubkey: "d".repeat(64), name: "Fizz", personaId: "builtin:fizz" },
+  ]);
+
+  assert.deepEqual(
+    groups.map((group) => group.name),
+    ["", "Fizz"],
+  );
+  assert.equal(groups[0].agents.length, 3, "no unnamed instance is dropped");
+});
+
+test("display grouping keeps every distinct identity and drops repeats", () => {
+  const agents = [
+    { pubkey: PUBKEY_A, name: "Claude", personaId: "builtin:fizz" },
+    { pubkey: PUBKEY_B, name: "Fizz", personaId: "builtin:fizz" },
+    { pubkey: PUBKEY_A, name: "Claude", personaId: "builtin:fizz" },
+  ];
+
+  const groups = groupAgentsForDisplay(agents);
+
+  assert.deepEqual(
+    groups.map((group) => group.name),
+    ["Claude", "Fizz"],
+  );
+  assert.deepEqual(
+    new Set(groups.flatMap((group) => group.agents).map(agentIdentityKey)),
+    new Set([
+      agentIdentityKey({ pubkey: PUBKEY_A }),
+      agentIdentityKey(agents[1]),
+    ]),
+  );
+  assert.equal(groups[0].agents.length, 1);
+});