diff --git a/.dump/app/benchmarks/2026-09-23-sdk-0-3-pins.md b/.dump/app/benchmarks/2026-09-23-sdk-0-3-pins.md new file mode 100644 index 00000000..a7cd93c3 --- /dev/null +++ b/.dump/app/benchmarks/2026-09-23-sdk-0-3-pins.md @@ -0,0 +1,94 @@ +# Step 12 benchmark record: SDK 0.3.270, CLI 2.1.270, Codex 0.154.0 + +Date: 2026-09-23. Track: app (`arena/01a0cbec-mauscode`, issue #14). Status: +install-time weight measured in this sandbox; bundle size per target and turn +latency NOT measured, so the benchmark gate stays CLOSED for those two and CI +owns them. + +## Environment + +Sandbox: 2 CPUs, 3 GB RAM, no display, egress intercepted for the hosts that +serve the Claude and Codex binaries. `.dump/app/roadmap/12-sdk-and-pins.md` §11 +asks for `NODE_OPTIONS=--max-old-space-size=4096 bun run build` and +`bun run package:linux`. Neither can run here: the heap flag alone exceeds +available memory. Both download scripts were attempted and both failed on egress +rather than on the pins, `claude:download` losing the TLS socket to the object +store and `codex:download` failing leaf-certificate verification. Everything +below is either a measurement taken here with the command that produced it, or an +explicit NOT RUN. + +## NOT RUN + +| Measurement | Why | Who runs it | +| --- | --- | --- | +| Bundle size per target, with and without the bump | needs `bun run build` at a 4 GB heap | CI | +| Bundle size per target, with and without `@dnd-kit` | same, and step 30 owns the renderer budget | CI, then step 30 | +| Packaged artifact size, with and without the `build.files` exclusion | needs `bun run package:linux` | CI | +| Turn latency for one scripted prompt, before and after | needs a packaged app and provider credentials | a dev machine | +| `bun run claude:download` and `codex:download` integrity runs | sandbox egress is intercepted | CI | + +## Measured here + +Install weight, `du -sh` on this checkout after all four dependency commits: + +| Path | Size | +| --- | --- | +| `node_modules` | 1.9 GB | +| `node_modules/@anthropic-ai` | 440 MB | +| `node_modules/@anthropic-ai/claude-agent-sdk-linux-x64` | 214 MB | +| `node_modules/@anthropic-ai/claude-agent-sdk-linux-x64-musl` | 208 MB | +| `node_modules/@anthropic-ai/sdk` | 14 MB | +| `node_modules/@anthropic-ai/claude-agent-sdk` | 5.0 MB | +| `node_modules/@img` (sharp's platform binaries) | 37 MB | +| `node_modules/@dnd-kit` (three packages) | 2.1 MB | +| `node_modules/sharp` | 1.3 MB | + +The two SDK platform packages are the whole story: 422 MB of a 1.9 GB install, +and they did not exist at `0.2.45`, which spawned bundled JavaScript instead of a +native binary (SDK changelog `0.2.113`). + +`manifest.json` at `0.3.270`, the size each packaged target would have carried +without the `build.files` exclusion: + +| Platform | Bytes | MB | +| --- | --- | --- | +| darwin-arm64 | 207500480 | 197.9 | +| darwin-x64 | 216316928 | 206.3 | +| linux-arm64 | 223862184 | 213.5 | +| linux-x64 | 223981040 | 213.6 | +| linux-arm64-musl | 216545032 | 206.5 | +| linux-x64-musl | 217894976 | 207.8 | +| win32-x64 | 227051168 | 216.5 | +| win32-arm64 | 218256032 | 208.1 | + +Lockfile rows: 2888 at `33475d8`, 2960 after all four dependency commits, with 16 +package rows added and 16 removed by the pin bump itself. + +Integrity, the one check this sandbox can make: `sha256sum` of +`node_modules/@anthropic-ai/claude-agent-sdk-linux-x64/claude` is +`3a624a5a7cd79bbad4d32bd7db36f1197ecf458bc5bf1e2aed81834a01ad3ef0`, which equals +the `linux-x64` checksum in the SDK's `manifest.json`, size `223981040`. The SDK +pin and the CLI pin are therefore the same artifact, not two independent guesses. + +Gate wall clocks on the same 2 CPU sandbox, as a regression proxy rather than a +product number. Measured at the head that added this file, before the render +test harness landed — the counts are that run's, kept because the timings +beside them are its own: `npm run test` over 103 files and 1885 tests took +50.5 s; `npm run test:node` 59 tests took 39.0 s; `npm run test:contracts` 382 +tests took 10.5 s; `npx biome check .` over 977 files took 3 s; `tsc +--noEmit` over the whole repo took 42 s to 65 s per run and reports 0 errors, +which is the ratchet's baseline rather than new debt. The suite has grown since +(harness, review fixes): the PR's CI quality job on the final head is the live +count, and it — not this snapshot — is what a later comparison reads. + +## What the numbers mean for the next step + +Step 30 records the renderer budget, and `@dnd-kit` is the dependency a +performance claim will be checked against: 2.1 MB installed, three packages, no +importer yet, so its bundle cost is still zero and becomes measurable the moment +step 17 mounts the shared `DndContext`. The `build.files` exclusion is worth +213.6 MB on a linux-x64 artifact and 207.8 MB on linux-x64-musl, which is the +difference between a packaged app that grew by about a fifth and one that did not. +The `sharp` declaration costs 37 MB of platform binaries in a dev install and +nothing in a packaged app, because it is a devDependency and the icon script runs +before packaging. diff --git a/.dump/app/decisions/2026-09-23-review-and-sonar-remediation.md b/.dump/app/decisions/2026-09-23-review-and-sonar-remediation.md new file mode 100644 index 00000000..18639fb4 --- /dev/null +++ b/.dump/app/decisions/2026-09-23-review-and-sonar-remediation.md @@ -0,0 +1,772 @@ +# What the review round on PR #69 was worth, and what it changed + +Roadmap step 12, issue #14, PR #69 on `arena/01a0cbec-mauscode`, review round of +2026-09-23 against head `eb4a2bf`. This is the record of every finding the round +produced, what each one was worth, and what was done about it, so the next reader +does not have to re-judge a bot's opinion or re-derive why two findings were left +standing. A second round follows it: the duplication sweep instructed after head +`2c8be15`, recorded under its own heading below against head `dbb9457`. +A third round follows that one: every finding and every SonarCloud state still +open taken to either a fix or a decline carrying its evidence, against heads +`19ffbe1` to `cacd3db`. + +AGENTS.md still says no review bot is configured. That is now false in three +directions: Sourcery, CodeAnt and Buoy all review here, and SonarQube Cloud runs a +quality gate on the pull request. Bot output was treated as evidence to verify +against the pinned artifacts and the tree, not as a verdict. + +## The round, finding by finding + +| Source | Finding | Verdict | Where it landed | +| --- | --- | --- | --- | +| Sourcery | A prompt suggestion stayed in the sub-chat atom after the next turn began | Real bug | `eb4a2bf` | +| Sourcery | Suggestions were stored by sub-chat id alone, so a late one from an older session could overwrite the current turn's | Real bug | `eb4a2bf` | +| CodeAnt | `KillBash`, `BashOutputTool`, `AgentOutput` and `AgentOutputTool` had no registry entry, so persisted calls rendered as generic rows | Real gap | `df4b974` | +| Buoy | `min-h-[32px]` should be `min-h-8`, twice, in the effort sub-menu | Declined | PR comment | +| SonarCloud | Quality gate failed: 4.2% duplication on new code against a 3% limit | Real, and this step's own making | `685aebd`, `fe09b5a` | +| SonarCloud | `handlePromptSuggestion` nested in the transformer closure | Real, cheap | `57e8cd3` | +| SonarCloud | Effort sub-menu props not read-only | Real, cheap | `3306846` | +| SonarCloud | `onData` cognitive complexity 43 against 15 | Real shape problem, mostly pre-existing | `dc182be`, `fe09b5a` | +| SonarCloud | `NewChatForm` cognitive complexity 23 against 15 | Pre-existing, deferred | below | +| SonarCloud | `renderPart` in `assistant-message-item.tsx` cognitive complexity 70 against 15 | Pre-existing, deferred | below | + +Duplication was the only condition the quality gate actually failed on. The three +cognitive-complexity reports are annotations against new code, and two of the three +are reported only because this PR touched lines inside functions that were already +over the limit. + +## The duplication was this step's own making + +The pin added three turn-shaping features to the capability manifest. Ten backends +each grew the same six lines: a three-line comment pointing at the research record, +then `effort`, `adaptiveThinking` and `promptSuggestions` set to false. Eight of the +ten were byte-identical, and a scan for repeated eight-line windows over the tree +returned them as the single largest duplicated block touching new code. + +`TURN_CONTROLS_OFF` in `src/shared/provider-capabilities.ts` now holds the "when in +doubt, false" answer in the module that owns the rule. A manifest spreads it and +names only what its own backend carries: Codex overrides `effort`, Claude turns all +three on and so inherits nothing. The forcing function is worth more than the line +count — a fourth flag added to the schema now fails typecheck in one place instead +of being silently missing from whichever manifest someone forgot. + +The second duplication source was not written by this step but was exposed by it. +`chat-chunk-atoms.ts` states that it was extracted from the Claude IPC transport so +the native runtime transport could reuse identical question, compacting and +prompt-extraction behaviour, and then the Claude transport kept its own copies: +`applyQuestionChunks`, `applyCompactingChunks`, `clearStalePendingQuestion`, +`extractPromptText` and `extractPromptImages` were called only from +`native-chat-transport.ts`, while `ipc-chat-transport.ts` carried the same logic +inline and two private methods byte-identical to the shared extractors. Refactoring +`onData` into handlers made that copy-paste visible; `fe09b5a` deletes it, 172 +lines, and the two transports can no longer drift on the question lifecycle. +`session-init` deliberately stays per transport, because the native runtime reads a +cached snapshot and fills the gaps while the CLI reports the full set on init. + +## Round 2: taking the duplication to zero, and where zero stops being the goal + +The instruction after head `2c8be15` was "eliminate all code duplication +correctly". Round 1 removed the two blocks a reviewer would have pointed at; this +round removed what was left that could be removed honestly, and writes down what +stayed and why. The measure is a windowed scan over the whole tree at 4- and +6-line windows, keeping only blocks whose copies contain lines this PR added — +stricter than the gate it feeds, since Sonar's CPD threshold for TypeScript is +about 100 tokens and a six-line window of short object entries is well under it. + +### `ALL_FEATURES_OFF` replaces `TURN_CONTROLS_OFF` + +`TURN_CONTROLS_OFF` named four of the thirteen flags, so every manifest still +restated the other nine and the shared default covered a third of the object. The +flags are not four. They are thirteen booleans that default off, because a +capability nobody proved is a capability this app must not advertise — round 1's +rule, applied to the whole object instead of to the slice that had just grown: + +- `FeatureFlags` is now `z.infer`, exported in place of a + hand-named partial, so the type cannot drift from the schema. +- `ALL_FEATURES_OFF` spells all thirteen `false` under `satisfies FeatureFlags`. + Adding a flag to the schema is a compile error there until its default is chosen + deliberately, and off is the only default that needs no edit. +- Each manifest reads `features: { ...ALL_FEATURES_OFF, }`. + A flag that is off because nobody proved it is no longer written at all; a flag + that is off *for a reason* keeps its reason inline — grok's undocumented `-r` + composition, cline's broken `--id`, openclaw's missing session ids, roo's + rejected prompt, the two unverified skills claims. + +The direction of the default did not move, so neither did the fail-safe: +`ALL_FEATURES_OFF` is a superset of `TURN_CONTROLS_OFF`, and no provider gained a +control it did not have. That was verified rather than assumed. The effective flag +matrix parsed out of every manifest at `2c8be15` was compared key by key against +the rewritten tree: 130 values across 10 providers, 0 differences; the default +itself checked for 13 keys, all `false`, covering every schema flag with none +outside it. Net: −77/+47 lines across eleven files, most of it deleted negatives. + +### One hook for the Claude half of the model picker + +`chat-input-area.tsx` and `new-chat-form.tsx` each carried a byte-identical +37-line `useAvailableModels`, an identical connection test and an identical +26-line `claude={{ ... }}` block — about 110 duplicated lines that had already +drifted once, when the effort rows were added to both surfaces by hand in the +round that shipped adaptive thinking. This duplication predates step 12; step 12 +added to it, which is what made it worth ending. + +`hooks/use-claude-model-picker.ts` now owns the model list and the offline-Ollama +overlay, the custom-config test, the connection test, the resolved Ollama model, +extended thinking, and the capability-driven effort rows. It returns the list plus +a ready props object typed against `AgentModelSelectorProps["claude"]`, exported +from the selector for the purpose, so the block cannot drift from the component it +feeds: add a prop to the selector and the shared object fails typecheck until it +supplies one. The connection test travels inside that object rather than in the +return, because neither surface reads it apart from the picker. + +Two things stay with each surface on purpose: `selectedModelId` and +`onSelectModel`. They are the only lines that differ — the composer also stamps the +sub-chat model id — and pulling them into the hook would mean passing callbacks +back out, which is the same coupling wearing a different hat. Net across the two +surfaces: −177/+22, and the drift class is gone rather than merely smaller. + +### The transports take the SDK's own options type + +Both chat transports restated +`sendMessages(options: { messages: UIMessage[]; abortSignal?: AbortSignal })` and +then repeated the same three lines reading the last user message. Both classes +`implements ChatTransport`, so the shape was the library's all along, +and restating it did not merely duplicate it — it narrowed it: the real options +carry `trigger`, `chatId`, `messageId` and `ChatRequestOptions`, none of which +either transport could see. + +`chat-chunk-atoms.ts`, which already holds the shared transport helpers, now also +holds `SendMessagesOptions = Parameters["sendMessages"]>[0]` +and `lastUserPrompt(messages)`. Each transport declares the derived type and reads +the turn once. This is the one duplication the compiler was already holding equal — +an implementation that stops matching its interface does not typecheck — and it was +still worth removing, because what the copies shared was a loss of contract. + +### Left standing, with reasons + +1. **Import statements.** `import { ALL_FEATURES_OFF, type ProviderCapability } from + "../../../shared/provider-capabilities"` is identical in ten manifests, and the + two surfaces share atom import lines. These are references, not behaviour: + nothing inside them can drift out of step with anything, and the only way to + "share" an import is a barrel module whose entire job is being imported. +2. **Declarative manifest data.** Seven manifests share + `contextWindow: null, latencyClass: "cloud", usageSurface: "native"`, and the two + that share `usageSurface: "none"` share it for the same reason. Unlike the flags, + these have no fail-safe default: a spread that quietly handed a new provider + cloud latency or a native usage surface would be a wrong value inherited + silently, where an off flag is a safe one. Restating a fact per provider is what + a capability table is for. +3. **Call sites of the new abstractions.** `const { availableModels, ... } = + useClaudeModelPicker(hiddenModels)` and `claude={{ ...claudePickerProps, ... }}` + appear in both surfaces because both surfaces use the shared thing. Two call + sites of one hook are the point, not the residue. +4. **Coincidental windows with pre-existing code.** A `break / default: / break` + switch tail in `transform.ts` matches three unrelated files; two providers' + comment prose matches. Neither is a copy of anything this step wrote. + +After this round the scan reports no duplicated block of six or more lines in which +both copies contain lines this PR added, other than the manifest data and the call +sites itemised above. The same scan before round 2 reported 21 such groups. + +### The gate after the sweep + +SonarQube Cloud on head `b75064c`: quality gate passed, duplication on new code +0.5% — 7 duplicated lines out of 1284 new ones — against the 3% limit, the 4.2% +that failed it, and the 1.0% round 1 reached. Security hotspots 0. New issues 1, +down from 2, for the reason recorded in the complexity section below. CodeAnt's +five gates pass and it approved this head. All fifteen substantive GitHub Actions +checks pass: Build on ubuntu-24.04, macos-14 and windows-2022, Package unsigned on +ubuntu-24.04 and macos-14, the lint/test/typecheck quality gates, the security +gates, both Socket reports and CodeRabbit; Buoy, Sourcery and DeepSource skip as +before. Buoy re-posted the same two `min-h-[32px]` → `min-h-8` suggestions and they +are declined again on the evidence already recorded here: the arbitrary form appears +14 times across `src/**/*.tsx`, the token form zero times. + +Sonar's per-file measure says where the 7 lines are, and it is one line in each of +seven provider manifests — cline, codex, cursor, grok, hermes, opencode, qwen. Every +file an abstraction touched reports zero duplicated new lines: both surfaces, the new +hook, the selector, both transports, the shared chunk helpers, +`provider-capabilities.ts`, `transform.ts`, `types.ts`, every test file and +`package.json`. The two manifests that report zero are openclaw and roo, which are +the two whose feature blocks carry reasoned false flags with comments — the comment +lines break the window. One duplicated line per manifest is consistent with the two +things the local scan also flags there, the data tail +(`latencyClass: "cloud", usageSurface: "native"`) and the spread line +(`features: { ...ALL_FEATURES_OFF, chat: true`), and with nothing else those files +contain. Neither is a decision stated twice, which is what item 2 above argues, and +0.5% is what is left when the decisions are each stated once. + +## Round 3: every finding to a fix or an evidenced decline + +The instruction after head `19ffbe1` was to take all of the new review findings and +all of the new SonarCloud states and work through them, leaving each with either a +fix or a decline that carries its evidence. Six commits: `41688f4`, +`e5b602a`, `5a98fb4`, `1900d14`, `cacd3db`, `97f4327`. Five findings were fixed +and three declined, and one of the three names a real defect that belongs to a +different contract rather than to this one. + +| Source | Finding | Verdict | Where it landed | +| --- | --- | --- | --- | +| CodeAnt | `availableModels` includes hidden Claude models while the picker filters them (`chat-input-area.tsx:519`) | Real, and round 2's own making | `41688f4` | +| CodeAnt | `claudePickerProps` hides configured models but `selectedModel` comes from the unfiltered list (`new-chat-form.tsx:2137`) | The same defect on the other surface | `41688f4` | +| CodeAnt | The materialize effect leaves a stale model id stored when every Claude model is hidden (`chat-input-area.tsx:542`) | Declined — pre-existing, unreachable as described, and the guard is load-bearing | reply `r4086282209` | +| Buoy | `min-h-[32px]` → `min-h-8`, twice, re-posted on lines this PR does not touch | Declined again, with the numbers | replies `r4086282510`, `r4086282751` | +| SonarCloud | Seven duplicated lines, one in each of seven provider manifests | Real block, wrong suspect: it was the probe, not the flags | `e5b602a` | +| SonarCloud | `renderPart` cognitive complexity 70 against 15 | Fixed, harness first | `5a98fb4`, `1900d14` | +| SonarCloud | `typescript:S3358` nested ternary in `probe-command.ts:37` | Real, introduced by `e5b602a` | `cacd3db` | +| SonarCloud | `typescript:S3358` nested ternary in the plan indicator | Real, and surfaced by the split: moved lines count as new | `97f4327` | +| SonarCloud | The seven duplicated manifest lines that remain | Declined — the shape of the table is what repeats, and no constant can hold it | below | + +### The picker's selection came from a list the picker did not show + +Round 2 extracted one hook for the Claude half of the picker and left each surface +the two props that were genuinely its own: which model is selected, and what +selecting one does. CodeAnt found what that left behind. The hook returned +`availableModels` exactly as `useAvailableModels` has always returned it — +`CLAUDE_MODELS`, unfiltered — and `props.models` filtered by `hiddenModels`. Each +surface kept a `selectedModel` reading the unfiltered one, so the picker offered +three models while the selection, the trigger label and the id written to +per-sub-chat storage could all be a fourth, hidden one. That is the drift the hook +was extracted to end, reintroduced one level up by returning both lists. + +`41688f4` filters once, inside the hook, and returns the filtered list as +`availableModels.models`, so there is one list and no surface can read the other. +Both surfaces then derive their selection from it the way every other provider in +both files already derives its own — `codexUiModels.find(...) || codexUiModels[0]` +and the rest — which replaces a `useState` plus a sync effect that could only ever +move towards a model it could find. Net −24/+31 across the hook and the two +surfaces. + +### The duplicated block on the manifests was the probe, not the flags + +Sonar's duplication endpoint on this PR named two block sets across the manifests. +The second, the one carrying the new lines, is a 37-line window starting at the +head of each capability object; the first is a sixteen-line `execFile` wrapper that +all ten manifests carried byte-identically, eight as `runBinary` and two as +`runLaunch`, differing only in the bound — fifteen seconds in eight, thirty in +openclaw and roo. Ten copies of one rule about what a capability probe may do: run +a CLI once, bounded, and resolve rather than reject, because the ordinary failure +is a binary that is not installed, and read `error.code` as a string errno for a +spawn failure versus a number for a non-zero exit. + +`providers/probe-command.ts` holds it once (`e5b602a`, −193/+74 across eleven +files), with the bound as a named export and the two longer-bound manifests passing +it explicitly. `cacd3db` then takes the one issue the extraction itself drew — +S3358 on `error ? (typeof error.code === "number" ? error.code : null) : 0`, three +outcomes in one expression with the reason for the middle test sitting in a comment +above it — and puts the rule in `exitCodeOf(error)`, where the explanation and the +branch are the same thing. + +**What stays duplicated there, and why no constant can fix it.** One line per +manifest still reports as duplicated, inside the capability-object window. Sonar's +CPD normalizes TypeScript string literals, so what matches across those files is +not a value stated twice — the values differ per provider — but the *shape* the +`ProviderCapability` type mandates: the same field names, in the same order, +because that is what makes the ten manifests one table. Extracting the shared lines +into a constant would mean either a spread whose defaults are silently wrong for +the provider that inherits them (the fail-safe argument already recorded for the +feature flags does not hold for `latencyClass` or `usageSurface`) or a partial type +that stops being a capability table. Seven lines out of 1284, 0.5% against a 3% +limit, is what is left when each decision is stated once and the shape of the table +is what repeats. + +### Nothing rendered the transcript dispatcher, so the output was recorded first + +`renderPart` was the last standing complexity finding: 70 against 15, a +`useCallback` inside `AssistantMessageItem` holding eighteen branch decisions, +eighteen closure reads and the JSX for each one. Round 2 declined it on the grounds +that nothing tested the file, and that a pins pull request cannot also rewrite a +legacy renderer safely. The first half of that was fixable, which changed the +answer: the instruction for this round was to work the findings, and the way to +make a 250-line dispatcher safe to restructure is to write down what it currently +produces. + +`5a98fb4` adds 28 snapshot tests, one message per branch — text, whitespace-only +text, step-start, a part that is neither text nor tool, Bash success and Bash +failure, reasoning, a completed thinking tool, Edit with its patch, Write, a plan +file as a card, a second plan operation as a mini indicator, web search, web fetch, +PlanWrite, ExitPlanMode, a todo list, a question awaiting an answer, a registry +tool, the renamed TaskOutput, a sub-agent task with nested tools, an orphaned +nested group, an MCP call, an unregistered tool, the collapsed-steps path, the +streaming path, the exploring group and the usage badges. No child component +reaches for trpc, the router, Sentry or electron, so the only context a render +needs is the tooltip provider the agents layout already supplies; the question +chime is mocked because jsdom has no `Audio` and a sound is not part of the output. +The message id is reset per test because it lands in the rendered DOM, so a +snapshot cannot depend on how many tests ran before it. Baseline stability was +checked rather than assumed: 28 snapshots written, then a second run with 0 written +and 0 obsoleted. + +Three devDependencies come with that, in a PR that otherwise only moves pins: +`jsdom` 30.1.1, `@testing-library/react` 16.3.3 and its `@testing-library/dom` +10.4.2 peer. Exact pins, no carets, and the lockfile was regenerated by bun 1.4.2 — +the version CI's `oven-sh/setup-bun` pins — so `bun install --frozen-lockfile` +accepts it. Five entries the diff removes reappear in it unchanged (`ansi-styles`, +`entities`, `lru-cache`, `parse5`, `yallist`): bun re-sorted sections rather than +re-resolving anything, and no existing dependency changed version. The vitest +environment stays `node` globally; this one file declares jsdom for itself, and the +include list gains `*.test.tsx`. + +`1900d14` then does the split, and the branch bodies do not change. They move to +module scope as one function per shape — text, sub-agent task, Bash, thinking, plan +operation, file edit, web search, web fetch, plan write, todo list, question, +registry row, unregistered tool — each taking the part, its index and one +`PartRenderContext` carrying what only the component knows. `renderMessagePart` +dispatches in the order it always did, which is the part that carries meaning: a +sub-agent `Task` and a Write to a plan file both claim a type the dispatch table +also names, and they have to win. Two things fall out — Write and Edit rendered the +identical `AgentEditTool` in two branches, so one function serves both table +entries, and the closure's eighteen-entry deps list becomes a `useMemo` around the +context with `renderPart` depending on that single object. + +The evidence that it worked is the absence of a diff: `git diff` reports no change +to the `.snap` file, and vitest wrote 0 and obsoleted 0 against the baseline +recorded one commit earlier. This is the model the round-2 record predicted — one +function per decision, an explicit context, a single dispatch — applied to a +renderer instead of a transport, and the reason the harness came first is that +"the snapshots still pass" is only evidence if they existed before the change. + +SonarCloud agreed on the analysis after `1900d14`: the S3776 report on +`renderPart` is gone, new technical debt fell from 60 minutes to 10, and the same +analysis raised something worth recording — `typescript:S3358` on the plan +indicator, a ternary inside a ternary inside JSX choosing between four strings. +It had sat in `renderPart` unreported for as long as the function existed, +because Sonar analyses a pull request against its new code and those lines were +old. Moving a line makes it new. `97f4327` puts the four strings and the two +questions in `planOperationLabel(isWrite, isOpStreaming)` and leaves the JSX one +ternary, and adds the two snapshots that pin the strings no fixture covered — +"Updating plan..." and "Updated plan" — so all four are now recorded. The +snapshot diff for that commit is additions only, 0 removed lines, which is the +same evidence as the split: 30 tests, 1 written, 0 updated. + +**Where the round ended.** SonarQube Cloud on head `97f4327`: quality gate passed, +**0 new issues**, **0 minutes** of new technical debt, 0 security hotspots, and +duplication on new code at **0.3%** — 7 lines out of 2302, the same seven manifest +lines argued above. Across the round the issue count went 1 → 2 → 1 → 0 and the debt +60 → 10 → 5 → 0 minutes, each step a commit: the probe extraction added the second +S3358, `cacd3db` removed it, the split removed the S3776 and surfaced the first +one on moved lines, and `97f4327` removed that. All ten substantive GitHub Actions +checks pass on the same head. + +That is a general hazard of this kind of remediation and it is worth stating +plainly: extracting code in a pull request re-reports whatever the extracted lines +already carried. The choice is between leaving a 70-complexity dispatcher alone +and finding out what else lives in it. Both findings found this way were five +minutes of work and one made the renderer better. + +### Declined: a stale id that nothing can read as a stale model + +CodeAnt, Major, on the materialize effect: when every Claude model is hidden, +`selectedModel` is undefined, the effect returns early, and the old id stays in +per-sub-chat storage, "so the next request still sends a hidden model". Declined, +on four facts (reply `r4086282209`): + +1. The effect is byte-identical at this PR's base (`33475d8`, + `chat-input-area.tsx:569-575`), early return included. Nothing here introduced + it. +2. Neither transport consults `hiddenModels`; both are untouched by this PR and both + resolve `MODEL_ID_MAP[selectedModelId] || MODEL_ID_MAP.opus` + (`ipc-chat-transport.ts:400`, `native-chat-transport.ts:81`). In the state the + finding describes *every* Claude model is hidden, so a cleared id resolves to + opus — also hidden. No value the effect could store makes the next request send + a visible model. +3. The early return is load-bearing. `models` is a synchronous filter over a static + list, so it is empty only when the user hid them all, never transiently. Leave + storage alone and the preference survives the cycle: hide everything, unhide + Sonnet and Opus, and `find(stored) || models[0]` lands back on the model they + chose. Write a default on the empty path and the same cycle lands on + `models[0]`, because the stored id no longer matches anything. +4. What `41688f4` did change is the part that was wrong: at base `selectedModel` + was `useState` seeded from the unfiltered list, so a hidden model stayed + selected, stayed in the trigger label and was written back on mount. It now + derives from the visible list, the label reads `"Select model"` + (`chat-input-area.tsx:792-794`), and nothing is written. + +The finding does name a real gap, one level down: hiding a model hides it from the +picker and not from the wire. Closing that means filtering at send time in the two +transports, which changes which model runs for every existing chat, mid-session, +and is a contract of its own rather than a line in a pins PR. It is recorded here +and in the PR body as a follow-up. This token cannot open an issue (403 on both +create and comment), so it is handed off in writing rather than filed. + +### Declined again: `min-h-[32px]` + +Buoy re-posted the same two suggestions on `agent-model-selector.tsx:367` and `:381` +after the earlier decline, on lines this PR does not touch. The decline stands and +now carries the arithmetic (replies `r4086282510`, `r4086282751`): tailwindcss is +`^3.4.17`, `tailwind.config.js` only `extend`s — no `spacing`, `minHeight` or +`fontSize` override — and nothing sets a root `font-size` (every `font-size` rule in +`globals.css` is scoped to a component), so `min-h-8` is `2rem` is `32px` and the +swap changes no computed style. `min-h-[32px]` appears 16 times under `src/`, +`min-h-8` appears 0 times, and 5 of the 16 are in the flagged file, so taking the +token on two lines leaves 14 arbitrary sites and introduces a second spelling of +one value. Both classNames also carry `py-[5px]` and `w-[calc(100%-8px)]`, neither +of which has a token form — 5px falls between `1`/4px and `1.5`/6px — so the line +stays arbitrary-valued either way. A repo-wide move to spacing tokens is a styling +sweep of its own. + +## The complexity findings this step does not take + +| Function | Reported | What this PR changed inside it | What clearing it needs | +| --- | --- | --- | --- | +| `renderPart`, `assistant-message-item.tsx:803` | 70 against 15 | +3 / −4 lines: one `if` removed, one equality swapped for `isSubagentToolType()` | The 250-line render dispatcher split into components. It closes over roughly a dozen locals — orphan and nested tool-call sets, the nested-tools map, collapse state, the file-open callback — so each extraction is a props contract of its own, and nothing tests the file | +| `NewChatForm`, `new-chat-form.tsx:216` | 23 against 15 | +11 lines, one of them a ternary, so one point of the 23 | Section extraction across a 2300-line component whose remaining complexity is spread through the JSX body as `&&` and ternary expressions rather than sitting in one block | + +Both are pre-existing conditions reported on new code because the PR touched them. +Neither blocks the gate. Both are left standing on purpose: a dependency-pin pull +request that also rewrites two legacy renderer components cannot be reviewed as a +dependency-pin pull request, and there is no test coverage to make the rewrite +safe. They are named here so they are handed off rather than quietly dropped, and +the transport refactor is the model for how to do them — one handler per side +effect, an explicit context, and a single exit path. + +**Update from round 2.** One of the two no longer stands, and nothing attacked it: +SonarCloud's issue list for this PR now returns a single issue, `typescript:S3776` +on `assistant-message-item.tsx:803`. Moving the picker wiring out of `NewChatForm` +(`e57175b`) took the ternary that fed the effort rows and the `||` chains that +resolved the custom config, the connection state and the Ollama model with it, and +the component came out under the threshold. A component that stops owning a decision +stops paying for its branches — the same argument as the transport refactor, arrived +at from the other end, and a reason to expect the `renderPart` split to be worth +doing on its own terms rather than as gate relief. `renderPart` is untouched by this +round and stands at 70 against 15. + +**Update from round 3.** The other one is gone as well, and this time something +attacked it. `5a98fb4` wrote down what `renderPart` renders — 28 snapshots, one +message per branch — and `1900d14` moved the branches to module scope against one +explicit context, leaving a dispatcher of eleven increments where a single function +carried seventy. The snapshots pass byte-identically, which is what makes the split +a refactor rather than a rewrite. The table above keeps its round-2 wording because +the reasoning that deferred it was sound when it was written, and what changed it +was not a better argument but a test file: "no coverage to make the rewrite safe" +was the load-bearing half of the decline, and that half was removable. + +## What was declined, and why + +Buoy asked for `min-h-8` in place of `min-h-[32px]` on two rows of the effort +sub-menu. `min-h-[32px]` appears 14 times across `src/**/*.tsx` and `min-h-8` +appears zero times, so the token form has no precedent in this renderer, one of the +two flagged lines is pre-existing Codex code this PR only generalized, and adopting +the token here would make this component the single exception while leaving the +other 14 arbitrary values in place. A repo-wide move to spacing tokens is a +formatting contract of its own. + +**Corrected in round 3.** The count at head `cacd3db` is 16 occurrences of +`min-h-[32px]` under `src/` and 0 of `min-h-8`, not 14 — the earlier number +counted `src/**/*.tsx` only. Buoy re-posted both suggestions after the decline and +they were declined again with the arithmetic and with the reason the swap cannot +even buy a token-only line: the same two classNames carry `py-[5px]` and +`w-[calc(100%-8px)]`, which have no token form. Round 3 records it in full. + +## One finding this step must not fix + +The pin renames tools, and the permission classifier keeps its own name table. +`READ_ONLY_TOOLS` in `src/shared/permissions/classifier.ts` lists `BashOutput`, +which the 2.1.270 binary no longer emits: it emits `TaskOutput`. `classifyToolName` +falls through to `approval` / `unclassified-tool`, "no classification, so it takes +the middle tier", for any name it does not know, so the direction of the drift is +fail-safe — reading background output now costs an approval instead of being +read-only, rather than the other way round. The same is true of `TaskStop` for +`KillShell` and `KillBash`, and of `Agent` for `Task`. + +It is recorded rather than changed here because the classifier belongs to the +permission-floor work of steps 10 and 11, which another lane owns, and a +dependency-pin step editing that table is exactly the cross-lane collision the lane +rules exist to prevent. Whoever owns the classifier should decide whether the +renamed tools take their predecessors' classes. + +## Verification in this sandbox + +Re-run in full against the round-2 head `dbb9457`; every row below is that run. +2 CPU, 3.9 GB, no Electron binary (the postinstall download is intercepted) and no +local build or package. + +| Gate | Result | +| --- | --- | +| `biome check .` | 978 files, 0 findings | +| `tsc --noEmit` | 0 errors, after building `packages/runtime-client` for its `dist` types | +| `ratchet:typecheck` | passed, 0 errors against a 0 baseline | +| `vitest run` | 103 files, 1891 passed, 1 skipped | +| `npm run test:node` | 59 passed, 0 failed | +| `npm run test:contracts` | 23 files, 382 passed | +| `npm run lint` | 923 files checked, no findings | +| `ratchet:audit` | passed, 3 critical baseline, no new critical advisories | +| `skills:verify` | 50 of 50 locked skills verified, 2 unrecorded project-owned (pre-existing) | +| `build`, `package:linux` | not runnable here; CI runs them and the results are recorded on the PR | + +### Round 3, re-run in full against head `97f4327` + +| Gate | Result | +| --- | --- | +| `biome check .` | 980 files, 0 findings | +| `npm run lint` | 925 files checked, no findings | +| `ratchet:typecheck` | passed, 0 errors against a 0 baseline | +| `vitest run` | 104 files, 1921 passed, 1 skipped | +| the render harness alone | 30 snapshots, stable across runs; the 28 recorded before the split pass byte-identically after it, and `git diff` on the `.snap` file is empty for the split and additions-only for the two tests added with the label extraction | +| `bun install --frozen-lockfile` | accepted the lock regenerated with the three new devDependencies; the five entries the diff removes reappear unchanged, so no existing dependency moved | +| `npm run test:node` | 59 passed, 0 failed | +| `npm run test:contracts` | 382 passed | +| `ratchet:audit` | passed, no new critical advisories against a 3-critical baseline | +| `skills:verify` | 50 of 50 locked skills verified, 2 unrecorded project-owned | +| GitHub Actions | Build ubuntu-24.04, macos-14, windows-2022; Package unsigned ubuntu-24.04, macos-14; quality gates; security gates; both Socket reports; CodeRabbit — all pass. Buoy, Sourcery, DeepSource skip | +| SonarQube Cloud | quality gate passed, 0 new issues, 0 debt, 0 hotspots, 0.3% duplication on 2302 new lines | + +## Round 4: the independent review, twenty-one threads + +A fourth reviewer — `kilo-code-bot`, six multipass reviews plus targeted +confirmations — opened twenty-one inline threads against head `5ea0b64` with a +`CHANGES_REQUESTED` review: five Major, six Moderate (later ten), five Low +across three addenda, plus a mediation roadmap in the PR thread. Each claim was +checked against the pinned SDK's own `.d.ts`, the installed bundle, this +repo's code and its recorded snapshots before anything was touched; the +disposition table below is the round's result. Nothing was accepted on the +reviewer's word alone, and nothing was declined without evidence in the reply. + +### Fixed — sixteen of twenty-one + +| Thread | What it claimed | What settled it | Commit | +| --- | --- | --- | --- | +| MCP peer contract (Moderate) | SDK 0.3.270 declares `@modelcontextprotocol/sdk` `^1.29.0`; the graph resolves `1.25.3` | Reproduced: `npm ls` → `invalid: "^1.29.0"`, `ELSPROBLEMS`. Exact `1.30.1` pinned, newest in range | `dfe7044` | +| Transform throws on malformed lines (Major) | `apiRetryMessage` and `handlePromptSuggestion` trust fields `toClaudeStreamMessage` proved only have a string `type`; qwen `feedLine` has no catch, so the throw escapes into the main process | Both handlers read only documented shapes; `feedLine` wraps premap/result/transform in a boundary that settles the turn. Claude router already caught; qwen was the process-killer | `ead68c1` | +| Launch rendered as completion (Major) | `AgentOutput.status` is `completed \| async_launched \| remote_launched`; the row asked only "streaming?" | `sdk-tools.d.ts` confirms the union; `isLaunchedAgentOutput` branches the title and the registry phrase. Three statuses pinned in a snapshot | `6804c04` | +| Nested ancestry orphaned (Major) | Grouping resolved a child's parent by first id segment among top-level tasks only, so `B:C` never found `A:B` | The transform composes `parentOriginal:childOriginal` from the SDK's immediate `parent_tool_use_id`; lookup now goes through every task's original id, keys children by the parent's full id, self-parent skipped as the cycle guard, recursive rows capped at three levels | `6804c04` | +| Inert focusable subtitles (Moderate) | Every registry subtitle wore `role="button"` and a tab stop; TaskOutput rows have no action | Our own snapshot contained `Task: task_1`; button semantics now require a handler. Snapshot deltas verified as exactly those two attribute deletions | `b4c9ebb` | +| `shell_id` dropped (Low) | `TaskStopInput` accepts the deprecated `shell_id`; the shared reader took only `task_id`/`taskId` | `sdk-tools.d.ts:920` confirms; reader takes `shell_id` under the same `Task:` label; registry test pins it | `b4c9ebb` | +| Effort is global (Major) | One `claudeEffortAtom` for every pane while the model beside it is per-sub-chat | Mirrored the Codex thinking family: `subChatClaudeEffortAtomFamily` + `lastSelected` keeping the old storage key; `in` not `??` so an explicit null stays that chat's answer. Five tests including migration | `07bf80a` | +| Native sends no effort (Moderate) | The picker shows effort on the Native engine; the transport omits the field | Contract verified before building: the pinned runtime-client exposes `setReasoningEffort` (`set_reasoning_effort`). Router validates `z.enum(EFFORT_LEVELS)` and applies it best-effort after `set_model` | `7f7a8f9` | +| Stale suggestion survives turn/engine (Moderate ×2) | Session equality stood in for turn ownership; native never cleared; abort/error left the row | `{text, turn, engine}` entry, per-sub-chat generation bumped by both transports at send, store-time and render-time gates, guarded clears on abort/error. Seven tests on the pure rules | `34ea81b` | +| Preference not authoritative (Moderate) | Option sent only when true, so an inherited `CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION` decided | Env var overridden both directions from the toggle (the SDK documents env beating settings), `promptSuggestions: false` sent as explicitly as true, store refuses chunks while off | `34ea81b` | +| Click erases draft (Moderate) | `setValue(suggestion)` clears and rebuilds the editor | `mergeDraftWithSuggestion`: replace only an empty/whitespace draft, otherwise append after one space, draft byte-stable. Five tests; voice path uses the same join | `5e24490` | +| Switch has no name (Moderate) | Sibling ``, no `aria-label`; 31 switches, 0 labelled | `aria-label="Prompt Suggestions"` on the new control; test queries `getByRole("switch", { name: ... })` | `7e2c988` | +| NaN claim (Low) | JS coerces `null` to 0; the transform normalizes with `?? 0` | Verified on Node 22; research record rewritten to the real contract | `2456829` | +| Benchmark counts (Low) | Row says 103 files / 1885 tests; head CI reports more | Row labelled as the pre-harness measurement it is, with CI named as the live count | `2456829` | +| Stale version strings (Low) | CLAUDE.md, openspec/project.md, permission-hook comment still name 0.2.45 / 2.1.45 / 0.137.0 | All three moved to 0.3.270 / 2.1.270 / 0.154.0; the hook's `["deny", "ask"]` claim re-read in the 0.3.270 bundle before the number moved; roadmap §16 added ratifying every lockfile addition | `2456829` | +| Probe `null` doc (Low) | `null` also follows timeout, signal, EACCES, max-buffer — not only "never ran" | Comment narrowed to "no usable exit code", naming ENOENT as the one case the helper can prove; behavior deliberately unchanged (pre-existing, deferred with the structured-result follow-up) | `2456829` | + +### Declined — four threads, evidence in each reply + +- **`conversation_reset` (Major).** `/clear` is a client-side builtin in this + app — it "creates new sub-chat" (`builtin-commands.ts`) — so the CLI's reset + event never reaches the transformer from any path the app owns. The + router lines the thread cites are unreachable for it. +- **Per-model capability matrix (Major).** `src/shared/effort.ts` already + records the SDK's documented clamp ("an effort above a model's + `maxEffortLevel` is clamped to it") and files per-model + `supportedEffortLevels` as the named follow-up; no model-info source exists + in-repo to build the matrix from. The engine half of the same thread was + fixed instead (native now sends effort), and the "hidden control still + sent" premise was checked: nothing hides the effort rows for custom or + offline — `efforts` is passed unconditionally — so there is no hidden + control to disagree with. +- **`TaskOutput` in `READ_ONLY_TOOLS` (Moderate).** The classifier belongs to + the sibling permissions lane; checked `arena/01a0bb80-mauscode` at push time + — still `BashOutput` only — so the handoff stands as a handoff, and touching + it here would collide with the lane that owns it. +- **Packaged artifact size (Moderate).** The benchmark already lists packaged + and bundle size as *needs `bun run package:linux`* rows owned by CI and step + 30; this sandbox cannot produce the artifact, and the record says so rather + than claiming a measurement it does not have. + +Two sub-asks rode along declined with their threads: capping the raw qwen line +before `JSON.parse` (identical feedLine at base, outside the diff cause, and +JSON.parse was already guarded) and restructuring the probe's null into a +structured result (behavior inherited from the base wrappers; the comment now +says what it proves, the follow-up keeps the redesign). + +### Where the round stood + +Ten commits from `5ea0b64` to `2456829`, every gate re-run in full at each of +the three code commits that needed it and at the docs commit: biome 987 files +0 findings, typecheck ratchet 0 errors against a 0 baseline, lint 932 files +clean, vitest 109 files / 1951 passed / 1 skipped (29 new tests across the +round), test:node 59, test:contracts 382, audit ratchet at the 3-critical +baseline, skills:verify 50 of 50. Pushes after `5e24490` are queued locally — +the session's GitHub token expired mid-round (401 on REST and GraphQL) — and +the replies to the twenty-one threads post when the connection is restored. + +## Round 5: five new threads, five fixes, and the Sonar leak period + +Kilo's reconciliation at 16:01Z opened five new inline findings against +`5e24490` (the twelve already-discussed rows in that summary were answers to +the round-4 threads). Each was verified in the code before anything was +touched; all five were real; all five were fixed: + +| Thread | Claim | Commit | +| --- | --- | --- | +| `qwen-print/session.ts` | Settling a malformed line left the child running; stdout kept feeding a dead turn | `c84a571` — settled turns refuse further lines; catch escalates SIGINT→SIGTERM→SIGKILL | +| `ipc-chat-transport.ts:506` | Provider `error` chunks never hit subscription `onError`, so a stored suggestion survived the failure | `e429440` — `error`/`auth-error` chunks clear under the generation guard | +| `suggestion-ownership.ts:42` | `suggestionIsCurrent` asked engine and turn but not the preference; off→on resurrected a withdrawn row | `3c3500e` — preference is the third argument the render gate passes | +| `agent-tool-utils.ts:171` | `nestedChildren` identity defeated task-row memo on every stream render | `faae395` — nesting map compared by content, not callback identity | +| `agent-tool-call.tsx:19` | Tooltip-only subtitles lost keyboard access when role/tabindex were removed | `8b0ee3d` — tab stop with no role, only when a tooltip exists | + +Replies posted on each thread; summary comment `5818332768`. + +### Sonar on the same heads + +After the round-4/5 pushes the leak period carried **10 open issues** (gate +still passed: 0 hotspots, 0.7% duplication on new code). Nine were cleared in +`1fe7cf9` — both S3776s by extraction (`openNativeTurnSession` / +`applyNativeEffort`, `buildNestingIndex`), the S3358 by hoisting the +max-retries half, S7755/S6582/S6551/S5906×2 as one-liners, and S6819 by +making the action subtitle a native ` + )} {thinkings.map((thinking) => { const isSelected = selectedThinking === thinking return ( @@ -349,7 +380,7 @@ function CodexThinkingSubMenu({ onFocus={cancelClose} className="flex items-center justify-between gap-4 min-h-[32px] py-[5px] px-1.5 mx-1 w-[calc(100%-8px)] rounded-md text-sm cursor-default select-none outline-none dark:hover:bg-neutral-800 hover:text-foreground transition-colors" > - {formatCodexThinkingLabel(thinking)} + {formatEffortLabel(thinking)} {isSelected && } ) @@ -940,6 +971,14 @@ export function AgentModelSelector({ className="scale-75" /> + {claude.efforts.length > 0 && ( + claude.onSelectEffort(null)} + /> + )} )} @@ -952,7 +991,7 @@ export function AgentModelSelector({ if (!selectedCodexModel) return null return ( <> - + +// Available Claude models, plus the Ollama ones an offline turn can use. +function useAvailableModels() { + const showOfflineFeatures = useAtomValue(showOfflineModeFeaturesAtom) + const { data: ollamaStatus } = trpc.ollama.getStatus.useQuery(undefined, { + refetchInterval: showOfflineFeatures ? 30000 : false, + enabled: showOfflineFeatures, // Only query Ollama when offline mode is enabled + }) + + const baseModels = CLAUDE_MODELS + + const isOffline = ollamaStatus ? !ollamaStatus.internet.online : false + const hasOllama = ollamaStatus?.ollama.available && (ollamaStatus.ollama.models?.length ?? 0) > 0 + const ollamaModels = ollamaStatus?.ollama.models || [] + const recommendedModel = ollamaStatus?.ollama.recommendedModel + + // Only show offline models if: + // 1. Debug flag is enabled (showOfflineFeatures) + // 2. Ollama is available with models + // 3. User is actually offline + if (showOfflineFeatures && hasOllama && isOffline) { + return { + models: baseModels, + ollamaModels, + recommendedModel, + isOffline, + hasOllama: true, + } + } + + return { + models: baseModels, + ollamaModels: [] as string[], + recommendedModel: undefined as string | undefined, + isOffline, + hasOllama: false, + } +} + +export function useClaudeModelPicker(hiddenModels: readonly string[], subChatId = "") { + const modelSets = useAvailableModels() + // Every other provider reads its selection from the list the hidden-model + // setting has already been applied to (`codexUiModels`, `rooUiModels` and the + // rest), and Claude was the exception: the picker received a filtered list + // while the selection, the label and the id written back to the sub-chat all + // read the unfiltered one, so hiding a model left it selected, displayed and + // sent. One list, the visible one, so the two cannot disagree. When every + // Claude model is hidden the list is empty and both surfaces fall back on + // their own — the composer offers nothing, the new-chat form sends its + // existing `?? "opus"` default — which is what hiding them all means. + const models = useMemo( + () => modelSets.models.filter((model) => !hiddenModels.includes(model.id)), + [modelSets.models, hiddenModels], + ) + const availableModels = { ...modelSets, models } + + // A custom config counts as connected: the turn goes to the endpoint it names + // rather than to an account this app signed into. + const customClaudeConfig = useAtomValue(customClaudeConfigAtom) + const hasCustomClaudeConfig = Boolean(normalizeCustomClaudeConfig(customClaudeConfig)) + const anthropicOnboardingCompleted = useAtomValue(anthropicOnboardingCompletedAtom) + const apiKeyOnboardingCompleted = useAtomValue(apiKeyOnboardingCompletedAtom) + const { data: claudeCodeIntegration } = trpc.claudeCode.getIntegration.useQuery() + const isClaudeConnected = + Boolean(claudeCodeIntegration?.isConnected) || + anthropicOnboardingCompleted || + apiKeyOnboardingCompleted || + hasCustomClaudeConfig + + const [selectedOllamaModel, setSelectedOllamaModel] = useAtom(selectedOllamaModelAtom) + // The model an offline turn actually runs: the picked one, else the one + // Ollama recommends, else the first one available. + const currentOllamaModel = + selectedOllamaModel || availableModels.recommendedModel || availableModels.ollamaModels[0] + + const [thinkingEnabled, setThinkingEnabled] = useAtom(extendedThinkingEnabledAtom) + + // The effort rows come from the backend's own capability profile, so a + // provider that reports no effort control shows no sub-menu. The VALUE is + // owned by the sub-chat beside the model it will be sent with: the composer + // passes its id, and the new-chat form passes none, which reads and writes + // the last-selected value a fresh chat inherits. + const { data: claudeCapability } = trpc.providers.get.useQuery({ id: "claude" }) + const claudeEfforts = claudeCapability?.features.effort ? EFFORT_LEVELS : [] + const [selectedClaudeEffort, setSelectedClaudeEffort] = useAtom( + subChatClaudeEffortAtomFamily(subChatId), + ) + + const props: SharedClaudePickerProps = { + models, + hasCustomModelConfig: hasCustomClaudeConfig, + isOffline: availableModels.isOffline && availableModels.hasOllama, + ollamaModels: availableModels.ollamaModels, + selectedOllamaModel: currentOllamaModel, + recommendedOllamaModel: availableModels.recommendedModel, + onSelectOllamaModel: setSelectedOllamaModel, + isConnected: isClaudeConnected, + thinkingEnabled, + onThinkingChange: setThinkingEnabled, + efforts: claudeEfforts, + selectedEffort: selectedClaudeEffort, + onSelectEffort: setSelectedClaudeEffort, + } + + // The connection test travels inside `props`; neither surface reads it apart + // from the picker, so it is not part of the return. + return { + availableModels, + hasCustomClaudeConfig, + currentOllamaModel, + props, + } +} diff --git a/src/renderer/features/agents/lib/chat-chunk-atoms.ts b/src/renderer/features/agents/lib/chat-chunk-atoms.ts index 83f64e38..c60dea51 100644 --- a/src/renderer/features/agents/lib/chat-chunk-atoms.ts +++ b/src/renderer/features/agents/lib/chat-chunk-atoms.ts @@ -6,7 +6,7 @@ * handling that is provider-specific (session-init, auth modals, error * toasts) stays in each transport. */ -import type { UIMessageChunk as SDKUIMessageChunk, UIMessage } from "ai" +import type { ChatTransport, UIMessageChunk as SDKUIMessageChunk, UIMessage } from "ai" import type { UIMessageChunk as WireUIMessageChunk } from "../../../../main/lib/claude/types" import type { SessionInfo } from "../../../lib/atoms" import { appStore } from "../../../lib/jotai-store" @@ -232,3 +232,28 @@ export function extractPromptImages(msg: UIMessage | undefined): ImageAttachment return images } + +/** + * What the AI SDK hands a transport on send. Derived from the library's own + * `ChatTransport` contract rather than restated per transport, so both engines + * accept the same options and neither narrows them by hand: when the SDK adds a + * field, both see it without an edit here. + */ +export type SendMessagesOptions = Parameters["sendMessages"]>[0] + +/** + * The newest user turn's prompt text and image attachments. Both engines read + * the last user message the same way, so the reading lives here; a missing + * message yields an empty prompt and no images, which each engine then rejects + * on its own terms. + */ +export function lastUserPrompt(messages: readonly UIMessage[]): { + prompt: string + images: ImageAttachment[] +} { + const lastUser = [...messages].reverse().find((message) => message.role === "user") + return { + prompt: extractPromptText(lastUser), + images: extractPromptImages(lastUser), + } +} diff --git a/src/renderer/features/agents/lib/composer-text.test.ts b/src/renderer/features/agents/lib/composer-text.test.ts new file mode 100644 index 00000000..03791532 --- /dev/null +++ b/src/renderer/features/agents/lib/composer-text.test.ts @@ -0,0 +1,31 @@ +/** + * The click that used to replace whatever the composer held with the + * suggestion, destroying any unsent draft. + */ +import { describe, expect, it } from "vitest" +import { mergeDraftWithSuggestion } from "./composer-text" + +describe("mergeDraftWithSuggestion", () => { + it("puts the suggestion in an empty draft", () => { + expect(mergeDraftWithSuggestion("", "Now run the gate")).toBe("Now run the gate") + }) + + it("treats a whitespace-only draft as empty", () => { + expect(mergeDraftWithSuggestion(" \n ", "Now run the gate")).toBe("Now run the gate") + }) + + it("keeps every character of a typed draft and appends after one space", () => { + expect(mergeDraftWithSuggestion("Wait — did the lockfile move?", "Now run the gate")).toBe( + "Wait — did the lockfile move? Now run the gate", + ) + }) + + it("keeps leading whitespace the draft already had", () => { + expect(mergeDraftWithSuggestion(" already indented", "next")).toBe(" already indented next") + }) + + it("adds no space the draft does not need", () => { + expect(mergeDraftWithSuggestion("finish this line ", "next")).toBe("finish this line next") + expect(mergeDraftWithSuggestion("end\n", "next")).toBe("end\nnext") + }) +}) diff --git a/src/renderer/features/agents/lib/composer-text.ts b/src/renderer/features/agents/lib/composer-text.ts new file mode 100644 index 00000000..4eafc640 --- /dev/null +++ b/src/renderer/features/agents/lib/composer-text.ts @@ -0,0 +1,19 @@ +/** + * What clicking a suggested next prompt does to the draft already in the + * composer. + * + * The rule is that a click accepts the suggestion, not that it discards what + * the user typed while the suggestion sat there: an empty or whitespace-only + * draft is replaced outright, and any other draft keeps every character it + * had — leading whitespace included — gaining the suggestion after it, joined + * by the one space the two strings would otherwise be missing. + * + * The voice path calls this with its already-trimmed current text, which is + * what it did inline before; the suggestion path passes the draft untouched. + * One join, two callers, each deciding its own input. + */ +export function mergeDraftWithSuggestion(draft: string, suggestion: string): string { + if (draft.trim().length === 0) return suggestion + const needsSpace = !/\s$/.test(draft) + return draft + (needsSpace ? " " : "") + suggestion +} diff --git a/src/renderer/features/agents/lib/ipc-chat-transport.ts b/src/renderer/features/agents/lib/ipc-chat-transport.ts index b4f56940..78455b47 100644 --- a/src/renderer/features/agents/lib/ipc-chat-transport.ts +++ b/src/renderer/features/agents/lib/ipc-chat-transport.ts @@ -1,8 +1,10 @@ /** - * NOTE (transplant): inlined question/compact chunk handling, stale-question - * clearing fix, extractText/extractImages, and log removals were transplanted - * from erenbertr/1code (Apache-2.0). Their auth-error toast replacement was - * NOT taken — this tree keeps the login-modal retry flow. + * NOTE (transplant): the question/compact chunk handling, the stale-question + * clearing fix, the prompt and image extraction, and the log removals were + * transplanted from erenbertr/1code (Apache-2.0). The first three now live in + * `./chat-chunk-atoms`, which both transports share, and the provenance record + * is NOTICE and UPSTREAM.md. Their auth-error toast replacement was NOT taken + * — this tree keeps the login-modal retry flow. */ import * as Sentry from "@sentry/electron/renderer" @@ -19,24 +21,34 @@ import { extendedThinkingEnabledAtom, historyEnabledAtom, normalizeCustomClaudeConfig, + promptSuggestionsEnabledAtom, selectedOllamaModelAtom, sessionInfoAtom, showOfflineModeFeaturesAtom, + subChatClaudeEffortAtomFamily, } from "../../../lib/atoms" import { appStore } from "../../../lib/jotai-store" import { trpcClient } from "../../../lib/trpc" import { - askUserQuestionResultsAtom, - compactingSubChatsAtom, - expiredUserQuestionsAtom, MODEL_ID_MAP, pendingAuthRetryMessageAtom, - pendingUserQuestionsAtom, subChatModelIdAtomFamily, + subChatPromptSuggestionAtomFamily, + subChatTurnGenerationAtomFamily, } from "../atoms" import { useAgentSubChatStore } from "../stores/sub-chat-store" import type { AgentMessageMetadata } from "../ui/agent-message-usage" -import type { LooseUIPart, SubscriptionChunk } from "./chat-chunk-atoms" +import { + applyCompactingChunks, + applyQuestionChunks, + type ChatChunkContext, + clearStalePendingQuestion, + type ImageAttachment, + lastUserPrompt, + type SendMessagesOptions, + type SubscriptionChunk, +} from "./chat-chunk-atoms" +import { mayStoreSuggestion } from "./suggestion-ownership" // Error categories and their user-friendly messages const ERROR_TOAST_CONFIG: Record< @@ -124,24 +136,267 @@ type IPCChatTransportConfig = { model?: string } -// Image attachment type matching the tRPC schema -type ImageAttachment = { - base64Data: string - mediaType: string - filename?: string +/** The session id off a `message-metadata` chunk, whose payload the subscription + * types as unknown. */ +function hasSessionId(value: unknown): value is { sessionId: string } { + return ( + typeof value === "object" && + value !== null && + typeof (value as { sessionId?: unknown }).sessionId === "string" + ) +} + +/** + * What the chunk handlers decided: `enqueue` hands the chunk to the AI SDK, + * `consumed` ends handling for a chunk the SDK has no type for, and `failed` + * means the stream was already errored. + */ +type ChunkOutcome = "enqueue" | "consumed" | "failed" + +/** + * What a handler needs besides the chunk: the shared question context both + * transports pass, widened with the turn this transport was built with and the + * session id the stream reports about itself. + */ +type ChunkContext = ChatChunkContext & { + /** The last 8 characters of the sub-chat id, which is what the stream logs tag. */ + subId: string + cwd: string + mode: AgentMode + prompt: string + images: ImageAttachment[] + /** Written by this stream's own metadata chunk, read by the suggestion after it. */ + sessionId: string | null + /** + * The turn generation this stream bumped when it started. A suggestion is + * stored under it, so a late chunk arriving after a newer send is refused + * instead of overwriting the newer turn's own. + */ + turnGeneration: number +} + +type ChunkController = ReadableStreamDefaultController + +/** + * What this session opened with. `chat-chunk-atoms.ts` leaves this one in each + * transport on purpose: the native runtime reads a cached snapshot and fills + * the gaps, while the Claude CLI reports the full set on init. + */ +function recordSessionInfo(chunk: SubscriptionChunk): void { + if (chunk.type !== "session-init") return + appStore.set(sessionInfoAtom, { + tools: chunk.tools, + mcpServers: chunk.mcpServers, + plugins: chunk.plugins, + skills: chunk.skills, + }) +} +/** + * An auth failure keeps this tree's modal-and-retry flow rather than a toast: + * park the turn so the modal can resend it after OAuth, then error the stream + * instead of closing it so the chat leaves "streaming" and the user can retry. + */ +function failTurnForAuth(ctx: ChunkContext, controller: ChunkController): ChunkOutcome { + // Store the failed message for retry after successful auth. + // readyToRetry=false prevents immediate retry; the modal sets it to true. + appStore.set(pendingAuthRetryMessageAtom, { + subChatId: ctx.subChatId, + provider: "claude-code", + prompt: ctx.prompt, + ...(ctx.images.length > 0 && { images: ctx.images }), + readyToRetry: false, + }) + appStore.set(claudeLoginModalConfigAtom, { + hideCustomModelSettingsLink: false, + autoStartAuth: false, + }) + // Show the Claude Code login modal + appStore.set(agentsLoginModalOpenAtom, true) + console.log(`[SD] R:AUTH_ERR sub=${ctx.subId}`) + // controller.error() rather than controller.close(), so the SDK Chat resets + // status from "streaming" to "ready". + controller.error(new Error("Authentication required")) + return "failed" +} + +/** + * A prompt suggestion belongs to the turn that produced it. The session id + * arrives on the metadata chunk the transform emits before the suggestion, so a + * late suggestion from an aborted or older run in the same sub-chat is dropped + * instead of overwriting this turn's. Neither chunk is one the AI SDK knows: + * the suggestion is consumed, the metadata is passed on. + */ +function routePromptSuggestion(chunk: SubscriptionChunk, ctx: ChunkContext): ChunkOutcome { + // Learn the session before the suggestion that follows it. The subscription's + // chunk type carries the metadata as unknown, so it is read through a + // predicate rather than a cast at the use site. + if (chunk.type === "message-metadata" && hasSessionId(chunk.messageMetadata)) { + ctx.sessionId = chunk.messageMetadata.sessionId + } + if (chunk.type !== "prompt-suggestion") return "enqueue" + if (ctx.sessionId && chunk.sessionId !== ctx.sessionId) return "consumed" + // The preference and the turn generation decide together: an inherited + // environment variable can make the CLI emit this while the app's switch is + // off, and a session id is reused across turns, so neither the switch nor + // the session alone answers whether the composer may still offer it. + const mayStore = mayStoreSuggestion({ + preferenceOn: appStore.get(promptSuggestionsEnabledAtom), + capturedTurn: ctx.turnGeneration, + currentTurn: appStore.get(subChatTurnGenerationAtomFamily(ctx.subChatId)), + }) + if (!mayStore) return "consumed" + appStore.set(subChatPromptSuggestionAtomFamily(ctx.subChatId), { + text: chunk.suggestion, + turn: ctx.turnGeneration, + engine: "legacy", + }) + return "consumed" +} + +/** A retry the CLI is already performing: said once, and not a stream chunk. */ +function announceRetry(chunk: SubscriptionChunk): ChunkOutcome { + if (chunk.type !== "retry-notification") return "enqueue" + toast.info("Retrying request", { + description: chunk.message || "Request was unsuccessful, trying again...", + duration: 4000, + }) + return "consumed" +} + +/** + * The copy an error toast shows, and the full text its copy action hands over. + * The category and debug payload come from the caller, which already read them + * for the log and Sentry, so the fallback is not decided twice. + */ +function errorToastCopy( + chunk: Extract, + ctx: ChunkContext, + category: string, + debugInfo: unknown, +): { title: string; description: string; details: string } { + // Available for every error, not only the categories this app recognizes. + const details = [ + `Error: ${chunk.errorText || "Unknown error"}`, + `Category: ${category}`, + `Chat ID: ${ctx.chatId}`, + `SubChat ID: ${ctx.subChatId}`, + `CWD: ${ctx.cwd}`, + `Mode: ${ctx.mode}`, + `Timestamp: ${new Date().toISOString()}`, + debugInfo ? `Debug Info: ${JSON.stringify(debugInfo, null, 2)}` : null, + ] + .filter(Boolean) + .join("\n") + + const config = ERROR_TOAST_CONFIG[category] + // For auth and API key failures the backend's own wording wins: it names the + // credential that failed, which this app's copy cannot. + const prefersBackendError = + category === "AUTH_FAILURE" || + category === "INVALID_API_KEY_SDK" || + category === "INVALID_API_KEY" + const rawDescription = prefersBackendError + ? chunk.errorText || config?.description || "An unexpected error occurred" + : config?.description || chunk.errorText || "An unexpected error occurred" + return { + title: config?.title || "Claude error", + // Truncate long descriptions for the toast (keep the first 300 chars). + description: + rawDescription.length > 300 ? `${rawDescription.slice(0, 300)}...` : rawDescription, + details, + } +} + +/** + * An error chunk is logged, sent to Sentry and toasted, and then still handed to + * the AI SDK: its message part is what the transcript shows afterwards. + */ +function reportErrorChunk(chunk: SubscriptionChunk, ctx: ChunkContext): void { + if (chunk.type !== "error") return + const debugInfo = "debugInfo" in chunk ? chunk.debugInfo : undefined + const category = debugInfo?.category || "UNKNOWN" + + // Detailed SDK error logging for debugging + console.error(`[SDK ERROR] ========================================`) + console.error(`[SDK ERROR] Category: ${category}`) + console.error(`[SDK ERROR] Error text: ${chunk.errorText}`) + console.error(`[SDK ERROR] Chat ID: ${ctx.chatId}`) + console.error(`[SDK ERROR] SubChat ID: ${ctx.subChatId}`) + console.error(`[SDK ERROR] CWD: ${ctx.cwd}`) + console.error(`[SDK ERROR] Mode: ${ctx.mode}`) + if (debugInfo) { + console.error(`[SDK ERROR] Debug info:`, JSON.stringify(debugInfo, null, 2)) + } + console.error(`[SDK ERROR] Full chunk:`, JSON.stringify(chunk, null, 2)) + console.error(`[SDK ERROR] ========================================`) + + Sentry.captureException(new Error(chunk.errorText || "Claude transport error"), { + tags: { errorCategory: category, mode: ctx.mode }, + extra: { debugInfo, cwd: ctx.cwd, chatId: ctx.chatId, subChatId: ctx.subChatId }, + }) + + const { title, description, details } = errorToastCopy(chunk, ctx, category, debugInfo) + toast.error(title, { + description, + duration: 12000, + action: { + label: "Copy Error", + onClick: () => { + navigator.clipboard.writeText(details) + toast.success("Error details copied to clipboard") + }, + }, + }) +} + +/** Enqueue without crashing on a stream that is already closed. */ +function enqueueChunk(controller: ChunkController, chunk: SubscriptionChunk): void { + try { + controller.enqueue(chunk as SDKUIMessageChunk) + } catch { + // Stream already closed, ignore enqueue failure + } +} + +/** Close without crashing on a stream that is already closed. */ +function closeQuietly(controller: ChunkController): void { + try { + controller.close() + } catch { + // Already closed + } +} + +/** + * The side effects a chunk has, in the order the stream needs them: questions + * first, so the stale-question clear sees the one just asked, then compaction + * and session info, then the chunks that end the turn or belong to this app + * rather than to the AI SDK. + */ +function routeChunk( + chunk: SubscriptionChunk, + ctx: ChunkContext, + controller: ChunkController, +): ChunkOutcome { + applyQuestionChunks(chunk, ctx) + applyCompactingChunks(chunk, ctx.subChatId) + recordSessionInfo(chunk) + clearStalePendingQuestion(chunk, ctx.subChatId) + + if (chunk.type === "auth-error") return failTurnForAuth(ctx, controller) + const suggestion = routePromptSuggestion(chunk, ctx) + if (suggestion !== "enqueue") return suggestion + const retry = announceRetry(chunk) + if (retry !== "enqueue") return retry + reportErrorChunk(chunk, ctx) + return "enqueue" } export class IPCChatTransport implements ChatTransport { constructor(private config: IPCChatTransportConfig) {} - async sendMessages(options: { - messages: UIMessage[] - abortSignal?: AbortSignal - }): Promise> { - // Extract prompt and images from last user message - const lastUser = [...options.messages].reverse().find((m) => m.role === "user") - const prompt = this.extractText(lastUser) - const images = this.extractImages(lastUser) + async sendMessages(options: SendMessagesOptions): Promise> { + const { prompt, images } = lastUserPrompt(options.messages) // Get sessionId for resume (server preserves sessionId on abort so // the next message can resume with full conversation context) @@ -151,10 +406,16 @@ export class IPCChatTransport implements ChatTransport { // Read extended thinking setting dynamically (so toggle applies to existing chats) const thinkingEnabled = appStore.get(extendedThinkingEnabledAtom) - // Max thinking tokens for extended thinking mode - // SDK adds +1 internally, so 64000 becomes 64001 which exceeds Opus 4.5 limit - // Using 32000 to stay safely under the 64000 max output tokens limit - const maxThinkingTokens = thinkingEnabled ? 32_000 : undefined + // Adaptive lets the model pick its own budget; disabled is what the toggle + // off has always meant but could not say while the field was a token count. + const thinking = thinkingEnabled + ? ({ type: "adaptive" } as const) + : ({ type: "disabled" } as const) + // null is "let the CLI choose", so a chat that never opened the picker keeps + // the model's own default instead of a level this app guessed. Read from + // THIS sub-chat's slot: two split panes carry two answers. + const effort = appStore.get(subChatClaudeEffortAtomFamily(this.config.subChatId)) + const promptSuggestions = appStore.get(promptSuggestionsEnabledAtom) const historyEnabled = appStore.get(historyEnabledAtom) const enableTasks = appStore.get(enableTasksAtom) @@ -178,10 +439,40 @@ export class IPCChatTransport implements ChatTransport { .allSubChats.find((subChat) => subChat.id === this.config.subChatId)?.mode || this.config.mode + // A suggestion belongs to the turn that produced it, so starting a turn + // bumps the generation — the store refuses a late chunk from the stream + // this send supersedes — and clears the last one: the composer must not + // offer a previous request's next step, and clicking it must not insert + // that into this prompt. + const turnGeneration = appStore.get(subChatTurnGenerationAtomFamily(this.config.subChatId)) + 1 + appStore.set(subChatTurnGenerationAtomFamily(this.config.subChatId), turnGeneration) + appStore.set(subChatPromptSuggestionAtomFamily(this.config.subChatId), null) + // Aborting or failing this turn takes its suggestion with it, but only if + // it is still this turn's: a superseding send may already have begun, and + // its own suggestion must not be wiped by the stream it replaced. + const clearOwnSuggestion = () => { + const entry = appStore.get(subChatPromptSuggestionAtomFamily(this.config.subChatId)) + if (entry?.turn === turnGeneration) { + appStore.set(subChatPromptSuggestionAtomFamily(this.config.subChatId), null) + } + } + // Stream tracking - const subId = this.config.subChatId.slice(-8) let _chunkCount = 0 let _lastChunkType = "" + // One context for the chunk handlers, so the session id this stream reports + // about itself is visible to the suggestion that follows it. + const ctx: ChunkContext = { + chatId: this.config.chatId, + subChatId: this.config.subChatId, + subId: this.config.subChatId.slice(-8), + cwd: this.config.cwd, + mode: currentMode, + prompt, + images, + sessionId: null, + turnGeneration, + } return new ReadableStream({ start: (controller) => { @@ -194,7 +485,12 @@ export class IPCChatTransport implements ChatTransport { projectPath: this.config.projectPath, // Original project path for MCP config lookup mode: currentMode, sessionId, - ...(maxThinkingTokens && { maxThinkingTokens }), + thinking, + ...(effort && { effort }), + // Sent in both directions: with the option absent, an inherited + // `CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION` in the shell decides, + // and the app's switch stops being the switch. + promptSuggestions, ...(modelString && { model: modelString }), ...(customConfig && { customConfig }), ...(selectedOllamaModel && { selectedOllamaModel }), @@ -208,238 +504,19 @@ export class IPCChatTransport implements ChatTransport { _chunkCount++ _lastChunkType = chunk.type - // Handle AskUserQuestion - show question UI - if (chunk.type === "ask-user-question") { - const currentMap = appStore.get(pendingUserQuestionsAtom) - const newMap = new Map(currentMap) - newMap.set(this.config.subChatId, { - subChatId: this.config.subChatId, - parentChatId: this.config.chatId, - toolUseId: chunk.toolUseId, - questions: chunk.questions, - }) - appStore.set(pendingUserQuestionsAtom, newMap) - - // Clear any expired question (new question replaces it) - const currentExpired = appStore.get(expiredUserQuestionsAtom) - if (currentExpired.has(this.config.subChatId)) { - const newExpiredMap = new Map(currentExpired) - newExpiredMap.delete(this.config.subChatId) - appStore.set(expiredUserQuestionsAtom, newExpiredMap) - } + // A provider failure arrives as an ordinary data chunk and is + // toasted by `routeChunk` — it never hits this subscription's + // `onError`. A failed turn leaves no next step behind: withdraw + // this turn's suggestion under the same generation guard as + // abort and transport failure, so a row stored before the + // failure cannot still be offered afterwards. + if (chunk.type === "error" || chunk.type === "auth-error") { + clearOwnSuggestion() } - // Handle AskUserQuestion timeout - move to expired (keep UI visible) - if (chunk.type === "ask-user-question-timeout") { - const currentMap = appStore.get(pendingUserQuestionsAtom) - const pending = currentMap.get(this.config.subChatId) - if (pending && pending.toolUseId === chunk.toolUseId) { - // Remove from pending - const newPendingMap = new Map(currentMap) - newPendingMap.delete(this.config.subChatId) - appStore.set(pendingUserQuestionsAtom, newPendingMap) - - // Move to expired (so UI keeps showing the question) - const currentExpired = appStore.get(expiredUserQuestionsAtom) - const newExpiredMap = new Map(currentExpired) - newExpiredMap.set(this.config.subChatId, pending) - appStore.set(expiredUserQuestionsAtom, newExpiredMap) - } - } - - // Handle AskUserQuestion result - store for real-time updates - if (chunk.type === "ask-user-question-result") { - const currentResults = appStore.get(askUserQuestionResultsAtom) - const newResults = new Map(currentResults) - newResults.set(chunk.toolUseId, chunk.result) - appStore.set(askUserQuestionResultsAtom, newResults) - } - - // Handle compacting status - track in atom for UI display - if ( - (chunk.type === "tool-input-start" && chunk.toolName === "Compact") || - (chunk.type === "tool-input-available" && chunk.toolName === "Compact") - ) { - const compacting = appStore.get(compactingSubChatsAtom) - const newCompacting = new Set(compacting) - // Compacting started - newCompacting.add(this.config.subChatId) - appStore.set(compactingSubChatsAtom, newCompacting) - } - if ( - (chunk.type === "tool-output-available" && - chunk.toolCallId?.startsWith("compact-")) || - (chunk.type === "tool-output-error" && chunk.toolCallId?.startsWith("compact-")) - ) { - const compacting = appStore.get(compactingSubChatsAtom) - const newCompacting = new Set(compacting) - // Compacting finished - newCompacting.delete(this.config.subChatId) - appStore.set(compactingSubChatsAtom, newCompacting) - } - - // Handle session init - store MCP servers, plugins, tools info - if (chunk.type === "session-init") { - appStore.set(sessionInfoAtom, { - tools: chunk.tools, - mcpServers: chunk.mcpServers, - plugins: chunk.plugins, - skills: chunk.skills, - }) - } - - // Clear pending questions ONLY when agent has moved on - // Don't clear on tool-input-* chunks (still building the question input) - // Clear when we get tool-output-* (answer received) or text-delta (agent moved on) - const shouldClearOnChunk = - chunk.type !== "ask-user-question" && - chunk.type !== "ask-user-question-timeout" && - chunk.type !== "ask-user-question-result" && - !chunk.type.startsWith("tool-input") && // Don't clear while input is being built - chunk.type !== "start" && - chunk.type !== "start-step" - - if (shouldClearOnChunk) { - const currentMap = appStore.get(pendingUserQuestionsAtom) - if (currentMap.has(this.config.subChatId)) { - const newMap = new Map(currentMap) - newMap.delete(this.config.subChatId) - appStore.set(pendingUserQuestionsAtom, newMap) - } - // NOTE: Do NOT clear expired questions here. After a timeout, - // the agent continues and emits new chunks — that's expected. - // Expired questions should persist until the user answers, - // dismisses, or sends a new message. - } - - // Handle authentication errors - show Claude login modal - // NOTE (mausCode): kept our modal+retry flow; their toast-only - // replacement was NOT transplanted. - if (chunk.type === "auth-error") { - // Store the failed message for retry after successful auth - // readyToRetry=false prevents immediate retry - modal sets it to true on OAuth success - appStore.set(pendingAuthRetryMessageAtom, { - subChatId: this.config.subChatId, - provider: "claude-code", - prompt, - ...(images.length > 0 && { images }), - readyToRetry: false, - }) - appStore.set(claudeLoginModalConfigAtom, { - hideCustomModelSettingsLink: false, - autoStartAuth: false, - }) - // Show the Claude Code login modal - appStore.set(agentsLoginModalOpenAtom, true) - // Use controller.error() instead of controller.close() so that - // the SDK Chat properly resets status from "streaming" to "ready" - // This allows user to retry sending messages after failed auth - console.log(`[SD] R:AUTH_ERR sub=${subId}`) - controller.error(new Error("Authentication required")) - return - } - - // Handle retry notification - show friendly toast instead of scary error - if (chunk.type === "retry-notification") { - toast.info("Retrying request", { - description: chunk.message || "Request was unsuccessful, trying again...", - duration: 4000, - }) - return // don't enqueue retry-notification as a stream chunk - } - - // Handle errors - show toast to user FIRST before anything else - if (chunk.type === "error") { - const debugInfo = "debugInfo" in chunk ? chunk.debugInfo : undefined - const category = debugInfo?.category || "UNKNOWN" - - // Detailed SDK error logging for debugging - console.error(`[SDK ERROR] ========================================`) - console.error(`[SDK ERROR] Category: ${category}`) - console.error(`[SDK ERROR] Error text: ${chunk.errorText}`) - console.error(`[SDK ERROR] Chat ID: ${this.config.chatId}`) - console.error(`[SDK ERROR] SubChat ID: ${this.config.subChatId}`) - console.error(`[SDK ERROR] CWD: ${this.config.cwd}`) - console.error(`[SDK ERROR] Mode: ${currentMode}`) - if (debugInfo) { - console.error(`[SDK ERROR] Debug info:`, JSON.stringify(debugInfo, null, 2)) - } - console.error(`[SDK ERROR] Full chunk:`, JSON.stringify(chunk, null, 2)) - console.error(`[SDK ERROR] ========================================`) - - // Track error in Sentry - Sentry.captureException(new Error(chunk.errorText || "Claude transport error"), { - tags: { - errorCategory: category, - mode: currentMode, - }, - extra: { - debugInfo: debugInfo, - cwd: this.config.cwd, - chatId: this.config.chatId, - subChatId: this.config.subChatId, - }, - }) - - // Build detailed error string for copying (available for ALL errors) - const errorDetails = [ - `Error: ${chunk.errorText || "Unknown error"}`, - `Category: ${category}`, - `Chat ID: ${this.config.chatId}`, - `SubChat ID: ${this.config.subChatId}`, - `CWD: ${this.config.cwd}`, - `Mode: ${currentMode}`, - `Timestamp: ${new Date().toISOString()}`, - debugInfo ? `Debug Info: ${JSON.stringify(debugInfo, null, 2)}` : null, - ] - .filter(Boolean) - .join("\n") - - // Show toast based on error category - const config = ERROR_TOAST_CONFIG[category] - const title = config?.title || "Claude error" - // For auth/API key failures, prefer original backend error to aid debugging - const preferOriginalError = - category === "AUTH_FAILURE" || - category === "INVALID_API_KEY_SDK" || - category === "INVALID_API_KEY" - // Use config description if set, otherwise fall back to errorText - const rawDescription = preferOriginalError - ? chunk.errorText || config?.description || "An unexpected error occurred" - : config?.description || chunk.errorText || "An unexpected error occurred" - // Truncate long descriptions for toast (keep first 300 chars) - const description = - rawDescription.length > 300 - ? `${rawDescription.slice(0, 300)}...` - : rawDescription - - toast.error(title, { - description, - duration: 12000, - action: { - label: "Copy Error", - onClick: () => { - navigator.clipboard.writeText(errorDetails) - toast.success("Error details copied to clipboard") - }, - }, - }) - } - - // Try to enqueue, but don't crash if stream is already closed - try { - controller.enqueue(chunk as SDKUIMessageChunk) - } catch (_e) { - // Stream already closed, ignore enqueue failure - } - - if (chunk.type === "finish") { - try { - controller.close() - } catch { - // Already closed - } - } + if (routeChunk(chunk, ctx, controller) !== "enqueue") return + enqueueChunk(controller, chunk) + if (chunk.type === "finish") closeQuietly(controller) }, onError: (err: Error) => { // Track transport errors in Sentry @@ -455,30 +532,26 @@ export class IPCChatTransport implements ChatTransport { }, }) + clearOwnSuggestion() controller.error(err) }, onComplete: () => { // Note: Don't clear pending questions here - let active-chat.tsx handle it // via the stream stop detection effect. Clearing here causes race conditions // where sync effect immediately restores from messages. - try { - controller.close() - } catch { - // Already closed - } + closeQuietly(controller) }, }, ) // Handle abort options.abortSignal?.addEventListener("abort", () => { + // A stopped turn leaves no next step behind: whatever arrived from + // it is withdrawn with the same generation guard as the error path. + clearOwnSuggestion() sub.unsubscribe() // trpcClient.claude.cancel.mutate({ subChatId: this.config.subChatId }) - try { - controller.close() - } catch { - // Already closed - } + closeQuietly(controller) }) }, }) @@ -487,53 +560,4 @@ export class IPCChatTransport implements ChatTransport { async reconnectToStream(): Promise | null> { return null // Not needed for local app } - - private extractText(msg: UIMessage | undefined): string { - if (!msg) return "" - if (msg.parts) { - const textParts: string[] = [] - const fileContents: string[] = [] - - for (const p of msg.parts) { - const part = p as LooseUIPart - if (part.type === "text" && part.text) { - textParts.push(part.text) - } else if (part.type === "file-content") { - // Hidden file content - add to prompt but not displayed in UI - const fileName = part.filePath?.split("/").pop() || part.filePath || "file" - fileContents.push(`\n--- ${fileName} ---\n${part.content}`) - } - } - - // Combine text and file contents - return textParts.join("\n") + fileContents.join("") - } - return "" - } - - /** - * Extract images from message parts - * Looks for parts with type "data-image" that have base64Data - */ - private extractImages(msg: UIMessage | undefined): ImageAttachment[] { - if (!msg?.parts) return [] - - const images: ImageAttachment[] = [] - - for (const part of msg.parts) { - // Check for data-image parts with base64 data - const data = (part as LooseUIPart).data - if (part.type === "data-image" && data) { - if (data.base64Data && data.mediaType) { - images.push({ - base64Data: data.base64Data, - mediaType: data.mediaType, - filename: data.filename, - }) - } - } - } - - return images - } } diff --git a/src/renderer/features/agents/lib/models.ts b/src/renderer/features/agents/lib/models.ts index 0262cfbe..7777572c 100644 --- a/src/renderer/features/agents/lib/models.ts +++ b/src/renderer/features/agents/lib/models.ts @@ -8,7 +8,6 @@ export { CODEX_MODELS, CODEX_SUBSCRIPTION_ONLY_MODEL_IDS, type CodexThinkingLevel, - formatCodexThinkingLabel, } from "../../../../shared/codex-model-id" export const CLAUDE_MODELS = [ diff --git a/src/renderer/features/agents/lib/native-chat-transport.ts b/src/renderer/features/agents/lib/native-chat-transport.ts index 1510570d..0b27e488 100644 --- a/src/renderer/features/agents/lib/native-chat-transport.ts +++ b/src/renderer/features/agents/lib/native-chat-transport.ts @@ -19,17 +19,24 @@ import { normalizeCustomClaudeConfig, sessionInfoAtom, showOfflineModeFeaturesAtom, + subChatClaudeEffortAtomFamily, } from "../../../lib/atoms" import { appStore } from "../../../lib/jotai-store" import { trpcClient } from "../../../lib/trpc" -import { MODEL_ID_MAP, pendingAuthRetryMessageAtom, subChatModelIdAtomFamily } from "../atoms" +import { + MODEL_ID_MAP, + pendingAuthRetryMessageAtom, + subChatModelIdAtomFamily, + subChatPromptSuggestionAtomFamily, + subChatTurnGenerationAtomFamily, +} from "../atoms" import { useAgentSubChatStore } from "../stores/sub-chat-store" import { applyCompactingChunks, applyQuestionChunks, clearStalePendingQuestion, - extractPromptImages, - extractPromptText, + lastUserPrompt, + type SendMessagesOptions, type SubscriptionChunk, } from "./chat-chunk-atoms" @@ -74,17 +81,15 @@ const NATIVE_ERROR_TOAST_CONFIG: Record { constructor(private config: NativeChatTransportConfig) {} - async sendMessages(options: { - messages: UIMessage[] - abortSignal?: AbortSignal - }): Promise> { - const lastUser = [...options.messages].reverse().find((m) => m.role === "user") - const prompt = extractPromptText(lastUser) - const images = extractPromptImages(lastUser) + async sendMessages(options: SendMessagesOptions): Promise> { + const { prompt, images } = lastUserPrompt(options.messages) // Read model selection dynamically per sub-chat (so split panes stay independent) const selectedModelId = appStore.get(subChatModelIdAtomFamily(this.config.subChatId)) const modelString = MODEL_ID_MAP[selectedModelId] || MODEL_ID_MAP.opus + // ...and the effort beside it, from the same family the legacy transport + // reads: both engines send what their pane's picker last chose. + const effort = appStore.get(subChatClaudeEffortAtomFamily(this.config.subChatId)) // Offline/Ollama routing is a legacy-path feature; refuse loudly rather // than silently running the turn against cloud credentials. @@ -114,6 +119,14 @@ export class NativeChatTransport implements ChatTransport { .allSubChats.find((subChat) => subChat.id === this.config.subChatId)?.mode || this.config.mode + // Turn ownership is shared with the legacy transport: bump the generation + // so a late suggestion from a still-open legacy stream is refused at the + // store, and clear whatever the previous turn left — this engine emits no + // suggestions of its own, and it inherits no stale ones. + const turnGeneration = appStore.get(subChatTurnGenerationAtomFamily(this.config.subChatId)) + 1 + appStore.set(subChatTurnGenerationAtomFamily(this.config.subChatId), turnGeneration) + appStore.set(subChatPromptSuggestionAtomFamily(this.config.subChatId), null) + const subId = this.config.subChatId.slice(-8) let chunkCount = 0 let lastChunkType = "" @@ -130,6 +143,9 @@ export class NativeChatTransport implements ChatTransport { projectPath: this.config.projectPath, mode: currentMode, ...(modelString && { model: modelString }), + // The same per-sub-chat effort the legacy transport sends: the + // daemon's `set_reasoning_effort` carries it the rest of the way. + ...(effort && { effort }), ...(customConfig?.token && { customToken: customConfig.token }), ...(customConfig?.baseUrl && { customBaseUrl: customConfig.baseUrl }), ...(images.length > 0 && { images }), diff --git a/src/renderer/features/agents/lib/subagent-tool-types.ts b/src/renderer/features/agents/lib/subagent-tool-types.ts new file mode 100644 index 00000000..b7895d45 --- /dev/null +++ b/src/renderer/features/agents/lib/subagent-tool-types.ts @@ -0,0 +1,26 @@ +/** + * The tool types that mean "a sub-agent is running", under every name the + * pinned Claude CLI has used for it. + * + * The 2.1.270 binary emits `Agent`. Its own normalization table maps the older + * spellings to the current ones, and the SDK changelog at 0.2.69 records why + * both exist: the wire name was reverted to `Task` with the note that it "will + * migrate to `Agent` in the next minor release", which the 0.3 line did. So a + * transcript persisted before the bump carries `Task` and one recorded after it + * carries `Agent`, and grouping, dispatch and suppression all have to treat the + * two as one tool or a resumed session renders differently from a new one. + * + * The background-task family is NOT in this list. `TaskCreate`, `TaskUpdate`, + * `TaskGet`, `TaskList` and `TaskStop` share the `Task` prefix and nothing else: + * they manage background shells and tasks, not sub-agents, and + * `assistant-message-item.tsx` keeps its own `TASK_TOOLS` set for them. + */ +export const SUBAGENT_TOOL_TYPES = ["tool-Task", "tool-Agent"] as const + +export type SubagentToolType = (typeof SUBAGENT_TOOL_TYPES)[number] + +const SUBAGENT_TOOL_TYPE_SET: ReadonlySet = new Set(SUBAGENT_TOOL_TYPES) + +export function isSubagentToolType(type: string): type is SubagentToolType { + return SUBAGENT_TOOL_TYPE_SET.has(type) +} diff --git a/src/renderer/features/agents/lib/suggestion-ownership.test.ts b/src/renderer/features/agents/lib/suggestion-ownership.test.ts new file mode 100644 index 00000000..c22ae36b --- /dev/null +++ b/src/renderer/features/agents/lib/suggestion-ownership.test.ts @@ -0,0 +1,66 @@ +/** + * The ownership rules a prompt suggestion passes through on its way from a + * provider stream into the composer, and the four ways it is refused. + */ +import { describe, expect, it } from "vitest" +import { + mayStoreSuggestion, + type PromptSuggestionEntry, + suggestionIsCurrent, +} from "./suggestion-ownership" + +describe("mayStoreSuggestion", () => { + it("stores a live turn with the preference on", () => { + expect(mayStoreSuggestion({ preferenceOn: true, capturedTurn: 3, currentTurn: 3 })).toBe(true) + }) + + it("refuses when the app's switch is off, whatever the stream emitted", () => { + // An inherited CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION can make the CLI send + // this; the preference still decides whether the composer ever sees it. + expect(mayStoreSuggestion({ preferenceOn: false, capturedTurn: 3, currentTurn: 3 })).toBe(false) + }) + + it("refuses a late arrival from a superseded turn", () => { + expect(mayStoreSuggestion({ preferenceOn: true, capturedTurn: 3, currentTurn: 4 })).toBe(false) + }) +}) + +describe("suggestionIsCurrent", () => { + const entry: PromptSuggestionEntry = { + text: "Now run the gate", + turn: 7, + engine: "legacy", + } + + it("keeps a suggestion while its engine, turn and preference still allow it", () => { + expect( + suggestionIsCurrent(entry, { engineNow: "legacy", turnNow: 7, preferenceOn: true }), + ).toBe(true) + }) + + it("hides a suggestion after the sub-chat switched engines", () => { + expect( + suggestionIsCurrent(entry, { engineNow: "native", turnNow: 7, preferenceOn: true }), + ).toBe(false) + }) + + it("hides a suggestion once another turn has started", () => { + expect( + suggestionIsCurrent(entry, { engineNow: "legacy", turnNow: 8, preferenceOn: true }), + ).toBe(false) + }) + + it("hides a stored suggestion once the preference is turned off", () => { + // Turning Prompt Suggestions off withdraws what is already in the atom; + // turning it back on must not resurrect a row the user dismissed. + expect( + suggestionIsCurrent(entry, { engineNow: "legacy", turnNow: 7, preferenceOn: false }), + ).toBe(false) + }) + + it("treats an empty atom as nothing to show", () => { + expect(suggestionIsCurrent(null, { engineNow: "legacy", turnNow: 7, preferenceOn: true })).toBe( + false, + ) + }) +}) diff --git a/src/renderer/features/agents/lib/suggestion-ownership.ts b/src/renderer/features/agents/lib/suggestion-ownership.ts new file mode 100644 index 00000000..695424ab --- /dev/null +++ b/src/renderer/features/agents/lib/suggestion-ownership.ts @@ -0,0 +1,54 @@ +/** + * Which prompt suggestions may be stored, and which stored ones may still be + * shown. Session equality alone answered neither: a session id is reused + * across turns, outlives an engine switch, and says nothing about whether the + * app's own switch is on. The rules live here so the transport that stores, + * the transport that clears, and the composer that renders all ask the same + * three questions — of the turn generation, the engine, and the preference — + * instead of each inventing its own. + */ +import type { SubChatEngine } from "../atoms" + +/** What the composer is allowed to offer, and the provenance that gates it. */ +export type PromptSuggestionEntry = { + text: string + /** The turn generation that produced it; every send bumps the family. */ + turn: number + /** The engine whose stream carried it: the legacy SDK, or the native runtime. */ + engine: SubChatEngine +} + +/** + * Whether a suggestion that just arrived may be written to the sub-chat's + * atom. The switch is authoritative — an inherited environment variable can + * make the CLI emit suggestions while the app's preference is off, and a + * consumed chunk with nowhere to go is the store's job to refuse — and a + * captured turn that is no longer the current one is a late arrival from a + * stream this app has already superseded. + */ +export function mayStoreSuggestion(args: { + preferenceOn: boolean + capturedTurn: number + currentTurn: number +}): boolean { + return args.preferenceOn && args.capturedTurn === args.currentTurn +} + +/** + * Whether a stored suggestion still describes the composer it would insert + * into: the preference is still on (turning Prompt Suggestions off withdraws + * whatever is already stored — and turning it back on must not resurrect it), + * same engine (a switch mid-flight changed who the next prompt would be + * addressed to), and same turn (something newer has started since). + */ +export function suggestionIsCurrent( + entry: PromptSuggestionEntry | null, + args: { engineNow: SubChatEngine; turnNow: number; preferenceOn: boolean }, +): entry is PromptSuggestionEntry { + return ( + entry !== null && + args.preferenceOn && + entry.engine === args.engineNow && + entry.turn === args.turnNow + ) +} diff --git a/src/renderer/features/agents/main/__snapshots__/assistant-message-item.test.tsx.snap b/src/renderer/features/agents/main/__snapshots__/assistant-message-item.test.tsx.snap new file mode 100644 index 00000000..d903756c --- /dev/null +++ b/src/renderer/features/agents/main/__snapshots__/assistant-message-item.test.tsx.snap @@ -0,0 +1,68 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > collapses the steps under a final text part 1`] = `"
Response

