Skip to content

Distinguish lost board access when loading parent choices - #2991

Merged
Chris0Jeky merged 1 commit into
mainfrom
issue-2974/parent-field-access-loss
Sep 11, 2026
Merged

Distinguish lost board access when loading parent choices#2991
Chris0Jeky merged 1 commit into
mainfrom
issue-2974/parent-field-access-loss

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

What

CardParentField now distinguishes lost board access from a transient read failure when it loads the parent candidates for a card.

  • A 403 on GET /boards/{boardId}/cards renders explicit permission-loss guidance: "You no longer have access to this board, so parent choices are unavailable. Reopening this card will not restore them. Ask a board admin to restore your access."
  • Every other failure (5xx, network, timeout, a response-less error) keeps the existing copy: "Parent choices could not be loaded. Close and reopen this card to retry."
  • Both states still disable the selector and clear the candidate list, exactly as before.

Why

From the source-confirmed MEDIUM in PR #2965 (comment #2965 (comment)). If board access is revoked after the card editor opens, the candidates GET answers 403. The old catch {} discarded the status and always showed the close-and-reopen retry copy, so the user was told to retry something that can never succeed — reopening the card re-issues the same refused read, and the server rejects the parent write too. The defect is misleading permission recovery, not an authorization bypass: writes were and remain server-authorized.

Scope is deliberately narrow. 403 alone is treated as permission loss — deliberately narrower than the shared isAccessDeniedError (403 OR 404), following the same reasoning as useReviewProposals.isForbiddenError: a 404 from a list endpoint is a routing/board-gone fact, not a permission signal, so it keeps the generic copy.

Stale-request isolation is untouched: the generation counter still gates every write (the success assignment, the new failure classification and the loading reset), so an obsolete request's completion — success or failure — never overwrites a newer request's state. Two spec cases pin both orderings.

Tests

New spec frontend/taskdeck-web/src/tests/components/board/CardParentField.spec.ts (the component had none):

  • happy path — candidates listed, self excluded, selector enabled, no alert;
  • revoked access (403) — permission copy shown, retry copy absent, selector disabled, candidates cleared, no automatic re-read;
  • ordinary read failure (500) — retry copy shown, permission copy absent;
  • response-less network error — classified transient, not permission loss;
  • obsolete request rejecting 403 after a newer request succeeded — no alert, newer candidates retained;
  • obsolete request succeeding after a newer request got a 403 — permission alert retained, stale candidates not rendered.

Checks run (worktree C:/wt/td-2974, Node 24.x)

  • npx vitest --run --maxWorkers=2 src/tests/components/board/CardParentField.spec.ts — 6 passed.
  • npx vitest --run --maxWorkers=2 src/tests/components/CardModal.spec.ts — 40 passed (the parent field's only consumer).
  • npm run typecheck — clean.
  • npm run build — clean.

NOT verified

  • No browser or Playwright reproduction of the revoked-access path; the 403 is exercised through a mocked cardsApi.getCards, as the originating triage also claimed no browser repro.
  • The full frontend vitest suite was not run here (it OOMs on this box without --maxWorkers); CI covers it.
  • No backend change and no i18n keys: CardParentField carries hardcoded English strings today like its siblings in components/board/, so the new copy follows the file's existing idiom rather than introducing a first translation key for this component.
  • No canonical doc update: this is a user-facing copy/classification fix inside one component, not a change to shipped architecture, sequencing, or a hard-to-reverse decision.

Closes #2974

A 403 on the parent-candidates GET meant board access was revoked after
the card editor opened, but the catch discarded the status and rendered
the generic "close and reopen this card to retry" copy. Reopening cannot
restore access, so the guidance sent the user round a loop that could
never succeed.

Classify the failure instead: 403 renders explicit permission-loss
guidance, every other failure (5xx, network, timeout) keeps the existing
retry copy. Both states still disable the selector, and the existing
generation guard is unchanged, so an obsolete request's completion -
success or failure - never overwrites a newer request's state.

Spec covers revoked access, an ordinary read failure, a response-less
network failure, both obsolete-completion orderings and the happy path.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T16:59:14.475822Z 75458ba PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Fresh-context independent review at exact head 75458ba8a55aeb00607efd20d2262aa74f45dd6c against base b9dcc9523d45845e25d3c6345aee0f19e1253f2b: CLEAN, no CRITICAL/HIGH findings.

Verified by the reviewer from source (read-only, no execution):

  • Error shape: cardsApi.getCards returns the raw axios rejection, so response.status is the real runtime field (same idiom as useReviewProposals.isForbiddenError, useErrorMapper); httpRetry does not retry 403 into a different shape; backend CardsController documents 403 as no read access, so the copy is grounded.
  • Generation guard: every post-await write sits inside the request === generation check; loadError is reset at the start of each load, so a stale permission state cannot survive a newer successful read.
  • Template: both branches render role="alert", and the selector stays disabled in both failure states (parity with the previous single branch).
  • No client-derived write permission; allowed still reads server-supplied canWrite/isArchived.
  • The two obsolete-completion specs use deferred promises and settle the stale request after the newer one has completed, in both directions.

Non-blocking notes, recorded here rather than as commits (law 2c):

  • LOW: the "404 keeps generic copy" design note has no pinning test; a 404/500 it.each would make it regression-proof.
  • LOW: a deleted board also yields 403, so "ask a board admin to restore your access" is slightly off in that narrow, practically unreachable case.
  • LOW: spec relies on the activePinia fallback and omits unmount() where sibling specs pass pinia explicitly; convention drift only.

Merge gate: waiting for the pending hosted checks at this head and the 3-minute aging floor; will merge with a merge commit once green.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Re-running the single failed job once (Backend Unit windows-latest: CliTestHarnessTests.RunAsync_WhenCommandCompletes_RecordsFullStartupLifecycle timed out waiting for the CLI process; frontend-only diff, Application tests 4343/4343 green on the same job). This is the one permitted re-measure; a second failure parks the PR. Prior diagnosis of this Windows CLI subprocess timeout: #1530.

@Chris0Jeky
Chris0Jeky merged commit 44f5122 into main Sep 11, 2026
53 of 54 checks passed
@Chris0Jeky
Chris0Jeky deleted the issue-2974/parent-field-access-loss branch September 11, 2026 17:55
@github-project-automation github-project-automation Bot moved this from Pending to Done in Taskdeck Execution Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Distinguish lost board access when loading parent choices

1 participant