diff --git a/project-docs/specs/project_management_app.md b/project-docs/specs/project_management_app.md index 182177179..2a37f734d 100644 --- a/project-docs/specs/project_management_app.md +++ b/project-docs/specs/project_management_app.md @@ -3,7 +3,7 @@ > **Product:** CommandCenter · **Feature:** Projects (the People Center's primary work-management > module, sliced into every other Center) · **Created:** 2026-08-05 · **Updated: 2026-08-10** > (status truth pass + tenancy alignment — R4; **WS-27ag shell/mobile slice built the same -> day**) · +> day**; **S4 convergence slice built the same day — §11.21**) · > **Status:** ✅ **WS-27 a–t MERGED AND DEPLOYED** (a b d e f i j k l m n via #390/#393/#394/#398; > o–t via **#399**; **u–z via #408**, 2026-08-10) — migrations **146, 147, 150, 152, 155, 156, > 160, 161, 164, 165, 166 are applied on prod** (164/165/166 log-verified on the 2026-08-10 @@ -19,6 +19,31 @@ > six-purpose header splits into a title row and an action row. Frontend only — no migration, > no API change. ⚠️ **The phone-viewport and four-theme visual pass is still owed**: no > browser was runnable in the build environment (§11.20's closing note). · +> 🟢 **S1 BUILT 2026-08-10, on branch, NOT merged and NOT deployed** (§9.2, under WS-27ad) — +> the /tasks board card and column adopt /projects' chrome under the owner's ruling that +> *"the Tasks app is only a slice of the Projects app"*: one column shape and surface, one +> gutter, the shell's `completed` and `atCursor` props finally passed by their /tasks caller, +> the shared `AvatarStack` instead of a private copy, the selection checkbox moved OUT of the +> card as a sibling target, and the title clamped to two lines in the shared file — which +> **amends ad's recorded "modal select-mode is KEPT" decision on the card side**. Frontend +> only — no migration, no API change. ⚠️ Visual pass still owed for the same reason as af/ag. · +> 🟢 **S3 (selection/bulk parity) BUILT 2026-08-10 for the /tasks LIST surfaces, on branch +> `ws-s3-selection-bulk-parity`, NOT merged and NOT deployed** (§9 ticket "S3") — WS-27ad's +> kept modal select-mode is **reversed by owner ruling** ("Projects is canonical, Tasks +> conforms"): `selectMode` is now a derived mirror of "something is selected" and gates only +> the bulk bar, the checkbox is permanent and sits outside the row content, shift-sweep is +> ungated, the bar moved to the top onto Projects' chrome and primitives, and select-all +> exists. ⚠️ The board card (`TaskCard`/`TaskBoard`, sibling slice S1) and `WaitingForView` +> are still modal, and no browser was run here either. · +> 🟢 **S4 BUILT 2026-08-10, on branch, NOT merged and NOT deployed** (§11.21) — the three +> findings where **Projects**, not Tasks, carried the defect: `MyWork`'s active pill moves +> off `bg-accent` onto the house `bg-primary/10 text-primary` (now fenced by a **sixth +> conformance rule**, per-file and ratcheted), `MyWork`'s bespoke fourth task row is rebuilt +> on `TaskCardShell`/`TaskMeta`/`StatusChip`, and the board's one conflated empty state +> becomes two — filtered-to-nothing (with **Clear filters**) vs genuinely empty — off the +> existing `isFiltered` predicate, through a promoted `src/components/EmptyState.tsx`. +> Frontend only — no migration, no API change. ⚠️ **The four-theme sweep is owed** for the +> same reason: no browser runs in this environment. · > **Owner:** vjvarada · **Board row: WS-27** > > **Tenancy (audited 2026-08-10 — this spec previously cited no tenancy decision at all).** @@ -566,6 +591,75 @@ independent of the ClickUp work, because it is a move between two tables we own: ⚠️ **Not started, and it is the largest single piece of WS-27 remaining.** Until it lands there are two personal task stores, which is the state this decision exists to end. +#### 7.5.1 The destination table — every column, named *(added 2026-08-10)* + +> **Why this exists.** Step 2 above says "a copy rather than a translation." That sentence is +> true of the **seven** overlay columns `pm_task_personal` already mirrors, and false of the +> rest. `gtd_items` carries **thirty-one** columns plus a `gtd_waiting` side table; fifteen of +> them have no `pm_tasks` home at all. Executed as written, this migration would be a +> **feature deletion wearing the word "copy"** — the founder priority matrix, all of +> timeboxing, and the entire Waiting-For view would simply stop existing. Nobody would notice +> until the data was already gone, because the migration would report success. +> +> So the destination of every column is named here **before** the migration is written. A +> column that reaches the day of the move without a row in this table is a bug in this spec, +> not a judgement call for whoever happens to be executing. + +**The governing question for each column is *whose fact is it?*** — because that decides the +table, and getting it wrong is what a mirror-shaped design does to you later: + +* a **task** fact is true for everyone looking at the row → `pm_tasks` +* a **member** fact can differ between two people on the same task → `pm_task_personal` + +The test that settles the hard cases: *if Ana and Ben are both on this task, can their answers +legitimately differ?* If yes, it is per-member. This is the same argument `147_projects_personal.sql` +already makes for `disposition` (one person is doing it, the other is waiting on it), applied +consistently rather than only where it was first noticed. + +| `gtd_items` column | Destination | Why | +|---|---|---| +| `disposition`, `next_action`, `context`, `energy`, `time_estimate_mins`, `is_two_minute`, `defer_until`, `clarified_at` | **`pm_task_personal`** ✅ exists | The seven-plus-one already mirrored 1:1 by migration 147. This is the part that genuinely is a copy. | +| `title`, `description`, `due_at`, `completed_at`, `created_at`, `updated_at` | **`pm_tasks`** ✅ exists | Same name, same meaning. | +| `parent_item_id` | `pm_tasks.parent_task_id` ✅ | Same shape. | +| `archived_at` | `pm_tasks.archived_at` ✅ | Same shape. | +| `assignees` (JSONB, mig 91) | `pm_task_assignees` ✅ | Rows, not JSONB. `assignee` (the older singular, mig 48) folds into the same set. | +| `attachments` (JSONB, mig 52) | `pm_task_attachments` ✅ | Rows, not JSONB. | +| `sort_key` (mig 58) | `pm_view_task_positions` ✅ | D-PM-5: order is per view, not a column on the task. | +| `project_id` → `gtd_projects` | `pm_tasks.project_id` | `LOCAL` rows land in the owner's personal project (§3.11). | +| `workflow_stage` (mig 57) | `pm_tasks.status_id` | Free text → a real status row. The status→stage map already exists; the **colour** already agrees via `src/lib/statusAccent.ts`. | +| **`important`, `leveraged`, `kept_mine`, `urgent_window_hours`** (mig 68) | **`pm_task_personal`** — 🔴 **NEW columns** | The founder priority matrix. Ana may rate a shared task important and Ben may not; it is a judgement, not a property of the work. Per-member by the test above. | +| **`scheduled_start`, `scheduled_end`** (76), **`flexible`** (79), **`actual_start`, `actual_end`** (80) | **`pm_task_personal`** — 🔴 **NEW columns** | Timeboxing. *When I plan to do it* and *when I actually did* are mine; two people on one task book their own calendars. Note `pm_tasks.start_date` is a **different** fact (when the work starts, shared) and must not be conflated. | +| **`deep_work`** (mig 96) | **`pm_task_personal`** — 🔴 **NEW column** | A sibling of `energy`, which is already per-member. My concentration classification, not the task's. | +| `is_hard_date` | **`pm_tasks`** — 🔴 **NEW column** | The one calendar-shaped flag that is *not* personal: a deadline is either immovable or it is not, and that is true for everyone. | +| **`origin`** (JSONB, mig 65) | **`pm_tasks`** — 🔴 **NEW**, or the P-9 `(external_source, external_id)` pair | Email-capture provenance: where this task came from. A task fact. Prefer the generic provenance pair (`plane_pm_research_2026-08.md` P-9) over a second JSONB blob — one provenance vocabulary, not two. | +| `source`, `account_id`, `provider_task_id`, `provider_url`, `provider_status`, `sync_state` | **Retire with WS-27g**, not here | These are the ClickUp arm. They become meaningless when the provider retires, so they are a *legitimate* drop — **but only if WS-27g lands first or concurrently**. ⚠️ If WS-27h runs first, the `SourceBadge` loses its data while the integration is still live. Sequencing constraint, recorded. | +| `is_mine` | **Derived, dropped** | `personal.derive_disposition` already computes it from `pm_task_assignees`. A stored copy of a derivable fact is the mirror problem in miniature. | +| `deleted_at` (mig 67) | **Dropped → `archived_at`** | `pm_*` has no soft-delete and gains none: pervasive soft-delete was **refused** (P-31). Rows with `deleted_at` migrate as archived, and the reason travels with them. | +| `user_id` | **Dissolved** | Becomes `pm_projects.personal_owner` (which project) plus `pm_task_personal.member_email` (whose overlay). One column becomes two because it was doing two jobs. | +| `synced_at` | **Dropped** | Meaningless without the provider arm. | +| `horizon_id` → `gtd_horizons` | 🔴 **BLOCKED — WS-21 owns Horizons and is DO-NOT-DISPATCH** (`work_plan.md` §4) | This migration **cannot** decide the fate of a feature another workstream owns. Either WS-21 rules first, or `gtd_horizons` and this FK outlive the retirement as an explicitly-parked island. Do not quietly drop it. | +| **`gtd_waiting`** (table: `item_id`, `waiting_on`, `delegated_at`, `expected_by`, `last_nudged_at`, `resolved`, `created_at`) | 🔴 **NEW table `pm_task_waiting`**, keyed `(task_id, member_email)` | The whole Waiting-For view (WS-18, built 2026-08-02) rests on this. Per-member for the same reason as `disposition`: Ana waits on Ben while Ben waits on a vendor, about one task. A single delegation row per task cannot express that. `item_id` becomes `task_id`; the member half of the key is **new** — the legacy table had no such column because the legacy store was single-user by construction (`gtd_items.user_id`), which is precisely the assumption one store removes. | + +**What this changes about the ticket.** WS-27h is no longer "move rows between two tables we +own." It is: + +1. a **schema** step — 🔴 twelve new columns on `pm_task_personal`, one on `pm_tasks`, one new + `pm_task_waiting` table, plus the provenance decision. Expand/contract per **R6**: nullable + with defaults, tighten later, never rename in place. +2. the **union read** (step 1 above), which is independently valuable — it is what lets the + Tasks app show a Projects view of the same person's work without the two disagreeing about + what "my tasks" means. **This is the step to build first**; it unblocks UI parity work + without touching a single row. +3. the **data move**, which is 🔴 owner-gated. +4. the **predicate retirement** (step 3 above). + +**Sequencing constraints, both load-bearing:** WS-27g before or with the provider-column drop; +WS-21's Horizons ruling before `horizon_id` can be resolved either way. + +**Done-when, added:** a test asserts every `gtd_items` column and every `gtd_waiting` column +appears in this table with a destination, so a column added to the legacy store after this was +written cannot reach the migration unnoticed — the failure mode this section exists to prevent. + ### 7.4 Retirement inventory (WS-27g, second half) System A ClickUp arm: `ingestion/sources/clickup/` (client, normaliser, webhook), `scheduler.py`'s ClickUp job, `scripts/clickup_sync.py`, `/webhooks/clickup` from @@ -1370,11 +1464,20 @@ shims staying shims, no second name→class palette). card-shaped hole in the column). `shown_fields` gating unchanged. - **Selection.** `lib/selection.ts` holds `clickSelect` / `range` / `toggle` / `prune` / `allSelected`; `stepCursor`'s duplicated sweep now reads it. Projects' page and the Tasks - store both drive it, so shift-click and Shift+Arrow behave identically. **Tasks' modal + store both drive it, so shift-click and Shift+Arrow behave identically. ~~**Tasks' modal select-mode is KEPT**, with the reason in `tasks/components/ItemList.tsx`: `selectMode` changes what a *click means*, and a permanent checkbox on a `TaskCard` would take the - drag-grip gutter or make one gesture mean two things. The mode is the entry; the grammar - inside it converged. + drag-grip gutter or make one gesture mean two things.~~ **Struck on the card side by S1 + below** — the checkbox went *outside* the card instead of into it, so it takes no gutter + from the grip and no gesture means two things; the premise that those were the only two + options was the error. + select-mode is KEPT**, with the reason in `tasks/components/ItemList.tsx`.~~ + **REVERSED by owner ruling 2026-08-10 — "Projects is canonical, Tasks conforms" — and + built for the list surfaces the same day (S3, below).** The kept reason (a permanent + checkbox would take the drag-grip gutter or make one gesture mean two things) was never + structural: Projects had already solved it by putting the box OUTSIDE the card as a + sibling in the row, while Tasks put it INSIDE, absolutely positioned over the grip — so + the collision was one Tasks had built for itself. - **Board chrome.** Drop-gap reorder beats append-on-drop and is now `components/DropGap.tsx` + `lib/boardDrop.ts` (`gapKey`, `dropIndexFor` — the downward intra-group off-by-one, previously buried in `taskStore.reorderItem`), consumed by both @@ -1397,6 +1500,82 @@ the seam test with its reason; `tasks/lib/contextColors.ts` uses raw Tailwind pa classes (`sky-500`…) rather than semantic tokens — legal under the conformance suite, off the token system, and a Tasks-only axis with no Projects counterpart. +**S1 — board card and column convergence (round 3).** ✅ **BUILT 2026-08-10**, on branch, +NOT merged. *(Owner ruling, not re-litigated: "the Tasks app is only a slice of the Projects +app" — **Projects is canonical**; where the two disagree and neither is clearly better, +Tasks conforms. A GTD-specific need Projects has no equivalent for is a legitimate reason to +diverge, provided the reason is written next to the code.)* Frontend only, three files: +`app/tasks/components/{TaskCard,TaskBoard}.tsx` and the shared +`components/TaskCardShell.tsx`. Nothing in `app/projects/` was edited — the Projects-side +effect (item 6) travels through the shell. + +1. **Column chrome.** `rounded-xl` + `bg-secondary/30` → `rounded-lg border border-border + bg-card`, refusal overlay `rounded-lg`, cards spaced by the column's `space-y-1` instead + of a per-card `mb-2`. Cards and drop gaps are now siblings, because `space-y-*` only + reaches direct children. The drag-over highlight and the accent cap stay. +2. **Completed treatment.** `TaskCardShell` had accepted `completed` since ad and **nothing + under `app/tasks/` had ever passed it**, so a done task was dimmed and struck through on + one board and drawn as live work on the other, from one component, with every test green. + `completed={Boolean(item.completedAt)}` now reaches both the shell and `TaskCardTitle`. +3. **Cursor ring.** The board's wrapper `
` is gone; the + card takes `atCursor` and the shell draws the ring on the card's own radius. `useFlash`'s + `attach` moved onto the card element too, so the thing that flashes is the card. +4. **Avatars.** `TaskCard`'s private `Avatar`/`AvatarStack` are deleted for + `components/TaskMeta`'s shared pair, `max={1}` on this app's narrow rows. +5. **The checkbox.** Out of the card and into a left gutter as a sibling in a `flex + items-start gap-1.5` row with `stopPropagation` — /projects' pattern — and **always + present**, not mode-gated. `selectMode` no longer changes what a click on a card means; + `TaskCard` has no `selectMode` prop at all. The **drag grip is dropped, not relocated**: + the whole card is `draggable`, so it was never a handle, only a hint pointing at a spot + that is not special; /projects draws none; and the shell's hover lift is the affordance + both boards already use. The `pr-5` the grip reserved is gone with it. +6. **Title truncation** — *the one judgement call, taken in the shared file.* /projects + clamped to one line, /tasks wrapped without limit. `TaskCardTitle` now clamps to + `line-clamp-2` **for both apps** and strips a caller-supplied `truncate`/`line-clamp-*` + rather than merging it, because `white-space: nowrap` and `display: -webkit-box` on one + element resolve by CSS source order — invisible in review. Cost: a /tasks next action + longer than two lines ends in an ellipsis where it used to wrap in full (the full text is + in the focus modal); a /projects card is up to one line taller. `truncate` is still + passed at `projects/components/TaskBoard.tsx` and is now a no-op — deleting it is a + one-line follow-up for whoever next owns that file. + +**Kept, deliberately** (structural, backed by fields `pm_tasks` does not have): the priority +badge and suggestion badge, the project chip, the `SourceBadge`, the inline `ScheduleButton`, +`item.nextAction` under the title, and `StatusPill`'s interactivity — though its +`!selectMode` gate is gone, since that gate only existed because the card *was* the checkbox. + +**Fences** (R7). All three are source scans in `src/lib/sharedTaskUi.test.ts`, and the file +now says why they cannot be render tests: `vitest.config.ts` is `environment: "node"` with +`include: ["src/**/*.test.ts"]`, so there is no DOM and `.tsx` test files are not collected — +adding jsdom to fence one prop is a larger change than the thing fenced. (1) *every caller of +`TaskCardShell` passes `completed` and `atCursor`* — the tag scanner is brace-aware, because +a lazy regex stops at the `>` of the first arrow function and would pass for the wrong +reason; (2) *no board re-implements the card cursor ring* (`ring-ring` absent from both +`TaskBoard.tsx` files, present in the shell); (3) *both boards' columns use the same radius*. +`AvatarStack` and `TaskCardTitle` joined the `SEAM` table, and `SEAM` rows gained an +`except` map with a per-file argument plus a staleness check — `components/room/Identity.tsx` +exports a different `AvatarStack` (room participants, photographs, presence rings, and the +per-person identity hues conformance deliberately excepts from theming). All scans strip +comments first: the first run failed on the code comment explaining which class had just +been *removed*, and a gate a comment can trip teaches people not to comment. + +⚠️ **The audit's premise about `rounded-xl` is wrong in this tree, and the fence was written +to the true rule instead.** `AGENTS.md` rule 6 and `DESIGN_SYSTEM.md` §4 say `rounded-xl` is +"a fixed 12px that ignores Graphite's 0.125rem" — but `src/app/globals.css` (the `@theme` +block, ~ll. 217-227) derives the **whole** `--radius-*` scale from `--radius`, and +`--radius-xl` is literally `var(--radius)`, i.e. the same value `rounded-lg` resolves to. +`rounded-xl` is therefore fully themed here, and a tree-wide ratchet on it would have +baselined **274 correctly-themed occurrences across ~70 files**. What was actually wrong is +narrower and is what shipped: two boards drew one object at two radii on two surfaces. The +doc claim should be corrected by whoever owns `AGENTS.md`; it is left alone here rather than +edited from a ticket that owns three component files. + +⚠️ **Not verified: how any of this looks.** No browser is runnable here (Playwright cannot +install), so the Fluent → Material → Graphite sweep and the phone-viewport pass are owed at +review, exactly as for af and ag. What was checked is `npx tsc --noEmit`, the full +`npx vitest run`, `npx vitest run src/lib/theme/`, `eslint` on the changed files, and each +new fence mutation-measured red before being reverted byte-identical. + **WS-27ae — export, delta-sync, small columns.** 🟢 AGENT-SAFE, **not this wave** *(P-26, P-27, P-28 rest)*. Filtered-list CSV export on the export-job pattern; a delta-sync list variant plus satellite `updated_at` bumps for agents/mobile; `is_epic`, per-user view state @@ -1436,6 +1615,49 @@ accumulated tree-wide. ⚠️ Measured, and the ticket was wrong: `/tasks` held > than left implicit. None of that substitutes for looking at the running app on a phone in > four themes. That gate is still open. +**S3 — selection and bulk-action grammar parity (`/tasks` conforms to `/projects`).** +🟢 AGENT-SAFE, frontend only, no migration. ✅ **BUILT 2026-08-10 for the LIST surfaces** +*(owner observation: "Tasks shows the selection checkbox only when the appropriate setting +is there in the select options on top; in the Projects app the checkbox for selecting a +task is present." Owner ruling: Projects is canonical.)* Branch `ws-s3-selection-bulk-parity` +— **not merged, not deployed.** + +- **`selectMode` is no longer a mode.** It is a derived mirror of `selectedIds.size > 0` + maintained in one helper (`taskStore.applySelection`), and it decides only whether the + bulk bar is up. The "Select" button and `setSelectMode` are gone; `ItemList`, + `TaskListGrouped` and `FlatList` do not read `selectMode` at all. +- **The checkbox is unconditional and OUTSIDE the row content** — its own gutter beside the + drag grip, so a click on the row still opens the task and the two gestures stop competing + for one gutter. Drag-reorder is therefore no longer switched off while something is + selected. +- **Shift-sweep is ungated** on both list surfaces (it required select mode; Projects never + did), through the same `@/lib/selection` + `@/lib/cursor` both apps already shared. +- **The bulk bar converged on Projects'**: top-mounted on the same `border-b border-border + bg-muted px-3 py-2` chrome, built from `Button`/`Badge` instead of the hand-rolled + outline ` - {TRIAGE.filter((t) => t.value !== task.disposition).map((t) => ( - + ))} -
+ ); } diff --git a/workbench/control_plane/src/app/projects/components/TaskBoard.tsx b/workbench/control_plane/src/app/projects/components/TaskBoard.tsx index 4f6684720..fad9d28ad 100644 --- a/workbench/control_plane/src/app/projects/components/TaskBoard.tsx +++ b/workbench/control_plane/src/app/projects/components/TaskBoard.tsx @@ -25,6 +25,7 @@ */ import { AvatarStack, TaskMeta } from "@/components/TaskMeta"; import { DropGap } from "@/components/DropGap"; +import { EmptyState } from "@/components/EmptyState"; import Icon from "@/components/Icon"; import { StatusChip } from "@/components/StatusChip"; import { TaskCardShell, TaskCardTitle } from "@/components/TaskCardShell"; @@ -43,10 +44,13 @@ import { } from "../lib/board"; import { taskRef, visibleChips } from "../lib/card"; import { clampCursor, stepCursor } from "../lib/cursor"; +import { emptyStateCopy } from "../lib/emptyState"; import { type BoardLanes, + type Filters, type GroupBy, type TaskGroup, + isFiltered, personLabel, } from "../lib/grouping"; import { mergePlans, quickAddPrefill } from "../lib/quickAdd"; @@ -64,6 +68,15 @@ const NOBODY: ReadonlySet = new Set(); interface Props { groups: TaskGroup[]; groupBy: GroupBy; + /** + * S4 — the view's filters, for the empty state alone. + * + * Required rather than optional: an unwired call site would silently blame + * the project's statuses for a board somebody filtered to nothing, which is + * the defect this props pair exists to end. `tsc` is the fence. + */ + filters: Filters; + onClearFilters: () => void; /** WS-27y — the second axis and its lane state. */ lanes: BoardLanes; onToggleLane: (key: string) => void; @@ -92,6 +105,8 @@ interface Props { export function TaskBoard({ groups, groupBy, + filters, + onClearFilters, lanes, onToggleLane, onShowEmptyLanes, @@ -306,7 +321,7 @@ export function TaskBoard({ cellTasks: TaskRow[], laneKey: string | null ) => { - const scope = laneKey === null ? colKey : `${colKey}${laneKey}`; + const scope = laneKey === null ? colKey : `${colKey}\0${laneKey}`; const gap = (index: number) => (
  • - Nothing to show. Clear a filter, or this project has no statuses yet. -

    + ); } diff --git a/workbench/control_plane/src/app/projects/components/TaskList.tsx b/workbench/control_plane/src/app/projects/components/TaskList.tsx index 155b15bd0..3867889ce 100644 --- a/workbench/control_plane/src/app/projects/components/TaskList.tsx +++ b/workbench/control_plane/src/app/projects/components/TaskList.tsx @@ -17,6 +17,7 @@ * value (`lib/quickAdd.ts` owns that mapping), and an arrow-key cursor walks * the rows — Shift extends the WS-27n selection, Enter opens the panel. */ +import { EmptyState } from "@/components/EmptyState"; import Icon from "@/components/Icon"; import { StatusChip } from "@/components/StatusChip"; import { AvatarStack, TaskMeta } from "@/components/TaskMeta"; @@ -28,7 +29,14 @@ import { projectsApi } from "../lib/api"; import { sortForView } from "../lib/board"; import { taskRef, visibleChips } from "../lib/card"; import { clampCursor, stepCursor } from "../lib/cursor"; -import { type GroupBy, type TaskGroup, personLabel } from "../lib/grouping"; +import { emptyStateCopy } from "../lib/emptyState"; +import { + type Filters, + type GroupBy, + type TaskGroup, + isFiltered, + personLabel, +} from "../lib/grouping"; import { quickAddPrefill } from "../lib/quickAdd"; import { QuickAdd } from "./QuickAdd"; import { useFlash } from "./useFlash"; @@ -38,6 +46,15 @@ const NOBODY: ReadonlySet = new Set(); interface Props { groups: TaskGroup[]; groupBy: GroupBy; + /** + * S4 — the view's filters, for the empty state alone. + * + * Required rather than optional: an unwired call site would silently draw + * "no tasks here yet" over a filtered-to-nothing list, which is the exact + * defect this props pair exists to end. `tsc` is the fence. + */ + filters: Filters; + onClearFilters: () => void; statuses: StatusRow[]; /** WS-27y — where a quick-added task is created (the selected node). */ projectId: string; @@ -57,6 +74,8 @@ interface Props { export function TaskList({ groups, groupBy, + filters, + onClearFilters, statuses, projectId, shownFields, @@ -167,7 +186,23 @@ export function TaskList({ } if (total === 0) { - return

    No tasks here yet.

    ; + // S4 — two states, never one. "No tasks here yet" told somebody who had + // filtered the list that their project was empty; `isFiltered` is the same + // predicate the toolbar's Clear button reads, so the state and the control + // that caused it cannot disagree. + const copy = emptyStateCopy({ canvas: "list", filtered: isFiltered(filters) }); + return ( + + ); } const columnCount = onToggle ? 6 : 5; diff --git a/workbench/control_plane/src/app/projects/lib/accent.test.ts b/workbench/control_plane/src/app/projects/lib/accent.test.ts index 398f411cf..439614f8d 100644 --- a/workbench/control_plane/src/app/projects/lib/accent.test.ts +++ b/workbench/control_plane/src/app/projects/lib/accent.test.ts @@ -13,8 +13,9 @@ import { describe, expect, it } from "vitest"; import { statusAccent } from "@/lib/statusAccent"; -import { accentForGroup, accentForStatus } from "./accent"; +import { accentForDisposition, accentForGroup, accentForStatus } from "./accent"; import type { StatusRow } from "./api"; +import { DISPOSITION_LANES, LANE_LABELS, OTHER_LANE } from "./mywork"; const status = (over: Partial): StatusRow => ({ id: "s1", @@ -91,3 +92,43 @@ describe("accentForGroup", () => { expect(new Set(hues).size).toBe(4); }); }); + +describe("accentForDisposition", () => { + /** + * True when the shared vocabulary can read a hue out of the label itself. + * + * `keywordHue` is private to `statusAccent.ts`, so this asks the question the + * only way a caller can: if the answer does not move when the positional + * index moves, a keyword decided it. + */ + const keywordDecides = (label: string) => + JSON.stringify(statusAccent({ name: label, index: 0 })) === + JSON.stringify(statusAccent({ name: label, index: 1 })); + + it("agrees with the keyword route wherever that route has an opinion", () => { + // AGENTS.md rule 5: a category and a name must resolve to the same colour, + // or "Waiting on" is amber in one app and something else in the other. + const pinned = [...DISPOSITION_LANES, OTHER_LANE].filter((lane) => + keywordDecides(LANE_LABELS[lane] ?? lane), + ); + expect(pinned, "no lane label carries a keyword — check LANE_LABELS").not.toEqual([]); + for (const lane of pinned) { + expect(accentForDisposition(lane), `${lane} disagrees with its own name`).toEqual( + statusAccent({ name: LANE_LABELS[lane] ?? lane }), + ); + } + }); + + it("gives the four work lanes four different hues", () => { + // They are read side by side in one scrolling pane; two lanes wearing one + // colour is the /projects-board-in-grey failure at a smaller scale. + const dots = DISPOSITION_LANES.map((lane) => accentForDisposition(lane).dot); + expect(new Set(dots).size).toBe(DISPOSITION_LANES.length); + }); + + it("renders an unmapped disposition rather than failing", () => { + // `REFERENCE`, `PROJECT`, and whatever the server grows next. + expect(accentForDisposition("REFERENCE").dot).toBeTruthy(); + expect(accentForDisposition("")).toEqual(statusAccent({ name: "" })); + }); +}); diff --git a/workbench/control_plane/src/app/projects/lib/accent.ts b/workbench/control_plane/src/app/projects/lib/accent.ts index 61f27f5ac..ab9cf80a9 100644 --- a/workbench/control_plane/src/app/projects/lib/accent.ts +++ b/workbench/control_plane/src/app/projects/lib/accent.ts @@ -17,6 +17,7 @@ import { statusAccent, type StatusAccent } from "@/lib/statusAccent"; import type { StatusRow } from "./api"; import type { GroupBy } from "./grouping"; +import { LANE_LABELS } from "./mywork"; /** * The accent for one status. @@ -65,3 +66,39 @@ export function accentForGroup( } return statusAccent({ index, total }); } + +/** + * The accent for a **My Work** disposition (S4). + * + * The personal lens has no statuses — a row's state there is its GTD + * disposition — so this is the fact it hands the shared vocabulary. Written as + * hue NAMES fed through `statusAccent`, never as classes: a `Record` of + * tailwind strings here would be the second palette `sharedTaskUi.test.ts` + * exists to forbid. + * + * **The map agrees with the name-keyword route wherever that route has an + * opinion** — `Inbox` is gray and `Waiting on` is amber by keyword, so those + * two are pinned to what /tasks would draw for a stage of the same name + * (AGENTS.md rule 5; `accent.test.ts` asserts the agreement rather than + * trusting it). `NEXT` and `SOMEDAY` are free choices because no keyword + * matches them: blue is this UI's "active" tone, which is what a next action + * is, and violet is the one remaining distinct hue. + * + * An unmapped disposition — `PROJECT`, `REFERENCE`, anything the server grows + * later — falls through to the name route rather than throwing, so a new value + * renders quietly instead of taking the pane down. + */ +const DISPOSITION_HUES: Record = { + INBOX: "gray", + NEXT: "blue", + WAITING: "amber", + SOMEDAY: "violet", + OTHER: "gray", +}; + +export function accentForDisposition(disposition: string): StatusAccent { + return statusAccent({ + color: DISPOSITION_HUES[disposition] ?? null, + name: LANE_LABELS[disposition] ?? disposition, + }); +} diff --git a/workbench/control_plane/src/app/projects/lib/emptyState.test.ts b/workbench/control_plane/src/app/projects/lib/emptyState.test.ts new file mode 100644 index 000000000..31b4bb53b --- /dev/null +++ b/workbench/control_plane/src/app/projects/lib/emptyState.test.ts @@ -0,0 +1,108 @@ +/** + * The fence for S4 item 3: an empty canvas must say WHICH kind of empty it is. + * + * The defect these assertions describe shipped as one sentence carrying both + * causes — "Clear a filter, or this project has no statuses yet" — so the tests + * that matter are the ones about what each state does NOT say. + */ + +import { describe, expect, it } from "vitest"; + +import { ICON_REGISTRY } from "@/lib/theme/icon-registry"; + +import { EMPTY_FILTERS, isFiltered } from "./grouping"; +import { emptyStateCopy } from "./emptyState"; + +const noMatch = (canvas: "board" | "list") => + emptyStateCopy({ canvas, filtered: true, onStatusAxis: canvas === "board" }); + +describe("the filtered state", () => { + it("blames the filters and offers the way out", () => { + for (const canvas of ["board", "list"] as const) { + const copy = noMatch(canvas); + expect(copy.filtered).toBe(true); + expect(copy.message).toMatch(/filter/i); + } + }); + + it("never mentions the OTHER cause", () => { + // The whole defect: a reader who filtered has no reason to be told the + // project might have no statuses, and cannot tell which half applies. + for (const canvas of ["board", "list"] as const) { + const copy = noMatch(canvas); + expect(`${copy.message} ${copy.hint ?? ""}`).not.toMatch(/status/i); + } + }); + + it("reads the same on both canvases — one fact, one sentence", () => { + expect(noMatch("board")).toEqual(noMatch("list")); + }); +}); + +describe("the genuinely-empty state", () => { + it("never mentions filters", () => { + const cases = [ + emptyStateCopy({ canvas: "list", filtered: false }), + emptyStateCopy({ canvas: "board", filtered: false, onStatusAxis: false }), + emptyStateCopy({ canvas: "board", filtered: false, onStatusAxis: true }), + ]; + for (const copy of cases) { + expect(copy.filtered).toBe(false); + expect(`${copy.message} ${copy.hint ?? ""}`).not.toMatch(/filter/i); + } + }); + + it("says what an empty status axis actually means", () => { + const copy = emptyStateCopy({ + canvas: "board", + filtered: false, + onStatusAxis: true, + }); + expect(copy.message).toMatch(/status/i); + expect(copy.hint).toMatch(/column/i); + }); + + it("claims nothing about statuses off the status axis", () => { + // Grouped by assignee or priority, "no columns" means "no tasks" — the + // project's statuses are not what is missing. + const copy = emptyStateCopy({ + canvas: "board", + filtered: false, + onStatusAxis: false, + }); + expect(`${copy.message} ${copy.hint ?? ""}`).not.toMatch(/status/i); + expect(copy).toEqual(emptyStateCopy({ canvas: "list", filtered: false })); + }); +}); + +describe("the two states are distinguishable", () => { + it("differ in message and icon, not only in tone", () => { + const filtered = noMatch("list"); + const empty = emptyStateCopy({ canvas: "list", filtered: false }); + expect(filtered.message).not.toBe(empty.message); + expect(filtered.icon).not.toBe(empty.icon); + }); + + it("every icon is mapped in every pack", () => { + // An unmapped name renders the Lucide glyph on Fluent and Material — one + // outline icon in a screen of Material Symbols, which reads as a bug. The + // first draft used `FilterX`, which is mapped nowhere. + const icons = [ + emptyStateCopy({ canvas: "list", filtered: true }).icon, + emptyStateCopy({ canvas: "list", filtered: false }).icon, + emptyStateCopy({ canvas: "board", filtered: false, onStatusAxis: true }).icon, + ]; + for (const icon of icons) { + expect(ICON_REGISTRY[icon]?.fluent, `${icon} has no Fluent mapping`).toBeTruthy(); + expect(ICON_REGISTRY[icon]?.material, `${icon} has no Material mapping`).toBeTruthy(); + } + }); + + it("the caller's predicate is the shared one", () => { + // Not a re-derivation: `isFiltered` is what the toolbar's Clear button and + // the `filtered` badge already read, and the empty state must agree with + // the control that caused it. + expect(isFiltered(EMPTY_FILTERS)).toBe(false); + expect(isFiltered({ ...EMPTY_FILTERS, overdue: true })).toBe(true); + }); +}); diff --git a/workbench/control_plane/src/app/projects/lib/emptyState.ts b/workbench/control_plane/src/app/projects/lib/emptyState.ts new file mode 100644 index 000000000..43cb1d090 --- /dev/null +++ b/workbench/control_plane/src/app/projects/lib/emptyState.ts @@ -0,0 +1,77 @@ +/** + * Projects · what an empty canvas says (S4). + * + * Two states, never one. The board used to draw *"Nothing to show. Clear a + * filter, or this project has no statuses yet."* — a sentence that names both + * possible causes and leaves the reader to work out which applies to them, and + * the list drew a bare *"No tasks here yet."* even when a filter was hiding + * everything. `/tasks` had this right: `NoMatchState` says the filters did it + * and offers a way out, `EmptyState` says there is genuinely nothing. + * + * The predicate is `grouping.isFiltered` — the same one the toolbar's Clear + * button and the `filtered` badge already read. A second "is anything + * filtering?" rule here would be the third answer to one question, and the two + * would disagree the first time a filter was added. + * + * Pure, and tested (`emptyState.test.ts`), because the copy IS the feature: a + * component test could not run here (vitest is node-env, `.test.ts` only) and + * "does this screen explain itself" is otherwise fenced by nothing. + */ + +/** Which surface is empty. They differ only where they honestly differ. */ +export type EmptyCanvas = "board" | "list"; + +export interface EmptyStateCopy { + /** + * Lucide name for ``. + * + * ⚠️ **Must be a name `icon-registry.json` maps in every pack.** An unmapped + * name silently falls back to the Lucide glyph, and one Lucide outline in a + * screen of Material Symbols reads as a bug, not as a style. `FilterX` is + * exactly that trap — mapped nowhere — which is why the filtered state wears + * `SearchX` (`search-off` / `search-info`). `emptyState.test.ts` checks it. + */ + icon: string; + message: string; + hint?: string; + /** + * True when filters are the cause — the caller offers **Clear filters**. + * Carried rather than re-derived so the copy and the control cannot drift + * apart: the state that blames the filters is exactly the state that undoes + * them. + */ + filtered: boolean; +} + +export function emptyStateCopy(input: { + canvas: EmptyCanvas; + filtered: boolean; + /** + * Board only: the columns come from the project's statuses, so an unfiltered + * board with no columns means the project has no statuses — a fact worth + * saying, and one the list can never be in. + */ + onStatusAxis?: boolean; +}): EmptyStateCopy { + if (input.filtered) { + return { + icon: "SearchX", + message: "No tasks match your filters.", + hint: "Clear them to see everything here again.", + filtered: true, + }; + } + if (input.canvas === "board" && input.onStatusAxis) { + return { + icon: "Columns3", + message: "This project has no statuses yet.", + hint: "Statuses are the board's columns — add one and tasks have somewhere to sit.", + filtered: false, + }; + } + return { + icon: "ClipboardList", + message: "No tasks here yet.", + filtered: false, + }; +} diff --git a/workbench/control_plane/src/app/projects/page.tsx b/workbench/control_plane/src/app/projects/page.tsx index 063169ed0..cdf567f8b 100644 --- a/workbench/control_plane/src/app/projects/page.tsx +++ b/workbench/control_plane/src/app/projects/page.tsx @@ -1170,6 +1170,9 @@ function ProjectsWorkspace() { changeFilters(EMPTY_FILTERS)} lanes={lanes} onToggleLane={(key) => setLanes((current) => ({ @@ -1195,6 +1198,8 @@ function ProjectsWorkspace() { changeFilters(EMPTY_FILTERS)} statuses={statuses} projectId={selected.id} shownFields={shownFields} diff --git a/workbench/control_plane/src/app/tasks/components/FlatList.tsx b/workbench/control_plane/src/app/tasks/components/FlatList.tsx index a2bbf1c0b..bbc2fe79c 100644 --- a/workbench/control_plane/src/app/tasks/components/FlatList.tsx +++ b/workbench/control_plane/src/app/tasks/components/FlatList.tsx @@ -26,8 +26,6 @@ import { useTaskStore } from "../lib/taskStore"; import { GtdItem, ViewKey } from "../lib/types"; import { TaskCard } from "./TaskCard"; -const NOBODY: ReadonlySet = new Set(); - export function FlatList({ items, view, @@ -37,7 +35,6 @@ export function FlatList({ view: ViewKey; showPriority?: boolean; }) { - const selectMode = useTaskStore((s) => s.selectMode); const selectedIds = useTaskStore((s) => s.selectedIds); const toggleSelected = useTaskStore((s) => s.toggleSelected); const extendSelection = useTaskStore((s) => s.extendSelection); @@ -64,12 +61,14 @@ export function FlatList({ ) ) return; - const picked = selectMode ? selectedIds : NOBODY; + // Ungated, as on /projects: Shift+Arrow sweeps whether or not anything is + // already selected. It used to require entering select mode first. + const picked = selectedIds; const next = stepCursor( rows, { cursor: cursorAt, anchor, selection: picked }, event.key, - selectMode && event.shiftKey, + event.shiftKey, ); if (!next) return; event.preventDefault(); @@ -95,17 +94,29 @@ export function FlatList({ > {items.map((item, index) => { const atCursor = cursorAt >= 0 && cursorAt === index; + const selected = selectedIds.has(item.id); return (
    - {selectMode ? ( -
    ); })} diff --git a/workbench/control_plane/src/app/tasks/components/ItemDetail.tsx b/workbench/control_plane/src/app/tasks/components/ItemDetail.tsx index 18c79fed0..55bf3be28 100644 --- a/workbench/control_plane/src/app/tasks/components/ItemDetail.tsx +++ b/workbench/control_plane/src/app/tasks/components/ItemDetail.tsx @@ -58,7 +58,24 @@ const DISP_TONE: Record = { const ESTIMATES = [5, 15, 30, 60, 120, 240]; -export function ItemDetail() { +/** + * The docked detail pane's entry point — it reads the store's `selectedItemId` + * and picks the surface that item needs. Mounted by `tasks/page.tsx` as the + * third column beside the item list (the house layout, DESIGN_SYSTEM §6), the + * same composition `projects/page.tsx` uses for `TaskPanel`. + * + * `onMaximize` raises the same detail into `TaskFocusModal` for the reading + * width the pane cannot give; `onClose` dismisses the pane. Both are optional — + * without them the maximise button falls back to the store's `openFocus` and no + * ✕ is drawn, so any other mount site keeps working. + */ +export function ItemDetail({ + onMaximize, + onClose, +}: { + onMaximize?: () => void; + onClose?: () => void; +} = {}) { const items = useTaskStore((s) => s.items); const backend = useTaskStore((s) => s.backend); const pushItem = useTaskStore((s) => s.pushItem); @@ -70,14 +87,48 @@ export function ItemDetail() { // Inbox items get the Clarify decision tree (F2). A clarified task gets the // editable detail view below. Keyed by id so state resets per item. + // + // Reachable in the docked pane the ordinary way: select an inbox item, then + // switch to a task view — `selectedItemId` survives the view change. The + // clarify tree carries no header of its own, so the pane's ✕ has to be added + // here or that selection would be undismissable. if (item && item.disposition === "INBOX") { - return ; + if (!onClose) return ; + return ( +
    +
    +
    +
    + +
    +
    + ); } if (item) { - return ; + return ( + + ); } + // Reachable with a selection that no longer resolves — a hydrate/workspace + // switch replaces `items` while `selectedItemId` survives (taskStore keeps it + // across a reload; only the delete paths clear it). return ; } @@ -99,6 +150,8 @@ export function TaskDetail({ backend, pushItem, focused, + onMaximize, + onClose, }: { item: GtdItem; backend: string; @@ -106,6 +159,12 @@ export function TaskDetail({ /** true when rendered inside the full-page focus overlay (hides the * expand button; wider content handled by the modal wrapper). */ focused?: boolean; + /** Docked pane only: raise this same detail into the focus overlay. + * Omitted → the button falls back to the store's `openFocus`. */ + onMaximize?: () => void; + /** Docked pane only: dismiss the pane and give the width back to the list. + * Omitted → no ✕ is drawn (the overlay has its own). */ + onClose?: () => void; }) { const projects = useTaskStore((s) => s.projects); const contexts = useTaskStore((s) => s.contexts); @@ -211,12 +270,16 @@ export function TaskDetail({ Focus )} + {/* Maximise — the docked pane is 380px and this detail is dense + (nine sections plus the provider ones), so the reading width the + overlay gives is an affordance, not a leftover. Hidden inside the + overlay itself, which is already the wide mode. */} {!focused && ( + {/* Close the docked pane (Projects' TaskPanel closes from its own ✕ + too). The overlay draws its own ✕, so this stays off there. */} + {!focused && onClose && ( + + selectAllVisible(visibleIds)} + aria-label="Select every task this view is showing" + className="h-3.5 w-3.5 accent-primary" + /> + Select all + )} )} + {/* The bulk bar — TOP-mounted, directly above the rows it acts on, where + /projects puts it (`projects/components/BulkBar.tsx`: `border-b` + + `bg-muted`, built from Button/Badge). It was bottom-mounted here with + hand-rolled ` + ) : ( + + )} + + + {/* Why this bar is shorter than /projects', and is not unfinished: + its status / assignee / tags / priority controls drive ONE + endpoint, `POST /projects/tasks/bulk`, which resolves each field + per task and reports per-task refusals. /tasks has no counterpart + — `/items/bulk` takes a disposition and `/items/bulk-archive` an + archive flag, and that is the whole bulk surface; `gtd_items` has + no tags column at all (its nearest axis, @context, is single + valued). So archive/restore/delete is the honest set. Widening it + is a gateway ticket, not a bar redesign. */} + + )} + {loading ? (
    @@ -322,9 +407,12 @@ export function ItemList() { // is a PERSON rather than a stage (spec §1 line 46, §6). ) : isBoard ? ( - // The Kanban board (drag-to-refile). Multi-select works on the board - // itself now: in select mode the cards become checkboxes and drag is - // suppressed, so a batch can be archived/deleted without leaving the board. + // The Kanban board (drag-to-refile). ⚠️ The board's cards still draw + // their checkbox only while `selectMode` is true, and `selectMode` is + // now derived from "something is selected" — so on the board the FIRST + // pick comes from Select all (or from a list surface) until the card + // gains its permanent box the way /projects' board card has one. That + // move belongs to `TaskCard`/`TaskBoard`, not to this file.
    @@ -341,80 +429,10 @@ export function ItemList() { // /tasks surfaces where the arrow keys did nothing. )} - - {/* Bulk action bar — archive/restore/delete the current selection. */} - {selectMode && selectedIds.size > 0 && ( -
    - - {selectedIds.size} selected - -
    - {isArchiveView ? ( - { - bulkArchive([...selectedIds], false); - clearSelection(); - }} - /> - ) : ( - { - bulkArchive([...selectedIds], true); - clearSelection(); - }} - /> - )} - { - requestDelete([...selectedIds]); - clearSelection(); - }} - /> - -
    -
    - )}
    ); } -function BulkAction({ - icon: Icon, - label, - onClick, - danger = false, -}: { - icon: ThemedIcon; - label: string; - onClick: () => void; - danger?: boolean; -}) { - return ( - - ); -} - /** Shown when the view has items but the toolbar filters hid them all — a * different message from the true-empty state so the user knows to clear. */ function NoMatchState() { diff --git a/workbench/control_plane/src/app/tasks/components/TaskBoard.tsx b/workbench/control_plane/src/app/tasks/components/TaskBoard.tsx index d6640d39a..1985729b1 100644 --- a/workbench/control_plane/src/app/tasks/components/TaskBoard.tsx +++ b/workbench/control_plane/src/app/tasks/components/TaskBoard.tsx @@ -5,7 +5,7 @@ import { QuickAdd } from "@/components/QuickAdd"; import { useFlash } from "@/components/useFlash"; import { gapKey } from "@/lib/boardDrop"; import { clampCursor, stepCursor } from "@/lib/cursor"; -import { useCallback, useMemo, useState } from "react"; +import { Fragment, useCallback, useMemo, useState } from "react"; import { GtdItem, ViewKey } from "../lib/types"; import { useTaskStore } from "../lib/taskStore"; import { TaskCard } from "./TaskCard"; @@ -73,9 +73,16 @@ export function TaskBoard({ const updateItem = useTaskStore((s) => s.updateItem); const quickAddNext = useTaskStore((s) => s.quickAddNext); const openFocus = useTaskStore((s) => s.openFocus); - // Multi-select for bulk archive/delete — works right on the board now. While - // selecting, cards become selection toggles and drag is suppressed (a checkbox - // and a drag handle on the same card would fight each other). + // Multi-select for bulk archive/delete — works right on the board. + // + // S1: a card is no longer a selection toggle. Its checkbox is a permanent + // sibling of the card (see `TaskCard`), so clicking a card always opens it and + // `selectMode` no longer changes what a click MEANS. What it still does here + // is suppress the drag: `lib/dropRules.dropRefusal` treats select mode as a + // refusal with its own user-facing copy, so making cards draggable while that + // stands would offer a gesture every column then refuses. Un-gating the drag + // is a change to `dropRules` and the bulk bar together, not to this file + // alone — recorded here rather than half-done. const selectMode = useTaskStore((s) => s.selectMode); const selectedIds = useTaskStore((s) => s.selectedIds); const toggleSelected = useTaskStore((s) => s.toggleSelected); @@ -285,14 +292,23 @@ export function TaskBoard({ onDragLeave={() => setOverCol((c) => (c === col.key ? null : c))} onDrop={() => dropColumn(col.key)} className={[ - "relative flex h-full w-72 shrink-0 flex-col overflow-hidden rounded-xl border bg-secondary/30", + // S1: the column's chrome is /projects' — `rounded-lg border + // border-border bg-card`, where this board drew `rounded-xl` on a + // `bg-secondary/30` well. Both radii are themed here (globals.css + // derives the whole `--radius-*` scale from `--radius`, and + // `--radius-xl` IS `--radius`), so this is a convergence, not a + // theming fix: two boards side by side drew the same object at two + // corner radii on two surfaces. The drop highlight stays — + // /projects has no equivalent, and a column that does not react + // while a card hovers over it reads as a refusal. + "relative flex h-full w-72 shrink-0 flex-col overflow-hidden rounded-lg border bg-card", isOver ? "border-primary bg-primary/5" : "border-border", ].join(" ")} > {/* WS-27y backport: the refusal, said on the target while the card hovers — same overlay grammar as the Projects board. */} {refusal ? ( -
    +
    {refusal}
    ) : null} @@ -317,9 +333,16 @@ export function TaskBoard({ {colItems.length}
    -
    + {/* S1: `space-y-1` on the column, /projects' gutter, rather than a + `mb-2` on each card — the gap belongs to the list, and putting + it on the child left the drop gaps spaced differently from the + cards they sit between. Cards and gaps are siblings here for the + same reason they are on /projects: `space-y-*` only reaches + direct children, so a card wrapped in a div of its own opts out + of the spacing the column just set. */} +
    {colItems.map((i, idx) => ( -
    + {/* drop gap ABOVE this card (manual reorder) */} {manual && ( dropAtIndex(col.key, idx)} /> )} -
    = 0 && rows[cursorAt] === i.id - ? "ring-2 ring-ring" - : "", - ].join(" ")} - > - toggleSelected(i.id, shift, rows)} - onDragStart={() => setDragId(i.id)} - onDragEnd={() => { setDragId(null); setOverCol(null); setDropAt(null); }} - /> -
    -
    + = 0 && rows[cursorAt] === i.id} + draggable={!selectMode} + selected={selectedIds.has(i.id)} + // The column IS the stage here — a per-card status pill + // would just repeat it, so it's off on the board. + showStage={false} + onToggleSelected={(shift) => toggleSelected(i.id, shift, rows)} + onDragStart={() => setDragId(i.id)} + onDragEnd={() => { setDragId(null); setOverCol(null); setDropAt(null); }} + /> + ))} {/* trailing gap → drop at the end */} {manual && colItems.length > 0 && ( diff --git a/workbench/control_plane/src/app/tasks/components/TaskCard.tsx b/workbench/control_plane/src/app/tasks/components/TaskCard.tsx index 3d4802af6..e4e8eee45 100644 --- a/workbench/control_plane/src/app/tasks/components/TaskCard.tsx +++ b/workbench/control_plane/src/app/tasks/components/TaskCard.tsx @@ -2,13 +2,12 @@ import Icon, { themedIcon } from "@/components/Icon"; import { TaskCardShell, TaskCardTitle } from "@/components/TaskCardShell"; -import { TaskMeta } from "@/components/TaskMeta"; +import { AvatarStack, TaskMeta } from "@/components/TaskMeta"; import { useState } from "react"; import { GtdItem } from "../lib/types"; import { gtdMetaChips } from "../lib/cardMeta"; import { useTaskStore } from "../lib/taskStore"; import { useCardActions } from "../lib/useCardActions"; -import { initials } from "../lib/utils"; import { contextAccent } from "../lib/contextColors"; import { SourceBadge } from "./SourceBadge"; import { PriorityBadge, SuggestionBadge } from "./PriorityControls"; @@ -34,8 +33,9 @@ export function TaskCard({ draggable = false, showPriority = false, showStage = true, - selectMode = false, selected = false, + atCursor = false, + innerRef, onToggleSelected, onDragStart, onDragEnd, @@ -53,11 +53,18 @@ export function TaskCard({ * isn't otherwise visible (Engage, Priority, a lens-grouped list, the flat * lists) it stays on so the status lives on the card itself. */ showStage?: boolean; - /** Multi-select mode (board): show a checkbox and toggle selection on click - * instead of opening the focus modal. Drag is suppressed by the parent. */ - selectMode?: boolean; selected?: boolean; - /** `shift` extends the selection from the anchor (`@/lib/selection`). */ + /** The keyboard cursor stands on this card (`@/lib/cursor`). Handed to the + * shell, which owns the ring — the board must not draw its own (S1). */ + atCursor?: boolean; + /** Registers the card's own element with the landing flash (`useFlash`), the + * same element /projects attaches: the thing that flashes is the card. */ + innerRef?: (element: HTMLElement | null) => void; + /** Toggles this card's membership of the bulk selection; `shift` extends the + * range from the anchor (`@/lib/selection`). Its presence is what puts the + * checkbox on the card — there is no selection *mode* on the card any more + * (S1, /projects' pattern): the checkbox is a second target beside the card, + * so a click on the card always means "open" and never "select". */ onToggleSelected?: (shift: boolean) => void; onDragStart?: (e: React.DragEvent) => void; onDragEnd?: (e: React.DragEvent) => void; @@ -68,8 +75,18 @@ export function TaskCard({ const project = item.projectId ? projects.find((p) => p.id === item.projectId) : undefined; + // The owner set, as the shared `AvatarStack` wants it — plain identifiers. + // `assignees` is the full list and `assignee` its first entry, but only the + // singular one is filled for a local task, so fall back rather than draw a + // named owner with no avatar. + const assignees = item.assignees?.length + ? item.assignees + : item.assignee + ? [item.assignee] + : []; // Owners beyond the primary (shown as a "+N" on the avatar). - const extraAssignees = Math.max(0, (item.assignees?.length ?? 0) - 1); + const extraAssignees = Math.max(0, assignees.length - 1); + const assigneeNames = assignees.map((p) => p.name); // Shared actions (schedule / stage / done / eliminate) power both the inline // controls and the right-click menu, so they never drift. @@ -192,6 +209,7 @@ export function TaskCard({ return ( <>
    openFocus(item.id)} @@ -221,9 +239,11 @@ export function TaskCard({ )} - {item.assignee && ( - - )} + {/* `max={1}` keeps this app's one-avatar-plus-"+N" reading, which + is what the deleted local AvatarStack drew; /projects shows up + to three because its rows are wider. Same component either way + (S1) — the cap is a caller's decision, the pixels are not. */} +
    @@ -232,90 +252,109 @@ export function TaskCard({ ); } - // In select mode the card is a selection toggle, not a link: clicking checks - // the box (drag is disabled by the parent) so a batch can be archived/deleted - // right on the board. A selected card gets a primary ring. - const activate = (shift: boolean) => { - if (selectMode) onToggleSelected?.(shift); - else openFocus(item.id); - }; + // The card opens the task. Always — there is no mode in which a click on it + // means something else (S1, /projects' rule). Selecting is the checkbox's + // job, and it is a different target; `shift` is ignored here because the + // range-extend gesture belongs to the control that starts a range. + const activate = () => openFocus(item.id); + const done = Boolean(item.completedAt); return ( <> {/* WS-27ad: the box is `@/components/TaskCardShell`, the same one the /projects board draws — same radius, padding, `bg-card` surface, border and shadow lift. What goes INSIDE stays this app's: the GTD badges, the context menu and the priority pair are not concepts - /projects has. */} - - {selectMode ? ( + /projects has. + + S1: the checkbox is a SIBLING of the box, not a child of it — the + same `flex items-start gap-1.5` row /projects lays out. Inside the + card it sat on top of the drag grip in the one corner both wanted, + and a card that is also a checkbox is one gesture with two meanings. + Outside, there are two targets and the card's whole surface stays the + open affordance. */} +
    + {onToggleSelected ? ( - onToggleSelected?.((e.nativeEvent as MouseEvent).shiftKey) + onToggleSelected((e.nativeEvent as MouseEvent).shiftKey) } + // The click must not also open the task. onClick={(e) => e.stopPropagation()} aria-label={selected ? "Deselect task" : "Select task"} - className="absolute right-1.5 top-1.5 h-4 w-4 accent-primary" + className="mt-3 h-4 w-4 shrink-0 accent-primary" /> - ) : ( - draggable && ( - - ) - )} - {/* Top corners: priority pill top-LEFT (above the title — always shown - on the board, it's the ranking signal) · the action nudge top-RIGHT. - pr-5 keeps the nudge clear of the absolute grip/checkbox. */} -
    - - - - -
    -
    - {!selectMode && showStage && } - {item.title} -
    - {item.nextAction && item.nextAction !== item.title && ( -

    - {item.nextAction} -

    - )} - {project && ( - - - {project.outcome} - - )} -
    {meta}
    -
    - -
    - - {item.assignee && ( - - - - {extraAssignees > 0 - ? `${item.assignee.name} +${extraAssignees}` - : item.assignee.name} + ) : null} + + {/* Top corners: priority pill top-LEFT (above the title — always + shown on the board, it's the ranking signal) · the action nudge + top-RIGHT. No `pr-5` any more: the corner the grip and the + checkbox used to share is free. The drag grip is GONE rather than + relocated — the whole card is `draggable`, so it never was a + handle, only a hint pointing at a spot that is not special; + /projects draws none; and the shell's hover lift is the affordance + both boards already use to say "pick me up". */} +
    + + + + +
    +
    + {/* Shown whenever the surface does not already say what the stage + is — the board passes `showStage={false}` because its columns + ARE the stage. It used to be hidden in select mode too, which + was a consequence of the card being the checkbox; it is not one + any more. The pill stays interactive (S1). */} + {showStage && } + {item.title} +
    + {item.nextAction && item.nextAction !== item.title && ( +

    + {item.nextAction} +

    + )} + {project && ( + + + {project.outcome} + + )} +
    {meta}
    +
    + +
    + + {item.assignee && ( + + + + {extraAssignees > 0 + ? `${item.assignee.name} +${extraAssignees}` + : item.assignee.name} + - - )} + )} +
    -
    - + +
    {contextMenu} ); @@ -340,25 +379,10 @@ function ScheduleButton({ onClick }: { onClick: () => void }) { ); } -function Avatar({ name }: { name: string }) { - return ( - - {initials(name)} - - ); -} - -/** The primary owner's avatar with a small "+N" when a task has more than one - * assignee, so a shared task reads as shared at a glance. */ -function AvatarStack({ primary, extra }: { primary: { name: string }; extra: number }) { - return ( - - - {extra > 0 && ( - - +{extra} - - )} - - ); -} +// S1: the local `Avatar` / `AvatarStack` that used to live here are gone. They +// were a second copy of `@/components/TaskMeta`'s pair — same initials, same +// "+N" badge, same ring — differing only in that this one took a Person and a +// count while the shared one takes identifiers. A second implementation of a +// shared seam is a defect (AGENTS.md rule 4), and this one was the reason a +// /tasks avatar and a /projects avatar could drift apart without either app's +// tests noticing. diff --git a/workbench/control_plane/src/app/tasks/components/TaskFocusModal.tsx b/workbench/control_plane/src/app/tasks/components/TaskFocusModal.tsx index 944d450e9..81bfb5b85 100644 --- a/workbench/control_plane/src/app/tasks/components/TaskFocusModal.tsx +++ b/workbench/control_plane/src/app/tasks/components/TaskFocusModal.tsx @@ -1,32 +1,63 @@ "use client"; import Icon from "@/components/Icon"; -import { useEffect } from "react"; +import { useEffect, useMemo } from "react"; import { useTaskStore } from "../lib/taskStore"; import { TaskDetail } from "./ItemDetail"; +const NOOP = () => {}; + // Full-page task view — a ClickUp/Linear-style focused overlay over the same -// editable TaskDetail. Opened via the expand button in the side-panel detail -// (or wherever openFocus is called). Escape / backdrop / × closes it. -export function TaskFocusModal() { +// editable TaskDetail. Escape / backdrop / × closes it. It has two lives, and +// `tasks/page.tsx` picks which one by whether it passes `itemId`: +// +// * UNCONTROLLED (``) — driven by the store's +// `focusedItemId`, i.e. `openFocus` from anywhere. This is the phone branch, +// where the surface IS the screen (the same move Projects makes at +// `projects/page.tsx:1329`), and every desktop surface that carries no +// docked pane: Inbox, Engage, Calendar, Assistant. +// * CONTROLLED (`itemId` + `onClose`) — the explicit **maximise** from the +// docked detail pane. The pane is 380px and this detail is far denser than +// Projects' (nine sections plus the provider ones), so maximise is what +// keeps this `max-w-3xl` reading width available. The page derives `itemId` +// from the pane's own selection, so maximise is a mode of that selection: +// deleting the task, or navigating to another one (a subtask opened from +// inside the overlay moves `selectedItemId`), lands back on the docked pane +// instead of leaving a stale overlay up. +export function TaskFocusModal({ + itemId, + onClose, +}: { + /** Present (even as `null`) → controlled. Absent → store-driven. */ + itemId?: string | null; + onClose?: () => void; +} = {}) { const focusedItemId = useTaskStore((s) => s.focusedItemId); const items = useTaskStore((s) => s.items); const backend = useTaskStore((s) => s.backend); const pushItem = useTaskStore((s) => s.pushItem); const closeFocus = useTaskStore((s) => s.closeFocus); - const item = focusedItemId - ? items.find((i) => i.id === focusedItemId) - : undefined; + const controlled = itemId !== undefined; + const openId = controlled ? itemId : focusedItemId; + // useMemo, not a plain ternary: the Escape listener below depends on it, and + // a fresh closure each render would re-bind the listener on every keystroke + // the detail receives. + const close = useMemo( + () => (controlled ? (onClose ?? NOOP) : closeFocus), + [controlled, onClose, closeFocus], + ); + + const item = openId ? items.find((i) => i.id === openId) : undefined; useEffect(() => { - if (!focusedItemId) return; + if (!openId) return; const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape") closeFocus(); + if (e.key === "Escape") close(); }; window.addEventListener("keydown", onKey); return () => window.removeEventListener("keydown", onKey); - }, [focusedItemId, closeFocus]); + }, [openId, close]); if (!item) return null; @@ -36,15 +67,17 @@ export function TaskFocusModal() { )} - {selectMode ? ( - // In select mode the whole row toggles selection; the content is inert - // so a click selects rather than opening the task. - - ) : ( -
    - -
    - )} + {/* The row content is the OPEN affordance, always — it no longer turns + into a selection toggle behind a mode. Selecting is the checkbox. */} +
    + +
    {hasSubtasks && expanded && }
    diff --git a/workbench/control_plane/src/app/tasks/lib/selectionParity.test.ts b/workbench/control_plane/src/app/tasks/lib/selectionParity.test.ts new file mode 100644 index 000000000..858537bd5 --- /dev/null +++ b/workbench/control_plane/src/app/tasks/lib/selectionParity.test.ts @@ -0,0 +1,184 @@ +/** + * Selection parity with /projects — the fence for the rule this slice + * introduces (R7). + * + * The rule, stated once: **selection is always available on /tasks, and + * `selectMode` decides only whether the bulk bar is up.** It is no longer a + * mode you enter, and it never again changes what a click means. A row's click + * opens the task; the checkbox beside it selects. That is /projects' grammar, + * which the owner ruled canonical on 2026-08-10. + * + * Two halves, because the rule can be broken from two directions: + * + * 1. **The store** can let `selectMode` drift from "something is selected", + * at which point it is a mode again and the bulk bar starts lying about + * what a bulk action would hit. + * 2. **A surface** can start consulting `selectMode` again — which is exactly + * how the modal behaviour was written the first time, one `selectMode ? …` + * at a time. So the three list surfaces are asserted not to read it at all. + * + * The second half is a source-text check for the same reason `conformance.test` + * and `sharedTaskUi.test` are: there is no DOM test harness in this tree, and a + * regression here is invisible in review — it renders fine, it just means + * something different. + */ + +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +import { beforeEach, describe, expect, it } from "vitest"; + +import { useTaskStore } from "./taskStore"; + +const read = (rel: string) => + readFileSync(fileURLToPath(new URL(rel, import.meta.url)), "utf8"); + +/** The surfaces this slice owns: every one draws rows and a selection. */ +const SURFACES = { + "components/ItemList.tsx": read("../components/ItemList.tsx"), + "components/TaskListGrouped.tsx": read("../components/TaskListGrouped.tsx"), + "components/FlatList.tsx": read("../components/FlatList.tsx"), +}; + +const ROWS = ["a", "b", "c", "d"]; + +describe("the store cannot turn selectMode back into a mode", () => { + beforeEach(() => { + useTaskStore.getState().clearSelection(); + }); + + /** The whole invariant, checked after every transition below. */ + const invariant = () => { + const s = useTaskStore.getState(); + expect( + s.selectMode, + "selectMode must be exactly `selectedIds.size > 0` — it means 'the bulk " + + "bar is up', nothing else. Route the write through applySelection().", + ).toBe(s.selectedIds.size > 0); + }; + + it("has no way to enter a mode: the setter is gone", () => { + expect( + "setSelectMode" in useTaskStore.getState(), + "setSelectMode is the entry to a mode that no longer exists. Selection " + + "is available unconditionally; nothing turns it on.", + ).toBe(false); + }); + + it("rises with the first pick and falls with the last", () => { + const { toggleSelected } = useTaskStore.getState(); + invariant(); + expect(useTaskStore.getState().selectMode).toBe(false); + + toggleSelected("b", false, ROWS); + invariant(); + expect(useTaskStore.getState().selectMode).toBe(true); + + toggleSelected("b", false, ROWS); + invariant(); + expect(useTaskStore.getState().selectMode).toBe(false); + }); + + it("shift-click still sweeps the shared range, with nothing to enter first", () => { + const { toggleSelected } = useTaskStore.getState(); + toggleSelected("b", false, ROWS); + toggleSelected("d", true, ROWS); + expect([...useTaskStore.getState().selectedIds].sort()).toEqual(["b", "c", "d"]); + // The anchor stays put, so a second shift-click widens the same range. + expect(useTaskStore.getState().selectAnchor).toBe("b"); + invariant(); + }); + + it("select-all takes the VISIBLE set, and unticks to nothing", () => { + const { selectAllVisible } = useTaskStore.getState(); + // The filtered view, not the store: two of the four rows are on screen. + selectAllVisible(["a", "c"]); + expect([...useTaskStore.getState().selectedIds].sort()).toEqual(["a", "c"]); + invariant(); + + selectAllVisible(["a", "c"]); + expect(useTaskStore.getState().selectedIds.size).toBe(0); + invariant(); + }); + + it("prunes a selection that outlived its filter", () => { + const { toggleSelected, pruneSelection } = useTaskStore.getState(); + toggleSelected("a", false, ROWS); + toggleSelected("d", true, ROWS); + expect(useTaskStore.getState().selectedIds.size).toBe(4); + + // The search box narrowed the view to one row: a later "Archive" must not + // reach the three that scrolled out of the filter. + pruneSelection(["c"]); + expect([...useTaskStore.getState().selectedIds]).toEqual(["c"]); + invariant(); + + pruneSelection([]); + expect(useTaskStore.getState().selectedIds.size).toBe(0); + invariant(); + }); + + it("keeps the selection identity stable when nothing left the screen", () => { + const { toggleSelected, pruneSelection } = useTaskStore.getState(); + toggleSelected("a", false, ROWS); + const before = useTaskStore.getState().selectedIds; + // This runs from an effect on every filter keystroke; a fresh Set each time + // would re-render every subscriber for no change. + pruneSelection(ROWS); + expect(useTaskStore.getState().selectedIds).toBe(before); + }); +}); + +describe("no list surface consults selectMode", () => { + it.each(Object.entries(SURFACES))("%s never reads it", (_file, source) => { + expect( + source.includes("s.selectMode") || /\bselectMode\b\s*(\?|&&)/.test(source), + "A surface gating an affordance on selectMode is the modal behaviour " + + "coming back. The checkbox is unconditional and the row's click opens " + + "the task — see the note at the top of ItemList.tsx.", + ).toBe(false); + }); + + it("every row surface draws a checkbox with no condition on it", () => { + for (const file of ["components/TaskListGrouped.tsx", "components/FlatList.tsx"]) { + const source = SURFACES[file as keyof typeof SURFACES]; + // The checkbox is a plain sibling of the row content, so the JSX around + // it is a label — not a ternary picking between a box and a grip. + expect(source, `${file} lost its row checkbox`).toContain('type="checkbox"'); + expect( + /\{\s*select\w*\s*\?[\s\S]{0,200}type="checkbox"/.test(source), + `${file} draws its checkbox behind a condition again`, + ).toBe(false); + } + }); + + it("the header offers select-all", () => { + expect(SURFACES["components/ItemList.tsx"]).toContain("selectAllVisible"); + }); +}); + +describe("the bulk bar is the one /projects draws", () => { + const CHROME = "border-b border-border bg-muted px-3 py-2"; + + it("both apps mount it at the top with the same chrome", () => { + // Same string in both files: a bar that moves or re-tints on one side has + // to move on the other, which is the whole point of "one product". + expect(read("../../projects/components/BulkBar.tsx")).toContain(CHROME); + expect( + SURFACES["components/ItemList.tsx"], + "The /tasks bulk bar was bottom-mounted (`border-t`) with hand-rolled " + + "buttons. It is top-mounted on the shared chrome now.", + ).toContain(CHROME); + }); + + it("its controls are primitives, not hand-rolled buttons", () => { + const source = SURFACES["components/ItemList.tsx"]; + expect(source).toContain('from "@/components/ui/Button"'); + expect(source).toContain('from "@/components/ui/Badge"'); + expect( + source, + "`BulkAction` was a hand-rolled + ) : null} + + ); +} diff --git a/workbench/control_plane/src/components/TaskCardShell.tsx b/workbench/control_plane/src/components/TaskCardShell.tsx index b9d9d944a..85d24212b 100644 --- a/workbench/control_plane/src/components/TaskCardShell.tsx +++ b/workbench/control_plane/src/components/TaskCardShell.tsx @@ -10,10 +10,17 @@ * hover and different selected states. Two objects, one concept. * * /tasks' box wins, and the reason is not seniority: `bg-card` is the token - * that means "a raised surface" (a board column is already `bg-secondary/30` - * and `bg-background` is the page under it, so a /projects card was a card- - * shaped hole rather than a card), and the shadow lift is what makes a - * draggable object read as pick-up-able before anybody drags it. + * that means "a raised surface" (`bg-background` is the page under it, so a + * /projects card was a card-shaped hole rather than a card), and the shadow + * lift is what makes a draggable object read as pick-up-able before anybody + * drags it. **That lift is the only drag affordance either board has** — + * neither draws a grip, because the whole card is `draggable` and a grip + * points at a spot that is not special (S1). + * + * S1 also removed the last claim this comment used to make about the surface + * underneath: /tasks' columns were `bg-secondary/30` and /projects' `bg-card`, + * and both are `bg-card` now. A card is told apart from its column by border + * and shadow, not by a second fill. * * WHAT goes inside stays each app's business — /projects feeds it * `lib/card.ts` facts and honours its own `shown_fields`, /tasks feeds it GTD @@ -97,6 +104,17 @@ export function TaskCardShell({ ); } +/** + * A caller-supplied class that fights the shell's own clamp. + * + * Stripped rather than merged, and that is the whole design decision: `truncate` + * sets `white-space: nowrap`, `line-clamp-2` sets `display: -webkit-box`, and + * when both land on one element which of them wins is CSS source order — i.e. + * invisible in review and not necessarily the same in dev and in a production + * build. So the clamp belongs to exactly one place, this file. + */ +const CLAMP_OVERRIDE = /\b(?:truncate|line-clamp-\d+|whitespace-nowrap)\b/g; + /** * The card's title line. * @@ -104,6 +122,21 @@ export function TaskCardShell({ * the two cards: /projects drew a plain `text-sm`, /tasks a `text-[13px] * font-medium leading-snug`, and side by side that alone made them look like * different products. + * + * **How many lines it gets is decided here, for both apps (S1).** They + * disagreed: /projects clamped a board title to one line, /tasks let it wrap + * without limit — and neither is right for the other's data. A `pm_tasks` title + * is usually a noun phrase, so one line loses little; a GTD next action is + * often a whole sentence ("email Priya the revised quote before the review"), + * and one line cuts it exactly where the verb's object would have been. Two + * lines is the compromise, taken in the shared file so the two boards cannot + * drift apart again (AGENTS.md rule 4). + * + * What it costs: a /tasks title longer than two lines now ends in an ellipsis + * where it used to wrap in full, and a /projects card is up to one line taller. + * The full text is a click away in either app's detail view. A surface that + * genuinely needs a different clamp should grow a prop here rather than pass a + * class — see `CLAMP_OVERRIDE`. */ export function TaskCardTitle({ children, @@ -117,9 +150,9 @@ export function TaskCardTitle({ return (

    {children} diff --git a/workbench/control_plane/src/lib/sharedTaskUi.test.ts b/workbench/control_plane/src/lib/sharedTaskUi.test.ts index fb31bd30a..d6983230f 100644 --- a/workbench/control_plane/src/lib/sharedTaskUi.test.ts +++ b/workbench/control_plane/src/lib/sharedTaskUi.test.ts @@ -51,7 +51,13 @@ const FILES = sourceFiles(); * `const FOO: Record<…> =`) rather than a mention, so a file that imports the * symbol, re-exports it, or names it in a comment is not an offender. */ -const SEAM: { what: string; home: string; declaration: RegExp }[] = [ +const SEAM: { + what: string; + home: string; + declaration: RegExp; + /** Files allowed to match anyway, each with the argument for why (S1). */ + except?: Record; +}[] = [ { what: "the keyboard cursor", home: "lib/cursor.ts", @@ -107,23 +113,75 @@ const SEAM: { what: string; home: string; declaration: RegExp }[] = [ home: "components/TaskCardShell.tsx", declaration: /export\s+function\s+TaskCardShell\b/, }, + { + what: "the card title, and how many lines it gets", + home: "components/TaskCardShell.tsx", + declaration: /export\s+function\s+TaskCardTitle\b/, + }, + { + /** + * S1. The offender this catches was NOT exported — `app/tasks/components/ + * TaskCard.tsx` held a private `Avatar` + `AvatarStack` pair drawing the + * same initials, the same "+N" and the same ring as the shared one. So the + * `export` is optional in this pattern, unlike every row above it: a second + * copy is a defect whether or not anybody else can import it. + */ + what: "the avatar stack", + home: "components/TaskMeta.tsx", + declaration: /(?:^|\n)\s*(?:export\s+)?function\s+AvatarStack\b/, + except: { + "components/room/Identity.tsx": + "a name collision, not a second copy: the room strip takes " + + "RoomParticipant rows, draws photographs with a presence ring, and " + + "colours each face by the per-person identity hue that " + + "conformance.test.ts excepts from the colour rule precisely because it " + + "must NOT follow the theme. Folding it into TaskMeta's initials would " + + "mean teaching a task chip about presence. Renaming one of the two is " + + "the real fix and it is a rooms decision, not a Projects↔Tasks one", + }, + }, + { + what: "the empty state", + home: "components/EmptyState.tsx", + declaration: /export\s+function\s+EmptyState\b/, + }, ]; describe("one implementation, consumed twice", () => { - it.each(SEAM)("$what is declared only in $home", ({ home, declaration }) => { - const offenders = FILES.filter((f) => f !== home && declaration.test(read(f))); - expect( - offenders, - `A second copy of what ${home} owns. Import it (or re-export it); ` + - "two implementations of one interaction is how /projects and /tasks " + - "stopped looking like one product.", - ).toEqual([]); - }); + it.each(SEAM)( + "$what is declared only in $home", + ({ home, declaration, except }) => { + const offenders = FILES.filter( + (f) => f !== home && !(f in (except ?? {})) && declaration.test(read(f)), + ); + expect( + offenders, + `A second copy of what ${home} owns. Import it (or re-export it); ` + + "two implementations of one interaction is how /projects and /tasks " + + "stopped looking like one product.", + ).toEqual([]); + }, + ); it.each(SEAM)("$home exists and still declares it", ({ home, declaration }) => { expect(FILES, `${home} is missing — update SEAM in this file`).toContain(home); expect(read(home)).toMatch(declaration); }); + + it.each(SEAM.filter((s) => s.except))( + "every exemption from $what still names a real one", + ({ declaration, except }) => { + // An exemption outliving its offender is latitude nobody asked for — the + // same rule conformance.test.ts applies to PALETTE_EXCEPTIONS. + for (const f of Object.keys(except ?? {})) { + expect(FILES, `${f} is gone — drop its exemption`).toContain(f); + expect( + read(f), + `${f} no longer declares this — drop its exemption`, + ).toMatch(declaration); + } + }, + ); }); describe("both apps reach the shared modules", () => { @@ -152,6 +210,11 @@ describe("both apps reach the shared modules", () => { ["projects", "components/TaskCardShell"], ["projects", "components/DropGap"], ["projects", "lib/boardDrop"], + // S4 — /tasks is deliberately absent: `ItemList.tsx` still holds the + // original local `NoMatchState`/`EmptyState` pair this was promoted FROM, + // and retiring them onto the shared box is a `/tasks` edit that another + // slice holds open. Add the row in the change that does it. + ["projects", "components/EmptyState"], ["tasks", "lib/cursor"], ["tasks", "lib/selection"], ["tasks", "components/QuickAdd"], @@ -230,3 +293,150 @@ describe("neither task app re-declares the colour palette", () => { ).toEqual([]); }); }); + +/** + * S1 — the card shell's props are USED, and the boards agree about the column. + * + * ## Why these are source scans and not render tests + * + * The obvious fence for "a caller passes `completed`" is to render the card and + * look for the strike-through. This runner cannot: `vitest.config.ts` sets + * `environment: "node"` and `include: ["src/**\/*.test.ts"]` — no DOM, and + * `.tsx` test files are not even collected. Adding jsdom and a rendering library + * to fence one prop is a bigger change than the thing being fenced, so these + * read the source, exactly as the rest of this file and `conformance.test.ts` + * do. **What they can prove is that the wiring exists; what they cannot prove is + * what it looks like.** The second half is `DESIGN_SYSTEM.md` §8 — switch the + * theme and look at both boards. + */ +describe("the shared card shell is wired, not merely imported", () => { + /** + * Source with its comments removed. + * + * Every scan below looks for a class name, and the comment that explains why + * a class was REMOVED contains it — the first run of this block failed on the + * note in `app/tasks/components/TaskBoard.tsx` saying it no longer draws + * `rounded-xl`. A gate that a code comment can trip is a gate that teaches + * people not to comment. (`conformance.test.ts` learned the same thing about + * `hsl(…)` in a comment; the `//` guard here is its lookbehind, which keeps + * a `https://` out of it.) + */ + const code = (rel: string) => + read(rel) + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/(?` opening tag in the tree, with its file. + * + * Brace-aware rather than `//`, because the lazy regex + * stops at the `>` of the first arrow function in the attribute list — so it + * would read a tag as ending at `onActivate={() =>`, and whether the props + * below were seen would depend on the order somebody happened to write them + * in. That is a fence that passes for the wrong reason. + */ + const shellTags = (): { file: string; tag: string }[] => { + const out: { file: string; tag: string }[] = []; + for (const file of FILES) { + const text = code(file); + for (const m of text.matchAll(/" && depth === 0) break; + } + out.push({ file, tag: text.slice(m.index, i) }); + } + } + return out; + }; + + it("something actually renders it", () => { + // Guards the shape of every assertion below: a regex that matches nothing + // passes every `every()` in this block. + expect(shellTags().length).toBeGreaterThan(1); + }); + + /** + * The bug this exists for: `TaskCardShell` grew `completed` for /projects, and + * for months **nothing under `src/app/tasks/` passed it** — so a finished task + * was dimmed and struck through on one board and drawn like live work on the + * other, from one component, with every test green. A prop that only one of + * two callers sets is a divergence with a shared component's name on it. + */ + it.each(["completed", "atCursor"])( + "every caller says whether the card is %s", + (prop) => { + const missing = shellTags() + .filter(({ tag }) => !new RegExp(`\\b${prop}=`).test(tag)) + .map(({ file }) => file); + expect( + missing, + `These render a task card without telling it \`${prop}\`, so the shell ` + + "falls back to `false` and the surface silently loses the treatment " + + "the other board has. If a surface genuinely has no keyboard cursor " + + "or no notion of done, pass the literal and say why — do not drop " + + "the prop, which is indistinguishable from forgetting it.", + ).toEqual([]); + }, + ); + + /** + * The cursor ring belongs to the shell (`atCursor`), and the /tasks board used + * to re-draw it on a wrapper div of its own — the ring then sat a pixel off + * the card's own radius, and the two boards had two implementations of one + * signal. List and table surfaces are deliberately NOT in scope: their rows + * are not cards and draw `ring-inset` on a ``/`

  • ` themselves. + */ + it("no board re-implements the card cursor ring", () => { + const BOARDS = [ + "app/tasks/components/TaskBoard.tsx", + "app/projects/components/TaskBoard.tsx", + ]; + for (const f of BOARDS) expect(FILES, `${f} moved`).toContain(f); + const offenders = BOARDS.filter((f) => /ring-ring/.test(code(f))); + expect( + offenders, + "Pass `atCursor` to TaskCardShell instead. A second copy of the ring is " + + "how the two boards stopped agreeing about where the cursor is.", + ).toEqual([]); + expect( + read("components/TaskCardShell.tsx"), + "…and the shell has to still draw it.", + ).toMatch(/ring-ring/); + }); + + /** + * The column chrome, held equal. + * + * ⚠️ This is NOT a theming rule, and it would be wrong to write it as one. + * `AGENTS.md` rule 6 says `rounded-xl` "is a fixed 12px that ignores + * Graphite's 0.125rem" — in this tree that is **false**: `src/app/globals.css` + * derives the whole `--radius-*` scale from `--radius` inside `@theme`, and + * `--radius-xl` is literally `var(--radius)`, i.e. the same value + * `rounded-lg` resolves to. A tree-wide `rounded-xl` ratchet would therefore + * baseline ~274 occurrences that are all correctly themed. + * + * What was actually wrong is narrower and is what this checks: the two boards + * drew one object — a column of task cards — at two radii on two surfaces + * (`rounded-xl` + `bg-secondary/30` vs `rounded-lg` + `bg-card`). /projects is + * canonical, so both are `rounded-lg` on `bg-card` now, and a fixed-radius + * class in either file is a re-divergence. + */ + it("both boards draw their columns with the same radius", () => { + const offenders = [ + "app/tasks/components/TaskBoard.tsx", + "app/projects/components/TaskBoard.tsx", + ].filter((f) => /\brounded-(?:xl|2xl|3xl)\b/.test(code(f))); + expect( + offenders, + "The two boards' columns have to be the same shape — `rounded-lg`, " + + "/projects' chrome. This is a continuity rule, not a theming one: " + + "`rounded-xl` IS themed here (globals.css maps --radius-xl to " + + "--radius), it is just a different corner from the board next door.", + ).toEqual([]); + }); +}); diff --git a/workbench/control_plane/src/lib/sidePanelStore.ts b/workbench/control_plane/src/lib/sidePanelStore.ts index 527a06637..5bceff576 100644 --- a/workbench/control_plane/src/lib/sidePanelStore.ts +++ b/workbench/control_plane/src/lib/sidePanelStore.ts @@ -240,7 +240,7 @@ let _ctxSig: string | null = null; /** Stable list of docs open for `sessionId` (same ref until it actually changes). */ export function getOpenDocsForSession(sessionId: string): OpenDoc[] { const fresh = _state.docs.filter((d) => d.sessionId === sessionId); - const sig = fresh.map((d) => d.path).join(""); + const sig = fresh.map((d) => d.path).join("\0"); if (_ctxCache && _ctxSig === sig) return _ctxCache; _ctxCache = fresh; _ctxSig = sig; diff --git a/workbench/control_plane/src/lib/sourceHygiene.test.ts b/workbench/control_plane/src/lib/sourceHygiene.test.ts new file mode 100644 index 000000000..e62ac9d44 --- /dev/null +++ b/workbench/control_plane/src/lib/sourceHygiene.test.ts @@ -0,0 +1,67 @@ +/** + * Source files must be readable by the tools that police them. + * + * `src/app/projects/components/TaskBoard.tsx` carried a literal NUL byte at + * offset 10623 — a composite-key separator written as a raw NUL byte inside a + * template literal rather than the two-character escape `\0`. Both produce the + * same runtime string. Only one of them keeps the file plain text. + * + * The consequence was not cosmetic. **ripgrep stops at the first NUL**, so every + * directory-wide scan silently saw only the first third of that file — and it + * stayed silent when the file was named explicitly too. This repo's guard rails + * are overwhelmingly source scans (`sharedTaskUi.test.ts`, the theme conformance + * suite, the Python-side structural tests, and every `grep` a human or an agent + * runs while deciding what is true). A file that reads as binary is a file those + * scans quietly exempt, and an exemption nobody declared is the worst kind: the + * fences all pass, and they pass because they never looked. + * + * A NUL is the only byte that does this, which is why this test checks for that + * and not for "unusual characters" generally — the tree is full of legitimate + * em-dashes, arrows and emoji, and a broad rule would be noise that gets muted. + */ + +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +const SRC = join(__dirname, ".."); + +/** Every source file under `src/`, excluding build output and dependencies. */ +function sourceFiles(dir: string, found: string[] = []): string[] { + for (const entry of readdirSync(dir)) { + if (entry === "node_modules" || entry === ".next" || entry.startsWith(".")) { + continue; + } + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + sourceFiles(full, found); + } else if (/\.(ts|tsx|js|jsx|css|json|md)$/.test(entry)) { + found.push(full); + } + } + return found; +} + +describe("source files stay greppable", () => { + it("no source file contains a NUL byte", () => { + const offenders = sourceFiles(SRC) + .map((path) => ({ path, at: readFileSync(path).indexOf(0) })) + .filter(({ at }) => at !== -1) + .map(({ path, at }) => `${path.slice(SRC.length + 1)} (offset ${at})`); + + expect( + offenders, + "A NUL byte makes ripgrep treat the file as binary and stop reading at " + + "that offset — so every source-scanning fence in this repo silently " + + "skips the rest of the file while still reporting green. If you need a " + + "NUL in a string, write the escape `\\0`: same runtime value, plain-text " + + "file.", + ).toEqual([]); + }); + + it("finds files at all", () => { + // Guards the guard: a walker that silently matched nothing would make the + // test above pass while checking an empty list. + expect(sourceFiles(SRC).length).toBeGreaterThan(200); + }); +}); diff --git a/workbench/control_plane/src/lib/theme/conformance.test.ts b/workbench/control_plane/src/lib/theme/conformance.test.ts index e7446f7ee..e47f612b4 100644 --- a/workbench/control_plane/src/lib/theme/conformance.test.ts +++ b/workbench/control_plane/src/lib/theme/conformance.test.ts @@ -473,6 +473,87 @@ describe("no raw Tailwind palette colours", () => { }); }); +// ── Rule 6: active/selected wears the house token ─────────────────────────── + +describe("active and selected use the house token", () => { + /** + * `bg-accent text-accent-foreground` — the synonym, not the norm. + * + * AGENTS.md rule 6 names the measured house token for active/selected: + * `bg-primary/10 text-primary`, which is what /tasks, /email and + * `src/components` draw. `accent` is a *different* token in every theme (on + * Graphite it is barely distinguishable from `secondary`, on Material it is + * a tinted surface), so a pill selected in Projects and a pill selected in + * Tasks were two different colours on every theme at once — one product, + * two selections. Nothing in the five rules above could see it: both halves + * are perfectly legal theme tokens, wrongly paired. + * + * Deliberately narrow — the PAIR, not `bg-accent` alone. `hover:bg-accent` + * is an ordinary hover tint and `bg-accent/10` is a chip; flagging those + * would make this the gate somebody switches off. + */ + const ACCENT_ACTIVE = /\bbg-accent\s+text-accent-foreground\b/g; + + /** + * Where the pair is a hue rather than a state, with the argument. + * + * The bar is the same as COLOR_EXCEPTIONS': it has to be the wrong rule, not + * merely inconvenient to migrate. + */ + const ACTIVE_EXCEPTIONS: Record = { + "lib/statusAccent.ts": + "the violet lane's CHIP — a tag/status hue, not a selection; `accent` is " + + "the one token pair that reads distinctly without competing with primary " + + "(see the constant's own note)", + }; + + /** + * The remaining call sites, per file. Same ratchet as the rules above: a + * file with no budget must be clean, a baselined file may not get worse, and + * one that got better fails until its number comes down. + * + * `app/projects/components/MyWork.tsx` was in this list at 2 and is not any + * more (S4). The three below are the rest of the Projects sweep. + */ + const ACTIVE_DEBT: Record = { + "app/projects/components/FilterBar.tsx": 2, + "app/projects/components/SearchPalette.tsx": 1, + "app/people/page.tsx": 1, + }; + + const activeExcepted = (rel: string) => matches(rel, Object.keys(ACTIVE_EXCEPTIONS)); + + it("a file with no budget uses bg-primary/10 text-primary", () => { + const offenders = sourceFiles() + .filter((f) => !(f in ACTIVE_DEBT) && !activeExcepted(f)) + .map((f) => [f, count(read(f), ACCENT_ACTIVE)] as const) + .filter(([, n]) => n > 0); + expect( + offenders, + "Active/selected is `bg-primary/10 text-primary` (AGENTS.md rule 6) — the " + + "token every other app in this tree selects with. `bg-accent " + + "text-accent-foreground` resolves to a different colour per theme, so " + + "the same selection reads two ways in two apps.", + ).toEqual([]); + }); + + it("no baselined file gets worse, and none is stale", () => { + const drift = Object.entries(ACTIVE_DEBT) + .map(([f, budget]) => ({ file: f, budget, actual: count(read(f), ACCENT_ACTIVE) })) + .filter((r) => r.actual !== r.budget); + expect(drift, "Update ACTIVE_DEBT to match reality — down only.").toEqual([]); + }); + + it("every exception names a file that still needs one", () => { + for (const f of Object.keys(ACTIVE_EXCEPTIONS)) { + expect( + count(read(f), ACCENT_ACTIVE), + `${f} no longer uses the pair — drop it from ACTIVE_EXCEPTIONS`, + ).toBeGreaterThan(0); + } + }); +}); + // ── The published contract stays published ────────────────────────────────── describe("the --cc-* contract matches its documentation", () => {