QA pass over the member-owned boards and home page work #212

Merged

Grandiras merged 1 commit from qa-home-and-boards into main 2026-09-22 19:28:02 +00:00

Conversation 2 Commits 1 Files changed 45 +865 -293

Grandiras commented 2026-09-22 19:23:44 +00:00

Owner

Copy link

A QA pass over the merged #191–#195 (member-owned boards, the home page, per-member order). I reviewed the code and ran the full e2e suite against main: 16 of 158 tests failed, because the bots merged without running e2e, which CI doesn't run. All 158 pass on this branch. Two tests are timing-flaky and have been hardened; both are noted below.

Closes #205, #209, #210.

Security

• Any member could reorder any board's columns. Settings/Columns.OnColumnMoved is [JSInvokable] and takes its column id from the DOM, and RequireAdminister only asked whether the viewer was an admin, not an admin of which board. Every member is an admin of the boards they make, so every member qualified. BoardViewer now carries the BoardId it was built for, and RequireAdminister(actor, boardId) refuses any mismatch (BoardPermissionsTests).
• The same shape applied to cards: BoardPage.OnCardMoved → MoveCardAsync took no viewer, so a member could reorder cards on a board they can't see. It now takes the board the page resolved and refuses a card that isn't on it.

Behaviour fixes

• Removing a member hands their boards on. The super-admin couldn't remove anyone who had ever made a board, and the refusal told them to "make somebody else an admin first", which they can't do on a board they can't open. Removal now promotes the longest-standing other member (BoardSuccession, BoardSuccessionTests). It refuses only when a board has nobody else on it.
• The default board is seeded only into a database being created. It used to be seeded whenever no boards existed, so a deployment whose members deleted every board got a shared "Board" back on the next restart.
• The startup admin repair no longer crashes the boot when a super-admin already holds a plain grant on an admin-less board. It promotes that row instead of inserting a duplicate key.
• Former deployment admins are named at startup. Anyone promoted in the old UI who isn't in Allowlist:Admins loses /admin, so the app now logs a warning per DID. There's also a note in deploy/README.md.
• "For you" stops showing other people's long-missed deadlines. A shared board's finished cards keep their old dates, and six of them from July filled the strip permanently. Deadlines missed more than a week ago now only show on your own cards (ForYouService.Lookback).
• Last page:
  • The cookie is forgotten on sign-in and sign-out. It belongs to the browser, so the next member on the same machine resumed onto the previous member's page.
  • A resume now checks /admin pages (for a non-super-admin, following one would loop) and a board's settings pages.
  • An allowlisted member who is refused a page goes home, instead of to "you're not on the allowlist".
  • /launch/remember accepts a same-host Origin from browsers that send no Sec-Fetch-Site (Safari before 16.4).
• #205: the search box is exactly centred at 1440px again, in English and German. The header's board-settings link now reads "Settings" (its full name stays as the accessible name and tooltip), and the super-admin's Members link is a row in the account menu at every width.
• CommentList no longer re-reads and decrypts the thread on every render of the dialog. The calendar's dialog no longer shows edit controls for a round trip to somebody who can't edit.

Docs

• #209: CardPermissions's class comment.
• #210: auth.md on /admin/boards, frontend.md on the default board.
• Other stale paragraphs in boards-and-permissions.md: the / route, the Admin flyout, ViewerDid, the demotion path, and "a call site cannot ask about a different board".
• ui.md's antiforgery claim (the middleware doesn't check a JSON-bound endpoint).
• testing.md, data.md, and deploy/README.md.

e2e

• A shared helpers/members.js. One of its three copies filled signup fields that don't exist.
• openBoard picks the oldest board. Since #195 the first tile is the one you last opened, so three tests had been asserting nothing.
• A separate browser context for each member. Pages in one context share cookies, so a second sign-in hijacked the first.
• Tests that assumed a new board is everyone's.
• share.spec cleaned up via /.
• tests/e2e/smoke-access.png removed, and screenshots are now ignored.
• Flaky, now retried: filter.spec's drop (a press before the interop re-wires the columns selects text instead) and access.spec's switcher check.

