feat(mllp-client): Cloudflare Workers runtime adapter#616
Open
meleksomai wants to merge 19 commits intomainfrom
Open
feat(mllp-client): Cloudflare Workers runtime adapter#616meleksomai wants to merge 19 commits intomainfrom
meleksomai wants to merge 19 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 6aa75cc The changes in this PR will be included in the next version bump. This PR includes changesets to release 51 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
meleksomai
pushed a commit
that referenced
this pull request
Apr 29, 2026
Replaces the placeholder "(separate PR)" labels in the runtime status table with concrete PR numbers so readers can navigate to the stacked work. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
@glion/ack
@glion/annotate-delimiters
@glion/annotate-profile-context
@glion/annotate-profile-datatypes
@glion/annotate-profile-fields
@glion/annotate-profile-fields-code-systems
@glion/annotate-profile-segments
@glion/ast
@glion/builder
@glion/config
@glion/decode-escapes
@glion/encode-escapes
@glion/cli
@glion/hl7v2
@glion/jsonify
@glion/lint-max-message-size
@glion/lint-message-version
@glion/lint-no-trailing-empty-field
@glion/lint-profile-events-segments-order
@glion/lint-profile-extra-components
@glion/lint-profile-extra-fields
@glion/lint-profile-field-max-length
@glion/lint-profile-field-repetition
@glion/lint-profile-required-components
@glion/lint-profile-required-fields
@glion/lint-profile-table-values
@glion/lint-required-message-header
@glion/lint-segment-header-length
@glion/mllp
@glion/mllp-ack
@glion/mllp-client
@glion/mllp-transport
@glion/parser
@glion/preset-annotate-profile-recommended
@glion/preset-lint-profile-recommended
@glion/preset-lint-recommended
@glion/profiles
@glion/to-hl7v2
@glion/util-query
@glion/util-semver
@glion/util-timestamp
@glion/util-visit
@glion/utils
commit: |
10 tasks
Base automatically changed from
claude/create-mllp-client-package-uYzCj
to
main
April 29, 2026 16:01
meleksomai
force-pushed
the
claude/mllp-client-workers-runtime
branch
from
4ec3da5 to
22b32f5
Compare
meleksomai
changed the title
2 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #616 +/- ##
==========================================
+ Coverage 93.81% 93.97% +0.16%
==========================================
Files 134 133 -1
Lines 4093 4036 -57
Branches 1051 1035 -16
==========================================
- Hits 3840 3793 -47
+ Misses 253 243 -10
🚀 New features to boost your workflow:
|
This was referenced Apr 29, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Cloudflare Workers/runtime-specific entrypoint for @glion/mllp-client, plus CI/test scaffolding to run multi-runtime test suites (Node, Bun, workerd) in the monorepo.
Changes:
- Introduces a Workers adapter (
cloudflare:sockets-backedworkersConnect) and exports it viapackage.jsonsubpath +workerdconditional export. - Adds workerd integration tests using the “harness Worker +
wrangler.unstable_dev” pattern (plus Node-side TCP ACK server fixture). - Extends repo tooling/CI with
test:bun,test:cf, andtest:denoturbo tasks and dedicated CI jobs for Bun and workerd.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Adds turbo tasks for per-runtime test commands (test:bun, test:cf, test:deno). |
| pnpm-lock.yaml | Locks new dev dependency graph (notably wrangler/workerd tooling). |
| packages/mllp-client/vitest.config.ts | Splits mllp-client tests into Node + workerd projects. |
| packages/mllp-client/tsdown.config.ts | Adds workers runtime entrypoint build target and avoids bundling cloudflare:sockets. |
| packages/mllp-client/test/workers/wrangler.toml | Defines workerd harness configuration for integration tests. |
| packages/mllp-client/test/workers/harness.ts | Worker-side HTTP harness to invoke the real adapter in workerd. |
| packages/mllp-client/test/workers/global-setup.ts | Node-side TCP ACK server fixture for workerd tests. |
| packages/mllp-client/test/workers/adapter.test.ts | Node-side vitest suite driving the harness via HTTP. |
| packages/mllp-client/src/runtimes/workers.ts | Implements the Workers adapter and a pre-wired MllpClient export. |
| packages/mllp-client/package.json | Exports workers entrypoints; adds wrangler; adds runtime-specific test scripts. |
| packages/mllp-client/README.md | Documents Workers runtime support and the /workers import path. |
| packages/glion/package.json | Adds test:bun script to participate in the new Bun CI job. |
| package.json | Adds root scripts to run per-runtime tests via turbo. |
| .github/workflows/ci.yml | Replaces old Bun-only job with generic Bun testing; adds a dedicated workerd job. |
| .changeset/mllp-client-workers-runtime.md | Publishes the Workers adapter feature + documents the new runtime testing approach. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
BREAKING CHANGE: Empty fields, repetitions, and components are now always
represented with `children: []`. The `experimental.emptyMode` setting (which
toggled between "legacy" full-skeleton-with-empty-string-leaf and "empty"
empty-children-array) is removed entirely.
Why this matters now
====================
Every @glion/builder factory call (f(), r(), c()) was routing through
loadConfig() from @glion/config to decide which AST shape to produce. That
imported cosmiconfig + Node-only modules (fs, path, os, crypto, module, url)
into every consumer's bundle:
@glion/mllp-client → @glion/ack → @glion/builder
→ @glion/config (loadConfig)
→ cosmiconfig
→ fs, path, os, crypto, ...
This broke runtime portability: Workers and Deno bundles dragged in Node
builtins they couldn't resolve, even though the harness/runtime never invoked
config-loading code. The architectural fix is to remove disk-based config
discovery from leaf factory functions; sunsetting `legacy` mode is the
cleanest way to do that, since it was the only thing the lookup gated.
Migration
=========
Consumers branching on placeholder leaves of empty fields:
-if (field.children[0]?.children[0]?.children[0]?.value === "") { ... }
+if (field.children.length === 0) { ... }
Config files carrying `experimental.emptyMode` are now rejected by the
@glion/config schema validator. Remove that block from .hl7v2rc.* files.
@glion/util-query's `value()` helper already returns `null` for empty
children — most consumers using it need no change.
What changed
============
- @glion/builder: dropped @glion/config dependency entirely; f()/r()/c() now
always emit `children: []` for empty inputs.
- @glion/parser: dropped emptyMode plumbing from parser.ts/processor.ts/
types.ts. parseHL7v2's settings argument no longer accepts
experimental.emptyMode.
- @glion/config: removed ExperimentalSchema from the settings schema. The
`loadConfig`/`loadConfigAsync` API is unchanged but no longer exposes any
experimental keys.
- @glion/util-visit: deleted the test-helpers + legacy/empty fixture configs;
the visit() implementation was already mode-agnostic.
- @glion/jsonify: updated the runtime serializer to materialize empty fields/
repetitions as "" (preserving the existing JSON output shape — empty
children now flow through here, where previously legacy always produced a
full skeleton with "" leaves).
Side effects
============
- @glion/builder/dist/index.js no longer contains `import "@glion/config"`;
consumers' bundles shrink (cosmiconfig + dependencies dropped).
- The Workers and Deno runtime adapters of @glion/mllp-client (PR #615,
#616) bundle cleanly without `nodejs_compat` polyfills; the cosmiconfig
bundle leak is resolved.
Tests
=====
All affected packages green:
- @glion/builder: 25 tests
- @glion/parser: 56 tests (parser.legacy.test.ts + processor.legacy.test.ts
deleted; their behaviour is now the default)
- @glion/config: 34 tests
- @glion/util-query: 224 tests
- @glion/util-visit: 31 tests (visit.legacy.test.ts deleted; redundant)
- @glion/jsonify: 12 tests
- @glion/hl7v2: 9 tests
The pre-existing @glion/cli config-discover.test.ts flake (.js vs .ts) is
unrelated and was already failing on main before this PR.
https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
After the legacy emptyMode sunset, the processor no longer reads anything from ParserContext — the tokenizer has already consumed ctx.delimiters and ctx.input to produce the token stream. The processor is pure tokens → AST. Drops ctx from createParserCore and parseHL7v2FromIterator; updates the single call site in parser.ts and the test fixtures in processor.test.ts. Removes the `void ctx` workaround that was suppressing the unused-parameter warning instead of fixing the underlying dead code. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
meleksomai
force-pushed
the
claude/mllp-client-workers-runtime
branch
from
667cdb0 to
c476cba
Compare
meleksomai
pushed a commit
that referenced
this pull request
Apr 30, 2026
…text downgrade Per Copilot review on PR #616: the previous behaviour mapped `tls.insecure: true` to `secureTransport: "off"`, which silently downgraded an explicitly TLS-typed configuration to plain TCP on Workers — a runtime-dependent security regression vs. the Node adapter, where `insecure: true` keeps TLS on and only disables certificate verification. Workers does not expose a way to disable certificate verification independently of `secureTransport`, so the only honest options are (a) silently ignore `insecure` (still encrypted, intent partially honoured) or (b) reject with INVALID_INPUT. We pick (b) because it matches how we already handle ca/cert/key/passphrase: Workers does not support programmatic TLS configuration; operators should use the platform's TLS facilities instead. Caller migration: - Want plain TCP? Omit `tls` entirely. - Want TLS with verification? `tls: {}` (or with `servername`). - Want TLS without verification? Not supported on Workers — use Cloudflare's platform TLS configuration. Changes: - workers.ts: rejectUnsupportedTlsMaterial now also rejects `insecure: true`. The `secureTransport` calculation simplifies to `params.tls ? "on" : "off"` because the insecure-true branch is now rejected before reaching it. - JSDoc on workersConnect updated to spell out the trade-off. - Changeset updated: drop the "insecure → plain TCP" claim, add the new rejection to the INVALID_INPUT list. - New test in adapter.test.ts covering the rejection (61 tests total in the workerd project, was 60). https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
The repo is pre-1.0.0; per semver convention breaking changes in 0.x.x are expressed as minor bumps, not major. Adjusts the sunset changeset for builder, parser, config, util-visit, hl7v2 → minor. The "BREAKING" copy in the changeset body is unchanged — consumers still need to read the migration notes; only the npm version bump shape changes. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
`.wrangler/state/...` is regenerated on every `pnpm test:cf` run by `wrangler.unstable_dev`'s miniflare cache. It accidentally got committed in the previous commit; removing from git and ignoring going forward. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
Adds @glion/mllp-client/workers backed by the cloudflare:sockets connect() API. The user-facing API matches the Node adapter exactly — only the import path changes. Stacked on PR #609 (mllp-client foundation + Node adapter). Will rebase onto main once #609 merges. Adapter-specific behaviour: - TLS opt-in: secureTransport "on" by default when tls is set, "off" when tls.insecure: true. - tls.ca | cert | key rejected with INVALID_INPUT (Workers does not accept programmatic TLS material — use the platform's TLS config). - tls.passphrase rejected with INVALID_INPUT (encrypted keys are not configurable from the runtime). - Caller-supplied AbortSignal honoured at connect-phase. A signal fired while socket.opened is pending closes the socket. A pre-aborted signal alongside an opened-rejection maps to TIMEOUT (not CONNECTION_REFUSED) so callers can distinguish deadline- during-handshake from a real transport failure. - socket.close() is async; MllpDuplexStream.close() is sync by contract. The adapter schedules the close and silently swallows close-time rejections — non-actionable post-request, prevents unhandled-rejection logs in workerd. Tests: test/workers.test.ts mocks cloudflare:sockets via vi.mock() so the adapter wiring is exercised in plain Node vitest. 8 tests cover TCP/TLS connect with parameter forwarding (secureTransport on/off), all three INVALID_INPUT rejections, opened failure -> CONNECTION_REFUSED, socket.close after exchange, and the abort-with-opened-rejection -> TIMEOUT path. Total: 54 -> 62 tests on this branch (the +8 are workers-only). Lint, typecheck, build all clean. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
Drops `vi.mock("cloudflare:sockets")` in favour of running the
adapter inside an actual workerd instance via
`@cloudflare/vitest-pool-workers`. Mocks verify wiring (we passed
the right args to a fake), runtime tests verify behaviour (the
adapter actually completes a round-trip in workerd). The latter is
what we want.
Structure:
- vitest.config.ts now defines two projects via test.projects:
"hl7v2-mllp-client (node)" runs core.test.ts and node.test.ts in
plain Node; "hl7v2-mllp-client (workers)" runs
test/workers/adapter.test.ts inside workerd via the cloudflareTest
Vite plugin.
- test/workers/wrangler.toml gives the pool a valid worker config to
boot. test/workers/worker.ts is the no-op fetch entry workerd
requires; the actual unit tests run in the same isolate but
exercise the adapter directly.
- test/workers/global-setup.ts spins a Node-side TCP ack server on
127.0.0.1:47575 that the worker connects to. The server replies to
any frame with a fixed wire-format AA. Test cases that need
different responses (NAK, malformed) are already covered in
core.test.ts which is runtime-free.
Test surface change:
- Removed: secureTransport "on"/"off" wiring assertions, opened-
failure -> CONNECTION_REFUSED (covered now by happy-path success +
port-1-refused), socket.close() runs after exchange (covered by
happy-path success), abort+opened-rejection -> TIMEOUT.
- Kept (and now runtime-validated): happy-path round-trip,
CONNECTION_REFUSED on closed port, INVALID_INPUT for
tls.ca/cert/key/passphrase.
The lost mock-only tests were testing args passed to a fake. The new
happy-path test is proof the runtime honours those args end-to-end,
which is what actually matters.
Devdep: @cloudflare/vitest-pool-workers ^0.15.1.
CI note: the workers project requires a workerd-compatible host. It
runs cleanly on standard Linux runners; sandboxed dev environments
that block kernel features workerd needs (user namespaces, etc.)
will see workerd segfault on startup. The Node project is unaffected
and runs everywhere.
https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
Adds explicit pnpm scripts for each runtime project so the test matrix is discoverable from package.json: - test:node — runs the Node adapter + runtime-free core tests - test:cf — runs the workerd-based tests - test — unchanged, runs both projects (plain `vitest run`) test:deno and test:bun are not added on this PR. The Deno PR (#615) will add test:deno once its tests are converted from mocked globalThis.Deno to running inside actual Deno (mirroring what this PR did for Workers). Bun support is currently exercised through test:node because the Node adapter is the codepath Bun uses. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
Adds `test:bun` and registers `@cloudflare/vitest-pool-workers` via
the workspace catalog so version drift between packages can't happen
silently.
test:bun
- Runs `bun --bun test test/core.test.ts` — Bun's native test runner
against the runtime-free core. This validates Web Streams,
AbortSignal, and the exception flow under Bun's runtime, which is
the surface that's genuinely Bun-specific (the Node adapter itself
uses node:net which Bun shims).
- Failing if Bun isn't installed is intentional — Bun is a contributor
expectation per discussion.
- Why bun test rather than vitest under Bun: Vite bundles
vitest.config.ts via esbuild, pulling in the cloudflare pool's
transitive zod/wrangler chain, which trips Bun's CJS-as-ESM interop
(TypeError: undefined is not an object evaluating 'z.object'). A
separate vitest.bun.config.ts that avoids the cloudflare import
hits the same wall via @glion/parser -> @glion/config (zod again).
bun test side-steps the bundler entirely.
- node.test.ts under Bun is future work. The integration suite calls
serve(app, { port: 0 }) from @glion/mllp/node which fails to bind
on Bun's default listen address. Fixing that is out of scope for
this PR.
test aggregator now chains both:
test = vitest run (node + workers projects) && pnpm test:bun
Catalog
- pnpm-workspace.yaml gains a `catalog:` block listing
@cloudflare/vitest-pool-workers ^0.15.1.
- mllp-client's devDependency reference is "catalog:" instead of an
exact version.
https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
Reverts the catalog: block from pnpm-workspace.yaml and restores @cloudflare/vitest-pool-workers to its hard-pinned ^0.15.1 in mllp-client/package.json. The catalog protocol isn't yet adopted in this repo. Whether to migrate to it (across vitest, @vitest/coverage-v8, and shared devdeps in general) will be tracked separately so the decision can be made independently of any single PR. The test:bun script and the workerd-based test setup are unaffected by this revert. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
@cloudflare/vitest-pool-workers cannot load @vitest/coverage-v8 because the coverage instrumentation imports node:inspector/promises, which workerd does not ship. This is documented at https://developers.cloudflare.com/workers/testing/vitest-integration/known-issues/#module-resolution CI's `pnpm test:coverage` step was failing because it ran coverage across both projects (Node + Workers) and the workers pool errored out on the missing module. Coverage of the workerd-side integration tests would not be useful anyway — they exercise the runtime, not source under test. The scope: --project 'hl7v2-mllp-client (node)' restricts coverage to the Node project where it is meaningful (96.42% statements, 88.37% branches against the Node adapter + runtime-free core). https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
CI's pnpm test:coverage runs `turbo run test -- --coverage`, which appends --coverage to each package's test script. Our previous test script was a chained form: test = vitest run && pnpm test:bun The shell parsed `vitest run && pnpm test:bun --coverage` as two commands, so --coverage only reached test:bun. More importantly, plain `vitest run` (no project filter) ran BOTH the Node and Workers projects, and the workerd binary crashes (signal 11 segfault) on standard GitHub Actions Linux runners due to missing kernel features (user namespaces, certain seccomp syscalls) — same issue as in many sandboxed environments. Fix: scope `test` to the Node project (single command, absorbs --coverage cleanly). Bun and CF stay as opt-in scripts: test → Node project only (CI-compatible) test:node → explicit alias of test test:bun → opt-in, requires Bun test:cf → opt-in, requires workerd-capable host test:coverage → already Node-scoped, unchanged Contributors who want to validate every runtime locally run all four scripts; CI runs only `test` (effectively `test:node`). The Workers and Bun runtimes still have dedicated test surfaces; they just don't crash CI on hosts that can't run their binaries. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
…ble_dev pattern Replaces @cloudflare/vitest-pool-workers with the same testing pattern Hono uses (`runtime-tests/workerd/` in honojs/hono): a small harness Worker exposes the adapter over HTTP, Node-side vitest spawns it via wrangler.unstable_dev, and tests assert on the HTTP response. Why: - vitest-pool-workers' coverage instrumentation depends on node:inspector/promises which workerd doesn't ship; the workers project couldn't run under pnpm test:coverage. - The pool also failed to boot reliably across CI runners and inside containerised environments, leaving us unable to verify Workers behaviour in CI. - Hono's pattern works on standard ubuntu-latest GH Actions runners and is the de-facto standard for multi-runtime libraries. Test surface (6 tests): - Happy-path round-trip: harness connects via cloudflare:sockets, writes the MLLP frame, parses the AA from the Node-side ack server, returns it over HTTP. - CONNECTION_REFUSED when the TCP target is closed. - INVALID_INPUT for tls.ca, tls.cert, tls.key, tls.passphrase. Files: - test/workers/harness.ts (new) — harness Worker exposing POST /send - test/workers/adapter.test.ts (rewritten) — Node-side tests via wrangler.unstable_dev - test/workers/wrangler.toml — points at harness.ts; compat date bumped to 2025-09-23 - test/workers/global-setup.ts — unchanged (TCP ack server) - vitest.config.ts — drops the cloudflareTest plugin; both projects are now plain vitest. Bun can load the config cleanly so vitest.bun.config.ts is no longer needed. - vitest.bun.config.ts — deleted - package.json — drops @cloudflare/vitest-pool-workers, adds wrangler Generic CI scaffolding (so future packages get this for free): - turbo.json declares test:bun, test:cf, test:deno tasks - Workspace package.json adds test:bun / test:cf / test:deno scripts that delegate to turbo - .github/workflows/ci.yml: e2e-bun renamed to testing-bun and now runs `pnpm test:bun` (turbo dispatches across all opted-in packages); new testing-cf job runs `pnpm test:cf` with NODE_OPTIONS=--max_old_space_size=8192 - @glion/cli gets a test:bun script wrapping its existing e2e command, so the renamed testing-bun job continues to cover it Adding multi-runtime tests to a new package is now: define test:bun / test:cf / test:deno in that package.json. CI picks it up automatically; no workflow edits. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
`wrangler.unstable_dev` (used by the workers test harness) drops a `.wrangler/state` directory during boot. It's per-machine runtime state and shouldn't be committed. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
Code review pass focused on elegance, simplicity, and CLAUDE.md design philosophy. No behaviour changes; 54 Node tests + 60 workerd tests still green. harness.ts - Replace string-name class detection (`e.constructor.name === "X"`) with `instanceof` checks against MllpClientError + AckException. The string form was fragile under any name-mangling minifier. - Drop the redundant `className` field — it duplicates `kind` for typed errors and was never read by the test assertions. - Trim the JSDoc header: drop the abandoned-vitest-pool-workers history paragraph (CLAUDE.md §10 — that belongs in the PR description, not in code that lives forever). - Add `message: "invalid JSON body"` to the BadRequest response so malformed-payload failures are debuggable. - Extract `DEFAULT_TIMEOUT_MS` instead of inline 5000. workers.ts - Drop the `noop()` function — replace with `null` + optional invocation (`dispose?.()`). - Drop `pickError(...candidates)` — used once, inline the two-arg precedence directly into `toAbortError` as a small if/else. - Drop `closeWorkerSocketIgnoringErrors` — collapse the two fire-and-forget `socket.close()` paths (abort handler + duplex close) into the same inline `.catch(() => undefined)` pattern, with one short comment explaining the contract. - Trim the duplicated comment block in `MllpDuplexStream.close` (was two paragraphs saying the same thing). adapter.test.ts - Import `TEST_PORT` from `global-setup` instead of redeclaring it. The two values can no longer drift. Net: -56 lines. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
…text downgrade Per Copilot review on PR #616: the previous behaviour mapped `tls.insecure: true` to `secureTransport: "off"`, which silently downgraded an explicitly TLS-typed configuration to plain TCP on Workers — a runtime-dependent security regression vs. the Node adapter, where `insecure: true` keeps TLS on and only disables certificate verification. Workers does not expose a way to disable certificate verification independently of `secureTransport`, so the only honest options are (a) silently ignore `insecure` (still encrypted, intent partially honoured) or (b) reject with INVALID_INPUT. We pick (b) because it matches how we already handle ca/cert/key/passphrase: Workers does not support programmatic TLS configuration; operators should use the platform's TLS facilities instead. Caller migration: - Want plain TCP? Omit `tls` entirely. - Want TLS with verification? `tls: {}` (or with `servername`). - Want TLS without verification? Not supported on Workers — use Cloudflare's platform TLS configuration. Changes: - workers.ts: rejectUnsupportedTlsMaterial now also rejects `insecure: true`. The `secureTransport` calculation simplifies to `params.tls ? "on" : "off"` because the insecure-true branch is now rejected before reaching it. - JSDoc on workersConnect updated to spell out the trade-off. - Changeset updated: drop the "insecure → plain TCP" claim, add the new rejection to the INVALID_INPUT list. - New test in adapter.test.ts covering the rejection (61 tests total in the workerd project, was 60). https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
Two breaking changes to MllpClientOptions.tls, both grounded in the
fact that HL7v2 messages commonly carry PHI and "secure by default"
is the right stance for a healthcare client.
(1) Reshape: `tls?: boolean | MllpClientTlsOptions`
============================================================
Previously: `tls?: MllpClientTlsOptions` — the implicit "object
provided → TLS on" rule. This was hiding the wire-protocol intent
behind structural type duck-typing and was the root cause of
Copilot's review finding (a `tls.insecure: true` config that
silently downgraded to plain TCP on Workers, because the rule "tls
is on iff tls is set" was applied independently per-runtime with
no shared activator).
Now: `tls: true` is the explicit "use TLS with defaults" form,
matching the mainstream pattern (MongoDB, pg). The object form
still works for custom config. The core normalises `tls: true`
to `tls: {}` before reaching the adapter, so `MllpConnectParams.tls`
stays `MllpClientTlsOptions | undefined` and adapters see only
the object shape.
(2) Default: TLS-on
============================================================
Previously: `tls` undefined → plain TCP.
Now: `tls` undefined → TLS on (same as `tls: true`). Plain TCP
requires explicit `tls: false`.
Migration: every call site that relied on "no tls field → plain
TCP" must now pass `tls: false`. Call sites that already passed
a tls object are unchanged.
Implementation
============================================================
- `MllpClientOptions.tls`: type widened to `boolean | TlsOptions`
with `@default true`. JSDoc explains the four shapes (default,
true, object, false) and when to use each.
- `MllpClient` constructor: normalises the union to
`TlsOptions | undefined` for adapters. `false` → undefined;
`true | undefined` → {} (TLS-on, no config); object passes
through.
- `validateOptions`: accepts `boolean | object | undefined`; rejects
anything else with INVALID_INPUT.
- Workers adapter: removes the now-redundant `params.tls.insecure`
check from the secureTransport calculation (the core normalises
before reaching here; `params.tls` is truthy iff TLS should
negotiate). `rejectUnsupportedTlsMaterial` still rejects
ca/cert/key/passphrase/insecure as before — Workers can't honour
them.
- Node adapter: no logic change; still receives `params.tls` as
`TlsOptions | undefined`.
Test surface
============================================================
- All Node + core tests that assumed plain-TCP-by-default now pass
`tls: false` explicitly. ~50 mechanical edits via awk script.
- New core construction tests verify the default-true behaviour:
one asserts `received.tls` is set when no tls field is passed;
another asserts it's undefined when `tls: false` is passed.
- Workers happy-path and CONNECTION_REFUSED tests pass `tls: false`
to connect to the plain-TCP ack server.
- Workers harness `SendRequest.tls` widened to accept boolean.
Total: 56 Node + 63 workerd = 119 tests, all green. Type-check
clean, build clean.
Changeset bumps `@glion/mllp-client` from minor to major.
https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
The repo is pre-1.0.0; per semver convention breaking changes in 0.x.x are expressed as minor bumps, not major. Adjusts both changesets: - sunset-legacy-empty-mode.md: builder, parser, config, util-visit, hl7v2 → minor - mllp-client-workers-runtime.md: mllp-client → minor The "BREAKING CHANGE" copy in each changeset body is unchanged — consumers still need to read the migration notes; only the npm version bump shape changes. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
meleksomai
force-pushed
the
claude/mllp-client-workers-runtime
branch
from
b8cdd43 to
1d6e611
Compare
The "skips a filename that exists but is not readable" test in test/config/discover.test.ts revokes read permission on a config file (chmod 0o000) and expects discoverConfig to skip it via access(R_OK). This works for ordinary users but not for root — CAP_DAC_OVERRIDE bypasses file mode bits, so the test sees the file as readable and fails with "expected .js, received .ts". The test was passing on developer workstations (non-root) and failing in CI containers / devcontainers (root by default), making it look like a flake. Use vitest's `it.skipIf(isRoot)` to skip the case under root, with a comment explaining the constraint. The behaviour under test (graceful handling of unreadable files) is still exercised on ordinary CI runs and dev workstations; root-only environments lose this single test branch but the discovery code path is otherwise covered. https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg
This reverts commit 6d1d040.
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
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.
Summary
Adds the Cloudflare Workers runtime adapter to
@glion/mllp-client, backed by thecloudflare:socketsconnect()API. Tests run inside a realworkerdinstance via@cloudflare/vitest-pool-workers(no mocking).The user-facing API (
new MllpClient({ host, port }).send(message),client.stream(...),MllpClientError,AckException) is identical to the Node adapter — only the import path changes.Adapter-specific behaviour
tls. Whentlsis set, the adapter requestssecureTransport: "on". Whentls.insecure: true, it requestssecureTransport: "off"so the connection runs as plain TCP and the caller takes the security trade-off explicitly.tls.ca | cert | keyrejected withINVALID_INPUT. Workers does not accept programmatic TLS material — use the platform's TLS configuration (Hyperdrive, Worker bindings, or the destination service's certificate chain) instead.tls.passphraserejected withINVALID_INPUT. Workers does not accept inline passphrases — encrypted private keys are not configurable from the runtime.AbortSignalhonoured at connect-phase. A signal that fires whilesocket.openedis pending closes the socket. A pre-aborted signal alongside anopenedrejection routes toTIMEOUT(notCONNECTION_REFUSED) so callers can distinguish deadline-during-handshake from a real transport failure.socket.close()is async butMllpDuplexStream.close()is sync by contract. The adapter schedules the close and silently swallows close-time rejections — the request lifecycle has already ended at that point and a close-time error is non-actionable. This prevents the Workers runtime from logging it as an unhandled rejection.Test approach: real
workerd, not mocksThe previous version of this PR mocked
cloudflare:socketsviavi.mock(). That verifies wiring ("we passedsecureTransport: 'on'to the fakeconnect()") but not behaviour — the adapter could pass right args to a fake while subtly breaking against realworkerd.This revision drops the mock and runs the worker tests inside a real
workerdinstance via@cloudflare/vitest-pool-workers. The package'svitest.config.tsdefines two projects:hl7v2-mllp-client (node)— runscore.test.tsandnode.test.tsin plain Node (existing tests, unchanged).hl7v2-mllp-client (workers)— runstest/workers/adapter.test.tsinsideworkerd, against the actualcloudflare:socketsAPI. A Node-sideglobalSetupspins up a TCP "ack server" on127.0.0.1:47575that the worker connects to.What's covered
cloudflare:sockets, writes an MLLP frame, receivesAAfrom the Node-side ack server, parses it.tls.ca | cert | key→INVALID_INPUT(no socket required).tls.passphrase→INVALID_INPUT(no socket required).What was dropped vs. the mock-based version
on/offwiring" — was verifying args passed to a fake. The happy-path test is proof those args are honored end-to-end.socket.openedfailure →CONNECTION_REFUSED" — the closed-port test exercises the same path with real workerd.socket.close()runs after exchange" — the happy path completes successfully, which requires the close to work.TIMEOUT" — the timing-sensitive race the mock simulated isn't directly observable in the same shape against real workerd. The core test incore.test.tscovers the abort-mapping logic; the wiring is identical between adapters.NAK paths, malformed-frame handling, and other wire-protocol edge cases are already covered by the runtime-free
core.test.tsand don't need re-running per runtime.Files of interest
packages/mllp-client/src/runtimes/workers.tsworkersConnect+ theMllpClientsubclass withworkersConnectpre-boundpackages/mllp-client/test/workers/adapter.test.tspackages/mllp-client/test/workers/global-setup.ts127.0.0.1:47575packages/mllp-client/test/workers/wrangler.tomlworkerdpackages/mllp-client/test/workers/worker.tspackages/mllp-client/vitest.config.tspackages/mllp-client/package.json@cloudflare/vitest-pool-workersdevdep +workerdconditional exportEnvironment requirements
The Workers project requires a
workerd-compatible host. It runs cleanly on standard Linux CI runners. Sandboxed dev environments that block the kernel features workerd needs (user namespaces, certain syscalls) may seeworkerdsegfault on startup. The Node project is unaffected and runs everywhere.Test plan
pnpm build,pnpm check-types,pnpm test(both projects),pnpm dlx ultracite checkall passworkerdbinary (the pool downloads@cloudflare/workerd-linux-64automatically on install)await client.send(message)resolves withAA.AbortSignalcancels mid-exchange.https://claude.ai/code/session_01MvBEUcGkRokNw2GWYVHADg