fix(permissions): keep board-access reads behind confirmed writes - #3330
Chris0Jeky wants to merge 15 commits into
Conversation
|
Fresh source-backed verification for exact head
The permissions cases cover read→revoke/update, reverse reads, independent-board loading, same-user logout/login, old-session mutation and stale failure. The canonical suite additionally covers grant. The test-only Required run was superseded/cancelled, so no hosted canonical-red claim is made. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71a20bc8ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72d71c8500
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| error.value = null | ||
| session.requireUserId('board access management') | ||
| const access = await boardAccessApi.grantAccess(boardId, dto) | ||
| if (!ownsSession(owner)) return access |
There was a problem hiding this comment.
When a same-user token refresh occurs while a grant is awaiting its response, this branch resolves the mutation successfully but discards its cache update and success receipt; BoardAccessView.handleGrant consequently clears and closes the form even though the newly granted entry remains absent. Update and revoke have the same behavior, and the token watcher only retries unresolved reads, so no automatic reconciliation occurs. Re-fetch the affected board under the replacement token after a stale mutation settles rather than silently returning its result.
Useful? React with 👍 / 👎.
1b69685 to
6f88600
Compare
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9882632580
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return | ||
| } | ||
|
|
||
| if (activeReadByBoard.has(boardId)) return |
There was a problem hiding this comment.
When a token rotates while a mutation is pending, the retired operation drops loading, allowing a new same-board refresh before the mutation commits. That GET can snapshot the pre-mutation access list yet remain active until the mutation response arrives; this early return then treats it as reconciliation, and the stale response becomes the final cache state with no follow-up read. Fresh evidence at exact head 988263258 is this new active-read short circuit; the regression test covers only the case with no active read. Await or supersede the active read and ensure a read started after mutation settlement reconciles the committed grant, update, or revoke.
Useful? React with 👍 / 👎.
|
@codex review |
3447837 to
c9abfd0
Compare
|
@codex review |
|
@codex review |
1 participant
Closes #3328. Related residual: #3352.
Summary
Makes board-access cache writes, messaging and loading explicitly owned by the initiating read/mutation and session lifetime:
Server authorization remains authoritative. Same-entry mutation serialization remains in stacked PR #3335.
Test-first evidence
The original suite covers read-versus-grant/update/revoke races, reverse reads, independent loading, same-user relogin, stale mutation settlement and grant deduplication. Review-regression head
6bbf8d04139bef2dca91d290b910e1c07a8e76aaran canonical Ubuntu/Windows frontend qualification; Ubuntu JUnit recorded 7,161 tests, exactly 2 failures, 0 errors, both loaded-cache preservation cases.Issue #3352 added test-only head
d28697ede737ec42ae3e696c7a7cedcb753de347, covering token rotation while a board has no cache entry and its first read is pending. A dependency-free actual-module runner changed from onegetAccesscall with loading false to two calls for the same board with old settlement suppressed and the fresh result installed.Supplemental execution does not replace hosted qualification.
Current head and remaining gates
Exact final head:
72d71c8500f534c10e8bb70c025922f9381f0103Keep draft until this exact head passes Smart CI, Extended and the complete Required CI matrix on Ubuntu and Windows, followed by repeat independent review. Stacked PR #3335 must then be reconciled to this corrected parent and requalified.
Green CI is qualification evidence, not merge authorization.