afterEach deleted the variable outright, so a developer who exported it to
silence a provider lost it after the first test file ran. The
essential-traffic variable next to it was already saved and restored; this
makes both behave the same way, since neither belongs to the test.
19 pass / 0 fail with the variable inherited from `.env`, with an explicit
CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC=1, and with
HAHA_MARKET_DISABLE_PROVIDERS=skillhub set in the environment — the case
that previously came back unset.
The check landed scoped to src/server/ws because blanking desynced on 6 of
2149 files and a desync reports a live import as dead. All four causes are
fixed, so the scope is now every source root no compiler checks — src,
scripts and adapters — and 0 of 2541 files desync.
The bugs, each with a regression test that places the import's only
reference after the construct so a desync makes it go dead:
- The token before a slash was read back out of the raw source, so the last
word of a preceding comment decided whether `/` opened a regex. In
useIssueFlagBanner.ts `// …correction tone` made the next line's regex lex
as a division, and its apostrophe opened a string that ate the line. A
comment is whitespace to the grammar; it now contributes nothing.
- That token accumulated across whitespace, so `return false` became one
token named `returnfalse` and the following `return /re/` no longer looked
like a keyword. This is what broke markdownImages.ts and dead-imports.ts
itself.
- `input! / 10` divides but `!/re/.test(x)` negates, and both put `!` before
the slash. What precedes the `!` settles it.
- Character classes and quotes inside regular expressions, fixed earlier.
desktop/ stays out of scope: its tsconfig already sets noUnusedLocals, and
scanning it anyway finds nothing — the cross-check that this agrees with a
real compiler. `blankingIsSound` still refuses to analyse a file whose
blanked form no longer parses, so a future desync is reported, not acted on.
Routing follows the scope: policyPrefixes now names adapters/, scripts/ and
src/ instead of the three scripts/ subdirectories and src/server/ws/. The
check reads these files rather than importing them, so the import graph
cannot select the lane on its own. `does not widen docs, policy, or coverage
lanes` split in two — its fixture selects the policy lane through its own
files now, so the dependent-must-not-widen invariant moved to a desktop
fixture that still shows it.
Verified by mutation, each reverted from an explicit backup: planting
`plantedProbeSymbol` into one file per root reported all four and failed
check:policy; reverting each of the three lexer fixes failed exactly its own
regression test; removing 'src/' from policyPrefixes failed the routing test
and made change-policy report policy=false for a src-only diff.
check:policy is 223 pass / 0 fail in 8.6s, up from 2s — the scan is 3.8s
over 2150 files and the planted-import test covers every one of them.
Found by widening scripts/pr/dead-imports.ts past src/server/ws once its
lexer stopped desyncing. None of these bindings appear anywhere in their own
file; several are the tail of a move that left the import behind, and
useMergedTools still lists replBridgeEnabled in a dependency array after the
useAppState call that set it was stubbed out.
Nine statements lost their last binding. Each target was checked for a
top-level side effect first: node:fs, node:fs/promises and bun:bundle have
none, ws/events.js is a type-only import that never emits, AppState.tsx,
model/model.ts and markdown-style.ts declare only, and utils/config.ts does
register a cleanup but is imported by 157 other files in src, so
attachments.ts dropping it cannot unregister anything.
No behavior change intended and none observed: src is 2995 pass / 37 skip /
30 fail across 273 files before and after, with the 30 failures identical
test for test, and adapters is 437 pass / 0 fail both times. The 30 are
pre-existing and unrelated.
Nothing checked src/ for unreferenced imports. desktop/tsconfig.json sets
noUnusedLocals and eslint covers desktop/ only; the root tsconfig.json sets
no such option and nothing installs typescript or bun-types at the root, so
no tool reads it at all. Splitting handler.ts left nine imports whose
symbols had moved out, and a human found them by reading the diff.
Measured before choosing. Under a temporary tsconfig extending the root
one, tsc reports 3225 errors over src/ and scripts/ before noUnusedLocals
and 3871 after — 646 net, on a baseline that already fails. That option
would land disabled, so this adds a narrow check instead: imports only,
one directory.
The analysis is lexical like module-graph.ts, but it blanks comments and
literal text first, so a symbol kept alive only by a comment still reports
dead — the exact shape the handler.ts split left behind. Template
substitutions, `//` inside a URL string and quotes inside a regex literal
must survive that blanking or a live import reads as dead;
src/utils/terminalShellEnvironment.ts is the last one and mis-lexing it
blanked 130 lines. Where blanking desyncs anyway the result no longer
parses, so the file is reported as degraded rather than mis-analysed: 6 of
2149 files repo-wide, none under src/server/ws.
src/server/ws/ also joins policyPrefixes. The check reads its files rather
than importing them, so the import graph cannot route a ws-only diff to
this lane, and without the prefix the check would never run on the diffs it
was written for.
Verified by mutation, each reverted from an explicit backup:
- planted `import { randomUUID }` into src/server/ws/events.ts →
check:policy 216 pass / 1 fail, reporting
"src/server/ws/events.ts:1 randomUUID"
- disabled line-comment blanking → 1 fail; block-comment blanking → 2 fail;
regex-literal detection → 2 fail (the extra one is the desync guard)
- pointed DEAD_IMPORT_ROOTS at a missing directory → 1 fail, so the check
cannot silently scan nothing
- dropped src/server/ws/ from policyPrefixes → 1 fail, and change-policy
reports policy=false for a ws-only diff
check:policy is 219 pass / 0 fail; src/server/__tests__/websocket-handler
.test.ts is 87 pass / 0 fail.
Bun auto-loads the cwd `.env`. A developer `.env` that sets
CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC makes isEssentialTrafficOnly()
true, so providerFetch throws MarketUpstreamError before the stubbed
upstream is ever called: 3 pass / 16 fail at the repo root, 19 pass in a
worktree without `.env`.
Back up and delete the var in beforeEach and restore it in afterEach,
matching what market-install/market-api/market-service already do. The
`non-essential traffic gate` describe still sets it to '1' in its own
(inner, later-running) beforeEach, so the gate stays covered.
Verified 19 pass / 0 fail with the var inherited from `.env`, with an
explicit =1, and with it cleared; and 1746 pass / 0 fail across all 78
files in src/server/__tests__/ under both env states.
One `thinking` message carries two granularities. handler.ts:2956 forwards a
`thinking_delta` fragment, which the client must concatenate raw to rebuild one
thought. handler.ts:2787 forwards a block the CLI already finished, which is a
separate thought. The client could not tell them apart and concatenated both, so
two finished blocks rendered as "plan the fix carefullythen run tests" — the
words run together on screen.
30a55a173 copied that concatenation into the history-mapping path, where every
block is finished, so the transcript view glues them every time. Its test then
copied the glued string into its expectation, which pinned the missing separator
as correct: anyone fixing it first has to argue with a green assertion. That is
the same failure mode as 2262973a4 asserting `getByText('deepseek-reasoner')`
while another model's number was on screen, a day apart.
The emit site now says which kind it sent (`complete` on the wire) instead of
leaving the renderer to infer it, and both merge sites go through one
`joinThinkingContent` so they cannot drift. Merging adjacent blocks into a single
bubble stays — that part was deliberate and tested; only the separator was
missing.
Three mutations, each caught: dropping the separator for finished blocks reddens
two store tests, adding one for fragments reddens the streaming tests, and
removing the server's `complete` flag reddens the translate golden.
That last one only works now because the golden had no scenario for the
whole-block emit path at all — it covered `thinking_delta` and nothing else, so
the flag could have been dropped with every check green. Added one.
62f648cb2 put a help button next to the base URL input and labelled it "Base URL
help" / "接口地址填写说明". That name legitimately contains the field's own label,
so getByLabelText(/Base URL|接口地址/i) started matching two elements and six
providers tests failed. They fail the same way on main — confirmed against a
temporary worktree there, 6 failed / 104 passed — so this is 62f648cb2's
breakage, not the merge's.
The accessible names are right: a screen reader user hears "Base URL, edit text"
and "Base URL help, button". It is the queries that were loose — they wanted the
input and never said so. Switched to getByRole('textbox', { name }), matching
line 1849 in the same file, which already did this and never broke.
Not a loosened assertion: removing htmlFor from the label reddens seven tests, so
the association between label and input is still what they pin.
Same conflict shape as before — the pre-split Settings.tsx against the shell.
Main's four hunks split across two panels: the rail's comment and its new
data-testid plus --settings-tab-offset margin stay in Settings.tsx, while the
non-essential-traffic toggle and its two destructured store fields go to
settings/GeneralSettings.tsx. It reuses SETTINGS_CHECKBOX_INPUT_CLASS and
SettingsCheckboxMark, which that module already imports from ./shared.
The six generalSettings failures carried over unchanged and still reproduce on
main; they are 62f648cb2's, not this merge's.
One conflict, same structural cause as the last merge: git offers the pre-split
Settings.tsx against the 183-line shell. All four of main's hunks belong to
ProviderFormModal, which now lives in settings/ProviderSettings.tsx — the useId
import, the addToast on a successful model fetch, and the base-URL field growing
an explicit label plus a help tooltip. Ported there; tsc caught the one import
(useUIStore) the move needed.
Everything else auto-merged, including the two files both sides changed:
chatStore.ts keeps main's pushAssistantHistoryThinking alongside the removal of
the content-equality replay guard — different functions, history mapping versus
the live path — and the five locales land at 2521 keys each.
Note: six generalSettings tests fail after this merge and they fail identically
on main. 62f648cb2 added an IconButton labelled 'Base URL help' next to the base
URL input without updating generalSettings.test.tsx, so getByLabelText(/Base
URL/i) now matches both. Confirmed against a temporary worktree at main: 6
failed / 104 passed there too. Not introduced here, and fixed separately.
Add a "Block non-essential external traffic" switch in Settings > General,
enabled by default, that stops all non-essential outbound requests
(telemetry, update checks, official registry, marketplaces, preconnect).
Model API calls to user-configured providers are never affected.
The setting lives in cc-haha/settings.json and is mapped into the server
process env (CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC) at startup, so
spawned CLI sessions inherit the existing essential-traffic gate.
Closes gaps found during review: plugin autoupdate (github.com /
downloads.claude.ai), WebFetch domain_info preflight, market provider
fetches, api preconnect, desktop auto-update check, and official
marketplace auto-install (whose policy_blocked state is cleared when the
toggle is turned off again).
The existing rule — production changes need a same-area regression test — is
satisfied by exactly the tests that failed to hold. ContextUsageIndicator.tsx was
fixed three times in ninety minutes; all three fixes shipped tests, and all three
tests covered only their own hop. 21 of the last 70 fix commits edit lines
another fix wrote within 30 days.
Coverage was never the missing signal. That file sits at 87% branch coverage.
What the tests had in common was shape, so this writes the shape down, with the
commit behind each rule:
- drive the transition instead of assigning the state it produces (component
tests call setState 744 times and a real store action 3 times, and assigned
state cannot expose "A did not update B")
- assert the invariant, not today's output (2262973a4 asserted the bug and the
next fix inverted that exact line)
- cover both directions of any drop/merge rule (the replay guard was only ever
tested for "discard the replay")
- test the join, not each end (deleting ChatInput's refreshNonce term left 314
tests green)
- never retune an existing test's inputs to keep it green (128f75ab5 changed five
tests' props rather than accept they described unreachable states)
- do not mock the module under test
- comparing content to decide identity means the identity exists upstream
Plus the two blind spots worth checking rather than trusting: desktop/electron is
not instrumented at all, and Bun's LCOV emits no branch records, so src/ and
adapters/ report 100% branch coverage for data that was never collected.
The desktop file gains the module-placement rules the Settings split and the
reachability guard produced.
appendAssistantTextMessage scanned the entire message array and discarded any
incoming text byte-identical to an already-hydrated reply. Live text never
carries a transcriptMessageId, so the guard was permanently armed, and a turn can
legitimately produce the same short string twice — "Done.", "好的", a one-line
command result. mergeRestoredTranscriptMessageIds back-fills a transcript id onto
the first one by exact content match, so the first reply becomes the poison pill
for the second: the user watches it stream in and it vanishes at
message_complete, with no recovery path.
Content equality cannot separate a replay from a repeat. d39e82b62 tried bounding
the scan to the current turn and was reverted in 3a630db11 — it still dropped
same-turn repeats and disarmed the guard entirely whenever a user_text sat at the
tail. No scan boundary fixes this, because the distinguishing signal is identity,
not text.
Identity is already enforced, one layer up and in the same commit that added this
guard. de52656bb's real fix is uuid dedup in conversationService
(isReplayedSdkMessage). Verified it cannot be bypassed: the SDK channel has a
single ingress at handler.ts:583, handleSdkPayload drops every already-seen uuid
at conversationService.ts:1047 before parsing continues, and its 2000-uuid window
covers the worst case de52656bb measured — 858 messages replayed 31 times.
So the whole-array scan goes. The narrower tail guard above it stays: it only
fires while the tail is still the hydrated message, which a same-turn repeat
separated by a tool call never is. The thinking-block guard also stays, because a
thinking block carries no id and has no identity check to defer to.
The replay test from de52656bb now asserts the real contract — replayed text is
appended, identity is the server's job — rather than a client behaviour that the
server makes unreachable. Re-adding the guard reddens both it and the new
repeat test.
ContextUsageIndicator.tsx was fixed three times in ninety minutes — 2262973a4
(switching models blanked the percentage), b4807697f (after the switch the number
never moved again, and the stale number was relabelled with the new model's
name), 128f75ab5 (a brand-new session spun on open). Each shipped a test, and
each test covered only its own hop.
The sharpest evidence that more tests was never the answer: 2262973a4's test
asserted `getByText('deepseek-reasoner')` at a moment when the screen showed
kimi-k2.6's 21% — it wrote the second bug in as a passing assertion, and
b4807697f had to invert that exact line 63 minutes later. 128f75ab5 in turn had
to rewrite five other tests' props (messageCount={0} -> {1}, chatState="idle" ->
"thinking") because those combinations never described a real session; they were
parameters that happened to reach a branch.
Coverage was not the gap either: the file sits at 87.4% branch coverage.
So this is shaped the other way round — one invariant, re-checked after every
transition, instead of a per-hop expected value that can be edited into agreement
with the code. State is driven only through handleServerMessage and setSelection,
so the inputs cannot be tuned the way hand-written props can.
Measured against the three regressions, mutating each one back in:
removing the new-session fetch gate old tests: 3 red this test: red
removing ChatInput's nonce addition old tests: 314 green this test: red
taking the model label from the runtime old tests: 1 red this test: red
The middle row is why it renders <ActiveSession /> against the real stores. Half
of b4807697f's repair lives in the store and in ChatInput's props arithmetic
(refreshNonce = compactCount + runtimeConfigReadyCount, ChatInput.tsx:1445); a
component test can only hand-write refreshNonce={1} and skips that link. Server,
store and indicator each have a test for their own end of that chain, and nothing
crossed it.
One thing the first draft got wrong, worth recording: driving the turn as a bare
status flip left messageCount at 0, and the fetch gate is `messageCount > 0 ||
chatState !== 'idle'`, so the sequence closed the gate again on the way back to
idle. That state cannot occur in a real session — the same trap 128f75ab5's prop
edits fell into. It now drives real content frames.
The picker that appears under each model field is easy to miss, so surface
a success toast pointing the user at the dropdown once a non-empty list
loads. Empty lists keep the existing inline notice.
`check:agent-flow` proves the protocol with the mock CLI, which is what makes it
CI-safe and lets any contributor run it with no credentials. It cannot prove the
thing this product actually is: a desktop agent talking to a real model. That can
only run where the credentials are, so this lane is local and manual by
construction — registered in no quality-gate mode, referenced by no workflow, and
live.test.ts fails if either changes.
Six scenarios, sharing the existing harness rather than a second copy of it:
first turn, permission allow, permission deny, interrupt, reconnect, and history
recovery. Prompts induce the behaviour instead of dictating it, and assertions
only look at protocol shape and side effects on disk — never at generated text —
so the lane passes on any provider, including a local one. The three flows left
out (api-error, tool-error, runtime-select) each carry a written reason, because
a silently missing flow reads as a covered one.
Spending someone's quota is the failure mode worth engineering against, so the
runner refuses to guess: no implicit fallback to the active provider, an ambiguous
selector is an error rather than a pick, and without --yes it prints the provider,
model and config path it would use and exits without sending anything. User state
is copied into a throwaway config dir and the real ~/.claude is fingerprinted
before and after — a run that writes to it fails loudly instead of being cleaned
up quietly.
Not yet run end to end: the local LM Studio endpoint answers 502 here, so the six
runners have only been verified for structure. Target resolution, the
confirmation gate, and lane placement are covered by 14 tests that need no
provider at all.
Subagent transcripts persist streamed thinking snapshots, and the
history-mapping path pushed every block verbatim, so the subagent run
page rendered long runs of repeated empty "thought" bubbles. Give the
history path the same guards as the streaming path: drop blank blocks,
drop verbatim replays, replace prefix-growth snapshots, and merge
adjacent blocks while keeping deterministic ids for stable React keys.
4f9fec876 added this workflow with `cron: '0 18 * * *'`. That was the wrong call
to make unilaterally: the repository had no scheduled workflow at all before it,
so this was not one more cron among several but the introduction of recurring CI
spend — about ninety minutes per run — on a schedule nobody asked for.
The reasoning for the sweep still holds: a per-PR gate only covers what the diff
reaches, so it is blind to checks no recent PR selected and to failures that only
appear when the whole suite runs together. Keeping the workflow on
`workflow_dispatch` keeps that one click away without deciding for the maintainer
when to spend the time.
pr-quality-workflow.test.ts now asserts the absence of `schedule:` and `cron:`
rather than their presence, so a schedule cannot drift back in unnoticed —
verified by adding the cron back and watching the test go red. The docs' four-tier
table renames the tier accordingly; calling it "Nightly" when nothing runs nightly
is exactly the kind of comment that outlives its code.
- Add both gateways as featured presets pointing at their Anthropic-compatible
roots (https://api.fenno.ai, https://api.qnaigc.com) with auth_token strategy
- Ship no default model ids: the available catalog depends on the plan the user
bought, so they fetch the live list and pick one instead
- Keep modelContextWindows, which stays useful per picked model id and covers
ids the built-in table cannot resolve (claude-opus-5, namespaced 七牛云 ids)
- Add both as sponsors in the English and Chinese READMEs, and list them in the
preset docs
Two leftovers a review found, neither with any runtime effect.
src/server/ws/handler.ts kept nine imports whose only consumers moved out in
eabdd12c3 / f7debb0b4 / 35d6021bd. Nothing caught them: the root tsconfig sets no
`noUnusedLocals` (desktop's does), and the eslint config added in f36c9cb49 only
covers desktop/ — so nothing checks `src/` for dead imports at all. Worth its own
change; this just clears the ones the splits created.
537b48ba7 removed nine translation keys with BackgroundTasksBar but missed four,
because that component picked them through a ternary —
`t(count === 1 ? '...CountOne' : '...CountMany', { count })` — and the deletion
scan matched only `t('literal')`. It checked that the keys it deleted had no
other consumer, but not that the dead component had other keys; the error was in
the safe direction and left twenty lines of dead translation. All five locales
stay aligned at 2518.
Review caught a way the previous listener could kill a user's running shells with
no reload at all. installMainWindowNavigationGuards cancels external http(s)
navigation in `will-navigate` and hands it to the system browser, but Chromium
dispatches DidStartNavigation before the throttle that cancellation runs in — so
a blocked navigation still emits `did-start-navigation`. Dropping a URL anywhere
outside the composer (useComposerFileDrop only preventDefaults over the composer
panel with files attached) would have started a top-level navigation, had it
cancelled, and taken every `npm run dev` and its children with it while the app
stayed on screen.
`did-navigate` fires only once a main-frame navigation has committed, so a
cancelled one never reaches it, and Electron does not emit it for in-page
navigation — which also removes the need for the same-document predicate. The
reload paths this cleanup exists for all commit, so they still reap.
rendererNavigation.ts goes with the predicate: previewLifecycle.ts was its only
remaining caller, so it returns to the standalone form it had before.
Mutating the listener back to `did-start-navigation` reddens the cancelled
navigation case, which is what shows the test pins the commit boundary rather
than merely the existence of a listener.
This reverts commit d39e82b62.
Review found the narrowed guard is wrong in both directions, and I reproduced
both against the store:
same-turn repeat -> assistant_text "Done." count: 1 (expected 2)
user_text at the tail -> "Done." count: 2 (the old full scan gave 1)
The first is the bug d39e82b62 claimed to fix, still present: within one turn the
agent can say "Done.", call a tool, and say "Done." again, and a mid-turn
loadHistory back-fills a transcriptMessageId onto the first — so the second is
still dropped. "A genuine repeat can only arrive after the last user_text" is
simply false for multi-block turns.
The second is a regression I introduced. currentTurnStartIndex returns
messages.length whenever a user_text sits at the tail, which disarms the scan
completely — and a user_text lands there mid-turn on two real paths:
appendOptimisticQueuedUserMessage (chatStore.ts:2145, user types ahead while the
agent runs) and appendReplayedUserMessage (:3025, an older turn's prompt during a
whole-buffer replay).
Content equality cannot separate a replay from a repeat, so no scan boundary
fixes this. The real signal is identity — toolUseId already dedupes tool_use, and
the server's uuid check (conversationService.isReplayedSdkMessage, added in the
same commit as this guard) is the actual defense. Fixing it properly belongs in
its own change, not smuggled into a merge.
Reverting restores main's behavior exactly: the original drop is pre-existing
there, so this trades a known state for a known state instead of shipping a new
regression.
`workspace-service.test.ts` cleared the filesystem access-root registry in
afterEach only, so it assumed an empty registry on entry. That registry is one
module-level Set shared by every test file in the process, and these tests open by
asserting a path is *not* reachable — a precondition they cannot assume, only
establish.
In a full `bun test ./src/server` run they inherited the entire macOS temp root.
title-service.test.ts:463 and sessions.test.ts:2860 both create a session on the
bare os.tmpdir(), and registering a session's workDir is exactly what
sessionService is supposed to do — so by the time this file ran, every path under
/var/folders/.../T was allowed and "rejects a file outside the workdir" failed.
Confirmed with a probe: isWithinRegisteredFilesystemRoot(tmpdir/...) was true at
that point, while '/' was not, so this is test bleed and not a widened sandbox.
Pair the clear with a beforeEach, matching filesystemAccessRoots.test.ts and
h5-access-auth.test.ts. Removing it again reddens the test under a full run, which
is what shows the fix is the isolation rather than the assertion.
The failure predates this branch: main fails the same test, plus three more.
Server suite now 2144/0, verified across three consecutive full runs.
Two real conflicts, one of them structural.
desktop/src/pages/Settings.tsx — git offered the whole 4128-line pre-split file
as "theirs" against the 183-line shell, which is not a merge anyone can review.
Resolved by keeping the split and porting main's nine hunks to where that code
now lives: the rail width, its comment and TabButton's padding stay in
Settings.tsx; the ModelIdCombobox import and the five ProviderFormModal changes
(the canFetchModels split into hasModelsBaseUrl/hasModelsApiKey, modelPickerItems
becoming modelPickerGroups, the two new hint branches, and the Input+Dropdown pair
collapsing into ModelIdCombobox) go to settings/ProviderSettings.tsx.
Verified rather than assumed: every line main added is present somewhere in the
split, every construct it removed is gone (modelPickerItems, canFetchModels =
Boolean(...), the supplementary Dropdown), and the import specifier gained the
level the new directory needs.
desktop/package.json — taking main's version wholesale dropped the eslint setup
from f36c9cb49. Reconstructed with both sides: main's six prosemirror packages and
the three eslint devDependencies, with lint back to eslint + tsc.
Everything else merged clean, including the files both sides touched:
src/server/ws/handler.ts (main's four title-generation changes all sit in code the
three splits left behind), desktop/src/stores/chatStore.ts (main's mention/
repository-launch state alongside the turn-scoped replay guard), and the five
locales — 2526 - 9 removed + 5 added = 2522, still aligned across all languages.
Checks: desktop lint + 4049 tests + build, check:electron, check:policy all green.
The one server failure, workspace-service.test.ts "rejects a file outside the
workdir", fails identically on main — confirmed against a temporary worktree at
main, which fails it plus three more. It passes 3/3 in isolation; the registry it
asserts on is a module-level Set shared across test files.
Coverage answers "is this tested", never "should this exist", and the difference
cost real work. BackgroundTasksBar, SessionTaskBar and TeamStatusBar lost their
last import in 56a4be3d1 when SessionActivityPanel replaced them. Nothing noticed:
598b968ee then wrote tests for two of them — "cover the components left at zero" —
purely to lift changed-lines coverage past the gate, and c712f5285 restyled all
three during the UI redesign. 577 component lines plus 348 test lines were kept
alive for code no user could reach, along with nine translation keys carried in
five locales.
Delete all of it, and add the check that would have caught it: every .tsx under
src must be reachable by static import from a script tag in index.html or
gallery.html. Reading the entries out of the HTML rather than hardcoding them
means a new entry brings its whole subtree with it. The allowlist is empty and
should stay that way.
Scoped to .tsx deliberately. The .ts side has entry points a static graph cannot
see — workspaceDiffHighlight.worker.ts is a `new Worker(new URL(...))` target and
src/preview-agent/** is built into its own bundle — so covering it needs an
allowlist, which is where this kind of check goes to die.
Two mutations: putting BackgroundTasksBar.tsx back names it exactly, and breaking
ENTRY_HTML trips the entry-point guard first so the reader is not sent hunting
through 160 falsely-unreachable components.
Also drops the now-dangling desktop/src/mocks/ coverage exclusion, deleted in
33df50b9c.
The reconnect-replay guard in appendAssistantTextMessage scanned the entire message
array for a hydrated assistant_text with the same trimmed content, and live text
never carries a transcriptMessageId — so the guard was permanently armed against
every reply. Any answer matching an earlier one exactly was discarded at
message_complete, after the user had already watched it stream in.
Short acknowledgements repeat constantly ("Done.", "好的", a one-line command
result), and it does not take an old session: mergeRestoredTranscriptMessageIds
back-fills transcript ids onto live replies by exact content match, so the first
identical reply in a purely live session becomes the poison pill for the second.
There was no recovery path either. appendedCompletionMessage stays false, so the
completion notification is skipped, and the follow-up loadHistory takes the
live-merge branch — annotate ids, drop duplicates, insert parent tool messages,
append goal events — none of which can re-add a missing assistant_text. The reply
stayed gone until the tab was closed and reopened.
Bound the scan to the current turn. Replayed text belongs to a turn the user
already sent, so its hydrated copy sits before the newest user_text; a genuine
repeat can only arrive after it. The boundary separates the two exactly, and comes
from `messages` rather than the session's streamAttemptStartIndex, which none of
the twelve call sites pass.
Two mutations: removing the turn bound reddens the new test, and removing the
guard entirely reddens the existing replay test at chatStore.test.ts:1496 — the
pair is what shows the boundary distinguishes replay from repeat instead of just
disabling the guard. The chatStore golden snapshot is unchanged.
webContents.once('destroyed') was the terminal service's only lifecycle hook, and a
reload does not emit it. The app reloads the renderer deliberately on
render-process-gone and on sustained unresponsive (rendererLifecycle.ts), and both
ErrorBoundary and StartupErrorView give the user a reload button. The renderer's
session map is module state wiped by that reload, and kill() needs a session id the
reloaded renderer no longer has — so every shell kept running with its children
(dev servers, watchers, builds), invisible and unkillable, until before-quit.
Subscribe to did-start-navigation as well and run the same teardown, mirroring
installPreviewCleanupOnRendererNavigation, which already solved exactly this for
the preview.
The main-frame / same-document predicate moves into rendererNavigation.ts and both
callers anchor on it. Electron has shipped this event in two shapes — details
object and trailing positional args — and reading only one silently disables the
cleanup on whichever build uses the other; that is not a rule worth spelling out
twice.
`off` rather than a second `removeListener` overload: a target type with two call
signatures is not structurally assignable from Electron's overloaded
removeListener, so the obvious spelling rejects the real WebContents.
Three mutations verified against the new tests: dropping the subscription (2 red),
ignoring isSameDocument so in-page routing kills live shells (2 red), and skipping
the detach so listeners accumulate per terminal (1 red).
suppressClickRef is set when a drag starts and cleared only inside handleTabClick,
which is reachable only from a tab's own onClick. finalizeDrag clears every other
drag ref but not this one. Release a drag away from any tab — below the strip or
outside the window — and no tab handler runs, so the flag survives and the user's
next click on any tab hits the early return and is silently lost.
Cleared in handleTabMouseDown instead: every genuine click is preceded by its own
mousedown on the tab, so the suppression can only ever apply to the gesture that
set it. Deliberately not cleared in finalizeDrag — `click` fires after `mouseup`,
so that would defeat the suppression it exists for.
Only a vertical exit leaks. The dragged tab carries translateX and stays under the
cursor, so a horizontal drag ending over a neighbour consumes the click normally.
TraceList carried a private formatDuration that stopped at seconds, so the same
TraceSessionSummary.totalDurationMs under the same t('trace.modelTime') label read
"739.0s" in the list and "12m 19s" in the detail view, which uses the shared
formatDurationMs. A two-hour trace printed "7200.0s". The private formatCompact
had the same problem for tokens: it rounds above 100, so 150500 read "151k" in
the list and "150.5k" everywhere else.
Both forks are gone; the list now imports from lib/trace/formatters like
TraceSession.tsx already did.
The existing assertion moves from '4.7s' to '4.71s' — it was pinning the fork's
output, which is why nothing caught this. The new case uses 739_000 ms, since
only durations past a minute expose the difference.
Last of the panel extractions, and the largest. The list and the add/edit modal
come out together on purpose: they share forty-odd helpers for reading and
rewriting settings.json — auth env, model mappings, the 1M-context marker,
context windows, tool-search and beta flags — so separating them would have meant
either duplicating that layer or inventing a module for it before anything
needed one. Only ProviderSettings is exported; the rest was always private.
Of the 1963 moved lines exactly three differ, and both kinds are forced by the
move: the `export` prefix, and two `import('../api/providers')` expressions that
gain a level with the new directory. Dynamic imports live in function bodies, so
the mechanical rewrite of the import block does not reach them — tsc caught both.
paletteEscapes.test.ts guards an allowlist of files permitted to use Tailwind's
stock palette, and it went red exactly as designed: the two `neutral` classes in
the QR panel had moved out of Settings.tsx. The entry follows the code to
settings/H5AccessSettings.tsx; the classes themselves are untouched.
Settings.tsx 2186 -> 183 — the shell, TabButton, and the two small panels that
are nothing but a header plus a list.
- Trim redundant intro text, install-guide badge, and test model names from the desktop preview
- Move the sponsorship section right below the preview for sponsor visibility
- Shorten the TeamoRouter sponsor copy
Takes the repo and social-link constants with it — nothing else in Settings.tsx
referenced them. isValidHttpProxyUrl stays in ./shared because the General panel
needs it too.
Pure move: both extracted ranges are byte-identical in the new module.
lint clean, 174 Settings tests pass.
Settings.tsx 2583 -> 2186.
At 1342 lines it was the largest of the seven panels and the most frequently
edited. It takes the four output-style label helpers and the network-timeout
bounds with it, since nothing else in Settings.tsx used them.
Pure move: all three extracted ranges are byte-identical in the new module.
SettingsOutputStyle.test.tsx now imports GeneralSettings from its new home
rather than through Settings.tsx, so the re-export indirection is gone too.
lint clean, 174 Settings tests pass.
Settings.tsx 4034 -> 2583.
SETTINGS_CHECKBOX_INPUT_CLASS, SettingsCheckboxMark and isValidHttpProxyUrl are
the only declarations in Settings.tsx that more than one panel uses. They move
ahead of the remaining panel extractions: leaving them behind would make every
extracted panel import from the file that imports it, and that cycle is the kind
of fragility the split is meant to remove.
Pure move — the three declarations are byte-identical apart from the `export`
prefix. lint clean, 174 Settings tests pass.
Settings.tsx held seven unrelated panels in 4639 lines. H5 access comes out
first because it is provably self-contained: its eight URL/port helpers have no
other caller in the file, and it shares no helper with any other panel, so the
move needs no dependency injection and cannot change behavior.
Verified as a pure move, not a rewrite — both extracted ranges (the 96 helper
lines and the 459-line component) appear byte-identical in the new module, the
only edit being the `export` prefix. tsc and eslint are clean and the 174 tests
across the nine Settings suites pass, including the H5 panel's own coverage for
the QR launch URL, token generation, the LAN risk confirmation and fixed ports.
Settings.tsx 4639 -> 4034.