[sparck75]

What and why

This closes the clear defects the ICM onboarding (#16) found but did not fix. The owner decided on 2026-10-15 to fix them in one PR per repository. Anything that needs the owner's decision is listed below and left unchanged. The PR is based on main at 3e2e8fe. #16 is not merged yet, so this PR touches no .github/icm/ file (see "ICM facts after #16 merges").

Nothing here publishes: no version bump, no tag. package.json's version stays 1.2.0.

Findings

#	Finding	Outcome
1	Security Audit is red: npm audit --audit-level=moderate on main's lockfile reports 37 vulnerabilities (1 critical, 31 high, 5 moderate), including runtime axios <= 1.19.0. The advisories were published after main's last green run.	Fixed in three commits. Each range change is listed below. The audit now reports found 0 vulnerabilities.
2	The package root needs socket.io-client, which is declared an optional peer. src/index.ts:126 re-exports WebSocketClient, and src/ws-client.ts:1 imports socket.io-client when the module loads, so a REST-only consumer gets MODULE_NOT_FOUND.	Fixed: socket.io-client is now loaded inside connect(). Existing imports are unchanged. Test: tests/optional-peer.test.ts.
3	The stray src/client.ts.patch	Removed. Nothing referenced it, and the listAll code it patches no longer exists in src/client.ts.
4a	Tags v1.1.0 and v1.2.0 point at feature-branch commits (392a6c3, fc6ab66), not at main	Left for the owner (below)
4b	CHANGELOG.md stops at 1.1.0	Left for the owner (below). An ## [Unreleased] section now records only this PR's changes, which is true and trivial.
4c	The duplicate-delivery key is subscription_id:unknown when there is no aggregate (src/receiver.ts:80)	Left for the owner (below)

1. Advisories (route advisory)

Reproduction: npm ci && npm audit --audit-level=moderate on 3e2e8fe reports 37 vulnerabilities (5 moderate, 31 high, 1 critical). The packages are axios, handlebars (critical, through ts-jest), brace-expansion, braces and sprintf-js.

1. 41f3f0e runs npm audit fix within the declared ranges. It changes only the lockfile. It resolves axios 1.20.0 (runtime), handlebars 4.7.10, brace-expansion and the rest. That leaves 34 vulnerabilities, all from two packages:

   • braces <= 3.0.3 (GHSA-vfj7-8cjw-p6xm, high): no patched release exists (3.0.3 is the latest). jest 29 reaches it through micromatch.
   • sprintf-js <= 1.1.3 (GHSA-hp3w-g68c-fv3c, moderate): no patched release exists. It is reached only through jest's coverage instrumentation: babel-plugin-istanbul → @istanbuljs/load-nyc-config → js-yaml@3 → argparse@1 → sprintf-js.

2. 584ea64 raises three ranges, all development-only, because braces has no fixed version and jest 30 no longer depends on it:

   • jest ^29.7.0 → ^30.5.2
   • jest-util ^29.7.0 → ^30.5.1
   • @types/jest ^29.5.14 → ^30.0.0

   ts-jest 29.4 already accepts jest 30, so its range stays. All tests pass and coverage stays above the floor.

3. c2ea5a7 adds an overrides entry: @istanbuljs/load-nyc-config → js-yaml ^4.1.0. Even jest 30 still reaches sprintf-js (babel-plugin-istanbul@8 keeps load-nyc-config@1.1.0, which pins js-yaml 3). load-nyc-config calls only js-yaml.load() (index.js:80), and js-yaml 4 keeps that call, safe by default. With the override, argparse@1 and sprintf-js leave the tree. This also affects the development tree only.

The runtime range for axios stays ^1.7.9. A fresh consumer install already resolves 1.20.0. Raising the floor would stop a consumer's old lockfile from keeping a vulnerable axios, but it changes what this package declares, so it is listed for the owner.

2. The optional peer

Reproduction, before the fix:

• From a build of 3e2e8fe, packed and installed into an empty project, so only axios and zod are present: node -e "require('@alteriom/webhook-client')" fails with Error: Cannot find module 'socket.io-client'.
• npx jest tests/optional-peer.test.ts against the old src/ws-client.ts fails both tests with Cannot find module 'socket.io-client', and the stack ends at src/ws-client.ts:1:1 ← src/index.ts:126.

The fix, in src/ws-client.ts:

• Only the Socket type is imported statically. TypeScript erases it, so no runtime require remains.
• A local loadSocketIo() requires socket.io-client on the first connect().
• If socket.io-client itself cannot be found, connect() throws WebSocketClient needs the optional peer dependency socket.io-client: npm install socket.io-client. Any other load error is re-thrown unchanged.

The other option was a separate subpath export (@alteriom/webhook-client/websocket). I did not choose it: it would remove WebSocketClient from the root, which breaks every current import { WebSocketClient } from '@alteriom/webhook-client'. The lazy require keeps the root export, the constructor, connect()'s signature (still synchronous) and the declaration files. The emitted dist/ws-client.d.ts holds no socket.io-client reference. When the peer is installed, behaviour does not change.

The test, tests/optional-peer.test.ts:

• The root check runs src/index in a plain Node process (ts-node, transpile-only), where resolving socket.io-client fails as it does for a consumer. It runs outside jest's module registry and outside coverage. It asserts that the REST client, receiver, adapters and WebSocketClient all load.
• An in-process jest test (jest.doMock throwing MODULE_NOT_FOUND) checks connect()'s error.
• Running the root check outside jest is deliberate. Loading the whole root inside jest pulls src/guards.ts and src/adapters.ts, which no suite loads today, into the coverage report. Because roots is tests/, unloaded files are not counted. Global branch coverage would then fall to 49.8%, under the 54% floor, for reasons unrelated to this fix.

After the fix, the packed build installed without the peer gives root loaded: function function, and connect() throws the message above. With socket.io-client@4 installed, connect() opens a socket as before: against a closed local port it emits error with xhr poll error.

README.md already says "REST-only consumers don't need it", and that is now true, so it is unchanged.

Credentials

None found. The onboarding found none either: the secret-looking strings in tests/ and README.md are placeholders.

Checks run (locally, Node 24 / npm 11, after a clean npm ci)

Check	Result
npm audit --audit-level=moderate	found 0 vulnerabilities (was 37)
npm test -- --coverage	6 suites, 101 tests pass (was 5 / 99). Coverage: statements 65.27, branches 61.82, functions 60.79, lines 65.38, against floors of 57/54/53/57
npm run lint	0 errors, the same 8 existing any warnings
npm run build	ok; dist/ 108,389 bytes, under the 110 KB limit in ci.yml
npm pack --dry-run	21 files, dist/ + README + LICENSE
npm ls --all	clean

CI on this PR (Node 22, head 314bac8): Lint, Test (22) (6 suites, 101 tests), Build (bundle 108,389 bytes), Security Audit (found 0 vulnerabilities), CodeQL and Analyze all pass.

Left for the owner

• Tags v1.1.0 and v1.2.0 are not on main. Nothing is broken, and moving a published tag would change what it names. Options: (a) leave them and tag future versions on main after the merge, as ICM: make webhook-client ICM-ready #16's release.md says; (b) re-point them. Recommendation: (a).
• CHANGELOG.md has no 1.2.0 section, and its 1.1.0 entry omits AgentSubscriptions (feat: v1.1.0 — AgentSubscriptions client + fix delivery_mode types (#9, #10) #11). Options: (a) back-fill both from feat: v1.1.0 — AgentSubscriptions client + fix delivery_mode types (#9, #10) #11 and feat: v1.2.0 — agentSubscriptions.poll() helper for on_demand mode #12 at the next release; (b) leave the history as is. Recommendation: (a), in the next release PR, because that release needs a changelog edit anyway. This PR only adds [Unreleased] for its own changes.
• Duplicate key subscription_id:unknown (src/receiver.ts:80): with no aggregate, two different signed deliveries to one subscription count as duplicates. The Python SDK does the same (ICM: make alteriom-webhook-client-python ICM-ready alteriom-webhook-client-python#3), so this is a design shared by both SDKs. Options: key on a delivery id if the connector sends one, or skip duplicate detection when there is no aggregate. Recommendation: decide it once for both SDKs, from what the connector sends.
• axios floor. Raising axios from ^1.7.9 to ^1.20.0 makes every consumer resolve a patched axios, even one whose lockfile pins an older 1.x. Recommendation: do it in the next release (a patch-level change for consumers).

ICM facts after #16 merges

#16 adds .github/icm/, which is not on main yet, so these fact updates wait for its merge. Each gets verified_commit set to the new tip:

• _shared/gotchas.md "The optional peer is loaded by the package root" becomes false: the root no longer needs the peer. Remove the section, or reduce it to "loaded on connect()", and repoint src/ws-client.ts:1.
• _shared/architecture.md:18 cites src/ws-client.ts wrapping socket.io-client (still true); add "loaded on first connect()".
• _shared/build-and-test.md "Where a test belongs" should list tests/optional-peer.test.ts. "Audit at moderate fails CI" in gotchas.md should mention the overrides entry.
• _shared/gotchas.md "The changelog lags the tags": still true for 1.2.0; mention the [Unreleased] section.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
🔒 Security Review	✅ Completed 2026-10-15T17:25:39.859355Z	314bac8	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.