The lockfile is clean.

"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > groups three consecutive exploring tools 1`] = `"
Response

Read all three.

"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > keeps a nested subagent's descendants under it instead of orphaning them 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > keeps every part visible while the last message streams 1`] = `"

Lint is clean so far.

"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a Bash call with its command, output and exit code 1`] = ` +"
" +`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a PlanWrite 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a Write with its content 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a completed thinking tool 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a failing Bash call 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a plan file as a plan card 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a plan operation that is still streaming as a shimmer 1`] = `"
Creating plan...
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a question awaiting an answer 1`] = `"
SDK pin•Interrupted
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a reasoning part 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a registry tool as a single row 1`] = `"
Read
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a second plan operation as a mini indicator, not a card 1`] = `"
Created plan
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a subagent task with its nested tools 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a text part 1`] = `"

Both pins moved together.

"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a todo list 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a tool nobody registered as its bare name 1`] = `"
SomethingTheNextCliAdds
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a web fetch 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders a web search with its results 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders an Edit with its patch 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders an MCP tool call 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders an orphaned nested group as an incomplete task 1`] = `"
Subagent interruptedIncomplete task
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders nothing for ExitPlanMode 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders nothing for a part that is neither text nor a tool 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders nothing for a step-start marker 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders nothing for a text part that is only whitespace 1`] = `"
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders the four things a plan operation's indicator can say 1`] = `"
Updating plan...
Updated plan
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders the renamed background-output tool 1`] = `"
Got outputTask: task_1
"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > renders usage and git badges from message metadata 1`] = `"

