Skip to content

feat(tasks): filter the project task list by status, search it by PR/issue number, and bulk-edit a selection - #3

Open
sheeerth wants to merge 9 commits into
mainfrom
feat/task-list-filters-bulk-edit
Open

sheeerth wants to merge 9 commits into
mainfrom
feat/task-list-filters-bulk-edit

Conversation

@sheeerth

@sheeerth sheeerth commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Tracking plan: .ai/runs/2026-08-23-task-list-filters-bulk-edit.md
Status: complete

🎯 Goal

  • The per-project Tasks list could only be narrowed by the Active/Archived tabs and a free-text box over title/branch/workflow, and every row had to be archived one at a time. This PR adds a Status facet filter with live per-option counts, makes the search box find a task by its PR or issue number, and adds row selection with a bulk action bar (archive, restore, mark read, mark unread) so a batch of finished tasks is one gesture instead of ten.

What Changed

  • packages/web/src/lib/tasks-table.ts — the pure filter model. referenceNeedle() recognizes #909, a bare 909, pr 909 and issue 42; filterRuns() answers them against taskReferences(), the same list the row's own chip is built from, so search can never surface a number the table would not show. Alongside it: TaskListFilters, filterTaskList() (facet AND search), statusFacetOptions() (options from the statuses present, counts computed the way the global page computes them), plus toggleStatusFilter / activeFilterCount / hasActiveTaskFilters for the Clear affordance. The facet's vocabulary is the status pill's word (deriveAttention().label), not RunRecord.status, so it can offer scheduled and monitoring and can never disagree with the column beside it.
  • packages/web/src/lib/task-selection.ts (new) — the pure selection model: toggleSelected, toggleAllVisible, the tri-state selectionSummary (which intersects the selected ids with the rows currently on screen, so a stale pick is inert), bulkActionTargets (per-action gating on the same rules the single-row actions use), and bulkResultMessage for an honest receipt.
  • packages/web/src/routes/tasks-overview.tsx — the Status pill and Clear in the header, a selection column on the table (tri-state header box) and on the <md cards, the bulk action bar, a filter-aware empty state, and the route-level fan-out: Promise.allSettled over the existing per-run endpoints, one invalidation, and a toast that can say "Archived 1 of 2 tasks — 1 failed: …".

No server, contract or persisted-state change: every action already had a per-run route (POST /runs/:id/archive, /read, /unread).

