opencode-bot: changes for issue #192 #201

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/560); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-201/run-560.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-18 19:33:50 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This PR moves the deployment-wide admin role onto per-board grants (BoardMember.Role), introduces BoardPermissions as the single gate for running a board (rename/columns/tags/access/delete), and reworks BoardViewer/BoardAccess so a viewer's IsAdmin is now a per-board fact resolved from grant rows rather than the allowlist snapshot. I checked the full PR branch (not just the truncated diff) for the parts that are easy to get wrong in a change like this, and it holds up:

No leftover references to the removed MemberRole enum or old BoardViewer.For(user, allowlist) overload anywhere in the tree.
BoardService.SetAccessAsync computes the last-admin refusal against the post-save state and throws before SaveChangesAsync, so a rejected save can't partially commit — and it correctly allows "remove last admin + add a new one" in one save, as documented.
MemberService.RemoveAsync has its own last-board-admin guard (a member can be the last admin of a board without being a super-admin), independent of the access-page guard.
DatabaseInitializer.EnsureBoardAdminsAsync runs after seeding admins/default board, only touches genuinely admin-less boards, and is idempotent.
The migration's SQL backfill filters on the plaintext Members.Role/BoardMember columns (consistent with them being unencrypted by design) and upserts with ON CONFLICT DO UPDATE against the (BoardId, MemberDid) PK, correctly promoting existing plain grants rather than skipping them.
All new/changed strings (admin.access.admin, admin.access.lastadmin, admin.error.notboardadmin, admin.members.remove.lastadmin, etc.) are present in both en and de dictionaries in LocaleService.cs, and stale keys (admin.members.role*, admin.boards.error.duplicate, admin.access.admin.note) were removed from both.
ColumnService/TagService/BoardService mutators consistently take a BoardViewer actor and call BoardPermissions.RequireAdminister before writing.
docs/ (auth.md, boards-and-permissions.md, data.md, orientation.md) and Changelog.cs (both locales) were updated in the same PR, matching the repo's own convention.
New BoardPermissionsTests cover the sharp edge explicitly (a super-admin is not automatically an admin of a given board).

I didn't find any correctness or security issues worth blocking on.

