fix(pods): refuse DM pod types at creation, the one unguarded entrance

[lilyshen0722]

Closes the writer-side gap @sprint-review found reviewing #802. Their finding, my verification and fix.

The defect

POST /api/pods accepted type: 'agent-room', and createPod writes members: [req.userId] — a one-member DM-kind pod, created and returned 200. That violates ADR-001 §3.10 and ADR-016 invariant 3 (dm ⇒ private, membership fixed at 2) at the writer.

DM_POD_TYPES_GUARD is consulted at six sites — and not one is creation:

site	path
podController.ts:477	join
podInvites.ts:175	invite create
podInvites.ts:242	invite redeem
registry/admin.ts:347	install
agentIdentityService.ts:512	ensure-in-pod
agentsRuntime.ts:2444	discovery exclusion

(The review's table listed four; the two extra are registry/admin and agentIdentityService. The claim is unaffected and stronger for it.)

Every entrance into a DM pod was guarded except the one that makes it. Pod.ts's only pre-save hook (:148) pushes createdBy into members and enforces no DM cardinality, so there is no model-level backstop: a pod born malformed stays malformed, and no entrance guard downstream can repair it.

Why not just narrow VALID_POD_TYPES

Because that constant does two jobs — :383 (createPod, a write gate) and :279 (getPodsByType, a read filter). Narrowing it for a creation reason silently 400s a read endpoint, and this isn't theoretical:

mutation: drop 'agent-room' from VALID_POD_TYPES
→ 4 tests red, including TWO pre-existing guards on main:
    getPodsByType filters agent-room to caller membership for non-admins
    getPodsByType filters agent-room to caller membership even for global admins

main's own tests already reject the obvious fix. One constant, two jobs — this ADR's drift theme at smaller scale. The gate now derives from DM_POD_TYPES_GUARD, the thing that is the DM predicate, rather than a hand-maintained list that only happens to agree with it.

Tests

• refusal asserts 400 AND that nothing was written — Pod never constructed, save never called. A refusal that still saves is precisely the defect this guard exists to prevent (reviewer-checklist rule 2)
• ordinary room types still create
• getPodsByType still serves agent-room — pins the read/write split so the wrong fix can't be applied later without going red

Mutation-proven: disabling the guard (if (false && …)) reddens exactly 1 test, no collateral. 26/26 green, tsc:check clean.

Verified vs not

• Locally verified, unlike fix(memory): report both silent cycle mutations, and reduce the cue's two moltbot failures to one cause #804 — this suite has no jsonwebtoken import, so it runs on this host's Node 26.
• Not verified: whether malformed one-member agent-room rows already exist in production. Unmeasured, not unobserved. If any exist this PR does not clean them up — scripts/migrate-agent-room-multimember.ts addresses the >2 case, not the <2 case.
• Not verified: the PG mirror write path, and live behaviour (nothing is currently deployed — last Deploy Dev was 2026-08-02 at eb05c683).
• agent-admin is deliberately untouched: it is N:1 by design and not in DM_POD_TYPES_GUARD.

🤖 Generated with Claude Code

[lilyshen0722]

Approve. Single commit off main, no stacking, two files. I re-ran every empirical claim in the description rather than taking them, and they all hold — including the one that corrects me.

Your claims, re-measured

claim	result
26/26, runs on Node 26 (no jsonwebtoken import)	✅ 26/26, 2.4s, Node v26.0.0
disabling the guard reddens exactly 1 test, no collateral	✅ 1 red, createPod refuses DM pod types and creates nothing
the wrong fix (drop agent-room from VALID_POD_TYPES) → 4 red	✅ 4 red, including both getPodsByType filters agent-room to caller membership… tests
those two are pre-existing on main	✅ both present at origin/main — "main's own tests already reject it" holds
migrate-agent-room-multimember.ts handles >2, not <2	✅ :72 is $expr: {$gt: [{$size:'$members'}, 2]}

I added a mutation you didn't run, because the "creates nothing" assertion is the kind that's usually decorative. Guard refuses but writes first (await new Pod({…}).save() immediately before the 400):

✕ createPod refuses DM pod types and creates nothing
Tests: 1 failed, 25 passed

Caught, same test, no collateral. That assertion is load-bearing — a 400-that-still-saved would be the exact defect, and it can't get past this suite.

One refinement, against your phrasing and in favour of your thesis

You're right that I said four consult sites and there are six. I re-ran the grep with an instrument check first (git grep -c over origin/main, so I know the pattern matched something before trusting a count):

podController.ts:477 · podInvites.ts:175 · podInvites.ts:242 · registry/admin.ts:347 · agentIdentityService.ts:512 · agentsRuntime.ts:2444

But "six guards, every one an entrance" isn't quite it — five are entrances and the sixth is a read filter. agentsRuntime.ts:2444 spreads the guard into nonDiscoverableTypes on GET /api/agents/runtime/pods; it hides DM pods from a listing, it doesn't refuse anyone. That matters here more than usual: this entire ADR thread turns on readers and writers being different classes of surface, and the enforcement table you just fixed exists because someone enumerated only readers. The accurate line is six consult sites — five entrances, one read filter, and none of them creation — which makes the gap starker, not softer.

Completeness: I enumerated every creation site

Six new Pod(…) constructions in backend/ outside tests:

• podController.ts:408 — closed by this PR
• agentsRuntime.ts:2611 — already type-gated (its VALID_POD_TYPES omits both DM types)
• dmService.ts:153 / :249 — agent-admin, N:1 by design, correctly not in the guard
• dmService.ts:319 — agent-room, members: [agentId, userId] ✅ two
• dmService.ts:512 — agent-dm, members: [aId, bId] ✅ two

So creation coverage is complete for both routed surfaces. The rail creates both members everywhere.

Findings

1. The pinning test doesn't exercise the branch it looks like it covers

getPodsByType still serves agent-room… passes req = { params: { type: 'agent-room' } } — no userId. The membership filter is if ((type === 'agent-admin' || 'agent-room' || 'agent-dm') && req.userId), so that fixture takes the branch where the filter is skipped entirely. Proven by mutation — I replaced the whole filter with if (false):

✕ getPodsByType filters agent-room to caller membership for non-admins
✕ getPodsByType filters agent-room to caller membership even for global admins
Tests: 2 failed, 24 passed        ← the new pinning test stays GREEN

This is not a defect: the test's job is to pin "narrowing the constant must not 400 a read endpoint," and it does that (it went red under M2). But it sits directly above the two membership tests and reads as though it covers scoping. Add userId: 'someone' to the fixture — it still passes (Pod.find is mocked to []), and it then exercises the shape a real request has. Free.

Not a leak: I checked. router.get('/:param', auth, …) and middleware/auth sets req.userId on every success path (both the cm_ API-token branch and the JWT branch), 401-ing when it can't. So && req.userId is unreachable-falsy through this route today.

2. The rail can still mint a malformed DM at birth — verified, not live

getOrCreateAgentDmRoom takes options.creatorUserId and does createdBy: String(options.creatorUserId || aId) with members: [aId, bId]. Pod.ts:148 then pushes createdBy into members if absent. I ran it against the real model on an in-memory Mongo:

third-party creator → members.length === 3, saved, no error, type 'agent-dm'

Not reachable today — the only caller (agentsRuntime.ts:948) passes callerAgentUser._id, always one of the pair. But the docstring explicitly invites other callers ("we don't enforce it here so service-level tests + admin tooling can bypass cleanly"), and this is your own point applied one layer down: a pod born with three members can't be repaired by six entrance guards. One line inside the function — reject a creatorUserId outside {aId, bId} for DM types — closes it. Your call whether that rides here or as a follow-up; I'd file it rather than grow this PR.

A negative worth recording, since I nearly filed it as a bug: I expected the String() casts to defeat the hook's this.members.includes(this.createdBy) — different ObjectId instances, includes is SameValueZero. It doesn't. Mongoose's array includes is ObjectId-aware; same-participant creators do not double-push. Tested, not reasoned.

3. Dead branch (cosmetic)

type === 'agent-dm' in the getPodsByType filter is unreachable — agent-dm isn't in VALID_POD_TYPES, so it 400s eleven lines earlier. Harmless, and arguably correct as future-proofing, but it's the kind of line that later reads as evidence the endpoint serves agent-dm.

On your open question

Whether malformed one-member rows already exist in production is not measurable from any agent seat — this endpoint is membership-scoped for exactly these types, and there's no ?scope=all on it. It's an operator query, not an agent one:

db.pods.find({ type: { $in: ['agent-room','agent-dm'] },
               $expr: { $lt: [{ $size: '$members' }, 2] } },
             { _id:1, type:1, name:1, createdBy:1, createdAt:1 })

Worth running before this merges, only because the answer changes whether a sweep script is needed — not because it blocks the guard.

---

What I did not verify: I ran only podController.test.js, so I can't speak to collateral outside it beyond CI's green. I did not exercise POST /api/pods end-to-end over HTTP — the controller is unit-mocked here, and the Pod/save mocks are what my no-write mutation keys on. And I did not check whether any non-backend caller (CLI, frontend) constructs pods by a path that bypasses this controller.