FM-PS-LYNX-008: architecture stack + verify fixes

[djwok]

Collapses the remaining FM-PS-LYNX-008 stack into one PR targeting main.

H (verification gate) already landed in #23. Tip branch drkz/fm-ps-lynx-008-native-build-fix-4330 was rebased onto current main: the duplicate verification-gate commit was skipped as already applied; no conflict resolutions. GitHub refused to retarget stacked #30 (Cannot change the base branch because the pull request is part of a stack), so #30 was closed and replaced by this PR from the same head.

Intermediate stacked PRs #24–#29 (and closed #30) are superseded by this tip.

Stack (E → A → B+C → F → D+G + verify fixes)

E — one LynxHost for PrimJS globals

src/host.ts: PrimJSLynxHost + fake test host. NativeModules, GlobalEventEmitter, platform, text codec, fetch, and AbortController install go through one seam. Not public API.

A — SyncStreamTransport

Three adapters: NativeHttpFetch (primary; streamingId / GlobalEventEmitter — issue #21 realtime), LynxFetchModule, host-fetch. LynxRemote.ts is a thin pick. FM-PS-LYNX-003 diagnostics removed.

B+C — NativeSyncHttp split + idle-complete fallback

Native /sync/stream HTTP lives in NativeSyncHttp (Android + iOS), not on the SQL RPC module. Idle-complete is the fallback only when streamingId is absent.

F — shared NDJSON fixtures

Canonical catalog at shared/fixtures/sync-stream.json, replayed by JS tests and native fixture tests.

D+G — Android ps_sql JNI + iOS host event sender

Android SQL RPC is JNI PsSqlEngine → shared/ps_sql (androidx.sqlite-bundled removed). iOS no longer walks UIWindow; incremental stream needs setSharedStreamEventSender: (showcase already registers the LynxView).

Web download

Host-fetch keeps a usable Fetch Response.body. PrimJSLynxHost finds bare lynx on lynx-bg, so an emitter is always present on web — do not wait on native onData for identifier fetch. streamingId still uses GlobalEventEmitter. src/globals.ts no longer redeclares Lynx globals (rspeedy TS2403).

Native build fix

iOS: local PSLynxStreamEventSender protocol so sendGlobalEvent:withParams: compiles without importing LynxView. Android NDK: AttachEnv uses JNIEnv** under __ANDROID__, OpenJDK void** for Linux make test.

Confirmed not dropped

• streamingId realtime path (native-http Hardening: keep native realtime on streamingId path (regression + README) #21)
• SyncStreamTransport pick order
• LynxHost / PrimJSLynxHost
• NativeSyncHttp split from SQL RPC
• ps_sql JNI engine
• host-fetch body-keep for Lynx-for-Web

Verify

• pnpm typecheck, pnpm lint, pnpm test
• make test (Linux; includes JNI -c of ps_sql_jni.cc)
• make test-ios / Android NDK + instrumentation need Mac

Tracking: #18

  

[cursor]

FM-PS-LYNX-010 · code review of the landed stack (main @ 442ec6f vs pre-#32 f30f585)

Analysis only — no code changes. Verified locally on Node 22.18: pnpm typecheck ✅, pnpm lint ✅, pnpm test ✅ (92/92), make test ✅ (ps_sql, NDJSON fixtures, JNI -c). Two throwaway repro tests (kept out of the repo) back the High findings below.

Legend: [new] introduced/reshaped by #32 · [carried] pre-existing in #13, kept by the refactor and now part of the documented architecture.

---

Blocker

None. The stack builds and the realtime streamingId path works end-to-end in the JS tests.

---

High

H1. disconnect() never cancels a live native /sync/stream — httpFetchAbort is unreachable once the response resolves [carried, formalised by #32] — src/sync/transport/NativeHttpFetch.ts, src/sync/transport/events.ts

AbstractRemote.fetchStreamRaw (shared-internals 1.2.0) aborts the inner controller only while the request is pending; once res.body.getReader() exists it calls reader.cancel(reason) instead. NativeHttpFetch wires httpFetchAbort solely to request.signal's abort event, and createStreamingReader().cancel() only flips finished and drops the handler. Net effect on device:

• The native HTTP connection stays open after disconnect() / reconnect / token refresh (Android: until the 120 s read timeout fires on the next byte; iOS: until H2 kills it).
• Chunks that keep arriving hit slotListener with no handler → rememberEarly → buffered forever in the module-level earlyEvents map (see H3).
• This is the spec's own item 3 (“httpFetchAbort(streamingId) cancels the native request”) and the old “Cancellation” verification obligation; neither is exercised by a test — test/lynx-remote-stream.test.ts:386 only asserts aborted === undefined.

Repro (fake host, fetchStream + controller.abort() after first chunk): httpFetchAbort called with: undefined | native still delivering to 1 listener(s).

Fix shape: have the reader returned by streamingReader(streamingId) call httpFetchAbort(streamingId) from cancel() (or pass an onCancel hook from NativeHttpFetch), and add a fetchStream + abort test asserting the native abort fires.

H2. iOS stream is force-closed every 150 s (timeoutIntervalForResource) [carried] — ios/src/StreamingHttp.mm:137-138

config.timeoutIntervalForResource = StreamReadTimeoutSec() + ConnectTimeoutSec() (= 150 s). timeoutIntervalForResource is the total lifetime of the load, not an idle gap (default 7 days). A healthy PowerSync stream lives for hours, so NSURLSession will fail it with NSURLErrorTimedOut at 150 s → onError("The request timed out.") → onEnd → PowerSync logs an error, applies retry backoff and reconnects — every 2.5 minutes, forever. make test-ios's hold_open fixture only holds 5 s so it cannot see this. Leave timeoutIntervalForRequest at 120 s (that one is the idle timer) and drop/raise timeoutIntervalForResource.

H3. earlyEvents retains every chunk delivered via emit/trigger for the life of the process [carried] — src/sync/transport/events.ts:180-187, 46-56

hookEmitter wraps emitter.emit/trigger with rememberEarly(name, data) unconditionally. liveStreamNames is never populated (dead Set), so the guard is always false and every onData payload is pushed into earlyEvents even when a handler is attached and consumes it. drainEarly runs only once per attachStreamHandler, so nothing ever evicts them. Memory grows with total NDJSON downloaded whenever the Lynx runtime dispatches through emit/trigger (the _events.get hook path avoids it only when a handler exists). Repro: after a fully-consumed stream on name X, a second streamingReader(X) immediately receives "chunk-1\n" again. Fix: mark names live in attachStreamHandler (populate liveStreamNames), clear on cancel/onEnd, and cap the buffer.

---

Medium

M1. Android abort() does not close the socket [carried] — android/.../NativeSyncHttp.java:60-68, StreamingHttp.java:106-140
abort only sets the AtomicBoolean; the reader loop polls it between blocking in.read() calls (120 s timeout). No conn.disconnect() from the abort path, so cancellation waits for the next server byte (~20 s keepalive) and still emits that chunk before exiting. Keep the HttpURLConnection (or InputStream) reachable from abort and disconnect() it. Matters as soon as H1 is fixed. Android instrumentation test has no abort case (iOS's does).

M2. Library Gradle build shells out to node [new] — android/build.gradle:57-70
fetchSqliteAmalgamation (Exec → node scripts/fetch-native-deps.mjs --sqlite) is wired into preBuild/configureCMake* for every consumer. The tarball already ships third_party/sqlite/sqlite3.{c,h} (pack test asserts it), yet on the first build Gradle has no task history and runs the Exec anyway — failing if node is not on the Gradle daemon's PATH (Android Studio launched from the dock is the classic case) or offline. Suggest onlyIf { !sqlite3.c.exists() } (or drop the task from the library and keep it in the host app only). Also new consumer requirements (NDK, CMake ≥ 3.22, c++_shared) are only mentioned in examples/hosts/android/README.md, not the root README / spec registration section.

M3. GlobalEventEmitter monkey-patch breaks consumer removeListener [carried] — events.ts:155-190
hookEmitter replaces addListener so foreign listeners are stored in foreignListeners behind a single slot fn, but removeListener/removeAllListeners are not wrapped — app/ReactLynx code can no longer unsubscribe from the shared GlobalEventEmitter after the first /sync/stream. enterEarlyCapture() runs on every streaming fetch, so this affects every native consumer. The three transport files (bytes.ts, events.ts, response.ts) were also added to the oxlint ignore list; events.ts is the file that most needs the lint.

M4. onError drops already-queued chunks [carried] — events.ts:264-274
read() checks failure before queue, so onData* lines received just before onError are discarded instead of being surfaced before the throw. Sequence contract is onData* → onError? → onEnd; the reader should drain the queue first, then throw.

M5. Pre-aborted signal still issues the native request [carried] — NativeHttpFetch.ts:73-80
If signal.aborted is already true, abortNative() is a no-op (no streamIdForAbort yet) and httpFetch is still called; the request is only cancelled after headers return. Standard fetch rejects with AbortError up front — reject before calling native.

---

Low

• L1. StreamingHttp.java:110-115 — SocketTimeoutException is reported as a clean onEnd (comment is self-contradictory) and the UTF-8 carry is not flushed. Report as onError or at least flush.
• L2. ps_sql_jni.cc — no ExceptionCheck() after CallStaticObjectMethod/CallVoidMethod; a pending Java exception in toWritable/invoke makes the following JNI calls UB. Deliver also attaches/detaches a JNI env per callback (costly on the engine thread; consider a thread-local attach).
• L3. abort-controller.ts:71-87 — polyfill overwrites both globals when either is missing (can clobber a real AbortController); AbortSignal.timeout reason text is “Timeout waiting for lock”; installAbortControllerPolyfill() runs at module scope in both abort-controller.ts and host.ts.
• L4. bytes.ts:297-321 headerMap — a Headers instance (not plain object / array) yields {}; syncStreamRequestFromFetch String(options.resource) stringifies a Request as [object Request]. Both fine for today's AbstractRemote, brittle for upgrades.
• L5. NativeSyncHttp.java:159-162 / IdleCompleteHttp.mm:73-89 — idle-complete envelope carries both body and bodyBase64 (~2.3× payload across the bridge). JS prefers body and only falls back to bodyBase64 when it is empty; send one.
• L6. NativeSyncHttp.mm:49 — the __weak shared sender is process-global; multiple LynxViews (or a recreated view after navigation) route events to whichever view registered last / none. Document “re-register on view recreation” or key the sender by module instance.
• L7. sync_http_policy.h vs Android: IdleCompleteHttp.java / StreamingHttp.java constants and NativeSyncHttp.STREAM_EVENT_PREFIX are documented as “keep in lockstep” with the header but are hand-copied; no test asserts equality (iOS includes the header directly).
• L8. Linux make test only compiles ps_sql_jni.cc (-c); nothing links/loads it, so JNI signature typos (toWritable descriptor, Callback.invoke) are only caught on device.

---

Nits

• ADR filename 0003-native-module-http-is-streaming-fallback.md contradicts its own title/body (native HTTP is now the primary path; idle-complete is the fallback).
• .scratch/fm-ps-lynx-008/issue-18-*-comment.md are paste-ready GitHub comment artifacts committed to main.
• liveStreamNames (events.ts:12) is dead code.
• ios/src/StreamingHttp.mm:132-134 indentation glitch; default Content-Type for bodies was removed (harmless — AbstractRemote.buildRequest sets it).
• android/src/main/cpp/CMakeLists.txt adds SQLITE_DEFAULT_MEMSTATUS=0 / SQLITE_OMIT_DEPRECATED=1; shared/CMakeLists.txt (desktop) does not — three define lists now (Makefile, podspec, two CMakes).
• NativeSyncHttp.mm onHeaders always sends statusText: "".
• createStreamingReader keeps a single wake slot; concurrent read() calls would strand the first waiter (fine for AbstractRemote's sequential reads).

---

Test gaps (summary)

1. No JS test that abort after the stream is established calls httpFetchAbort (H1) — the existing streaming test asserts the opposite branch only.
2. No test that already-consumed chunks are released (H3) or that cancel() stops buffering.
3. No Android abort test (instrumentation covers fixtures only); no long-lived (>150 s) iOS fixture (H2).
4. No pre-aborted-signal test (M5); no removeListener round-trip on a hooked emitter (M3).

API / ADR drift

• Spec “Sync transport” item 3 and ADR-0003 promise native cancellation via httpFetchAbort; the JS side cannot deliver it after headers (H1).
• Spec/README say idle-complete is “only the fallback when no event sender is registered” — true; but sync_http_policy.h's stated intent for PS_SYNC_HTTP_STREAM_READ_TIMEOUT_MS (“so keepalive gaps do not abort”) is undermined on iOS by H2.
• Spec registration section still reads as “Autolink lynx.lib.json … Lookup name NativePowerSyncModule” with no mention of the new NDK/CMake/node build-time requirements (M2).

---

Review by Claude Fable 5.1 (Cursor cloud agent), FM-PS-LYNX-010 item 2.