Skip to content

Sync wire diet: binary envelope frames + permessage-deflate - #1057

Merged
arul28 merged 1 commit into
mainfrom
ade/t3-sync-compression-c8368d49
Aug 10, 2026
Merged

Sync wire diet: binary envelope frames + permessage-deflate#1057
arul28 merged 1 commit into
mainfrom
ade/t3-sync-compression-c8368d49

Conversation

@arul28

@arul28 arul28 commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Tier 1C of the t3code competitor research, items 1 and the transport half of item 2. Builds on #1056's projection layer rather than duplicating it — this changes how bytes are framed, not what is in them.

What changed

Compressed envelopes travel as binary frames. encodeSyncEnvelope compressed the payload and then base64'd it into a JSON text frame — a flat +33% re-inflation of exactly the bytes it had just removed. Compressed envelopes now use a binary container (magic ADE1, uint32 header length, envelope-minus-payload as JSON, compressed bytes raw), gated on a new binaryEnvelopes hello capability. A peer that does not declare it keeps the base64 wire byte for byte.

The magic prefix is load-bearing, not decoration: wsDataToText has always decoded Buffer frames as utf8 and transports do deliver text that way, so sniffing four bytes is what keeps a text frame arriving as data on the text path.

Oversized envelopes chunk as binary too, which matters more than it looks — a text chunk base64s an envelope whose payload is already base64 and budgets its slice down to 3/4 to pay for the expansion, so the tax compounds precisely on the largest frames.

permessage-deflate on both sync WebSocket servers, with the application codec skipped when a peer negotiates it. The transport's dictionary persists across frames, so it compresses changeset traffic better than per-envelope compression can.

Measurements

All on 26.2 MiB of this machine's real crsql_changes rows, batched through the production batcher (selectChangesetBatchChunk) and encoded through the production encoder at the 720 KiB frame budget iOS negotiates.

catch-up (250-row) live broadcast (4-row)
legacy gzip@4KiB + base64 6.52 MiB 12.74 MiB
today, deflate@512 + base64 6.51 MiB 8.47 MiB
binary frames 4.03 MiB 5.60 MiB
−38.1% vs today −33.9% vs today

Per envelope in isolation the saving is the flat 25% base64 tax; the remainder is the chunk path no longer paying it twice. That path is not hypothetical — one real db_version group in this database is 11.4 MiB and cannot be split, because the pump must ack rows sharing a db_version together.

permessage-deflate, measured as bytes actually written to a real socket, live-broadcast sized:

bytes written
app-level deflate only (today) 7.34 MiB
both layers 5.31 MiB
transport only 4.14 MiB

Stacking the two is worse than either alone, which is why the application codec is skipped when the extension is negotiated.

A correction to the research report

The report's headline for this item — "today 2.08x, shared-window deflate 7.64x, 3.68x smaller than today" — is not reproducible, and the gap is a baseline error rather than a measurement error.

  • 2.08x is the legacy path, not today's. It is gzip at a 4 KiB threshold measured on small live-broadcast envelopes, where 89% (3080/3447) fall below the threshold and ship completely uncompressed. A peer that negotiates deflate@512 — which current iOS does — is at 3.2x–4.9x today, not 2.08x.
  • Context takeover is worth far less than advertised. Measured against today's negotiated path at matched batch sizes, an app-level shared-window deflate stream gives 1.35x at catch-up sizes and 1.62x at live-broadcast sizes. Deflate's 32 KiB window already captures most of the repetition inside a single 256 KiB envelope, so the benefit only appears once envelopes are small relative to the window — the variable the report did not isolate.

The measured decomposition, live-broadcast sized: raw 26.65 MiB → today 7.33 MiB → binary frames 5.51 MiB → streaming context takeover 4.52 MiB. Binary framing is the reliable, stateless win; context takeover adds 12–18% on top of it and only on small envelopes.

This also changes the design for the rest of item 2. permessage-deflate already is context takeover, done by the transport, and it measured better than the hand-rolled equivalent (4.14 vs 4.52 MiB) because it compresses raw JSON rather than already-deflated bytes and carries no per-envelope header. So for every peer that can negotiate it — browsers, the desktop renderer, Node sync peers — item 2 is complete with no per-peer stream state, no ordering coupling, and no reset story. A bespoke app-level stream is now only worth considering for iOS, which cannot negotiate the extension.