⏱ 2m 0s · 15 turns · tokens: 94.6k in / 9.1k out (+1434.9k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This PR moves the deployment-wide admin role onto per-board grants (`BoardMember.Role`), introduces `BoardPermissions` as the single gate for running a board (rename/columns/tags/access/delete), and reworks `BoardViewer`/`BoardAccess` so a viewer's `IsAdmin` is now a per-board fact resolved from grant rows rather than the allowlist snapshot. I checked the full PR branch (not just the truncated diff) for the parts that are easy to get wrong in a change like this, and it holds up: - No leftover references to the removed `MemberRole` enum or old `BoardViewer.For(user, allowlist)` overload anywhere in the tree. - `BoardService.SetAccessAsync` computes the last-admin refusal against the *post-save* state and throws before `SaveChangesAsync`, so a rejected save can't partially commit — and it correctly allows "remove last admin + add a new one" in one save, as documented. - `MemberService.RemoveAsync` has its own last-board-admin guard (a member can be the last admin of a board without being a super-admin), independent of the access-page guard. - `DatabaseInitializer.EnsureBoardAdminsAsync` runs after seeding admins/default board, only touches genuinely admin-less boards, and is idempotent. - The migration's SQL backfill filters on the plaintext `Members.Role`/`BoardMember` columns (consistent with them being unencrypted by design) and upserts with `ON CONFLICT DO UPDATE` against the `(BoardId, MemberDid)` PK, correctly promoting existing plain grants rather than skipping them. - All new/changed strings (`admin.access.admin`, `admin.access.lastadmin`, `admin.error.notboardadmin`, `admin.members.remove.lastadmin`, etc.) are present in both `en` and `de` dictionaries in `LocaleService.cs`, and stale keys (`admin.members.role*`, `admin.boards.error.duplicate`, `admin.access.admin.note`) were removed from both. - `ColumnService`/`TagService`/`BoardService` mutators consistently take a `BoardViewer actor` and call `BoardPermissions.RequireAdminister` before writing. - `docs/` (auth.md, boards-and-permissions.md, data.md, orientation.md) and `Changelog.cs` (both locales) were updated in the same PR, matching the repo's own convention. - New `BoardPermissionsTests` cover the sharp edge explicitly (a super-admin is not automatically an admin of a given board). I didn't find any correctness or security issues worth blocking on. --- ⏱ 2m 0s · 15 turns · tokens: 94.6k in / 9.1k out (+1434.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-201/run-560.md)

opencode-reviewer requested changes

2026-09-18 19:34:14 +00:00

Dismissed

opencode-reviewer left a comment

🛑 Verdict: request changes — blocking issues below.

The per-board roles rework is well thought through — BoardPermissions is enforced in every mutator, the last-admin refusal is computed against the post-save state, the migration backfills in SQL with the right ON CONFLICT semantics, and the no-admin-bypass change to BoardAccess.VisibleTo is consistently applied (admin pages keep their access-ignored reads, member-facing paths all build viewers via ViewerOnAsync). Locale keys for the new refusals exist in both languages, and the unit tests were updated to the new BoardViewer shape.

One blocker: the PR removes Role from the Member entity while leaving the column in the database — but that column was created by InitialSchema as integer NOT NULL with no default. EF will now omit it from every INSERT INTO "Members", and Postgres will reject those inserts with a not-null violation. That breaks DatabaseInitializer.SeedAdminsAsync (a fresh database fails at startup, and any deployment where a configured admin lacks a row fails too), /signup, the admin members page and the access-request approval — i.e. every way a member is added. The column needs either a SET DEFAULT 0 in the AddBoardRoles migration, or to stay mapped on the entity (unread) until the later drop.

Minor, non-blocking: Boards.razor CreateAsync passes the current DID, which is null-safe in the service but would create an admin-less board until the next restart's EnsureBoardAdminsAsync repair if the claim were ever missing; and AddBoardRoles.Down() drops BoardMembers.Role without writing post-deploy promotions back to Members.Role, so a rollback loses admins promoted after deploy — consistent with the documented trade-off, just worth being aware of.

⏱ 3m 22s · 15 steps · $0.0630 · tokens: 233.2k in / 14.9k out (+686.7k cache-read) · Go: 5h 41% · wk 41% · mo 22%
📜 Full session transcript

🛑 **Verdict: request changes** — blocking issues below. The per-board roles rework is well thought through — `BoardPermissions` is enforced in every mutator, the last-admin refusal is computed against the post-save state, the migration backfills in SQL with the right `ON CONFLICT` semantics, and the no-admin-bypass change to `BoardAccess.VisibleTo` is consistently applied (admin pages keep their access-ignored reads, member-facing paths all build viewers via `ViewerOnAsync`). Locale keys for the new refusals exist in both languages, and the unit tests were updated to the new `BoardViewer` shape. **One blocker:** the PR removes `Role` from the `Member` entity while leaving the column in the database — but that column was created by `InitialSchema` as `integer NOT NULL` with **no default**. EF will now omit it from every `INSERT INTO "Members"`, and Postgres will reject those inserts with a not-null violation. That breaks `DatabaseInitializer.SeedAdminsAsync` (a fresh database fails at startup, and any deployment where a configured admin lacks a row fails too), `/signup`, the admin members page and the access-request approval — i.e. every way a member is added. The column needs either a `SET DEFAULT 0` in the `AddBoardRoles` migration, or to stay mapped on the entity (unread) until the later drop. Minor, non-blocking: `Boards.razor` `CreateAsync` passes the current DID, which is null-safe in the service but would create an admin-less board until the next restart's `EnsureBoardAdminsAsync` repair if the claim were ever missing; and `AddBoardRoles.Down()` drops `BoardMembers.Role` without writing post-deploy promotions back to `Members.Role`, so a rollback loses admins promoted after deploy — consistent with the documented trade-off, just worth being aware of. --- --- ⏱ 3m 22s · 15 steps · $0.0630 · tokens: 233.2k in / 14.9k out (+686.7k cache-read) · Go: 5h 41% · wk 41% · mo 22% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-201/run-2026-09-18T19-30-52-049Z.md)

src/Ponente.Kanban/Data/Entities/Member.cs

					
				@ -37,3 +31,3 @@

				    public bool UsesLocalProfile { get; set; }

				    public MemberRole Role { get; set; } = MemberRole.Member;

				    // Members.Role is deliberately gone from the entity but still in the

Unmapping Members.Role while the column stays integer NOT NULL with no database default (see InitialSchema, line 36 — no defaultValue) breaks every member insert: EF omits the column from the INSERT and Postgres raises a not-null violation. This hits DatabaseInitializer.SeedAdminsAsync (fresh-DB startup fails), /signup, the admin members page and access-request approval. Either keep the property mapped-but-unread for this release, or add ALTER TABLE "Members" ALTER COLUMN "Role" SET DEFAULT 0 to the AddBoardRoles migration.

Unmapping `Members.Role` while the column stays `integer NOT NULL` with no database default (see `InitialSchema`, line 36 — no `defaultValue`) breaks every member insert: EF omits the column from the INSERT and Postgres raises a not-null violation. This hits `DatabaseInitializer.SeedAdminsAsync` (fresh-DB startup fails), `/signup`, the admin members page and access-request approval. Either keep the property mapped-but-unread for this release, or add `ALTER TABLE "Members" ALTER COLUMN "Role" SET DEFAULT 0` to the `AddBoardRoles` migration.

opencode-bot commented

2026-09-18 19:34:50 +00:00

🤖 On it — the reply will link the full session transcript.

🤖 On it — the reply will link the full session transcript.

opencode-bot added 1 commit

2026-09-18 19:38:04 +00:00

Migrations: default Members.Role to 0 so unmapped inserts still work

CI / build (pull_request) Successful in 37s

Details

59385b417c

Members.Role is unmapped on the entity but stays in the schema as
integer NOT NULL with no default, so every EF INSERT INTO "Members"
omitted it and Postgres rejected it — breaking admin seeding, signup,
member adds and access-request approval. Set DEFAULT 0 in AddBoardRoles
Up (and restore the no-default state in Down).

opencode-bot commented

2026-09-18 19:38:04 +00:00

What I did

The review's blocker was correct, and I fixed it on opencode/issue-192 (commit 59385b4).

The blocker — Members.Role unmapped but NOT NULL with no default. Verified against InitialSchema (line 36: Role = table.Column<int>(..., nullable: false) — no default) and the new model snapshot, which no longer maps the column. Every EF INSERT INTO "Members" would omit it and Postgres would reject it, breaking admin seeding, /signup, member adds and access-request approval.

Fix in AddBoardRoles:

Up: ALTER TABLE "Members" ALTER COLUMN "Role" SET DEFAULT 0 — the inert value, since the real role now lives per board on BoardMembers and the super-admin comes from the allowlist. Comment explains why unmapping alone wasn't enough.
Down: ALTER COLUMN "Role" DROP DEFAULT, restoring the column to exactly the state InitialSchema created, since the build being rolled back to maps the column and writes it explicitly.

Verification (real Postgres 15, local cluster): I wrote a temporary test that ran the full migration chain against a fresh database and then inserted a Member via EF — the exact broken path. Without the fix it fails with 23502: null value in column "Role" of relation "Members" violates not-null constraint; with the fix it passes and information_schema confirms the default is 0. I then removed the temporary test (it needs a live Postgres, so it doesn't belong in the suite) and ran the full test suite: 482/482 passed, build clean.

The two minor findings — deliberately not changed:

Boards.razor CreateAsync with a missing DID claim: the service is null-safe and EnsureBoardAdminsAsync repairs an admin-less board on the next restart; the claim is on an admin-only page, so I left it.
Down() not writing post-deploy promotions back to Members.Role: the reviewer themselves noted this is consistent with the documented rollback trade-off.

No follow-up issues filed — everything actionable was either fixed here or already documented.

⏱ 3m 14s · 39 steps · $0.0399 · tokens: 112.6k in / 5.9k out (+669.7k cache-read) · Go: 5h 42% · wk 41% · mo 23%

📜 Full session transcript

## What I did The review's blocker was correct, and I fixed it on `opencode/issue-192` (commit `59385b4`). **The blocker — `Members.Role` unmapped but `NOT NULL` with no default.** Verified against `InitialSchema` (line 36: `Role = table.Column<int>(..., nullable: false)` — no default) and the new model snapshot, which no longer maps the column. Every EF `INSERT INTO "Members"` would omit it and Postgres would reject it, breaking admin seeding, `/signup`, member adds and access-request approval. Fix in `AddBoardRoles`: - **Up:** `ALTER TABLE "Members" ALTER COLUMN "Role" SET DEFAULT 0` — the inert value, since the real role now lives per board on `BoardMembers` and the super-admin comes from the allowlist. Comment explains why unmapping alone wasn't enough. - **Down:** `ALTER COLUMN "Role" DROP DEFAULT`, restoring the column to exactly the state `InitialSchema` created, since the build being rolled back to maps the column and writes it explicitly. **Verification** (real Postgres 15, local cluster): I wrote a temporary test that ran the full migration chain against a fresh database and then inserted a `Member` via EF — the exact broken path. Without the fix it fails with `23502: null value in column "Role" of relation "Members" violates not-null constraint`; with the fix it passes and `information_schema` confirms the default is `0`. I then removed the temporary test (it needs a live Postgres, so it doesn't belong in the suite) and ran the full test suite: **482/482 passed**, build clean. **The two minor findings — deliberately not changed:** - `Boards.razor` `CreateAsync` with a missing DID claim: the service is null-safe and `EnsureBoardAdminsAsync` repairs an admin-less board on the next restart; the claim is on an admin-only page, so I left it. - `Down()` not writing post-deploy promotions back to `Members.Role`: the reviewer themselves noted this is consistent with the documented rollback trade-off. No follow-up issues filed — everything actionable was either fixed here or already documented. --- ⏱ 3m 14s · 39 steps · $0.0399 · tokens: 112.6k in / 5.9k out (+669.7k cache-read) · Go: 5h 42% · wk 41% · mo 23% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/issue-201/run-2026-09-18T19-34-50-687Z.md)

opencode-bot commented

2026-09-18 19:39:06 +00:00

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

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

opencode-reviewer approved these changes

2026-09-18 19:39:31 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Re-review: the blocker from the previous pass is fixed. The AddBoardRoles migration now runs ALTER TABLE "Members" ALTER COLUMN "Role" SET DEFAULT 0, so the unmapped NOT NULL column no longer breaks member inserts (seeding, /signup, the members page, access-request approval); Down() restores the original no-default state, which is correct since the rolled-back build maps and writes the column. The migration comments document the reasoning well.

Everything else from the previous pass still holds: no leftover MemberRole references, no stale DuplicateBoardNameException or BoardViewer.For(user, allowlist) call sites, BoardPermissions.RequireAdminister enforced in every board mutator, the last-admin refusal computed against the post-save state, and the new refusal strings present in both locales.

Remaining minor, non-blocking notes (unchanged from last pass): AddBoardRoles.Down() drops BoardMembers.Role without writing post-deploy promotions back to Members.Role, so a rollback loses admins promoted after deploy — a documented trade-off; and Boards.razor CreateAsync passes the current DID, which is null-safe in the service but would create an admin-less board until the next restart's EnsureBoardAdminsAsync repair if the claim were ever missing.

⏱ 0m 25s · 4 steps · $0.0107 · tokens: 42.5k in / 1.3k out (+121.3k cache-read) · Go: 5h 42% · wk 41% · mo 23%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Re-review: the blocker from the previous pass is fixed. The `AddBoardRoles` migration now runs `ALTER TABLE "Members" ALTER COLUMN "Role" SET DEFAULT 0`, so the unmapped `NOT NULL` column no longer breaks member inserts (seeding, `/signup`, the members page, access-request approval); `Down()` restores the original no-default state, which is correct since the rolled-back build maps and writes the column. The migration comments document the reasoning well. Everything else from the previous pass still holds: no leftover `MemberRole` references, no stale `DuplicateBoardNameException` or `BoardViewer.For(user, allowlist)` call sites, `BoardPermissions.RequireAdminister` enforced in every board mutator, the last-admin refusal computed against the post-save state, and the new refusal strings present in both locales. Remaining minor, non-blocking notes (unchanged from last pass): `AddBoardRoles.Down()` drops `BoardMembers.Role` without writing post-deploy promotions back to `Members.Role`, so a rollback loses admins promoted after deploy — a documented trade-off; and `Boards.razor` `CreateAsync` passes the current DID, which is null-safe in the service but would create an admin-less board until the next restart's `EnsureBoardAdminsAsync` repair if the claim were ever missing. --- --- ⏱ 0m 25s · 4 steps · $0.0107 · tokens: 42.5k in / 1.3k out (+121.3k cache-read) · Go: 5h 42% · wk 41% · mo 23% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-201/run-2026-09-18T19-39-06-323Z.md)

Grandiras merged commit 8817967e29 into main

2026-09-18 19:40:21 +00:00

Grandiras deleted branch opencode/issue-192

2026-09-18 19:40:21 +00:00

Grandiras referenced this pull request from a commit

2026-09-18 19:40:23 +00:00

opencode-bot: changes for issue #192 (#201)

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.

No reviewers