Skip to content

use npm list instead of custom traversal to get native module versions#70

Merged
gmaclennan merged 3 commits into
mainfrom
ac/update-native-versions-detection
May 28, 2026
Merged

use npm list instead of custom traversal to get native module versions#70
gmaclennan merged 3 commits into
mainfrom
ac/update-native-versions-detection

Conversation

@achou11

@achou11 achou11 commented May 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Currently not an issue, but somehow had a deps tree locally such that npm list sodium-native looked like this:

└─┬ @comapeo/core@7.1.0
  ├─┬ @hyperswarm/secret-stream@6.9.1
  │ └─┬ sodium-universal@5.0.1
  │   └── sodium-native@5.1.0
  └─┬ sodium-universal@4.0.1
    └── sodium-native@4.3.3

When running npm backend:build, only sodium-native@4.3.3 was detected and built for, which caused runtime errors related to failing to load the library for sodium-native@5.1.0. I've since updated locally such that it now looks like this, which works with the existing implementation:

└─┬ @comapeo/core@7.1.0
  ├─┬ @hyperswarm/secret-stream@6.9.1
  │ ├─┬ noise-curve-ed@2.1.0
  │ │ └─┬ sodium-universal@5.0.1
  │ │   └── sodium-native@5.1.0
  │ ├─┬ noise-handshake@4.2.0
  │ │ └─┬ sodium-universal@5.0.1
  │ │   └── sodium-native@5.1.0
  │ ├─┬ sodium-secretstream@1.2.0
  │ │ └─┬ sodium-universal@5.0.1
  │ │   └── sodium-native@5.1.0
  │ └─┬ sodium-universal@5.0.1
  │   └── sodium-native@5.1.0
  └─┬ sodium-universal@4.0.1
    └── sodium-native@4.3.3

The custom fs traversal for finding versions most likely doesn't account for the "extra" layer seen with sodium-universal in the failing example. Instead of updating the existing implementation, I think it'd be safer to use npm directly via its ls command (docs). Passing the --parseable flag causes the stdout to be a newline-separated list of absolute paths to the module e.g.

❯ npm list sodium-native --parseable
/<redacted>/backend/node_modules/sodium-native
/<redacted>/backend/node_modules/@hyperswarm/secret-stream/node_modules/sodium-native
/<redacted>/backend/node_modules/noise-curve-ed/node_modules/sodium-native
/<redacted>/backend/node_modules/noise-handshake/node_modules/sodium-native
/<redacted>/backend/node_modules/sodium-secretstream/node_modules/sodium-native

From an eye test, this approach is slightly slower than the existing approach but is less prone to error.