Compatibility

  • Both wire changes are capability-gated. binaryEnvelopes is declared only by iOS in this change; an older build never declares it and receives the identical base64 wire.
  • Browsers and the web client are untouched — they get permessage-deflate from the transport and never see a binary frame.
  • Shared types, sync host, and iOS models moved together, per this repo's contract-drift bug class.
  • Windows: no platform-specific code; framing and zlib are platform-neutral.

Verification

  • 566 sync tests pass, including the 179 syncHostService tests.
  • 16 new tests in syncBinaryFrame.test.ts: container round-trip, empty body, truncated and lying header lengths, header cap, non-object header, JSON-text-delivered-as-binary taking the text path, payload equality between the binary and base64 wires, uncompressed payloads staying JSON text, gzip and deflate, binary chunking under the frame budget, out-of-order reassembly, and chunk-header validation.
  • iOS xcodebuild BUILD SUCCEEDED against the current scheme.

🤖 Generated with Claude Code

ADE   Open in ADE  ·  ade/t3-sync-compression-c8368d49 branch  ·  PR #1057

Summary by CodeRabbit

  • New Features

    • Added support for compressed binary sync frames to improve data transfer efficiency.
    • Added binary chunking and reassembly for large synchronization payloads.
    • Desktop, CLI, and iOS clients can advertise and use binary envelope support.
    • Existing text-based synchronization remains supported for compatibility.
  • Bug Fixes

    • Improved validation for malformed frames, unsupported versions, invalid compression, and oversized payloads.
    • Enhanced frame-size tracking and WebSocket compression handling for more reliable transfers.
  • Tests

    • Expanded coverage for binary frames, compression, chunk ordering, validation, and compatibility scenarios.

Greptile Summary

The PR adds capability-gated binary framing for compressed sync envelopes and enables permessage-deflate on the sync WebSocket servers.

  • Adds the ADE1 binary container and raw binary chunking/reassembly.
  • Updates host encoding, parsing, byte accounting, and compression negotiation.
  • Adds matching iOS decoding and capability advertisement.
  • Expands protocol and binary-frame test coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/ade-cli/src/services/sync/syncProtocol.ts Adds binary envelope encoding, parsing, chunk assembly, and transport-compression coordination without an accepted follow-up finding.
apps/ade-cli/src/services/sync/syncBinaryFrame.ts Implements a bounded magic-prefixed binary container with defensive header decoding.
apps/ade-cli/src/services/sync/syncHostService.ts Integrates binary frame negotiation, parsing, byte accounting, and permessage-deflate-aware encoding.
apps/ade-cli/src/services/sync/sharedSyncListener.ts Enables the shared permessage-deflate configuration on the shared sync listener.
apps/ios/ADE/Services/SyncService.swift Adds matching iOS binary-frame decoding, chunk reassembly, and capability advertisement.
apps/desktop/src/shared/types/sync.ts Adds the shared binary-envelope capability constant.
apps/ade-cli/src/services/sync/syncBinaryFrame.test.ts Covers binary container validation, compression parity, chunking, and reassembly.
apps/ade-cli/src/services/sync/syncProtocol.test.ts Updates protocol tests to support mixed text and binary wire frames.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Host
  Client->>Host: hello(capabilities)
  alt binaryEnvelopes and application compression
    Host->>Client: ADE1 header + compressed binary body
    opt frame exceeds negotiated budget
      Host->>Client: ADE1 binary envelope_chunk frames
      Client->>Client: Reassemble binary envelope
    end
  else permessage-deflate negotiated
    Host->>Client: JSON text frame compressed by WebSocket transport
  else legacy peer
    Host->>Client: JSON text with base64-compressed payload
  end
Loading

Reviews (4): Last reviewed commit: "Send compressed sync envelopes as binary..." | Re-trigger Greptile

