Grandiras/Ponente

Fork 0

Fix the access spec's setAccess flake, and stop settings flashing "denied" #244

Merged

Grandiras merged 1 commit from fix/access-spec-setaccess-flake into main

2026-10-03 13:29:50 +00:00

Grandiras commented

2026-10-03 01:54:32 +00:00

Owner

setAccess in tests/e2e/access.spec.js flaked in full runs ("a restricted board keeps its admin…", "the last admin cannot be removed"): 15 s timeout with Save access still disabled.

Cause (reproduced)

On an isolated, port-shifted stack, a MutationObserver on the access page shows the same sequence on every visit:

prerender [form] -> circuit's first render [no form, "denied" empty state] -> loaded [form]

The settings page re-renders from scratch when the circuit attaches, before its board has resolved, and SettingsFrame treated "not resolved yet" as "denied". setAccess read the form with one-shot isChecked() / count() calls to decide whether a save was needed. A count() landing in that gap saw no grant row (logged in 3 of 60 unthrottled calls), so the helper went on to "set" access the board already had. Save then never enables.

With the db container CPU-throttled to stretch the gap, a second race showed up. After save.click(), setAccess waited for Save to go disabled, but Save is also disabled mid-save (_busy). The next page load read the old grants, and the super-admin test failed on "last admin".

Fix

Spec: the two no-op calls are gone (a new board is already restricted to its maker, as admin), and so is the already read. setAccess now only asks for changes it can see land. It waits for the unsaved hint (.admin-hint) to go rather than for Save to disable. The admin tick gets its own short timeout, because its box stays disabled until the circuit has the grant, and toPass never interrupts a stuck attempt.
App: SettingsFrame shows the loading spinner until the board resolves, as BoardPage already does. "Denied" no longer flashes for the board's own admin (85–570 ms under the throttled db), and an e2e data-outcome="denied" assertion can't pass on the transient. No changelog line: on a healthy db the flash lasts one round trip, so few members will notice it gone.
Docs: boards-and-permissions.md no longer says Save disabling is how tests know a write landed. testing.md gains the one-shot-read trap.

Verification

Isolated stack (ponenteflake, ports 1809x):

access.spec.js --repeat-each=10: 50/50 unthrottled, 50/50 with db at 0.25 CPU (the old helper failed once under this setting), 50/50 at 0.1 CPU.
Full suite: 157/158. The one failure is search.spec.js "search finds a card…": a modal-backdrop-dockable covers the search box. It never opens a settings page and passes alone, so it's a separate order-dependent flake and isn't touched here.
tools/verify.sh: build + 584 unit tests green.

🤖 Generated with Claude Code

`setAccess` in `tests/e2e/access.spec.js` flaked in full runs ("a restricted board keeps its admin…", "the last admin cannot be removed"): 15 s timeout with **Save access** still disabled. ## Cause (reproduced) On an isolated, port-shifted stack, a MutationObserver on the access page shows the same sequence on every visit: ``` prerender [form] -> circuit's first render [no form, "denied" empty state] -> loaded [form] ``` The settings page re-renders from scratch when the circuit attaches, before its board has resolved, and `SettingsFrame` treated "not resolved yet" as "denied". `setAccess` read the form with one-shot `isChecked()` / `count()` calls to decide whether a save was needed. A `count()` landing in that gap saw no grant row (logged in 3 of 60 unthrottled calls), so the helper went on to "set" access the board already had. Save then never enables. With the db container CPU-throttled to stretch the gap, a second race showed up. After `save.click()`, `setAccess` waited for Save to go disabled, but Save is also disabled **mid-save** (`_busy`). The next page load read the old grants, and the super-admin test failed on "last admin". ## Fix - **Spec**: the two no-op calls are gone (a new board is already restricted to its maker, as admin), and so is the `already` read. `setAccess` now only asks for changes it can see land. It waits for the unsaved hint (`.admin-hint`) to go rather than for Save to disable. The admin tick gets its own short timeout, because its box stays disabled until the circuit has the grant, and `toPass` never interrupts a stuck attempt. - **App**: `SettingsFrame` shows the loading spinner until the board resolves, as `BoardPage` already does. "Denied" no longer flashes for the board's own admin (85–570 ms under the throttled db), and an e2e `data-outcome="denied"` assertion can't pass on the transient. No changelog line: on a healthy db the flash lasts one round trip, so few members will notice it gone. - **Docs**: `boards-and-permissions.md` no longer says Save disabling is how tests know a write landed. `testing.md` gains the one-shot-read trap. ## Verification Isolated stack (`ponenteflake`, ports 1809x): - `access.spec.js --repeat-each=10`: 50/50 unthrottled, 50/50 with db at 0.25 CPU (the old helper failed once under this setting), 50/50 at 0.1 CPU. - Full suite: 157/158. The one failure is `search.spec.js` "search finds a card…": a `modal-backdrop-dockable` covers the search box. It never opens a settings page and passes alone, so it's a separate order-dependent flake and isn't touched here. - `tools/verify.sh`: build + 584 unit tests green. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-10-03 01:54:33 +00:00