@achou11 achou11 requested a review from gmaclennan May 18, 2026 19:54
gmaclennan added a commit that referenced this pull request May 19, 2026
@gmaclennan @claude
fix(ios-tests): serialise STOPPING/STOPPED observers in testFullLifec…
64bdaa9 
…ycleStateTransitions (#71)

## Summary

Fixes a race in
[`testFullLifecycleStateTransitions`](ios/Tests/NodeJSServiceTests.swift#L582)
that intermittently fails the macOS Swift Package Tests job ([example
failure on
#70](https://github.com/digidem/comapeo-core-react-native/actions/runs/26056852629/job/76606978112)):

```
NodeJSServiceTests.swift:624: error:
  -[ComapeoCoreTests.NodeJSServiceTests testFullLifecycleStateTransitions] :
  XCTAssertLessThan failed: ("3") is not less than ("2") -
  STOPPING should come before STOPPED
```

The recorded `transitions` array ended up `[starting, started, stopped,
stopping]` rather than the expected `[starting, started, stopping,
stopped]`.

## The race

Same shape as PR #59, slightly different underlying mechanic. PR #59
fixed the sister test `testStopSendsShutdownMessageOverIPC`; this test
was introduced in PR #47 and wasn't covered by that fix.

`NodeJSService.applyAndEmit` releases the service lock *before* invoking
`onStateChange?(derived)`
([NodeJSService.swift:277](ios/NodeJSService.swift#L277), then
[:319](ios/NodeJSService.swift#L319)) — a deliberate requirement of
[`testObserverCanReenterLockedMethodFromCallback`](ios/Tests/NodeJSServiceTests.swift#L634),
which asserts the callback can re-enter locked methods like `cleanup()`
without deadlocking. The state mutations are linearised under the lock,
but the post-unlock observer invocations are not.

The test calls `signalExit()` *before* `service.stop(timeout: 1)`:

```swift
// Stop
signalExit()
service.stop(timeout: 1)
```

That makes the node thread's `applyAndEmit` (writing `nodeRuntime =
.exited(.requested)` → derives STOPPED) race the main thread's
`applyAndEmit` from inside `stop()` (writing `stopRequested = true` →
derives STOPPING). Even when the locked state transitions run in the
right order, the post-unlock `onStateChange` callbacks can be reordered
by thread scheduling, so the recorded array sees STOPPED before
STOPPING. On a quiet runner the main thread usually wins; on a loaded
macOS-14-arm64 CI runner it sometimes loses.

## The fix

Identical pattern to PR #59: dispatch `stop()` to a background queue,
register a STOPPING expectation, and `wait(for: [stoppingExpectation])`
before calling `signalExit()`. That pins the observer ordering —
STOPPING is appended to `transitions` before the node thread is ever
unblocked, so the later STOPPED append from the node thread necessarily
lands at a higher index.

```swift
let stopFinished = expectation(description: "stop() returned")
DispatchQueue.global().async {
    service.stop(timeout: 1)
    stopFinished.fulfill()
}
wait(for: [stoppingExpectation], timeout: 5)
signalExit()
wait(for: [stopFinished], timeout: 5)
```

## Test plan

- [ ] `swift test --filter
NodeJSServiceTests/testFullLifecycleStateTransitions` × 50 — all pass
- [ ] `swift test` (full suite) — clean

(I'm on a Linux container so can't run `swift test` here; verifying via
CI on this PR.)

https://claude.ai/code/session_01BRgG9bgdFy9pZCov2uHyef

---
_Generated by [Claude
Code](https://claude.ai/code/session_01BRgG9bgdFy9pZCov2uHyef)_

Co-authored-by: Claude <noreply@anthropic.com>
@gmaclennan

gmaclennan commented May 19, 2026

Copy link
Copy Markdown
Member

Added 835f46a just because spawning an npm ls for every module seems a lot, and we can also skip the package.json lookup. 3.27s --> 0.57s on my machine.

@gmaclennan gmaclennan merged commit 043f3be into main May 28, 2026
7 checks passed
@gmaclennan gmaclennan deleted the ac/update-native-versions-detection branch May 28, 2026 08:43
gmaclennan added a commit that referenced this pull request Jun 10, 2026
@gmaclennan
Merge remote-tracking branch 'origin/main' into e2e-maestro
378fe89 
* origin/main:
  fix(sentry): make exit telemetry lossless and stop cross-process clobbering (#84)
  feat(sentry): land Phases 6 + 7a — Android exit reasons & iOS MetricKit app-exit telemetry (#72)
  chore(build): use npm list instead of custom traversal to get native module versions (#70)
gmaclennan added a commit that referenced this pull request Jun 16, 2026
@gmaclennan
Merge remote-tracking branch 'origin/main' into map-server-integration
9439b06 
* origin/main:
  chore(e2e): add e2e tests on browserstack via Maestro (#56)
  fix(sentry): make exit telemetry lossless and stop cross-process clobbering (#84)
  feat(sentry): land Phases 6 + 7a — Android exit reasons & iOS MetricKit app-exit telemetry (#72)
  chore(build): use npm list instead of custom traversal to get native module versions (#70)
@optic-release-automation optic-release-automation Bot mentioned this pull request Jun 22, 2026
Merged
gmaclennan added a commit that referenced this pull request Jun 22, 2026
@gmaclennan
[OPTIC-RELEASE-AUTOMATION] release/v1.0.0-pre.2 (#112)
15131a5 
## Optic Release Automation

This **draft** PR is opened by Github action
[optic-release-automation-action](https://github.com/nearform-actions/optic-release-automation-action).

A new **draft** GitHub release
[v1.0.0-pre.2](https://github.com/digidem/comapeo-core-react-native/releases/tag/untagged-c499977757c9745e56b2)
has been created.

Release author: @gmaclennan

#### If you want to go ahead with the release, please merge this PR.
When you merge:

- The GitHub release will be published

- The npm package with tag pre will be published according to the
publishing rules you have configured



- No major or minor tags will be updated as configured


#### If you close the PR

- The new draft release will be deleted and nothing will change

## What's Changed
* Android Testing Infrastructure & Bug Fixes by @gmaclennan in
#3
* chore: prebuild example/android; harden instrumented tests by
@gmaclennan in
#10
* Integrate @comapeo/core via IPC over Unix sockets by @gmaclennan in
#5
* chore: adjust repo setup by @achou11 in
#12
* chore: minor fixes based on expo-doctor by @achou11 in
#13
* Add iOS support & test infrastructure by @gmaclennan in
#6
* chore: add architecture docs & plans by @gmaclennan in
#11
* update some native deps used in backend by @achou11 in
#14
* iOS Phase 1: unified JS bundle + smoke test (simulator-only) by
@gmaclennan in
#15
* iOS Phase 2: xcframework Embed & Sign for native addons by @gmaclennan
in #16
* Phase 2 Android: jniLibs packaging + unified rollup loader plugin by
@gmaclennan in
#17
* chore: post-Phase-2 cleanup — comments, plan docs, agents.md by
@gmaclennan in
#33
* android: read abiFilters from reactNativeArchitectures (#30) by
@gmaclennan in
#35
* refactor: simplify build-backend.ts; rollup writes directly to native
asset trees by @gmaclennan in
#34
* chore: fix eslint configuration by @achou11 in
#41
* android: audit 16 KB page alignment on every shipped .so by
@gmaclennan in
#43
* Add rootkey persistence and lifecycle state management by @gmaclennan
in #36
* chore: move example app into apps directory by @achou11 in
#18
* refactor: per-component lifecycle state with derived ComapeoState by
@gmaclennan in
#47
* android: fold waitForFile into connect retry loop by @gmaclennan in
#52
* chore: add e2e testing app by @achou11 in
#49
* fix(android): drop setUnlockedDeviceRequired from rootkey wrapper key
by @gmaclennan in
#57
* fix(backend): cache stopping/error frames for late joiners by
@gmaclennan in
#58
* fix(ios-tests): wait for STOPPING before signalling node exit by
@gmaclennan in
#59
* fix(android): drain JNI stdio pumps before returning from node::Start
by @gmaclennan in
#60
* Sentry integration: Phase 1 + Phase 2a + Phase 2b by @gmaclennan in
#54
* feat(backend): polywasm-backed undici on iOS, re-enable maps plugin by
@gmaclennan in
#62
* ci: drop unreliable Android emulator snapshot caching by @gmaclennan
in #64
* feat(sentry): land Phase 3 — backend loader + RPC tracing by
@gmaclennan in
#63
* fix(ios-tests): serialise STOPPING/STOPPED observers in
testFullLifecycleStateTransitions by @gmaclennan in
#71
* use npm list instead of custom traversal to get native module versions
by @achou11 in
#70
* feat(sentry): land Phases 6 + 7a — Android exit reasons & iOS
MetricKit app-exit telemetry by @gmaclennan in
#72
* fix(sentry): make exit telemetry lossless and stop cross-process
clobbering by @gmaclennan in
#84
* chore(e2e): add e2e tests on browserstack via Maestro by @achou11 in
#56
* feat(sentry): migrate to @sentry/react-native v8; exit telemetry as
Application Metrics by @gmaclennan in
#73
* Map server integration by @gmaclennan in
#86
* chore(deps): upgrade to Expo SDK 56 (React Native 0.85) by @gmaclennan
in #87
* chore(ci): add release workflow by @gmaclennan in
#90
* chore: fix npm script and release build script by @gmaclennan in
#91
* chore(pack): don't try to package build files by @gmaclennan in
#92
* fix: start fastify listening by @gmaclennan in
#93
* perf(backend): switch bundler from rollup to rolldown by @gmaclennan
in #94
* fix(ci): ignore-scripts in ios npm installs by @gmaclennan in
#96
* fix(ci): replace --ignore-scripts with npm strict-allow-scripts
allowlist by @gmaclennan in
#106
* feat(config): let the consuming app supply the default project config
by @gmaclennan in
#95
* chore(release): merge prerelease branch. by @gmaclennan in
#110

## New Contributors
* @achou11 made their first contribution in
#12

**Full Changelog**:
https://github.com/digidem/comapeo-core-react-native/commits/v1.0.0-pre.2

<!--

<release-meta>{"id":342868678,"version":"v1.0.0-pre.2","npmTag":"pre","opticUrl":"https://optic-zf3votdk5a-ew.a.run.app/api/generate/"}</release-meta>
-->
@gmaclennan gmaclennan added the maintenance Refactor / test / chore / ci / build (changelog) label Jun 22, 2026
@optic-release-automation optic-release-automation Bot mentioned this pull request Jun 22, 2026
Merged
gmaclennan added a commit that referenced this pull request Jun 22, 2026
@gmaclennan
[OPTIC-RELEASE-AUTOMATION] release/v1.0.0-pre.2 (#133)
20a3454 
## Optic Release Automation

This **draft** PR is opened by Github action
[optic-release-automation-action](https://github.com/nearform-actions/optic-release-automation-action).

A new **draft** GitHub release
[v1.0.0-pre.2](https://github.com/digidem/comapeo-core-react-native/releases/tag/untagged-352a6c41c12fd02dec37)
has been created.

Release author: @gmaclennan

#### If you want to go ahead with the release, please merge this PR.
When you merge:

- The GitHub release will be published

- The npm package with tag pre will be published according to the
publishing rules you have configured



- No major or minor tags will be updated as configured


#### If you close the PR

- The new draft release will be deleted and nothing will change

<!-- Release notes generated using configuration in .github/release.yml
at 7fe80b4 -->

## What's Changed
### 🚀 Features
* Integrate @comapeo/core via IPC over Unix sockets by @gmaclennan in
#5
* Add iOS support & test infrastructure by @gmaclennan in
#6
* iOS Phase 1: unified JS bundle + smoke test (simulator-only) by
@gmaclennan in
#15
* iOS Phase 2: xcframework Embed & Sign for native addons by @gmaclennan
in #16
* Phase 2 Android: jniLibs packaging + unified rollup loader plugin by
@gmaclennan in
#17
* android: read abiFilters from reactNativeArchitectures (#30) by
@gmaclennan in
#35
* Add rootkey persistence and lifecycle state management by @gmaclennan
in #36
* Sentry integration: Phase 1 + Phase 2a + Phase 2b by @gmaclennan in
#54
* feat(backend): polywasm-backed undici on iOS, re-enable maps plugin by
@gmaclennan in
#62
* feat(sentry): land Phase 3 — backend loader + RPC tracing by
@gmaclennan in
#63
* feat(sentry): land Phases 6 + 7a — Android exit reasons & iOS
MetricKit app-exit telemetry by @gmaclennan in
#72
* feat(sentry): migrate to @sentry/react-native v8; exit telemetry as
Application Metrics by @gmaclennan in
#73
* Map server integration by @gmaclennan in
#86
* feat(config): let the consuming app supply the default project config
by @gmaclennan in
#95
### 🐛 Bug Fixes
* fix(android): drop setUnlockedDeviceRequired from rootkey wrapper key
by @gmaclennan in
#57
* fix(backend): cache stopping/error frames for late joiners by
@gmaclennan in
#58
* fix(ios-tests): wait for STOPPING before signalling node exit by
@gmaclennan in
#59
* fix(android): drain JNI stdio pumps before returning from node::Start
by @gmaclennan in
#60
* fix(ios-tests): serialise STOPPING/STOPPED observers in
testFullLifecycleStateTransitions by @gmaclennan in
#71
* fix(sentry): make exit telemetry lossless and stop cross-process
clobbering by @gmaclennan in
#84
* fix: start fastify listening by @gmaclennan in
#93
* fix(ci): ignore-scripts in ios npm installs by @gmaclennan in
#96
* fix(ci): replace --ignore-scripts with npm strict-allow-scripts
allowlist by @gmaclennan in
#106
* fix(release): stop `npm pack --dry-run` leaking dry-run into backend
install by @gmaclennan in
#129
### ⚡ Performance
* perf(backend): switch bundler from rollup to rolldown by @gmaclennan
in #94
### ⬆️ Dependencies
* update some native deps used in backend by @achou11 in
#14
* chore(deps): upgrade to Expo SDK 56 (React Native 0.85) by @gmaclennan
in #87
### 🏗️ Maintenance
* Android Testing Infrastructure & Bug Fixes by @gmaclennan in
#3
* chore: prebuild example/android; harden instrumented tests by
@gmaclennan in
#10
* chore: adjust repo setup by @achou11 in
#12
* chore: minor fixes based on expo-doctor by @achou11 in
#13
* chore: add architecture docs & plans by @gmaclennan in
#11
* chore: post-Phase-2 cleanup — comments, plan docs, agents.md by
@gmaclennan in
#33
* refactor: simplify build-backend.ts; rollup writes directly to native
asset trees by @gmaclennan in
#34
* chore: fix eslint configuration by @achou11 in
#41
* android: audit 16 KB page alignment on every shipped .so by
@gmaclennan in
#43
* chore: move example app into apps directory by @achou11 in
#18
* refactor: per-component lifecycle state with derived ComapeoState by
@gmaclennan in
#47
* android: fold waitForFile into connect retry loop by @gmaclennan in
#52
* chore: add e2e testing app by @achou11 in
#49
* ci: drop unreliable Android emulator snapshot caching by @gmaclennan
in #64
* use npm list instead of custom traversal to get native module versions
by @achou11 in
#70
* chore(e2e): add e2e tests on browserstack via Maestro by @achou11 in
#56
* chore(ci): add release workflow by @gmaclennan in
#90
* chore: fix npm script and release build script by @gmaclennan in
#91
* chore(pack): don't try to package build files by @gmaclennan in
#92
* chore(release): merge prerelease branch. by @gmaclennan in
#110
* ci(e2e): retry BrowserStack builds on infra-class flakes by
@gmaclennan in
#113
### Other Changes
* ci: derive changelog labels from PR titles + add Dependabot by
@gmaclennan in
#114

## New Contributors
* @achou11 made their first contribution in
#12
* @optic-release-automation[bot] made their first contribution in
#112

**Full Changelog**:
https://github.com/digidem/comapeo-core-react-native/commits/v1.0.0-pre.2

<!--

<release-meta>{"id":342970724,"version":"v1.0.0-pre.2","npmTag":"pre","opticUrl":"https://optic-zf3votdk5a-ew.a.run.app/api/generate/"}</release-meta>
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmaclennan gmaclennan Awaiting requested review from gmaclennan

Assignees

No one assigned

Labels

maintenance Refactor / test / chore / ci / build (changelog)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@achou11 @gmaclennan