Separately (#211, already merged)

While running this suite, a real OAuth token for the admin DID sat in the local test DB, and profile.spec wrote to that account's real Bluesky profile. #211 makes it impossible for a dev sign-in to act on an Atmosphere account.

🤖 Generated with Claude Code

A QA pass over the merged #191–#195 (member-owned boards, the home page, per-member order). I reviewed the code and ran the full e2e suite against `main`: **16 of 158 tests failed**, because the bots merged without running e2e, which CI doesn't run. All 158 pass on this branch. Two tests are timing-flaky and have been hardened; both are noted below. Closes #205, #209, #210. ## Security - **Any member could reorder any board's columns.** `Settings/Columns.OnColumnMoved` is `[JSInvokable]` and takes its column id from the DOM, and `RequireAdminister` only asked whether the viewer was an admin, not an admin *of which board*. Every member is an admin of the boards they make, so every member qualified. `BoardViewer` now carries the `BoardId` it was built for, and `RequireAdminister(actor, boardId)` refuses any mismatch (`BoardPermissionsTests`). - The same shape applied to cards: `BoardPage.OnCardMoved` → `MoveCardAsync` took no viewer, so a member could reorder cards on a board they can't see. It now takes the board the page resolved and refuses a card that isn't on it. ## Behaviour fixes - **Removing a member hands their boards on.** The super-admin couldn't remove anyone who had ever made a board, and the refusal told them to "make somebody else an admin first", which they can't do on a board they can't open. Removal now promotes the longest-standing other member (`BoardSuccession`, `BoardSuccessionTests`). It refuses only when a board has nobody else on it. - **The default board is seeded only into a database being created.** It used to be seeded whenever no boards existed, so a deployment whose members deleted every board got a shared "Board" back on the next restart. - **The startup admin repair no longer crashes the boot** when a super-admin already holds a plain grant on an admin-less board. It promotes that row instead of inserting a duplicate key. - **Former deployment admins are named at startup.** Anyone promoted in the old UI who isn't in `Allowlist:Admins` loses `/admin`, so the app now logs a warning per DID. There's also a note in `deploy/README.md`. - **"For you" stops showing other people's long-missed deadlines.** A shared board's finished cards keep their old dates, and six of them from July filled the strip permanently. Deadlines missed more than a week ago now only show on your own cards (`ForYouService.Lookback`). - **Last page:** - The cookie is forgotten on sign-in and sign-out. It belongs to the browser, so the next member on the same machine resumed onto the previous member's page. - A resume now checks `/admin` pages (for a non-super-admin, following one would loop) and a board's settings pages. - An allowlisted member who is refused a page goes home, instead of to "you're not on the allowlist". - `/launch/remember` accepts a same-host `Origin` from browsers that send no `Sec-Fetch-Site` (Safari before 16.4). - **#205:** the search box is exactly centred at 1440px again, in English and German. The header's board-settings link now reads "Settings" (its full name stays as the accessible name and tooltip), and the super-admin's `Members` link is a row in the account menu at every width. - `CommentList` no longer re-reads and decrypts the thread on every render of the dialog. The calendar's dialog no longer shows edit controls for a round trip to somebody who can't edit. ## Docs - #209: `CardPermissions`'s class comment. - #210: `auth.md` on `/admin/boards`, `frontend.md` on the default board. - Other stale paragraphs in `boards-and-permissions.md`: the `/` route, the Admin flyout, `ViewerDid`, the demotion path, and "a call site cannot ask about a different board". - `ui.md`'s antiforgery claim (the middleware doesn't check a JSON-bound endpoint). - `testing.md`, `data.md`, and `deploy/README.md`. ## e2e - A shared `helpers/members.js`. One of its three copies filled signup fields that don't exist. - `openBoard` picks the oldest board. Since #195 the first tile is the one you last opened, so three tests had been asserting nothing. - A separate browser context for each member. Pages in one context share cookies, so a second sign-in hijacked the first. - Tests that assumed a new board is everyone's. - `share.spec` cleaned up via `/`. - `tests/e2e/smoke-access.png` removed, and screenshots are now ignored. - Flaky, now retried: `filter.spec`'s drop (a press before the interop re-wires the columns selects text instead) and `access.spec`'s switcher check. ## Separately (#211, already merged) While running this suite, a real OAuth token for the admin DID sat in the local test DB, and `profile.spec` wrote to that account's real Bluesky profile. #211 makes it impossible for a dev sign-in to act on an Atmosphere account. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit 2026-09-22 19:23:44 +00:00

QA pass over the member-owned boards and home page work ... e9421b9dd8

Fixes found reviewing #191–#195 and running the e2e suite against them
(16 of 158 failed on main; all pass here).

Security:
- A board admin's viewer now carries the board it was built for, and
  RequireAdminister refuses any other: the column list's drag handler
  takes ids from the DOM, so every member (each is an admin of a board
  they made) could reorder any board's columns. MoveCardAsync likewise
  takes the page's board and refuses a card that is not on it.

Behaviour:
- Removing a member hands each board they were the last admin of to its
  longest-standing other member (BoardSuccession); it used to refuse with
  advice the super-admin could not follow.
- The default board is seeded only into a database being created, not
  whenever no boards exist.
- The startup admin repair promotes a grant it already has instead of
  inserting a duplicate key and failing the boot.
- Former deployment admins who are not configured are named at startup.
- "For you" stops showing other people's deadlines missed over a week
  ago, which filled the strip for good on a shared board.
- The last-page cookie is forgotten on sign-in and sign-out; a resume
  checks /admin pages and a board's settings pages too; an allowlisted
  member refused a page goes home rather than to "not on the allowlist";
  /launch/remember accepts a same-host Origin from browsers that send no
  Sec-Fetch-Site.
- The comment thread no longer re-reads on every render of the dialog,
  and the calendar's dialog no longer flashes its edit controls.
- The header's search box is centred again at 1440px: the board-settings
  link reads "Settings", and Members is a row in the account menu.

Docs brought back in line (CardPermissions, auth, frontend, ui, testing,
data, boards-and-permissions, deploy). e2e: a shared signup helper,
openBoard picks the oldest board, separate browser contexts per member,
and the tests that assumed a new board is everyone's.

Fixes #205
Fixes #209
Fixes #210

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

opencode-bot commented 2026-09-22 19:23:45 +00:00

Collaborator

Copy link

🤖 Reviewing this PR — the review will link the full session transcript.

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review -->

opencode-reviewer approved these changes 2026-09-22 19:27:31 +00:00

opencode-reviewer left a comment

Copy link

✅ Verdict: approve — no blocking issues found.

A thorough QA pass, and the two security holes are closed correctly, not cosmetically.

Security fixes verified against the code:

• RequireAdminister(actor, boardId) now refuses a viewer built for a different board. I checked every call site: BoardService (rename/delete/SetAccess), ColumnService and TagService all pass the board of the row being written, and every page-built viewer comes from BoardService.ViewerOnAsync(boardId, did), which carries the BoardId. BoardViewer.For(...) (BoardId = 0) appears only on read paths, so the new check cannot refuse a legitimate admin. BoardPermissionsTests pins both the mismatch and the BoardId-less viewer.
• MoveCardAsync takes the board the page resolved and refuses a card not on it; BoardPage is the only caller and passes _board.Id.

Behaviour fixes verified:

• MemberService.RemoveAsync promotes the longest-standing other member in the same save (tracked entities, one SaveChangesAsync), and throws LastBoardAdminException before any write when a board has nobody else — so a partial promotion can never land. BoardSuccession is pure and pinned, including the tie-break.
• The default board is seeded only when no migration had been applied before this boot, asked before MigrateAsync — right in both ordinary cases (fresh DB, existing DB).
• EnsureBoardAdminsAsync promotes a held plain grant instead of inserting a duplicate (board, DID) key.
• The resume checks (IsSuperAdminPage, IsAboutBoard including settings pages) and the sign-in/sign-out cookie forget are consistent, and the / middleware renders home on a refused target rather than redirecting, so the loop described in the PR can't happen.
• ForYouService.Lookback, the CommentList reload guard (record-struct equality is the right tool), and the Calendar modal viewer being computed before the dialog renders are all correct.

Docs and tests are updated in the same PR as the convention requires, the e2e helper consolidation removes the copy that could never have passed, and the two flaky tests are hardened rather than papered over.

Two non-blocking observations:

• A database whose one and only migration run crashed partway (some migrations applied, no boards) no longer gets the default board re-seeded on the next boot, where the old "whenever there are no boards" rule would have repaired it. Narrow, self-inflicted edge; the new rule is right for every ordinary case.
• The OnRedirectToAccessDenied override always redirects, dropping the default handler's AJAX→403 branch. Harmless here since Blazor circuit authorization doesn't travel that HTTP path, but worth knowing if a fetch-based caller ever hits an authorized endpoint.

No Changelog.Unreleased additions, but every change here refines behaviour of features already listed in the unreleased block, so nothing member-visible is going unannounced.

---

---

⏱ 3m 46s · 14 steps · $0.0632 · tokens: 261.8k in / 9.6k out (+637.7k cache-read) · Go: 5h 9% · wk 3% · mo 34%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. A thorough QA pass, and the two security holes are closed correctly, not cosmetically. **Security fixes verified against the code:** - `RequireAdminister(actor, boardId)` now refuses a viewer built for a different board. I checked every call site: `BoardService` (rename/delete/SetAccess), `ColumnService` and `TagService` all pass the board of the row being written, and every page-built viewer comes from `BoardService.ViewerOnAsync(boardId, did)`, which carries the `BoardId`. `BoardViewer.For(...)` (BoardId = 0) appears only on read paths, so the new check cannot refuse a legitimate admin. `BoardPermissionsTests` pins both the mismatch and the BoardId-less viewer. - `MoveCardAsync` takes the board the page resolved and refuses a card not on it; `BoardPage` is the only caller and passes `_board.Id`. **Behaviour fixes verified:** - `MemberService.RemoveAsync` promotes the longest-standing other member in the same save (tracked entities, one `SaveChangesAsync`), and throws `LastBoardAdminException` before any write when a board has nobody else — so a partial promotion can never land. `BoardSuccession` is pure and pinned, including the tie-break. - The default board is seeded only when no migration had been applied before this boot, asked before `MigrateAsync` — right in both ordinary cases (fresh DB, existing DB). - `EnsureBoardAdminsAsync` promotes a held plain grant instead of inserting a duplicate (board, DID) key. - The resume checks (`IsSuperAdminPage`, `IsAboutBoard` including settings pages) and the sign-in/sign-out cookie forget are consistent, and the `/` middleware renders home on a refused target rather than redirecting, so the loop described in the PR can't happen. - `ForYouService.Lookback`, the `CommentList` reload guard (record-struct equality is the right tool), and the Calendar modal viewer being computed before the dialog renders are all correct. **Docs and tests** are updated in the same PR as the convention requires, the e2e helper consolidation removes the copy that could never have passed, and the two flaky tests are hardened rather than papered over. Two non-blocking observations: - A database whose one and only migration run crashed partway (some migrations applied, no boards) no longer gets the default board re-seeded on the next boot, where the old "whenever there are no boards" rule would have repaired it. Narrow, self-inflicted edge; the new rule is right for every ordinary case. - The `OnRedirectToAccessDenied` override always redirects, dropping the default handler's AJAX→403 branch. Harmless here since Blazor circuit authorization doesn't travel that HTTP path, but worth knowing if a fetch-based caller ever hits an authorized endpoint. No `Changelog.Unreleased` additions, but every change here refines behaviour of features already listed in the unreleased block, so nothing member-visible is going unannounced. --- --- ⏱ 3m 46s · 14 steps · $0.0632 · tokens: 261.8k in / 9.6k out (+637.7k cache-read) · Go: 5h 9% · wk 3% · mo 34% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-212/run-2026-09-22T19-23-45-537Z.md) <!-- opencode-reviewed-head:e9421b9dd83dbc19bfaea79fde217c454028d1c4 --> <!-- opencode-verdict:approve -->

Grandiras merged commit 454a05fadb into main 2026-09-22 19:28:02 +00:00

Grandiras referenced this pull request from a commit 2026-09-22 19:28:02 +00:00

Merge pull request 'QA pass over the member-owned boards and home page work' (#212) from qa-home-and-boards into main

Grandiras referenced this pull request 2026-09-22 19:42:36 +00:00

Address boards by a stable key instead of their database id #213

Grandiras referenced this pull request 2026-09-22 21:19:18 +00:00

Member-owned boards and a home page: decisions and order of work #190

Sign in to join this conversation.

Reviewers

No reviewers

opencode-reviewer

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Grandiras/Ponente!212

No description provided.