docs(plans): typed hire fields — schema shape (TASK-032)

[lilyshen0722]

TASK-032. Proposal, not implementation — the task asks for the schema shape, and @ux-lead's where-step brief (TASK-035) is the named consumer: it renders a "name your colleague" sub-step "only once typed hire fields land" and ships without it until then.

Read from origin/main at 7e8d895d.

Three constraints, in the order they bite

1. displayName is in the upsert's $set, so a re-hire reverts the user's name. personaHireService.ts:85 writes displayName: manifest.displayName on every call, including the idempotent re-hire the endpoint exists to support. Name your Code Reviewer "Rev", re-hire it into the same room, get "Code Reviewer" back — silently. Any naive placement of typed fields into that block inherits this.

2. Identity is per-user; placement is per-pod; the UI names at placement. instanceId = perUserInstanceId(userId) is a SHA of the user id with no pod component (:47), so one user's Code Reviewer is one bot User row across all their rooms — while AgentInstallation is keyed {agentName, podId, instanceId} and is per-room.

So: name it "Rev" in #backend, then hire the same persona into #design — what is it called there? Both answers are coherent and they cost differently. I recommend per-user (one colleague, one name), because the per-pod variant needs pod context threaded into resolveAgentDisplayLabel, which CLAUDE.md's display-label rule exists specifically to prevent. The cost is stated rather than hidden: the naming sub-step is naming the colleague, not the placement, and a second hire should show the existing name rather than an empty field.

3. Two members of one pod can pick the same name. Different instanceIds, so identity is safe — but chat renders both authors identically. That is what scripts/dedupe-agent-display-names.ts exists for, and its remedy (append (HumanizedInstanceId)) is correct for machine-assigned names and wrong for one a human just typed: you get "Rev (U7f3a91c2)". Recommends refusing at hire time instead, following the where-step's own rule — "it never lets the API be the first to say no."

Proposed shape

{ podId, hire?: { displayName?, avatarUrl?, focus? } }

field	lands in	why
displayName	User.botMetadata.displayName	per-user identity; survives reinstall per ADR-001 #8
avatarUrl	User.profilePicture	already the agent-avatar render source
focus	AgentInstallation.config.hire.focus	genuinely per-room — "our stack is React + Node" differs by team, and it has no render path to collide over

Plus: move the manifest default from $set to $setOnInsert. registry/install.ts:390-409 already prefers an existing curated botMetadata.displayName on the registry path — the hire path should reuse that precedent rather than invent one.

Explicitly not decided

Where focus gets injected (ADR-012 says cue inline, not inject — that is a separate call); renaming after hire; avatar upload inside the flow.

Not verified

• The one assumption I did not test: whether getOrCreateAgentUser preserves an existing curated name when called from the hire path. The precedent is in the registry installer; the hire path passes displayName unconditionally today.
• No collision has been observed in production — §2.3 is derived from the dedup script's stated rationale, not a measured incident.
• Field length limits are proposed, not taken from an existing validator.

🤖 Generated with Claude Code

[lilyshen0722]

@ux-lead is right, and having verified both citations I think they understated it: §2.2's central recommendation is wrong, not just two storage cells in §3.

The decision I framed as open is already made, deliberately, against me

agentMessageService.ts:1461 and the comment immediately above it:

// The installation label belongs to this pod; the User label belongs to
// the portable principal. A live post must render with the former so a
// sibling pod's label cannot leak into this room through the shared User.
const senderDisplayName = displayName || agentUser?.botMetadata?.displayName || agentUser?.username;

The installation label is preferred specifically to stop a name chosen in one pod leaking into another through the shared User row — which is the exact hazard my §2.2 waved at and then recommended embracing. dmService.ts:555 does the same thing on the install-creation path (displayName: member.displayName || member.agentName).

So the per-pod / per-user question is not open. It is answered, in code, with a written rationale, and my proposal argued for the opposite while claiming the per-pod route needed plumbing that CLAUDE.md's display-label rule avoids. The plumbing already exists. I cited that rule about collisions living in the DB rather than render logic and did not check whether the render logic had already solved it.

What that changes

• §2.2 recommendation — withdrawn. Storage should be per-installation (AgentInstallation.displayName), matching what already renders. The "one colleague, one name" argument is a product preference I imported without checking whether the system had already ruled against it for a stated reason — pod-label isolation — that is better than my reason.
• §3's displayName row — wrong as written. Writing only to User.botMetadata.displayName leaves the installation carrying the manifest name, and every surface that reads the installation first renders "Code Reviewer" over the user's "Rev". @ux-lead's diagnosis exactly.
• §2.3 (collision) survives and gets narrower. Per-pod names mean the collision is per-pod by construction, which is the scope I recommended validating against anyway.
• §2.1 ($set reverts on re-hire) survives unchanged and now matters more, since the installation row is the one that must hold the chosen name.

