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

backend/__tests__/service/install.podid-operator-injection.test.js

@@ -0,0 +1,137 @@
+/**
+ * Regression test for /api/registry/install accepting a query operator as
+ * `podId`.
+ *
+ * CodeQL flagged the `AgentInstallation.findOne` at install.ts and the alert
+ * was first read as a false positive, on the premise that `Pod.findById(podId)`
+ * + 404 above it already rejects anything that is not a real id. It does not.
+ * Mongoose casts an operator object in a filter position rather than throwing,
+ * so `Pod.findById({ $ne: null })` MATCHES the first pod in the collection and
+ * the 404 never fires — the raw object then reaches every Mongoose filter
+ * below, one of which (`AgentProfile.findOneAndUpdate`) is an upsert key.
+ * The route's own comment says why that set is stated as a position rather
+ * than a count: podId also flows through helpers which query on it again.
+ *
+ * The first test is the positive control for that premise: it asserts the
+ * upstream guard really does match, so a later refactor cannot quietly turn
+ * this suite green for the wrong reason.
+ */
+
+const express = require('express');
+const request = require('supertest');
+const jwt = require('jsonwebtoken');
+
+const { setupMongoDb, closeMongoDb } = require('../utils/testUtils');
+
+const User = require('../../models/User');
+const Pod = require('../../models/Pod');
+const { AgentRegistry, AgentInstallation } = require('../../models/AgentRegistry');
+const AgentProfile = require('../../models/AgentProfile').default || require('../../models/AgentProfile');
+
+const registryRoutes = require('../../routes/registry');
+
+const JWT_SECRET = 'test-jwt-secret-install-podid-operator';
+
+jest.setTimeout(60000);
+
+describe('Install endpoint rejects a query operator as podId', () => {
+  let app;
+  let installer;
+  let installerToken;
+  let pod;
+
+  beforeAll(async () => {
+    process.env.JWT_SECRET = JWT_SECRET;
+    await setupMongoDb();
+
+    app = express();
+    app.use(express.json());
+    app.use('/api/registry', registryRoutes);
+
+    installer = await User.create({
+      username: 'operator-installer',
+      email: 'operator-installer@test.com',
+      password: 'password123',
+      entitlements: { cloudAgents: true },
+    });
+    installerToken = jwt.sign({ id: installer._id.toString() }, JWT_SECRET);
+
+    pod = await Pod.create({
+      name: 'Operator-Injection Test Pod',
+      type: 'chat',
+      createdBy: installer._id,
+      members: [installer._id],
+    });
+
+    await AgentRegistry.create({
+      agentName: 'openclaw',
+      displayName: 'Cuz 🦞',
+      description: 'OpenClaw test',
+      manifest: {
+        name: 'openclaw',
+        version: '1.0.0',
+        capabilities: [],
+        context: { required: [], optional: [] },
+      },
+      latestVersion: '1.0.0',
+      versions: [{ version: '1.0.0', manifest: { name: 'openclaw', version: '1.0.0', capabilities: [], context: { required: [], optional: [] } }, publishedAt: new Date() }],
+      registry: 'private',
+    });
+  });
+
+  afterAll(async () => {
+    await closeMongoDb();
+  });
+
+  beforeEach(async () => {
+    await AgentInstallation.deleteMany({});
+    await AgentProfile.deleteMany({});
+  });
+
+  // Positive control for the premise the fix rests on. If this ever starts
+  // throwing or returning null, the entry-point guard below is still correct
+  // but this suite is no longer testing the thing it claims to.
+  it('control: Pod.findById with an operator object matches rather than 404ing', async () => {
+    const matched = await Pod.findById({ $ne: null }).lean();
+    expect(matched).toBeTruthy();
+    expect(String(matched._id)).toBe(String(pod._id));
+  });
+
+  it('400s on an operator object instead of letting it reach the Mongoose filters', async () => {
+    const res = await request(app)
+      .post('/api/registry/install')
+      .set('Authorization', `Bearer ${installerToken}`)
+      .send({
+        agentName: 'openclaw',
+        instanceId: 'aria',
+        podId: { $ne: null },
+        version: '1.0.0',
+        scopes: ['context:read'],
+      });
+
+    expect(res.status).toBe(400);
+    expect(res.body.error).toMatch(/podId must be a string/);
+
+    // Nothing may have been written on the way to the rejection — the
+    // AgentProfile write is an upsert keyed on podId, so a leak here would
+    // create a row rather than merely read one.
+    expect(await AgentInstallation.countDocuments({})).toBe(0);
+    expect(await AgentProfile.countDocuments({})).toBe(0);
+  });
+
+  it('still installs normally when podId is the ordinary id string', async () => {
+    const res = await request(app)
+      .post('/api/registry/install')
+      .set('Authorization', `Bearer ${installerToken}`)
+      .send({
+        agentName: 'openclaw',
+        instanceId: 'aria',
+        podId: pod._id.toString(),
+        version: '1.0.0',
+        scopes: ['context:read'],
+      });
+
+    expect(res.status).toBe(200);
+    expect(await AgentInstallation.countDocuments({ podId: pod._id })).toBe(1);
+  });
+});

