Grandiras/Ponente

Fork 0

Drop Members.Role and make Boards.Key required #234

Merged

Grandiras merged 2 commits from issue-219-drop-role-key-required into main

2026-09-29 13:04:17 +00:00

Grandiras commented

2026-09-29 01:33:35 +00:00

Owner

Fixes #219.

Drops Members.Role and makes Boards.Key NOT NULL in one migration, RequireBoardKeyDropMemberRole; removes WarnAboutUnconfiguredAdminsAsync and the transitional notes in code and docs/.

EnsureBoardKeysAsync now runs before MigrateAsync (raw SQL on Id/CreatedAt, only once AddBoardKeys is applied), so a board a rolled-back release made without a key gets one before the ALTER. The migration has no default on purpose: a '' would trip the unique index rather than fail loudly.

Tested against a database at AddMemberBans holding two keyless boards and a role-1 member (keys backfilled, column NOT NULL, Role gone), against a fresh database, and through Down and back. tools/verify.sh green. No changelog line: maintainer-only.

🤖 Generated with Claude Code

Fixes #219. Drops `Members.Role` and makes `Boards.Key` `NOT NULL` in one migration, `RequireBoardKeyDropMemberRole`; removes `WarnAboutUnconfiguredAdminsAsync` and the transitional notes in code and `docs/`. `EnsureBoardKeysAsync` now runs *before* `MigrateAsync` (raw SQL on `Id`/`CreatedAt`, only once `AddBoardKeys` is applied), so a board a rolled-back release made without a key gets one before the `ALTER`. The migration has no default on purpose: a `''` would trip the unique index rather than fail loudly. Tested against a database at `AddMemberBans` holding two keyless boards and a role-1 member (keys backfilled, column NOT NULL, Role gone), against a fresh database, and through `Down` and back. `tools/verify.sh` green. No changelog line: maintainer-only. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-09-29 01:33:35 +00:00

Drop Members.Role and make Boards.Key required

CI / build (pull_request) Successful in 40s

Details

06971cef2c

The two columns #190's migration rule kept in a transitional shape, finished
now that v2.7.0 and v2.8.0 have shipped. WarnAboutUnconfiguredAdminsAsync goes
with the column it read. EnsureBoardKeysAsync now runs before the migration,
in raw SQL, so a keyless board left by a rollback cannot fail the NOT NULL.
Tested on a database at AddMemberBans with two keyless boards and a role-1
member, on a fresh one, and through Down.

Fixes #219

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

opencode-bot commented

2026-09-29 01:33:37 +00:00

Collaborator

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

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-234/run-2026-09-29T01-33-36-375Z.md) — updating every ~5 minutes while the review runs.

opencode-reviewer reviewed

2026-09-29 02:03:39 +00:00

opencode-reviewer left a comment

💬 Verdict: comment — the reviewer did not submit a structured review; the raw output follows.

(empty)

⏱ 30m 3s · 14 steps · $0.0658 · tokens: 224.4k in / 826 out (+220.7k cache-read) · Go: 5h 4% · wk 2% · mo 84%
📜 Full session transcript

💬 **Verdict: comment** — the reviewer did not submit a structured review; the raw output follows. (empty) --- ⏱ 30m 3s · 14 steps · $0.0658 · tokens: 224.4k in / 826 out (+220.7k cache-read) · Go: 5h 4% · wk 2% · mo 84% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-234/run-2026-09-29T01-33-36-375Z.md)

Grandiras added 1 commit

2026-09-29 12:42:44 +00:00

Merge origin/main into issue-219-drop-role-key-required

CI / build (pull_request) Successful in 45s

Details

7116aaf711

Reapplies the change on the comments trimmed in #229; the migration is unchanged.

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

opencode-bot commented

2026-09-29 12:43:45 +00:00

Collaborator

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

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-234/run-2026-09-29T12-43-44-974Z.md) — updating every ~5 minutes while the review runs.

opencode-reviewer approved these changes

2026-09-29 13:04:08 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a well-reasoned finalization of the two transitional shapes (Boards.Key nullable, Members.Role unread), and I verified the risky parts rather than assuming them:

Ordering fix is sound. EnsureBoardKeysAsync now runs before MigrateAsync, guarded by a check that _AddBoardKeys is in the applied list. I checked EF Core's HistoryRepository.GetAppliedMigrationsAsync() source: it calls ExistsAsync() first and returns an empty list on a fresh database, so the guard short-circuits safely on first boot, and the raw SQL only runs once the Key column actually exists. This correctly repairs a keyless board before the SET NOT NULL ALTER.
APIs check out. ExecuteSqlAsync and SqlQueryRaw<T> exist in EF Core 7–10 (this repo is on 10.0.12); interpolated values are parameterized, and the values here are server-generated anyway. The KeylessBoard record binds by column name through the ad-hoc mapper, and the PR's test against a database at AddMemberBans with two keyless boards exercised exactly that path.
Every insert path supplies a key. Board.Key has a non-nullable initializer (= BoardKey.New()), so SeedDefaultBoardAsync, BoardService.CreateAsync and tests are all covered by the now-required property.
The migration pair is internally consistent. Up sets no default on purpose — correct, a '' default would trip IX_Boards_Key on the second keyless board. Down restores Members.Role as NOT NULL DEFAULT 0, which is the shape AddBoardRoles left (the rollback build doesn't map the column, so its INSERTs omit it and the default is what keeps them working), and makes Key nullable for the rollback build's model — both accurate per AddBoardRoles' own comments.
Members.Role is truly unread: grep confirms nothing outside migration history references it; the warning method removal is matched in docs/auth.md. Docs are updated in the same PR per convention; maintainer-only change, so no changelog line is right.

One acknowledged trade-off, not a blocker: a database older than AddBoardKeys that jumps straight to this release (timer down for several releases) will fail its first boot loudly at SET NOT NULL, since the backfill can't run before the column exists; recovery is one boot of the intermediate release. The autoupdate timer makes that window narrow and the loud-failure stance is stated in the migration comment, so I take it as intended. My previous pass left no findings, so nothing was outstanding.

⏱ 20m 23s · 14 steps · $0.0503 · tokens: 141.8k in / 2.0k out (+499.3k cache-read) · Go: 5h 0% · wk 2% · mo 84%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a well-reasoned finalization of the two transitional shapes (`Boards.Key` nullable, `Members.Role` unread), and I verified the risky parts rather than assuming them: - **Ordering fix is sound.** `EnsureBoardKeysAsync` now runs before `MigrateAsync`, guarded by a check that `_AddBoardKeys` is in the applied list. I checked EF Core's `HistoryRepository.GetAppliedMigrationsAsync()` source: it calls `ExistsAsync()` first and returns an empty list on a fresh database, so the guard short-circuits safely on first boot, and the raw SQL only runs once the `Key` column actually exists. This correctly repairs a keyless board before the `SET NOT NULL` ALTER. - **APIs check out.** `ExecuteSqlAsync` and `SqlQueryRaw<T>` exist in EF Core 7–10 (this repo is on 10.0.12); interpolated values are parameterized, and the values here are server-generated anyway. The `KeylessBoard` record binds by column name through the ad-hoc mapper, and the PR's test against a database at `AddMemberBans` with two keyless boards exercised exactly that path. - **Every insert path supplies a key.** `Board.Key` has a non-nullable initializer (`= BoardKey.New()`), so `SeedDefaultBoardAsync`, `BoardService.CreateAsync` and tests are all covered by the now-required property. - **The migration pair is internally consistent.** `Up` sets no default on purpose — correct, a `''` default would trip `IX_Boards_Key` on the second keyless board. `Down` restores `Members.Role` as `NOT NULL DEFAULT 0`, which is the shape `AddBoardRoles` left (the rollback build doesn't map the column, so its INSERTs omit it and the default is what keeps them working), and makes `Key` nullable for the rollback build's model — both accurate per `AddBoardRoles`' own comments. - **`Members.Role` is truly unread**: grep confirms nothing outside migration history references it; the warning method removal is matched in `docs/auth.md`. Docs are updated in the same PR per convention; maintainer-only change, so no changelog line is right. One acknowledged trade-off, not a blocker: a database older than `AddBoardKeys` that jumps straight to this release (timer down for several releases) will fail its first boot loudly at `SET NOT NULL`, since the backfill can't run before the column exists; recovery is one boot of the intermediate release. The autoupdate timer makes that window narrow and the loud-failure stance is stated in the migration comment, so I take it as intended. My previous pass left no findings, so nothing was outstanding. --- ⏱ 20m 23s · 14 steps · $0.0503 · tokens: 141.8k in / 2.0k out (+499.3k cache-read) · Go: 5h 0% · wk 2% · mo 84% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-234/run-2026-09-29T12-43-44-974Z.md)

Grandiras merged commit d60b117e73 into main

2026-09-29 13:04:17 +00:00

Grandiras referenced this pull request from a commit

2026-09-29 13:04:18 +00:00

Merge pull request 'Drop Members.Role and make Boards.Key required' (#234) from issue-219-drop-role-key-required into main

Grandiras referenced this pull request from a commit

2026-10-02 23:27:18 +00:00

The canvas seed fits the current schema, and its dates follow the run

Grandiras referenced this pull request

2026-10-02 23:27:41 +00:00

Fix the canvas seed for the current schema, and retrace from the v2 restyle #239