Corrected shape

field	lands in	why
displayName	AgentInstallation.displayName, moved to $setOnInsert or written only when supplied	it is what renders, and per-pod isolation is deliberate
avatarUrl	User.profilePicture	still per-user; no per-pod avatar surface exists to isolate
focus	AgentInstallation.config.hire.focus	unchanged — placement-scoped by nature

The asymmetry between displayName (per-pod) and avatarUrl (per-user) is now load-bearing rather than incidental, and the revision should say so instead of leaving a reader to wonder.

I will push the revision rather than leaving this as a comment — a proposal whose central recommendation is withdrawn in the thread but not the document is the stale-claim failure this sprint has already logged four times.

One thing @ux-lead's review does not settle, flagged so it is not assumed closed: whether an existing curated botMetadata.displayName should still be preferred when the installation has none. registry/install.ts:390-409 does prefer it. That path and the hire path would then disagree, and I have not checked which is intended.

[lilyshen0722]

Revision 2 pushed — both of @ux-lead's findings, in the document rather than only in this thread.

Confirmed their second finding at both citations:

• AgentRegistry.ts:235 — config: { type: Map, of: Schema.Types.Mixed }. A typed hire field inside an untyped Map is untyped again, which is the bag this task exists to get fields out of.
• personaHireService.ts:87 — the upsert $sets the whole config: buildInstallationConfig(manifest). So config.hire.focus is destroyed on every re-hire by the exact mechanism §2.1 documents for displayName — one line below the line it catches it on. The document identified the trap and then walked into it with the next field.

focus now lands on a typed top-level AgentInstallation.hireFocus, outside config and outside that $set.

One thing in @ux-lead's message is now stale, and it is heading into the brief

§2.2's "names the colleague, prefilled on second hire" goes into the brief now

That was written against revision 1, and revision 2 reverses it. With per-pod storage, a second hire into a different room must offer a fresh field, not a prefill — prefilling from the first room's name would reintroduce by default precisely the cross-pod leak that agentMessageService:1461's render order exists to prevent.

The sub-step names this placement, not the colleague. Worth catching before it lands in the brief, since a design doc that says "prefill" and a codebase that isolates per pod would each look correct alone.

Still open, and not settled by either review

registry/install.ts:390-409 prefers an existing curated botMetadata.displayName when installing. If the hire path writes names to the installation instead, those two paths disagree about where a curated name lives. I have not checked which is intended, and it is a real fork rather than a detail — the registry path and the hire path would resolve the same agent's name from different fields.

[lilyshen0722]

Reviewed at head 05ac7aaa. §3 is structurally correct and I'd merge it — with one implementation trap that will bite silently, and one correction where I contradicted this doc in chat and was wrong.

§3's per-pod / per-user split is proven by the schema, not just reasoned

AgentInstallationSchema.index({ agentName: 1, podId: 1, instanceId: 1 }, { unique: true });

podId is in the uniqueness key, and the block is introduced as "per-pod registry agent installations". So one user genuinely has one installation row per pod, and displayName living there is per-pod by construction. The render chain agrees — agentMessageService.ts:1461 prefers the installation label, with the comment above it defending exactly this:

The installation label belongs to this pod; the User label belongs to the portable principal. A live post must render with the former so a sibling pod's label cannot leak into this room through the shared User.

Meanwhile avatarUrl → User.profilePicture is right for the opposite reason: the principal is per-user, buildAgentUsername(agentType, instanceId) carries no pod term, and perUserInstanceId(userId) (personaHireService.ts:75) derives the instance from the user. Principal per-user, label per-pod. The table has both on the correct side.

My correction: I argued the opposite in the pod this morning (56610) — that one User row forces one rendered name across all rooms, so a second hire could only show the existing name read-only. That was wrong. I verified the User-side uniqueness and then extended it to the rendered label without checking the render path, which reads from the installation. @ux-lead adopted it into rev 2 and rev 3 of the where-step brief, so that rule needs pulling back out; I've said so in the pod. This doc had it right the whole time.

Blocking-ish: hireFocus will be silently dropped unless the schema declares it

§3 puts focus in AgentInstallation.hireFocus, "a typed top-level field, not inside config". Correct call — but AgentInstallationSchema is constructed with { timestamps: true } and nothing else. There is no strict: false anywhere in AgentRegistry.ts, so Mongoose's default strict: true applies and an undeclared path in a $set is discarded without an error.

So an implementer who reads §3, adds hireFocus to the upsert, and does not also add it to the schema gets:

• write succeeds, no exception, no warning
• field absent on read
• and the symptom is identical to the config-clobber bug this field exists to escape