backend/__tests__/service/install.preserves-displayname.test.js

@@ -238,4 +238,66 @@ describe('Install endpoint preserves curated displayName (cycle-of-Aria regressi
     // Registry default is acceptable when there's no curated identity to preserve.
     expect(installation.displayName).toBe('Cuz 🦞');
   });
+
+  // TASK-032, ruled 2026-09-02: AgentInstallation is canonical; the User row
+  // is a seed for an installation that has no name, never a preference over
+  // one that does. Before the ruling this case reverted a per-pod label to
+  // the shared identity's label on every re-install, while the read paths
+  // (agentMessageService, dmService) already preferred the installation.
+  //
+  // This is the discriminating case for that precedence and nothing else in
+  // the file reaches it: every other test starts from an empty
+  // AgentInstallation collection, where seed and preference are the same
+  // value and the two orderings agree.
+  //
+  // The reachable shape is uninstall-then-reinstall, not install-twice: an
+  // ACTIVE row makes the route 400 ('Agent already installed in this pod'),
+  // so the only way a second install meets an existing row is the
+  // reactivation path — where ADR-001 §3 identity continuity keeps the row
+  // (and its curated name) across the uninstall.
+  it('keeps an existing installation displayName over the curated User row on re-install', async () => {
+    await AgentInstallation.create({
+      agentName: 'openclaw',
+      podId: pod._id,
+      instanceId: 'aria',
+      installedBy: installer._id,
+      version: '1.0.0',
+      status: 'uninstalled',
+      scopes: ['context:read'],
+      // Curated for THIS pod, and deliberately different from the User row's
+      // 'Aria' so the assertion cannot pass by coincidence.
+      displayName: 'Aria (Sprint Desk)',
+    });
+
+    const res = await request(app)
+      .post('/api/registry/install')
+      .set('Authorization', `Bearer ${installerToken}`)
+      .send({
+        agentName: 'openclaw',
+        instanceId: 'aria',
+        podId: pod._id.toString(),
+        version: '1.0.0',
+        scopes: ['context:read', 'summaries:read', 'messages:write'],
+        // No explicit displayName — the caller is re-installing, not renaming.
+      });
+
+    expect(res.status).toBe(200);
+
+    const installation = await AgentInstallation.findOne({
+      podId: pod._id,
+      agentName: 'openclaw',
+      instanceId: 'aria',
+    }).lean();
+    expect(installation.displayName).toBe('Aria (Sprint Desk)');
+    // The two values the old precedence would have written instead.
+    expect(installation.displayName).not.toBe('Aria');
+    expect(installation.displayName).not.toBe('Cuz 🦞');
+
+    const profile = await AgentProfile.findOne({
+      podId: pod._id,
+      agentName: 'openclaw',
+      instanceId: 'aria',
+    }).lean();
+    expect(profile.name).toBe('Aria (Sprint Desk)');
+  });
 });

backend/routes/registry/install.ts

@@ -133,6 +133,26 @@ installRouter.post('/install', installRateLimit, auth, async (req: any, res: any
       return res.status(400).json({ error: 'Invalid agentName: must match /^(@[a-z0-9-]+\\/)?[a-z0-9-]+$/' });
     }
 
+    // Mongoose does not reject an operator object in a filter position — a
+    // `{$ne: null}` here casts cleanly and MATCHES, so the `Pod.findById`
+    // 404 below is not a guard against one. Reject a non-string at the entry
+    // point, the same shape agentName already uses above, so no query below
+    // can be handed a query operator. `undefined` is left to fall through to
+    // the existing 404 / self-serve 400 so no legitimate path changes status
+    // code.
+    //
+    // Deliberately no count of the filters downstream. podId reaches them
+    // both directly and through helpers that fan it out again —
+    // `AgentInstallation.install` (a `findOne` AND a `create`),
+    // `AgentIdentityService.ensureAgentInPod` (a third `Pod.findById`),
+    // `AgentMessageService.postMessage` — so any number is a claim about how
+    // far the reader followed the value, and it decays on the next refactor.
+    // The property that actually holds is positional: this returns before
+    // every one of them.
+    if (podId !== undefined && podId !== null && typeof podId !== 'string') {
+      return res.status(400).json({ error: 'podId must be a string' });
+    }
+
     // Fetched once and reused for the #609 owner-scoping decision below and
     // the cloud-entitlement gate further down.
     const installerUser = await User.findById(userId).select('role entitlements isBot').lean();
@@ -418,20 +438,54 @@ installRouter.post('/install', installRateLimit, auth, async (req: any, res: any
       ...AUTO_GRANTED_INTEGRATION_SCOPES,
     ]));
 
