Repository navigation
fix(sse): widen compression worker gate to accept structured-clone-safe values (#13154) - #14391
Merged
Merged
Conversation
…fe values (#13154) isStrictlySerializable rejected any undefined value and any non-plain object (Date/Map/Set/RegExp) before ever checking whether structuredClone (what worker.postMessage actually uses) would accept it. strategySelector.ts's runCompressionAsync always builds the 9-key workerOptions object with every key explicitly present, so provider: undefined alone (the common case when no provider is resolved yet) rejected almost every real stacked/rtk/standard compression call, forcing it onto the main event loop instead of a worker thread. Widen isStrictlySerializable to accept undefined and the structured-clone- native Date/Map/Set/RegExp types (copied, not walked, by structuredClone), while still rejecting functions and Symbols. Companion fix: compressionWorker.ts's stacked-mode branch called applyStackedCompression directly on the raw job body, skipping the adaptBodyForCompression/restore step that the sync in-process path (strategySelector.ts's runCompression) always applies for Responses input[] and Kiro conversationState envelopes. Because the over-strict gate above meant essentially no real stacked call ever reached the worker, this was dead code until this fix; without also fixing it, widening the gate would have shipped a live regression that miscompresses Responses/Kiro bodies whenever they are now correctly routed to the worker.
… new-code complexity ratchet (#13154)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #13154
Root cause
isStrictlySerializableinopen-sse/services/compression/compressionWorkerProtocol.tsdidif (typeof value !== "object") return false;before anything else, so anyundefinedvalueanywhere in the tree rejected the whole object — and any non-plain object (
Date/Map/Set/RegExp) failedisPlainObjectand was rejected too, even thoughstructuredClone(whatworker.postMessageactually uses) accepts all of these natively.strategySelector.ts'srunCompressionAsyncalways builds the 9-keyworkerOptionsobjectwith every key explicitly present (
model,supportsVision,providerTransport,provider,imageTransportFidelity,sourceFormat,targetFormat,compressionStage,config), soprovider: undefinedalone (the common case when no provider has been resolved yet) rejectedalmost every real
stacked/rtk/standardcompression call, forcing the heavy caveman/rtkpipeline onto the main event loop instead of a worker thread.
Fix
isStrictlySerializable: acceptundefinedimmediately (it round-trips throughstructuredCloneas an absent/undefined key), and acceptDate/Map/Set/RegExp(copiednatively by
structuredClone, not walked recursively) before theisPlainObjectrejection.Functions and Symbols are still rejected; the existing path-based cycle detection (
seenadd/delete, fix(compression): track only the recursion path in isStrictlySerializable (#13154) #13423) is untouched.
Companion fix (found while validating this change, same PR):
compressionWorker.ts'sstacked-mode branch calledapplyStackedCompressiondirectly on the raw job body, skippingthe
adaptBodyForCompression/restorestep that the sync in-process path(
strategySelector.ts'srunCompression) always applies for Responsesinput[]and KiroconversationStateenvelopes. Because the over-strict gate above meant essentially no realstackedcall ever reached the worker, this branch was dead code — discovered when theexisting
preserves Responses bodies and hard-budget resultstest started failing once thegate widening actually routed that call to the worker for the first time (worker output
diverged from the sync path: off-by-one token count, missing hard-budget validation warning).
Without this companion fix, widening the gate alone would have shipped a live regression
(miscompressed Responses/Kiro bodies whenever routed to the worker). Fixed by mirroring the
same adapt/restore wrapping in the worker's stacked branch.
Regression test
New file
tests/unit/compression-worker-gate-13154-repro.test.ts(mirrors the plan-file'sproven repro exactly). Before the fix:
After the fix:
Gates run
node --import tsx/esm --test tests/unit/compression-worker-gate-13154-repro.test.ts→ 3/3 passnode --import tsx/esm --test tests/unit/compression/compression-worker.test.ts→ 13/13 pass (full suite, including the companion-fix regression)npm run typecheck:core→ exit 0node scripts/check/check-open-sse-typecheck.mjs→openSseTypecheckErrors=0, OKnpx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>→ 0 errorsnode scripts/check/check-file-size.mjs→ OK, no frozen file over its baselinenode scripts/check/check-changelog-integrity.mjs→ OKnpx eslint --no-config-lookup --config eslint.complexity.config.mjs(compressionWorker.ts, scoped) → 0 violationsnpx eslint --no-config-lookup --config eslint.sonarjs.config.mjs(compressionWorker.ts, scoped) → 0 violationscheck-complexity.mjs/check-cognitive-complexity.mjsratchets (pre-companion-fix baseline) → both OK, within baselineExisting tests aligned
tests/unit/compression/compression-worker.test.ts:rejects functions, symbols, classes, special objects, cycles, and non-finite numbersinto
rejects functions, symbols, cycles, and non-finite numbers(kept) plus two new testsasserting
Date/Map/Set/RegExpandundefinedare now accepted — this is alignment tothe corrected contract (these values are structured-clone-safe), not a weakening: the cycle,
function, Symbol and non-finite-number rejections are all still asserted.
strategySelector.ts's exact 9-keyworkerOptionsshape with
providerunset, assertingisCompressionWorkerEligible(...) === true.preserves Responses bodies and hard-budget resultsneeded no edit — it started passingagain once the companion fix above was added (see Root cause / Fix for the investigation).
Plan-file:
_tasks/pipeline/bugs/2-implementing/13154-fix-backend-compression-worker-gate-rejects-structured-cloneab.plan.md