Done.

"`; + +exports[`AssistantMessageItem, one message per branch of the part dispatcher > tells a launch apart from a finished subagent 1`] = `"
"`; diff --git a/src/renderer/features/agents/main/assistant-message-item.test.tsx b/src/renderer/features/agents/main/assistant-message-item.test.tsx new file mode 100644 index 00000000..3be9c3f7 --- /dev/null +++ b/src/renderer/features/agents/main/assistant-message-item.test.tsx @@ -0,0 +1,733 @@ +// @vitest-environment jsdom +/** + * What the transcript renderer produces, pinned before it is restructured. + * + * `renderPart` in `assistant-message-item.tsx` decides what every kind of + * message part looks like. It was a ~250-line dispatcher at a cognitive + * complexity SonarQube reported as 70 against a threshold of 15, and until this + * file existed nothing in the suite rendered any of it: the vitest environment + * is `node`, so the dispatcher's behaviour was verified by reading it. It is now + * `renderMessagePart` plus one module-scope function per shape — a change this + * file made safe rather than a change this file describes. + * + * These snapshots are the golden output for one message per branch of that + * dispatcher, written against the code as it stood and committed before the + * split, so the restructuring could be checked against what the renderer + * actually produced rather than against a reviewer's memory of it. The split + * landed one commit later and changed no byte of them. Nothing here asserts + * intent or good taste. It records output, and it stands as the guard on the + * next change to any of these branches, where the acceptable result is a diff + * somebody meant. + */ +import { fireEvent, render } from "@testing-library/react" +import { beforeEach, describe, expect, it, vi } from "vitest" +import { TooltipProvider } from "../../../components/ui/tooltip" +import type { Message, MessagePart } from "../stores/message-store" +import { AssistantMessageItem } from "./assistant-message-item" + +// The chime plays when a message ends on a question awaiting an answer. jsdom +// has no Audio, and a sound is not part of the output these snapshots pin. +vi.mock("../lib/play-question-sound", () => ({ playQuestionSound: vi.fn() })) + +// jsdom implements neither, and Radix's collapsible reaches for both while the +// step group renders. +vi.stubGlobal( + "ResizeObserver", + class { + observe() {} + unobserve() {} + disconnect() {} + }, +) +vi.stubGlobal( + "IntersectionObserver", + class { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return [] + } + }, +) + +/** + * The transcript renders inside the agents layout, which wraps its tree in a + * tooltip provider; the tool rows need that context to render at all. + */ +function renderMessage(message: Message, streaming: boolean): string { + const { container } = render( + + + , + ) + return container.innerHTML +} + +/** + * Each message gets its own id: the module keeps a per-message state cache to + * survive the AI SDK mutating parts in place, and a shared id would let one + * test's cache decide the next test's memo comparison. + */ +let messageSequence = 0 + +// The id lands in the rendered DOM, so it is reset per test: a snapshot must not +// depend on how many tests ran before it. +beforeEach(() => { + messageSequence = 0 +}) + +function renderParts(parts: MessagePart[], streaming = false): string { + const message: Message = { + id: `msg-${++messageSequence}`, + role: "assistant", + parts, + } + return renderMessage(message, streaming) +} + +/** The same message as `renderParts`, but with the DOM still attached to click on. */ +function renderPartsDom(parts: MessagePart[]): HTMLElement { + const { container } = render( + + + , + ) + return container +} + +/** A completed tool call, which is the state every fixture below starts from. */ +function tool(type: string, id: string, input: unknown, output?: unknown): MessagePart { + return { type, toolCallId: id, state: "output-available", input, output } +} + +describe("AssistantMessageItem, one message per branch of the part dispatcher", () => { + it("renders a text part", () => { + expect(renderParts([{ type: "text", text: "Both pins moved together." }])).toMatchSnapshot() + }) + + it("renders nothing for a text part that is only whitespace", () => { + expect(renderParts([{ type: "text", text: " \n " }])).toMatchSnapshot() + }) + + it("renders nothing for a step-start marker", () => { + expect(renderParts([{ type: "step-start" }])).toMatchSnapshot() + }) + + it("renders nothing for a part that is neither text nor a tool", () => { + expect( + renderParts([{ type: "file-content", filePath: "notes.txt", content: "hidden" }]), + ).toMatchSnapshot() + }) + + it("renders a Bash call with its command, output and exit code", () => { + expect( + renderParts([ + tool( + "tool-Bash", + "toolu_bash_1", + { command: "rg -n 'ALL_FEATURES_OFF' src" }, + { + stdout: "src/shared/provider-capabilities.ts:41:export const ALL_FEATURES_OFF\n", + exitCode: 0, + }, + ), + ]), + ).toMatchSnapshot() + }) + + it("renders a failing Bash call", () => { + expect( + renderParts([ + tool( + "tool-Bash", + "toolu_bash_2", + { command: "bun run build" }, + { stdout: "", stderr: "electron-builder exited with 1", exitCode: 1 }, + ), + ]), + ).toMatchSnapshot() + }) + + it("renders a reasoning part", () => { + expect( + renderParts([ + { type: "reasoning", text: "The registry names six legacy spellings.", state: "done" }, + ]), + ).toMatchSnapshot() + }) + + it("renders a completed thinking tool", () => { + expect( + renderParts([ + { + type: "tool-Thinking", + toolCallId: "toolu_think_1", + toolName: "Thinking", + state: "output-available", + input: { text: "Check the changelog for the wire name." }, + output: { completed: true }, + }, + ]), + ).toMatchSnapshot() + }) + + it("renders an Edit with its patch", () => { + expect( + renderParts([ + tool( + "tool-Edit", + "toolu_edit_1", + { + file_path: "/repo/src/shared/provider-capabilities.ts", + old_string: "export const TURN_CONTROLS_OFF", + new_string: "export const ALL_FEATURES_OFF", + }, + { + structuredPatch: [ + { lines: ["- export const TURN_CONTROLS_OFF", "+ export const ALL_FEATURES_OFF"] }, + ], + }, + ), + ]), + ).toMatchSnapshot() + }) + + it("renders a Write with its content", () => { + expect( + renderParts([ + tool("tool-Write", "toolu_write_1", { + file_path: "/repo/src/shared/effort.ts", + content: "export const EFFORT_LEVELS = ['minimal', 'low', 'medium', 'high'] as const\n", + }), + ]), + ).toMatchSnapshot() + }) + + it("renders a plan file as a plan card", () => { + expect( + renderParts([ + tool("tool-Write", "toolu_plan_1", { + file_path: "/repo/claude-sessions/plans/step-12-plan.md", + content: "# Step 12\n\nMove the pins together.\n", + }), + ]), + ).toMatchSnapshot() + }) + + it("renders a second plan operation as a mini indicator, not a card", () => { + expect( + renderParts([ + tool("tool-Write", "toolu_plan_2", { + file_path: "/repo/plans-a-plan.md", + content: "# First\n", + }), + tool("tool-Edit", "toolu_plan_3", { + file_path: "/repo/plans-a-plan.md", + old_string: "# First", + new_string: "# Second", + }), + ]), + ).toMatchSnapshot() + }) + + it("renders a plan operation that is still streaming as a shimmer", () => { + expect( + renderParts( + [ + { + type: "tool-Write", + toolCallId: "toolu_plan_4", + state: "input-streaming", + input: { file_path: "/repo/plans-arriving-plan.md", content: "# Still arriving\n" }, + }, + tool("tool-Edit", "toolu_plan_5", { + file_path: "/repo/plans-arriving-plan.md", + old_string: "# Still arriving", + new_string: "# Arrived", + }), + ], + true, + ), + ).toMatchSnapshot() + }) + + it("renders the four things a plan operation's indicator can say", () => { + expect( + renderParts( + [ + { + type: "tool-Edit", + toolCallId: "toolu_plan_6", + state: "input-streaming", + input: { + file_path: "/repo/plans-labels-plan.md", + old_string: "", + new_string: "# Arriving", + }, + }, + tool("tool-Edit", "toolu_plan_7", { + file_path: "/repo/plans-labels-plan.md", + old_string: "# Arriving", + new_string: "# Arrived", + }), + tool("tool-Write", "toolu_plan_8", { + file_path: "/repo/plans-labels-plan.md", + content: "# Arrived, and last, so a card\n", + }), + ], + true, + ), + ).toMatchSnapshot() + }) + + it("renders a web search with its results", () => { + expect( + renderParts([ + tool( + "tool-WebSearch", + "toolu_search_1", + { query: "claude agent sdk 0.3.270 changelog" }, + { + results: [ + { + content: [ + { title: "SDK changelog", url: "https://example.com/changelog" }, + { title: "Release notes", url: "https://example.com/releases" }, + ], + }, + ], + }, + ), + ]), + ).toMatchSnapshot() + }) + + it("renders a web fetch", () => { + expect( + renderParts([ + tool( + "tool-WebFetch", + "toolu_fetch_1", + { url: "https://example.com/docs/pins" }, + { result: "Fetched the pin table.", bytes: 12048, code: 200 }, + ), + ]), + ).toMatchSnapshot() + }) + + it("renders a PlanWrite", () => { + expect( + renderParts([ + tool("tool-PlanWrite", "toolu_planwrite_1", { + plan: { + status: "completed", + steps: [ + { title: "Pin the SDK and the CLI together", status: "completed" }, + { title: "Register the renamed tools", status: "completed" }, + ], + }, + }), + ]), + ).toMatchSnapshot() + }) + + it("renders nothing for ExitPlanMode", () => { + expect(renderParts([tool("tool-ExitPlanMode", "toolu_exit_1", {})])).toMatchSnapshot() + }) + + it("renders a todo list", () => { + expect( + renderParts([ + tool( + "tool-TodoWrite", + "toolu_todo_1", + { + todos: [ + { content: "Pin the SDK", status: "completed", activeForm: "Pinning the SDK" }, + { + content: "Register the tools", + status: "in_progress", + activeForm: "Registering the tools", + }, + ], + }, + { oldTodos: [], newTodos: [{ content: "Pin the SDK", status: "completed" }] }, + ), + ]), + ).toMatchSnapshot() + }) + + it("renders a question awaiting an answer", () => { + expect( + renderParts([ + { + type: "tool-AskUserQuestion", + toolCallId: "toolu_ask_1", + state: "call", + input: { + questions: [ + { + question: "Which SDK version should the pin take?", + header: "SDK pin", + multiSelect: false, + options: [ + { label: "0.3.270", description: "Matches the CLI pin." }, + { label: "0.3.280", description: "One release ahead." }, + ], + }, + ], + }, + }, + ]), + ).toMatchSnapshot() + }) + + it("renders a registry tool as a single row", () => { + expect( + renderParts([tool("tool-Read", "toolu_read_1", { file_path: "/repo/src/shared/effort.ts" })]), + ).toMatchSnapshot() + }) + + it("renders the renamed background-output tool", () => { + expect( + renderParts([ + tool("tool-TaskOutput", "toolu_taskoutput_1", { task_id: "task_1" }, { output: "done" }), + ]), + ).toMatchSnapshot() + }) + + it("renders a subagent task with its nested tools", () => { + expect( + renderParts([ + tool("tool-Agent", "toolu_agent_1", { + subagent_type: "general-purpose", + description: "Find every pin the step has to move", + }), + tool("tool-Read", "toolu_agent_1:0", { file_path: "/repo/package.json" }), + tool( + "tool-Bash", + "toolu_agent_1:1", + { command: "git diff --stat" }, + { stdout: "", exitCode: 0 }, + ), + ]), + ).toMatchSnapshot() + }) + + it("renders an orphaned nested group as an incomplete task", () => { + expect( + renderParts([ + tool("tool-Bash", "toolu_ghost_1:0", { command: "ls" }, { stdout: "", exitCode: 0 }), + tool("tool-Read", "toolu_ghost_1:1", { file_path: "/repo/AGENTS.md" }), + ]), + ).toMatchSnapshot() + }) + + it("tells a launch apart from a finished subagent", () => { + const html = renderParts([ + tool( + "tool-Agent", + "toolu_async_1", + { subagent_type: "general-purpose", description: "Watch the queue" }, + { + status: "async_launched", + agentId: "agent_1", + description: "Watch the queue", + prompt: "watch", + outputFile: "/tmp/agent.log", + }, + ), + tool( + "tool-Agent", + "toolu_remote_1", + { subagent_type: "general-purpose", description: "Run elsewhere" }, + { + status: "remote_launched", + taskId: "task_9", + description: "Run elsewhere", + prompt: "run", + }, + ), + tool( + "tool-Agent", + "toolu_done_1", + { subagent_type: "general-purpose", description: "Finished run" }, + { status: "completed", prompt: "done" }, + ), + ]) + // Two launches say launched; only the `completed` status says completed. + expect(html.match(/Launched Subagent/g)).toHaveLength(2) + expect(html).toContain("Completed Subagent") + expect(html).toMatchSnapshot() + }) + + it("keeps a nested subagent's descendants under it instead of orphaning them", () => { + const html = renderParts([ + tool("tool-Agent", "toolu_agent_1", { + subagent_type: "general-purpose", + description: "Outer agent", + }), + tool("tool-Agent", "toolu_agent_1:toolu_agent_2", { + subagent_type: "general-purpose", + description: "Inner agent", + }), + tool("tool-Read", "toolu_agent_2:toolu_read_9", { file_path: "/repo/nested.txt" }), + ]) + // The parent is resolved through the full id map: no top-level task is + // named `toolu_agent_2`, and before that lookup existed the Read stood up + // a fake "Incomplete task" instead of living under the inner agent. + expect(html).not.toContain("Incomplete task") + expect(html).toMatchSnapshot() + }) + + it("keeps a non-actionable subtitle out of the tab order", () => { + const container = renderPartsDom([ + tool("tool-TaskOutput", "toolu_taskoutput_1", { task_id: "task_1" }, { output: "done" }), + ]) + // TaskOutput's subtitle identifies the output and does nothing else, so + // it must be plain text: no role, no tab stop. (The positive case — a + // subtitle with an action — is pinned in agent-tool-call.test.tsx; here + // the transcript renders without the file-open provider, so no row would + // have an action to assert.) + const inert = Array.from(container.querySelectorAll("span")).find( + (s) => s.textContent === "Task: task_1", + ) + expect(inert).toBeTruthy() + expect(inert?.getAttribute("role")).toBeNull() + expect(inert?.getAttribute("tabindex")).toBeNull() + }) + + it("renders a three-level subagent chain once each level is expanded", () => { + const container = renderPartsDom([ + tool("tool-Agent", "toolu_agent_1", { + subagent_type: "general-purpose", + description: "Outer agent", + }), + tool("tool-Agent", "toolu_agent_1:toolu_agent_2", { + subagent_type: "general-purpose", + description: "Inner agent", + }), + tool("tool-Read", "toolu_agent_2:toolu_read_9", { file_path: "/repo/nested.txt" }), + ]) + const clickHeader = (needle: string) => { + const header = Array.from(container.querySelectorAll('[role="button"]')).find( + (el) => el.textContent?.includes(needle), + ) + expect(header, `a header containing ${needle}`).toBeTruthy() + fireEvent.click(header as HTMLElement) + } + clickHeader("Outer agent") + clickHeader("Inner agent") + // The Read row's subtitle is the file's basename — proof the descendant + // renders under the inner agent rather than at the top level or nowhere. + expect(container.textContent).toContain("nested.txt") + }) + + it("renders an MCP tool call", () => { + expect( + renderParts([ + tool("tool-mcp__filesystem__read_file", "toolu_mcp_1", { path: "/tmp/pins.json" }), + ]), + ).toMatchSnapshot() + }) + + it("renders a tool nobody registered as its bare name", () => { + expect( + renderParts([tool("tool-SomethingTheNextCliAdds", "toolu_future_1", {})]), + ).toMatchSnapshot() + }) + + it("collapses the steps under a final text part", () => { + expect( + renderParts([ + tool( + "tool-Bash", + "toolu_collapse_1", + { command: "bun install --frozen-lockfile" }, + { stdout: "", exitCode: 0 }, + ), + tool("tool-Read", "toolu_collapse_2", { file_path: "/repo/bun.lock" }), + { type: "text", text: "The lockfile is clean." }, + ]), + ).toMatchSnapshot() + }) + + it("keeps every part visible while the last message streams", () => { + expect( + renderParts( + [ + tool( + "tool-Bash", + "toolu_stream_1", + { command: "bun run lint" }, + { stdout: "", exitCode: 0 }, + ), + { type: "text", text: "Lint is clean so far." }, + ], + true, + ), + ).toMatchSnapshot() + }) + + it("groups three consecutive exploring tools", () => { + expect( + renderParts([ + tool("tool-Read", "toolu_explore_1", { file_path: "/repo/src/a.ts" }), + tool("tool-Read", "toolu_explore_2", { file_path: "/repo/src/b.ts" }), + tool("tool-Read", "toolu_explore_3", { file_path: "/repo/src/c.ts" }), + { type: "text", text: "Read all three." }, + ]), + ).toMatchSnapshot() + }) + + it("renders usage and git badges from message metadata", () => { + const message: Message = { + id: `msg-${++messageSequence}`, + role: "assistant", + parts: [{ type: "text", text: "Done." }], + metadata: { + inputTokens: 1200, + outputTokens: 340, + cacheReadInputTokens: 800, + cacheCreationInputTokens: 0, + }, + } + expect(renderMessage(message, false)).toMatchSnapshot() + }) +}) + +describe("AssistantMessageItem, the outer memo against in-place mutation", () => { + /** + * Round 8's claim, verified: `areMessagePropsEqual` snapshots text lengths, + * every part's state, and — before this round — only the last part's input, + * so a NON-last nested tool mutating its input in place with an unchanged + * state let the outer memo skip the render, and every row memo behind it + * (fingerprint included) never ran. The nested Read below prints its file + * name from its input; a trailing tool keeps it out of the last-part slot, + * which is exactly the hole the old snapshot had. + */ + it("re-renders when a completed question's result is replaced", () => { + // CodeAnt 4104721522: the ask card draws its answer from part.result. + // Replacing that reference with state, input, and output untouched must + // still pass the outer memo, or the card keeps the old answer forever. + const part: MessagePart = { + type: "tool-AskUserQuestion", + toolCallId: "toolu_result_swap", + state: "result", + input: { + questions: [ + { + question: "Which pin?", + header: "Pin", + multiSelect: false, + options: [ + { label: "0.3.270", description: "Matches the CLI pin." }, + { label: "0.3.280", description: "One release ahead." }, + ], + }, + ], + }, + result: { answers: { "Which pin?": "0.3.270" } }, + } as MessagePart + const message: Message = { id: `msg-${++messageSequence}`, role: "assistant", parts: [part] } + + const ui = () => ( + + + + ) + + const { container, rerender } = render(ui()) + expect(container.innerHTML).toContain("0.3.270") + + // Prime the per-message snapshot with the answered state. + rerender(ui()) + + // The sync assigns a new result object; state, input, and output never + // move, so only a snapshot that compares `result` can see this. + part.result = { answers: { "Which pin?": "0.3.280" } } as MessagePart["result"] + rerender(ui()) + + expect(container.innerHTML).toContain("0.3.280") + expect(container.innerHTML).not.toContain("0.3.270") + }) + + it("re-renders when a non-last nested tool mutates its input in place", () => { + const child: MessagePart = tool("tool-Read", "A:B", { file_path: "src/one.ts" }) + const parts: MessagePart[] = [ + { type: "text", text: "Working on it." }, + tool( + "tool-Task", + "A", + { subagent_type: "Explore", description: "walk" }, + { status: "completed" }, + ), + child, + // Trailing tool: the last part's input is unchanged by the mutation + // below, so the old last-part-only tracking has nothing to see. + tool("tool-Bash", "Z", { command: "ls" }, { stdout: "", exitCode: 0 }), + ] + const message: Message = { id: `msg-${++messageSequence}`, role: "assistant", parts } + + // A fresh element per render: reusing one element object would let React + // bail on identical props before the memo comparator is ever consulted. + const ui = () => ( + + + + ) + + const { container, rerender } = render(ui()) + + // Expand the task row so the nested Read is in the DOM at all. + const toggle = container.querySelector('[role="button"][aria-expanded]') + expect(toggle).not.toBeNull() + fireEvent.click(toggle as Element) + expect(container.innerHTML).toContain("one.ts") + + // Prime the per-message snapshot with the pre-mutation state: the first + // comparison caches, whichever branch it takes. + rerender(ui()) + + // The AI SDK mutates the part in place: same part object, same + // "output-available" state, new input value. + child.input = { file_path: "src/two.ts" } + rerender(ui()) + + expect(container.innerHTML).toContain("two.ts") + expect(container.innerHTML).not.toContain("one.ts") + }) +}) diff --git a/src/renderer/features/agents/main/assistant-message-item.tsx b/src/renderer/features/agents/main/assistant-message-item.tsx index 55ba0486..a8fae65b 100644 --- a/src/renderer/features/agents/main/assistant-message-item.tsx +++ b/src/renderer/features/agents/main/assistant-message-item.tsx @@ -2,7 +2,16 @@ import { useAtomValue } from "jotai" import { ListTree, MoreHorizontal } from "lucide-react" -import { memo, useCallback, useContext, useEffect, useMemo, useRef, useState } from "react" +import { + memo, + type ReactNode, + useCallback, + useContext, + useEffect, + useMemo, + useRef, + useState, +} from "react" import { normalizeCodexToolPart } from "../../../../shared/codex-tool-normalizer" import { DropdownMenu, @@ -19,6 +28,7 @@ import { cn } from "../../../lib/utils" import { selectedProjectAtom, showMessageJsonAtom } from "../atoms" import { isAssistantMessageQuestion } from "../lib/is-question" import { playQuestionSound } from "../lib/play-question-sound" +import { isSubagentToolType } from "../lib/subagent-tool-types" import { useFileOpen } from "../mentions" import type { Message } from "../stores/message-store" import { @@ -44,7 +54,8 @@ import { parseMcpToolType, type ToolDisplayPart, } from "../ui/agent-tool-registry" -import { isPlanFile } from "../ui/agent-tool-utils" +import { isTerminalStateString } from "../ui/agent-tool-state" +import { isPlanFile, nestingFingerprintOf } from "../ui/agent-tool-utils" import { AgentWebFetchTool } from "../ui/agent-web-fetch-tool" import { AgentWebSearchCollapsible } from "../ui/agent-web-search-collapsible" import { GitActivityBadges } from "../ui/git-activity-badges" @@ -243,6 +254,82 @@ function normalizeAcpParts(parts: unknown[]): NormalizedPart[] { }) } +type NestingIndex = { + nestedToolsMap: Map + nestedToolIds: Set + orphanTaskGroups: Map + orphanToolCallIds: Set + orphanFirstToolCallIds: Set +} + +/** + * Which parts nest under which subagent, and which ones lost their parent. + * + * A composite id is `parentOriginal:childOriginal` — the SDK names a child's + * parent by that parent's ORIGINAL tool id, never by the parent's own + * composite — so a nested task's original id is its last segment, and it is + * what the task's own children carry before the colon. Looking the first + * segment up in the top-level ids alone (what this did before) finds `A` under + * `A:B`, but orphans everything under `A:B`, because no top-level task is ever + * named just `B`. + * + * Module scope so the useMemo above it is one line and this function owns its + * own complexity: the dispatcher's cognitive-complexity budget is for the + * render branches, not for bookkeeping the pure rules already test. + */ +function buildNestingIndex(messageParts: NormalizedPart[]): NestingIndex { + const nestedToolsMap = new Map() + const nestedToolIds = new Set() + const taskParts = messageParts.filter( + (p): p is NormalizedPart & { toolCallId: string } => + isSubagentToolType(p.type) && !!p.toolCallId, + ) + const taskFullIdByOriginalId = new Map() + for (const task of taskParts) { + const segments = task.toolCallId.split(":") + taskFullIdByOriginalId.set(segments.at(-1) ?? task.toolCallId, task.toolCallId) + } + const orphanTaskGroups = new Map() + const orphanToolCallIds = new Set() + const orphanFirstToolCallIds = new Set() + + for (const part of messageParts) { + if (!part.toolCallId?.includes(":")) continue + const parentOriginalId = part.toolCallId.split(":")[0] + const parentFullId = + parentOriginalId === undefined ? undefined : taskFullIdByOriginalId.get(parentOriginalId) + // The self check is the cycle guard: a part that names itself as its + // own parent would otherwise sit in its own children forever. + if (parentFullId !== undefined && parentFullId !== part.toolCallId) { + // Keyed by the parent's FULL id: that is the id `renderSubagentTask` + // looks children up by, whether the parent sits at the top level + // (`A`) or inside another task (`A:B`). + if (!nestedToolsMap.has(parentFullId)) { + nestedToolsMap.set(parentFullId, []) + } + nestedToolsMap.get(parentFullId)?.push(part) + nestedToolIds.add(part.toolCallId) + continue + } + let group = orphanTaskGroups.get(parentOriginalId ?? "") + if (!group) { + group = { parts: [], firstToolCallId: part.toolCallId } + orphanTaskGroups.set(parentOriginalId ?? "", group) + orphanFirstToolCallIds.add(part.toolCallId) + } + group.parts.push(part) + orphanToolCallIds.add(part.toolCallId) + } + + return { + nestedToolsMap, + nestedToolIds, + orphanTaskGroups, + orphanToolCallIds, + orphanFirstToolCallIds, + } +} + // Exploring tools - these get grouped when 3+ consecutive const EXPLORING_TOOLS = new Set([ "tool-Read", @@ -480,10 +567,37 @@ export interface AssistantMessageItemProps { // Cache for tracking previous message state per sub-chat/message // (to detect AI SDK in-place mutations without cross-chat collisions) // Stores both text lengths and tool states for complete change detection +interface PartIOSnapshot { + state: string | undefined + input: unknown + output: unknown + // The rendered fields beyond IO: the ask card draws its answers from + // `result` and its failure line from `errorText`/`error`, so a change + // in any of them with state, input, and output untouched still has to + // pass the comparison below. + result: unknown + error: unknown + errorText: unknown + json: string | undefined +} + interface MessageStateSnapshot { textLengths: number[] partStates: (string | undefined)[] - lastPartInputJson: string | undefined + /** + * Every part's input, output, result, error, and errorText, stringified — + * but only once per state of the part. A nested tool can mutate its IO in + * place while its state and + * every text length around it stay unchanged, and nothing downstream of + * this memo runs when it skips a render, so the check has to see it. The + * cost stays bounded because a part whose state string is terminal and + * whose input/output references are unchanged is SETTLED: the SDK does not + * reopen a completed part, so its cached string still describes it and the + * comparison reuses it in O(1). Only live parts (streaming input, growing + * output) serialize per comparison, bounded by the active tool's payload + * rather than the whole transcript's — the round-9 reviews' point. + */ + partIO: PartIOSnapshot[] } const messageStateCache = new Map() @@ -543,19 +657,50 @@ function areMessagePropsEqual( // Get current message state from parts const nextParts = next.message?.parts || [] - const lastPart = nextParts[nextParts.length - 1] + + // Read the previous snapshot first: the per-part IO check below reuses its + // strings for settled parts instead of serializing them again. + const cachedState = cacheKey ? messageStateCache.get(cacheKey) : undefined const currentState: MessageStateSnapshot = { textLengths: nextParts.map((p) => getTrackedPartTextLength(p)), // Track ALL part states - critical for detecting Edit plan file streaming! partStates: nextParts.map((p) => p.state), - // Track tool input changes - this is critical for tool streaming! - lastPartInputJson: lastPart?.input ? JSON.stringify(lastPart.input) : undefined, + // Track every part's input AND output — tool streaming arrives as in-place + // mutation of both, on non-last parts too (parallel calls, nested tools). + partIO: nextParts.map((p, i) => { + const prev = cachedState?.partIO?.[i] + if ( + prev !== undefined && + prev.state === p.state && + prev.input === p.input && + prev.output === p.output && + prev.result === p.result && + prev.error === p.error && + prev.errorText === p.errorText && + isTerminalStateString(prev.state) + ) { + return prev // settled: same terminal state, same references + } + return { + state: p.state, + input: p.input, + output: p.output, + result: p.result, + error: p.error, + errorText: p.errorText, + json: + p.input === undefined && + p.output === undefined && + p.result === undefined && + p.error === undefined && + p.errorText === undefined + ? undefined + : JSON.stringify([p.input, p.output, p.result, p.error, p.errorText]), + } + }), } - // Get cached state from previous render - const cachedState = cacheKey ? messageStateCache.get(cacheKey) : undefined - // If no cache, this is first comparison - cache and allow render if (!cachedState || !cacheKey) { if (cacheKey) messageStateCache.set(cacheKey, currentState) @@ -576,10 +721,14 @@ function areMessagePropsEqual( } } - // Compare last part's input (detects tool input streaming!) - if (cachedState.lastPartInputJson !== currentState.lastPartInputJson) { - messageStateCache.set(cacheKey, currentState) - return false // Tool input changed + // Compare every part's input/output (detects in-place tool streaming the + // state and text-length checks cannot see). Settled parts carried their + // cached string over above, so this is a reference compare for them. + for (let i = 0; i < currentState.partIO.length; i++) { + if (cachedState.partIO?.[i]?.json !== currentState.partIO[i].json) { + messageStateCache.set(cacheKey, currentState) + return false // A part's input or output changed + } } // Compare ALL part states (detects Edit plan file streaming!) @@ -590,10 +739,397 @@ function areMessagePropsEqual( } } - // Nothing changed - skip re-render + // Nothing changed - skip re-render. The snapshot still advances: a history + // refresh can hand back fresh part objects whose content is identical, and + // without recording them every following comparison would re-serialize the + // whole transcript looking for a change that is not there — the exact + // per-tick cost the settled-part rule exists to stop, resurfacing. + messageStateCache.set(cacheKey, currentState) return true } +/** One plan-file operation this message carries, in the order it arrived. */ +type PlanOperation = { type: "write" | "edit"; part: NormalizedPart; index: number } + +/** What the plan-file pass found across the whole message. */ +type PlanOpsSummary = { + operations: PlanOperation[] + hasAnyPlanOperation: boolean + isStreaming: boolean + lastOperationType: "write" | "edit" | null +} + +/** + * Everything a part renderer reads that is not the part itself: this message's + * identity and stream state, the grouping its parts implied, and how it + * collapses. One object instead of eighteen closure reads, which is what lets + * the dispatch and the renderers live at module scope, be read one branch at a + * time, and be tested without the component that owns the values. + */ +type PartRenderContext = { + messageId: string + status: string + isStreaming: boolean + isLastMessage: boolean + subChatId: string + projectPath: string | undefined + onOpenFile: ReturnType + nestedToolsMap: Map + /** Children of any subagent, by the subagent's full composite id. */ + nestedChildren: (toolCallId: string) => NormalizedPart[] + /** + * This render's snapshot of the whole nesting map, for the task-row memo. + * A plain string so every row compares the same immutable value instead of + * walking the map through the shared tool-state cache (which would let the + * first row consume a grandchild mutation for all the others). + */ + nestingFingerprint: string + nestedToolIds: Set + /** Nested calls whose parent task part never arrived. */ + orphans: { + toolCallIds: Set + firstToolCallIds: Set + taskGroups: Map + } + planOps: PlanOpsSummary + collapse: { + shouldCollapse: boolean + collapseBeforeIndex: number + visibleStepsCount: number + lastCollapsedPlanOp: PlanOperation | null + } +} + +type PartRenderer = (part: NormalizedPart, idx: number, ctx: PartRenderContext) => ReactNode + +/** + * A nested call under a parent that never arrived, which is not the first of its + * group: the first one stands in for the missing parent and renders the rest + * inside itself, so the others are suppressed where they sit. + */ +function isSuppressedOrphan(part: NormalizedPart, ctx: PartRenderContext): boolean { + const { toolCallIds, firstToolCallIds } = ctx.orphans + if (!part.toolCallId || !toolCallIds.has(part.toolCallId)) return false + return !firstToolCallIds.has(part.toolCallId) +} + +/** The incomplete task the first orphaned nested call of a group stands in for. */ +function renderOrphanTaskGroup( + part: NormalizedPart, + idx: number, + ctx: PartRenderContext, +): ReactNode { + const { toolCallIds, firstToolCallIds, taskGroups } = ctx.orphans + if (!part.toolCallId || !toolCallIds.has(part.toolCallId)) return null + if (!firstToolCallIds.has(part.toolCallId)) return null + const parentId = part.toolCallId.split(":")[0] + const group = taskGroups.get(parentId) + if (!group) return null + return ( + + ) +} + +function renderTextPart( + part: NormalizedPart, + idx: number, + isFinal: boolean, + ctx: PartRenderContext, +): ReactNode { + const { messageId, isLastMessage, isStreaming, collapse } = ctx + const { collapseBeforeIndex, visibleStepsCount } = collapse + if (!part.text?.trim()) return null + const isFinalText = isFinal && idx === collapseBeforeIndex + const isTextStreaming = isLastMessage && isStreaming + return ( + + ) +} + +function renderSubagentTask(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + const nestedTools = ctx.nestedToolsMap.get(part.toolCallId ?? "") || [] + return ( + + ) +} + +function renderBashTool(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + return ( + + ) +} + +function renderThinkingTool(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + return ( + + ) +} + +/** A Write or Edit whose target is a plan file, which the transcript shows as plan steps. */ +function isPlanOperationPart(part: NormalizedPart): boolean { + if (part.type !== "tool-Write" && part.type !== "tool-Edit") return false + const toolInput = part.input as { file_path?: string } | null | undefined + return isPlanFile(toolInput?.file_path || "") +} + +/** + * What a plan operation's one-line indicator says: the verb its own tool carries, + * in the tense the stream state asks for. Four strings behind two questions, + * which reads as a table here and as a ternary inside a ternary inside JSX there. + */ +function planOperationLabel(isWrite: boolean, isOpStreaming: boolean): string { + if (isOpStreaming) return isWrite ? "Creating plan..." : "Updating plan..." + return isWrite ? "Created plan" : "Updated plan" +} + +/** + * Plan files: unified handling + * - In collapsed steps: all show mini indicator, last collapsed op's card shown separately after finalParts + * - In final parts: all but last show mini indicator, last shows full card + */ +function renderPlanOperation(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + const { planOps, status, subChatId, isStreaming, isLastMessage, collapse } = ctx + const { shouldCollapse, collapseBeforeIndex, lastCollapsedPlanOp } = collapse + + // Use part.toolCallId to find operation since idx may be adjusted for collapsed parts + const opIndex = planOps.operations.findIndex((op) => op.part.toolCallId === part.toolCallId) + if (opIndex === -1) return null + + const originalIndex = planOps.operations[opIndex]?.index ?? -1 + const isInCollapsedSteps = + shouldCollapse && collapseBeforeIndex !== -1 && originalIndex < collapseBeforeIndex + const isLastCollapsedOp = lastCollapsedPlanOp?.part.toolCallId === part.toolCallId + const isLastOperation = opIndex === planOps.operations.length - 1 + + // If this is the last collapsed plan op, hide it here (card shown after CollapsibleSteps) + if (isInCollapsedSteps && isLastCollapsedOp) { + return null + } + + // Show mini indicator for: + // - All operations in collapsed steps (except last collapsed, handled above) + // - All operations except last in final parts + const showMiniIndicator = isInCollapsedSteps || !isLastOperation + + if (showMiniIndicator) { + const isWrite = part.type === "tool-Write" + const { isPending } = getToolStatus(part, status) + const isOpStreaming = + isPending || (part.state === "input-streaming" && isStreaming && isLastMessage) + const label = planOperationLabel(isWrite, isOpStreaming) + + return ( +
+ + {isOpStreaming ? ( + + {label} + + ) : ( + label + )} + +
+ ) + } + + // Last operation in final parts: show full card + return ( + + ) +} + +/** A file edit that is not a plan step: Write and Edit render the same card. */ +function renderFileEditTool(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + return ( + + ) +} + +function renderWebSearch(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + return +} + +function renderWebFetch(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + return +} + +function renderPlanWrite(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + return ( + + ) +} + +function renderTodoList(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + return ( + + ) +} + +function renderQuestionTool(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + const { isPending, isError } = getToolStatus(part, ctx.status) + return ( + + ) +} + +/** A tool the registry knows: one row, clickable when it was a file read. */ +function renderRegistryTool(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + const { onOpenFile, projectPath, status } = ctx + const meta = AgentToolRegistry[part.type] + const { isPending, isError } = getToolStatus(part, status) + // Make Read tool clickable to open file in viewer + // Capture the path at render: part objects can be mutated in place during streaming. + const toolInput = part.input as { file_path?: string } | null | undefined + const readFilePath = + part.type === "tool-Read" && onOpenFile ? (toolInput?.file_path ?? null) : null + const handleClick = readFilePath && onOpenFile ? () => onOpenFile(readFilePath) : undefined + return ( + + ) +} + +/** A tool nobody registered: an MCP call by its server and tool, else its bare name. */ +function renderUnlistedTool(part: NormalizedPart, idx: number, ctx: PartRenderContext): ReactNode { + // MCP tool calls (pattern: tool-mcp____) + const mcpInfo = parseMcpToolType(part.type) + if (mcpInfo) { + return + } + + if (part.type?.startsWith("tool-")) { + return ( +
+ {part.type.replace("tool-", "")} +
+ ) + } + + return null +} + +/** + * The part types with a renderer of their own. Order does not matter here + * because the keys are distinct; what does matter is that the dispatcher consults + * this table only after the shapes that claim a type before its own renderer — + * a sub-agent task, and a Write or Edit aimed at a plan file. + */ +const PART_RENDERERS: Record = { + "tool-Bash": renderBashTool, + reasoning: renderThinkingTool, + "tool-Thinking": renderThinkingTool, + "tool-Write": renderFileEditTool, + "tool-Edit": renderFileEditTool, + "tool-WebSearch": renderWebSearch, + "tool-WebFetch": renderWebFetch, + "tool-PlanWrite": renderPlanWrite, + // ExitPlanMode tool is hidden - plan is shown in sidebar instead + "tool-ExitPlanMode": () => null, + "tool-TodoWrite": renderTodoList, + "tool-AskUserQuestion": renderQuestionTool, +} + +/** + * What one part of a message looks like, decided in the order the transcript has + * always decided it: the suppressions first, then text, then the two shapes that + * claim a type before its own renderer does, then the table, then the registry, + * then the shapes nobody registered. + */ +function renderMessagePart( + part: NormalizedPart, + idx: number, + isFinal: boolean, + ctx: PartRenderContext, +): ReactNode { + if (part.type === "step-start") return null + if (isSuppressedOrphan(part, ctx)) return null + + const orphanTask = renderOrphanTaskGroup(part, idx, ctx) + if (orphanTask) return orphanTask + + if (part.toolCallId && ctx.nestedToolIds.has(part.toolCallId)) return null + if (part.type === "exploring-group") return null + if (part.type === "text") return renderTextPart(part, idx, isFinal, ctx) + if (isSubagentToolType(part.type)) return renderSubagentTask(part, idx, ctx) + if (isPlanOperationPart(part)) return renderPlanOperation(part, idx, ctx) + + const renderer = PART_RENDERERS[part.type] + if (renderer) return renderer(part, idx, ctx) + if (part.type in AgentToolRegistry) return renderRegistryTool(part, idx, ctx) + return renderUnlistedTool(part, idx, ctx) +} + export const AssistantMessageItem = memo(function AssistantMessageItem({ message, isLastMessage, @@ -631,51 +1167,7 @@ export const AssistantMessageItem = memo(function AssistantMessageItem({ orphanTaskGroups, orphanToolCallIds, orphanFirstToolCallIds, - } = useMemo(() => { - const nestedToolsMap = new Map() - const nestedToolIds = new Set() - const taskPartIds = new Set( - messageParts - .filter( - (p): p is NormalizedPart & { toolCallId: string } => - p.type === "tool-Task" && !!p.toolCallId, - ) - .map((p) => p.toolCallId), - ) - const orphanTaskGroups = new Map() - const orphanToolCallIds = new Set() - const orphanFirstToolCallIds = new Set() - - for (const part of messageParts) { - if (part.toolCallId?.includes(":")) { - const parentId = part.toolCallId.split(":")[0] - if (taskPartIds.has(parentId)) { - if (!nestedToolsMap.has(parentId)) { - nestedToolsMap.set(parentId, []) - } - nestedToolsMap.get(parentId)?.push(part) - nestedToolIds.add(part.toolCallId) - } else { - let group = orphanTaskGroups.get(parentId) - if (!group) { - group = { parts: [], firstToolCallId: part.toolCallId } - orphanTaskGroups.set(parentId, group) - orphanFirstToolCallIds.add(part.toolCallId) - } - group.parts.push(part) - orphanToolCallIds.add(part.toolCallId) - } - } - } - - return { - nestedToolsMap, - nestedToolIds, - orphanTaskGroups, - orphanToolCallIds, - orphanFirstToolCallIds, - } - }, [messageParts]) + } = useMemo(() => buildNestingIndex(messageParts), [messageParts]) // Collect all plan operations (Write/Edit) for unified handling const planOpsSummary = useMemo(() => { @@ -746,7 +1238,6 @@ export const AssistantMessageItem = memo(function AssistantMessageItem({ shouldCollapse && collapseBeforeIndex !== -1 ? messageParts.slice(0, collapseBeforeIndex) : [] const visibleStepsCount = stepParts.filter((p) => { if (p.type === "step-start") return false - if (p.type === "tool-TaskOutput") return false if (p.type === "tool-ExitPlanMode") return false if (p.toolCallId && nestedToolIds.has(p.toolCallId)) return false if ( @@ -797,244 +1288,46 @@ export const AssistantMessageItem = memo(function AssistantMessageItem({ [messageParts], ) - const msgMetadata = message?.metadata as AgentMessageMetadata - - const renderPart = useCallback( - (part: NormalizedPart, idx: number, isFinal = false) => { - const toolInput = part.input as { file_path?: string } | null | undefined - if (part.type === "step-start") return null - if (part.type === "tool-TaskOutput") return null - - if (part.toolCallId && orphanToolCallIds.has(part.toolCallId)) { - if (!orphanFirstToolCallIds.has(part.toolCallId)) return null - const parentId = part.toolCallId.split(":")[0] - const group = orphanTaskGroups.get(parentId) - if (group) { - return ( - - ) - } - } - - if (part.toolCallId && nestedToolIds.has(part.toolCallId)) return null - if (part.type === "exploring-group") return null - - if (part.type === "text") { - if (!part.text?.trim()) return null - const isFinalText = isFinal && idx === collapseBeforeIndex - const isTextStreaming = isLastMessage && isStreaming - return ( - - ) - } - - if (part.type === "tool-Task") { - const nestedTools = nestedToolsMap.get(part.toolCallId ?? "") || [] - return - } - - if (part.type === "tool-Bash") - return ( - - ) - if (part.type === "reasoning" || part.type === "tool-Thinking") { - return ( - - ) - } - - // Plan files: unified handling - // - In collapsed steps: all show mini indicator, last collapsed op's card shown separately after finalParts - // - In final parts: all but last show mini indicator, last shows full card - if (part.type === "tool-Write" || part.type === "tool-Edit") { - const filePath = toolInput?.file_path || "" - if (isPlanFile(filePath)) { - // Use part.toolCallId to find operation since idx may be adjusted for collapsed parts - const opIndex = planOpsSummary.operations.findIndex( - (op) => op.part.toolCallId === part.toolCallId, - ) - if (opIndex === -1) return null - - const originalIndex = planOpsSummary.operations[opIndex]?.index ?? -1 - const isInCollapsedSteps = - shouldCollapse && collapseBeforeIndex !== -1 && originalIndex < collapseBeforeIndex - const isLastCollapsedOp = lastCollapsedPlanOp?.part.toolCallId === part.toolCallId - const isLastOperation = opIndex === planOpsSummary.operations.length - 1 - - // If this is the last collapsed plan op, hide it here (card shown after CollapsibleSteps) - if (isInCollapsedSteps && isLastCollapsedOp) { - return null - } - - // Show mini indicator for: - // - All operations in collapsed steps (except last collapsed, handled above) - // - All operations except last in final parts - const showMiniIndicator = isInCollapsedSteps || !isLastOperation - - if (showMiniIndicator) { - const isWrite = part.type === "tool-Write" - const { isPending } = getToolStatus(part, status) - const isOpStreaming = - isPending || (part.state === "input-streaming" && isStreaming && isLastMessage) - - return ( -
- - {isOpStreaming ? ( - - {isWrite ? "Creating plan..." : "Updating plan..."} - - ) : isWrite ? ( - "Created plan" - ) : ( - "Updated plan" - )} - -
- ) - } - - // Last operation in final parts: show full card - return ( - - ) - } - } - - if (part.type === "tool-Edit") - return ( - - ) - if (part.type === "tool-Write") - return ( - - ) - if (part.type === "tool-WebSearch") - return - if (part.type === "tool-WebFetch") - return - if (part.type === "tool-PlanWrite") - return ( - - ) - - // ExitPlanMode tool is hidden - plan is shown in sidebar instead - if (part.type === "tool-ExitPlanMode") { - return null - } - - if (part.type === "tool-TodoWrite") { - return ( - - ) - } - - if (part.type === "tool-AskUserQuestion") { - const { isPending, isError } = getToolStatus(part, status) - return ( - - ) - } + // One pure string per render for every task row's memo — see + // `nestingFingerprintOf`: comparing the map through arePartsEqual would + // advance the shared tool-state cache, and the first row to walk it would + // consume the change a later row needed to see. + const nestingFingerprint = useMemo(() => nestingFingerprintOf(nestedToolsMap), [nestedToolsMap]) - if (part.type in AgentToolRegistry) { - const meta = AgentToolRegistry[part.type] - const { isPending, isError } = getToolStatus(part, status) - // Make Read tool clickable to open file in viewer - // Capture the path at render: part objects can be mutated in place during streaming. - const readFilePath = - part.type === "tool-Read" && onOpenFile ? (toolInput?.file_path ?? null) : null - const handleClick = readFilePath && onOpenFile ? () => onOpenFile(readFilePath) : undefined - return ( - - ) - } - - // MCP tool calls (pattern: tool-mcp____) - const mcpInfo = parseMcpToolType(part.type) - if (mcpInfo) { - return - } - - if (part.type?.startsWith("tool-")) { - return ( -
- {part.type.replace("tool-", "")} -
- ) - } + const msgMetadata = message?.metadata as AgentMessageMetadata - return null - }, + // One context object, so the dispatch and every renderer it calls can live at + // module scope: this component says what this message's values are, and + // renderMessagePart decides what each part looks like. + const partContext = useMemo( + () => ({ + messageId: message.id, + status, + isStreaming, + isLastMessage, + subChatId, + projectPath, + onOpenFile, + nestedToolsMap, + nestedChildren: (toolCallId: string) => nestedToolsMap.get(toolCallId) ?? [], + nestingFingerprint, + nestedToolIds, + orphans: { + toolCallIds: orphanToolCallIds, + firstToolCallIds: orphanFirstToolCallIds, + taskGroups: orphanTaskGroups, + }, + planOps: planOpsSummary, + collapse: { + shouldCollapse, + collapseBeforeIndex, + visibleStepsCount, + lastCollapsedPlanOp, + }, + }), [ nestedToolsMap, + nestingFingerprint, nestedToolIds, orphanToolCallIds, orphanFirstToolCallIds, @@ -1054,6 +1347,12 @@ export const AssistantMessageItem = memo(function AssistantMessageItem({ ], ) + const renderPart = useCallback( + (part: NormalizedPart, idx: number, isFinal = false) => + renderMessagePart(part, idx, isFinal, partContext), + [partContext], + ) + // Detect when the assistant's final text part is a question awaiting user input. // Only treat as "question" once streaming has finished — partial text may not yet // include the trailing question mark. diff --git a/src/renderer/features/agents/main/chat-input-area.tsx b/src/renderer/features/agents/main/chat-input-area.tsx index 0930aee2..494cc360 100644 --- a/src/renderer/features/agents/main/chat-input-area.tsx +++ b/src/renderer/features/agents/main/chat-input-area.tsx @@ -9,7 +9,7 @@ */ import { useAtom, useAtomValue, useSetAtom } from "jotai" -import { ChevronDown, Zap } from "lucide-react" +import { ChevronDown, Sparkles, Zap } from "lucide-react" import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react" import { createPortal } from "react-dom" import { toast } from "sonner" @@ -31,20 +31,14 @@ import { import { agentsSettingsDialogActiveTabAtom, agentsSettingsDialogOpenAtom, - anthropicOnboardingCompletedAtom, - apiKeyOnboardingCompletedAtom, codexApiKeyAtom, codexOnboardingCompletedAtom, - customClaudeConfigAtom, customHotkeysAtom, - extendedThinkingEnabledAtom, hiddenModelsAtom, normalizeCodexApiKey, - normalizeCustomClaudeConfig, pinnedOpenRouterModelsAtom, - selectedOllamaModelAtom, + promptSuggestionsEnabledAtom, sessionInfoAtom, - showOfflineModeFeaturesAtom, } from "../../../lib/atoms" import { blobToBase64, @@ -83,18 +77,21 @@ import { subChatModelIdAtomFamily, subChatOpenclawModelIdAtomFamily, subChatOpenRouterModelIdAtomFamily, + subChatPromptSuggestionAtomFamily, subChatQwenModelIdAtomFamily, subChatRooModelIdAtomFamily, + subChatTurnGenerationAtomFamily, } from "../atoms" import { AgentsSlashCommand, type SlashCommandOption } from "../commands" import { AgentModelSelector, type AgentProviderId } from "../components/agent-model-selector" import { AgentSendButton } from "../components/agent-send-button" import type { UploadedFile, UploadedImage } from "../hooks/use-agents-file-upload" +import { useClaudeModelPicker } from "../hooks/use-claude-model-picker" import type { PastedTextFile } from "../hooks/use-pasted-text-files" +import { mergeDraftWithSuggestion } from "../lib/composer-text" import { clearSubChatDraft, saveSubChatDraftWithAttachments } from "../lib/drafts" import { getModeIcon, getModeLabel, getModeTooltip } from "../lib/mode-display" import { - CLAUDE_MODELS, CLINE_MODELS, CODEX_MODELS, CODEX_SUBSCRIPTION_ONLY_MODEL_IDS, @@ -107,6 +104,7 @@ import { ROO_MODELS, } from "../lib/models" import type { DiffTextContext, SelectedTextContext } from "../lib/queue-utils" +import { suggestionIsCurrent } from "../lib/suggestion-ownership" import { AgentsFileMention, AgentsMentionsEditor, @@ -124,44 +122,6 @@ import { AgentTextContextItem } from "../ui/agent-text-context-item" import { VoiceWaveIndicator } from "../ui/voice-wave-indicator" import { handlePasteEvent } from "../utils/paste-text" -// Hook to get available models (including offline models if Ollama is available and debug enabled) -function useAvailableModels() { - const showOfflineFeatures = useAtomValue(showOfflineModeFeaturesAtom) - const { data: ollamaStatus } = trpc.ollama.getStatus.useQuery(undefined, { - refetchInterval: showOfflineFeatures ? 30000 : false, - enabled: showOfflineFeatures, // Only query Ollama when offline mode is enabled - }) - - const baseModels = CLAUDE_MODELS - - const isOffline = ollamaStatus ? !ollamaStatus.internet.online : false - const hasOllama = ollamaStatus?.ollama.available && (ollamaStatus.ollama.models?.length ?? 0) > 0 - const ollamaModels = ollamaStatus?.ollama.models || [] - const recommendedModel = ollamaStatus?.ollama.recommendedModel - - // Only show offline models if: - // 1. Debug flag is enabled (showOfflineFeatures) - // 2. Ollama is available with models - // 3. User is actually offline - if (showOfflineFeatures && hasOllama && isOffline) { - return { - models: baseModels, - ollamaModels, - recommendedModel, - isOffline, - hasOllama: true, - } - } - - return { - models: baseModels, - ollamaModels: [] as string[], - recommendedModel: undefined as string | undefined, - isOffline, - hasOllama: false, - } -} - export interface ChatInputAreaProps { // Editor ref - passed from parent for external access editorRef: React.RefObject @@ -469,6 +429,21 @@ export const ChatInputArea = memo(function ChatInputArea({ // Model dropdown state const [isModelDropdownOpen, setIsModelDropdownOpen] = useState(false) + const promptSuggestionAtom = useMemo( + () => subChatPromptSuggestionAtomFamily(subChatId), + [subChatId], + ) + const [promptSuggestion, setPromptSuggestion] = useAtom(promptSuggestionAtom) + // The generation the composer is on: a stored suggestion names the turn + // that produced it, and one from an older turn — or the other engine — is + // not this composer's next step, whatever a late stream wrote. + const turnGeneration = useAtomValue( + useMemo(() => subChatTurnGenerationAtomFamily(subChatId), [subChatId]), + ) + // Turning the switch off withdraws whatever suggestion is already stored; + // the render gate asks for the preference so a re-enable cannot resurrect it. + const promptSuggestionsOn = useAtomValue(promptSuggestionsEnabledAtom) + const subChatModelIdAtom = useMemo(() => subChatModelIdAtomFamily(subChatId), [subChatId]) const [selectedSubChatModelId, setSelectedSubChatModelId] = useAtom(subChatModelIdAtom) const subChatCodexModelIdAtom = useMemo( @@ -550,22 +525,27 @@ export const ChatInputArea = memo(function ChatInputArea({ refetchOnWindowFocus: false, retry: 1, }) - const [selectedOllamaModel, setSelectedOllamaModel] = useAtom(selectedOllamaModelAtom) - const availableModels = useAvailableModels() - const [selectedModel, setSelectedModel] = useState( + const hiddenModels = useAtomValue(hiddenModelsAtom) + // The Claude half of the picker is shared with the new-chat form; which model + // is selected, and what selecting one does, belong to this surface alone. + const { + availableModels, + hasCustomClaudeConfig, + currentOllamaModel, + props: claudePickerProps, + } = useClaudeModelPicker(hiddenModels, subChatId) + // Derived from the visible list, the way every other provider derives its + // selection (`codexUiModels.find(...) || codexUiModels[0]` and the rest). Local + // state plus a sync effect kept a model that the picker no longer offers: hide + // one mid-session and it stayed selected, labelled and sent until the next + // mount, because the effect only ever moved towards a model it could find. + const selectedModel = useMemo( () => - availableModels.models.find((m) => m.id === selectedSubChatModelId) || + availableModels.models.find((model) => model.id === selectedSubChatModelId) || availableModels.models[0], + [availableModels.models, selectedSubChatModelId], ) - // Sync selectedModel when per-subChat atom value changes (e.g., after localStorage hydration) - useEffect(() => { - const model = availableModels.models.find((m) => m.id === selectedSubChatModelId) - if (model && model.id !== selectedModel.id) { - setSelectedModel(model) - } - }, [availableModels.models, selectedModel.id, selectedSubChatModelId]) - // Materialize the resolved Claude model into per-subChat storage once mounted. // This prevents later global default changes from affecting existing sub-chats. useEffect(() => { @@ -574,13 +554,8 @@ export const ChatInputArea = memo(function ChatInputArea({ setSelectedSubChatModelId(selectedModel.id) }, [provider, selectedModel?.id, setSelectedSubChatModelId]) - const hiddenModels = useAtomValue(hiddenModelsAtom) - // Connection status for providers - const anthropicOnboardingCompleted = useAtomValue(anthropicOnboardingCompletedAtom) - const apiKeyOnboardingCompleted = useAtomValue(apiKeyOnboardingCompletedAtom) const codexOnboardingCompleted = useAtomValue(codexOnboardingCompletedAtom) - const { data: claudeCodeIntegration } = trpc.claudeCode.getIntegration.useQuery() const { data: cursorIntegration } = trpc.cursor.getIntegration.useQuery() const { data: grokIntegration } = trpc.grok.getIntegration.useQuery() const { data: qwenIntegration } = trpc.qwen.getIntegration.useQuery() @@ -778,14 +753,6 @@ export const ChatInputArea = memo(function ChatInputArea({ setSelectedSubChatRooModelId(selectedRooModel.id) }, [provider, selectedRooModel?.id, setSelectedSubChatRooModelId]) - const customClaudeConfig = useAtomValue(customClaudeConfigAtom) - const normalizedCustomClaudeConfig = normalizeCustomClaudeConfig(customClaudeConfig) - const hasCustomClaudeConfig = Boolean(normalizedCustomClaudeConfig) - const isClaudeConnected = - Boolean(claudeCodeIntegration?.isConnected) || - anthropicOnboardingCompleted || - apiKeyOnboardingCompleted || - hasCustomClaudeConfig const isCursorConnected = Boolean(cursorIntegration?.isConnected) const isGrokConnected = Boolean(grokIntegration?.isConnected) const isQwenConnected = Boolean(qwenIntegration?.isConnected) @@ -793,13 +760,6 @@ export const ChatInputArea = memo(function ChatInputArea({ const isOpenclawConnected = Boolean(openclawIntegration?.isConnected) const isRooConnected = Boolean(rooIntegration?.isConnected) - // Determine current Ollama model (selected or recommended) - const currentOllamaModel = - selectedOllamaModel || availableModels.recommendedModel || availableModels.ollamaModels[0] - - // Extended thinking (reasoning) toggle - const [thinkingEnabled, setThinkingEnabled] = useAtom(extendedThinkingEnabledAtom) - const selectedModelLabel = useMemo(() => { if (provider === "codex") { return selectedCodexModel.name @@ -1215,9 +1175,7 @@ export const ChatInputArea = memo(function ChatInputArea({ if (result.text?.trim()) { const current = (editorRef.current?.getValue() || "").trim() const transcribed = result.text.trim() - const needsSpace = current.length > 0 && !/\s$/.test(current) - const newValue = current + (needsSpace ? " " : "") + transcribed - editorRef.current?.setValue(newValue) + editorRef.current?.setValue(mergeDraftWithSuggestion(current, transcribed)) editorRef.current?.focus() } else { toast.info("No speech detected") @@ -1869,6 +1827,38 @@ export const ChatInputArea = memo(function ChatInputArea({ ) : null } > + {suggestionIsCurrent(promptSuggestion, { + engineNow: engine, + turnNow: turnGeneration, + preferenceOn: promptSuggestionsOn, + }) && ( +
+ + + +
+ )}
!hiddenModels.includes(m.id)), + ...claudePickerProps, selectedModelId: selectedModel?.id, onSelectModel: (modelId) => { const model = availableModels.models.find((item) => item.id === modelId) || availableModels.models[0] if (!model) return - setSelectedModel(model) setSelectedSubChatModelId(model.id) setLastSelectedModelId(model.id) }, - hasCustomModelConfig: hasCustomClaudeConfig, - isOffline: availableModels.isOffline && availableModels.hasOllama, - ollamaModels: availableModels.ollamaModels, - selectedOllamaModel: currentOllamaModel, - recommendedOllamaModel: availableModels.recommendedModel, - onSelectOllamaModel: setSelectedOllamaModel, - isConnected: isClaudeConnected, - thinkingEnabled, - onThinkingChange: setThinkingEnabled, }} codex={{ models: codexUiModels, diff --git a/src/renderer/features/agents/main/new-chat-form.tsx b/src/renderer/features/agents/main/new-chat-form.tsx index 60227d28..cd8546fa 100644 --- a/src/renderer/features/agents/main/new-chat-form.tsx +++ b/src/renderer/features/agents/main/new-chat-form.tsx @@ -78,20 +78,13 @@ import { import { agentsSettingsDialogActiveTabAtom, agentsSettingsDialogOpenAtom, - anthropicOnboardingCompletedAtom, - apiKeyOnboardingCompletedAtom, chatSourceModeAtom, codexApiKeyAtom, codexOnboardingCompletedAtom, - customClaudeConfigAtom, customHotkeysAtom, - extendedThinkingEnabledAtom, hiddenModelsAtom, normalizeCodexApiKey, - normalizeCustomClaudeConfig, pinnedOpenRouterModelsAtom, - selectedOllamaModelAtom, - showOfflineModeFeaturesAtom, } from "../../../lib/atoms" import { blobToBase64, @@ -111,6 +104,7 @@ import { AgentModelSelector, type AgentProviderId } from "../components/agent-mo import { AgentSendButton } from "../components/agent-send-button" import { CreateBranchDialog } from "../components/create-branch-dialog" import { useAgentsFileUpload } from "../hooks/use-agents-file-upload" +import { useClaudeModelPicker } from "../hooks/use-claude-model-picker" import { useFocusInputOnEnter } from "../hooks/use-focus-input-on-enter" import { usePastedTextFiles } from "../hooks/use-pasted-text-files" import { useToggleFocusOnCmdEsc } from "../hooks/use-toggle-focus-on-cmd-esc" @@ -122,7 +116,6 @@ import { saveGlobalDrafts, } from "../lib/drafts" import { - CLAUDE_MODELS, CLINE_MODELS, CODEX_MODELS, CODEX_SUBSCRIPTION_ONLY_MODEL_IDS, @@ -149,44 +142,6 @@ import { VoiceWaveIndicator } from "../ui/voice-wave-indicator" import { formatTimeAgo } from "../utils/format-time-ago" import { handlePasteEvent } from "../utils/paste-text" -// Hook to get available models (including offline models if Ollama is available and debug enabled) -function useAvailableModels() { - const showOfflineFeatures = useAtomValue(showOfflineModeFeaturesAtom) - const { data: ollamaStatus } = trpc.ollama.getStatus.useQuery(undefined, { - refetchInterval: showOfflineFeatures ? 30000 : false, - enabled: showOfflineFeatures, // Only query Ollama when offline mode is enabled - }) - - const baseModels = CLAUDE_MODELS - - const isOffline = ollamaStatus ? !ollamaStatus.internet.online : false - const hasOllama = ollamaStatus?.ollama.available && (ollamaStatus.ollama.models?.length ?? 0) > 0 - const ollamaModels = ollamaStatus?.ollama.models || [] - const recommendedModel = ollamaStatus?.ollama.recommendedModel - - // Only show offline models if: - // 1. Debug flag is enabled (showOfflineFeatures) - // 2. Ollama is available with models - // 3. User is actually offline - if (showOfflineFeatures && hasOllama && isOffline) { - return { - models: baseModels, - ollamaModels, - recommendedModel, - isOffline, - hasOllama: true, - } - } - - return { - models: baseModels, - ollamaModels: [] as string[], - recommendedModel: undefined as string | undefined, - isOffline, - hasOllama: false, - } -} - // Agent providers const agents: { id: string @@ -263,25 +218,14 @@ export function NewChatForm({ isMobileFullscreen = false, onBackToChats }: NewCh }, []) const [workMode, setWorkMode] = useAtom(lastSelectedWorkModeAtom) const debugMode = useAtomValue(agentsDebugModeAtom) - const customClaudeConfig = useAtomValue(customClaudeConfigAtom) - const normalizedCustomClaudeConfig = normalizeCustomClaudeConfig(customClaudeConfig) - const hasCustomClaudeConfig = Boolean(normalizedCustomClaudeConfig) // Connection status for providers - const anthropicOnboardingCompleted = useAtomValue(anthropicOnboardingCompletedAtom) - const apiKeyOnboardingCompleted = useAtomValue(apiKeyOnboardingCompletedAtom) const codexOnboardingCompleted = useAtomValue(codexOnboardingCompletedAtom) - const { data: claudeCodeIntegration } = trpc.claudeCode.getIntegration.useQuery() const { data: cursorIntegration } = trpc.cursor.getIntegration.useQuery() const { data: grokIntegration } = trpc.grok.getIntegration.useQuery() const { data: qwenIntegration } = trpc.qwen.getIntegration.useQuery() const { data: clineIntegration } = trpc.cline.getIntegration.useQuery() const { data: openclawIntegration } = trpc.openclaw.getIntegration.useQuery() const { data: rooIntegration } = trpc.roo.getIntegration.useQuery() - const isClaudeConnected = - Boolean(claudeCodeIntegration?.isConnected) || - anthropicOnboardingCompleted || - apiKeyOnboardingCompleted || - hasCustomClaudeConfig const setSettingsDialogOpen = useSetAtom(agentsSettingsDialogOpenAtom) const setSettingsActiveTab = useSetAtom(agentsSettingsDialogActiveTabAtom) const setJustCreatedIds = useSetAtom(justCreatedIdsAtom) @@ -345,9 +289,15 @@ export function NewChatForm({ isMobileFullscreen = false, onBackToChats }: NewCh } }, [enabledAgents, fallbackAgent, lastSelectedAgentId, selectedAgent.id]) - // Get available models (with offline support) - const availableModels = useAvailableModels() - const [selectedOllamaModel, setSelectedOllamaModel] = useAtom(selectedOllamaModelAtom) + const hiddenModels = useAtomValue(hiddenModelsAtom) + // The Claude half of the picker is shared with the chat composer; which model + // is selected, and what selecting one does, belong to this surface alone. + const { + availableModels, + hasCustomClaudeConfig, + currentOllamaModel, + props: claudePickerProps, + } = useClaudeModelPicker(hiddenModels) const [lastSelectedCodexModelId, setLastSelectedCodexModelId] = useAtom( lastSelectedCodexModelIdAtom, ) @@ -369,7 +319,6 @@ export function NewChatForm({ isMobileFullscreen = false, onBackToChats }: NewCh const [lastSelectedGeminiModelId, setLastSelectedGeminiModelId] = useAtom( lastSelectedGeminiModelIdAtom, ) - const [thinkingEnabled, setThinkingEnabled] = useAtom(extendedThinkingEnabledAtom) const { data: geminiAuth } = trpc.gemini.getAuthStatus.useQuery() const { data: geminiCliStatus } = trpc.gemini.getCliStatus.useQuery() const isGeminiConnected = @@ -389,20 +338,15 @@ export function NewChatForm({ isMobileFullscreen = false, onBackToChats }: NewCh retry: 1, }) - const [selectedModel, setSelectedModel] = useState( + // Derived from the visible list, as the other nine providers derive theirs, + // so a model hidden in settings cannot stay selected for the next chat. + const selectedModel = useMemo( () => - availableModels.models.find((m) => m.id === lastSelectedModelId) || availableModels.models[0], + availableModels.models.find((model) => model.id === lastSelectedModelId) || + availableModels.models[0], + [availableModels.models, lastSelectedModelId], ) - // Sync selectedModel when atom value changes (e.g., after localStorage hydration) - useEffect(() => { - const model = availableModels.models.find((m) => m.id === lastSelectedModelId) - if (model && model.id !== selectedModel.id) { - setSelectedModel(model) - } - }, [lastSelectedModelId, selectedModel.id, availableModels.models.find]) - - const hiddenModels = useAtomValue(hiddenModelsAtom) const storedCodexApiKey = useAtomValue(codexApiKeyAtom) const hasAppCodexApiKey = Boolean(normalizeCodexApiKey(storedCodexApiKey)) const codexUiModels = useMemo(() => { @@ -588,9 +532,6 @@ export function NewChatForm({ isMobileFullscreen = false, onBackToChats }: NewCh selectedModel?.id, ]) - // Determine current Ollama model (selected or recommended) - const currentOllamaModel = - selectedOllamaModel || availableModels.recommendedModel || availableModels.ollamaModels[0] const claudeAgent = enabledAgents.find((agent) => agent.id === "claude-code") || fallbackAgent const selectedModelLabel = useMemo(() => { if (selectedAgent.id === "codex") { @@ -2189,27 +2130,15 @@ export function NewChatForm({ isMobileFullscreen = false, onBackToChats }: NewCh setSettingsDialogOpen(true) }} claude={{ - models: availableModels.models.filter( - (m) => !hiddenModels.includes(m.id), - ), + ...claudePickerProps, selectedModelId: selectedModel?.id, onSelectModel: (modelId) => { const model = availableModels.models.find((m) => m.id === modelId) || availableModels.models[0] if (!model) return - setSelectedModel(model) setLastSelectedModelId(model.id) }, - hasCustomModelConfig: hasCustomClaudeConfig, - isOffline: availableModels.isOffline && availableModels.hasOllama, - ollamaModels: availableModels.ollamaModels, - selectedOllamaModel: currentOllamaModel, - recommendedOllamaModel: availableModels.recommendedModel, - onSelectOllamaModel: setSelectedOllamaModel, - isConnected: isClaudeConnected, - thinkingEnabled, - onThinkingChange: setThinkingEnabled, }} codex={{ models: codexUiModels, diff --git a/src/renderer/features/agents/ui/agent-task-tool.tsx b/src/renderer/features/agents/ui/agent-task-tool.tsx index a51173de..9afdd7ae 100644 --- a/src/renderer/features/agents/ui/agent-task-tool.tsx +++ b/src/renderer/features/agents/ui/agent-task-tool.tsx @@ -7,22 +7,49 @@ import { TextShimmer } from "../../../components/ui/text-shimmer" import { keyItems } from "../../../lib/react-keys" import { cn } from "../../../lib/utils" import { selectedProjectAtom } from "../atoms" +import { isSubagentToolType } from "../lib/subagent-tool-types" import { useFileOpen } from "../mentions" import { AgentToolCall } from "./agent-tool-call" import { AgentToolInterrupted } from "./agent-tool-interrupted" import { AgentToolRegistry, getToolStatus, type ToolDisplayPart } from "./agent-tool-registry" import type { ToolPartLike } from "./agent-tool-state" -import { areTaskToolPropsEqual } from "./agent-tool-utils" +import { + areTaskToolPropsEqual, + isLaunchedAgentOutput, + type NestedToolsLookup, +} from "./agent-tool-utils" interface AgentTaskToolProps { part: ToolPartLike nestedTools: ToolPartLike[] + /** + * Children of any nested subagent, so a subagent inside a subagent renders + * as its own expandable task rather than a flat line. Absent means flat + * rows only (the depth cap, and callers that have no map to offer). + */ + nestedChildren?: NestedToolsLookup + /** + * The message component's snapshot of the whole nesting map, for the memo + * only. A plain string: a grandchild lives under another key, and comparing + * fingerprints lets every row see the same mutation without any of them + * consuming the shared tool-state cache on the way past. + */ + nestingFingerprint?: string + /** How many subagent levels deep this call already is; the cap counts them. */ + depth?: number chatStatus?: string } // Constants for rendering const MAX_VISIBLE_TOOLS = 5 const TOOL_HEIGHT_PX = 24 +/** + * Subagent levels this component will nest into itself. The wire composes ids + * as `parentOriginal:childOriginal`, so real transcripts stop at depth two; + * the cap exists so an id scheme nobody has seen yet cannot recurse without + * bound — a level at or past it falls back to the flat registry row. + */ +const MAX_SUBAGENT_RENDER_DEPTH = 3 // Format elapsed time in a human-readable format function formatElapsedTime(ms: number): string { @@ -38,6 +65,9 @@ function formatElapsedTime(ms: number): string { export const AgentTaskTool = memo(function AgentTaskTool({ part, nestedTools, + nestedChildren, + nestingFingerprint, + depth = 0, chatStatus, }: AgentTaskToolProps) { const selectedProject = useAtomValue(selectedProjectAtom) @@ -124,9 +154,11 @@ export const AgentTaskTool = memo(function AgentTaskTool({ const subtitle = getSubtitle() - // Get title text based on status + // Get title text based on status: a launch is not a finish, and the row + // says which one it was. const getTitle = () => { - return isPending ? "Running Subagent" : "Completed Subagent" + if (isPending) return "Running Subagent" + return isLaunchedAgentOutput(part.output) ? "Launched Subagent" : "Completed Subagent" } // Show interrupted state if task was interrupted without completing @@ -217,6 +249,31 @@ export const AgentTaskTool = memo(function AgentTaskTool({ (nestedPart as { toolCallId?: unknown }).toolCallId ?? nestedPart.type ?? "part", ), ).map(({ key, item: nestedPart }) => { + // A subagent inside a subagent is its own task row — same + // header, same expansion, its own children — so its descendants + // render under it instead of being flattened into this level. + if ( + nestedPart.type && + isSubagentToolType(nestedPart.type) && + nestedChildren && + depth < MAX_SUBAGENT_RENDER_DEPTH + ) { + const rawChildId = (nestedPart as { toolCallId?: unknown }).toolCallId + // Only a string id names a child; an object would stringify to + // "[object Object]" and look up nothing (or the wrong thing). + const childId = typeof rawChildId === "string" ? rawChildId : "" + return ( + + ) + } const nestedMeta = nestedPart.type ? AgentToolRegistry[nestedPart.type] : undefined if (!nestedMeta) { return ( diff --git a/src/renderer/features/agents/ui/agent-tool-call.test.tsx b/src/renderer/features/agents/ui/agent-tool-call.test.tsx new file mode 100644 index 00000000..57f7a2d1 --- /dev/null +++ b/src/renderer/features/agents/ui/agent-tool-call.test.tsx @@ -0,0 +1,77 @@ +// @vitest-environment jsdom +/** + * When a registry row's subtitle is a button and when it is only text. + * + * `AgentToolCall` used to give every subtitle `role="button"`, a tab stop and + * Enter/Space handling, and attached the handler only when the row had an + * action — so every `TaskOutput` and `TaskStop` row (and any row rendered + * without its file-open provider) was a focusable, screen-reader-announced + * control that did nothing. Pinned here: no affordance, no button; an + * action or a tooltip that needs a keyboard entry, a native button that + * carries the focus without declaring a `tabIndex` (Sonar S6845 reads the + * declaration on a non-interactive element, and Radix's own TooltipTrigger + * is a button). + */ +import { render } from "@testing-library/react" +import { describe, expect, it } from "vitest" +import { EyeIcon } from "../../../components/ui/icons" +import { TooltipProvider } from "../../../components/ui/tooltip" +import { AgentToolCall } from "./agent-tool-call" + +function renderCall(ui: React.ReactElement) { + return render({ui}) +} + +describe("AgentToolCall subtitle affordance", () => { + it("is plain text when the row has no action and no tooltip", () => { + const { getByText } = renderCall( + , + ) + const subtitle = getByText("Task: task_1") + expect(subtitle.getAttribute("role")).toBeNull() + expect(subtitle.getAttribute("tabindex")).toBeNull() + }) + + it("is a button when only a tooltip needs a keyboard entry", () => { + // TooltipTrigger hangs off focus; a truncated path that only a mouse can + // reveal is not keyboard-accessible. A native button is Radix's own + // default trigger: it carries the tab stop natively, so no tabIndex sits + // on a non-interactive element (Sonar S6845), and with no action to + // press it gets no handler to fake one. + const { getByRole } = renderCall( + , + ) + const subtitle = getByRole("button", { name: "src/very/long/path/to/file.ts" }) + expect(subtitle.tagName).toBe("BUTTON") + expect(subtitle.getAttribute("tabindex")).toBeNull() + }) + + it("is a button when the row has an action to press", () => { + const { getByRole } = renderCall( + {}} + />, + ) + // A native button: implicit role, in the tab order, no hand-rolled keys. + const subtitle = getByRole("button", { name: "effort.ts" }) + expect(subtitle.tagName).toBe("BUTTON") + }) +}) diff --git a/src/renderer/features/agents/ui/agent-tool-call.tsx b/src/renderer/features/agents/ui/agent-tool-call.tsx index 2f8ccd9b..bfb7efe3 100644 --- a/src/renderer/features/agents/ui/agent-tool-call.tsx +++ b/src/renderer/features/agents/ui/agent-tool-call.tsx @@ -4,6 +4,64 @@ import { memo } from "react" import { TextShimmer } from "../../../components/ui/text-shimmer" import { Tooltip, TooltipContent, TooltipTrigger } from "../../../components/ui/tooltip" +/** + * The subtitle span, wearing button semantics when there is an affordance to + * press — an action, or a tooltip that needs a keyboard entry point — and + * nothing at all when there is neither. + * + * The tooltip-only case used to be a focusable span with `tabIndex={0}` and + * no role: Sonar's S6845 reads that as a tab stop on a non-interactive + * element, and the alternative of `role="button"` with an Enter handler that + * does nothing is the control that lies (which is the `TaskOutput` and + * `TaskStop` rows this component was already fixed for). The button here is + * the resolution Radix itself picks: `TooltipTrigger` renders a button by + * default, so focus is the affordance, native focusability carries the tab + * stop without declaring one, and activation has nothing to fake. + */ +function subtitleSpan( + content: React.ReactNode, + className: string, + onClick?: () => void, + tooltipFocusable = false, +): React.ReactElement { + // Reset the native button's UA styles so it sits in the row like the span it + // replaces — same fonts, colors, spacing — while keeping real button + // semantics (implicit role, keyboard activation, no hand-rolled keydown). + const buttonReset = + "appearance-none border-0 bg-transparent p-0 m-0 font-[inherit] text-[inherit]" + // No action and nothing to reveal: passive text, out of the tab order. + if (!onClick && !tooltipFocusable) return {content} + // The tooltip-only button is a trigger, not a link: it must not advertise a + // pointer click the UA stylesheet would otherwise promise. + const cursor = onClick ? "" : " cursor-default" + return ( + + ) +} + +/** The subtitle, wrapped in its tooltip when the meta describes one. */ +function subtitleWithTooltip( + span: React.ReactElement, + tooltipContent?: string, +): React.ReactElement { + if (!tooltipContent) return span + return ( + + {span} + + + {tooltipContent} + + + + ) +} + interface AgentToolCallProps { icon: React.ComponentType<{ className?: string }> title: string @@ -31,58 +89,18 @@ export const AgentToolCall = memo( const titleStr = String(title) const subtitleContent = subtitle ? subtitle : undefined - // Render subtitle with optional tooltip + // Render subtitle with optional tooltip; only an actionable one is a button. const clickableClass = onClick ? " cursor-pointer hover:text-muted-foreground transition-colors" : "" + const subtitleClass = `text-muted-foreground/60 font-normal truncate min-w-0${clickableClass}` - const subtitleElement = subtitleContent ? ( - tooltipContent ? ( - - - {/* biome-ignore lint/a11y/useSemanticElements: compact inline action; a native button would require style resets. */} - { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault() - onClick?.() - } - }} - > - {subtitleContent} - - - - - {tooltipContent} - - - - ) : ( - /* biome-ignore lint/a11y/useSemanticElements: compact inline action; a native button would require style resets. */ - { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault() - onClick?.() - } - }} - > - {subtitleContent} - - ) - ) : null + const subtitleElement = subtitleContent + ? subtitleWithTooltip( + subtitleSpan(subtitleContent, subtitleClass, onClick, Boolean(tooltipContent)), + tooltipContent, + ) + : null return (
diff --git a/src/renderer/features/agents/ui/agent-tool-registry.test.ts b/src/renderer/features/agents/ui/agent-tool-registry.test.ts new file mode 100644 index 00000000..a54ab5c1 --- /dev/null +++ b/src/renderer/features/agents/ui/agent-tool-registry.test.ts @@ -0,0 +1,148 @@ +/** + * The registry entries the 0.3.270 pin renamed, and the sharing that keeps the + * two spellings of one tool from drifting apart. The names come from the pinned + * CLI itself: grepping the 2.1.270 platform binary for its emitted tool table + * returns `Agent`, `TaskOutput` and `TaskStop` with no `Task`, no `BashOutput` + * and no `KillShell`, and its normalization table maps `KillShell` and + * `KillBash` to `TaskStop` and `BashOutput`, `BashOutputTool`, `AgentOutput`, + * `AgentOutputTool` to `TaskOutput`. The legacy keys stay registered because + * transcripts persisted before the bump carry them. + */ +import { describe, expect, it } from "vitest" +import { isSubagentToolType, SUBAGENT_TOOL_TYPES } from "../lib/subagent-tool-types" +import { AgentToolRegistry, type ToolDisplayPart } from "./agent-tool-registry" + +const pendingSubagent: ToolDisplayPart = { + state: "input-available", + input: { subagent_type: "Explore", description: "Audit the permission gate" }, +} + +const streamingSubagent: ToolDisplayPart = { + state: "input-streaming", + input: { subagent_type: "Explore", description: "Audit the permission gate" }, +} + +const finishedSubagent: ToolDisplayPart = { + state: "output-available", + input: { + subagent_type: "Explore", + description: "Read the transform and list every chunk kind it emits", + }, + output: { task: { subject: "Run the gate" } }, +} + +const taskOutputById: ToolDisplayPart = { + state: "output-available", + input: { task_id: "bg_7" }, + output: { task: { subject: "Run the gate" } }, +} + +const shellOutputByPid: ToolDisplayPart = { + state: "input-available", + input: { pid: 4242 }, +} + +describe("agent tool registry: renamed sub-agent and background task tools", () => { + it("registers the sub-agent under both names as one meta", () => { + expect(AgentToolRegistry["tool-Agent"]).toBeDefined() + // Same object, not an equal copy: two entries for one tool drift apart the + // first time someone edits the wording of one of them. + expect(AgentToolRegistry["tool-Agent"]).toBe(AgentToolRegistry["tool-Task"]) + }) + + it("registers TaskOutput as the meta BashOutput already had", () => { + expect(AgentToolRegistry["tool-TaskOutput"]).toBe(AgentToolRegistry["tool-BashOutput"]) + expect(AgentToolRegistry["tool-TaskStop"]).toBeDefined() + expect(AgentToolRegistry["tool-KillShell"]).toBeDefined() + }) + + // The pinned CLI's normalization table folds these six names into the two it + // emits, so each one has to reach the same meta or a persisted call renders as + // an unnamed generic row. Listed here rather than derived from the registry: + // dropping a key from the registry must fail this test, not shrink it. + it.each(["tool-BashOutput", "tool-BashOutputTool", "tool-AgentOutput", "tool-AgentOutputTool"])( + "routes %s to the TaskOutput meta", + (alias) => { + expect(AgentToolRegistry[alias]).toBe(AgentToolRegistry["tool-TaskOutput"]) + }, + ) + + it.each(["tool-KillShell", "tool-KillBash"])("routes %s to the shell-stopping meta", (alias) => { + expect(AgentToolRegistry[alias]).toBe(AgentToolRegistry["tool-KillShell"]) + // The current name says task, because what it stops may be a sub-agent. + expect(AgentToolRegistry[alias]).not.toBe(AgentToolRegistry["tool-TaskStop"]) + }) + + it("titles a sub-agent by state", () => { + const meta = AgentToolRegistry["tool-Agent"] + expect(meta.title(streamingSubagent)).toBe("Preparing agent") + expect(meta.title(pendingSubagent)).toBe("Running Explore") + expect(meta.title(finishedSubagent)).toBe("Explore completed") + }) + + it("names the sub-agent even when the payload carries no type", () => { + const meta = AgentToolRegistry["tool-Agent"] + expect(meta.title({ state: "input-available", input: {} })).toBe("Running Agent") + expect(meta.title({ state: "output-available", input: {} })).toBe("Agent completed") + }) + + it("calls a launched sub-agent a launch, not a completion", () => { + const meta = AgentToolRegistry["tool-Agent"] + const base: ToolDisplayPart = { + state: "output-available", + input: { subagent_type: "Explore", description: "Hand the run off" }, + } + expect(meta.title({ ...base, output: { status: "async_launched" } })).toBe("Explore launched") + expect(meta.title({ ...base, output: { status: "remote_launched" } })).toBe("Explore launched") + // A real completion still says so. + expect(meta.title({ ...base, output: { status: "completed" } })).toBe("Explore completed") + }) + + it("truncates a long sub-agent description and hides it while streaming", () => { + const meta = AgentToolRegistry["tool-Agent"] + expect(meta.subtitle?.(streamingSubagent)).toBe("") + expect(meta.subtitle?.(pendingSubagent)).toBe("Audit the permission gate") + // The registry's rule is 47 characters plus an ellipsis, so a 53 character + // description loses its last word rather than growing the row. + expect(meta.subtitle?.(finishedSubagent)).toBe( + "Read the transform and list every chunk kind it...", + ) + }) + + it("reads a task id or a shell pid into one subtitle", () => { + const meta = AgentToolRegistry["tool-TaskOutput"] + expect(meta.title(shellOutputByPid)).toBe("Getting output") + expect(meta.subtitle?.(shellOutputByPid)).toBe("PID: 4242") + expect(meta.title(taskOutputById)).toBe("Got output") + expect(meta.subtitle?.(taskOutputById)).toBe("Task: bg_7") + expect(meta.subtitle?.({ state: "output-available", input: {} })).toBe("") + }) + + it("says task for TaskStop and shell for the name it replaced", () => { + expect(AgentToolRegistry["tool-TaskStop"].title(shellOutputByPid)).toBe("Stopping task") + expect(AgentToolRegistry["tool-TaskStop"].title(taskOutputById)).toBe("Stopped task") + expect(AgentToolRegistry["tool-KillShell"].title(shellOutputByPid)).toBe("Stopping shell") + expect(AgentToolRegistry["tool-KillShell"].title(taskOutputById)).toBe("Stopped shell") + // Both read the same subtitle, so a task id renders under either name. + expect(AgentToolRegistry["tool-TaskStop"].subtitle?.(taskOutputById)).toBe("Task: bg_7") + // `shell_id` is TaskStopInput's deprecated spelling of `task_id`: a call + // that carries only it still has to say which task it stopped. + expect( + AgentToolRegistry["tool-TaskStop"].subtitle?.({ + state: "output-available", + input: { shell_id: "shell_1" }, + }), + ).toBe("Task: shell_1") + }) + + it("counts the sub-agent family and leaves the background task family out", () => { + expect([...SUBAGENT_TOOL_TYPES]).toEqual(["tool-Task", "tool-Agent"]) + expect(isSubagentToolType("tool-Task")).toBe(true) + expect(isSubagentToolType("tool-Agent")).toBe(true) + // Same prefix, different tool: these manage background work, not sub-agents. + expect(isSubagentToolType("tool-TaskCreate")).toBe(false) + expect(isSubagentToolType("tool-TaskStop")).toBe(false) + expect(isSubagentToolType("tool-TaskOutput")).toBe(false) + expect(isSubagentToolType("tool-Bash")).toBe(false) + }) +}) diff --git a/src/renderer/features/agents/ui/agent-tool-registry.tsx b/src/renderer/features/agents/ui/agent-tool-registry.tsx index e649c9c7..8eb56810 100644 --- a/src/renderer/features/agents/ui/agent-tool-registry.tsx +++ b/src/renderer/features/agents/ui/agent-tool-registry.tsx @@ -30,11 +30,10 @@ import { WriteFileIcon, } from "../../../components/ui/icons" import { getToolLifecycleState } from "./agent-tool-state" +import { isLaunchedAgentOutput } from "./agent-tool-utils" export { getToolStatus } from "./agent-tool-state" -export type ToolVariant = "simple" | "collapsible" - /** Tool input/output fields read by the registry display callbacks. */ export type ToolDisplayPart = { state?: string @@ -56,6 +55,9 @@ export type ToolDisplayPart = { subject?: string status?: string taskId?: string | number + task_id?: string | number + /** `TaskStopInput`'s deprecated spelling of `task_id`; same value. */ + shell_id?: string | number pid?: string | number text?: string plan?: { status?: string; title?: string; steps?: { status?: string }[] } @@ -66,6 +68,8 @@ export type ToolDisplayPart = { numLines?: number task?: { subject?: string } tasks?: unknown[] + /** `AgentOutput.status`: `completed`, or one of the two launch hand-offs. */ + status?: string } } @@ -74,7 +78,6 @@ export interface ToolMeta { title: (part: ToolDisplayPart) => string subtitle?: (part: ToolDisplayPart) => string tooltipContent?: (part: ToolDisplayPart, projectPath?: string) => string - variant: ToolVariant } function isInputStreaming(part: { state?: unknown; output?: unknown; result?: unknown }) { @@ -152,23 +155,81 @@ function calculateDiffStats(oldString: string, newString: string) { return { addedLines, removedLines } } -export const AgentToolRegistry: Record = { - "tool-Task": { - icon: SparklesIcon, - title: (part) => { - if (isInputStreaming(part)) return "Preparing agent" - const subagentType = part.input?.subagent_type || "Agent" - return isPendingState(part) ? `Running ${subagentType}` : `${subagentType} completed` - }, - subtitle: (part) => { - // Don't show subtitle while input is still streaming - if (isInputStreaming(part)) return "" - const description = part.input?.description || "" - return description.length > 50 ? `${description.slice(0, 47)}...` : description - }, - variant: "simple", +/** + * The sub-agent tool under both names it has had. The pinned CLI emits `Agent`, + * and its own history explains why two names exist: the SDK changelog at 0.2.69 + * reverted the wire name with the note that it "will migrate to `Agent` in the + * next minor release", so transcripts this app already persisted carry `Task`. + * One object behind two keys, because two entries for one tool drift apart the + * first time someone edits the wording of one of them. + */ +const subagentTool: ToolMeta = { + icon: SparklesIcon, + title: (part) => { + if (isInputStreaming(part)) return "Preparing agent" + const subagentType = part.input?.subagent_type || "Agent" + if (isPendingState(part)) return `Running ${subagentType}` + // A launched run is still running somewhere else; only a `completed` + // status has actually finished. + return isLaunchedAgentOutput(part.output) + ? `${subagentType} launched` + : `${subagentType} completed` + }, + subtitle: (part) => { + // Don't show subtitle while input is still streaming + if (isInputStreaming(part)) return "" + const description = part.input?.description || "" + return description.length > 50 ? `${description.slice(0, 47)}...` : description }, +} + +/** + * A shell reports `pid`, a background task reports `task_id`, and the persisted + * app shape spells the same field `taskId`. One reader for all three so the + * renamed tools do not each grow their own subtitle rule. + */ +function backgroundTaskSubtitle(part: ToolDisplayPart): string { + const pid = part.input?.pid + if (pid) return `PID: ${pid}` + // `shell_id` is `TaskStopInput`'s deprecated spelling of `task_id` — the + // same identifier under the name older transcripts carry — so a stop that + // sends only it still says which task it stopped. + const taskId = part.input?.task_id ?? part.input?.taskId ?? part.input?.shell_id + return taskId ? `Task: ${taskId}` : "" +} + +/** + * Background output under the name the CLI emits now and the name it emitted + * before: the 2.1.270 binary normalizes `BashOutput`, `BashOutputTool`, + * `AgentOutput` and `AgentOutputTool` to `TaskOutput`, so one meta serves the + * current wire name and the legacy rows already in the transcript store. + */ +const backgroundOutputTool: ToolMeta = { + icon: Terminal, + title: (part) => (isPendingState(part) ? "Getting output" : "Got output"), + subtitle: backgroundTaskSubtitle, +} + +const stopShellTool: ToolMeta = { + icon: XCircle, + title: (part) => (isPendingState(part) ? "Stopping shell" : "Stopped shell"), + subtitle: backgroundTaskSubtitle, +} + +/** + * `TaskStop` is what the same binary normalizes `KillShell` and `KillBash` to, + * so its wording says task rather than shell: the thing being stopped at that + * name is a background task, which may be a sub-agent rather than a shell. + */ +const stopTaskTool: ToolMeta = { + icon: XCircle, + title: (part) => (isPendingState(part) ? "Stopping task" : "Stopped task"), + subtitle: backgroundTaskSubtitle, +} +export const AgentToolRegistry: Record = { + "tool-Task": subagentTool, + "tool-Agent": subagentTool, "tool-Grep": { icon: SearchIcon, title: (part) => { @@ -204,7 +265,6 @@ export const AgentToolRegistry: Record = { return pattern.length > 40 ? `${pattern.slice(0, 37)}...` : pattern }, - variant: "simple", }, "tool-Glob": { @@ -231,7 +291,6 @@ export const AgentToolRegistry: Record = { return pattern.length > 40 ? `${pattern.slice(0, 37)}...` : pattern }, - variant: "simple", }, "tool-Read": { @@ -252,7 +311,6 @@ export const AgentToolRegistry: Record = { const filePath = part.input?.file_path || "" return getDisplayPath(filePath, projectPath) }, - variant: "simple", }, "tool-Edit": { @@ -283,14 +341,12 @@ export const AgentToolRegistry: Record = { return "" }, - variant: "simple", }, // Cloning indicator - shown while sandbox is being created "tool-cloning": { icon: GitBranch, title: () => "Cloning repo", - variant: "simple", }, // Planning indicator - shown when streaming starts but no content yet @@ -312,7 +368,6 @@ export const AgentToolRegistry: Record = { ] return messages[Math.floor(Math.random() * messages.length)] }, - variant: "simple", }, "tool-Write": { @@ -328,7 +383,6 @@ export const AgentToolRegistry: Record = { if (!filePath) return "" // Don't show "file" placeholder during streaming return filePath.split("/").pop() || "" }, - variant: "simple", }, "tool-Bash": { @@ -350,7 +404,6 @@ export const AgentToolRegistry: Record = { }) return normalized.length > 50 ? `${normalized.slice(0, 47)}...` : normalized }, - variant: "simple", }, "tool-WebFetch": { @@ -369,7 +422,6 @@ export const AgentToolRegistry: Record = { return url.slice(0, 30) } }, - variant: "simple", }, "tool-WebSearch": { @@ -384,7 +436,6 @@ export const AgentToolRegistry: Record = { const query = part.input?.query || "" return query.length > 40 ? `${query.slice(0, 37)}...` : query }, - variant: "collapsible", }, // Planning tools @@ -402,7 +453,6 @@ export const AgentToolRegistry: Record = { if (todos.length === 0) return "" return `${todos.length} ${todos.length === 1 ? "item" : "items"}` }, - variant: "simple", }, // Task management tools @@ -415,7 +465,6 @@ export const AgentToolRegistry: Record = { const subject = part.input?.subject || "" return subject.length > 40 ? `${subject.slice(0, 37)}...` : subject }, - variant: "simple", }, "tool-TaskUpdate": { @@ -442,7 +491,6 @@ export const AgentToolRegistry: Record = { } return taskId ? `#${taskId}` : "" }, - variant: "simple", }, "tool-TaskGet": { @@ -458,7 +506,6 @@ export const AgentToolRegistry: Record = { } return taskId ? `#${taskId}` : "" }, - variant: "simple", }, "tool-TaskList": { @@ -469,7 +516,6 @@ export const AgentToolRegistry: Record = { return count !== undefined ? `Listed ${count} tasks` : "Listed tasks" }, subtitle: () => "", - variant: "simple", }, "tool-PlanWrite": { @@ -498,7 +544,6 @@ export const AgentToolRegistry: Record = { } return steps.length > 0 ? `${completed}/${steps.length} steps` : "" }, - variant: "simple", }, "tool-ExitPlanMode": { @@ -507,7 +552,6 @@ export const AgentToolRegistry: Record = { return isPendingState(part) ? "Finishing plan" : "Plan complete" }, subtitle: () => "", - variant: "simple", }, // Notebook tools @@ -521,33 +565,20 @@ export const AgentToolRegistry: Record = { if (!filePath) return "" return filePath.split("/").pop() || "" }, - variant: "simple", - }, - - // Shell management tools - "tool-BashOutput": { - icon: Terminal, - title: (part) => { - return isPendingState(part) ? "Getting output" : "Got output" - }, - subtitle: (part) => { - const pid = part.input?.pid - return pid ? `PID: ${pid}` : "" - }, - variant: "simple", }, - "tool-KillShell": { - icon: XCircle, - title: (part) => { - return isPendingState(part) ? "Stopping shell" : "Stopped shell" - }, - subtitle: (part) => { - const pid = part.input?.pid - return pid ? `PID: ${pid}` : "" - }, - variant: "simple", - }, + // Shell and background task management. The pinned CLI emits `TaskOutput` and + // `TaskStop`; every other key here is a name its own normalization table folds + // into one of those two, kept because a transcript persisted before the bump + // still carries the old spelling and would otherwise render as a generic row. + "tool-TaskOutput": backgroundOutputTool, + "tool-BashOutput": backgroundOutputTool, + "tool-BashOutputTool": backgroundOutputTool, + "tool-AgentOutput": backgroundOutputTool, + "tool-AgentOutputTool": backgroundOutputTool, + "tool-TaskStop": stopTaskTool, + "tool-KillShell": stopShellTool, + "tool-KillBash": stopShellTool, // Note: ListMcpResources, ReadMcpResource and their "Tool"-suffixed variants // are handled by AgentMcpToolCall via parseMcpToolType() for richer output display @@ -558,7 +589,6 @@ export const AgentToolRegistry: Record = { title: (part) => { return isPendingState(part) ? "Compacting..." : "Compacted" }, - variant: "simple", }, // Extended Thinking @@ -572,7 +602,6 @@ export const AgentToolRegistry: Record = { // Show first 50 chars as preview return text.length > 50 ? `${text.slice(0, 47)}...` : text }, - variant: "collapsible", }, } diff --git a/src/renderer/features/agents/ui/agent-tool-state.ts b/src/renderer/features/agents/ui/agent-tool-state.ts index be3e9ae8..016b7547 100644 --- a/src/renderer/features/agents/ui/agent-tool-state.ts +++ b/src/renderer/features/agents/ui/agent-tool-state.ts @@ -31,6 +31,21 @@ export type ToolPartLike = { input?: unknown output?: unknown result?: unknown + error?: unknown + errorText?: unknown +} + +/** + * The state strings that say the SDK has finished with this part — the state + * string alone, deliberately not `getToolLifecycleState().isTerminal`, which + * also counts a present `output`: a part whose output arrived before its + * state string caught up is still live, and the settled-part serializers + * below must keep serializing it every time. + */ +const TERMINAL_STATE_STRINGS = new Set(["output-available", "output-error", "result", "error"]) + +export function isTerminalStateString(state: unknown): boolean { + return typeof state === "string" && TERMINAL_STATE_STRINGS.has(state) } export function getToolLifecycleState(part: ToolPartLike): ToolLifecycleState { @@ -38,11 +53,7 @@ export function getToolLifecycleState(part: ToolPartLike): ToolLifecycleState { const hasOutput = hasValue(part?.output) const hasResult = hasValue(part?.result) const isInputStreaming = state === "input-streaming" - const isTerminalState = - state === "output-available" || - state === "output-error" || - state === "result" || - state === "error" + const isTerminalState = isTerminalStateString(state) const isError = state === "output-error" || state === "error" || diff --git a/src/renderer/features/agents/ui/agent-tool-utils.test.ts b/src/renderer/features/agents/ui/agent-tool-utils.test.ts new file mode 100644 index 00000000..4a301e57 --- /dev/null +++ b/src/renderer/features/agents/ui/agent-tool-utils.test.ts @@ -0,0 +1,234 @@ +/** + * The task-row memo's view of the message-level nesting map. The regression + * under test: comparing that map through `arePartsEqual` walks the shared + * module-level tool-state cache, so the FIRST task row's comparator consumed + * every streaming grandchild's mutation and the rows after it saw a clean + * cache and skipped re-render. A pure fingerprint (computed once per render + * in the message component) makes every row's answer identical, because + * neither of them writes anything. + */ +import { describe, expect, it } from "vitest" +import type { ToolPartLike } from "./agent-tool-state" +import { areTaskToolPropsEqual, areToolPropsEqual, nestingFingerprintOf } from "./agent-tool-utils" + +function taskPart(id: string): ToolPartLike { + return { + type: "tool-Task", + toolCallId: id, + state: "output-available", + input: { subagent_type: "Explore", description: "walk" }, + output: { status: "completed", totalSteps: 3 }, + } +} + +function grandchild( + id: string, + filePath: string, + state: string = "output-available", +): ToolPartLike { + return { + type: "tool-Read", + toolCallId: id, + state, + input: { file_path: filePath }, + output: { content: `contents of ${filePath}` }, + } +} + +/** Two independent maps with structurally identical content. */ +function twinMaps(): { a: Map; b: Map } { + const a = new Map([ + ["A", [taskPart("A")]], + ["A:B", [grandchild("A:B:C", "src/one.ts")]], + ]) + const b = new Map([ + ["A", [taskPart("A")]], + ["A:B", [grandchild("A:B:C", "src/one.ts")]], + ]) + return { a, b } +} + +describe("nestingFingerprintOf", () => { + it("is empty for an absent or empty map", () => { + expect(nestingFingerprintOf(undefined)).toBe("") + expect(nestingFingerprintOf(new Map())).toBe("") + }) + + it("is stable across rebuilds with equal content", () => { + const { a, b } = twinMaps() + // A rebuilt map yields the same string, and string identity (not map + // identity) is what the row memo compares. + expect(nestingFingerprintOf(a)).toBe(nestingFingerprintOf(b)) + }) + + it("changes when a streaming grandchild mutates in place", () => { + const map = new Map([ + ["A", [taskPart("A")]], + ["A:B", [grandchild("A:B:C", "src/one.ts", "input-streaming")]], + ]) + const before = nestingFingerprintOf(map) + // The AI SDK mutates parts in place — same part, same input object, the + // file_path rewritten underneath. A live (non-terminal) part is + // re-serialized every render, so the fingerprint sees it. + const child = map.get("A:B")?.[0] + expect(child).toBeDefined() + const input = child?.input as { file_path: string } + input.file_path = "src/two.ts" + const after = nestingFingerprintOf(map) + expect(after).not.toBe(before) + }) + + it("holds a settled terminal part's segment until a reference moves", () => { + // The round-9 cost bound: a completed part whose input/output references + // have not moved is not re-stringified, so a deep rewrite of a FINISHED + // part does not reach the fingerprint — the SDK does not reopen one. + // What keeps it honest is the other half: a changed reference or state + // re-serializes immediately, which is the case the row memos feed on. + const map = new Map([ + ["A:B", [grandchild("A:B:C", "src/one.ts")]], // terminal by default + ]) + const before = nestingFingerprintOf(map) + const child = map.get("A:B")?.[0] + const input = child?.input as { file_path: string } + input.file_path = "src/deep-rewritten.ts" // same reference, settled part + expect(nestingFingerprintOf(map)).toBe(before) + + // A new input object (the shape a replacement takes) breaks the settle. + if (child) child.input = { file_path: "src/replaced.ts" } + expect(nestingFingerprintOf(map)).not.toBe(before) + }) + + it("changes when only a terminal part's result is replaced", () => { + // CodeAnt 4104721117: the ask card renders `result`, so a wholesale + // replacement with state, input, and output all untouched has to move + // the fingerprint — the settle gate compares the reference too. + const part: ToolPartLike = { + type: "tool-AskUserQuestion", + toolCallId: "result-only-call", + state: "result", + input: { questions: [{ question: "Which pin?", header: "Pin" }] }, + result: { answers: { "Which pin?": "0.3.270" } }, + } + const before = nestingFingerprintOf(new Map([["A", [part]]])) + part.result = { answers: { "Which pin?": "0.3.280" } } + const after = nestingFingerprintOf(new Map([["A", [part]]])) + expect(after).not.toBe(before) + }) + + it("changes when a nested part's state moves from pending to done", () => { + const map = new Map([ + [ + "A:B", + [ + { + type: "tool-Read", + toolCallId: "A:B:C", + state: "input-available", + input: { file_path: "src/one.ts" }, + }, + ], + ], + ]) + const before = nestingFingerprintOf(map) + const child = map.get("A:B")?.[0] + if (child) child.state = "output-available" + expect(nestingFingerprintOf(map)).not.toBe(before) + }) +}) + +describe("areTaskToolPropsEqual fingerprint comparison", () => { + const ownPart = taskPart("A") + const ownNested: ToolPartLike[] = [grandchild("A:B:C", "src/one.ts")] + + /** + * First sight of a toolCallId always counts as changed — `hasToolStateChanged` + * seeds the shared cache on the way past, the same one-time render production + * does after mount. Accept-expectations run after this priming pass. + */ + function primeCache(): void { + const props = { part: ownPart, nestedTools: ownNested } + // The comparator returns early on the first uncached id, so one pass + // seeds the main part only; a second pass seeds the nested list too. + areTaskToolPropsEqual(props, { ...props }) + areTaskToolPropsEqual(props, { ...props }) + } + + it("rejects when the fingerprint moved — twice in a row", () => { + // The old cache-backed comparator returned true on the SECOND call: the + // first call had consumed the mutation for everyone. Two identical calls + // must both reject, or the next task row in the list never re-renders. + const { a, b } = twinMaps() + const before = nestingFingerprintOf(a) + const child = b.get("A:B")?.[0] + if (child) child.output = { content: "streamed further" } + const after = nestingFingerprintOf(b) + + const prev = { part: ownPart, nestedTools: ownNested, nestingFingerprint: before } + const next = { part: ownPart, nestedTools: ownNested, nestingFingerprint: after } + expect(areTaskToolPropsEqual(prev, next)).toBe(false) + expect(areTaskToolPropsEqual(prev, next)).toBe(false) + }) + + it("accepts when content is equal under a rebuilt map", () => { + const { a, b } = twinMaps() + const prev = { + part: ownPart, + nestedTools: ownNested, + nestingFingerprint: nestingFingerprintOf(a), + } + const next = { + part: ownPart, + nestedTools: ownNested, + nestingFingerprint: nestingFingerprintOf(b), + } + primeCache() + expect(areTaskToolPropsEqual(prev, next)).toBe(true) + }) + + it("accepts two rows with no fingerprint and no nesting at all", () => { + const props = { part: ownPart, nestedTools: ownNested } + primeCache() + expect(areTaskToolPropsEqual(props, { ...props })).toBe(true) + }) + + it("still falls back to lookup identity when no fingerprint is supplied", () => { + const lookup = () => ownNested + const props = { part: ownPart, nestedTools: ownNested, nestedChildren: lookup } + primeCache() + expect(areTaskToolPropsEqual(props, { ...props })).toBe(true) + expect(areTaskToolPropsEqual(props, { ...props, nestedChildren: () => ownNested })).toBe(false) + }) + + it("still detects a change in the row's own part", () => { + const { a } = twinMaps() + const fp = nestingFingerprintOf(a) + const mutated = taskPart("A") + mutated.state = "input-streaming" + expect( + areTaskToolPropsEqual( + { part: ownPart, nestedTools: ownNested, nestingFingerprint: fp }, + { part: mutated, nestedTools: ownNested, nestingFingerprint: fp }, + ), + ).toBe(false) + }) +}) + +describe("areToolPropsEqual", () => { + it("moves the row when only a terminal part's result is replaced", () => { + // Same claim at the row level (CodeAnt 4104721522's sibling): the ask + // card renders `result`, so the row snapshot has to compare it — by + // reference, because every writer assigns it wholesale alongside the + // state flip rather than mutating it in place. + const part: ToolPartLike = { + type: "tool-AskUserQuestion", + toolCallId: "row-result-only-call", + state: "result", + input: { questions: [{ question: "Which pin?", header: "Pin" }] }, + result: { answers: { "Which pin?": "0.3.270" } }, + } + expect(areToolPropsEqual({ part }, { part })).toBe(false) // first call primes + expect(areToolPropsEqual({ part }, { part })).toBe(true) // nothing moved + part.result = { answers: { "Which pin?": "0.3.280" } } + expect(areToolPropsEqual({ part }, { part })).toBe(false) // result-only change + }) +}) diff --git a/src/renderer/features/agents/ui/agent-tool-utils.ts b/src/renderer/features/agents/ui/agent-tool-utils.ts index 2d046071..797483ab 100644 --- a/src/renderer/features/agents/ui/agent-tool-utils.ts +++ b/src/renderer/features/agents/ui/agent-tool-utils.ts @@ -7,7 +7,7 @@ * and compare cached values, not object references. */ -import { getToolLifecycleState, type ToolPartLike } from "./agent-tool-state" +import { getToolLifecycleState, isTerminalStateString, type ToolPartLike } from "./agent-tool-state" // ============================================================================ // TOOL STATE CACHE @@ -20,6 +20,15 @@ interface CachedToolState { state: string | undefined inputJson: string // JSON stringified input for deep comparison outputJson: string // JSON stringified output for deep comparison + // References, deliberately not JSON: every writer in this repo assigns + // these wholesale when it sets them (claude.ts's three result sites pair + // the assignment with a state transition; nothing deep-mutates them the + // way the SDK does input and output — rounds 7 and 8). The ask card + // renders `result` and `errorText`, so a change in either has to move + // the row even when state, input, and output sit still. + result: unknown + error: unknown + errorText: unknown } const toolStateCache = new Map() @@ -28,6 +37,7 @@ export function clearToolStateCachesByToolCallIds(toolCallIds: string[]) { for (const toolCallId of toolCallIds) { toolStateCache.delete(toolCallId) askUserStateCache.delete(toolCallId) + fingerprintSegmentCache.delete(toolCallId) } } @@ -36,6 +46,9 @@ function getToolStateSnapshot(part: ToolPartLike): CachedToolState { state: typeof part.state === "string" ? part.state : undefined, inputJson: JSON.stringify(part.input || {}), outputJson: JSON.stringify(part.output || {}), + result: part.result, + error: part.error, + errorText: part.errorText, } } @@ -51,7 +64,10 @@ function hasToolStateChanged(toolCallId: string, part: ToolPartLike): boolean { const changed = cached.state !== current.state || cached.inputJson !== current.inputJson || - cached.outputJson !== current.outputJson + cached.outputJson !== current.outputJson || + cached.result !== current.result || + cached.error !== current.error || + cached.errorText !== current.errorText if (changed) { toolStateCache.set(toolCallId, current) @@ -129,10 +145,164 @@ export function areToolPropsEqual( /** * Compare function for AgentTaskTool which has additional nestedTools prop. */ +/** + * A subagent's own children by its id, and the message-level map that stands + * behind it. A grandchild is not in this task's own `nestedTools`, and only a + * change under some other key says it moved. Identity of either is useless + * here — `messageParts` is rebuilt every render (the AI SDK mutates parts in + * place), so the map and any callback over it are fresh objects with + * unchanged contents. What the memo compares is `nestingFingerprintOf`'s + * snapshot of that map: one string, every row, no cache writes. + */ +export type NestedToolsLookup = (toolCallId: string) => ToolPartLike[] +export type NestedToolsMapLike = ReadonlyMap + +interface FingerprintSegment { + mapKey: string + state: unknown + input: unknown + output: unknown + result: unknown + error: unknown + errorText: unknown + segment: string +} + +/** + * Settled segments from the previous fingerprint, keyed by toolCallId — + * see `nestingFingerprintOf` for when a segment may be reused. + */ +const fingerprintSegmentCache = new Map() + +/** + * An immutable snapshot of the message-level nesting map, as one string. + * + * The map itself cannot be compared by content through `arePartsEqual`: that + * comparator advances the module-level `toolStateCache`, so the first task row + * to walk the map would consume every mutation and the rows after it would + * see a clean cache and skip a grandchild that changed. Computed ONCE per + * render in the message component and carried as a plain string, the compare + * is pure — two rows asking "did anything under the tree move?" both get the + * same answer, because neither of them writes anything. + * + * Reads the same fields the tool-state snapshot records (`state`, `input`, + * `output`) plus the identity fields, and never the tool-state cache. + * + * Cost is bounded like the outer memo's: a part whose state string is + * terminal and whose input/output references are unchanged is settled (the + * SDK does not reopen a completed part), so its segment is reused from a + * private cache in O(1) instead of re-stringified — a finished transcript + * costs reference compares, and only live parts pay for serialization on + * each render. The cache is written here, once per render in the message + * component; the row comparators only ever read the returned string, so the + * single-consumer property that motivated the fingerprint is untouched. + */ +function fingerprintSegmentOf(mapKey: string, part: ToolPartLike): string { + // A part without a usable toolCallId keys under "" — which is never stored, + // so it never reads a cached segment either: it serializes every time. + const key = typeof part.toolCallId === "string" ? part.toolCallId : "" + const prev = fingerprintSegmentCache.get(key) + if ( + prev?.mapKey === mapKey && + prev?.state === part.state && + prev?.input === part.input && + prev?.output === part.output && + prev?.result === part.result && + prev?.error === part.error && + prev?.errorText === part.errorText && + isTerminalStateString(prev?.state) + ) { + return prev.segment // settled: same terminal state, same references + } + // JSON.stringify the tuple rather than join(): `state` is `unknown`, and + // join() would fall back to Object's default stringification for any + // non-string it meets, collapsing two different objects into one + // "[object Object]" and hiding a change behind it. `result`, `error`, and + // `errorText` ride along because the ask card and the lifecycle view + // render them: a result-only update has to move the rows like any other. + const segment = JSON.stringify([ + mapKey, + part.type, + part.toolCallId, + part.state, + part.input, + part.output, + part.result, + part.error, + part.errorText, + ]) + if (key !== "") { + fingerprintSegmentCache.set(key, { + mapKey, + state: part.state, + input: part.input, + output: part.output, + result: part.result, + error: part.error, + errorText: part.errorText, + segment, + }) + } + return segment +} + +export function nestingFingerprintOf(map: NestedToolsMapLike | undefined): string { + if (!map || map.size === 0) return "" + const segments: string[] = [] + for (const [id, parts] of map) { + for (const part of parts) { + segments.push(fingerprintSegmentOf(id, part)) + } + } + return segments.join("\u0001") +} + +/** + * A result that launched work instead of finishing it. The pinned SDK types + * `AgentOutput.status` as `completed` | `async_launched` | `remote_launched`: + * the latter two mean the run was handed off — to the background, or to a + * remote session — and is still going there, so a row that calls them a + * completion reads as subagent work that ended when it has not. + */ +export function isLaunchedAgentOutput(output: unknown): boolean { + const status = (output as { status?: unknown } | null | undefined)?.status + return status === "async_launched" || status === "remote_launched" +} + export function areTaskToolPropsEqual( - prevProps: { part: ToolPartLike; nestedTools: ToolPartLike[]; chatStatus?: string }, - nextProps: { part: ToolPartLike; nestedTools: ToolPartLike[]; chatStatus?: string }, + prevProps: { + part: ToolPartLike + nestedTools: ToolPartLike[] + nestedChildren?: NestedToolsLookup + nestingFingerprint?: string + depth?: number + chatStatus?: string + }, + nextProps: { + part: ToolPartLike + nestedTools: ToolPartLike[] + nestedChildren?: NestedToolsLookup + nestingFingerprint?: string + depth?: number + chatStatus?: string + }, ): boolean { + // Descendants beyond this task's own `nestedTools` are visible only through + // the message-level map. Compare the render's fingerprint of it — a pure + // string, so two rows can both see the same grandchild mutation without + // either consuming the other's change out of the tool-state cache. Checked + // first so the completed short circuit below cannot hide it. + if ((prevProps.nestingFingerprint ?? "") !== (nextProps.nestingFingerprint ?? "")) return false + // Fallback for callers that offer a lookup without a fingerprint. + if ( + prevProps.nestingFingerprint === undefined && + nextProps.nestingFingerprint === undefined && + prevProps.nestedChildren !== nextProps.nestedChildren + ) { + return false + } + if (prevProps.depth !== nextProps.depth) return false + // Compare main part first if (!arePartsEqual(prevProps.part, nextProps.part)) return false diff --git a/src/renderer/lib/atoms/claude-effort.test.ts b/src/renderer/lib/atoms/claude-effort.test.ts new file mode 100644 index 00000000..4be23465 --- /dev/null +++ b/src/renderer/lib/atoms/claude-effort.test.ts @@ -0,0 +1,70 @@ +// @vitest-environment jsdom +/** + * Claude effort is owned by the sub-chat that chose it: one split pane + * setting `max` must not change what the other pane sends. The pre-existing + * global value survives as `lastSelected`, which is what a chat that has + * never picked reads and what the new-chat form (no sub-chat id yet) reads + * and writes. + */ +import { createStore } from "jotai" +import { afterEach, describe, expect, it, vi } from "vitest" +import { lastSelectedClaudeEffortAtom, subChatClaudeEffortAtomFamily } from "." + +afterEach(() => { + localStorage.clear() +}) + +describe("sub-chat Claude effort", () => { + it("keeps two sub-chats' picks independent", () => { + const store = createStore() + store.set(subChatClaudeEffortAtomFamily("chat-a"), "max") + store.set(subChatClaudeEffortAtomFamily("chat-b"), "low") + + expect(store.get(subChatClaudeEffortAtomFamily("chat-a"))).toBe("max") + expect(store.get(subChatClaudeEffortAtomFamily("chat-b"))).toBe("low") + // The global a third chat falls back on has not moved. + expect(store.get(lastSelectedClaudeEffortAtom)).toBeNull() + }) + + it("falls back to the last-selected pick for a chat that has never chosen", () => { + const store = createStore() + store.set(lastSelectedClaudeEffortAtom, "high") + expect(store.get(subChatClaudeEffortAtomFamily("chat-untouched"))).toBe("high") + }) + + it("treats a chat's explicit null as its own answer, not a miss", () => { + const store = createStore() + store.set(lastSelectedClaudeEffortAtom, "high") + store.set(subChatClaudeEffortAtomFamily("chat-a"), null) + // `high` is what a fresh chat gets; chat-a asked for the CLI default and + // must keep reading null rather than someone else's level. + expect(store.get(subChatClaudeEffortAtomFamily("chat-a"))).toBeNull() + expect(store.get(subChatClaudeEffortAtomFamily("chat-b"))).toBe("high") + }) + + it("reads and writes the last-selected pick when there is no chat yet", () => { + const store = createStore() + store.set(subChatClaudeEffortAtomFamily(""), "xhigh") + expect(store.get(lastSelectedClaudeEffortAtom)).toBe("xhigh") + // ...and a chat created afterwards inherits it. + store.set( + subChatClaudeEffortAtomFamily("chat-new"), + store.get(subChatClaudeEffortAtomFamily("")), + ) + expect(store.get(subChatClaudeEffortAtomFamily("chat-new"))).toBe("xhigh") + }) + + it("carries the pre-existing global storage key over as the last-selected pick", async () => { + // `getOnInit` reads storage when the atom module loads, so the seed has + // to be in place before the atoms are imported — which is exactly the + // production shape: an upgraded app starts with last version's value + // already on disk. Resetting the module registry replays that start. + vi.resetModules() + localStorage.setItem("preferences:claude-effort", JSON.stringify("low")) + const { lastSelectedClaudeEffortAtom: lastSelected, subChatClaudeEffortAtomFamily: family } = + await import(".") + const store = createStore() + expect(store.get(lastSelected)).toBe("low") + expect(store.get(family("chat-untouched"))).toBe("low") + }) +}) diff --git a/src/renderer/lib/atoms/index.ts b/src/renderer/lib/atoms/index.ts index 02c4c032..7fcfbdaf 100644 --- a/src/renderer/lib/atoms/index.ts +++ b/src/renderer/lib/atoms/index.ts @@ -1,5 +1,6 @@ import { atom } from "jotai" -import { atomWithStorage } from "jotai/utils" +import { atomFamily, atomWithStorage } from "jotai/utils" +import type { EffortLevel } from "../../../shared/effort" import { desktopViewAtom as _desktopViewAtom } from "../../features/agents/atoms" import { createRendererSecretStorage } from "../renderer-secrets" import { DEFAULT_DARK_THEME_ID, DEFAULT_LIGHT_THEME_ID } from "../themes/builtin-themes" @@ -350,6 +351,61 @@ export const extendedThinkingEnabledAtom = atomWithStorage( { getOnInit: true }, ) +// Preferences - Claude reasoning effort +// The levels come from `src/shared/effort.ts`, the one vocabulary the main +// process validates a request against. `null` means the chat never picked one +// and the CLI decides, which is the behaviour before this setting existed. +// +// Effort belongs to the sub-chat that chose it: the model beside it in the +// picker has always been per-sub-chat, and one split pane setting `max` must +// not change what the other pane sends. The pre-existing global value is kept +// as `lastSelected` — what a chat that has never picked one reads, and what +// the new-chat form writes before there is a chat to write into. The storage +// key for it is the old one, so every existing pick carries over untouched. +export const lastSelectedClaudeEffortAtom = atomWithStorage( + "preferences:claude-effort", + null, + undefined, + { getOnInit: true }, +) +const subChatClaudeEffortStorageAtom = atomWithStorage>( + "preferences:claude-effort-by-subchat", + {}, + undefined, + { getOnInit: true }, +) +export const subChatClaudeEffortAtomFamily = atomFamily((subChatId: string) => + atom( + (get) => { + // `in`, not `??`: a chat that explicitly chose "no effort" stores null, + // and a null that means "this chat decided" must not read as a miss and + // fall back to someone else's pick. + const stored = get(subChatClaudeEffortStorageAtom) + if (subChatId && subChatId in stored) return stored[subChatId] + return get(lastSelectedClaudeEffortAtom) + }, + (get, set, effort: EffortLevel | null) => { + if (!subChatId) { + set(lastSelectedClaudeEffortAtom, effort) + return + } + const current = get(subChatClaudeEffortStorageAtom) + if (current[subChatId] === effort) return + set(subChatClaudeEffortStorageAtom, { ...current, [subChatId]: effort }) + }, + ), +) + +// Preferences - Prompt suggestions +// Off by default: turning it on asks the backend for a suggested next prompt at +// the end of a turn and puts one clickable row above the composer. +export const promptSuggestionsEnabledAtom = atomWithStorage( + "preferences:prompt-suggestions-enabled", + false, + undefined, + { getOnInit: true }, +) + // Preferences - History (Rollback) // When enabled, allow rollback to previous assistant messages export const historyEnabledAtom = atomWithStorage( diff --git a/src/shared/codex-model-id.ts b/src/shared/codex-model-id.ts index 7daa3c07..4f73ca87 100644 --- a/src/shared/codex-model-id.ts +++ b/src/shared/codex-model-id.ts @@ -21,7 +21,19 @@ * `.dump/global/decisions.md`; the two literals the release note carried, * `gpt-5.5` and `gpt-5.4`, were both refused. */ -export const CODEX_REASONING_EFFORTS = ["low", "medium", "high", "xhigh"] as const +import type { EffortLevel } from "./effort" + +/** + * The four levels Codex sends. `satisfies` keeps the tuple type the picker + * already depends on while proving every member is in the one effort vocabulary + * in `src/shared/effort.ts`. + */ +export const CODEX_REASONING_EFFORTS = [ + "low", + "medium", + "high", + "xhigh", +] as const satisfies readonly EffortLevel[] export type CodexReasoningEffort = (typeof CODEX_REASONING_EFFORTS)[number] @@ -41,8 +53,10 @@ export type CodexReasoningEffort = (typeof CODEX_REASONING_EFFORTS)[number] * Whoever retires it should rename all 25 at once, drop this alias and the * `CodexThinkingLevel` line from the re-export in * `src/renderer/features/agents/lib/models.ts`, and confirm - * `npm run typecheck` still reports 0 errors. The effort label itself is - * `formatCodexThinkingLabel` below, which stays. + * `npm run typecheck` still reports 0 errors. The effort label is + * `formatEffortLabel` in `src/shared/effort.ts`; the picker calls that directly + * now that both backends share one effort vocabulary, and the Codex-named + * wrapper it used to call is gone rather than left as a second name for it. */ export type CodexThinkingLevel = CodexReasoningEffort @@ -73,11 +87,6 @@ export const CODEX_MODELS = [ }, ] as const -export function formatCodexThinkingLabel(thinking: CodexThinkingLevel): string { - if (thinking === "xhigh") return "Extra High" - return thinking.charAt(0).toUpperCase() + thinking.slice(1) -} - export const DEFAULT_CODEX_UI_MODEL = "gpt-5.5" /** Effort sent when a chat has no stored effort and the CLI answers nothing. */ diff --git a/src/shared/effort.test.ts b/src/shared/effort.test.ts new file mode 100644 index 00000000..1b870400 --- /dev/null +++ b/src/shared/effort.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest" +import { CODEX_REASONING_EFFORTS } from "./codex-model-id" +import { EFFORT_LEVELS, formatEffortLabel, isEffortLevel } from "./effort" + +describe("the shared effort vocabulary", () => { + it("keeps the levels the pinned Claude Agent SDK declares", () => { + // `Options.effort` at 0.3.270 is low | medium | high | xhigh | max. This + // fails on purpose if the list is edited without re-reading the SDK type. + expect([...EFFORT_LEVELS]).toEqual(["low", "medium", "high", "xhigh", "max"]) + }) + + it("keeps every Codex effort inside the shared vocabulary", () => { + for (const effort of CODEX_REASONING_EFFORTS) { + expect(isEffortLevel(effort)).toBe(true) + } + }) + + it("accepts the advertised levels and nothing else", () => { + expect(EFFORT_LEVELS.every((level) => isEffortLevel(level))).toBe(true) + expect(isEffortLevel("default")).toBe(false) + expect(isEffortLevel("")).toBe(false) + expect(isEffortLevel("MAX")).toBe(false) + }) + + it("labels the levels the picker already labelled them", () => { + expect(formatEffortLabel("low")).toBe("Low") + expect(formatEffortLabel("medium")).toBe("Medium") + expect(formatEffortLabel("high")).toBe("High") + // The one level whose capitalized form is not the label the Codex picker + // shipped, and the reason this is a function rather than a capitalize call. + expect(formatEffortLabel("xhigh")).toBe("Extra High") + expect(formatEffortLabel("max")).toBe("Max") + }) +}) diff --git a/src/shared/effort.ts b/src/shared/effort.ts new file mode 100644 index 00000000..89ef82b3 --- /dev/null +++ b/src/shared/effort.ts @@ -0,0 +1,35 @@ +/** + * One reasoning-effort vocabulary for every backend that accepts one. + * + * The Claude Agent SDK at the pin in `package.json` (0.3.270) types its + * `Options.effort` as `"low" | "medium" | "high" | "xhigh" | "max"`, and its + * `ModelInfo` reports `supportedEffortLevels` from the same set. Codex has + * always shipped four of those five, `max` being the one it does not send. Two + * lists for one concept is how two backends drift, the same way the Codex model + * id and its effort once drifted apart as one slash-joined string, so the wider + * set lives here and each backend proves it is a subset: + * `CODEX_REASONING_EFFORTS` in `codex-model-id.ts` carries + * `satisfies readonly EffortLevel[]`. + * + * Which levels a given model accepts is the CLI's answer, not this module's. + * The SDK documents that an effort above a model's `maxEffortLevel` is clamped + * to it, so the picker offers the vocabulary and the runtime settles the model. + * Reading `ModelInfo.supportedEffortLevels` per model is the follow-up that + * retires a static list, mirroring what `providers/codex-models.ts` already does + * for the Codex catalog. + */ +export const EFFORT_LEVELS = ["low", "medium", "high", "xhigh", "max"] as const + +export type EffortLevel = (typeof EFFORT_LEVELS)[number] + +const EFFORT_VALUES: ReadonlySet = new Set(EFFORT_LEVELS) + +export function isEffortLevel(value: string): value is EffortLevel { + return EFFORT_VALUES.has(value) +} + +/** The row label the picker shows for an effort level. */ +export function formatEffortLabel(level: EffortLevel): string { + if (level === "xhigh") return "Extra High" + return level.charAt(0).toUpperCase() + level.slice(1) +} diff --git a/src/shared/provider-capabilities.ts b/src/shared/provider-capabilities.ts index 5bcfb4d0..a8d9ce96 100644 --- a/src/shared/provider-capabilities.ts +++ b/src/shared/provider-capabilities.ts @@ -72,6 +72,12 @@ export const featureFlagsSchema = z.object({ skills: z.boolean(), structuredOutput: z.boolean(), fileCheckpointing: z.boolean(), + /** The backend accepts a reasoning-effort level from `src/shared/effort.ts` on a turn. */ + effort: z.boolean(), + /** The backend can pick its own thinking budget per turn instead of being handed one. */ + adaptiveThinking: z.boolean(), + /** The backend can suggest a next prompt after it finishes a turn. */ + promptSuggestions: z.boolean(), }) export const providerCapabilitySchema = z.object({ @@ -93,6 +99,40 @@ export type ProviderCapability = z.infer /** The two answers to "who enforces the permission floor of roadmap step 10". */ export type PermissionFloor = ProviderCapability["security"]["permissionFloor"] +/** The feature flags themselves, so a manifest can be typed against them. */ +export type FeatureFlags = z.infer + +/** + * Every feature at the rule above's own answer: false. + * + * A manifest spreads this and then claims what its backend carries end to end, + * so the flags nothing in mausCode wires for any backend yet are stated once + * here instead of once per manifest, and a claim reads as a claim. Forgetting + * one fails safe, which is the direction the rule already asks for: the UI must + * not offer what a manifest did not say. A flag added to the schema fails + * typecheck here rather than going silently missing from one manifest. + * + * A negative claim that was investigated keeps its own line and its reason in + * the manifest — `grok` forks, `openclaw` does not resume — because that is + * evidence about a backend, not a default. The evidence behind the three + * turn-shaping flags is in `.dump/app/research/2026-09-13-sdk-0-3-bump.md`. + */ +export const ALL_FEATURES_OFF = { + chat: false, + images: false, + resume: false, + fork: false, + mcp: false, + subagents: false, + cron: false, + skills: false, + structuredOutput: false, + fileCheckpointing: false, + effort: false, + adaptiveThinking: false, + promptSuggestions: false, +} satisfies FeatureFlags + /** * The floor behind a sub-chat provider id, which is the vocabulary the chat UI * holds. The manifests are keyed by backend id and live in main, so the renderer diff --git a/vitest.config.ts b/vitest.config.ts index 14d6a002..d279d7a1 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,9 +2,10 @@ import { defineConfig } from "vitest/config" export default defineConfig({ test: { - // Node environment: tests cover main-process logic, not the renderer. + // Node environment by default: tests cover main-process logic. A renderer + // test declares `// @vitest-environment jsdom` at the top of its own file. environment: "node", - include: ["src/**/*.test.ts"], + include: ["src/**/*.test.ts", "src/**/*.test.tsx"], // node:test suites (not vitest) — run via `npm run test:node`. exclude: ["src/main/lib/runtime/*.test.ts", "node_modules"], // Main-process modules pull in electron/native deps; keep tests pure.