Skip to content

refactor(client): merge the two companion worker close-code normalizers - #3552

Merged
thymikee merged 2 commits into
mainfrom
refactor/worker-close-code-merge
Oct 13, 2026
Merged

thymikee merged 2 commits into
mainfrom
refactor/worker-close-code-merge

Conversation

@thymikee

@thymikee thymikee commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Why

normalizeCloseCode widened a bridge-supplied code into the 1001-1015 range, normalizeOutgoingCloseCode then narrowed anything unwelcome to 3001 — so the first stage's only surviving outputs were 1000 and the 3000-4999 range, exactly what the second stage passes through. One function now answers the question the pair was asked, and handleBridgeWebSocketClose no longer normalizes before the normalizer.

Behaviour

Unchanged. The two stages composed to 1000 | 3000-4999 | 3001 for every input: checked by running old-pair and merged function side by side over 6,044 inputs (the whole [-20, 6000] range plus -2^31, 2^31-1, ±2^53, 1e18, 1000.5, NaN, ±Infinity) — 0 mismatches. client-companion-tunnel-worker.test.ts still covers the close path unchanged, so no new test is owed for a merge with no observable difference.

Validation

At 063fc974a: lint, typecheck, the worker suite, and check:affected --run pass. Gross diff 27 lines in one file.

The one Coverage red is a pre-existing timing flake in packages/platform-apple/src/snapshot-source/lifecycle.test.ts (100 ms maxDurationMs budget against a responseDelayMs: 80 fixture, real timers); the same test fails on unrelated branches and on main, and this change does not reach it.

Pulled out of #3551, where it was incidental, so that PR carries only the stop-confirmation change.

View guided diff Turn on auto-fix

normalizeCloseCode widened a bridge code into 1001-1015 and
normalizeOutgoingCloseCode then narrowed everything unwelcome to 3001, so the
pair composed to `1000 | 3000-4999 | 3001` — exactly what the second stage alone
answers for any input. One normalizer, and the bridge-close handler stops
normalizing before the normalizer.
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.17 MB 5.17 MB -104 B
Package (unpacked) 5.17 MB 5.17 MB -104 B
Package (download) 1.55 MB 1.55 MB -21 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 20.8 ms 21.2 ms +0.3 ms
CLI --help 60.9 ms 61.9 ms +1.0 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread src/client/client-companion-tunnel-worker.ts Outdated
@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready. The merge of the two close-code normalizers into one looks correct at 063fc97, and all 19 checks pass. The Coverage flake the author mentioned in lifecycle.test.ts is unrelated to a single-file change in src/client/client-companion-tunnel-worker.ts, and there are no conflicts.

Not blocking: the comment at https://github.com/callstack/agent-device/blob/063fc97/src/__tests__/client-companion-tunnel-worker.test.ts#L834 still describes two normalization stages, but there is now one, so it could say that stream-fractional pins the non-integer rejection (without it, 3500.5 would reach the wire truncated to 3500 instead of 3001); take it or leave it.

I did not re-run the 6,044-input differential, and I ran no local or live tests. I checked equivalence by hand across every input class. Existing tests only exercise code 1000, so nothing pins the 3001 fallback; that is fine for a pure refactor.

The inline thread on the doc comment naming the RFC 6455 subranges (#3552 (comment)) is fixed at this commit, so you can resolve it.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 13, 2026
@thymikee
thymikee merged commit 75b8579 into main Oct 13, 2026
24 of 25 checks passed
@thymikee
thymikee deleted the refactor/worker-close-code-merge branch October 13, 2026 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant