diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..076aae266 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,109 @@ +# AGENTS.md — Open Integration Engine (engine) + +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 +> **[`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 +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** (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 | +|---|---|---| +| `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/AGENTS_USER_EXAMPLE.md b/AGENTS_USER_EXAMPLE.md new file mode 100644 index 000000000..59a4bbe55 --- /dev/null +++ b/AGENTS_USER_EXAMPLE.md @@ -0,0 +1,244 @@ +# 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 `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 `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 [`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 +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/`). + +> **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). +- 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`.