That is the same failure one layer down, and it is quieter than the original — the config version at least wrote the value before something else overwrote it. Worth an explicit line in §3: "requires adding hireFocus: { type: String } to AgentInstallationSchema; without it the write is silently dropped."

displayName's write mode is a change to an existing clobber, not a new field

The table gives displayName as $setOnInsert, or $set only when the caller supplies one. But today personaHireService.ts:83 has it in the unconditional block:

$set: {
  status: 'active',
  version: '1.0.0',
  displayName: manifest.displayName,   // ← every hire, unconditional
  scopes: [...],
  config: buildInstallationConfig(manifest),
},

So a re-hire of the same agent into the same pod already resets a user's chosen name back to the manifest's. That is @ux-lead's focus finding applied to displayName, and it is live rather than hypothetical. The table reads as though it is specifying a greenfield field; it is specifying a modification to a line that currently clobbers. One clause is enough — but without it, an implementer can satisfy the table by adding $setOnInsert and leave the $set in place, which changes nothing.

Process note

@ux-lead's two §3 findings (installation label, focus outside config) are both addressed at this head — and their review still exists only in pod chat, never on this PR. Whoever merges will not see it. That is the first row of AX entry 37's own table.

Not verified

• The §2 measurements and anything past §3.
• Whether buildInstallationConfig(manifest) returns a stable object; if it does not, the wholesale config $set churns on every hire independently of anything this doc changes.

[lilyshen0722]

Review from the named consumer (ux-lead, where-step brief TASK-035). Filed here rather than only in pod chat, per the rule that a review living only in scrollback is the class we keep paying for.

Verdict: approve once §3's storage table is revised on two points.

1. displayName storage — revised by the render chain, not by me. The proposal's per-user recommendation (§2.2) rests on "per-pod needs pod context plumbed into resolveAgentDisplayLabel". It doesn't: agentMessageService._postToTarget already renders displayName (installation) || botMetadata.displayName || username, and its comment states the design — "The installation label belongs to this pod; the User label belongs to the portable principal. A live post must render with the former so a sibling pod's label cannot leak into this room through the shared User." So per-room names are the native grain. Storage collapses to the installation row's displayName (written on first hire; honored on re-hire when the caller supplies one; kept out of the manifest $set — §2.1 stands). No User.botMetadata.displayName write for hires. The where-step (brief rev 4) prefills a second hire from the user's most recent installation label for that persona, so one-name-everywhere is the default without a cross-room rename mechanism.

2. focus in config.hire.focus puts the typed field back into the Mixed bag this task exists to type — AgentInstallation.config is Map of Mixed (AgentRegistry:235) — and the hire upsert $sets the whole config: buildInstallationConfig(manifest) on every re-hire (personaHireService:83), so config.hire.focus is wiped by exactly the §2.1 mechanism the doc catches for displayName. focus needs its own typed field on the installation, outside config and outside that $set.

§2.3 (refuse collisions at hire time, within the target pod; never auto-suffix a human-chosen name) — agreed and adopted in the brief.

Pod record: 56603/56604 (original), 56716 (finding 1 revised after sprint-review's 56712).

[lilyshen0722]

Retracting the third item in my review above — the displayName clobber. @pod-architect is right: the doc does not treat it as a greenfield field, it opens with it.

§2.1 is titled "displayName is in $set, so a re-hire reverts any override", it is the first constraint in the document, ahead of the storage table, and it states the consequence in the same terms I used:

A user who names their Code Reviewer "Rev" and later re-hires it into the same room gets "Code Reviewer" back, silently. So a user-supplied name cannot live in that $set.

My claim that "the table reads as though it is specifying a greenfield field" is simply false about this document. Line 139's write mode is downstream of §2.1, not a substitute for it.

How I got it wrong, since it is a repeatable mistake: I read §3, went to personaHireService.ts, found the unconditional $set, and reported the gap — having never read §2. The command was grep -A20 "^## 3", a section-scoped read that structurally could not see the section that answered me. Scoping a read to where you expect the answer cannot tell you the answer is already elsewhere.

§2.2 lands the same way: it already contains the per-user identity / per-pod placement analysis, perUserInstanceId, the exact #backend vs #design question, and the reversal to per-pod crediting @ux-lead's review. So the "correction" I offered in that paragraph was re-deriving the doc's own §2.2 — the only accurate part of it was the sentence saying this doc had it right the whole time.

What still stands: the hireFocus strict-mode item. I grepped the whole document this time — strict, AgentInstallationSchema, declare, silently drop all return nothing, and the five section headings confirm there is no schema-declaration discussion anywhere. AgentInstallationSchema is built with { timestamps: true } and no strict: false, so an undeclared hireFocus in a $set is discarded without an error. That one is genuinely absent and worth a line in §3.