Skip to content

Commit 54015e4

Browse files
authored
feat(desktop): select several tasks in the Session rail (#4365)
* feat(desktop): select several tasks in the Session rail The rail could act on one task at a time. Clearing out a run of finished ones meant opening the ⋯ menu, confirming, and repeating — once per task. ⌘/Ctrl-click marks a row instead of opening it, Shift-click marks a run, and a bar in the rail's sticky chrome offers archive and delete over the marked set. Escape clears it; Delete asks for the marked set rather than the focused row, because deleting one of several rows a user marked is the shape of an unrecoverable surprise. Nothing renders until something is marked, so the rail at rest is unchanged. A range never leaves its group. A project group's collapsed state lives inside Astryx's `SideNavItem` and is not readable from the rail, so a range across groups could quietly include rows nobody can see. Within one group the question does not arise: both endpoints had to be clicked, and a row that can be clicked is on screen. The selection is a THIRD rail context, for the reason the chrome is a second one. It changes on every modified click while the list does not, and folding it into `SessionRailData` would give that value a new identity per click — the ~1,000-fiber render the split exists to prevent (#4109). Rows read it directly rather than receiving it through the memoized row renderer, so a selection change re-renders the rows on screen and nothing above them. Both sweeps reuse the machinery the archived-task purge already had rather than copying its accounting. `purgeSessions` becomes `sweepSessions`, parameterized by how each id decides the archived premise: Settings asserts it for every target, and the rail reads it off the task exactly as single-row delete does, because the rail lists unarchived tasks and asserting it there would refuse them all. Bulk archive accounts by count and first failure instead, since archiving has no third disposition to report. Neither raises a toast per task: one sweep is one message. Generated-by: Claude Opus 5 via Claude Code * feat(desktop): make the rail's multi-select an explicit mode Review of the first design found it undiscoverable and, at the rail's real width, broken. ⌘/Shift-click is invisible to anyone who has not been told about it, and the bar put a count and three text buttons on one line: at 244px the buttons measured 228px and the count was squeezed to 12px, where it wrapped one character per line. Selection is now a mode. It is entered from the row's ⋯ menu — where a person already looks for what a row can do — and while it is on, every row carries a checkbox and a master box sits above them showing "已选 1 / 3". The master box ticks and unticks every listed row and reads `indeterminate` in between, which is the usual state during a selection and the one a checked/unchecked pair cannot express. "All" means the rows the rail is listing, not every task in the catalog. A box that silently reached past what sits under it would name a number the user never agreed to. Unticking every row is select-none, not leave: a mode that ended itself on the last untick would take the checkboxes away mid-gesture, and one mis-click would cost the user the way back in. Pruning to nothing keeps the mode for the same reason — the rows went away because the catalog changed, not because the user was finished. Escape and the 取消 button leave. The bar heads the LIST rather than the chrome, and moved out of SideNav's `topContent` to say so. Up there it landed above the one hairline SideNav draws under the whole sticky region — grouped with 按时间 / 按项目 and cut off from the rows it governs, with its own rule making a second line 9px from the first. It is `position: sticky` inside the scroller instead, so it still does not scroll away, over the rail's own ground colour. Two rows, not one, because one does not fit. The commands are `secondary` rather than `ghost`: a bare label beside a checkbox and a count does not read as something to press. Generated-by: Claude Opus 5 via Claude Code * fix(desktop): keep the rail's bulk wording inside its budgeted module CI's renderer architecture check refused the selection hook: features/session-navigation/controller/use-session-selection.ts: feature imports unbudgeted renderer legacy code: ../../../locales/shell-copy.js A feature may not reach into renderer legacy copy without a line in `renderer-architecture.json`, and `session-row-actions.ts` has one only because it predates the rule. Adding a second entry would grow the ledger the check exists to shrink, so the wording moved instead of the budget. `archiveSelected` and `deleteSelected` now live in `session-row-actions`, which already holds this feature's copy: each confirms, runs its sweep, and reports the outcome. The bare sweeps below them stay silent, because Settings' purge still phrases its own confirm — the caller that genuinely owns different wording. The rail's phrasing was never that caller; it is the feature's own, and it belongs where the feature keeps its strings. The hook keeps what is actually its business: the marked set, freezing it at the press, the busy flag, and clearing it afterwards while the mode stays on. It no longer needs a locale or a toast API at all. `npm run check:architecture` now passes locally, along with check:app-shell-hooks and check:asf-headers — gates this branch had never run before CI ran them for it. Generated-by: Claude Opus 5 via Claude Code * docs(desktop): record the selection bar in the Astryx surface inventory CI's Astryx surface gate failed on the new file: astryx surface inventory is stale - on disk but not in .paths (1): packages/ui/src/session-selection-bar.tsx The inventory tracks every renderer file that renders Astryx components and what each one reaches for, so a new component file has to be recorded. It is generated, not hand-written: this is the output of `npm run astryx:surface-inventory:write`, unedited. The new file lands as `aligned` (Button, CheckboxInput), and `session-history-list.tsx` gains `CheckboxInput` in its own row. No blocker or reimplementation entries appear. I had run the architecture gate this time but not this one. Rather than find the next gate the same way, I enumerated every `npm run` CI invokes and ran the ones this branch can affect: astryx:surface-inventory and its tests, astryx:theme, check:renderer-architecture (base mode), check:app-shell-hooks, check:asf-headers, format:check, lint, typecheck, and the @maka/ui and @maka/desktop suites. All pass, and astryx:theme leaves the tree clean. Generated-by: Claude Opus 5 via Claude Code * chore(desktop): stop exporting an unused selection type CI's Knip gate found dead surface: Unused exported types (1) SessionSelectionGesture features/session-navigation/testing.ts I exported it beside the selection model for tests to use, and then the tests never did — they pass object literals, which TypeScript checks structurally against `applySessionSelectionGesture`'s own parameter. The type still exists where it is used; only the testing re-export is gone. Third gate this branch has failed one at a time, so this time I read the CI workflow and ran every step it invokes that this branch can affect: knip for both workspaces, script-entrypoints, the app-shell hook gate and its own test, astryx:theme --check, the app-icon drift tests, the Astryx surface inventory and its tests, check:renderer-architecture in base mode, check:asf-headers, lint, format:check, the full build, repo-wide typecheck, and the @maka/ui and @maka/desktop suites. All pass, and `npm run build` leaves the tree clean. Generated-by: Claude Opus 5 via Claude Code * fix(desktop): keep one session switch off every rail row CI's e2e render contract caught a real regression, and it is the very contract this branch claimed to respect: switching sessions does not rewrite the whole Session rail rail rows touched by one session switch, of 12 Expected: <= 2 Received: 12 Two independent causes, which is why fixing either alone left the count at 12 and why I measured them one at a time before believing either. **A per-render array prop.** Rows were handed their group's session ids so a Shift-click range could know where it may reach. Computed in the render — even once per group rather than once per row — that array has a new identity every time, and `SessionNavRow`'s `memo` compares props, not the identity of the factory that produced them. Memoizing it does not help: every candidate key is derived from `rail.sessions`, whose identity moves on a session switch, so the memo rebuilds exactly when it must not. The prop is gone, and with it the modifier-click gestures it existed for. The rail's selection is checkboxes and a master box now; ⌘-click was the earlier design's affordance and nothing in the current one needs a range. `SessionSelectionGesture`, the range and anchor in the model, and `sessionSelectionGestureMode` go with it. **A context every row subscribes to.** A context consumer re-renders when its value changes and `memo` cannot stop it, so a row reading the whole selection re-rendered whenever `listedSessionIds` moved — again, on every session switch. `SessionRailRowSelection` is now a separate, narrower context holding only what a row needs, memoized on the selection alone; the bar keeps the wide one. Verified by running the spec locally, which this branch had never done: failing at 12 before, `1 passed` after, and passing on `main` throughout. Also rebased onto current main, where `@maka/ui`'s test script is now `test:dist`. Two `app-update-attestation` cases fail on this machine with and without these changes — the packaged Electron runtime's TUF/Sigstore checks — so they are environmental, not this branch's. Generated-by: Claude Opus 5 via Claude Code * fix(desktop): answer both review findings on the rail's bulk sweeps Two P2s from review, both reproduced before fixing and both covered by a test that fails when the fix is reverted. **A settled sweep cleared whatever was marked, not what it asked about.** `Done` stays enabled during a sweep because leaving asks nothing of the Host — so a person can leave the selection, re-enter from another row's menu and mark B while A's request is still out. The `finally` replaced the whole set with an empty one, discarding B to answer A's completion. It now removes exactly the submitted ids and leaves anything else alone; the mode still stays on. A deferred-command regression drives that sequence, and reverting the fix fails it. **Bulk delete bypassed the linked-subtask contract.** The Host archives a deleted parent's ordinary subagent tasks rather than deleting them, and `main` now exposes `previewRemoval` plus `archivedSubtaskCount` so a destructive confirm can warn about the survivors and the toast can report how many moved. Single-row delete uses both; this new path used neither, so deleting a selected parent made its subtasks reappear under Archived with no warning and no explanation. `deleteSelected` now asks the Host for a preview per selected task before confirming — the renderer's projection cannot answer it — and a single preview failure makes the warning uncertain rather than silently under-reporting a destructive set. The sweep already accumulated `archivedSubtasks` once rebased; the toast reports that executed total, not the estimate. Four cases cover the warning, the uncertain fallback, the silent case, and a declined confirm. Rebased onto `9249bf3f`, which is where those Host affordances arrived. The conflicts were the two halves of one line — main's destructured `archivedSubtaskCount` and this branch's per-id archived premise — and two additive copy blocks. Generated-by: Claude Opus 5 via Claude Code * chore(docs): refresh the Astryx surface inventory total CI failed on the surface gate with only the `.md` stale: - docs/astryx-surface-file-inventory.md does not match generator output The diff is one line — `232 files … aligned 231` becomes `233 … 232`. A file that renders Astryx arrived on `main` while this branch was out, and `.paths` came with it; the totals line in the table did not. Worth naming because it is a shape this branch has now hit twice: the gate passed locally and failed in CI, because CI checks the branch MERGED with current `main` and I was four commits behind. Running the gates on an un-rebased branch proves less than it looks like it does. Rebased onto `8fa1e894` and regenerated with `astryx:surface-inventory:write`. Generated-by: Claude Opus 5 via Claude Code
1 parent 3db83c2 commit 54015e4

20 files changed

Lines changed: 2000 additions & 40 deletions
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import assert from 'node:assert/strict';
21+
import { describe, test } from 'node:test';
22+
import {
23+
EMPTY_SESSION_SELECTION,
24+
enterSessionSelection,
25+
exitSessionSelection,
26+
pruneSessionSelection,
27+
sessionSelectionMasterState,
28+
setAllSessionsSelected,
29+
type SessionSelection,
30+
} from '../../renderer/features/session-navigation/testing.js';
31+
32+
const GROUP = ['a', 'b', 'c', 'd', 'e'];
33+
34+
function ids(selection: SessionSelection): string[] {
35+
return [...selection.selectedIds].sort();
36+
}
37+
38+
/** What a row's checkbox does, as the hook applies it. */
39+
function mark(selection: SessionSelection, sessionId: string): SessionSelection {
40+
return {
41+
active: true,
42+
selectedIds: new Set([...selection.selectedIds, sessionId]),
43+
};
44+
}
45+
46+
describe('selection mode', () => {
47+
test('entering marks nothing on its own', () => {
48+
const entered = enterSessionSelection(EMPTY_SESSION_SELECTION);
49+
assert.equal(entered.active, true);
50+
assert.deepEqual(ids(entered), []);
51+
});
52+
53+
test('leaving drops the mode and the marks together', () => {
54+
assert.equal(exitSessionSelection().active, false);
55+
assert.deepEqual(ids(exitSessionSelection()), []);
56+
});
57+
58+
test('unticking every row is select-none, not leave', () => {
59+
// A mode that ended itself on the last untick would take the checkboxes
60+
// away mid-gesture, and one mis-click would cost the user the way back.
61+
const all = setAllSessionsSelected(EMPTY_SESSION_SELECTION, GROUP, true);
62+
const none = setAllSessionsSelected(all, GROUP, false);
63+
assert.deepEqual(ids(none), []);
64+
assert.equal(none.active, true);
65+
});
66+
67+
test('an emptied selection keeps the mode it was in', () => {
68+
// It used to settle on the shared EMPTY value, which also carries
69+
// `active: false` — so a catalog change that pruned the last row would have
70+
// taken the checkboxes away while the user was still selecting.
71+
const pruned = pruneSessionSelection(mark(EMPTY_SESSION_SELECTION, 'a'), []);
72+
assert.deepEqual(ids(pruned), []);
73+
assert.equal(pruned.active, true);
74+
});
75+
});
76+
77+
describe('the master box', () => {
78+
test('marks exactly the rows the rail is listing', () => {
79+
// Not every task in the catalog: the box sits above these rows, and a
80+
// selection that reached past them would name a number nobody agreed to.
81+
assert.deepEqual(ids(setAllSessionsSelected(EMPTY_SESSION_SELECTION, ['a', 'b'], true)), [
82+
'a',
83+
'b',
84+
]);
85+
});
86+
87+
test('reads unchecked, indeterminate, then checked', () => {
88+
assert.equal(sessionSelectionMasterState(EMPTY_SESSION_SELECTION, GROUP), false);
89+
assert.equal(sessionSelectionMasterState(mark(EMPTY_SESSION_SELECTION, 'b'), GROUP), 'indeterminate');
90+
assert.equal(
91+
sessionSelectionMasterState(setAllSessionsSelected(EMPTY_SESSION_SELECTION, GROUP, true), GROUP),
92+
true,
93+
);
94+
});
95+
96+
test('an empty list is unchecked, never checked', () => {
97+
// `every` over an empty array is vacuously true, which would tick the box
98+
// above no rows at all.
99+
assert.equal(sessionSelectionMasterState(EMPTY_SESSION_SELECTION, []), false);
100+
});
101+
102+
test('a mark outside the listed rows does not make it checked', () => {
103+
assert.equal(sessionSelectionMasterState(mark(EMPTY_SESSION_SELECTION, 'zzz'), GROUP), 'indeterminate');
104+
});
105+
});
106+
107+
describe('pruneSessionSelection', () => {
108+
test('drops ids the catalog no longer lists', () => {
109+
const selection = setAllSessionsSelected(EMPTY_SESSION_SELECTION, ['a', 'b'], true);
110+
assert.deepEqual(ids(pruneSessionSelection(selection, ['a'])), ['a']);
111+
});
112+
113+
test('returns the same value when nothing was dropped', () => {
114+
// Identity matters here: this runs on every catalog refresh, and a new Set
115+
// each time would re-render every row of the rail.
116+
const selection = mark(EMPTY_SESSION_SELECTION, 'a');
117+
assert.equal(pruneSessionSelection(selection, ['a', 'b']), selection);
118+
});
119+
});

0 commit comments

Comments
 (0)