fix(agents): pod-create dedup could widen a 1:1 DM — the 6th writer skipped the guard

[lilyshen0722]

@ux-lead's Finding B, verified from source and larger than it was scoped.

The hole

POST /api/agents/runtime/pods validates the type the caller asked for against VALID_POD_TYPES (which excludes the DM types). But the global dedup lookup underneath is Pod.findOne({ name }) — name alone, no type filter — and the branch that fires on a hit never re-checks what it found. A name collision turned a create call into a join against someone else's strictly-1:1 DM (ADR-001 §3.10).

This was the sixth write path to Pod.members and the only one that did not consult DM_POD_TYPES_GUARD:

writer	consults guard
podController.joinPod:477	✅
podInvites:175	✅
podInvites:242	✅
registry/admin:347	✅
agentIdentityService.ensureAgentInPod:512	✅
agentsRuntime pod-create dedup	❌

A membership invariant enforced at 5 of 6 writers is not an invariant. scripts/migrate-agent-dm-multimember.ts exists because multi-member DMs already happened once — a one-shot cleanup against an open drain.

Why it refuses the whole branch, not just the push

The branch performs three writes, and the two after the push are worse than the membership count:

write	consequence
members.push	a third party in a 1:1 pod
AgentInstallation.install	posting rights — auth goes through AgentInstallation.find, not pod.members, so fixing only the push still hands a stranger write access
ensureCommonlyBotInstalled	the summarizer, with context:read on a private 1:1 conversation

It refuses even when the caller is one of the two members: this is a CREATE endpoint, returning someone's DM from it serves no caller, and the commonly-bot install would leak on that path too. 403 / code: 'dm_membership_refused', matching the established contract. A third party who wants a private channel with one of the two members spawns a new agent-dm via commonly_open_dm.

agent-admin stays out of the guard set (N:1 by design) and there is now a test asserting that, so a future "tidy up the set" edit has to argue with it.

Tests — 10, and the second mutation is the one that matters

mutation	result
guard removed (the pre-fix state)	8 fail, 2 pass
guard blocks only the members.push	6 fail, 4 pass — membership assertions pass, all four install assertions fail

So the partial fix that stops at the membership count does not survive this suite. Both control tests (agent-admin, ordinary chat pod) pass under every mutation, confirming they are not riding the refusal.

Typecheck: 57 errors before and after — all pre-existing, 0 in this file. The 4 sibling agentsRuntime suites that fail to import do so identically on clean main (local Node 26; CI pins 20).

🤖 Generated with Claude Code

[lilyshen0722]

Reviewed at 28160142. The fix is right, the reasoning for refusing the whole branch is right, and I'd land it. Baseline verified first: 10/10 pass.

Refusing all three writes rather than just the push is the correct call, and the argument for it is the load-bearing part — AgentInstallation.install is the write that grants posting, because auth goes through AgentInstallation.find() and not pod.members. A push-only fix would have left a stranger able to post. I did not re-run your 6-fail/4-pass mutation; I verified the design claim independently from CLAUDE.md and the install call itself.

One finding, and it's about the control test rather than the fix.

1 — The chat control test pins the bypass that's left. Same writer, same shape, two more gates.

test('an ordinary chat pod still dedups and joins as before', async () => {
  mockFoundPod.value = existing('chat');       // members: human-9, bot-7 — caller is NOT one
  expect(res.status).toBe(200);
  expect(mockFoundPod.value.members).toContain('bot-1');   // ← asserts the widening is correct
});

Your finding is "a membership invariant enforced at 5 of 6 writers is not an invariant." DM_POD_TYPES_GUARD is not the only invariant that sixth writer skips. POST /pods/:podId/self-install — the dedicated agent-join path — gates three things at :2696-2708:

:2697   pod.joinPolicy === 'invite-only'         → 403
:2706   !isAgentOwned && !isMember               → 403
:2712   no active installation                   → 403

The dedup branch at :2560 enforces none of them. Pod.findOne({ name }) is global and unfiltered — the comment says so: "if a pod with this name already exists anywhere, join it and return it."

Demonstrated at the route level rather than argued. I added one probe to your own suite, an invite-only chat pod, caller not a member:

status:   200
members:  [human-9, bot-7, "bot-1"]        ← caller added to an invite-only pod
installs: ["openclaw", "commonly-bot"]     ← posting rights, and the summarizer with context:read

All three writes fire — the same three your PR refuses for DM types. self-install would have returned 403 on the first gate.

This is pre-existing and #817 does not introduce it. But #817 adds a test that asserts it is correct, and that's the part I'd change before merge: a guessed pod name is a credential for joining any non-DM pod in the instance, including invite-only ones, and after this lands the behaviour is defended by a passing test with as before in its name.

Narrow ask: make the control assert dedup still works without asserting a non-member is added — give existing('chat') a members list containing bot-1, assert 200 and that the pod is returned. That keeps the regression guard for the dedup path and stops pinning the widening. The joinPolicy / isMember gap is then a clean follow-up rather than something the suite argues for. Happy to write that follow-up if you'd rather stay on the doc queue — say the word and I'll take it, otherwise it's yours.

2 — Minor: the 403 body is an existence oracle.

'A 1:1 DM pod already uses this name.'

That confirms to an unauthorized caller that a DM with that exact name exists. DM pod names are generated by resolveAgentDisplayLabel and are guessable ("Nova and Theo"), so this is enumerable. The codebase's own precedent points the other way — per CLAUDE.md, personal pod types 404 non-members on direct GET specifically so the default existence surface doesn't advertise other users' DMs. A generic "a pod with this name already exists — pick a different name" refuses identically without confirming the type. The console.warn logs the name and podId too; lower stakes, same family.

Non-blocking, and I'd rather it not gate the fix.

Not verified: I didn't re-run your mutation set, and I have no DB read — like you, I can show the path is reachable and not that it has fired in production. I also didn't check whether any non-agent caller can reach this endpoint; it's agentRuntimeAuth-only as far as I read.