Repository navigation
feat(ios): move the RNS engine + RNode/mesh BLE radios into the Network Extension - #213
Open
torlando-tech wants to merge 93 commits into
Open
torlando-tech wants to merge 93 commits into
torlando-tech wants to merge 93 commits into
Conversation
Engine-agnostic core for the node-service v1 contract (.scratch/ne-node-contract). New local SwiftPM package packages/ColumbaNode: - Typed IDL subset (command vocabulary, Intent, NodeError, distinct ID wrappers) - RFC 8785 canonical JSON encoder + pure-Swift SHA-256; locked to the contract's examples.json reference digest byte-for-byte - NodeStore: one SQLite store (WAL/FULL, typed SQLValue params, explicit NULL) with durable stage-first admission, the command ledger (idempotent re-admit, body-conflict detection, ledger-first over changed policy), LocalSequence reservation, and the node change index 11 unit tests green on Linux. Engine-agnostic: no RNS dependency, so Python RNS / Reticulum-Go / microReticulum all drop in behind the engine-adapter seam.
The clean seam for the node-service v1 contract (contract 6, 15, ADR 0001): - Control: bounded app<->NE control channel - [0xF5 0x02] framing, hard 64 KiB envelope cap, hello/admit/query/act request+reply codec. admit carries only the commandID; the complete body is never inline (staged in the shared store, resolved against the ledger). Unknown 0xF5 version is a hard error that never falls through to a legacy path. - Engine: NodeEngine adapter protocol - the ONLY surface the node owner uses to reach a Reticulum impl, so Python RNS / Reticulum-Go / microReticulum all drop in behind it. StubEngine fails closed before a real engine is installed. - NodeStore.intent(for:) so a node owner resolves an admit's commandID. Tests now 20 on Linux: framing, 64 KiB cap, unknown-version, hello/admit/query round-trips, and the stage-in-store -> admit-over-channel -> resolve-from-ledger flow (proving the body never crosses the wire inline).
Adds the packages/ColumbaNode local SwiftPM package to the Columba.xcodeproj local package refs and links the ColumbaNode product into both the ColumbaApp (typed facade) and ColumbaNetworkExtension (node owner) targets. The NE still uses the existing C++ node; this only makes the shared seam available for the Python-RNS engine + control-channel work.
…solution) Xcode's SwiftPM cannot resolve a local package product that lives in a package nested inside another referenced local package (the repo-root "." package). Move ColumbaNode + SQLite3Shim + ColumbaNodeTests up to the repo root as targets of the existing ColumbaApp package (the same "." product RNSAPI and SwiftBLEBridge use) and repoint the ColumbaNode XCSwiftPackageProductDependency to it, removing the dead nested local-package reference. - Package.swift: +ColumbaNode/SQLite3Shim targets + ColumbaNodeTests - project.pbxproj: ColumbaNode product dep now has no explicit package field (resolves from the root package, like RNSAPI); nested local ref removed - Sources/ColumbaNode, Sources/SQLite3Shim, Tests/ColumbaNodeTests moved up - packages/ColumbaNode removed
The engine-agnostic Python core that a Reticulum implementation runs against when it is the node-owner side of the contract. Two halves of one contract: Swift (Sources/ColumbaNode) and Python (engine/columba_node) are the SAME algorithm, both locked to the same contract reference vector so a logical intent produces the identical byte form + digest in both languages. - canonical.py: RFC 8785 canonical JSON + SHA-256 (pure Python, no RNS) - control.py: [0xF5 0x02] framing (64 KiB hard cap, unknown-version hard error) + Request/Reply wire codec (hello/admit/query/act) mirroring the Swift ControlTypes field-for-field - tests: prove Python reproduces the contract's canonicalIntentUTF8 + bodySHA256 byte-for-byte (the cross-language parity keystone), the framing cap/version rules, and that hello/admit replies canonicalize to the exact bytes the Swift encoder produces This is the "language-agnostic IDL" requirement made concrete: the wire bytes do not change when the engine (Python RNS / Reticulum-Go / microReticulum) behind the seam changes.
The Python half of the engine-adapter seam (contract 15). Deliberately thin: maps the contract surface (descriptor/capabilities/runtime-snapshot/execute/ capability-gate) onto the EXISTING app/rns_bridge.py operations (start / send_opportunistic / stop), so minimal custom Python runs in the node owner and the real Reticulum logic stays in the bridge. - engine.py: PythonRNSNodeEngine. The RNS bridge is INJECTED (bridge arg) so the mapping is provable on any CPython; production passes app.rns_bridge. Advertises durableMessaging supported, all other features unsupported (fail-closed). execute maps the LXMF send reasons contract-significantly: queued->ok+operation change, requesting-path->interrupted (durable-resumable), not-started->unavailable, bad-hash->invalidArgument, no-propagation-node-> transportUnavailable (retryable), else dependencyFailed. - tests/test_engine.py: 16 mapping tests (fake bridge) - descriptor shape + capability set, runtime snapshots, start delegation, idempotent stop, the capability gate, and every execute result-reason mapping. - _rns_nodes.py: self-contained two-node RNS helpers (Mac/RNS only; RNS import guarded so the Linux controller skips the integration test, not fails it). Verified: 32 pure-python tests green (canonical 6 + control 10 + engine 16). The real-RNS two-node boot+send integration runs in the Mac venv (RNS 1.5.3).
Drives PythonRNSNodeEngine against the real app.rns_bridge with real RNS + LXMF: start boots the node and reports a ready descriptor with real 64-hex identity + destination hashes; execute(submitMessage) to a malformed destination maps the real send_opportunistic bad-hash result to invalidArgument. Import RNS is guarded so the Linux controller SKIPS these (not fails them).
Real RNS spawns daemon threads (delayed re-announce, native stamp generator) that do not settle under a bare unittest process exit, so the unittest form of the real-RNS check hung in teardown even though boot + stop both succeeded. - engine/columba_node/scripts/real_rns_check.py: standalone script that drives PythonRNSNodeEngine against the real app.rns_bridge (RNS 1.5.3 on the Mac), asserts start/gate/execute-mapping/stop, then os._exit(rc) so the daemon threads can't wedge the process. Prints SKIP and exits 0 where RNS is absent. - remove tests/test_rns_integration.py (the hanging unittest form); the standalone script is the honest "adapter drives real RNS" gate.
RNS Identity.hash is 16 bytes = 32 hex chars; 64-hex is the destination hash. The adapter correctly reported the real 32-hex identity; the check's assertion had the wrong length. Validate all-hex instead.
The node-owner coordinator (contract 6.5): a serialized actor that services the bounded control channel (hello/admit/query/act), runs stage-first admission through NodeStore, and drives the pluggable NodeEngine. Engine- agnostic (Python RNS / Reticulum-Go / microReticulum all sit behind it). Enforces the admission/execution split: admit commits the durable disposition in one transaction; execution runs outside it on the owner's serial actor; the engine's domain changes are committed by the owner via NodeStore.commitChanges (the single writer for the node group, contract 5). Fail-closed: out-of-epoch -> storeReplaced, stale boot -> bootChanged, unknown framing version -> protocolFailure (never a legacy fallthrough). Adds NodeStore.commitChanges + 9 NodeOwner tests (fake engine, hermetic).
commitChanges held the non-reentrant NSLock and then called the locking epochValue getter (and, in the empty-list path, highWater) - a self-deadlock that hung the first test to exercise a non-empty commit. Guard the empty case before locking and read the private epoch field directly (safe while holding the lock). Caught by a Mac sample: main thread in psynch_mutexwait inside commitChanges -> epochValue -> lock.lock().
Two defects the Mac test run exposed (the deadlock is fixed; these were next): - A repeated admit for an already-accepted command re-ran the engine side effect (executeCalls 2, expected 1). Re-running a durable message send would duplicate it. Gate execution on an executedThisBoot set (cleared per hello) so admit is idempotent per contract: the ledger is checked first and the committed record is returned without re-executing. - CommandRecord's decode dropped the committed rejection's typed error (rejection: nil) even though the encode emits it. decodeCommandRecord now decodes the rejection via decodeError, so a committed rejection carries its code/retry/field back over the control channel (contract 3.4).
The real engines are long-running + async: the C++ microRNS node is an async actor, and embedded Python RNS (CPython) will be too. A synchronous seam can't drive them from the node owner without bridging. Make start/stop/runtimeSnapshot/execute async; keep canExecute synchronous (a pure capability check the store's admission policy calls inside its transaction - it must not do I/O). NodeOwner methods that drive the engine (handle/dispatch/hello/admit/runExecution/handleQuery/act/ currentDescriptor) become async to match. This is the shape the NE wiring needs to adapt NEReticulumNode (an async actor) behind the seam.
The reused NE now services the contract control channel over the existing 0xF5 IPC (contract 6, 16): - NEMicroRNSEngine: the in-NE NodeEngine adapter wrapping the existing NEReticulumNode (C++ microRNS) behind the seam. First engine to drive a real Reticulum implementation through the node owner; Python RNS / Reticulum-Go drop in as sibling conformances. Maps submitMessage onto sendLxmfForIPC with the contract result-reason mapping (queued->ok, requestingPath->interrupted, badHash/notStarted/other->typed rejection). - PacketTunnelProvider.handleAppMessage: a [0xF5 0x02] control frame routes to the NodeOwner (built over the shared columba-node.db + NEMicroRNSEngine) BEFORE the legacy ProxyRequest branch - an unknown control version is a hard protocol error, never a legacy fallthrough (contract 6). The legacy 0xF5 0x01 path is preserved (contract 16: cut over later). - ControlChannel.isControlFrame: the cheap [0xF5 0x02] prefix check. - AppGroupPaths.nodeServiceStoreURL(): the shared columba-node.db path both the app facade and NE owner open (single source of truth, no drift). - stopTunnel releases the node owner (clean WAL close). pbxproj: NEMicroRNSEngine.swift added to the NE target.
A public type cannot have a public init taking an internal type - the NE build failed with 'initializer cannot be declared public because its parameter uses an internal type'. The adapter wraps the internal NEReticulumNode and is only constructed by the NE's PacketTunnelProvider, so internal is correct.
The app half of the seam: - NodeControlClient (ColumbaNode): the app-side client for the node-service v1 control channel. Stages a submitMessage intent durably in the shared store, then drives the NE node owner with hello+admit over the injected app->NE transport (mirrors ProxyRnsBackend's transport-injection shape). Returns the node's committed CommandRecord. - SQLiteConnection: PRAGMA busy_timeout=5000 for cross-process safety - the app and the NE open the SAME WAL database (app stages, NE admits); without it an overlapping BEGIN IMMEDIATE fails with SQLITE_BUSY instead of waiting. - test-node-send deep link (lxma://test-node-send?to=HEX&content=...): DEBUG-only trigger that drives the new 0xF5 0x02 path on a real device, independent of the legacy backend.lxmf path (contract 16 keeps the legacy path). This is the seam to test the node contract end-to-end on the iPhone: app stages -> control frame -> NE node owner -> real RNS engine.
The app now imports ColumbaNode (NodeControlClient + the test trigger), which transitively needs the SQLite3Shim C module. The NE target already declared the ColumbaNode product in packageProductDependencies; the app target only had the Frameworks build-phase entry, so Xcode could not resolve SQLite3Shim. Add the product to the app target's packageProductDependencies.
The OS delivers a launch-time --payload-url BEFORE startPythonBackend registers the ColumbaTestNodeSend observer, so a bare NotificationCenter post was lost (no listener) - the trigger logged but never drove the control channel. Park the request in AppServices.pendingNodeSend from the deep-link handler; drainNodeSendProbe() consumes it exactly once after the observer + tunnel are ready. Also wait for a connected tunnel (30s) before the hello+admit round-trip so the control frames reach the NE node owner.
First slice of moving the entire Reticulum runtime into the Network Extension: - link + embed Python.xcframework into the appex (Frameworks + Embed Frameworks (NE)) - NE-specific Python bridging header (ColumbaNEPython-Bridging-Header.h) exposing only the CPython C API - keeps the NE's RNSAPI/ReticulumSwift collision rule - 'Install Python stdlib (NE)' build phase: install_python lays the stdlib into the appex so PYTHONHOME = <appex>/python - NEPythonRuntime: CPython lifecycle for the NE process (isolated config, PyGILState), logs sys.version to ext-diag - non-blocking init probe in startTunnel so the first device run exercises it The real Python RNS engine (wheels + bridge) behind the NodeEngine seam is the next slice; this proves the interpreter boots in the NE sandbox.
The sequential edits had landed the NE Embed-Frameworks (EFEMB) and Install-stdlib (EPIPS) build-phase blocks in the wrong ODBF sections, which broke the project parse (Unable to read project). Relocated both into their correct sections (PBXCopyFilesBuildPhase / PBXShellScriptBuildPhase) and added the Python.xcframework reference to the NE Frameworks phase. Verified: xcodebuild -list parses; all build-file ids present with expected counts.
…+ live node Slice 2 of the in-NE Python RNS runtime. Builds on the CPython-in-NE foundation (9be0f91) and de-risks the whole port by proving the real Reticulum Python runtime loads and runs inside the NE process on-device: - NE stdlib build phase now also copies the RNS/LXMF wheels to <appex>/app_packages + rns_bridge.py to <appex>/app, and runs install_python with the wheels as a 2nd arg so their .so extensions (CRNS, cffi, cryptography) get re-extensioned + signed (mirrors the app). - NEPythonRuntime gains addSiteDir/prependSysPath (appex-relative) + probeRNS (platform.system->Darwin patch, import RNS/LXMF/rns_bridge, construct RNS.Node, keep it in __main__._ne_node) + probeNodeRunning (lets the RNS daemon threads run, reports isRunning()). - startTunnel runs the probe non-blockingly after init; logs [NE-PY-RNS] to ext-diag.log. On-device probe result is the gate for the next slice (the NodeEngine conformance that drives this node).
…m node The NE now runs ONLY Python RNS. The abandoned ReticulumSwift/LXMFSwift C++ microReticulum node (NEReticulumNode) is no longer constructed or started in startTunnel - running both runtimes in the NE's memory budget crash-looped the process (startTunnel re-entering every ~7s). The Python RNS probe is now the sole runtime bring-up. The reticulumNode property is now dead (never set); the control channel degrades to fail-closed (node owner not ready / StubEngine) until the Python RNS engine conformance lands. Removing the dead C++ source + framework links from the NE build is the next step.
The probe now runs as discrete GIL stages (prep / importRNS / importBridge / nodeConstruct / nodeRunning), each logging its result to ext-diag BEFORE the next. If the NE is terminated mid-probe, the last logged stage names the step that died (import RNS loads CRNS/cffi/cryptography; RNS.Node() starts the daemon). Also confirmed the previous crash-loop log was a STALE appex (it still logged a line removed in the prior build), so the loop attribution was unreliable - this staged probe + a clean reinstall will localize it properly.
The NE crash loop was NOT a memory limit (your decision point): the crash logs show SIGSEGV/EXC_BAD_ACCESS at 0x10 in my own probe - readMainAttr called PyImport_ImportModule after withGIL released the GIL. After init the embed thread has released the GIL (so RNS bg threads can run), so every C-API call on a Task thread must re-acquire it. runStage now does the run AND the result-read inside one withGIL block. This is on-path for the real engine (same GIL requirement), not a reason to move off Python RNS. Also added a DEBUG-only test-start-tunnel deep link (on the existing test-* seam) that brings the Model B tunnel up headlessly - it bypasses the BackgroundDeliveryGate (manual tap) so the in-NE Python RNS process can be booted + exercised via devicectl --payload-url. Needed because the earlier clean uninstall (no longer required - in-place install works; a running NE keeps its old appex until the tunnel restarts) wiped the app's persisted background-delivery approval.
The GIL fix (0fd3ced) worked: the NE no longer crash-loops - all five RNS stages (prep/importRNS/importBridge/nodeConstruct/nodeRunning) now run to completion with the 2s daemon delay honored. But every stage reported '(no output)' because the stage script was interpolated into the try: block without indenting its lines - only the first line got the leading newline, so the body landed at column 0 -> IndentationError -> the whole wrapper failed to compile -> _stage_out was never set -> readMainAttr returned nil. Now indentPython() indents every line, and runStage reports the PyRun_SimpleString return code so a wrapper failure is visible instead of a silent '(no output)'. This will surface the real RNS stage results (versions, node=running/stopped) on the next device run.
A running NE keeps its old appex until the tunnel is torn down, so to exercise an NE rebuild on-device the sequence is stop -> install -> start. test-start-tunnel (added in 0fd3ced) handles the start; this adds the companion test-stop-tunnel (tunnelManager.stop()). Both are DEBUG-only deep links on the existing test-* seam, Model B only.
The failure branches return String and the success branch returns
String? (readMainAttr), so Swift inferred T=String and rejected the
String? return. Explicit { () -> String? in } unifies them.
The probe's nodeConstruct used RNS.Node, which does not exist - the real API (used by the app's known-good rns_bridge.py) is RNS.Reticulum(config_dir). That AttributeError was my wrong class name, not an RNS problem. Confirmed on-device so far: RNS=1.4.2 LXMF=1.1.0 imports and loads in the NE (the CRNS/cffi/cryptography .fwork extensions load fine), and the GIL fix killed the crash loop. This makes the probe construct the actual RNS runtime (tempfile.mkdtemp config dir, which iOS sets under a writable per-process tmp for extensions) and report RNS.Transport.interfaces, to confirm the full runtime runs in the NE.
The app's ProxyRnsBackend talks to the NE over the legacy ProxyRequest IPC. Before this, dispatch() routed every op to the removed C++ NEReticulumNode (nil) -> .unsupported -> 30 retries -> 'backend not ready'. Now dispatch routes to NEPythonRNS, the in-NE Python RNS engine, which drives rns_bridge.py through the embedded CPython (NEPythonRuntime.callBridge). This is the seam the north-star calls for: the NE is the sole place Reticulum runs (Python RNS, the reference implementation), the app is UI-only over the language-agnostic IPC, and the in-app RNS is off. - NEPythonRNS: reads the app's shared-keychain identity (Foundation-only), resolves the App-Group-shared RNS config dir, and exposes one JSON call into rns_bridge per op (start/stop/announce/status/local_info/ drain_events/persist/send_opportunistic). - NEPythonRuntime.callBridge: generic rns_bridge.<fn> call (bytes carried as base64, result as JSON), one withGIL block for run+read. - App (Model B): also writes the RNS config + identity to the SHARED App-Group dir (AppGroupPaths.rnsConfigDirectoryURL) - the app's private Application Support was unreadable to the NE - and persists the identity hash hex so the NE keys the same dir. - PacketTunnelProvider: dispatchPython maps each ProxyRequest onto a rns_bridge call + the Proxy* wire types; the dead C++ dispatch + the reticulumNode property are removed; the control-channel nodeOwner uses the fail-closed StubEngine (the Python NodeEngine conformance is a follow-up). Verified on-device so far: CPython 3.13 + RNS 1.4.2 import and a RNS.Reticulum constructs + runs (ifaces=3) in the NE; the crash loop was a GIL bug in the probe, now fixed.
IOSRNodeInterface overrides the base Interface callback with a stale signature that lacks the size kwarg RNS 1.5.2 passes on every announce transmission (Transport.py:1600: interface.sent_announce(size=len(raw))). The resulting TypeError is caught by rns_bridge.announce() and surfaced as ok=false, which the app renders as error 3 / no success indicator. The announce bytes are actually sent (peer sees the traffic), so the symptom was 'announce works but green text never appears.' Match the RNS 1.5.2 base signature and account for the byte count (the base class does self.atxc += 1; self.atxb += size). Verified on-device: delivery + telephony announce now both report 'sent' after the fix.
The NE dispatcher answered .announce/.announceTelephony with a bare bool and discarded the Python rns_bridge return's reason field, so a failing op (ok=false) was indistinguishable from a successful one at the log layer - this hid the sent_announce() size kwarg bug for a while. Log the reason on the failure paths (and the bridge-level failure points in NEPythonRNS.call) so a future ok=false is explained in ext-diag without a code change. Gated behind #if DEBUG per the NE diagnostic convention, so Release builds carry no extra logging.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…T address Android randomizes its BLE address across GATT sessions, so a peer that was seen at address A re-surfaces its identity at a fresh address B on reconnect. IOSBLEDriver._raw_on_identity_received only added the new address, leaving the upstream BLEInterface.address_to_identity map accumulating one entry per reconnect (observed live: 5 entries for a single identity). RNS then kept routing announces to the stale dead addresses (SEND-NO-TARGET), the root cause of flaky BLE announce delivery. When the same identity is seen at a new address, emit on_address_changed (old, new, id) so the upstream _address_changed_callback migrates address_to_identity / address_to_interface / interface.peer_address and the fragmenter+reassembler keys to the new address, evicting the stale one. Verified on device: after a fresh NE install the map stays bounded at one entry per identity across live GATT address rotations. Wire the existing-but-unwired test_ios_ble_driver_callbacks module into CI.
…path switch P1 #2 (ne-python-architecture-review): _set_inbox_path / _set_grdb_store re-point the per-identity path but the openers early-returned when a connection already existed, so an in-NE identity switch (A -> B, same process) kept writing identity B's durable events + inbound into identity A's store. Track the path each connection is bound to and close + reopen it when the path is re-pointed.
…host mode P1 #1 (ne-python-architecture-review): start() unconditionally called _set_inbox_path/_set_grdb_store, flipping _durable_mode on for BOTH the NE and the shipping in-process backend. The shipping backend runs in the app process with a process-local config dir and no ModelBInboundReplay, so forcing durable mode there can lose inbound or bypass field/UI processing. Add host_persistence to start() (default "in_process"); only the NE passes "ne" to enable the durable inbox + GRDB store. The shipping PythonBridge passes no such arg, so it keeps the established in-process inbound path.
…projection P1 #4 (ne-python-architecture-review): a failed inbound GRDB write (store missing/ locked, e.g. app not yet launched) returned with the message lost 'until the peer re-sends'. Retain the raw artifact durably in a NE-owned JSONL file (ingress-retry.jsonl, next to the store) so the app's drain_inbox cannot delete it, and re-project it into the store once the store is reachable again (triggered by a later successful inbound write). Projection is idempotent (INSERT OR REPLACE by message_id). The success path still gates on the write, so a retained message does not post its banner/newMessage ping until it is actually persisted.
This comment has been minimized.
This comment has been minimized.
The event-drain worker (requestDrain -> drainNow) suspends across the .drainEvents IPC await. A stop() landing in that window bumps startGeneration and clears lastSeenAnnounce, but the resumed worker then yielded its drained payload onto the stream and re-wrote lastSeenAnnounce for a node that is stopped - resurrecting state and surfacing events on a stopped incarnation (and, with a later start(), enabling a concurrent drain). drainNow now captures the startGeneration before the await and re-checks it under stateLock after, discarding a stale payload. The lastSeenAnnounce diff/update is also serialized under stateLock (stop() clears it under the same lock, so the prior unguarded access was a data race). Regression test (ProxyDrainRaceTests) drives the real ProxyRnsBackend through its injectable send transport: a held .drainEvents reply + stop() must not yield; a live drain must. Both green on the sim.
P1 #3 (stranded sends) + P1 #6 (send-response ambiguity). The outbox *reader* was deleted with NEReticulumNode, so a send enqueued on NE-down was never replayed; and a naive drain could double-send, because the fallback assumes a missing reply means no acceptance when the NE may already have sent it. The replay reader lives in the NE start path (reticulum stays solely in the NE); the outbox read + stable-sendId dedup is the pure, unit-testable piece. - OutboxEntry gains sendId (app-assigned UUID at enqueue; optional so pre-migration entries decode + replay once). - SentIdStore: durable idempotent sent-id set in the App-Group container, shared by app + NE, survives an NE restart. - OutboxReplayCoordinator.pendingReplays(): drain the queue, return only the entries whose sendId is not yet recorded (nil-id always qualifies). - ProxyRequest.lxmfSend gains an optional sendId wire field; the app assigns one id per logical send and uses it in both the live IPC request and the outbox fallback. - NE lxmfSend dedups on sendId (skip + report committed if already sent), marks it sent on commit; NE start() replays the outbox through that path, re-queueing entries that fail to send so they retry on the next start. - Registered the new Shared files (Model B app + NE, not shipping) and the test (Model B tests) in the pbxproj + the Model B target-isolation canonical lists. Regression test (OutboxReplayDedupTests, sim): recorded-id re-enqueue is not replayed; unrecorded/nil ids replay. Full ColumbaModelBAppTests green (20 cases).
rns_bridge.drain_inbox deleted the whole ne_inbox table on read, so a lost IPC reply or an app termination before the consumer processed the batch lost delivery-state updates and announces permanently (review Spec #5, Contract §5). Replace the destructive read with a bounded, non-destructive read that returns each row with its seq, and advance the cursor only on an explicit ack: - drain_inbox: SELECT seq + payload, return each event with its seq, no DELETE - ack_inbox(max_seq): DELETE only rows at or below the cursor (idempotent, safe below the floor) - ProxyEvent.seq (Int?, self-mapping) + ProxyRequest.ackInbox(maxSeq:) - app drainNow: re-emit the batch, then fire a fire-and-forget .ackInbox at the highest seq (guarded by startGeneration so a stale drain across a stop/restart does not advance the cursor) - NE dispatchPython .ackInbox -> NEPythonRNS.ackInbox -> rns_bridge.ack_inbox Delivery is now at-least-once: unacked rows survive a lost reply / app death and are re-returned on the next drain; announce dedup + the UI's message-hash idempotency make the re-delivery idempotent. TDD: RnsBridgeInboxDrainTest (5 tests, red->green) in test_rns_bridge_persistence (already a wired CI static module). Full CI static list 210 passed; ColumbaModelBAppTests 20 passed, 0 failed; 0 em-dashes added.
MODEL_B_BACKGROUND_DELIVERY.md still described the retired ReticulumSwift / NEReticulumNode / app-owned-radio topology. Rewritten to the current shape: Python RNS (rns_bridge.py) runs in-process in the NE as the SOLE runtime; the app is a UI satellite over ProxyIPC. Topology diagram, ownership table, inbound/outbound/announce flows, IPC + cursor-acked event bridge, and the verification checklist updated; the 2026-06-02 evidence marked historical (it predates both the target split and the engine migration). (Note: the 28MB swiftly-x86_64.tar.gz removal landed in the prior commit 4c777e1 alongside the cursor-ack fix.)
The p2-deadfiles deletion removed AppGroupBLEDriver.swift,
ModelBRNodeService.swift, AppGroupRNodeSeamTransport.swift,
AppGroupRNodeSeamWire.swift, and AppGroupRNodeServer.swift from the repo,
but the authoritative reconciler (support/isolate-modelb-targets.rb) and its
lockstep test copy still named them as required Model-B-only sources. The
reconciler's model_b_source_references does project.files.fetch { raise }, so
any run of it (or the isolation test, if ever re-wired) would raise on the
missing file refs, and the test's hardcoded count (26) was inflated by the 5
dead entries.
Removed the 5 dead entries from both the reconciler and the test's
MODEL_B_ONLY_SOURCE_PATHS copy (kept in lockstep, same order), and fixed the
test's count 26 -> 21. Verified: both files Ruby-syntax-OK, lists in lockstep
(21 == 21, same order), every remaining reconciler-listed Model B file exists
on disk, and 0 of the 5 dead names remain in the reconciler.
Note: MODEL_B_DECLARATIONS_ABSENT_FROM_SHIPPING (type-name guard) and the
LIVE retired-lookalikes (ModelBBLEService / AppGroupBLEServer /
AppGroupBLESeamTransport, referenced by the NE BLE driver seam) are untouched.
0 em-dashes added.
…(Std #3) test_model_b_ble_service_gated_on_enabled_ble_interface asserted that AppServices still starts the app-side BLE host (ModelBBLEService.shared.start under a shouldStart guard) - but Phase 2 moved the mesh CoreBluetooth radio into the Network Extension (SwiftBLEBridge linked into the NE target, driven in-process by the NE's Python driver). The app-side start hook is now a no-op (syncModelBBLEService), so the assertion failed. The review (architecture review, Verification section) recommended updating the test to assert current NE ownership rather than restore the retired behavior. Updated: assert the app no longer starts the host (0 ModelBBLEService.shared .start( calls), that syncModelBBLEService is a no-op with the Phase 2 log line, and that the call sites still run the no-op. The service-level gating assertions (shouldStart/hasEnabledBLEInterface, no opt-in API) are unchanged - they still hold. Full CI static list 220 passed; 0 em-dashes added.
…ming (Std #2/#3) Two Std #3 cleanups: 1. signalQuality: the 60/75/90 dBm -> SignalQuality bucket mapping was copy- pasted in AppServices (Int?) and ProxyRnsBackend (Int). Consolidated into a single static SignalQuality.quality(forRssi: Int?) in RNSAPI (imported by both app-side targets); both call sites now route through it and the two local copies are removed. The buckets can no longer drift between the app and the Model B proxy. 2. StubEngine doc (Std #2 'make incomplete status explicit'): the review explicitly objected to describing wiring the durable production contract as a 'one-line engine swap'. Reworded both PacketTunnelProvider doc blocks to state the real work remaining (NEPythonRNS NodeEngine conformance + admission recovery + execution recovery + app-facade integration) instead of the 'one-line change in nodeOwnerIfNeeded' framing. The BLE peer-grouping loop (AppServices + PacketTunnelProvider) is left as-is: it depends on SwiftBLEBridge.BleConnectionDetails (not a Shared/Foundation target) and the two copies produce different output types, so consolidating it would add a cross-target dependency + pbxproj entry for a small win - a judgment-call cleanup the review flags, not a merge gate. Model B build green; 0 em-dashes added.
Owner
Author
|
@greptile review |
Greptile's fresh review of 9108464 came back 0/5 with 12 issues. All verified against the code and fixed; the 6 P1s are real data-loss bugs in the outbox/announce/ack paths this branch introduced. P1 #1 IOSRNodeInterface: atxc/atxb/arxc/arxb were never initialized; the first announce raised AttributeError. Now initialized in __init__. P1 #2 IOSBLEDriver: on a MAC-rotation reconnect, onIdentityReceived fired before the new link connected but re-routed immediately; a failed handshake left the route pointed at a dead address. The route migration is now deferred to a pending map and applied only when onDeviceConnected confirms the new address (immediate path for the already-connected case; cleanup on disconnect/stop). Test updated to the new behavior + new connect-fails test. P1 #3 OutboxQueue/OutboxReplayCoordinator: pendingReplays() drained (read-all-and-clear) before the send, so a mid-loop stop dropped not-yet-sent entries. Drain is now non-destructive (queue.pending()) and commitSent() prunes only the confirmed-sent entries. P1 #4 NEPythonRNS: a failed replay only retried on next start. Added a bounded armReplayRetry() task (5s cadence) that re-runs replayOutbox() while the node is up and entries remain, self-terminating on drain or stop. P1 #5 SentIdStore: record() now returns Bool; markSent() returns it; lxmfSend surfaces a committed-but-unpersisted id (res['id_persisted']=false + loud log) instead of silently claiming the lost-reply guard holds. P1 #6 ModelBNetworkTransport: a re-delivered (ackInbox at-least-once) link_state=established for the active link was treated as a competing inbound call (BUSY + teardown of the live link). Now ignored when the linkId is the active one. P2 #7 NEPythonRNS: shortError() collapses a Python traceback to its final exception line (container paths live only in the File frames above it) so the debug log never carries device paths. Applied to call() and invoke(). P2 #8 rns_bridge: a delivery failing during a retry pass was dropped (never retained). Now failing deliveries are always retained, and the retry rewrite is merge-aware (re-reads the file, keeps failures + any new append, drops only projected ones). P2 #9 rns_bridge: saved messages were only re-tried on a later successful delivery. Added a generation-gated periodic retry loop (armed at start in NE mode, bailed on stop/reset) so retained mail is projected the moment the store becomes reachable. P2 #10 rns_bridge: a retried location-only share lost its skip_conversation decision (re-derived as a chat, creating an empty conversation). The reconstructed holder now preserves the recorded decision and the write honors it. P2 #11 rns_bridge: a re-projection re-raised the conversation unread count (the message row is INSERT OR REPLACE but the upsert always incremented). Now an already-stored message skips the conversation upsert. P2 #12 SentIdStore: the file was unbounded and each send scanned it twice. Now there is an in-memory id cache (size-refreshed) so contains/record are O(1), and the store is bounded by maxCapacity with oldest-id pruning. Verification: Model B build green; full CI static list 211 passed; full ColumbaModelBAppTests suite green (incl. OutboxReplayDedupTests 4/4, ProxyDrainRaceTests, RNodeSeamTests); 0 em-dashes added; Python compiles.
Owner
Author
|
@greptile review |
The fresh review of 95639b1 returned 0/5 with 3 inline findings (findings 4 and 5 in the summary text re-state the pre-95639b16 issues that this branch already fixed - armReplayRetry is wired in start() and record()/markSent() now return Bool with id_persisted surfacing; verified present). All 3 inline findings are real and in code from the previous pass; fixed: Issue 1 (repeat sends): commitSent() kept every nil-sendId (legacy) entry, so the bounded 5s outbox retry loop re-sent a successfully-sent legacy entry on every pass forever. Added OutboxEntry.legacyKey (a stable identity for a nil-id entry: dest+content+method+fields), replayOutbox() now collects the exact nil-id entries it confirmed sent into a legacyKeys set, and commitSent(legacyKeys:) prunes those specifically while still pruning sendId'd entries by the sent-id store. A failed legacy entry is NOT confirmed, so it stays for the next pass. Two regression tests added (testConfirmedLegacyEntryIsPruned, testFailedLegacyEntryIsKept). Issue 2 (thread race): SentIdStore is a shared final class whose cachedIds / cachedFileBytes / cacheLoaded are mutated by BOTH the NE live-send path and the outbox retry task, but withFileLock used only an fcntl/flock file lock - and that lock is per-process on macOS, so it does NOT serialize two threads in the same extension. Added an in-process NSLock (processLock) taken first in withFileLock, closing the same-process gap while the file lock still covers cross-process (app <-> NE) access. Every cache read/mutation already runs under withFileLock, so this single lock fully serializes in-process access. Issue 3 (hidden recovered mail): the periodic ingress-retry loop projected retained inbound artifacts into the store but never posted the network.columba.newMessage ping, so the app's Chats list and ModelBInboundReplay (which only re-scan on that ping) never picked up the recovered rows - a message retained while the store was unavailable stayed hidden until the next real message or app relaunch. The loop now posts the newMessage ping only when a pass actually recovered at least one message (projected > 0), so a no-op pass does not trigger a redundant reload. No banner is posted (recovery is not a live arrival; a banner per 5s pass would be worse than silent; the newMessage reload surfaces the row). Verification: Model B build green; full static CI list 211 passed; full ColumbaModelBAppTests suite green (incl. the 2 new legacy-pruning regression tests + all 6 OutboxReplayDedupTests); 0 em-dashes added; Python compiles.
The send path was unobservable in diag/ext-diag logs: the app proxy logs via os_log (absent from diag.log) and the NE lxmfSend only logged a dedup-skip or persist-warning, so a plain send or a requesting-path / not-started failure left zero trace and was indistinguishable from never being called. Log the outcome at the app ViewModel layer and the exact ok/reason at the NE lxmfSend layer so a real send's branch (ipc-fail -> queued, no-route -> requestingPath, or the Python reason) is readable.
Issue 1 (legacyKey collision): legacyKey was content-derived (dest, content, method, fields), so two pre-sendId entries with identical content but different enqueue times shared a key and commitSent() removed both when one succeeded and the other failed, losing the failed message. Add createdAt to the key so each entry has a unique identity; pruning a confirmed-sent entry no longer removes its identical sibling. Issue 2 (failed ID write invisible): lxmfSend set id_persisted=false in the NE response but the app's ProxySendOutcome had no such field, so the flag was dropped at the IPC boundary and the app reported success silently. Add idPersisted to ProxySendOutcome, wire it through mapSendOutcome, and log the degradation app-side. The outcome stays .queued (the message went out; flipping to failure would make the app re-enqueue and double-send, strictly worse) but the degradation is now visible to operators. Issue 3 (no newMessage after recovery): already fixed in 85c515e (rns_bridge.py posts network.columba.newMessage when projected > 0). Stale finding; no change. Adds a regression test: two identical-content legacy entries with distinct createdAt must have distinct keys, and pruning one must not remove the other.
Owner
Author
|
@greptile review |
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
1 participant
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.
NE sole-runtime migration (Model B)
Moves the entire Reticulum runtime + all radio drivers (RNode BLE and mesh
BLE) out of the app process and into the Network Extension. The app becomes
a UI-only process that talks to the NE over the IPC seam, per the original
planning docs.
What changed
a thin proxy over the IPC channel.
the app-side session seam is deleted.
(SwiftBLEBridge) is linked into the tunnel plugin and driven by the in-NE
Python IOSBLEInterface via the C-ABI. The app-side Model B host
(
ModelBBLEService) is neutered;getBLEConnectionInfos/isBLEActive/disconnectBLEPeerroute through the proxy to the NE..start, the nodeself-heals (re-drives start when a display name + config are present),
fixing the node-orphaning race that dropped RNode after a full NE relaunch.
UIBackgroundModes [bluetooth-central, bluetooth-peripheral]+NSBluetoothAlwaysUsageDescriptionto the NE.Without
bluetooth-peripheralthe OS demotes advertised service UUIDs tothe iOS-only "overflow" area, so Android (non-iOS) never discovered the
iPhone - a real cross-platform discovery bug, now fixed.
driver.get_peer_role()from a callback that runs on the BLE serial queue;the accessor now uses a dispatch-specific
serializedhelper (inline whenon-queue) instead of
queue.sync, which aborted with"dispatch_sync called on queue already owned by current thread" and
crash-looped the NE every ~9s.
announce_rate_* attrs, address normalization, BlueZ adapter health checks).
assertion to the live
RNodeSeam.swift(the old target was deleted in anearlier commit), fixed a test-node-send hook that logged a message-body
prefix (now logs byte count), and updated the BLE-bridge send-synchronicity
assertion for the re-entrancy accessor.
Verification (on-device, seq 3176)
startTunnelcount flat over a 60s window.peers=2: iOS + Android hold a live BLE link (waspeers=0pre-migration).state=2) through NE relaunches.Follow-ups (separate commits / PRs, not in this branch)
ModelBBLEService/AppGroupBLEServer/AppGroupBLESeamTransportfiles + deadbleSeam*SharedFrameQueue names(kept the
BLEDriverSeamcodec still used byRNodeSeam).RNodeWizard; full on-device smoke.