refactor: extract daemon progress parsing#753
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
thymikee
left a comment
There was a problem hiding this comment.
Reviewed the extraction against the original sendSocketRequest handler — this is a faithful move with good seam design (daemon-client.ts keeps ownership of settled/timeout state, the helper only parses), and the new tests cover the cases that matter: split lines across chunks, progress-before-response ordering, post-settle input, and invalid-line request context. Overall LGTM. Two minor notes:
-
settledordering aroundsocket.end()— in the original code,settled = truewas set beforesocket.end(). In the extracted helper, the order is nowclearTimeout()→socket.end()→resolve(response), andsettledis only flipped inside theresolvewrapper. Ifsocket.end()ever synchronously emitted'error'/'close', the handlers insendSocketRequestwould still seesettled === falseand reject a request that was about to resolve. It's a narrow window in practice, but swapping toresolve(response)beforesocket.end()(or passing amarkSettledcallback invoked first) would restore the original guarantee. -
Test file location — the new test lives in
src/utils/__tests__/daemon-client-progress.test.ts, but the module under test issrc/daemon-client-progress.ts(note the../../daemon-client-progress.tsimport). Moving it tosrc/__tests__/would match where the module sits and keep the utils test dir scoped tosrc/utils.
CI is green across the board (typecheck, unit, integration, coverage, Fallow) with a couple of Smoke Tests still running at review time — nothing blocking from my side once those finish.
Generated by Claude Code
87295fd to
1c4cef0
Compare
|
Addressed the two review notes:
Re-ran:
|
|
Summary
Moves socket progress stream parsing out of
src/daemon-client.tsand intosrc/daemon-client-progress.ts, keeping daemon-client as the public facade for lifecycle, transport selection, timeout handling, and request dispatch. Adds focused coverage for split socket progress lines, response envelopes, ignored post-settle input, and invalid-line request context.Touched files: 3. Scope stayed within the daemon-client progress seam.
Refs #727
Validation
Focused daemon-client/progress Vitest coverage passed.
pnpm check:quickpassed.pnpm check:fallow --base origin/mainpassed with no baseline changes.pnpm formatcompleted, andgit diff --check/ staged whitespace check passed.