🧪 Tests

  • npm run typecheck ✅ · npm test6221 passed / 327 files · npm run test:unit ✅ 36 · npm run build ✅ (incl. check:pack) · npm run test:package ✅ 15.
  • New: 40 cases across lib/tasks-table.test.ts (reference needle parsing, reference search, status vocabulary, facet counts, the status-order drift guard), lib/task-selection.test.ts (toggling, stale picks, per-action gating, select-all under a filter, receipt wording) and routes/tasks-overview.test.tsx (filtering through the real facet popover, Clear, empty-state wording, selection mechanics, action gating, checkbox-does-not-navigate on both row and card, and a route-level bulk archive where one run is refused).
  • om-auto-review-pr --autofix returned approve — no blockers, no majors; one minor (the bulk receipt's wording) was fixed in a4ab6fec and the gate re-run green on that commit. The repository's own CI workflow (Unit, build, E2E, and package) passed on this head.
  • Not verified in a live browser: npm run test:e2e could not provision the agent-browser provider here and exited TEST_E2E_STATUS=skipped, which is not a pass. This is a UI change, so that gap is real — see the run-summary comment.
  • Note on the environment: 6 pre-existing server tests fail when TMPDIR points inside the repository (they mkdtemp a "not a git repo" fixture, which then resolves to this repo). The gate above was run with TMPDIR=/tmp; the failures are unrelated to this change and reproduce on a clean checkout.

💥 Breaking Changes

  • None. Cockpit-only; no API, contract or storage surface is touched. The table gains a leading selection column, which shifts nothing semantically (its cells carry data-column-id="select" and are excluded from the column assertions).

📋 Progress

See the Progress section in the tracking plan.

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

📦 npm preview dry run — 0.10.0-pr3.13

⚠️ Dry run — the NPM_TOKEN secret is not configured, so nothing was actually published. See docs/publishing.md for the one-time admin setup.

Try this PR build (exact pinned version — copy-paste as-is):

npx cezar-cli@0.10.0-pr3.13                                # cockpit at http://localhost:4321
npx cezar-cli@0.10.0-pr3.13 run "…"                        # headless run
npx cezar-cli@0.10.0-pr3.13 server-deploy --platform <id>  # roll a server to this exact build

Also tagged: npm install -g cezar-cli@pr-3 (moving tag for this PR).
Packages: cezar-cli@0.10.0-pr3.13@open-mercato/cezar@0.10.0-pr3.13@open-mercato/cezar-api-client@0.10.0-pr3.13.

…us facet

The Tasks table's search box only ever matched title, branch and workflow, so a
task known by its tracker number — the way it is named in a PR body or a standup
— could only be found by remembering its title. `referenceNeedle` recognizes
`open-mercato#909`, a bare `909`, `pr 909` and `issue 42`, and `filterRuns` answers them
against `taskReferences`: the same list the row's own chip is built from, so
search can never surface a number the table would not show.

Alongside it, the pure model the status filter needs: `TaskListFilters`,
`filterTaskList` (facet AND search), `statusFacetOptions` with counts computed
the way the global page computes them, and the toggle/count helpers the Clear
affordance reads. The facet's vocabulary is the status PILL's word
(`deriveAttention().label`), not `RunRecord.status`, so it can offer
`scheduled` and `monitoring` — sub-states the record spells as failed/running —
and can never disagree with the column beside it.
… supports

`lib/task-selection.ts` is the pure half of multi-edit: toggling a row, the
tri-state header checkbox, and which of archive / restore / mark-read /
mark-unread a given selection can actually carry out.

Two rules it exists to enforce. A selection is always read against the rows
CURRENTLY ON SCREEN, so an id that outlived its row — filtered away, archived,
patched out by an SSE tick — is inert rather than able to act. And an action
offers itself only for the rows it would really change, using the SAME gates the
single-row actions use: a `review` run is not archivable by checkbox any more
than it is by the "Archive finished" broom, and only a `canBeUnread` row can be
put back to unread.
…ion of rows

The Tasks table gets the two things it was missing next to the global page: a
Status facet — the same searchable multi-select pill, with per-option row counts
— and a selection column whose bulk bar can archive, restore, mark read and mark
unread any number of rows at once.

The facet reads the status PILL's word, so it can offer `scheduled` and
`monitoring`, and it ANDs with the search box; Clear counts and undoes both. An
empty result now blames the filter when a filter is what emptied it, instead of
reporting an empty archive.

Multi-edit is deliberately conservative. Select-all covers the FILTERED list and
never the rows a filter is hiding; each action is offered only for the rows it
would really change (a `review` run is no more archivable by checkbox than by
the broom); the fan-out is `allSettled` over the per-run endpoints, so one
refused write neither cancels the batch nor gets reported as a success — the
toast says "Archived 1 of 2 tasks — 1 failed: …" and the list is refetched from
the server rather than trusted from the cache.
@sheeerth

sheeerth commented Aug 23, 2026

Copy link
Copy Markdown
Owner Author

🏷️ label rationale — om-auto-create-pr

Label work was attempted and skipped in full: none of the labels this repository's pipeline config (.ai/agentic.config.json) names exist in sheeerth/cezar, and the tracker descriptor's apply_label guard degrades a missing label to a logged skip rather than creating one. What the run would have applied, and why — updated after the automated review pass returned ✅ approve:

  • 🚀 merge-queue — pipeline state: the review is done and returned approve, so the PR is waiting on a human's merge decision rather than on more work. (It would have entered as review when the PR opened, and been moved here by om-auto-review-pr.)
  • 🟢 skip-qa — cockpit-local UI behavior with 40 new unit/route tests covering it, and no server, contract or persisted-state surface touched. Stated plainly, because it qualifies the label: the repo's real-browser smoke suite (npm run test:e2e) could not run here — the agent-browser provider could not be provisioned (no network to the Chrome-for-Testing hosts) and the script exited TEST_E2E_STATUS=skipped, which is not a pass. If you would rather have eyes on it in a browser first, swap this for needs-qa; the change is small and reversible either way.
  • feature — category: new capability on an existing screen, not a fix.
  • 🟡 priority-medium — an ordinary usability improvement; nothing is broken without it.
  • 🟢 risk-low — additive, cockpit-only, revertible by reverting three source files; the one behavioral edge (a bulk action acting on a row that left the view) is guarded and tested.

To make these labels real in this fork, run the tracker descriptor's ensure-label-taxonomy block (.ai/trackers/github.md → Labels → ensure-label-taxonomy) once; every later run will then apply them automatically.

@sheeerth

Copy link
Copy Markdown
Owner Author

🤖 om-auto-review-pr claimed this PR for an automated review pass (autofix mode: this run authored the PR). The in-progress label does not exist in this repository, so the label half of the claim degraded to a logged skip — the assignee and this comment are the lock. It will be released with a completion comment when the pass finishes.

Review finding (minor, user-facing copy): `Marked read 2 tasks.` is a log line,
not a sentence. English puts the object in the middle of "marked … read", so the
verb table now carries a trailing complement and the toast says "Marked 2 tasks
read." — and "Marked 0 of 1 task read — 1 failed" on the partial path.
@sheeerth

Copy link
Copy Markdown
Owner Author

🔍 Code Review: feat(tasks): filter the project task list by status, search it by PR/issue number, and bulk-edit a selection

🎯 Summary

This PR extends the per-project Tasks list (/p/:projectId/) with the three things it was missing next to the global /tasks page: a Status facet filter, search by PR/issue number, and row selection with a bulk action bar (archive, restore, mark read, mark unread). Reviewed scope is five files — packages/web/src/lib/tasks-table.ts, the new packages/web/src/lib/task-selection.ts, packages/web/src/routes/tasks-overview.tsx, and the two test files beside them — plus the run's execution plan under .ai/runs/.

The shape of the change is the right one for this codebase. All of the decision-making is pure and lives in lib/, table-tested, exactly as lib/task-groups.ts and lib/tasks-table.ts already are; the component only paints. It reuses the existing shared primitives rather than inventing parallel ones — the same FacetFilter pill the global page uses, the same toast helper, the same per-run API-client functions — and it adds no server route, contract schema, persisted field or runtime dependency, so none of BACKWARD_COMPATIBILITY.md's protected surfaces is in play.

Two design choices are worth calling out as good, because they are the ones that usually go wrong in a multi-select feature. First, the status facet's vocabulary is the status pill's word (deriveAttention().label), not RunRecord.status — so it can offer scheduled and monitoring, which the record spells as failed/running, and it cannot drift from the column beside it. Second, a selection is never trusted on its own: selectionSummary() intersects the selected ids with the rows currently on screen, so a pick that outlived its row (filtered away, archived, patched out by an SSE tick) is inert rather than able to archive something the user can no longer see. That is the exact failure mode this kind of feature ships with, and it is both guarded and tested (task-selection.test.ts, "ignores an id whose row is no longer on screen"; tasks-overview.test.tsx, "cannot act on a row that has left the view").

One finding was raised and fixed during this pass (see 🔹 Minor). Nothing else blocks.

Verdict

approve — there are no blockers and no majors. Every changed surface is additive, the full validation gate is green on the reviewed head, the behavior changes ship with unit and route-level tests (including the partial-failure path of the bulk fan-out), and the one minor finding was fixed in a4ab6fec before this verdict was recorded. Two nits remain and are the author's call.

Self-approval note: GitHub refuses a formal approving review on one's own PR, so this report is posted as a review comment rather than through review-pr --approve. The verdict is approve for every downstream consumer (labels, merge routing) exactly as if it had been submitted as a review.

🧪 Validation Gate

Command Status Notes
npm run typecheck ✅ PASS contract, api-client, server and web, all clean under strict + noUncheckedIndexedAccess.
npm test ✅ PASS 6221 passed / 327 files, 0 failed, on the final commit a4ab6fec (see the caveat below for the TMPDIR condition).
npm run test:unit ✅ PASS 36 node:test cases, 0 failed.
npm run build ✅ PASS tsc → dist/, vite → packages/cezar/web/dist/, then check:pack ok — 475 files, 85 under web/dist.
npm run test:package ✅ PASS 15 packaged-CLI e2e cases, 0 failed.

One environmental caveat, stated rather than buried. Six server tests (git-worktree, server/git, server/git-changes, server/health-forge, server/projects-api, server/automations-api) fail in this sandbox when TMPDIR points inside the repository, because they mkdtemp a "not a git repo" fixture which then resolves to this repo's root. They are unrelated to this diff — they touch no file it changes — and they pass on the same commit with TMPDIR=/tmp, which is how the gate above was run. Nothing here is a "pre-existing failure" excuse for a real break: the failure mode is the sandbox's temp-dir placement and it reproduces on a clean checkout of main.

npm run test:e2e (the real-browser smoke suite, which is the QA layer for user-facing changes per CODE_REVIEW.md) could not run: the agent-browser provider could not be provisioned here (no network to the GitHub Releases / Chrome-for-Testing hosts) and the script exited TEST_E2E_STATUS=skipped, which is explicitly not a pass. The UI has therefore been verified at the component/route level only, not in a live browser. That is the one gap in this review's evidence and it is why the change is scoped to be reversible.

Findings

🔹 Minor

  • packages/web/src/lib/task-selection.ts:150 (as reviewed; now fixed in a4ab6fec) — the bulk-edit receipt did not read as a sentence. BULK_DONE_VERB mapped read to the phrase Marked read, which produced the toast Marked read 2 tasks. — a log line, not English; the object belongs in the middle of "marked … read". This string is shown to a person after an action they cannot see the result of (the rows have usually left the view), so its readability is the whole point of it existing. Fix applied: the verb table now carries a trailing complement ({ verb: 'Marked', complement: ' read' }), the receipt reads Marked 2 tasks read. and, on the partial path, Marked 0 of 1 task read — 1 failed. The three table-tests in task-selection.test.ts were updated to pin the new wording.

💅 Nit

  • packages/web/src/routes/tasks-overview.tsx:769 and :1088a per-row tick box is described with the header's tri-state vocabulary: state={selected ? 'all' : 'none'}. It is correct and it keeps one component for both boxes, but 'all' for a single row reads oddly at the call site. A checked: boolean | 'mixed' prop would say the same thing more plainly. Not worth the churn on its own; worth folding into the next edit of this component. Author's call.
  • packages/web/src/routes/tasks-overview.tsx:180a bulk action clears the entire selection, including rows it did not touch. Archiving a selection of five where two are review rows leaves those two unticked even though nothing happened to them. The behavior is deliberate and commented (the acted-on rows are usually leaving the view, and a selection pointing at them invites a second click that does nothing), but "keep the untouched rows ticked" is also a defensible reading. Left as-is; flagged so the choice is visible rather than accidental.

💥 Breaking Changes

  • No exported/public symbol removed or renamed without a deprecation path — the only export-surface change is additive: FINISHED_STATUSES in lib/tasks-table.ts goes from module-private to exported so lib/task-selection.ts can gate "archivable" on the same set the "Archive finished" broom uses. Nothing was removed or renamed.
  • No function signature changed in a breaking way — filterRuns(runs, query) keeps its signature and its meaning; it gains an additional haystack (the row's own reference chip), which can only ever match more rows for a needle that looks like #909, never fewer. TasksOverview gains two props (onBulkAction, bulkPending), both optional with defaults, so every existing render site and test compiles unchanged.
  • No required type field removed or narrowed.
  • No HTTP route URL removed or renamed; no method changed — this PR adds no route and calls only the existing POST /runs/:id/archive, /read and /unread through the shared api-client helpers.
  • No field removed or retyped in an existing response shape — packages/contract is untouched.
  • No event or message name renamed or removed; no payload field removed — the SSE/UiEvent surface is untouched.
  • No CLI command or flag renamed or removed; no machine-parsed output format changed.
  • No database table or column renamed or removed — cezar has no database, and no .ai/cezar/ state shape is written or read differently. runs.json field semantics are untouched; the filter and selection state is component-local and never persisted.
  • No config key renamed and no default changed silently — no CEZ_* variable is added, so .env.example needs no update.
  • Where a contract had to change: none had to.

Two additional repo-specific gates, both clean: the graceful-degradation promise is unaffected (this is presentation over data the cockpit already had; with no gh, no remote or CEZ_DRY_RUN=1 the list still renders, filters and selects, and the three endpoints it calls are local), and the server dependency budget is untouched (no new package in any manifest; FacetFilter, cmdk and the Radix popover were already bundled for the global Tasks page and the ⌘K palette, so the cockpit's dependency graph does not grow).

On the UI performance gate: nothing became client-rendered that was not already (the cockpit is a Vite SPA end to end), no route-specific heavy module moved into a global provider, and the new imports are modules the bundle already contained. The one measurable addition is this feature's own code; check:pack reports the same 85 files under web/dist as before.

🧪 Test Coverage

What is covered, and how. 40 new cases across three files, all of which fail without the change:

  • lib/tasks-table.test.tsreferenceNeedle as a 13-row table (909, #909, pr 909, PR#909, pull 909, issue 42, issue#42, and the negatives 909 tokens, zod, v4, #, empty); reference search proving a kind keyword narrows (pr 909 finds the PR-creating run and not the issue run) and that a bare number still matches the title that contains it; taskStatusValue pinning the pill vocabulary including the two sub-states; filterTaskList for facet-only, multi-status OR, and facet-AND-search; statusFacetOptions for present-statuses-only options, priority ordering, and the counting rule that a facet's own ticks must not shrink its own counts; plus a drift guard that walks every label deriveAttention can currently emit and asserts the ladder has a place for each — a new attention label lands as a failing test rather than silently sorting to the bottom.
  • lib/task-selection.test.ts — toggling without mutation, the tri-state header, the stale-id rule, visible-order preservation, per-action gating (a review row is not archivable; a cancelled or archived row can never go unread), select-all confined to the visible list, and the receipt's three branches.
  • routes/tasks-overview.test.tsx — the filter driven through the real Radix+cmdk popover rather than a stubbed callback (narrow, un-narrow, OR two statuses, option counts and order, facet AND search, Clear counting both narrowings and undoing both), the empty state distinguishing "no tasks" from "your filter hid them" via a props change that plays the SSE case, reference search end-to-end, the full selection surface (bar appears/disappears, header tri-state, select-all under a filter, per-action counts and disabled reasons, pending disables), the checkbox-does-not-navigate rule on both the table row and the mobile card, and a route-level bulk archive in which one run is refused — asserting both POSTs were attempted, the toast names the server's reason, and the list is refetched rather than trusted.

Gaps, honestly. (1) No live-browser verification — npm run test:e2e skipped for want of the browser provider, so the visual result of the new column and bar is unverified outside jsdom; if packages/web/e2e/ gains a tasks-list spec later, "tick two rows and archive them" is the case to add. (2) The mobile card's selection is asserted at the DOM level but never at a real viewport, for the same reason. (3) bulkPending is covered for the disabled state, but not for a genuine in-flight overlap (a second batch started while the first is running) — the guard makes that unreachable through the UI, which is why it was not forced in a test.

@sheeerth

Copy link
Copy Markdown
Owner Author

🤖 om-auto-review-pr completed: APPROVED. Lock released.

The pass ran in autofix mode (this run authored the PR). It found no blockers and no majors, fixed the one minor finding it did raise — the bulk-edit receipt's wording — in a4ab6fec, and re-ran the full validation gate on that commit: typecheck ✅, npm test ✅ 6221/6221, npm run test:unit ✅ 36, npm run build ✅ (incl. check:pack), npm run test:package ✅ 15. The full report is the review comment above.

CI, disclosed rather than waited on: “Unit, build, E2E, and package” passed (3m22s) on this head. “Publish npm snapshot” is still pending — it is a publishing job, not a correctness signal, and this agent will not block on it. No in-progress/ci-monitoring label was set or cleared, because neither label exists in this repository (see the label-rationale comment).

@sheeerth

Copy link
Copy Markdown
Owner Author

🤖 om-auto-create-pr — run summary

Tracking plan: .ai/runs/2026-08-23-task-list-filters-bulk-edit.md
Branch: feat/task-list-filters-bulk-edit
Engine: om-auto-create-pr (steps: 12, --loop: no)
Final status: complete

📋 Summary of changes

  • Phase 1 — the pure filter model (packages/web/src/lib/tasks-table.ts). The search box learned tracker numbers: referenceNeedle() recognizes #909, a bare 909, pr 909, pull 909 and issue 42, and filterRuns() answers them against taskReferences() — the same list the row's own chip is built from, so search can never surface a number the table would refuse to show (that rule is what keeps Fix: issue-subject tasks show no issue link and their footer links an unrelated PR open-mercato/cezar#526's cross-repository mis-links out of it). Alongside it, the status-facet model: TaskListFilters, filterTaskList() (facet AND search), statusFacetOptions() with counts computed against the list as the other narrowing leaves it, and toggleStatusFilter / activeFilterCount / hasActiveTaskFilters.
  • Phase 2 — the pure selection model (packages/web/src/lib/task-selection.ts, new). Toggling, the tri-state header box, selectionSummary() (which intersects the ticked ids with the rows currently on screen), bulkActionTargets() (per-action gating on the same rules the single-row actions use) and bulkResultMessage().
  • Phases 3 & 4 — the surface (packages/web/src/routes/tasks-overview.tsx). A Status pill (the shared FacetFilter, same as the global page) and a counted Clear in the header; a selection column on the desktop table and on the <md cards; a bulk action bar offering Archive, Restore, Mark read and Mark unread, each counted by the rows it would really change and disabled with a reason when it applies to none; a filter-aware empty state; and the route-level fan-out over the existing per-run endpoints.
  • Areas touched at a glance: three source files and two test files under packages/web/src, plus the run's execution plan. No server, contract, persisted-state or dependency change.

External references honored

None — no --skill-url was passed.

🧪 Verification phases completed

  • Targeted validation (per phase): npm run typecheck:web plus the affected vitest files after each phase (lib/tasks-table.test.ts, lib/task-selection.test.ts, routes/tasks-overview.test.tsx), and the whole packages/web suite at the end of Phase 4 — which is how the design-guardian rule that reads a three-digit #nnn placeholder as a hex colour was caught and fixed before the gate.
  • Full validation gate (re-run in full on the final commit a4ab6fec): npm run typecheck ✅ · npm test6221 passed / 327 files · npm run test:unit ✅ 36 · npm run build ✅ (incl. check:pack ok — 475 files) · npm run test:package ✅ 15.
  • Sandbox caveat, disclosed not buried: six server tests (git-worktree, server/git, server/git-changes, server/health-forge, server/projects-api, server/automations-api) fail here when TMPDIR points inside the repository — they mkdtemp a "not a git repo" fixture that then resolves to this repo. They touch nothing this PR changes and pass on the same commit with TMPDIR=/tmp, which is how the gate above was run.
  • om-auto-review-pr review/autofix pass:approve. No blockers, no majors; no compatibility, security, scoping or breaking-change finding (the only export-surface change is additive — FINISHED_STATUSES becoming exported so the bulk bar and the "Archive finished" broom share one definition of finished). One minor was raised and fixed in a4ab6fec (the bulk receipt read Marked read 2 tasks., a log line rather than a sentence; it now reads Marked 2 tasks read.), and two nits were left documented as the author's call. Follow-up commit range: a4ab6fec only. The full report is the # 🔍 Code Review comment above.
  • CI: the repository's own workflow — Unit, build, E2E, and packagepassed on this head in 3m22s. Publish npm snapshot is still pending; it is a publishing job, not a correctness gate, and no agent is blocking on it.
  • 📸 UI evidence: none, and that is a real gap. This is a UI change, and the repo's real-browser layer (npm run test:e2e) could not run in this environment: the agent-browser provider could not be provisioned (no network reachable to the GitHub Releases / Chrome-for-Testing hosts) and the script exited TEST_E2E_STATUS=skipped, which is explicitly not a pass. The UI is therefore verified at the component and route level only — including through the real Radix + cmdk popover in jsdom — and not visually. npm run test:e2e on a networked machine, or a two-minute click-through, is what would close it.

🔍 How to verify

  • Manual smoke test: CEZ_DRY_RUN=1 npm run dev, open a project's Tasks page, and: (1) click Status, tick done, confirm the table narrows and the option counts match; (2) type a PR number from the Ref column into the search box — with and without a leading # — and confirm the row is found, then try issue <n> and confirm it does not match a PR of the same number; (3) press Clear and confirm it reports the right count and undoes both narrowings; (4) tick two finished rows, confirm the bar says 2 selected and Archive is enabled with a count while Restore is disabled with a reason, press Archive, and confirm the rows move to the Archived tab and the toast says Archived 2 tasks.; (5) on the Archived tab, select and Restore them back; (6) tick a row and click elsewhere on it — the tick must not open the task, and the row must still open when clicked anywhere else; (7) narrow the window below md and repeat (4) on the cards.
  • Areas to spot-check in the diff: lib/tasks-table.ts referenceNeedle (the regex is the one place a bad pattern would quietly widen search), lib/task-selection.ts selectionSummary / bulkActionTargets (the two rules that keep a bulk action honest), and the TasksOverviewRoute bulk mutation in routes/tasks-overview.tsx (the allSettled fan-out and its receipt).
  • Commands the reviewer can re-run: TMPDIR=/tmp npm run typecheck && TMPDIR=/tmp npm test && TMPDIR=/tmp npm run test:unit && TMPDIR=/tmp npm run build && TMPDIR=/tmp npm run test:package, or just the new suites: npm test -- packages/web/src/lib/tasks-table.test.ts packages/web/src/lib/task-selection.test.ts packages/web/src/routes/tasks-overview.test.tsx.
  • Rollback plan: git revert the range 97a5ecb6..a4ab6fec (or just the three source files). There is no migration, no persisted state and no feature flag to unwind — the filters and the selection live in component state and vanish with the component.

⚠️ What can go wrong (risk analysis)

  • Most likely regression: the table's new leading selection column shifting the column alignment of the queued-row colSpan={2} CPU/Mem cell. The existing alignment tests cover it (headers, normal rows and queued rows are asserted to stay aligned with folded columns), and the column assertions were narrowed to td:not([data-column-id="select"]) so they still describe the data columns rather than silently absorbing a new one.
  • Second-order effects: filterRuns() is shared, so widening it could have changed another surface — it has exactly one other consumer, filterTaskList() in the same module, so the blast radius is this table. The compare-variants strip is now narrowed by the status facet as well as by the search box; that is the same behavior the search box already had, but it does mean filtering to one status can hide a Compare offer for a group whose members are split across statuses.
  • Security-sensitive surfaces: N/A. No auth, permission, scoping or secret surface is touched; the cockpit talks to the same loopback endpoints it already used, through the shared api-client helpers.
  • Breaking-change impact: No contract surface changes. The one export-visibility change is additive, filterRuns's signature is unchanged (it can only match more rows, never fewer), and both new TasksOverview props are optional with defaults, so every existing render site compiles untouched.
  • Residual risk accepted: (1) no live-browser verification, for the provisioning reason above — accepted because the change is presentational, reversible, and covered at the route level; (2) the bulk fan-out is N client-side requests rather than one batch endpoint — accepted because every action already had a per-run route, the server is on loopback and setArchived/setRead are synchronous in-memory mutations with no await between read and write, so a concurrent burst cannot interleave; (3) a bulk action clears the whole selection, including rows it did not touch — deliberate, documented in the code, and flagged as a nit rather than silently chosen.

@sheeerth

Copy link
Copy Markdown
Owner Author

ℹ️ The same branch is now also open upstream as open-mercato#920 (open-mercato#920), which is where the review for merging should happen — this PR stays as the mirror carrying the run's artifacts (execution plan, code-review report, run summary). The diffs are identical; close whichever one you do not want to keep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants