refactor(desktop): rebuild the task rail on one shape and one vocabulary - #3033
Conversation
cb95932 to
ae6bfde
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughProblem solved
Source of truthThe PR extends existing paths. It does not create a parallel task system.
Scope and complexityThe new Settings page owns source selection, archived filtering, pagination, loading, retry, stale-request protection, and uncertain outcomes. These controls are required by the new import location. The rail now uses an unfiltered Host-level import coalescing is required to prevent duplicate concurrent imports after the dialog lifecycle was removed. The in-flight map removes completed and failed operations. The solution is the smallest coherent change shown by the supplied summary. It removes obsolete UI and state paths while retaining the existing catalog, import API, status model, and regression coverage. Deletions and simplifications
Further deletion is not evident from the supplied summary without weakening behavior or regression coverage. Risks
ValidationThe PR reports passing:
Stories and fixtures cover import success, failure, unavailable sources, archived tasks, task rows, status fixtures, narrow rails, and Settings integration. The current checkout has no diff or check artifacts. Required-check status is therefore unverified. Review-relevant risks
WalkthroughThe change adds task import settings, removes filtered session navigation, updates task-row status and timestamp presentation, and replaces session-oriented terminology across desktop, CLI, core, runtime, UI, tests, stories, and validation scripts. ChangesTask workspace experience
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to Moving import into a page can leave the task list stale after an uncertain completion and may allow the same source to be imported twice; completion also still opens the imported task rather than preserving the intended catalog-refresh flow. These behaviors should be fixed or explicitly accepted before merge. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/session-history-list.tsx (1)
82-83: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unused
copybinding fromSessionHistoryList. It has no references within the component.Source: Path instructions
🧹 Nitpick comments (3)
packages/ui/src/session-history-list.tsx (1)
697-783: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the shared vocabulary for the streaming and unread dots too.
staleresolves its colour throughdotForStatus('attention'), whilestreamingandhasUnreadhardcode'accent'. The stated purpose of this function is that one place turns a status word into a colour. Two of four signals bypass it, so the same drift this change removed can return.apps/desktop/src/renderer/styles/sidebar.css (1)
327-336: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOn touch devices the timestamp is never visible.
Inside
@media (hover: none)the action menu takes the slot and.maka-session-row-timestaysvisibility: hiddenpermanently. A sighted touch user therefore loses the row timestamp completely, not just while interacting. Assistive tech still gets the absolute timestamp from the row description, so this is a visible-only gap. Confirm that this is the intended trade for touch, or reserve room for both.apps/desktop/src/main/chat-readiness.ts (1)
137-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the fake-backend error copy.
The same literal appears in
messageForReasonandassertSessionCanSend. Define it once and reuse it in both paths. This prevents the two errors from drifting.Proposed simplification
+const FAKE_BACKEND_MESSAGE = + '当前任务来自旧的本地模拟连接,不能直接发送。请到 设置 · 模型 添加真实模型后新建任务。'; + function messageForReason( ... - return '当前任务来自旧的本地模拟连接,不能直接发送。请到 设置 · 模型 添加真实模型后新建任务。'; + return FAKE_BACKEND_MESSAGE; ... - '当前任务来自旧的本地模拟连接,不能直接发送。请到 设置 · 模型 添加真实模型后新建任务。', + FAKE_BACKEND_MESSAGE,As per path instructions, this is a concrete case where the changed code can be simplified.
Also applies to: 152-152
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c786d305-b1d2-44b2-8db1-a6691f07572f
📒 Files selected for processing (94)
apps/desktop/e2e/composer-skill-invocation.spec.tsapps/desktop/e2e/session-workbar.spec.tsapps/desktop/e2e/settings.spec.tsapps/desktop/e2e/sidebar-project-row.spec.tsapps/desktop/e2e/streaming-remount.spec.tsapps/desktop/src/main/__tests__/app-shell-session-purge.test.tsapps/desktop/src/main/__tests__/app-shell-session-settings-actions.test.tsapps/desktop/src/main/__tests__/bot-incoming-project-cwd.test.tsapps/desktop/src/main/__tests__/external-session-import-lifecycle.test.tsapps/desktop/src/main/__tests__/runtime-host-external-sessions-ipc-main.test.tsapps/desktop/src/main/__tests__/thread-search.test.tsapps/desktop/src/main/bot-incoming-main.tsapps/desktop/src/main/capability-snapshot.tsapps/desktop/src/main/chat-readiness.tsapps/desktop/src/main/computer-use/status-item.tsapps/desktop/src/main/e2e-fixture/scenarios-sessions.tsapps/desktop/src/main/e2e-fixture/scenarios-settings.tsapps/desktop/src/main/notifications-policy.tsapps/desktop/src/main/runtime-host-external-sessions-ipc-main.tsapps/desktop/src/main/search/thread-search.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/agent-graph-panel.tsxapps/desktop/src/renderer/app-shell-chat-actions.tsapps/desktop/src/renderer/app-shell-e2e-fixture.tsapps/desktop/src/renderer/app-shell-overlays.tsxapps/desktop/src/renderer/app-shell.tsxapps/desktop/src/renderer/command-palette-commands.tsapps/desktop/src/renderer/external-session-import-dialog.tsxapps/desktop/src/renderer/external-session-import-lifecycle.tsapps/desktop/src/renderer/locales/conversation-copy.tsapps/desktop/src/renderer/locales/external-session-import-copy.tsapps/desktop/src/renderer/locales/permission-center-copy.tsapps/desktop/src/renderer/locales/settings-daily-review-copy.tsapps/desktop/src/renderer/locales/settings-data-copy.tsapps/desktop/src/renderer/locales/settings-memory-copy.tsapps/desktop/src/renderer/locales/settings-navigation-copy.tsapps/desktop/src/renderer/locales/settings-preferences-copy.tsapps/desktop/src/renderer/locales/settings-projects-copy.tsapps/desktop/src/renderer/locales/settings-provider-copy.tsapps/desktop/src/renderer/locales/settings-shared-copy.tsapps/desktop/src/renderer/locales/settings-subagents-copy.tsapps/desktop/src/renderer/locales/settings-usage-copy.tsapps/desktop/src/renderer/locales/settings-web-search-copy.tsapps/desktop/src/renderer/locales/shell-copy.tsapps/desktop/src/renderer/locales/shell-remaining-copy.tsapps/desktop/src/renderer/nav-selection.tsapps/desktop/src/renderer/session-nav-filter.tsapps/desktop/src/renderer/session-status-presentation.tsapps/desktop/src/renderer/settings/import-tasks-settings-page.tsxapps/desktop/src/renderer/settings/settings-modal.tsxapps/desktop/src/renderer/settings/settings-nav.tsapps/desktop/src/renderer/settings/settings-surface.tsxapps/desktop/src/renderer/styles.cssapps/desktop/src/renderer/styles/external-session-import.cssapps/desktop/src/renderer/styles/sidebar.cssapps/desktop/stories/app-shell.stories.tsxapps/desktop/stories/command-search.stories.tsxapps/desktop/stories/session-workbar.stories.tsxapps/desktop/stories/settings/settings-pages.stories.tsxapps/desktop/stories/subagent-sessions.stories.tsxdocs/astryx-surface-file-inventory.mddocs/astryx-surface-file-inventory.pathspackages/cli/src/__tests__/pi-transcript.test.tspackages/cli/src/__tests__/pi-tui-runner.test.tspackages/cli/src/__tests__/run-session-selection.test.tspackages/cli/src/pi-transcript.tspackages/cli/src/pi-tui-runner.tspackages/cli/src/run-command-core.tspackages/core/src/bot-events.tspackages/core/src/connection-error-copy.tspackages/core/src/session.tspackages/core/src/settings.tspackages/runtime-host/src/protocol/session-continuity.tspackages/runtime-host/src/server/scheduled-task-coordinator.tspackages/runtime/src/__tests__/session-manager.test.tspackages/runtime/src/session-manager.tspackages/ui/src/components.tsxpackages/ui/src/conversation-copy.tspackages/ui/src/daily-review-copy.tspackages/ui/src/nav-selection.tspackages/ui/src/relative-time.tsxpackages/ui/src/runtime-resume-copy.tspackages/ui/src/scheduled-task-copy.tspackages/ui/src/session-history-list.tsxpackages/ui/src/session-list-panel.tsxpackages/ui/src/session-rename-dialog.tsxpackages/ui/src/session-sidebar-nav.tsxpackages/ui/src/session-status-presentation.tspackages/ui/src/shared-ui-copy.tspackages/ui/src/shell-controls-copy.tspackages/ui/src/tool-activity/copy.tspackages/ui/stories/model-picker.stories.tsxpackages/ui/stories/session-list-panel.stories.tsxscripts/check-astryx-alignment.mjs
💤 Files with no reviewable changes (6)
- packages/ui/src/components.tsx
- apps/desktop/src/renderer/external-session-import-lifecycle.ts
- apps/desktop/src/renderer/styles.css
- apps/desktop/src/renderer/styles/external-session-import.css
- apps/desktop/src/renderer/external-session-import-dialog.tsx
- apps/desktop/src/main/tests/external-session-import-lifecycle.test.ts
|
Covering the four items that were not inline threads — one outside-diff comment and three nitpicks. Three accepted, one declined.
Reviewed with CodeRabbit (automated review) and Claude Code (Anthropic). I traced each claim to current source before deciding, and verified the accepted fixes with |
…running Moving 导入任务 from a dialog into a Settings page removed the only thing stopping one intent from becoming two tasks, and this branch is where that happened. `ExternalSessionImportLifecycle` refused to close the dialog while an import was active; retiring it was described here as dropping a guard against nothing. It was not. The page it was replaced by is one the user may leave mid-import -- the import continues in Desktop Main by design -- and the page's `importingId` dies with it, so coming back and pressing 导入 again issues a second request against a source that is still importing. Nothing downstream deduplicates: `treats repeats as independent copies` is a pinned behaviour, so the second request lands a second task the user now has to tell apart. The guard belongs to the Host, not to the page that asked. Import is a Host operation and the Host is the only party that knows one is running; a client knows about its own requests, which is why a second window or the CLI would have reproduced this with the page's state intact. Concurrent repeats collapse onto the first attempt's promise and both callers get its outcome, success or failure, because it is one operation. Sequential repeats are untouched -- the entry is gone by the time the first settles -- so importing the same conversation again on purpose still makes an independent copy, which the existing test continues to pin. Reported by @M4n5ter in review of apache#3033. Refs apache#2984 Generated-by: Claude Code
M4n5ter
left a comment
There was a problem hiding this comment.
English
Approved.
I re-reviewed the updates through e812c2391 against the PR intent. Moving concurrent import coalescing into the Host closes the reported duplicate-import path at the correct ownership boundary, while preserving deliberate repeat imports after completion. The regression coverage pins both behaviors directly.
I also checked the removal of the collapsed rail Tasks row: the unconditional titlebar expand control, retained activeId selection, and session-row navigation provide a complete return path, with E2E coverage. All current checks are green.
简体中文
批准。
我结合本 PR 的真实意图重新审查了截至 e812c2391 的更新。将并发导入合并下沉到 Host,能够在正确的职责边界上修复此前的重复导入问题,同时保留导入完成后主动再次导入的行为;新增回归测试也直接固定了这两项契约。
我还核验了折叠侧栏移除“任务”入口后的返回路径:标题栏展开按钮始终可用,activeId 会保留当前任务标记,点击任务行会切回会话视图,并已有 E2E 覆盖。当前所有检查均已通过。
The dot the rail draws for a session passed through two lossy hops and three disagreeing sources, so it could not say what it meant. Collapse the mapping. `SessionStatus -> SessionStatusTone -> StatusDotVariant` becomes one table from status to Astryx's variant. Two of the seven tones had no distinct outcome at the end of that chain, and the collapse is what let `waiting_for_user` and `blocked` share `warning`: blocked now maps to `error`, so "cannot proceed until you fix a connection" and "holding a question for you" stop rendering alike. Read the running authority. `runningTurnIds` is the runtime's projection of the runs it holds; `session.ts` states why a persisted `status` cannot serve that purpose, and `settledSessionTransientIds` already reads it first over the same list. The row read neither -- a renderer-local streaming set, then the stored `status` -- so a task running under a bot channel or a second window read as idle. The streaming set stays below it, for the gap between this renderer sending a turn and the host reporting it back. Delete what nothing wrote. `review` and `done` were never written by anything, in any version, so no stored record can carry them and no reader of them was reachable; they leave `SESSION_STATUSES`, the wire enum, and the copy tables. `SessionLifecycleStatus` now aliases `SessionStatus` instead of restating it -- three hand-written copies of one enum is how those two values survived in two of them after the last writer went away. Delete the tautological filter. `SessionFilter` had one value left: `archived` moved to Settings in apache#2985 and `flagged` was never selected, which left a control whose answer is always the same answer. The 「会话」 row that selected it goes with it, as does the branch in `sessionMatchesNavSelection`. A stored filter is dropped rather than validated, which is the migration. Also gone: `SessionStatusPresentation.interactive`, which had no reader, and a doc comment naming a `SessionStatusIcon` and a chat-header badge as the tone matrix's consumers -- neither exists. `status: 'archived'` still duplicates `isArchived`. Consolidating those two rewrites stored rows, so it is apache#2984's PR 3, not this one. Refs apache#2984 Generated-by: Claude Code
Every task row in the rail now carries exactly two signal slots. Slot 1 is a single status dot, reserved as an 8px gutter even when there is nothing to show so titles stay aligned down the list. Slot 2 holds a compact timestamp at rest and swaps to the ⋯ menu on hover or keyboard focus, so the menu is no longer permanently mounted next to every title. The dot's meaning is resolved once, in priority order: live run (accent, pulsing) › persisted status › unread (accent, inactive rows only). The live run reads `runningTurnIds` first, matching `settledSessionTransientIds` over the same session list — a persisted `running` status can be left behind by a crash, so it must not outrank the authoritative projection. The stale pill is gone: it duplicated the timestamp it sat next to. The worktree mark falls back to the row tooltip rather than competing for a slot. `RelativeTime` gains `variant="compact"` instead of a second component; a 260px rail row cannot fit the medium-date-plus-time past-horizon fallback. Generated-by: Claude Code
The rail's top region now ends with the grouping switch, and the history below it starts directly at its first group. The list heading is gone. "会话" labelled a list that is the only thing under it, inside a landmark that already names the panel — and it existed mainly to give the grouping switch somewhere to sit. The switch moves into the sticky top region instead, where it stays reachable: hung off a heading it scrolled away with the very list it switches. It is also full width with text labels now. Two icon-only segments asked the rail to teach a clock and a folder, and the rail has nowhere to teach them; 按时间 / 按项目 is the entire vocabulary and fits inside the minimum 180px width with room to spare. Collapsed at 48px it does not render at all — no segment label fits, and the list it governs is not rendered either. The group headers lose their ➕. It was the 新任务 row's handler under a second name, three rows below the row itself, and it needed that second name precisely because two controls cannot share one accessible name. Generated-by: Claude Code
导入 was a rail row that opened a modal over the conversation. Import is not navigation: it is a rare setup errand, it needs a source, an archived filter and a paged directory of someone else's conversations to work through, and none of that belongs in a 260px column of the tasks you are working on. It becomes 设置 › 活动 › 导入任务, next to 已归档任务 — both are about the task catalog rather than the task in front of you. As a page the flow loses its selection state. A dialog needed one because its single primary action sat in a footer far from the rows; a page puts 导入 on the row itself, the way 已归档任务 puts 恢复 on its rows. That also retires `ExternalSessionImportLifecycle`, whose whole job was refusing to close a dialog mid-import — there is no dialog left to hold open, and an import that outlives the page is guarded by the mounted ref instead, so it can still finish but can no longer steer a shell the user has navigated away from. The source switch now appears only when there is more than one source. Codex is the only adapter that ships, so in practice every user was being shown a segmented control with a single segment — a control nobody can operate. With one source the section names it instead. Deliberately not carried over: keeping an imported task in sync with its source. The importer is a one-shot conversion and nothing behind it watches the source, so that control would be a promise no coordinator can keep. Generated-by: Claude Code
The product called the same object 会话, 对话, session and conversation depending on which file you landed in. Code, protocol and storage keep `session` — that is a durable contract with a wire format behind it — but every string a user reads now says 任务 / task. The rule is what the noun REFERS TO, not the word: - The Maka entity a user opens, renames, archives and searches → 任务 / task. - Conversing as a behavior stays 对话 / conversation: "与 Maka 对话" over a bot, "可用于对话的模型", "对话记录" as a message log. - Another agent's stored conversations stay 对话 / conversation. 导入任务 lists Codex's conversations and produces Maka tasks; collapsing both onto one word would erase exactly the distinction that page is about. - 侧边对话 stays: it is a chat beside a task, not a task in the rail. Two collisions had to be resolved rather than renamed. The per-task ledger was 会话任务, which would have become 任务任务 — its items are 待办 / to-dos, which is what they always were. And the automations row read "定时任务,N 个 未完成任务", two different meanings of 任务 in one label; the count now says "N 条进行中". The keyboard sheet loses its ←/→ row. It documented cycling between 会话/已标记/已归档, a filter this PR removed along with `SessionFilter`. Generated-by: Claude Code
`LongTitlesAndNarrow` framed the panel at 176px but let SideNav keep its own 260px `resizable` width, so the story clipped a wide rail instead of showing a narrow one — the timestamps it exists to check were outside the frame. It now drives the rail's own width, at 180px, which is the panel's `minWidth` and therefore the narrowest state a user can reach. Generated-by: Claude Code
bb39b33 deleted `SessionFilter` and took the 「会话」 row with it, on the reading that the row only selected a filter whose answer was always the same answer. Half right: its `isSelected` read the dead filter, but its `onClick` selected the SECTION, and it was the only control that did. Without it the rail has three sections and two rows. Collapsed at 48px — the default state (`readSessionListCollapsed`) — the list is not rendered at all, so 扩展 and 定时任务 became one-way doors: the only way back to a running task was 新任务, which answers "show me my tasks" by starting another one. `streaming-remount.spec.ts` walks exactly that path and had been rewritten to click a task row that a collapsed rail does not have. Restore the row as 任务, selected on `section === 'sessions'` and selecting the section with no filter. The filter deletion stands. Refs apache#2984 Generated-by: Claude Code
Deleting `SessionStatusTone` was right; replacing it with a private `SessionStatus -> StatusDotVariant` table was not. `status-vocabulary.ts` already owns "the one place a status word becomes a colour", so the private table made a second authority, and the two disagreed: a task waiting on a permission prompt drew `error` in the rail while the permission centre drew `attention` for the identical condition. Map the session enum to `StatusSemantic` and let `dotForStatus` pick the colour. `blocked` and `waiting_for_user` are both `attention`, which is what that semantic is defined as -- both are waiting on a person. Giving `blocked` `error` to tell them apart used colour for a distinction colour cannot carry; their labels and `describeBlockedReason` do that. Restore `review` and `done`. They have no writer in current source, but `SESSION_STATUSES` is read back out of storage, and narrowing it is a data migration rather than a cleanup: `resolveLegacyStatus` in the JSONL importer (removed in apache#2656) passed both values through into real SQLite stores verbatim, and `normalizeSessionHeader` throws for the WHOLE header on an unrecognised status, so one stored row carrying `done` fails an entire catalog page. The migration is its own change. `archived` and `aborted` get their dots back. They were `muted` before, `muted` resolved to a real `neutral` dot, and dropping them to `undefined` was a behaviour change I described as a consequence of collapsing the layer. Refs apache#2984 Generated-by: Claude Code
The rename was applied by rule and reviewed by hand, which is the process that produces both halves of this: strings that kept the old noun, and strings that took the new one where the old was right. Same-surface contradictions, which are what a user actually notices: - The to-do panel's error banner said 任务载入失败 next to a button labelled 重新载入待办 -- one `Banner`, two vocabularies. - `chat-readiness` said 当前任务 and then 新建会话 in one sentence. - The bot's help text offered to 清空当前对话开新会话 while the reply to that same command already said 任务已重置. - 定时任务 described itself as 安排定时任务,并回顾本机任务, and its delivery option read Agent 任务执行 -- two meanings of 任务 in one form. - `settlementFailed` called the side chat a 任务 and then a 侧边对话; `forkSetupFailed` called it 追问任务, a third name for one object. Both are 侧边对话 now, matching the en twins. zh/en divergence, where the catalogs share keys but stopped sharing meaning: Daily Review counted 任务 in zh and conversations in en (and its en empty state said conversations on one branch, tasks on the other); the terminal panel said "task" and then "the session's terminal". Missed outright: `tool-activity/copy.ts` (由源会话管理 on every terminal result chip), `connection-error-copy.ts`, and the CLI's own prose (`allow for session`, `Give this session full access`, `--continue`). Kept deliberately: 和 Maka 对话 as a behaviour, Codex's stored 对话 on the import page, 侧边对话, 对话框 for dialog, and `<session-id>` as a CLI contract. The palette's `nav:sessions` keywords get 会话 and 对话 back -- collapsing both onto 任务 left a duplicate entry and dropped the words a long-time user would still type. Refs apache#2984 Generated-by: Claude Code
…lary
`streaming` and `hasUnread` still wrote `'accent'` by hand while the status
and stale signals resolved through `dotForStatus`. Half a function deciding
its own colours is the drift this change set out to close -- the literal is
what let the rail and Settings disagree about the same fact before. Both are
`active`: streaming is the system working on it, and unread text is
"something happened here" rather than a question waiting on the user, which
is the line the two semantics exist to draw. `dotForStatus('active')` is
`'accent'`, so nothing moves on screen.
Also drops the `copy` binding `SessionHistoryList` no longer reads. It fed
the list's own aria-label, and the outer SideNav took that name over when the
rail was rebuilt.
Refs apache#2984
Generated-by: Claude Code
Desktop's copy claimed the status-to-dot mapping was "re-exported below" and that `describeBlockedReason` was defined there. Both moved to `@maka/ui` in this branch and neither is in the file; what is left is Desktop-only judgment -- which blocked reasons are worth acting on, and what to offer after a turn fails -- so the header says that instead. The contract the old header carried is real and had nowhere to live after the move: a UI label must never show a raw `SessionBlockedReason`, and a new reason has to extend the core enum and the copy matrix together or it reads as `unknown`. It now sits on `describeBlockedReason` in `@maka/ui`, where the matrix it constrains is. Refs apache#2984 Generated-by: Claude Code
…urces Both fixtures modelled states their real source cannot produce, which makes them evidence for nothing. The purge tests built a restored task by flipping `isArchived` alone, leaving `status: 'archived'` behind. `SessionStore.unarchive` writes both fields together, so no stored row looks like that. The sweep only reads `isArchived` today, so no assertion changes -- the point is that the next assertion written against these rows would be checking a state that cannot happen. A `restored` helper names the pair once. The 导入任务 story's `list` ignored `includeArchived` and `cursor`: it rendered the archived conversation while the filter was off and handed 加载更多 the same first page forever, under a comment claiming it demonstrated both controls. It now filters and pages, with a fourth conversation so the default view is a short first page rather than the whole list. Refs apache#2984 Generated-by: Claude Code
`fake_backend` reaches the user down two paths -- the reason table, and the header check in `assertSessionCanSend`, which never gets far enough to look a connection up. They said the same sentence in two copies, so renaming 会话 to 任务 had to be done twice. One `FAKE_BACKEND_MESSAGE` now. The English workspace help came out of the rename as "Any task can switch next to the input box", which makes the task the actor and never names what it switches. It says the project. Refs apache#2984 Generated-by: Claude Code
The Markdown half still listed `external-session-import-dialog.tsx` and `external-session-import.css` after this branch deleted them, because the two halves were edited by hand and `check-astryx-surface-inventory.mjs` only verifies that on-disk files appear in the inventory -- never that inventory rows still exist on disk. It is also not wired into CI, so the drift was invisible from both directions. Running the generator also picks up three files that arrived from main and were never listed: `runtime-host-profiles-section.tsx`, `runtime-host-ssh-terminal-dialog.tsx`, and `settings/runtime-host.css`. They are unrelated to this branch, but a generated file regenerated in halves is how it drifted in the first place. Refs apache#2984 Generated-by: Claude Code
Two gaps, both found by shooting the story for a before/after comparison. `review` and `done` were dropped from `statusSessions` when this branch briefly deleted them from `SESSION_STATUSES`. The enum values and their labels came back; the fixture did not, so the story that covers every status covered six of eight -- and the two missing ones are exactly the two whose colours this change decided on purpose, `attention` and `success`. `StoryFrame` also defaulted to 240px while `SessionListPanel`'s rail defaults to 260, so every story that did not pass a width was clipping the rail by 20px. That lands on the trailing slot, which is where this change puts the timestamp -- the stories could not show whether it fits. Stories that want a narrow rail still pass the width to both, which is what the note on `panelProps` is about. Refs apache#2984 Generated-by: Claude Code
Expanded, the row selected a section whose content was already on screen directly under it -- the same redundancy as the 会话 list heading this change deleted one row down, and clicking any task row does the same selection. It was kept for the collapsed rail, where the list is not rendered, on the argument that 扩展 and 定时任务 were otherwise one-way doors. That was wrong. Collapsed, the rail cannot switch tasks at all, so getting back to any task already means widening it; the titlebar's 展开侧边栏 toggle does that unconditionally (app-shell-chrome-actions.tsx renders `ChromeColumnToggle` with no guard), and `activeId` survives a section change, so the widened rail comes back with the task you left already marked. The row bought one click in a state the user is leaving regardless, and charged a permanent slot for it. `sessions` now has no control of its own on the rail. It is where you are unless you went somewhere, which is why the other two sections light up and this one has nothing to light. `streaming-remount` returns the way the product now offers -- widen, then click the row carrying `aria-current="page"` -- which is a better assertion than the old one anyway: it checks that the task survives the trip, not just that a button exists. Refs apache#2984 Generated-by: Claude Code
…running Moving 导入任务 from a dialog into a Settings page removed the only thing stopping one intent from becoming two tasks, and this branch is where that happened. `ExternalSessionImportLifecycle` refused to close the dialog while an import was active; retiring it was described here as dropping a guard against nothing. It was not. The page it was replaced by is one the user may leave mid-import -- the import continues in Desktop Main by design -- and the page's `importingId` dies with it, so coming back and pressing 导入 again issues a second request against a source that is still importing. Nothing downstream deduplicates: `treats repeats as independent copies` is a pinned behaviour, so the second request lands a second task the user now has to tell apart. The guard belongs to the Host, not to the page that asked. Import is a Host operation and the Host is the only party that knows one is running; a client knows about its own requests, which is why a second window or the CLI would have reproduced this with the page's state intact. Concurrent repeats collapse onto the first attempt's promise and both callers get its outcome, success or failure, because it is one operation. Sequential repeats are untouched -- the entry is gone by the time the first settles -- so importing the same conversation again on purpose still makes an independent copy, which the existing test continues to pin. Reported by @M4n5ter in review of apache#3033. Refs apache#2984 Generated-by: Claude Code
e812c23 to
f2dfdda
Compare
导入任务 shipped in #3033 as a relocation, not a design, and never got a pass over how it reads. Four things were wrong; this fixes the ones that survive the page's own constraints. - A row whose import Desktop Main could not confirm stayed disabled for the rest of the page's life with no explanation on the row — the reason lived only in a banner elsewhere. That row now carries a warning Badge, and its dead button carries the reason as a tooltip. Hovering a button that will not move is the one moment a user asks that question. Astryx routes tooltips through aria-describedby and keeps a disabled+tooltip button focusable via aria-disabled, so a keyboard user reaches the reason and activation stays blocked. - The 「包含已归档的对话」 filter was disabled during an import. Re-filtering the catalog starts no second import; freezing it only froze the page around one row. The rows stay disabled, because a success closes Settings and navigates — this page can only consume one terminal result. - 加载更多 read as a caption under the list. It is now a full-width secondary button. No "N remaining" is offered: the adapter returns a cursor, not a total. - The no-adapter empty state explained that Maka does not modify Codex's files, which is not the question an empty page raises. It now says no supported agent's session directory was found, and keeps the read-only promise that earns the permission to look. It names no agent: which ones are supported is the adapter registry's answer, not a copy string's. Not fixed, deliberately. 「再次导入同一个对话会创建一个独立的任务」 stays a standing line above the list. It only matters to someone asking "have I already imported this", and a successful import calls closeSettings(), so the page can never know that. Until an imported session persists its external origin, that sentence cannot become per-row information, and no other position beats the one it already has. Generated-by: Claude Code
The row import button ran `importConversation` through Astryx's `clickAction`, which Astryx executes inside a React 19 async transition. React holds a transition's state updates until the action settles, so `setImportingId` committed only after the import was already over. For the whole duration of an import the page therefore behaved as if none were running: the row kept reading 导入 instead of 正在导入…, the other rows stayed clickable, and both catalog controls stayed live -- including the archived filter, frozen for that purpose since #3033, and the source switch frozen in 552156f. A second import could be started, or the catalog replaced out from under the in-flight one. `clickAction` earns the clicked button its own pending state and nothing more. This lock is a page fact, so the page owns it: plain `onClick` runs outside the transition and `importingId` commits before the next event can be dispatched. `加载更多` moves back for the same reason -- inside the transition `loadingMore` could neither disable it nor say 正在加载…. Verified in Storybook against a never-settling import: with `onClick` the source switch, the archived filter and every other row are inert until it settles, and a second click on any of them is dropped. Generated-by: Claude Code
… catalog `importingId` was a bare source id — a pointer into a list the page lets the user replace. Everything it fed said "which import is running", and all of that lived on the row, so the row had to be kept alive: the archived filter was frozen for it in #3033, the source switch in 552156f, 重试 and 加载更多 in 861fdcc. Four controls, one rule, added one review round at a time. This PR already made the same fix for the unconfirmed banner, whose ids had the same problem. Making it once more removes the reason for all four freezes: `activeImport` carries the adapter, the id and the name, so the page can say 正在导入「X」 above the list, where no catalog change can take it away, and name why every 导入 is disabled. The catalog is then free to move underneath an import. `catalogFrozen` and its four readers are gone, and so is the id collision the source-switch freeze was closing — the row predicate compares adapter and id together. `ImportAttempt` is one record type for both slots, since "which conversation, in which source" is the same question whether the import is running or came back unconfirmed. Verified in Storybook: with an import that never settles, filtering its row away leaves the banner naming it and every remaining row disabled; with the fixture's failing import, one name then two in the unconfirmed banner, both rows disabled, and the lock released in between. Generated-by: Claude Code
…track of 设置 · 活动 · 导入任务 shipped in #3033 as a relocation and never got a pass over how it reads. The import behaviour itself is unchanged. The page kept source ids where it needed records. A bare id is a pointer into a list the user is free to replace -- filter it, switch source, retry a failed page -- so everything built on one either went mute or had to be protected by freezing the list. Both places that held ids now hold `ImportAttempt` (`{ adapterId, sourceSessionId, name }`). - The unconfirmed banner names the conversations to go look for, and stays true after their rows are gone. - The in-flight import is named above the list rather than only on its row, so no catalog control has to be frozen to keep it identifiable. The freezes the archived filter (#3033), the source switch and 重试/加载更多 carried are deleted, along with the cross-adapter id collision they were closing. - The page-level lock engages at all: Astryx runs `clickAction` inside a React 19 async transition, which holds state updates until the action settles, so the state committed only after the import was over. Plain `onClick` runs outside the transition. - `加载更多` is a full-width `secondary` button rather than a centred ghost label that read as a caption under the list. - The no-adapter empty state says what to do instead of repeating its title. Reviewed by M4n5ter; the in-flight-import refactor was raised by CodeRabbit finding the same freeze rule at a third and fourth control. Refs #2984
Summary
The task rail (sidebar) rebuild from #2984, PR 2 of 3. #2985 has merged; this targets
main.The rail carried four kinds of chrome that each answered a question nobody was asking. A ➕ on every group header was the 新任务 row's handler under a second name, three rows below the row itself. A list heading labelled a list that is the only thing under it, and existed mainly to give the grouping switch somewhere to sit. A 会话 nav row selected a section whose content was already on screen directly beneath it. And a 导入 row opened a modal over the conversation for a rare setup errand that needs a source, a filter and a paged directory. All four are gone.
What is left is one shape. The top region is 新任务 plus the two sections that are somewhere else — 扩展 and 定时任务 — and a full-width grouping switch, sticky above the history it governs. Tasks have no row of their own, because the list is the row: clicking any task selects that section, and the section is where you are unless you went somewhere. Every task row carries two visible slots: a status dot, and a timestamp that becomes the ⋯ menu on hover or keyboard focus.
Import becomes 设置 › 活动 › 导入任务, beside 已归档任务. As a page it needs no selection state: 导入 sits on the row, the way 恢复 sits on the archived page's rows. That retires
ExternalSessionImportLifecycle, which held a dialog open until its import finished — and whose real job, missed here until review, was keeping one intent from becoming two tasks; the Host now holds that invariant instead (see below). The source switch now appears only when there is more than one source — Codex is the only adapter that ships, so every user was being shown a segmented control with one segment.The terminology pass: the same object was 会话, 对话, session or conversation depending on the file. Code, protocol and storage keep
session; every string a user reads now says 任务 / task. The rule is what the noun refers to, so conversing as a behavior stays 对话, another agent's stored conversations stay 对话 (导入任务 lists Codex's conversations and produces Maka tasks), 侧边对话 stays, and<session-id>stays a CLI contract. Two collisions needed resolving rather than renaming: the per-task ledger was 会话任务, which would have become 任务任务 — its items are 待办 / to-dos; and the automations row read "定时任务,N 个未完成任务", which now says "N 条进行中".Refs #2984
Screens
The rail, both sides at the same 260px width, from story
Product/Sidebar Session List › ConversationStates— the story that also covers all eight statuses.What moved, top to bottom: the 会话 nav row gone, leaving 新任务 and the two sections that are elsewhere; the list heading and its ➕ replaced by the full-width 按时间 / 按项目 switch; the status dot moved to the leading edge and the timestamp took the trailing slot it shares with the ⋯ menu; the 已过期 pill gone, with the row's dimming carrying stale visually and the accessible name carrying it for everyone else. The colour change on 待审核的文件 diff is the point of the semantic pass —
reviewisattention, not a second shade of "live".What an adversarial review changed
The first seven commits made five claims about this codebase that are false, and a
⋯-menu claim that is false in the other direction. They are listed here because the fixes only make sense against them, and because four of the six were caught by review rather than by me.onClickwas also the only control selecting{section:'sessions'}. Deleting it on the filter argument was deleting the right row for the wrong reason — see the note below, which deletes it again on the real onearchived/aborteddraw no dotinfo → accenthad none;muted → neutraldrew a real dot. Dropping them toundefinedwas a behaviour change, described as a consequence of collapsing a layerrunningTurnIds, the running authoritySessionSummaryreaching Desktop carries that field — the identifier appears nowhere inpackages/runtime-host/srcorapps/desktop/src/mainoutside a test fixturereview/done, so no stored record can carry themresolveLegacyStatusin the JSONL importer (removed in #2656) passed both through into real SQLite stores verbatimopacity: 0is what keeps it in the tab order and the accessibility treeThe corrections, in the order they were made:
Status goes through the shared semantic layer. Deleting
SessionStatusTonewas right; replacing it with a privateSessionStatus → StatusDotVarianttable was not, becausestatus-vocabulary.tsalready owns "the one place a status word becomes a colour". The private table made a second authority and the two disagreed: a task waiting on a permission prompt drewerrorin the rail while the permission centre drewattentionfor the identical condition. The enum now maps toStatusSemanticanddotForStatuspicks the colour.blockedandwaiting_for_userare bothattention— both are waiting on a person, which is what that semantic is defined as; using colour to tell them apart was using colour for a distinction it cannot carry.reviewanddoneare restored.SESSION_STATUSESis read back out of storage andnormalizeSessionHeaderthrows for the whole header on an unrecognised status, so one stored row carryingdonefails an entire catalog page. Narrowing it is a data migration with its own review, not a cleanup — filed as #3058.The row keeps its facts. Collapsing to a single signal inside the resolver made the visual element the only carrier of the fact, so removing the trailing
Badgeremoved "stale" itself:staleTitleandstaleAriaLabelwere left with no reader, and the survivingopacitycue is cancelled on the selected row by design and announces nothing anyway. Worktree went the same way — anaria-labeled icon inside the button became atitleon a non-interactive ancestor, which contributes to neither the button's name nor its description. The row now resolves a list of signals, drawssignals[0], and puts the rest plus worktree and the absolute timestamp in one visually hidden span inside the button. Two slots, same pixels. Signals also stop eating each other:abortedkeeps its own dot instead of falling through to the unread branch, where an aborted task with unread text drew the same accent dot as a running one.Import. The success path already emitted
sessions:changed(since #2507), so a task imported while the user leaves Settings does reach the rail — the comment claiming otherwise is corrected. The real gap wascommit_outcome_unknown, which returned without emitting: the only trace was the page's own banner, and leaving the page unmounts it, which is exactly when someone comes back and imports the same conversation again.Gates. Deleting the import dialog left exact-string readers behind:
scripts/check-astryx-alignment.mjsexited 1 with ENOENT, and the surface inventory still listed both deleted files with no row for the page replacing them. The dialog's Item-row guards are dropped rather than migrated — they checked that a selected row stayed keyboard-reachable, and the new page has no selection.Terminology. Same-surface contradictions the rule pass left behind: the to-do panel's error banner said 任务载入失败 next to a 重新载入待办 button;
chat-readinesssaid 当前任务 then 新建会话 in one sentence; the bot's help offered to 清空当前对话开新会话 while the reply to that command said 任务已重置. Plus zh/en divergence in Daily Review and the terminal panel,tool-activity/copy.tsmissed outright, and the CLI's own prose.A later CodeRabbit round
Twelve findings, each traced to source before deciding. Eight accepted: two doc claims in
session-status-presentation.tsthat survived their own subject moving to@maka/ui; a deadcopybinding;streamingandhasUnreadstill writing'accent'by hand after the rest of the row moved ontodotForStatus; a restored-task fixture flippingisArchivedwithoutstatus, which no store writes; a 导入任务 story fixture that ignoredincludeArchivedandcursorunder a comment claiming it demonstrated both; a duplicatedfake_backendsentence the rename had to edit twice; and English project help that made the task the actor.Four declined, with the code in each thread. The largest was
commit_outcome_unknown"not refreshing the catalog" — it does, from Desktop Main, and the suggested remedy would have threaded a second notification path through three renderer layers past the IPC event that already carries it. One would have reverted a fix: the in-session to-do ledger's error copy must stay 待办, because 任务 now names the session, and calling both 任务 is the collision this PR resolves.Shooting the before/after above then found two gaps of its own, in the story that exists to show every status:
reviewanddonewere never restored to the fixture when the enum came back, andStoryFramewas 20px narrower than the rail it framed — clipping exactly the trailing slot this change introduces.The 任务 row, deleted twice
Worth stating plainly because the middle of this branch has it both ways. It was deleted first on a false premise, restored when an E2E test caught that premise, and is deleted again here on the argument that actually holds.
The restore rested on "collapsed, 扩展 and 定时任务 are one-way doors". That is wrong. Collapsed, the rail cannot switch tasks at all — the list is not rendered — so reaching any task already means widening the rail. The titlebar's 展开侧边栏 toggle does that with no guard (
ChromeColumnToggleinapp-shell-chrome-actions.tsx), it sits next to search and is always visible, andactiveIdis untouched by a section change, so the widened rail comes back with the task you left already carryingaria-current="page". The row bought one click in a state the user is leaving regardless.Expanded, it was redundant on its face: it selected a section whose content was rendered directly underneath it, and clicking any task row performs the same selection. That is the identical redundancy as the 会话 list heading deleted one row down — keeping one and deleting the other was the inconsistency.
streaming-remountnow returns the way the product offers: widen, then click the row carryingaria-current="page". That is a stronger assertion than the one it replaces, because it checks that the task survives the trip rather than that a button exists.One import, two tasks
@M4n5ter found the one real regression in this branch, and it is the cost of moving import from a dialog to a page.
importingIdlives in the page. The import deliberately outlives the page. So leaving 导入任务 mid-import and coming back re-enabled 导入 for a source that was still importing, and nothing downstream deduplicates —treats repeats as independent copiespins that as behaviour. One intent, two tasks to tell apart.This is exactly what
ExternalSessionImportLifecyclewas preventing, which the Summary above described as guarding nothing. It is corrected there too.Fixed at the Host rather than in the page, because the page cannot hold this invariant: a second window or the CLI reproduces the double-import with the page's state fully intact.
HostExternalSessionCoordinator.importSessionnow collapses concurrent requests for the sameadapterId + sourceSessionIdonto the first attempt's promise, and both callers get its outcome — success or failure — because it is one operation. Sequential repeats are untouched, so importing the same conversation again on purpose still makes an independent copy.One gap left open and named in the thread: a remounted page still labels a still-importing source 导入 rather than 导入中. Pressing it now yields the right task instead of a duplicate, so this is a label being briefly wrong rather than state being wrong; closing it needs a new "what is in flight" query the page does not have.
Verification
npm run format:check,npm run lint— clean@maka/desktop(preload + main + renderer + storybook),@maka/runtime-host,maka-agent; builds for@maka/core,@maka/ui,@maka/storage,@maka/mcp,@maka/runtime,@maka/code-modenode scripts/check-astryx-alignment.mjs— ok (exit 1 before this branch's fix)scripts/generate-astryx-surface-inventory.mjsrather than hand-edited, which also picked up three runtime-host files that arrived frommainand were never listed. The check that should have caught either drift is one-directional and not wired into CI — filed as Astryx surface inventory check is one-directional and not wired into CI #3064@maka/runtime-hostexternal-session-coordinator (9, including the new coalescing test — verified failing without the fix),@maka/uisession-history-row-actions (2), CLI pi-transcript + pi-tui-runner + run-command (162), plus the suites listed in earlier roundsapps/desktop/e2e/streaming-remount.spec.ts— both tests, repeatedly@maka/ui: a stale+worktree row now exposes 任务已过期 on the dot and "Git 工作树 · " in the button's accessible name; an aborted row with unread text exposes 已中止 on the dot and 未读消息 in the name; no duplicate 任务列表 landmarkopacity: 0at rest and1on hover and:focus-within, timestampvisibility: hiddenin the swap with its width reserved, grouping switch spanning the 新任务 row's box (8→252 at the default width), both labels fitting at the 180px minimumnpm --workspace @maka/desktop run smoke:storybook— 130 stories rendermainand from this branch, shot headless at the same width; the pipeline is a scratch script, not committedReview focus
The terminology pass touches ~50 files across two rounds. It was applied by rule, then reviewed phrase by phrase — the interesting cases are the deliberate exceptions under Summary.
AI-assisted (Claude Code), including an adversarial review round by Codex and three independent subagents (~50 findings) and a later CodeRabbit round (12). Findings were adjudicated against source rather than applied: seven pushed back across the two rounds, each answered in its thread with the code that contradicts it. CodeRabbit subsequently withdrew two of those on the evidence.
The finding that mattered most came from a human. @M4n5ter's double-import is the only defect in this branch that reaches a user's data, and none of the automated rounds surfaced it — worth stating, given how much of the above is machine-assisted. Everything here is verified by the commands listed; the reviewer should confirm the exceptions read correctly in context.
Follow-ups filed rather than folded in: #3058 (status enum migration), #3059 (Host live-run projection), #3060 (
viewModenot persisted), #3061 (streaming-remountflake), #3064 (surface-inventory check).Checklist
Does this PR entail a change in behavior?