Add community membership settings - #348
Conversation
1dee1f6 to
b910ecd
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
Two P2 lifecycle defects are detailed inline. Both can leave a legitimate owner/admin without the Membership destination; neither is a demonstrated relay authorization bypass.
Reviewed head b910ecd5b81ff9e2069b6b00b55eef906509e824, target base ac64f80a4f5e3f699d8de9fafb4329b5466e373d (merge base 47ea7e08eb124c5c723afa9a8c3626444b66398b). Mantis’s independent source lane is complete and reconciled. The contribution lifetime, selected-community session owner, roster projection, mutation/retry paths, and changed tests were inspected.
Validation limits: source-only; no local tests, builds, installs, app launches, or code execution. The reported human/browser acceptance was not independently repeated. A single hosted-CI snapshot associated with this head reports JavaScript/CI required failing: store-discovery.test.ts:148, the 501-membership workflow assertion (408 files/4861 tests passed, one failed). Six browser shards, measurements, Rust/tool integration, security and DCO report success; Windows was skipped. This is not a merge-readiness or runtime-validation claim. Failing hosted job.
Coverage: add the two lifecycle regressions at the real service/React boundary, not more browser role permutations. The new owner journey is representative whole-app wiring coverage; the ordinary-member negative case currently has no barrier establishing that the permission read finished, so it can pass while authorization is still loading. Please give that assertion a completed-read barrier (or cover it through real Settings/service integration at the lower layer).
b910ecd to
625388e
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
The two previously reported production lifecycle defects are addressed in source: Settings now retains visibility demand as contributions activate/retire, and a new zero-to-one membership demand re-reads cached authorization. The added service and membership regressions cover those mechanisms. I found no additional production defect in this bounded repair review.
One remaining P2 regression-test defect is detailed inline: the attempted completion barrier does not establish that authorization has been applied. This follows up the coverage issue in the prior review, not a new product requirement or a demonstrated authorization bypass.
Reviewed head 0e7595eaccbf8392342d89a46bde05dfb308dc77 against target base 1f71ee94f2dadb743a6529ded436b266e3c93045; follow-up to reviewed revision b910ecd5b81ff9e2069b6b00b55eef906509e824. Inspected contribution identity/lifetime, Settings demand, shared membership refresh, and existing mutation locking during refresh. Sources were isolated and Git-blob verified, with no dirty source inputs.
Validation limits: source-only; no tests, builds, installs, app launches, or execution of PR code. Reported human/browser acceptance was not independently repeated. The one current-head hosted-check snapshot exposed successful Semgrep OSS, zizmor, and DCO checks only; it did not expose JavaScript, browser, or aggregate CI results, so those are unverified here. This COMMENT is non-blocking feedback, not approval or merge authorization.
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
No new actionable findings in this two-file follow-up. The previous completion-barrier finding is addressed at the lower layer: Settings.test.tsx:218–296 mounts real Settings with SettingsCardsService and createCommunityMembership, holds the roster promise, observes loading, then waits for ready, role: member, and refreshing: false before asserting that Administration/Membership remain absent. A stalled or unapplied read can no longer satisfy that regression. The representative owner browser journey remains unchanged; the remaining negative browser smoke check is not itself proof of completed authorization.
Reviewed head db075d3f6df6617659b2717249964d54ac040357 against target base 1f71ee94f2dadb743a6529ded436b266e3c93045, specifically the repair since previously reviewed 0e7595eaccbf8392342d89a46bde05dfb308dc77. This follow-up changes tests only; the previously reviewed production lifecycle repairs are unchanged. Reviewed the actual contribution/visibility subscriptions and Settings consumer, using isolated Git-blob-verified source with no dirty inputs.
Validation limits: source-only; no tests, builds, installs, app launches, or PR-code execution. The new test mocks the session read boundary, so it covers role projection/service/React integration, not cryptographic verification or live transport. The one current-head hosted-check snapshot reports successful Semgrep OSS, zizmor, and DCO checks only; JavaScript, browser, and aggregate CI results were not exposed and remain unverified here. Author-reported human/browser acceptance was not independently repeated. This non-blocking COMMENT is not approval or merge authorization.
db075d3 to
51d25f7
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review (via Wes’s account)
One P2 host-navigation integration defect is detailed inline. Permission-gated Membership routes can be rejected before their visibility demand runs, including returning to Membership after changing communities. This is new evidence at the host route/detail boundary; the earlier late-contribution, demand-reacquisition, and applied-authorization-test findings remain addressed.
The sidebar now separates Administration from ordinary community cards, consumes the same filtered registry as the detail pane, and preserves native button selection semantics. The updated keyboard expectations match the new order. The lower-layer ordinary-member regression still waits for the applied role; the negative browser smoke check is not independent proof of completed authorization. Inspected success/cancel and error/retry paths in source, including route recovery and dialog focus ownership; actual keyboard/focus behavior was not exercised.
Reviewed head 51d25f75bff54a4e22455df73a7758b398ce0ccc against base 9c0b26c65b2781c4171f66de7bc66cc80816e3dd, following previously reviewed db075d3f6df6617659b2717249964d54ac040357. Separated rebased upstream changes from the feature delta and used an isolated, Git-blob-verified snapshot (1,711 blobs; no dirty source inputs). The repository is public; no internal-link/secret disclosure was found in the changed publication surface, commit messages, or PR description. The current description/discussion contained no attached images to inspect.
Validation limits: source-only; no tests, builds, installs, app launches, or PR-code execution. Author-reported human/browser testing was not repeated. One hosted-check snapshot for this head showed JavaScript and 11 of 12 browser shards still running; the remaining browser shard, browser measurements, Rust/tool integration, Semgrep, zizmor, and DCO succeeded; Windows was skipped. No CI polling or runtime/merge-readiness claim. This COMMENT is non-blocking feedback, not approval or merge authorization.
| onSection?: (section: string) => void; | ||
| navigationPane?: boolean; | ||
| }) { | ||
| useEffect(() => cards.retainVisibility(), [cards]); |
There was a problem hiding this comment.
[P2] Acquire route visibility before rejecting an addressed Membership section
This demand exists only while the Settings detail mounts, but useAppNavigation treats a card missing from the visibility-filtered snapshot as unavailable once its plugin is active (src/app/navigation.ts:86–109). App.tsx:165–186 then renders the navigation failure instead of Settings, so this effect cannot resolve the missing authorization.
A concrete history path is: open Membership in community A, select community B (which resets the shared membership projection), then use Back to return to Membership A. The target waits for A, but the absent card also sets failure; the failure branch returns before selecting A (navigation.ts:172–195). Neither the failure screen nor SettingsSidebar retains visibility, and Retry only retries the same navigation (270–285), so the owner/admin remains stuck until they leave the addressed route and open generic Settings. Reloading the Membership URL with a slow roster read can also fail prematurely; a later successful read does not clear the already-failed attempt.
Resolve the target community and retain permission-read demand at the Settings-route lifetime, including pending/error presentation, and distinguish pending authorization from an unavailable contribution before failing the route. Keep unverified navigation hidden. Add a real App/navigation regression for returning to Membership across a community switch and for a restored Membership URL with a held roster response; checking Settings or the sidebar in isolation misses this boundary.
21f6ab3 to
c276d39
Compare
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
d2b6a1b to
e141266
Compare
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
…d navigation Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No remaining material findings in the bounded takeover repair at 422d45f4. The addressed Membership route now waits for authorization in the correct community, handles read/transport retry without retrying unrelated denied scopes, and preserves section focus. The earlier lifecycle and applied-authorization coverage repairs remain intact. Both independent source-review lanes are clear; their review was source-only.
- Integration: current
main(0a498279) is included. Existing commits were preserved; the PR branch was updated without rewriting history. - Validation: full Vitest passed at merged production revision
31df86a7(433 files, 5,245 tests), as did TypeScript and all four channel-corner browser cases. The final revision adds only a two-line browser synchronization fix; the complete Settings file then passed in Chromium and WebKit (12/12), retaining the post-Enter focus assertion. Final-head push hooks passed types, 2,180 related tests, and design-system guards. The earlier combined browser run had two owner-focus failures; the corrected Settings rerun, not that combined run, supplies the final evidence. - Remaining gates: the delivery snapshot shows DCO passed, CI still running, and required human review outstanding. No native-device validation or new human acceptance is claimed. Please reload Membership as an owner, switch communities and use Back to return, and verify an ordinary member cannot access Membership. Expect pending authorization to recover to the correct community rather than fail early or show Profile.
This is a COMMENT, not approval or merge authorization.
* origin/main: feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/app/App.tsx
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ...
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ... Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
The moderation plugin's settings card became `buzz.moderation/membership`
("Membership") upstream in #348, so the rail's Invite to community item
still pointed at the removed `buzz.moderation/invites` section and would
land on an unavailable section. Point it at the Membership card and bring
the comments and the communities and shell docs in line with the card's
new name and its Invite members button. `useCommunityRole` now serves only
the rail, since the card reads the shared membership store instead.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Why
Community membership controls lived in a global Invites destination, even though every roster read and action applies to one selected community. Ordinary members could also discover the administration surface before the app checked their role.
What
How
One shared membership capability owns the verified roster read, role projection, retry state, and session fencing. Settings uses that projection for navigation visibility, and the Membership page uses the same evidence for rows and mutations.
The invite dialog no longer searches an ambiguous people directory. Direct addition accepts an exact npub or public key, while invite links remain a separate action.
Risk
This changes Settings contribution visibility and the existing community administration entry. Relay authorization remains the final authority for every mutation, and stale evidence locks writes until refresh.
Testing
Clay tested the running app and confirmed the Administration placement and that Membership stays hidden in a community where they are an ordinary member.
Focused Chromium and WebKit journeys exercised owner visibility, ordinary-member gating, the Membership page, and the revised invite dialog.
Bigger picture
This establishes the Administration group for future community capabilities. Community details, moderation, and hosting controls remain separate follow-up work.
Generated with Goose