Fix the access spec's setAccess flake, and stop settings flashing "denied"

CI / build (pull_request) Successful in 42s

Details

acf1fb223f

When the circuit attaches, a settings page's first interactive render
replaces the prerendered markup before its board has loaded, and showed
the "denied" empty state for that moment. setAccess read the form with
one-shot isChecked()/count() calls to decide whether a save was needed;
a count() landing in that gap saw no grant row, so it went on to "set"
access the board already had, and Save never enabled (15 s timeout).

- setAccess no longer decides whether to save: its only two no-op
  callers (a new board is already restricted to its maker, as admin)
  drop the call.
- It waits for the unsaved hint to go, not for Save to disable, which
  it also does mid-save: on a slow database the next step could read
  the old grants (reproduced with the db container CPU-throttled).
- An admin tick times out inside the retry, since its box stays
  disabled until the circuit has the grant.
- SettingsFrame shows a spinner until the board resolves, as BoardPage
  does, so "denied" no longer flashes for the board's own admin, and an
  e2e "denied" assertion cannot pass on the transient.

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

claude-bot commented

2026-10-03 01:54:36 +00:00

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

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

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/677); 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-244/run-677.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-10-03 01:55:04 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

The SettingsFrame fix is sound. BoardOutcome.Found is the enum default, so every settings page starts out Board == null && Outcome == Found before it resolves. That is exactly the window the spinner now covers. The Missing, Denied and Deleted outcomes, which resolve with Board == null, still fall through to the existing branches. The board-loading markup matches what BoardPage uses, and the docs are updated alongside it.

The spec changes are also reasonable. setAccess no longer reads one-shot state. It waits on the unsaved hint rather than on Save disabling, which also happens mid-save. The two removed no-op setAccess calls rely on a new board being restricted to its maker, which the comment and docs state.

Two minor notes, neither blocking:

The changelog line is skipped on the grounds that few members will notice. That is defensible.
The unrelated search.spec.js flake is correctly left out of scope.

🧠 claude-sonnet-5-5 · ⏱ 0m 10s · 5 turns · tokens: 17.0k in / 983 out (+92.1k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The `SettingsFrame` fix is sound. `BoardOutcome.Found` is the enum default, so every settings page starts out `Board == null && Outcome == Found` before it resolves. That is exactly the window the spinner now covers. The `Missing`, `Denied` and `Deleted` outcomes, which resolve with `Board == null`, still fall through to the existing branches. The `board-loading` markup matches what `BoardPage` uses, and the docs are updated alongside it. The spec changes are also reasonable. `setAccess` no longer reads one-shot state. It waits on the unsaved hint rather than on Save disabling, which also happens mid-save. The two removed no-op `setAccess` calls rely on a new board being restricted to its maker, which the comment and docs state. Two minor notes, neither blocking: - The changelog line is skipped on the grounds that few members will notice. That is defensible. - The unrelated `search.spec.js` flake is correctly left out of scope. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 10s · 5 turns · tokens: 17.0k in / 983 out (+92.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-244/run-677.md)

opencode-bot commented

2026-10-03 01:58:05 +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-244/run-2026-10-03T01-58-05-781Z.md) — updating every ~5 minutes while the review runs.

