From db05f0ffc7c118a95d6925bb809555aca5c8c635 Mon Sep 17 00:00:00 2001 From: Michael Hobbs Date: Wed, 8 Jul 2026 15:08:38 -0400 Subject: [PATCH 1/6] docs: add CLAUDE.md (engine AI-contributor guardrails) + CLAUDE_USER_EXAMPLE.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLAUDE.md: strict guidance for AI coding assistants working on the engine itself, designed to protect the project from low-quality sprawling AI changes ("vibeslop"). - Push-back-first: for non-trivial work, state the change + any concrete disagreement + a better alternative BEFORE editing (calibrated to blast radius, so no theater). - Anti-sprawl bounded in breadth AND depth (<=3 files; ~40-50 line / whole-method cap). - Preserve behavior/compat incl. the subtle breaks (charset, tz/locale, HL7 delimiters); golden-output test for donkey/serialization changes; red-green regression tests. - Healthcare security: no PHI logging; no unsafe deserialization/XXE (documented RCE class). - Issue-first-and-wait; MPL header copied from server/license-header.txt; no slop tells. - Grounded in the repo (Java 17 + Ant server/mirth-build.xml, module map, CONTRIBUTING). - No orchestration skill by design. Advisory; notes which gates belong in CI. CLAUDE_USER_EXAMPLE.md: a copy-me starter CLAUDE.md for a user's own channel/template repo (JS on the Rhino runtime) — Rhino/ES5 constraints, loop-scoping bug, Java interop, engine globals, code patterns; points at oie-examples and the docs site. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 109 +++++++++++++++++++ CLAUDE_USER_EXAMPLE.md | 239 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 348 insertions(+) create mode 100644 CLAUDE.md create mode 100644 CLAUDE_USER_EXAMPLE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..c3181297c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,109 @@ +# CLAUDE.md — Open Integration Engine (engine) + +Guidance for Claude Code (and any AI coding assistant) working on **this repository — the OIE engine +itself**. OIE is an open-source fork of Mirth Connect: a **production-critical, healthcare** integration +engine (HL7 / FHIR / DICOM) that runs in hospitals and labs, on a mature codebase with ~15 years of +history. Licensed **MPL-2.0**. + +> **Writing channel/template JavaScript that runs *on* the engine (transformers, filters, code +> templates)?** That's a different context with its own Rhino/ES5 rules — start from +> **[`CLAUDE_USER_EXAMPLE.md`](./CLAUDE_USER_EXAMPLE.md)**, not this file. + +## Prime directive: be a careful guest, not a rewriter +Optimize for the reviewer's trust and the project's long-term health — **never for volume of change.** A +tiny, correct, well-explained fix tied to an issue is a *success*; a large, plausible-looking, unrequested +change is a *cost* the maintainers must now audit in a healthcare setting. The failure mode this file +prevents: confident, sprawling, style-driven AI changes ("vibeslop") that look fine, pass a glance, and +quietly break behavior, compatibility, or trust. When a change can't be kept small, that's a signal to +**stop and discuss on the issue** — not to proceed. (There is deliberately no orchestration/automation skill +here: the project does not want large multi-file, single-PR agent changes.) + +**Green light:** if the change is issue-scoped, small, and test-backed — the thing you were asked to do — +just do it well. You don't need to re-ask permission for what's already agreed on the issue. Escalate only +on the triggers below. + +## Before you edit: state your read (and push back) +The most valuable thing you can do here is **refuse to dutifully implement a weak fix.** The moment someone +thinks *"I know how to fix this"* is exactly when a cleaner, safer, more idiomatic solution gets missed. + +- **For anything beyond a trivial one-liner, before editing code, first state:** (a) the change you'd make, + (b) whether you disagree with the requested approach and the **concrete** cost if you do — a *specific* + broken caller, a named compat/security surface, a symptom-vs-root-cause miss (never a hypothetical), and + (c) any better alternative with tradeoffs. Don't open code edits in the same turn you receive a non-trivial task. +- **Calibrate to blast radius.** A typo or one-line fix needs no options memo — make it well and say nothing. + Pushback theater (manufacturing objections on trivial work) wastes the reviewer trust this protects. If you + have no concrete objection, make the small change and move on. +- You are **advising skeptical maintainers, not obeying** them. They make the final call — take disagreement + to the issue — but silent compliance with a mediocre or risky fix is a failure. Don't flatter, don't pad. + +## Hard rules (non-negotiable) +1. **Issue first — and wait.** Per [`CONTRIBUTING.md`](./CONTRIBUTING.md), open/claim an issue *and get a + maintainer's triage or reply* before writing non-trivial code. Opening an issue then immediately coding + the whole change defeats the point. A tiny obvious fix: say so on the issue and keep the diff tiny. No PR + without a referenced issue. +2. **One concern; minimal diff; bounded in breadth *and* depth.** No drive-by edits (reformatting, + re-indenting, import reordering, renames in code you aren't otherwise changing). **Breadth:** touch **≤3 + files**; more → stop and discuss. **Depth:** if the fix means rewriting a whole method/class, or a single + file's diff exceeds ~40–50 changed lines, that's sprawl too — stop and discuss. A big single-file rewrite + is still slop. +3. **Match the surrounding code.** There's no autoformatter to hide behind — mirror the style/naming/idioms + of the file. Introduce no new patterns, libraries, frameworks, build tools, or dependencies without + explicit maintainer approval on the issue. +4. **Preserve behavior & compatibility.** Don't change public APIs, serialization formats, DB + schema/migrations, wire/protocol/HL7 behavior, extension-point signatures, or config compatibility without + maintainer sign-off — existing deployments upgrade in place. **Watch the subtle breaks that don't look like + API changes:** default charset, timezone/locale handling, null-vs-empty, HL7 delimiter/escape/segment + order, connection-pool defaults, XStream alias maps. For any change under `donkey/` or in a + serialize/parse path, include a **before/after golden-output test** proving behavior is unchanged (or the + change is the point and is signed off). +5. **Tests are load-bearing and must bite.** Add/adjust tests for any behavior change; a regression test must + **fail against the pre-fix code and pass after** — state that you verified this (line coverage is not + proof). **Never delete, skip, or weaken a test to go green.** The build (below) must pass before a PR. +6. **Security — this engine carries PHI.** + - **Never log message content, payloads, headers, or anything that could be PHI**; add no new logging of + message bodies or connector I/O; serialize no new fields into audit/event tables without sign-off. + - **No reflective/polymorphic deserialization** (XStream, Java `ObjectInputStream`) without an allowlist; + **XML parsers must disable DTDs/external entities (XXE).** This engine has a documented RCE history in + exactly these paths — don't reopen it. + - Be careful with SQL, crypto, auth, and file/network I/O. Suspected vulnerabilities go through + [`SECURITY.md`](./SECURITY.md), not a silent patch-and-PR. +7. **License headers:** copy the existing header **verbatim** from [`server/license-header.txt`](./server/license-header.txt) + onto any new file (it reads "Copyright (c) Mirth Corporation …" — do **not** invent your own MPL text). + Contributions are licensed MPL-2.0. +8. **Don't invent.** Verify classes, methods, and config keys against the actual code before using them; if + unsure whether something exists or is safe, say so — don't guess. +9. **No slop tells.** No emoji; no "as an AI"; no comments that restate the code or narrate the change; no + speculative abstractions or "future-proofing" (YAGNI); no dead code; no reflowing of existing comments. + +## Build & test (Java 17 + Ant) +- Toolchain is pinned in [`.sdkmanrc`](./.sdkmanrc) — install [SDKMAN](https://sdkman.io/) and run + `sdk env install` in the repo root (the JavaFX-bundled JDK is required; the `client` GUI imports JavaFX). +- **Build + test** (what CI runs off `main`): `cd server && ant -f mirth-build.xml -DdisableSigning=true -Dcoverage=true` + (the signed build drops the flags and runs on `main`). JUnit results land under + `*/build/test-results/**/*.xml`. **A red build is not reviewable — never open a PR on one.** + +## Module map (change the narrowest one that solves the issue) +| Module | What it is | Care level | +|---|---|---| +| `donkey/` | Core message-processing engine (queues, channels, message lifecycle). | **Highest** — the heart; small, golden-tested changes only. | +| `server/` | Server runtime, connectors, REST APIs, plugins, DAO/DB. | High — public API & compatibility surface. | +| `client/` | Swing **Administrator** GUI (pulls in JavaFX). | Medium. | +| `command/` | Mirth CLI client. | Medium. | +| `generator/` | Code generation used by the build. | Medium — build-affecting. | +| `tools/` | Assets/support files. | Low. | + +## Contribution flow +Follow [`CONTRIBUTING.md`](./CONTRIBUTING.md) (issue → fork → `feature/` branch → **draft** PR to +`main`, marked "Ready for review" only when truly done). Beyond what it documents: reviewers are +`@openintegrationengine/maintainers` (see `.github/CODEOWNERS`); project governance lives in the +`OpenIntegrationEngine/governance` repo. + +## Enforcement note +This file is **advisory** context, not an enforced gate. The load-bearing rules should be machine-checked in +CI / the PR template rather than left to good faith — a linked-issue requirement, a license-header check, a +"no new dependency / build-file change without a label" diff check, rejecting whitespace-only or +import-order-only hunks, and an emoji/AI-tell grep. If those checks aren't wired yet, adding them is a good +first contribution. + +> **Maintaining this file:** keep it lean — if a rule doesn't change agent behavior on most tasks, cut it; +> push sometimes-needed detail to a linked file. It works only if contributors actually read it. diff --git a/CLAUDE_USER_EXAMPLE.md b/CLAUDE_USER_EXAMPLE.md new file mode 100644 index 000000000..1b99a4637 --- /dev/null +++ b/CLAUDE_USER_EXAMPLE.md @@ -0,0 +1,239 @@ +# CLAUDE_USER_EXAMPLE.md — starter for your OIE channel & template code + +**This is a template, not a rule for this repo.** It is a starter `CLAUDE.md` for a *separate* repository — +**yours** — that holds the JavaScript you deploy into **Open Integration Engine** channels: source/ +destination transformers, filters, response transformers, attachment handlers, and Global / Code Template +scripts. Copy it to *your* repo's root as `CLAUDE.md` and fill in the `TODO` sections. + +> Working on the **OIE engine itself** (the Java in this repository)? That's a different job — see this +> repo's [`CLAUDE.md`](./CLAUDE.md), not this file. + +OIE executes channel/template JavaScript on the **Rhino** engine (bundled with the Java 17 runtime), so the +constraints below are about Rhino's partial ES6 support and Java interop — not about Node.js. Real, working +examples of channels and code templates live in +**[`OpenIntegrationEngine/oie-examples`](https://github.com/OpenIntegrationEngine/oie-examples)**; project +docs are at **[openintegrationengine.org](https://openintegrationengine.org)** and the community is on +**[Discord](https://discord.gg/azdehW2Zrx)**. + +Everything from **"Technical Constraints"** down is generic to OIE's Rhino runtime and should apply +unchanged; the `TODO` sections above it are where your project's specifics go. + +--- + +## Project Overview +> **TODO:** One paragraph — what your integration does, which upstream/downstream systems it connects, and +> the primary message types (HL7 v2, FHIR, X12, DICOM, custom JSON, …). + +## Commands +> **TODO:** Your project's scripts. A channel/template repo commonly has lint + a way to export/import +> channels; fill in what you actually use. +```bash +npm run eslint # lint the deployed JS +npm test # unit tests over pure logic (with mocked OIE globals) +# ...your channel export/import / deploy commands +``` + +## Architecture +> **TODO:** Your message flow and key channels/code-template libraries. A Mermaid diagram helps (see +> "Documentation standards"). + +## Source structure +> **TODO:** Outline your repo layout so the assistant knows where deployed code vs. build tooling lives. + +--- + +## Technical Constraints + +**OIE JavaScript engine.** OIE runs channel/template JavaScript on **Rhino**, which has only **partial ES6 +support**. The rules in this section apply to every source file deployed into a channel (transformers, +filters, response transformers, attachment handlers, and Global/Code Template scripts). + +> Files that run in a normal Node.js context (build tooling, tests, non-OIE utilities) are **not** subject +> to these rules — scope this section to the directory that gets deployed to OIE (commonly `src/`). + +### Supported ES6 features (safe to use) +- `const` and `let` — prefer over `var`. `const` by default, `let` when reassignment is needed (but see the + Rhino loop-scoping bug below). +- Arrow functions `() => {}` — fine in callbacks, `.map()`, `.filter()`, etc. +- Object/array destructuring — `const { a, b } = options` +- `Object.keys()`, `Object.values()`, `Object.entries()`, `Object.assign()` +- Array methods: `.map()`, `.filter()`, `.reduce()`, `.forEach()`, `.find()`, `.some()`, `.every()` + +### Prohibited ES6+ features (will break at runtime in OIE) +- **Template literals** — NEVER use backtick strings. `` `Hello ${name}` `` fails at runtime. Use string + concatenation or `Array.join()` (see "String building"). +- **Optional chaining `?.`** — not supported; use a try/catch helper (see "Safe property access"). +- **Nullish coalescing `??`** — use `||` or an explicit ternary. +- **`async`/`await`** — not supported. Transformers are synchronous; use callbacks/retries. +- **`Promise`** — not available in the runtime. +- **ES6 classes (`class`/`extends`)** — use constructor functions with `.prototype` methods. +- **ES6 modules (`import`/`export`)** — share code via Code Templates plus `/* global */` and + `/* exported */` comments (see "Module/export pattern"). +- **Spread syntax `...args`** — minimal/unreliable support; avoid, especially in parameter lists. +- **`for...of` loops** — use `.forEach()` or a traditional indexed `for` loop. +- **Default parameters** — use `param = param || defaultValue` instead. + +> **Version-dependent — verify on your server.** `Symbol`, `Map`, and `Set` exist on current OIE/Rhino +> builds but were missing on older Mirth ones. Different releases ship different Rhino versions (and some +> setups can be configured for other engines), so confirm any borderline feature against what your server +> actually runs. When in doubt, prefer the ES5-safe form. + +### Rhino loop-scoping bug — use `let` inside loop bodies, never `const` +Rhino hoists a `const`/`let` declared **inside** a loop body to the enclosing function scope instead of +re-creating it per iteration. With `const`, the binding is created once and silently reused — mutations +don't error, but the variable doesn't behave per spec. + +```javascript +// WRONG — Rhino reuses the same binding across iterations. +while ((result = regex.exec(str)) !== null) { + const replacer = String(values[result[1]] || '').padEnd(result[0].length); + newStr = newStr.replace(result[0], replacer); +} + +// CORRECT — use `let` so the per-iteration assignment actually takes effect. +while ((result = regex.exec(str)) !== null) { + // MUST USE LET — Rhino hoists a const declared inside a loop to the outer scope. + let replacer = String(values[result[1]] || '').padEnd(result[0].length); + newStr = newStr.replace(result[0], replacer); +} +``` +Applies to `for`, `for...in`, `while`, and `do/while` bodies. Declarations **outside** the loop are +unaffected — `const` is still preferred at function scope. + +### Java interop returns Java objects, not JS values +OIE APIs and `java.*` / `Packages.*` calls return **Java** objects, which don't behave like their JS +lookalikes: +- A Java `List`'s `.toArray()` returns a Java `Object[]`, **not** a JS array. `Array.isArray()` is `false` + for it and it has no `.push()`. Copy element-by-element into a real JS array first: + ```javascript + function toJsArray(v) { + if (v == null) return []; + if (Array.isArray(v)) return v; + const src = v.toArray ? v.toArray() : v; // Java List -> Java Object[] + if (src != null && typeof src.length === 'number' && typeof src !== 'string') { + const out = []; + for (let i = 0; i < src.length; i++) { + let ele = src[i]; // MUST USE LET (loop-scoping bug) + out.push(ele); + } + return out; + } + return [v]; + } + ``` +- Java strings are `java.lang.String`, not JS strings — wrap with `String(x)` before string ops or identity + comparisons. +- Java numbers (`BigInteger`, `Long`, …) may need `Number(x)` / `String(x)` normalization. + +### Legacy `var` usage +Older channel code often uses `var`. When editing such a file you may modernize `var` → `const`/`let` where +it's clearly safe, but don't refactor a whole file just to change declarations. + +## Global scope execution +All OIE scripts run in a **global scope** with engine-provided globals; there is no runtime module system — +code is shared via Code Templates. Commonly available: +- **Message data:** `msg`, `tmp` (transformers), `message`, `connectorMessage` +- **Maps:** `channelMap`, `connectorMap`, `responseMap`, `globalMap`, `globalChannelMap`, + `configurationMap`, `sourceMap` +- **Map accessor shorthands** (built-in): `$c(k[, v])` → `channelMap`, `$gc(k[, v])` → `globalChannelMap`, + `$g(k[, v])` → `globalMap`, `$s(k[, v])` → `sourceMap`, `$cfg(k)` → `configurationMap`. One arg gets, two + args sets. +- **Channel context:** `channelId`, `channelName`, `messageId`, `logger`, `router`, `destinationSet`, + `response`, `replacer` +- **Batch scripts:** `reader` (and `writer` where applicable) +- **OIE utility classes:** `ChannelUtil`, `SerializerFactory`, `DatabaseConnectionFactory`, `AttachmentUtil`, + `DateUtil`, `FileUtil`, `HTTPUtil`, `DICOMUtil`, `AlertSender`, `Lists`, `Maps` +- **Java/E4X:** `Packages`, `java`, `com`, `org`, `importPackage`, `XML` (E4X) + +> The exact set varies by script type and OIE version. The Administrator's script editor lists the +> authoritative variables for your environment. + +### `globalChannelMap` / `globalMap` persistence note +The global maps are backed by a Java `ConcurrentHashMap` and store **live JS object references** (not +JSON copies): mutating a retrieved object is visible without re-storing, and numbers survive round-trips as +numbers. They **cannot store `null`** (throws an NPE) — guard reads, e.g. `globalChannelMap.get(key) || {}`. + +## Custom helper conventions (optional, recommended) +The map accessors (`$c`, `$gc`, …) are built in, but OIE ships **no** helper for safe navigation (no `?.`) +or retries. Most teams define a few of their own in a Code Template and use them everywhere. **These are +project conventions, not OIE built-ins.** + +| Helper | Wraps / does | +|---------------|--------------------------------------------------------------------| +| `$t(fn)` | run `fn` in try/catch, return `undefined` on throw | +| `$retry(...)` | run an operation with retry/backoff (no `Promise`/`async` in OIE) | +| `$sleep(ms)` | block for `ms` (Java `Thread.sleep`) when a delay is unavoidable | + +> **TODO:** List the helpers your project defines (and where) so the assistant uses them instead of +> hand-rolling. Delete this section if you don't use any. + +## Code patterns + +### Module/export pattern (Code Templates) +No `import`/`export`. Functions in a Code Template are global in channels that reference that Template's +library. Declare the globals you rely on and mark what you expose so linting stays clean: +```javascript +function myFunction(param) { /* ... */ } + +/* global someGlobalHelper logger */ +/* exported myFunction */ +``` + +### String building (NO template literals) +```javascript +// NEVER: `Patient ${name} has id ${id}` +const label = 'Patient ' + name + ' has id ' + id; + +// Array join — preferred for SQL and multi-part strings: +const sql = ['SELECT * FROM orders WHERE id = ', id, ' AND status = ', status].join(''); +``` + +### Safe property access (replaces `?.`) +Define a tiny try/catch helper once in a Code Template, then wrap deep access so a missing intermediate +returns `undefined` instead of throwing: +```javascript +// Define once (Code Template); guarded so re-loading the library doesn't redeclare it. +if (typeof $t === 'undefined') { + function $t(cb) { try { return cb(); } catch (e) { /* swallow -> undefined */ } } +} +/* exported $t */ +``` +```javascript +// $t(() => a.b.c) === a?.b?.c +const value = $t(() => obj.deep.nested.property) || defaultValue; +const field = $t(() => msg.get('OBR.3.1')) || ''; +``` + +### Prototype-based "classes" (no ES6 `class`) +```javascript +function Widget(host, key) { this.host = host; this.key = key; } +Widget.prototype.get = function (id) { /* ... */ }; +Widget.prototype.save = function (data) { /* ... */ }; + +// Inheritance via Object.create: +function SpecialWidget(config) { Widget.call(this, config.host, config.key); } +SpecialWidget.prototype = Object.create(Widget.prototype); +``` + +## Channel configuration checklist +> **TODO:** Replace with your standards. A typical baseline: +- **Message storage / pruning:** retention appropriate to your compliance needs (longer for billing-relevant). +- **Custom metadata columns:** searchable columns for the identifiers you triage by (e.g. accession/order id, error flag). +- **Source connector:** always populate your key metadata column; strip no-op destinations you don't want. +- **Destination connectors:** for Channel Writers, pick an appropriate "Queue Message"/retry policy (e.g. + queue on failure) so transient downstream outages don't drop messages. + +## Testing +> **TODO:** Describe your setup. OIE-runtime code can't be executed by a Node test runner directly — either +> (a) keep pure logic in functions a Node test imports with mocked OIE globals, or (b) verify OIE-specific +> behavior on a running server. State which this repo uses and where the mocks live. + +## Documentation standards +Use Mermaid (or another Markdown-native format) so diagrams render in-repo: +```mermaid +flowchart TD + A["Inbound message"] --> B{Route?} + B -->|Match| C["Destination channel"] + B -->|No match| D["Error handler"] +``` +Useful types: `flowchart`, `sequenceDiagram`, `erDiagram`, `stateDiagram-v2`. From 60e7e24bf583feb2929367883bf60f0431e7c1db Mon Sep 17 00:00:00 2001 From: Michael Hobbs Date: Fri, 10 Jul 2026 14:37:36 -0400 Subject: [PATCH 2/6] docs: adopt AGENTS.md (agent-neutral); fix CI-path wording; note Rhino ES6 default - Rename CLAUDE.md -> AGENTS.md and CLAUDE_USER_EXAMPLE.md -> AGENTS_USER_EXAMPLE.md, and drop the Claude-specific wording, per maintainer preference in #342 for a vendor-neutral instructions file. - Fix the build note: the `-DdisableSigning=true -Dcoverage=true` build is what CI runs on PRs; on `main` CI runs the signed build (same target, without those flags). - Note that OIE defaults `rhino.languageversion = es6` (server/conf/mirth.properties), which is what makes arrow functions / `let` / `const` work on the bundled Rhino 1.7.13. --- CLAUDE.md => AGENTS.md | 18 +++++++++--------- ...E_USER_EXAMPLE.md => AGENTS_USER_EXAMPLE.md | 13 +++++++++---- 2 files changed, 18 insertions(+), 13 deletions(-) rename CLAUDE.md => AGENTS.md (90%) rename CLAUDE_USER_EXAMPLE.md => AGENTS_USER_EXAMPLE.md (95%) diff --git a/CLAUDE.md b/AGENTS.md similarity index 90% rename from CLAUDE.md rename to AGENTS.md index c3181297c..076aae266 100644 --- a/CLAUDE.md +++ b/AGENTS.md @@ -1,13 +1,12 @@ -# CLAUDE.md — Open Integration Engine (engine) +# AGENTS.md — Open Integration Engine (engine) -Guidance for Claude Code (and any AI coding assistant) working on **this repository — the OIE engine -itself**. OIE is an open-source fork of Mirth Connect: a **production-critical, healthcare** integration -engine (HL7 / FHIR / DICOM) that runs in hospitals and labs, on a mature codebase with ~15 years of -history. Licensed **MPL-2.0**. +Guidance for AI coding agents working on **this repository — the OIE engine itself**. OIE is an open-source +fork of Mirth Connect: a **production-critical, healthcare** integration engine (HL7 / FHIR / DICOM) that +runs in hospitals and labs, on a mature codebase with ~15 years of history. Licensed **MPL-2.0**. > **Writing channel/template JavaScript that runs *on* the engine (transformers, filters, code > templates)?** That's a different context with its own Rhino/ES5 rules — start from -> **[`CLAUDE_USER_EXAMPLE.md`](./CLAUDE_USER_EXAMPLE.md)**, not this file. +> **[`AGENTS_USER_EXAMPLE.md`](./AGENTS_USER_EXAMPLE.md)**, not this file. ## Prime directive: be a careful guest, not a rewriter Optimize for the reviewer's trust and the project's long-term health — **never for volume of change.** A @@ -78,9 +77,10 @@ thinks *"I know how to fix this"* is exactly when a cleaner, safer, more idiomat ## Build & test (Java 17 + Ant) - Toolchain is pinned in [`.sdkmanrc`](./.sdkmanrc) — install [SDKMAN](https://sdkman.io/) and run `sdk env install` in the repo root (the JavaFX-bundled JDK is required; the `client` GUI imports JavaFX). -- **Build + test** (what CI runs off `main`): `cd server && ant -f mirth-build.xml -DdisableSigning=true -Dcoverage=true` - (the signed build drops the flags and runs on `main`). JUnit results land under - `*/build/test-results/**/*.xml`. **A red build is not reviewable — never open a PR on one.** +- **Build + test** (run this locally; it's also the CI build on pull requests): + `cd server && ant -f mirth-build.xml -DdisableSigning=true -Dcoverage=true`. CI runs this unsigned + + coverage build on PRs; on `main` it runs the **signed** build (same target, without those flags). JUnit + results land under `*/build/test-results/**/*.xml`. **A red build is not reviewable — never open a PR on one.** ## Module map (change the narrowest one that solves the issue) | Module | What it is | Care level | diff --git a/CLAUDE_USER_EXAMPLE.md b/AGENTS_USER_EXAMPLE.md similarity index 95% rename from CLAUDE_USER_EXAMPLE.md rename to AGENTS_USER_EXAMPLE.md index 1b99a4637..59a4bbe55 100644 --- a/CLAUDE_USER_EXAMPLE.md +++ b/AGENTS_USER_EXAMPLE.md @@ -1,12 +1,12 @@ -# CLAUDE_USER_EXAMPLE.md — starter for your OIE channel & template code +# AGENTS_USER_EXAMPLE.md — starter for your OIE channel & template code -**This is a template, not a rule for this repo.** It is a starter `CLAUDE.md` for a *separate* repository — +**This is a template, not a rule for this repo.** It is a starter `AGENTS.md` for a *separate* repository — **yours** — that holds the JavaScript you deploy into **Open Integration Engine** channels: source/ destination transformers, filters, response transformers, attachment handlers, and Global / Code Template -scripts. Copy it to *your* repo's root as `CLAUDE.md` and fill in the `TODO` sections. +scripts. Copy it to *your* repo's root as `AGENTS.md` and fill in the `TODO` sections. > Working on the **OIE engine itself** (the Java in this repository)? That's a different job — see this -> repo's [`CLAUDE.md`](./CLAUDE.md), not this file. +> repo's [`AGENTS.md`](./AGENTS.md), not this file. OIE executes channel/template JavaScript on the **Rhino** engine (bundled with the Java 17 runtime), so the constraints below are about Rhino's partial ES6 support and Java interop — not about Node.js. Real, working @@ -51,6 +51,11 @@ filters, response transformers, attachment handlers, and Global/Code Template sc > Files that run in a normal Node.js context (build tooling, tests, non-OIE utilities) are **not** subject > to these rules — scope this section to the directory that gets deployed to OIE (commonly `src/`). +> **Why the "supported" list works:** OIE ships `rhino.languageversion = es6` by default +> (`server/conf/mirth.properties`), which puts the bundled Rhino (1.7.13) in ES6 mode — that's what enables +> arrow functions, `let`/`const`, and destructuring below. If your server overrides that setting to an older +> version, re-verify the borderline features. + ### Supported ES6 features (safe to use) - `const` and `let` — prefer over `var`. `const` by default, `let` when reassignment is needed (but see the Rhino loop-scoping bug below). From aa5ca2a1dba5605e6a06ddcf9b57925826444abb Mon Sep 17 00:00:00 2001 From: Michael Hobbs Date: Sat, 11 Jul 2026 15:43:20 -0400 Subject: [PATCH 3/6] docs: update AGENTS.md build section for the Gradle migration The Ant build was replaced by Gradle (24c96f5), so the build/test command in AGENTS.md was stale. Point it at the wrapper and mirror the CI invocation; add the two build traps that fail CI without an obvious cause (cold-cache dependency verification metadata, build-logic output parity), deferring the detail to CONTRIBUTING.md. --- AGENTS.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 076aae266..3f128841c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,13 +74,21 @@ thinks *"I know how to fix this"* is exactly when a cleaner, safer, more idiomat 9. **No slop tells.** No emoji; no "as an AI"; no comments that restate the code or narrate the change; no speculative abstractions or "future-proofing" (YAGNI); no dead code; no reflowing of existing comments. -## Build & test (Java 17 + Ant) +## Build & test (Java 17 + Gradle) - Toolchain is pinned in [`.sdkmanrc`](./.sdkmanrc) — install [SDKMAN](https://sdkman.io/) and run `sdk env install` in the repo root (the JavaFX-bundled JDK is required; the `client` GUI imports JavaFX). -- **Build + test** (run this locally; it's also the CI build on pull requests): - `cd server && ant -f mirth-build.xml -DdisableSigning=true -Dcoverage=true`. CI runs this unsigned + - coverage build on PRs; on `main` it runs the **signed** build (same target, without those flags). JUnit - results land under `*/build/test-results/**/*.xml`. **A red build is not reviewable — never open a PR on one.** + The build runs through the Gradle **wrapper**; no separate Gradle install. +- **Build + test** (run this from the repo root; it's also the CI build on pull requests): + `./gradlew build -PdisableSigning=true -Pcoverage=true` (`gradlew.bat` on Windows). CI runs this unsigned + + coverage build on PRs; on `main` it runs the **signed** build (`./gradlew build`). JUnit results land under + `*/build/test-results/**/*.xml`. **A red build is not reviewable — never open a PR on one.** +- Two traps that make CI fail in ways the diff doesn't explain — see [`CONTRIBUTING.md`](./CONTRIBUTING.md) + before you touch either (and note rule 3: neither is yours to do unprompted): + - **Bumping a dependency** means regenerating the checksum-verification metadata **with a cold cache** + (`gradle/libs.versions.toml` → `--write-verification-metadata`); a warm cache silently omits parent POMs + and only CI fails. + - **Changing build logic** is guarded by *output parity*, not unit tests: build `server/setup` before and + after and diff the trees — only your intended change may appear. ## Module map (change the narrowest one that solves the issue) | Module | What it is | Care level | From 9f9359659341ca2ab0d99ccab5a3a2da7c850345 Mon Sep 17 00:00:00 2001 From: Michael Hobbs Date: Wed, 22 Jul 2026 19:28:09 -0400 Subject: [PATCH 4/6] docs(AGENTS_USER_EXAMPLE): correct Rhino 1.7.13 channel-JS facts Verified against the shipped rhino-1.7.13.jar and the engine source, per @pacmano1's review on #343: - SQL: replace the string-built 'preferred for SQL' join() example (taught injection) with a parameterized executeCachedQuery(sql, List) call; drop SELECT *. Split non-SQL string building into its own example. - Template literals: not a runtime error but a SILENT skip-interpolation on 1.7.13 (emits raw ${...}); note the 1.7.14 version boundary. - for...of: works on 1.7.13 -> move out of 'prohibited' to a supported style-preference (prefer forEach/indexed for). - spread: state it as the hard parse error it is, not 'unreliable'. - Map accessors: 7 not 5 (add $co, $r); document $('key') returns '' on miss, $s as get-only, $cfg two-arg put does not persist. - $t: keep the by-design optional-chaining/try-default behavior, but note it swallows more than real ?. so never wrap must-succeed side-effect calls. - $sleep: note it blocks the channel thread. - loop scoping: add the closure-capture-of-loop-variable defect. - Stamp the runtime section 'verified on Rhino 1.7.13'. --- AGENTS_USER_EXAMPLE.md | 107 +++++++++++++++++++++++++++++++++-------- 1 file changed, 88 insertions(+), 19 deletions(-) diff --git a/AGENTS_USER_EXAMPLE.md b/AGENTS_USER_EXAMPLE.md index 59a4bbe55..1c84493ef 100644 --- a/AGENTS_USER_EXAMPLE.md +++ b/AGENTS_USER_EXAMPLE.md @@ -55,6 +55,11 @@ filters, response transformers, attachment handlers, and Global/Code Template sc > (`server/conf/mirth.properties`), which puts the bundled Rhino (1.7.13) in ES6 mode — that's what enables > arrow functions, `let`/`const`, and destructuring below. If your server overrides that setting to an older > version, re-verify the borderline features. +> +> **The runtime behaviors below were verified against Rhino 1.7.13** (what current OIE ships). Some are +> long-standing Ecma-incompatibility bugs that are safe as hard rules; others are version-scoped and will +> **flip when the engine bumps Rhino** (called out inline). Treat anything marked version-dependent as "verify +> on your server." ### Supported ES6 features (safe to use) - `const` and `let` — prefer over `var`. `const` by default, `let` when reassignment is needed (but see the @@ -63,20 +68,31 @@ filters, response transformers, attachment handlers, and Global/Code Template sc - Object/array destructuring — `const { a, b } = options` - `Object.keys()`, `Object.values()`, `Object.entries()`, `Object.assign()` - Array methods: `.map()`, `.filter()`, `.reduce()`, `.forEach()`, `.find()`, `.some()`, `.every()` - -### Prohibited ES6+ features (will break at runtime in OIE) -- **Template literals** — NEVER use backtick strings. `` `Hello ${name}` `` fails at runtime. Use string - concatenation or `Array.join()` (see "String building"). -- **Optional chaining `?.`** — not supported; use a try/catch helper (see "Safe property access"). -- **Nullish coalescing `??`** — use `||` or an explicit ternary. +- `for...of` — runs correctly on Rhino 1.7.13, but **prefer `.forEach()` or an indexed `for`**. It relies on + the iterator protocol (extra overhead at channel throughput, and Rhino's support across non-array iterables + is uneven), so avoiding it keeps channel code predictable across Rhino versions. Style preference, not a + correctness rule. + +### Prohibited ES6+ features (will break in OIE) +Unless noted, each of these is a **parse error** — the script fails to compile. +- **Template literals** — NEVER use backtick strings, but note the failure is **silent, not an error**: on + Rhino 1.7.13 `` `Hello ${name}` `` evaluates without throwing and skips interpolation, yielding the literal + text `Hello ${name}`. Nothing errors, the channel stays green, and e.g. a File Writer keyed on such a value + writes every message to one literally-named file. (Version-dependent: interpolation was implemented in Rhino + 1.7.14, so this flips if the engine bumps Rhino.) Use string concatenation or `Array.join()` (see "String + building"). +- **Optional chaining `?.`** — parse error; use a try/catch helper (see "Safe property access"). +- **Nullish coalescing `??`** — parse error; use `||` or an explicit ternary. - **`async`/`await`** — not supported. Transformers are synchronous; use callbacks/retries. -- **`Promise`** — not available in the runtime. -- **ES6 classes (`class`/`extends`)** — use constructor functions with `.prototype` methods. +- **`Promise`** — not available in the runtime (`Promise` is `undefined`). +- **ES6 classes (`class`/`extends`)** — parse error; use constructor functions with `.prototype` methods. - **ES6 modules (`import`/`export`)** — share code via Code Templates plus `/* global */` and `/* exported */` comments (see "Module/export pattern"). -- **Spread syntax `...args`** — minimal/unreliable support; avoid, especially in parameter lists. -- **`for...of` loops** — use `.forEach()` or a traditional indexed `for` loop. -- **Default parameters** — use `param = param || defaultValue` instead. +- **Spread syntax `...args`** — a **hard parse error** in both call and array-literal position; the script + won't compile. +- **Default parameters** — parse error; use `param = param || defaultValue` instead. +> `for...of` is **not** prohibited — it works on 1.7.13. See the supported list (prefer `forEach`/indexed +> `for` as a style choice). > **Version-dependent — verify on your server.** `Symbol`, `Map`, and `Set` exist on current OIE/Rhino > builds but were missing on older Mirth ones. Different releases ship different Rhino versions (and some @@ -105,6 +121,22 @@ while ((result = regex.exec(str)) !== null) { Applies to `for`, `for...in`, `while`, and `do/while` bodies. Declarations **outside** the loop are unaffected — `const` is still preferred at function scope. +**Second, related defect — never capture a loop variable in a closure created inside the loop.** On Rhino +1.7.13, closures made inside `for (let i = ...)` or `for (let k in obj)` all capture the **same** binding, so +they every see the *final* value (spec says each gets its own): +```javascript +// WRONG — on Rhino 1.7.13 all three functions return 3 (spec: 0, 1, 2) +var fns = []; +for (let i = 0; i < 3; i++) { fns.push(function () { return i; }); } + +// CORRECT — copy the control variable to a fresh body-level `let` first, then capture that +for (let i = 0; i < 3; i++) { + let n = i; + fns.push(function () { return n; }); +} +``` +(This is why the `toJsArray` example above copies `src[i]` into a body-level `let` before use.) + ### Java interop returns Java objects, not JS values OIE APIs and `java.*` / `Packages.*` calls return **Java** objects, which don't behave like their JS lookalikes: @@ -140,9 +172,15 @@ code is shared via Code Templates. Commonly available: - **Message data:** `msg`, `tmp` (transformers), `message`, `connectorMessage` - **Maps:** `channelMap`, `connectorMap`, `responseMap`, `globalMap`, `globalChannelMap`, `configurationMap`, `sourceMap` -- **Map accessor shorthands** (built-in): `$c(k[, v])` → `channelMap`, `$gc(k[, v])` → `globalChannelMap`, - `$g(k[, v])` → `globalMap`, `$s(k[, v])` → `sourceMap`, `$cfg(k)` → `configurationMap`. One arg gets, two - args sets. +- **Map accessor shorthands** (built-in — there are **seven**): `$co(k[, v])` → `connectorMap`, + `$c(k[, v])` → `channelMap`, `$s(k[, v])` → `sourceMap`, `$gc(k[, v])` → `globalChannelMap`, + `$g(k[, v])` → `globalMap`, `$cfg(k[, v])` → `configurationMap`, `$r(k[, v])` → `responseMap`. One arg + gets, two args put — with two caveats: treat **`$s` as get-only** (`sourceMap` is read-oriented; a two-arg + write is rejected in batch scripts and shouldn't be relied on elsewhere), and a two-arg **`$cfg` put + succeeds but does not persist** (runtime-only; it won't survive a restart). +- **`$('key')`** (single string arg) searches every map in order — response → connector → channel → source → + globalChannel → global → configuration — and returns **`''`** (empty string, *not* `null`/`undefined`) on a + total miss. So `$('x') == null` never fires; test with `!$('x')` or `$('x') === ''`. - **Channel context:** `channelId`, `channelName`, `messageId`, `logger`, `router`, `destinationSet`, `response`, `replacer` - **Batch scripts:** `reader` (and `writer` where applicable) @@ -165,9 +203,9 @@ project conventions, not OIE built-ins.** | Helper | Wraps / does | |---------------|--------------------------------------------------------------------| -| `$t(fn)` | run `fn` in try/catch, return `undefined` on throw | +| `$t(fn)` | run `fn` in try/catch, return `undefined` on throw (optional-chaining-like; see caveat) | | `$retry(...)` | run an operation with retry/backoff (no `Promise`/`async` in OIE) | -| `$sleep(ms)` | block for `ms` (Java `Thread.sleep`) when a delay is unavoidable | +| `$sleep(ms)` | block for `ms` (Java `Thread.sleep`) — **blocks the channel thread**; at full throughput it stalls the queue, so use sparingly | > **TODO:** List the helpers your project defines (and where) so the assistant uses them instead of > hand-rolling. Delete this section if you don't use any. @@ -189,9 +227,30 @@ function myFunction(param) { /* ... */ } // NEVER: `Patient ${name} has id ${id}` const label = 'Patient ' + name + ' has id ' + id; -// Array join — preferred for SQL and multi-part strings: -const sql = ['SELECT * FROM orders WHERE id = ', id, ' AND status = ', status].join(''); +// Array join — for multi-part NON-SQL strings (paths, log lines, delimited output): +const path = ['/data/', channelId, '/', messageId, '.hl7'].join(''); +``` + +### SQL — parameterize, never build query strings +**Never** concatenate or `join()` a value into SQL. Bind parameters so a value can't alter the query — this +engine carries PHI, so injection here is a breach, not a bug: +```javascript +// NEVER — string-built SQL, even via join(), is injectable: +// var sql = ['SELECT id FROM orders WHERE id = ', id].join(''); + +// CORRECT — parameterized query; ? placeholders + a bound-parameter list: +var conn = DatabaseConnectionFactory.createDatabaseConnection('org.postgresql.Driver', url, user, pass); +try { + var rs = conn.executeCachedQuery( + 'SELECT id, status FROM orders WHERE id = ? AND status = ?', + java.util.Arrays.asList(id, status)); // values are bound, never interpolated + // ... read rs ... +} finally { + conn.close(); +} ``` +Name the columns you need instead of `SELECT *`. `executeUpdate(sql, params)` takes the same bound-parameter +list for writes. ### Safe property access (replaces `?.`) Define a tiny try/catch helper once in a Code Template, then wrap deep access so a missing intermediate @@ -204,10 +263,20 @@ if (typeof $t === 'undefined') { /* exported $t */ ``` ```javascript -// $t(() => a.b.c) === a?.b?.c +// Two intended uses: +// 1. Optional-chaining-like navigation — a missing intermediate returns undefined instead of throwing, +// so $t(() => a.b.c) stands in for a?.b?.c: const value = $t(() => obj.deep.nested.property) || defaultValue; const field = $t(() => msg.get('OBR.3.1')) || ''; +// 2. Deliberate try/default around a risky call: +const parsed = $t(() => JSON.parse(raw)) || {}; ``` +> **Caveat (by design, but know it):** `$t` swallows **every** exception, which is *broader* than real `?.` +> — optional chaining only short-circuits on `null`/`undefined` and still propagates a thrown error, whereas +> `$t` turns any throw into a silent `undefined`. That's intentional (it's what makes the `$t(...) || default` +> idiom work), but never wrap a call whose failure you need to surface — a failed DB write or ACK send inside +> `$t` becomes a silent `undefined`, the exact failure mode this doc exists to prevent. Use it for navigation +> and best-effort reads, not for operations with side effects that must succeed. ### Prototype-based "classes" (no ES6 `class`) ```javascript From 544d3b6d7d35ccf5955b30593355824efcedfb2f Mon Sep 17 00:00:00 2001 From: Michael Hobbs Date: Wed, 22 Jul 2026 19:28:09 -0400 Subject: [PATCH 5/6] docs(AGENTS): reframe rule 2 from size caps to structure + verification Per @pacmano1 on #343: the numeric caps (<=3 files, ~40-50 lines) were a proxy that misfired both ways -- they would have branded the Ant->Gradle migration unwanted, and a routine dependency bump regenerating verification-metadata.xml trips them, while a tiny God-function PR passes. Size conflated two orthogonal things: the reach of a change and the structure of the code. Replace the caps with the actual intents -- one concern respecting SRP/single-level-of-abstraction (with orchestration/init and low-complexity sequences as legitimate long-function exceptions), and verification scaling with reach (rule 5). Anti-slop teeth are kept and sharpened; 'unscoped, unverified sprawl is the enemy, size is not.' --- AGENTS.md | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3f128841c..cc044043a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,12 +10,13 @@ runs in hospitals and labs, on a mature codebase with ~15 years of history. Lice ## Prime directive: be a careful guest, not a rewriter Optimize for the reviewer's trust and the project's long-term health — **never for volume of change.** A -tiny, correct, well-explained fix tied to an issue is a *success*; a large, plausible-looking, unrequested -change is a *cost* the maintainers must now audit in a healthcare setting. The failure mode this file +tiny, correct, well-explained fix tied to an issue is a *success*; an unrequested, plausible-looking change +that sprawls past its concern is a *cost* the maintainers must now audit in a healthcare setting. The failure mode this file prevents: confident, sprawling, style-driven AI changes ("vibeslop") that look fine, pass a glance, and -quietly break behavior, compatibility, or trust. When a change can't be kept small, that's a signal to -**stop and discuss on the issue** — not to proceed. (There is deliberately no orchestration/automation skill -here: the project does not want large multi-file, single-PR agent changes.) +quietly break behavior, compatibility, or trust. A large change isn't automatically unwelcome — the +Ant→Gradle migration was large and is now the build's foundation — but it must arrive **scoped to one concern +and with a verification plan proportional to its reach** (rule 5), never as an unaudited wall of diff. +Unscoped, unverified sprawl is the enemy; size is not. **Green light:** if the change is issue-scoped, small, and test-backed — the thing you were asked to do — just do it well. You don't need to re-ask permission for what's already agreed on the issue. Escalate only @@ -40,11 +41,15 @@ thinks *"I know how to fix this"* is exactly when a cleaner, safer, more idiomat maintainer's triage or reply* before writing non-trivial code. Opening an issue then immediately coding the whole change defeats the point. A tiny obvious fix: say so on the issue and keep the diff tiny. No PR without a referenced issue. -2. **One concern; minimal diff; bounded in breadth *and* depth.** No drive-by edits (reformatting, - re-indenting, import reordering, renames in code you aren't otherwise changing). **Breadth:** touch **≤3 - files**; more → stop and discuss. **Depth:** if the fix means rewriting a whole method/class, or a single - file's diff exceeds ~40–50 changed lines, that's sprawl too — stop and discuss. A big single-file rewrite - is still slop. +2. **One concern, respecting the code's existing structure.** No drive-by edits (reformatting, + re-indenting, import reordering, renames in code you aren't otherwise changing). The signal isn't diff + size — it's structure: a change sprawls when it spans **more than one concern**, or when it violates the + decomposition it edits (balloons a function past its single responsibility, mixes levels of abstraction, + collapses intention into implementation). Judge by SRP / single-level-of-abstraction, not a line count — + and long isn't automatically wrong: orchestration/initialization routines and low-complexity linear + sequences are legitimately long, so don't fragment them to hit a number. A genuinely large *single-concern* + change (a migration, a codebase-wide rename, a regenerated lockfile) is fine when its verification scales + with its reach (rule 5). 3. **Match the surrounding code.** There's no autoformatter to hide behind — mirror the style/naming/idioms of the file. Introduce no new patterns, libraries, frameworks, build tools, or dependencies without explicit maintainer approval on the issue. From 663f8017c57b96a319078a18638aa7b116155987 Mon Sep 17 00:00:00 2001 From: Michael Hobbs Date: Thu, 23 Jul 2026 13:53:20 -0400 Subject: [PATCH 6/6] docs(AGENTS): rule 5 must require reviewer-reproducible verification Per @pacmano1 on #343: a PR that 'looks fine' but has no way to reproduce the fix drives reviewers up the wall -- the mechanic who replaces the battery and calls it done without turning the key. Rule 5 only made the author verify (fail-before/pass-after test); extend it to require a 'How to verify' section giving the reviewer a runnable before/after, and the exact steps + observable result for behavior a unit test can't capture (wire output, deployed channel, REST path). Green CI is not starting the car. --- AGENTS.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cc044043a..daeab51ad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,9 +60,14 @@ thinks *"I know how to fix this"* is exactly when a cleaner, safer, more idiomat order, connection-pool defaults, XStream alias maps. For any change under `donkey/` or in a serialize/parse path, include a **before/after golden-output test** proving behavior is unchanged (or the change is the point and is signed off). -5. **Tests are load-bearing and must bite.** Add/adjust tests for any behavior change; a regression test must - **fail against the pre-fix code and pass after** — state that you verified this (line coverage is not - proof). **Never delete, skip, or weaken a test to go green.** The build (below) must pass before a PR. +5. **Tests are load-bearing, and the PR must show how to start the car.** Add/adjust tests for any behavior + change; a regression test must **fail against the pre-fix code and pass after** — state that you verified + this (line coverage is not proof). **Never delete, skip, or weaken a test to go green.** The build (below) + must pass. **And every fix's PR must include a `How to verify` section**: the concrete before/after a + *reviewer* can reproduce — the failing input and wrong output before, the correct output after — not merely + "tests pass." Green CI is the dashboard light going off; it is not turning the key. *(Replacing the battery + isn't the job — starting the car is.)* For behavior a unit test can't capture — wire output, a deployed + channel, a REST path — give the exact steps and the observable result. 6. **Security — this engine carries PHI.** - **Never log message content, payloads, headers, or anything that could be PHI**; add no new logging of message bodies or connector I/O; serialize no new fields into audit/event tables without sign-off.