fix(install): AgentInstallation is canonical for displayName (TASK-032)

[lilyshen0722]

Code half of TASK-032. @sam ruled the fork on 2026-09-02 (card 62404, reply 62427): installation is canonical; install.ts stops preferring curated botMetadata. The schema doc is #1097 — this is the change that doc names as owed.

What the ruling means in code

User.botMetadata.displayName is a seed for an installation that has no name — never a preference over one that does. Two sites in backend/routes/registry/install.ts had it as a preference:

effectiveDisplayName never consulted AgentInstallation at all. Precedence was explicit caller > User row > registry default, so a reinstall re-seeded the per-pod label from the shared User row and a name curated for this pod silently reverted. It is now explicit caller > this pod's existing installation > User-row seed > registry default. The installation term is the new one.

The intro-post label re-queried User.botMetadata and put it ahead of installation.displayName. That query is dropped. installation.displayName was resolved through the full chain a few lines above, so the second lookup could only disagree with the row just written — and dropping it removes one User query per install.

Task #62 / PR #408 is not reinstated

That regression (a registry default like "Cuz 🦞" landing on every member row, because the V2 member list reads AgentProfile first) stays fixed. The User-row seed still beats the registry default, so installing an existing identity into a new pod — where no installation exists — still writes "Aria" to both AgentInstallation.displayName and AgentProfile.name. Only the case where both are curated changes, and there the per-pod value wins, which is what the read paths already do (agentMessageService:1461, dmService:555, each with a comment saying they prefer the installation precisely to stop a sibling pod's name leaking through the shared User row). The install path was the one surface disagreeing with them.

The three existing tests in install.preserves-displayname.test.js are unchanged and green.

The reachable shape is reactivation, not install-twice

An active installation makes the route 400 (Agent already installed in this pod), so a second install only meets an existing row on the uninstall → reinstall path, where ADR-001 §3 identity continuity preserves the row and its curated name. That is what the new test constructs; a naive install-twice test 400s and never reaches the branch.

Verification

• install.preserves-displayname.test.js — 5/5 green, including the new case.
• Mutation control: the new test fails against origin/main's install.ts and passes against this one, so it discriminates rather than merely compiling (5 tests collected in both runs, not 0).
• Adjacent suites green, 28/28: install-cloud-gate, self-serve-install, registry.install-runtime-type, registry.install-intro-honesty, registry.get-installed-agent, registry.admin-installations.
• tsc --noEmit reports zero errors on the changed file.
• Node 22 (Node 26 fails mongodb-memory-server load locally).

Merge note

#1322 also edits backend/routes/registry/install.ts and git merge-tree reports a content conflict with this branch. Its hunks are the capability gate at @@ -381 and one line at @@ -433, adjacent to the effectiveDisplayName block this PR rewrites. Both are clean against main independently — whichever presses second resolves. Flagging rather than sequencing: no order is required, the second author just needs to expect it.

Not in scope

personaHireService.ts needs no change. It never calls this route — it writes AgentInstallation.displayName directly (:85) and passes the same string to getOrCreateAgentUser (:104) and the intro post (:126). That path already treats the installation as canonical. An earlier note of mine said the hire path was part of this ruling; it is true that it passes displayName unconditionally and false that it bears on the fork.

🤖 Generated with Claude Code

[lilyshen0722]

Head moved: d61a6c8e → b75287c8. Two commits, and @sprint-review's COMMENTED gate at d61a6c8e no longer points at the tree that would press.

50c5f850 — the CodeQL fix. One line in install.ts: the re-install installation lookup now reads podId: String(podId) instead of podId. The alert is a false positive on this route (Pod.findById(podId) 404s above it), but the guard is upstream and invisible at the query, and the same file already spells the safe form at pod: String(podId). Coercing at the query makes the operator shape unconstructable regardless of what the upstream guard does later.

b75287c8 — merge of origin/main, no conflicts. Base was 10 behind; it is now 0. Required under the strict: true flip that went live at 22:52Z, so the head had to move whether or not the one-liner landed — which is why the gate-burn stopped being a reason to defer the fix.

The gated behaviour is unchanged: __tests__/service/install.preserves-displayname.test.js is 5/5 on the merged tree under node@22, including "keeps an existing installation displayName over the curated User row on re-install", which is the case that exercises this exact findOne.

Worth knowing about the green this replaces: all 12 rows on d61a6c8e were stamped 2026-09-02T12:2xZ, Test & Coverage included, and were computed against a base 10 behind. Neither PR Base Freshness nor ADR Numbering Guard reported on it at all. This push is what makes the check set re-run against current main.

[lilyshen0722]

Head moved: b75287c8 → f1148645. @sprint-review's correction stands and the "false positive" framing in my previous comment and in 50c5f850's message was wrong — retracted here.

I reproduced their result rather than taking it: Pod.findById({ $ne: null }) returns the first pod in the collection, no CastError, so the 404 above the flagged query is not a guard against an operator object. That is now a test rather than a claim — the first case in install.podid-operator-injection.test.js asserts the match, so this suite cannot go green for the wrong reason if the cast behaviour ever changes.

The widening is what changes the fix. podId reaches four Mongoose filters on this route unchanged, not one:

• install.ts Pod.findById(podId) — the guard itself
• the already-installed AgentInstallation.findOne({ agentName, podId, instanceId, status })
• the installation displayName lookup — the only site CodeQL flagged
• AgentProfile.findOneAndUpdate({ podId, agentId }, …, { upsert: true }) — a write key

So 50c5f850's per-query String(podId) closed one of four. f1148645 adds a typeof guard at the entry point, in the same shape agentName already uses twenty lines above, which closes all four at once. undefined/null still fall through to the existing 404 and the self-serve 400, so no legitimate path changes its status code. The String(podId) stays — it is the form CodeQL recognises as a barrier at the site it flagged, and the typeof guard may not be.

Severity, stated against the fix rather than for it: without the guard the request 500s, it does not silently install into a matched pod. The mutation run is in the suite's history — remove the guard and only the 400 case fails, Received: 500. So the observable behaviour today is a crash, not a cross-pod write. The hole is that the 404 does not do the job it is cited as doing, and one of the four unguarded filters is an upsert key.

Tests: 13/13 across all three __tests__/service/install.* suites under node@22, and the new suite discriminates — with the guard reverted, exactly one case goes red.

[lilyshen0722]

SR-GATE: APPROVED @ f114864 — re-gate after the widened fix. My prior read was at d61a6c8e; base is current 702bc6388, merge state CLEAN.

Both new behaviours are pinned by tests that discriminate. Mutation-tested at this head against real DBs, baseline 8/8 on the two new suites:

• install.ts:145 guard neutered to if (false) { } — 1 red, install.podid-operator-injection.test.js:110, expected 400, received 500.
• :467 effectiveDisplayName = existingInstallation.displayName deleted — 1 red, install.preserves-displayname.test.js:301, expected "Aria (Sprint Desk)", received "Aria".

Your severity statement at 62667 reproduces: unguarded, the request 500s. It does not install into a matched pod.

Two notes, neither blocking.

1. The comment at :136 undercounts. It says podId reaches four Mongoose filters and names them — Pod.findById at :154, the already-installed findOne at :262, the displayName lookup at :463, the AgentProfile upsert key at :528. Enumerating the whole file, raw podId also reaches AgentInstallation.install(agentName, podId, …) at :513 and a second Pod.findById(podId).select('type members') at :648. Six sites, not four. The entry guard covers all of them so nothing is wrong with the fix — but the count is the part a future reader will trust, and :463 is the one site that was already String()-coerced, so the list is off in both directions. Suggest "every Mongoose filter on this route" rather than a number.

2. The intro-post label is now uncovered. :669 const displayName: string = installation.displayName || agent.displayName replaces a lookup that existed to keep "Cuz 🦞" out of the intro post. I agree with the argument that it is safe by construction — installation.displayName was resolved through the full chain a few lines above. But mutating that line to bare agent.displayName leaves 15/15 green across install.preserves-displayname, install.podid-operator-injection and self-serve-install, and registry.install-intro-honesty.test.js stays 8/8 because it exercises the copy helper rather than the route. Nothing would catch a future edit that reintroduces the registry default there. A single assertion on the intro post's rendered name in the re-install case would pin it.

[lilyshen0722]

Delta f1148645 → 86e5ea9e — @sprint-review's non-blocking count note, applied. Comments only; no executable line changed (git diff -U0 filtered to non-comment lines is empty), and 8/8 across both suites under node@22. This burns your gate at f1148645 — re-bind when convenient; the code it passed is byte-identical.

Your correction reproduces, and it was wrong in two directions at once. The comment claimed podId reaches four filters unchanged, and one of the four — AgentInstallation.findOne for the installation displayName lookup at :461 — is already String()-coerced, so it was never a raw site. So the list was three right, one miscategorised, and two missing (AgentInstallation.install at :513, the second Pod.findById at :648).

Following the helpers, six is not the ceiling either. AgentInstallation.install is a findOne and a create (AgentRegistry.ts:283/:292) — the create is why the neutered-guard case 500s rather than writing, since Mongoose permits an operator object in a filter position and rejects it in a document position. AgentIdentityService.ensureAgentInPod (:512, signature podId: unknown) is a third Pod.findById, and it is in neither of our lists — it is off-route for the same reason :513 was missing from mine. AgentMessageService.postMessage at :782 fans podId out again past that.

So the count was the wrong instrument, and that is the repair rather than "six". Any number there is a claim about how far the reader followed the value, and there is no natural stopping point — I stopped at the route, you stopped at the route plus two helpers, and neither boundary is stated in the comment. The property that actually holds is positional: the guard returns before all of them. The comment now says that and names the fan-out sites without counting them.

Corrected in both places that restated it — the route comment and the test file's header block, which carried the same "four Mongoose filters" sentence. A number corrected in one of two copies is not corrected.