Context used:

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview Aug 10, 2026 5:05am

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The sync protocol adds ADE1 binary envelopes, binary chunking, per-message deflate configuration, and wire-aware sizing. The sync host and iOS service negotiate the capability and support binary frame parsing, decompression, chunk assembly, and legacy text fallback.

Changes

Binary sync transport

Layer / File(s) Summary
Binary frame format
apps/ade-cli/src/services/sync/syncBinaryFrame.ts
Adds the ADE1 frame format with bounded JSON headers, raw payload bytes, validation, decoding, and byte-length calculation.
Protocol encoding and reassembly
apps/ade-cli/src/services/sync/syncProtocol.ts, apps/ade-cli/src/services/sync/syncProtocol.test.ts, apps/ade-cli/src/services/sync/syncBinaryFrame.test.ts, apps/desktop/src/shared/types/sync.ts
Adds binary envelope encoding, capability negotiation, binary chunk validation and reassembly, bounded decompression, binary parsing, and tests for round-trips, chunking, malformed input, and compression behavior.
Host WebSocket integration
apps/ade-cli/src/services/sync/sharedSyncListener.ts, apps/ade-cli/src/services/sync/syncHostService.ts
Enables per-message deflate, accepts and emits binary frames, supports binary and legacy chunks, skips redundant application compression, and measures wire-frame sizes.
iOS binary envelope handling
apps/ios/ADE/Services/SyncService.swift
Adds ADE1 decoding, bounded decompression, binary chunk assembly, capability advertisement, and binary-aware WebSocket message handling.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • arul28/ADE#1056: Overlaps in syncHostService.ts and SyncService.swift around sync payload handling.
  • arul28/ADE#726: Shares sync-host and iOS SyncService changes.
  • arul28/ADE#486: Shares sync frame sizing and chunking changes.

Suggested labels: desktop, ios

Suggested reviewers: nsxdavid

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main changes: binary envelope frames and permessage-deflate support for sync transport.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/t3-sync-compression-c8368d49

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/ios/ADE/Services/SyncService.swift (1)

17095-17105: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Reject a nested envelope_chunk after reassembly.

syncPreprocessIncomingData(reassembled) can return another envelope_chunk. handleIncoming(nested) then re-enters this same case, reassembles again, and recurses. No depth limit exists.

A host can drive this cheaply. One chunk frame with total: 1 whose body is another chunk frame advances one level per frame. Each level holds its reassembled Data alive across an await, so memory grows with depth and never unwinds until the innermost frame resolves.

The TypeScript host already treats this shape as a protocol error. apps/ade-cli/src/services/sync/syncHostService.ts line 3234 throws "Nested envelope_chunk frames are not allowed.". Apply the same rule here so both sides reject the same wire shape.

As per path instructions: "iOS Swift app — check for memory management, Swift conventions, and proper SwiftUI patterns".

