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.
Relakkes Yang