opencode-reviewer approved these changes

2026-10-03 01:59:39 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

The fix is sound and well-reasoned.

App change: The spinner condition in SettingsFrame is correct. BoardResolution distinguishes Found/Deleted/Missing/Denied, and SettingsPage._outcome starts as Found with _board null, so Board is null && Outcome == Found can only mean "not resolved yet" — after ResolveAsync, a null board always comes with a non-Found outcome, so no page can spin forever on a deleted/missing/denied board. The markup reuses the existing .board-loading/.spinner pattern used by BoardPage and every other settings page. Since the child content only renders in the CanManage branch, SettingsTabs' Board!.Key can't deref null.

Spec change: The already removal is safe — all four remaining setAccess calls request a state that differs from what the board has, so Save always enables (and the docstring now states the precondition explicitly). Waiting on .access-save .admin-hint instead of Save-disabled correctly skips the _busy window (Save disables both mid-save and post-write, the hint only goes when Dirty turns false). If a save is refused, Dirty stays true and the hint stays, so the helper still fails loudly rather than passing vacuously. The 1 s timeout on the admin tick's check() is justified: the box is disabled until its grant is ticked. expectDenied's toBeVisible retries past the new transient spinner, and the same is true for the denied assertion after untickAndSave in the super-admin test.

The doc updates match the new behavior. The separate search.spec.js flake being out of scope is reasonable given it doesn't touch settings and passes alone.

⏱ 1m 33s · 9 steps · $0.0120 · tokens: 59.4k in / 967 out (+55.9k cache-read) · Go: 5h 4% · wk 9% · mo 87%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The fix is sound and well-reasoned. **App change**: The spinner condition in `SettingsFrame` is correct. `BoardResolution` distinguishes `Found`/`Deleted`/`Missing`/`Denied`, and `SettingsPage._outcome` starts as `Found` with `_board` null, so `Board is null && Outcome == Found` can only mean "not resolved yet" — after `ResolveAsync`, a null board always comes with a non-`Found` outcome, so no page can spin forever on a deleted/missing/denied board. The markup reuses the existing `.board-loading`/`.spinner` pattern used by `BoardPage` and every other settings page. Since the child content only renders in the `CanManage` branch, `SettingsTabs`' `Board!.Key` can't deref null. **Spec change**: The `already` removal is safe — all four remaining `setAccess` calls request a state that differs from what the board has, so Save always enables (and the docstring now states the precondition explicitly). Waiting on `.access-save .admin-hint` instead of Save-disabled correctly skips the `_busy` window (Save disables both mid-save and post-write, the hint only goes when `Dirty` turns false). If a save is refused, `Dirty` stays true and the hint stays, so the helper still fails loudly rather than passing vacuously. The 1 s timeout on the admin tick's `check()` is justified: the box is `disabled` until its grant is ticked. `expectDenied`'s `toBeVisible` retries past the new transient spinner, and the same is true for the denied assertion after `untickAndSave` in the super-admin test. The doc updates match the new behavior. The separate `search.spec.js` flake being out of scope is reasonable given it doesn't touch settings and passes alone. --- ⏱ 1m 33s · 9 steps · $0.0120 · tokens: 59.4k in / 967 out (+55.9k cache-read) · Go: 5h 4% · wk 9% · mo 87% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-244/run-2026-10-03T01-58-05-781Z.md)

Grandiras merged commit 84c1e4642c into main

2026-10-03 13:29:50 +00:00

Grandiras deleted branch fix/access-spec-setaccess-flake

2026-10-03 13:29:50 +00:00

Grandiras referenced this pull request from a commit

2026-10-03 13:29:52 +00:00

Fix the access spec's setAccess flake, and stop settings flashing "denied" (#244)