🛡️ Proposed guard against nested chunk frames
       if let reassembled {
         // The reassembled envelope can be tens of megabytes — decode it off
         // the main actor like any first-class frame.
         let nested = try await Task.detached(priority: .userInitiated) {
           try syncPreprocessIncomingData(reassembled)
         }.value
         guard isCurrentConnectionGeneration(generation) else { return }
         if let nested {
+          guard nested.type != "envelope_chunk" else {
+            throw NSError(
+              domain: "ADE",
+              code: 10,
+              userInfo: [NSLocalizedDescriptionKey: "Nested envelope_chunk frames are not allowed."]
+            )
+          }
           try await handleIncoming(nested)
         }
       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ios/ADE/Services/SyncService.swift` around lines 17095 - 17105, Reject
nested envelope_chunk frames in the reassembly branch of handleIncoming: after
syncPreprocessIncomingData(reassembled), detect when the resulting nested frame
is itself an envelope_chunk and throw the protocol error "Nested envelope_chunk
frames are not allowed." before recursively calling handleIncoming(nested).
Preserve normal handling for non-chunk nested frames and keep the existing
connection-generation check.

Source: Path instructions

🧹 Nitpick comments (2)
apps/ios/ADE/Services/SyncService.swift (1)

16993-16996: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Preserve diagnostic context when a binary frame fails to decode.

For a binary frame, text is set to "" at line 16967. If the detached decode throws, handleIncomingFailure(error, text: text, task: task) receives an empty string. The failure report therefore carries no frame context at all.

Binary frames are the hardest frames to diagnose from a device log, because the payload is compressed and no readable text exists. Pass a short description instead, for example the frame byte count and the decoded header type.

♻️ Proposed diagnostic context for binary frames
           } catch {
             if self.socket === task {
-              self.handleIncomingFailure(error, text: text, task: task)
+              let failureContext = binaryFrame.map { "<ADE1 binary frame, \($0.count) bytes>" } ?? text
+              self.handleIncomingFailure(error, text: failureContext, task: task)
             }
             break
           }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ios/ADE/Services/SyncService.swift` around lines 16993 - 16996, Update
the binary-frame decode failure path in the surrounding sync handling to
construct a concise diagnostic description containing the frame byte count and
decoded header type, then pass it instead of the empty text value to
handleIncomingFailure(error:text:task:). Preserve the existing behavior for
non-binary frames and the self.socket === task guard.
apps/ade-cli/src/services/sync/syncProtocol.ts (1)

66-71: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Bound per-connection zlib memory for permessage-deflate.

SYNC_PER_MESSAGE_DEFLATE_OPTIONS retains the default 320 KiB per-connection zlib context pair and only limits concurrently running jobs via concurrencyLimit. Keep context takeover and add bounded zlibDeflateOptions / zlibInflateOptions so context retention per peer does not grow independently of the pool size.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ade-cli/src/services/sync/syncProtocol.ts` around lines 66 - 71, Update
SYNC_PER_MESSAGE_DEFLATE_OPTIONS to retain context takeover while adding bounded
zlibDeflateOptions and zlibInflateOptions settings, limiting per-connection zlib
context memory independently of concurrencyLimit. Use the supported zlib memory
configuration fields and preserve the existing threshold and job concurrency
behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/ade-cli/src/services/sync/syncProtocol.ts`:
- Around line 590-593: Set binaryType to "buffer" on every WebSocketServer used
to send sync binary envelopes, including the instances created by
syncHostService.ts, sharedSyncListener.ts, and related tests. Ensure all sync
server construction paths preserve binary frames as Buffer values so
parseSyncEnvelopeFrame can use isSyncBinaryFrame without converting them to
text.
- Around line 618-630: Update parseSyncBinaryEnvelope’s compression === "none"
branch to validate that type is "envelope_chunk" before constructing the raw
envelope; reject other uncompressed binary frame types with the protocol error
“Uncompressed binary sync envelopes must be envelope chunks.”

In `@apps/ios/ADE/Services/SyncService.swift`:
- Line 1838: Replace both Data-to-String conversions in the sync preprocessing
path and receive loop with the failable String(bytes:encoding:) initializer,
including the conversion near syncPreprocessIncoming and the corresponding
receive-loop conversion. Propagate the resulting optional failure through the
existing error-handling flow so invalid UTF-8 produces an explicit decode
failure rather than continuing to JSON parsing.

---

Outside diff comments:
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 17095-17105: Reject nested envelope_chunk frames in the reassembly
branch of handleIncoming: after syncPreprocessIncomingData(reassembled), detect
when the resulting nested frame is itself an envelope_chunk and throw the
protocol error "Nested envelope_chunk frames are not allowed." before
recursively calling handleIncoming(nested). Preserve normal handling for
non-chunk nested frames and keep the existing connection-generation check.

---

Nitpick comments:
In `@apps/ade-cli/src/services/sync/syncProtocol.ts`:
- Around line 66-71: Update SYNC_PER_MESSAGE_DEFLATE_OPTIONS to retain context
takeover while adding bounded zlibDeflateOptions and zlibInflateOptions
settings, limiting per-connection zlib context memory independently of
concurrencyLimit. Use the supported zlib memory configuration fields and
preserve the existing threshold and job concurrency behavior.

In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 16993-16996: Update the binary-frame decode failure path in the
surrounding sync handling to construct a concise diagnostic description
containing the frame byte count and decoded header type, then pass it instead of
the empty text value to handleIncomingFailure(error:text:task:). Preserve the
existing behavior for non-binary frames and the self.socket === task guard.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3618c693-7c49-4c15-8782-073275da2c7b

📥 Commits

Reviewing files that changed from the base of the PR and between 64f95e6 and 4988d1e.

📒 Files selected for processing (8)
  • apps/ade-cli/src/services/sync/sharedSyncListener.ts
  • apps/ade-cli/src/services/sync/syncBinaryFrame.test.ts
  • apps/ade-cli/src/services/sync/syncBinaryFrame.ts
  • apps/ade-cli/src/services/sync/syncHostService.ts
  • apps/ade-cli/src/services/sync/syncProtocol.test.ts
  • apps/ade-cli/src/services/sync/syncProtocol.ts
  • apps/desktop/src/shared/types/sync.ts
  • apps/ios/ADE/Services/SyncService.swift

Comment on lines +590 to +593
export function parseSyncEnvelopeFrame(raw: unknown): ParsedSyncEnvelope {
if (isSyncBinaryFrame(raw)) return parseSyncBinaryEnvelope(raw);
return parseSyncEnvelope(wsDataToText(raw));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check binaryType configuration and raw-message shapes on every sync WebSocket path.
set -euo pipefail

# Test: locate binaryType settings and WebSocketServer construction options.
rg -n -C4 'binaryType' --type=ts
rg -n -C6 'new WebSocketServer\(' --type=ts

# Test: inspect wsDataToText to see which raw shapes it already normalizes.
ast-grep run --pattern 'function wsDataToText($_) { $$$ }' --lang typescript
rg -n -C10 'wsDataToText' --type=ts

Repository: arul28/ADE

Length of output: 148


🏁 Script executed:

#!/bin/bash
set -u

printf '--- files ---\n'
git ls-files | rg 'apps/ade-cli/src/services/sync|syncProtocol|syncBinary|adapter|webSocket|WebSocket' || true

printf '\n--- target location context ---\n'
if [ -f apps/ade-cli/src/services/sync/syncProtocol.ts ]; then
  wc -l apps/ade-cli/src/services/sync/syncProtocol.ts
  sed -n '560,610p' apps/ade-cli/src/services/sync/syncProtocol.ts
fi

printf '\n--- search binaryType / WebSocketServer / wsDataToText ---\n'
rg -n -C4 'binaryType|WebSocketServer|wsDataToText|parseSyncEnvelopeFrame|isSyncBinaryFrame|parseSyncBinaryEnvelope' . || true

Repository: arul28/ADE

Length of output: 50367


🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- syncProtocol wsDataToText ---'
rg -n -C12 'function wsDataToText|const wsDataToText' apps/ade-cli/src/services/sync/syncProtocol.ts

printf '%s\n' '--- parseSyncEnvelopeFrame and parseSyncBinaryEnvelope ---'
sed -n '540,645p' apps/ade-cli/src/services/sync/syncProtocol.ts
sed -n '645,730p' apps/ade-cli/src/services/sync/syncProtocol.ts

printf '%s\n' '--- syncHostService message handler ---'
sed -n '3185,3240p' apps/ade-cli/src/services/sync/syncHostService.ts

printf '%s\n' '--- sharedSyncListener WebSocketServer construction ---'
sed -n '555,575p' apps/ade-cli/src/services/sync/sharedSyncListener.ts
rg -n -C4 'new WebSocketServer\([' apps/ade-cli/src/services/sync apps/desktop/src/main/services/sync --iglob '*.ts' || true

printf '%s\n' '--- binaryType occurrences in TypeScript only ---'
rg -n /binaryType/ --glob '*.ts' . || true

printf '%s\n' '--- deterministic Buffer / non-Buffer detection behavior ---'
node - <<'JS'
const data = [
  Buffer.from("ADE1xxxx"),
  new ArrayBuffer(10),
  new Uint8Array([0x41, 0x44, 0x45, 0x31, 0x78, 0x78, 0x78, 0x78]),
  [Buffer.from("ADE1xxxx")],
];
for (const raw of data) {
  const isBuffer = Buffer.isBuffer(raw);
  const text = Buffer.isBuffer(raw)
    ? raw.toString("utf8")
    : ((raw instanceof ArrayBuffer || raw instanceof Uint8Array) ? Buffer.from(raw).toString("utf8") : String(raw));
  console.log(JSON.stringify({
    type: Array.isArray(raw) ? "Buffer[]" : Array.from(raw).slice(0,4).filter(x => (x >= 32 && x <=126) || x === 9 || x === 10 || (x >= 13 && x <=13 || x >= 128)).join(""),
    isBuffer,
    text,
    jsonParse: (() => { try { JSON.parse(text); return "ok"; } catch (e) { return e.message; } })()
  }));
}
JS

Repository: arul28/ADE

Length of output: 12356


🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- syncHostService WebSocketServer construction ---'
sed -n '2680,2715p' apps/ade-cli/src/services/sync/syncHostService.ts

printf '%s\n' '--- WebSocketServer construction options in sync TypeScript ---'
rg -n -C5 'new WebSocketServer\(' apps/ade-cli/src/services/sync apps/desktop/src/main/services/sync --glob '*.ts' || true

printf '%s\n' '--- relay binary/frame handling references ---'
rg -n `binaryType|ArrayBuffer|Uint8Array|Buffer\.concat|Buffer\.from|send\(data|send\([^,]+,\s*\{\s*binary\s*:` apps/ade-cli/src/services/sync apps/desktop/src/main/services/sync apps/tunnel-relay --glob '*.ts' --glob '*.mjs' || true

printf '%s\n' '--- behavioral probe for wsDataToText non-Buffer binary raw shapes ---'
node - <<'JS'
const BufferLike = class { constructor(buf) { this.buf = buf; } toString() { return this.buf.toString('utf8'); } };
const nonBufferBinary = new Uint8Array(Buffer.from("EDE1bad envelope", "utf8"));
const result = (() => {
  if (typeof nonBufferBinary === "string") return "string";
  if (Buffer.isBuffer(nonBufferBinary)) return nonBufferBinary.toString("utf8");
  if (Array.isArray(nonBufferBinary)) return Buffer.concat(nonBufferBinary).toString("utf8");
  return String(nonBufferBinary);
})();
console.log(JSON.stringify({
  rawType: nonBufferBinary.constructor.name,
  outputType: typeof result,
  output: result.slice(0, 40),
  jsonValid: true,
}));
JS

Repository: arul28/ADE

Length of output: 47410


Set binaryType: "buffer" on every sync WebSocketServer that sends binary envelopes.

syncHostService.ts and sharedSyncListener.ts create WebSocketServer instances without binaryType, and tests also omit it. Since isSyncBinaryFrame only accepts Buffer, any ArrayBuffer-typed binary frame becomes UTF-8 text via wsDataToText, fails parseSyncEnvelope, and closes the sync connection with Invalid sync envelope JSON.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ade-cli/src/services/sync/syncProtocol.ts` around lines 590 - 593, Set
binaryType to "buffer" on every WebSocketServer used to send sync binary
envelopes, including the instances created by syncHostService.ts,
sharedSyncListener.ts, and related tests. Ensure all sync server construction
paths preserve binary frames as Buffer values so parseSyncEnvelopeFrame can use
isSyncBinaryFrame without converting them to text.

Comment thread apps/ade-cli/src/services/sync/syncProtocol.ts
maxUncompressedBytes: Int = maxUncompressedSyncEnvelopeBytes
) throws -> SyncPreprocessedEnvelope? {
guard SyncBinaryFrame.isBinaryFrame(data) else {
return try syncPreprocessIncoming(String(decoding: data, as: UTF8.self), maxUncompressedBytes: maxUncompressedBytes)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the failable String(bytes:encoding:) initializer for the two Data to String conversions.

SwiftLint reports optional_data_string_conversion on both lines. String(decoding:as:) never fails. It substitutes U+FFFD for invalid bytes, so a corrupt or mis-routed binary frame becomes a mangled string and then surfaces as a confusing JSON parse error instead of an explicit decode failure.

Line 1838 handles a binary frame without the ADE1 magic. Line 16969 handles the same case in the receive loop. Both are exactly the paths where a non-text frame can arrive.

🐛 Proposed fix for both conversions

Line 1838:

   guard SyncBinaryFrame.isBinaryFrame(data) else {
-    return try syncPreprocessIncoming(String(decoding: data, as: UTF8.self), maxUncompressedBytes: maxUncompressedBytes)
+    guard let text = String(bytes: data, encoding: .utf8) else {
+      throw NSError(
+        domain: "ADE",
+        code: 10,
+        userInfo: [NSLocalizedDescriptionKey: "Sync frame is neither a binary envelope nor valid UTF-8 text."]
+      )
+    }
+    return try syncPreprocessIncoming(text, maxUncompressedBytes: maxUncompressedBytes)
   }

Line 16969:

             } else {
-              text = String(decoding: data, as: UTF8.self)
+              text = String(bytes: data, encoding: .utf8) ?? ""
             }

Also applies to: 16969-16969

🧰 Tools
🪛 SwiftLint (0.65.0)

[Warning] 1838-1838: Prefer failable String(bytes:encoding:) initializer when converting Data to String

(optional_data_string_conversion)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ios/ADE/Services/SyncService.swift` at line 1838, Replace both
Data-to-String conversions in the sync preprocessing path and receive loop with
the failable String(bytes:encoding:) initializer, including the conversion near
syncPreprocessIncoming and the corresponding receive-loop conversion. Propagate
the resulting optional failure through the existing error-handling flow so
invalid UTF-8 produces an explicit decode failure rather than continuing to JSON
parsing.

Source: Linters/SAST tools

@arul28
arul28 force-pushed the ade/t3-sync-compression-c8368d49 branch from 6b29e0c to 4988d1e Compare August 10, 2026 01:44
@arul28
arul28 force-pushed the ade/t3-sync-compression-c8368d49 branch from 4988d1e to c47aadb Compare August 10, 2026 04:50
A compressed envelope was base64'd into a JSON text frame — a flat +33%
re-inflation of exactly the bytes compression had just removed. Compressed
envelopes now travel in a binary container instead, gated on a new
`binaryEnvelopes` hello capability so a peer that does not declare it keeps
the base64 wire byte for byte.

The container is a magic prefix, a uint32 header length, the envelope minus
its payload as JSON, then the compressed bytes raw. The magic is load-bearing
rather than decoration: `wsDataToText` has always decoded Buffer frames as
utf8, and transports do deliver text that way, so sniffing the first four
bytes is what keeps a text frame arriving as data on the text path.

Oversized envelopes chunk as binary too. This matters more than it looks: a
text chunk base64s an envelope whose payload is already base64, and budgets
its slice down to 3/4 to pay for the expansion, so the tax compounds exactly
on the largest frames. One real db_version group in this machine's database is
11.4 MiB and cannot be split — the pump must ack rows sharing a db_version
together — so that path is not hypothetical.

Measured through `encodeSyncEnvelopeFrames` on 26.2 MiB of this machine's real
CRR rows, at the 720 KiB frame budget iOS negotiates:

  catch-up (250-row batches)    6.51 MiB -> 4.03 MiB   38.1% smaller
  live broadcast (4-row)        8.47 MiB -> 5.60 MiB   33.9% smaller

Per envelope in isolation the saving is the flat 25% base64 tax; the rest is
the chunk path no longer paying it twice.

Also enables permessage-deflate on both sync WebSocket servers, and skips the
application codec when a peer negotiated it. The transport's dictionary
persists across frames, so it compresses changeset traffic better than
per-envelope compression can, and stacking the two is worse than either alone
— measured as bytes written to a real socket, live-broadcast sized: 7.34 MiB
app-level only, 5.31 MiB both, 4.14 MiB transport only. iOS cannot negotiate
the extension (`URLSessionWebSocketTask` has no support), which is precisely
why it gets the binary container instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@arul28
arul28 force-pushed the ade/t3-sync-compression-c8368d49 branch from c47aadb to c1867ce Compare August 10, 2026 05:05
@arul28
arul28 merged commit 870d738 into main Aug 10, 2026
37 checks passed
@arul28
arul28 deleted the ade/t3-sync-compression-c8368d49 branch August 10, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant