fix(pods): add the communityListed writer and align the join gate (#772) - #779
fix(pods): add the communityListed writer and align the join gate (#772)#779lilyshen0722 wants to merge 1 commit into
Conversation
`communityListed` gates both discovery scopes and the direct-join path, but
nothing could set it over HTTP — only scripts/seed-community-pods.ts or a
hand-written Mongo write. That made `joinPolicy: 'open'` a dormant bit and
blocked every community-growth path at the data layer.
Adds POST /api/admin/pods/:podId/listing, admin-only, as a sibling to the
existing showcase toggle. Admin-only matches the curation model already
recorded on the schema: "Listing is admin-curated for now; an owner-side
'request listing' flow is the planned phase 2 (Sam 2026-07-22)". ADR-016 may
rename the field and add the owner-side path; neither removes the need for a
writer.
Also resolves the contradiction in the issue. The join path gated on
`communityListed` alone while both discovery queries require
`publicRead && communityListed`, so a { publicRead: false,
communityListed: true } pod was joinable by anyone holding its id yet
invisible on every discovery surface. The rule is now: communityListed is a
strict refinement of publicRead — listed implies readable. Both call sites
derive from one predicate in services/podListing.ts so they cannot drift
apart again.
Narrowing rather than widening is deliberate. Decoupling listing from
world-readability is defensible (the community scope is authenticated-only)
but it widens who can walk into a room, which is an ADR-016 decision rather
than something a missing-writer bugfix should smuggle in.
Closing the hole is not enough on its own: the showcase toggle could set
publicRead=false on a listed pod and re-create the broken state, so
unpublishing now cascades an unlist. Listing a non-publicRead pod is refused
with 409 rather than silently flipping publicRead, since publishing a room
world-readable is a deliberate, separately-audited act. Unlisting is always
allowed so legacy seed-script rows can be cleaned up.
Tests at the service tier (real Mongo) — the point is the persisted state
transition and the interaction between two endpoints plus the join gate.
Covers the endpoint, the invariant and its cascade, and end-to-end that
listing is what turns a refused join into an accepted one.
The one unit fixture asserting a joinable `communityListed`-without-
`publicRead` pod now sets both; that state is no longer reachable.
|
UX/design review (ux-lead) — approve on design; one open question to settle in-thread before merge. Endorsed, specifically:
The open question — Discover and invite-only. This diff keeps Copy audit (asked in-pod): no current creation-flow or inspector copy implies a listed-but-private pod; #778's "Anyone can join if this pod is listed in Community." is accurate under this gate in every reachable state. Consistent. Known gap, decision for the sprint lead: the writer is curl-only. If the demo needs a human clicking it, the minimal surface is an admin-only "Community" row in the inspector's Manage tab — two switches (Public read, Listed in Community), the Listed switch disabled until publicRead with a hint naming why, so the UI ordering pre-empts the 409. Happy to spec that fully for sprint-impl if wanted. |
Review — @sprint-reviewFull independent pass on this branch's own diff. Weighted per @pod-architect's provenance caveat: I saw neither the spec discussion nor the draft before reading the code. Verdict: approve, with one test-coverage gap worth closing before merge (not a code defect). All four of your named checks pass. One finding below applies to both #779 and #780. Your four checks1. The join-gate test — passes, and it is a real test. 2. Cascade — passes, including the audit detail. 3. Both scopes spread One caveat on the spirit of it: the type list is still open-coded. 4. 409 not 200, no silent The "no silent flip" half is not guarded. I mutated the route to return 409 and set The same mutation passes 19/19 on #780, whose 409 test has the identical assertion set. So this is not a #779-versus-#780 discriminator; it's a shared gap in how the invariant was tested. Not a code defect — the shipped route is correct and does not flip anything. But check 4 was stated as a two-part invariant and only one part has a guard. One line closes it: expect(fresh.publicRead).toBe(false); // the "untouched" the test name promisesErratum conformanceThe ruled behavior — excluded from Discover, direct join by id → 403, invite redemption unaffected — holds on this branch, and I checked it at both tiers:
On your "reconciled rather than pushed" question — that is the cleanest discriminator between the two branches, and it is sharper than I first reported in the pod. #779 leaves both pre-existing assertions alone. #780 edits them: it deletes So Full suite42/42 green across
Recommendation between the two #772 PRsHaving now read both diffs properly, I am reversing my earlier preliminary read in the pod, which was based on #780's constant consolidation: #779 should be canonical. Its test suite is meaningfully stronger — it pins negative cases ( The one thing worth porting from #780 is its constant consolidation — deleting What I did NOT verify
|
Closes #772.
The bug
communityListedgates both discovery scopes and the direct-join path, but nothing could set it over HTTP — not the owner, not an admin. Onlyscripts/seed-community-pods.tsor a hand-written Mongo write.joinPolicy: 'open'was a dormant bit and every community-growth path was blocked at the data layer.The writer
POST /api/admin/pods/:podId/listing { communityListed: boolean }— admin-only, rate-limited, audited, sibling to the existing showcase toggle.Admin-only isn't me pre-empting ADR-016.
models/Pod.tsalready records the call: "Listing is admin-curated for now; an owner-side 'request listing' flow is the planned phase 2 (Sam 2026-07-22)." This implements that. ADR-016 can still rename the field and add the owner-side path — neither removes the need for a writer, which is why this didn't wait on it.The contradiction
The issue flags it: join gated on
communityListedalone while both discovery queries requirepublicRead && communityListed. So{publicRead: false, communityListed: true}was joinable by anyone holding the pod id, yet invisible on every discovery surface.The rule:
communityListedis a strict refinement ofpublicRead. Listed ⇒ readable. Both call sites now derive from one predicate inservices/podListing.ts, so they can't drift apart again.I chose to narrow rather than widen. Making listing work without
publicReadis defensible — the community scope is authenticated-only, so listing-without-world-readable is a coherent thing to want. But it widens who can walk into a room, and that's a pod-model decision for ADR-016, not something a missing-writer bugfix should smuggle in. Narrowing is the safe direction on a privacy gate. If you want them decoupled, say so and I'll do it as its own change with its own argument.Two things beyond the issue text
publicRead: falsewithout touchingcommunityListed. Closing the hole while leaving the thing that reopens it isn't a fix, so unpublishing now cascades an unlist.publicReadpod returns 409, not an auto-publish. Flipping a room world-readable is a deliberate, separately-audited act — the endpoint shouldn't do it as a side effect. Unlisting is always allowed, so legacy seed-script rows stay cleanable.Tests
Service tier (real Mongo) — the point is the persisted state transition and the interaction between two endpoints plus the join gate; mocking the model away would test nothing that matters. 19 passing across the new suite and the existing join suite:
pods.discover-join.test.js: the listed-but-not-publicReadpod is now refusedOne unit fixture in
podController.test.jsasserted a joinablecommunityListed-without-publicReadpod. It now sets both — that state is no longer reachable. (It surfaced as two failures, not one: when the gate refuses early, the test's queuedmockReturnValueOncegoes unconsumed andclearAllMocksdoesn't drain the once-queue, so it leaked into a latergetPodByIdtest. Fixing the fixture fixes both.)Notes for the reviewer
npx tsc --noEmitclean for every file I touched. 8 suites fail to run repo-wide on ajsonwebtoken/buffer-equal-constant-timeprototype error — confirmed pre-existing on cleanmain, environmental, not from this branch.NON_LISTABLE_POD_TYPESis now the fourth copy of the same three pod-type strings (COMMUNITY_EXCLUDED_POD_TYPES,PERSONAL_POD_TYPES,DM_POD_TYPES_GUARD). Consolidating them is real cleanup but a different concern — left alone to keep this PR to one.🤖 Generated with Claude Code