-    // Task #62 (round 2): prefer the curated User.botMetadata.displayName
-    // for the SAME agentName + instanceId over the registry-default
-    // (`agent.displayName` e.g. "Cuz 🦞" / "Codex"). PR #408 fixed this
-    // seam on the intro-post path; this is the install-path equivalent.
-    // Without this, installing an existing agent identity (e.g. openclaw:nova)
-    // into a NEW pod writes "Cuz 🦞" to both AgentInstallation.displayName
-    // AND AgentProfile.name — and the V2 member list reads AgentProfile FIRST,
-    // so users see "Cuz" on every member row even though the underlying
-    // identity has the right name. Order: explicit caller intent > existing
-    // identity > registry default. Resolve the existing identity through the
-    // same leak guard every display surface uses: historical rows may contain
-    // a runtime-shaped displayName such as "openclaw (nova)", which must not
-    // become a higher-precedence installation or profile label.
+    // TASK-032, ruled 2026-09-02: AgentInstallation is canonical for a
+    // display name. `User.botMetadata.displayName` is a SEED for an
+    // installation that has none — never a preference over one that does.
+    //
+    // Precedence: explicit caller intent > this pod's existing installation
+    // > the curated identity on the User row > registry default
+    // (`agent.displayName` e.g. "Cuz 🦞" / "Codex").
+    //
+    // The installation term is the new one and it is what the ruling adds.
+    // Without it a re-install re-seeded the per-pod label from the shared
+    // User row, so a name curated for THIS pod silently reverted — while
+    // the read paths (agentMessageService, dmService) already preferred the
+    // installation precisely to stop a sibling pod's name leaking through.
+    //
+    // Task #62 / PR #408 is NOT reinstated by this reordering: the User-row
+    // seed still beats the registry default, so installing an existing
+    // identity (e.g. openclaw:nova) into a NEW pod — where there is no
+    // installation yet — still writes "Aria" rather than "Cuz 🦞" to both
+    // AgentInstallation.displayName and AgentProfile.name. What changes is
+    // only the case where BOTH are curated, and there the per-pod value wins.
+    // The seed is still read through the same leak guard every display
+    // surface uses: historical rows may contain a runtime-shaped displayName
+    // such as "openclaw (nova)", which must not become a higher-precedence
+    // installation or profile label.
     let effectiveDisplayName: string = displayName || '';
+    if (!effectiveDisplayName) {
+      try {
+        const existingInstallation = await AgentInstallation.findOne({
+          agentName: agent.agentName,
+          podId: String(podId),
+          instanceId: normalizedInstanceId,
+        }).select('displayName').lean() as { displayName?: string } | null;
+        if (existingInstallation?.displayName) {
+          effectiveDisplayName = existingInstallation.displayName;
+        }
+      } catch (lookupErr) {
+        // Non-fatal — fall through to the User-row seed below. Logged with
+        // the agent identity so an operator chasing "the name reverted on
+        // re-install" can correlate the attempt, in the CodeQL-safe shape
+        // used by the sibling handlers (identifiers as arguments, not in
+        // the format-string slot).
+        console.warn('[install] installation displayName lookup failed', {
+          agent: agent.agentName,
+          instance: normalizedInstanceId,
+          error: (lookupErr as Error).message,
+        });
+      }
+    }
     if (!effectiveDisplayName) {
       try {
         const existingAgentUser = await User.findOne({
@@ -609,31 +663,17 @@ installRouter.post('/install', installRateLimit, auth, async (req: any, res: any
         // getOrCreateAgentUser. If we pass the AgentRegistry default
         // ("Cuz 🦞" / "Codex"), the User row's curated per-instance
         // displayName ("Aria") gets clobbered or sticky-dedup-suffixed
-        // to "Cuz 🦞 (Aria)". Resolve the intro display label by
-        // preferring the live agent identity (User.botMetadata.displayName)
-        // first, so we never overwrite a curated label with a registry default.
-        let displayName: string;
-        try {
-          const existingBot = await User.findOne({
-            isBot: true,
-            'botMetadata.agentName': agent.agentName,
-            'botMetadata.instanceId': normalizedInstanceId,
-          }).select('botMetadata').lean() as { botMetadata?: { displayName?: string } } | null;
-          displayName = existingBot?.botMetadata?.displayName
-            || installation.displayName
-            || agent.displayName;
-        } catch (lookupErr: unknown) {
-          // Fall back to the legacy chain if the identity lookup blew up —
-          // don't take down the intro flow on a transient mongo hiccup.
-          // Log with agent identity for operator correlation.
-          // Same CodeQL-safe shape as the install-path log above.
-          console.warn('[install] intro displayName lookup failed', {
-            agent: agent.agentName,
-            instance: normalizedInstanceId,
-            error: (lookupErr as Error).message,
-          });
-          displayName = installation.displayName || agent.displayName;
-        }
+        // to "Cuz 🦞 (Aria)".
+        //
+        // This used to re-query User.botMetadata and prefer it. Under
+        // TASK-032's ruling the installation is canonical, and
+        // `installation.displayName` was resolved a few lines above through
+        // the full precedence chain (explicit > installation > User-row seed
+        // > registry default) — so it already carries the curated label and
+        // the second lookup could only disagree with the row we just wrote.
+        // Reading it here keeps the intro post and the member list on one
+        // source of truth, and drops a per-install User query.
+        const displayName: string = installation.displayName || agent.displayName;
         const blurb = (agent.description || '').trim().replace(/\s+/g, ' ');
         // Skip the blurb when it just repeats the name (the publish step in
         // older CLI versions seeded description from displayName, producing