From 11aba0394e6150dc3497e34531e2a19e17b4778c Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 04:37:19 -0700 Subject: [PATCH] =?UTF-8?q?docs(solutions):=20converting=20a=20column=20li?= =?UTF-8?q?teral=20to=20a=20role=20makes=20it=20async=20=E2=80=94=20the=20?= =?UTF-8?q?four=20forms=20that=20ship=20green=20(#2710)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four review rounds across `TaskCard.tsx` and `TaskDetailModal.tsx` each found a **real defect**. None was in the conversion itself — every one came from the same property change. The fleet has ~600 guards left to convert against the same helpers, so this is written down rather than left in four commit messages. ## The property that changes ```ts task.column === "in-progress" // stable for the lifetime of the render tree isWipColumn // derived from fetched trait flags — CHANGES after first paint ``` Column trait flags arrive from a board-workflows fetch. Until it lands they are `undefined` and every role helper falls back to the legacy id. So a converted role is `false`, then `true`, within one mounted component. ## The four forms | | form | symptom | |---|---|---| | 1 | **stale memo** — deps still keyed only on `task.column` | timers, labels, completion dates frozen at first-paint values (4 instances in TaskCard) | | 2 | **frozen `useState` initializer** | the section does not start collapsed — it *appears later, already collapsed*, on a card nobody touched | | 3 | **eager action on a guess** — effect mutates state before flags resolve | a tab opens and instantly bounces; the correction never lands because the action destroyed the state it would have corrected | | 4 | **stale identity** — flags resolved, but for the *previous* entity | roles resolve from another task's workflow: confidently wrong rather than merely stale | **Form 4 defeats the obvious fix for form 3.** A `metadata === null` guard asks whether data *loaded*, not whether it describes the entity currently open — and it only appears in components that stay **mounted across entity changes**, which is why TaskCard never showed it and the modal did. ## Why a doc rather than four commit messages All four ship **green**: types pass, existing tests pass, and the **default board behaves identically** — because on the default lineage the legacy fallback and the resolved role agree. They diverge only on a **renamed board**, which is precisely the case the conversion exists to support. So the failure mode is: census count reaches zero, everything is green, and the feature is broken exactly where the programme was meant to fix it. A reviewer catching these one at a time is the expensive path, and it has now cost four rounds on two files. Also relevant: **this repo has no `react-hooks/exhaustive-deps` rule**, so form 1 has no automated backstop at all. ## Contents A checklist a converter can run against a component file, and the concrete fix shape for each form — including tagging fetched metadata with the id it describes, and applying that guard to the **role bindings** rather than only the effects (reordering effects fixes the call sites you noticed and leaves the bindings stale for everything else). Follows the convention already established by the engine-side scoping note in `architecture-patterns/fleet-self-healing-cluster-scoping.md`, which records the equivalent hazard for sync workflow reads. ## Verification `pnpm test:gate` green (10 / 158 / 487 / 71). `pnpm check:lifecycle-columns` exits 0. `pnpm lint` clean. Docs-only; no changeset. --------- Co-authored-by: Claude Opus 5 (1M context) --- ...ync-resolved-column-roles-in-components.md | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 docs/solutions/ui-bugs/async-resolved-column-roles-in-components.md diff --git a/docs/solutions/ui-bugs/async-resolved-column-roles-in-components.md b/docs/solutions/ui-bugs/async-resolved-column-roles-in-components.md new file mode 100644 index 0000000000..938fe5af39 --- /dev/null +++ b/docs/solutions/ui-bugs/async-resolved-column-roles-in-components.md @@ -0,0 +1,136 @@ +--- +category: ui-bugs +module: dashboard/components +tags: [lifecycle-columns, column-roles, fleet, react-hooks, async-state] +problem_type: conversion-hazard +applies_when: converting lifecycle-column literals in React components that read column trait flags +--- + +# Converting a column literal to a role turns a stable value into an async one + +Recorded 2026-07-30 while converting `TaskCard.tsx` and `TaskDetailModal.tsx`. Five separate review +rounds each found a real defect in those two files. None was in the conversion itself — every one +came from the same property change, and they are cheap to prevent and expensive to find one at a +time. + +**Status of those conversions, stated because an earlier draft of this line got it wrong.** It read +"after converting `TaskCard.tsx` (42 → 0) and `TaskDetailModal.tsx` (30 → 5)" as accomplished fact. +Neither reduction has landed: on `main` both files still measure 42 and 30, and the census baseline +in this very PR records the same. The work is in flight in #2688 and #2698. The hazard below is real +regardless — it was found BY those conversions, and it is what the four review rounds were about — +but a solutions doc that reports unlanded numbers as history is worse than no doc, because the next +reader has no way to tell which of its claims were measured. + +## The property that changes + +```ts +task.column === "in-progress" // stable for the lifetime of the render tree +isWipColumn // derived from fetched trait flags — CHANGES after first paint +``` + +Column trait flags arrive from a board-workflows fetch. Until it lands they are `undefined`, and +every role helper falls back to the legacy id — the right answer on a default board, the **wrong** +answer on a renamed one. So a converted role is `false`, then `true`, within the life of one mounted +component. + +Every defect below is that fact meeting a React idiom that assumed stability. + +## The four forms + +### 1. Stale memo + +A `useMemo` / `useCallback` whose body reads a role but whose dependency array still lists only +`task.column`. The role flips; the memo does not recompute. Symptom on a custom board: timers, +labels and completion dates frozen at their first-paint values. + +Found four instances in `TaskCard.tsx`. **This repo has no `react-hooks/exhaustive-deps` rule**, so +nothing flags it — check by hand, or with an AST pass over every hook in the file. + +### 2. Frozen `useState` initializer + +```ts +const [showSteps, setShowSteps] = useState(isWipColumn || …); // runs ONCE +``` + +Captures the pre-load fallback and never reconciles. Worse than it sounds when the surrounding +section is *itself* gated on the same role: the section does not start collapsed, it **appears +later, already collapsed**, on a card the operator never touched. + +Reconcile one-way, and only while untouched — a `touchedRef` set by the user's own toggle. Do not +auto-revert on the reverse transition; closing something a user is reading is worse than the bug. + +### 3. Eager action on an unresolved role + +An effect that *mutates* state — a tab redirect, a reset, a navigation — firing while flags are +still `undefined`. It acts on the fallback, and the correction never arrives because the action +already destroyed the state it would have corrected. + +Guard on resolution and wait. **When one branch is recoverable and the other is not, acting on a +guess should favour the recoverable one.** Showing a tab a moment too long self-corrects; discarding +a deliberate selection does not. + +### 4. Stale identity — the one that survives a "resolved" guard + +The subtlest, and it defeats the fix for form 3: + +```ts +if (workflowMoveMetadata === null) return; // asks "has it loaded?" — NOT "is it for THIS task?" +``` + +In a component that stays **mounted across entity changes**, the fetch effect resets the state on a +task change, but it may be *declared below* the consumers. On the render where the task switches, +they run first and see the previous task's flags. Non-null, so the guard passes, and roles resolve +from **another task's workflow** — confidently wrong rather than merely stale. + +Tag the payload with the id it describes and compare identity. Do not rely on effect ordering: it is +load-bearing, invisible, and one reorder away from breaking. + +```ts +const flagsAreForThisTask = metadata?.taskId === task.id; +const columnFlags = flagsAreForThisTask ? metadata?.currentColumnFlags : undefined; +``` + +Apply it to the **role bindings**, not only the effects. Reordering effects fixes the call sites you +noticed and leaves the bindings resolving from stale data for everything else. + +### 5. Settled-empty mistaken for unresolved — the fix for form 4 introduces this one + +Guarding on identity means writing `null` when a lookup returns nothing, or fails — which is +indistinguishable from *"has not resolved yet"*. The guard then never opens, the reconciliation never +runs, and the invalid state persists **forever**. Trading "acts on stale data" for "never acts" is +not a fix. + +Resolution has **three** states, not two: + +| state | value | +|---|---| +| unresolved | `null` | +| resolved with data | `{ entityId, …payload }` | +| **resolved empty** | `{ entityId }` | + +The third still identifies the entity, so consumers know the answer landed and should fall back to +the legacy id — a real answer for a workflow that declares nothing, not a placeholder. Settle the +**failure** path the same way, or a lost fetch leaves the UI waiting on it indefinitely. + +**Every fix in this list can introduce the opposite failure. Check that both directions terminate.** + +## Checklist for a component conversion + +1. Every hook whose body reads a role lists that role in its dependency array. +2. No `useState` initializer reads a role without a reconciliation path. +3. No effect *mutates* state from a role before the flags resolve. +4. If the component persists across entity changes, roles are guarded on **identity**, not on + non-null — and "resolved empty" is distinguishable from "unresolved", including on the failure + path. +5. Where a role gates both *whether something shows* and *whether it stays*, convert both together + — a half-converted pair makes the UI contradict itself, which is worse than either state alone. + +## Why this is worth a doc rather than four commit messages + +The conversions are mechanical and look finished when the file's census count reaches zero. All four +defects ship green: types pass, existing tests pass, and the default board behaves identically — +because on the default lineage the legacy fallback and the resolved role agree. **They only diverge +on a renamed board, which is precisely the case the conversion exists to support.** + +Related: `docs/solutions/architecture-patterns/fleet-self-healing-cluster-scoping.md` records the +engine-side hazard (sync workflow reads) for the same programme.