Add AI Skills for OIE plugin development and code review

AI Skills/README.md

@@ -0,0 +1,33 @@
+# AI Skills for OIE developers
+
+Skills for AI coding agents that read `SKILL.md` files — they help you **build and review** Open
+Integration Engine plugins and channels. Each skill is a folder; a `SKILL.md` holds the instructions and
+(where present) a `references/` folder holds detail loaded on demand.
+
+| Skill | Use it to… |
+|---|---|
+| [`oie-plugin-development/`](./oie-plugin-development/) | **Build** a plugin: Maven project setup, `plugin.xml`, server/client/shared classes, REST servlets, DB migrations, packaging & signing. |
+| [`oie-plugin-code-review/`](./oie-plugin-code-review/) | **Review Java/plugin code** — smells, thread/resource safety, JDBC & migrations, security/PHI, plugin lifecycle, tests, static-analysis findings. |
+| [`oie-channel-code-review/`](./oie-channel-code-review/) | **Review channel/template JavaScript** (the Rhino runtime) — block-scoping traps, E4X/HL7 field access, scope-map lifetime, per-message cost, PHI in logs. |
+
+Plugin *building* and channel *JavaScript* are different runtimes with different rules, so the two review
+skills are deliberately separate — use the one that matches what you're reviewing.
+
+Each `SKILL.md` opens with YAML frontmatter — `name` and `description` are the portable core; the review
+skills also carry optional tool-control keys (`allowed-tools`, `disable-model-invocation`). Those extra keys
+are optional metadata that agents which don't recognize them simply ignore.
+
+## Install
+Copy the skill folder(s) you want into the skills directory your agent reads (global or per-project — see
+your tool's docs). For example:
+
+```bash
+cp -r "AI Skills/oie-plugin-development" <your-agent-skills-dir>/
+```
+
+Then invoke it by name or let your agent pick it up from the `SKILL.md` description.
+
+## Credit
+These skills are adapted from work by **[@pacmano1](https://github.com/pacmano1)** — the OIE Plugin
+Development Guide and the "Irritable Developer Check" review checklist. Thank you. Content has been
+reorganized into the skill format (and the two review skills split by runtime); the domain knowledge is his.

AI Skills/oie-channel-code-review/SKILL.md

@@ -0,0 +1,130 @@
+---
+name: oie-channel-code-review
+description: Review OIE/Mirth channel & code-template JavaScript (the Rhino runtime) the way an experienced, AI-skeptical developer would — Rhino/ES5 constraints and the one real block-scoping trap, E4X/HL7 field access, scope-map lifetime, per-message cost, unclosed DB connections, PHI in logs, and channel test coverage. Run before deploying a channel or when asked to review transformer/filter/code-template JS. For Java/plugin/server code, use the oie-plugin-code-review skill instead.
+disable-model-invocation: true
+allowed-tools: Grep Read Bash
+---
+
+# Irritable Developer Check — OIE channel / JavaScript (Rhino)
+
+Review channel and code-template JavaScript the way an experienced developer would before deploying it. Be
+specific and cite `file:line`. Favor catching real problems over being agreeable.
+
+> **Credit:** adapted from the **"Irritable Developer Check" by [@pacmano1](https://github.com/pacmano1)** —
+> the checklist and its hard-won judgment are his. This is the **channel/JavaScript (Rhino)** half;
+> Java/plugin/server review lives in the companion **`oie-plugin-code-review`** skill (different runtime,
+> different rules). Thank you.
+>
+> The block-scoping rule below intentionally differs from the original "`var` only" guidance: it targets the
+> *actual* Rhino loop bug while keeping modern `const`/`let` everywhere they're safe (rationale is in the
+> rule itself).
+
+**First, ask which scope to review, and wait for the answer:** recent changes (git diff) vs. the full set of
+channel/template scripts. If it's not a git repo, ask for the paths (exported channel XML, code-template
+files, or a `src/` of deployed JS).
+
+## Principle: follow JS best practice; deviate only where Rhino forces it
+Channels, code templates, transformers, and filters run as JavaScript on **Rhino** (ES5-era, partial ES6,
+compiled to JVM bytecode) with **Java interop**. That forces a *specific, documented* set of deviations from
+modern JS — listed below. **Flag violations of those forced rules.** Do **not** flag modern JS that runs
+fine on Rhino, and do **not** push `var`-for-its-own-sake or other downgrades: over-correcting away from
+best practice is its own smell. Every forced deviation should be understood (and ideally commented) as *why*,
+not cargo-culted.
+
+## Reviewer self-check `[BIAS]`
+Before reporting, check yourself — these matter more than any single rule.
+- **False positives to look thorough**: don't invent findings to seem diligent. A clean category reported clean is a real result.
+- **Confident fabrication**: don't claim a global exists or that code does X from memory. Read the line or grep for it; cite the `file:line` you actually looked at.
+- **Sycophancy**: when the author defends the code, re-check the code, not the argument. Hold the finding until the code changes your mind.
+- **Over-engineering as "improvement"**: don't flag "this could be a class/framework" when the direct code is correct. Flag simplification, never elaboration.
+- **Theory-chasing / anchoring**: confirm the actual root cause before proposing a fix. (Classic trap: days lost on TLS/cipher theories when the real bug was a `const` reused inside a compiled Rhino loop.)
+- **Imposing mainstream idioms over what runs on Rhino**: match the runtime's reality. But the inverse is also a bug — don't downgrade correct, Rhino-safe modern JS to legacy forms out of superstition.
+- **Severity inflation**: separate correctness from taste; say which is which.
+- **Prematurely dismissing real findings**: don't wave a genuine problem away as "pre-existing" or "later." Report it; let the author decide.
+
+## Rhino runtime rules `[RHINO]`
+These differ from Java and catch bugs the Java sections never will. Confirm exact signatures/globals against
+the OIE version in use.
+
+- **Block scoping inside loops — the one real trap.** Default to `const`/`let` (modern best practice: no
+  hoisting surprises, no accidental reassignment). The *forced* exception: Rhino does **not** create a fresh
+  per-iteration binding for a `const`/`let` declared **inside a loop body** — it hoists one binding to
+  function scope, shared across iterations. What that shared binding does depends on the keyword:
+  - **`const` is the real bug**: it can't be reassigned, so every iteration keeps the first value (canonical
+    case: a `while` over HTTP response headers pushing six identical `Date`s) — and some builds throw
+    "redeclaration of const" on the second pass. **Flag `const` declared-and-reused inside a loop body.**
+  - **`let` is reassigned each iteration**, so reading it *within the same iteration* is correct — **prefer
+    `let` there.** The one case where the shared binding still bites: a **closure created inside the loop**
+    that captures the variable (all closures then see the final value). For that, iterate with `.forEach()` —
+    its callback parameter is a genuine per-iteration scope.
+
+  Do **not** flag `const`/`let` at function scope, and do **not** rewrite working code to `var` — that
+  discards block-scoping safety to dodge one bug. (Some report the same mis-alias in `if`/`switch` blocks on
+  certain builds — verify against your Rhino version; a binding declared inside any block and reused is the
+  first suspect when values behave oddly.)
+- **Watch for the ES6 features Rhino handles poorly**: template literals, `async`/`await`, `Promise`,
+  optional chaining `?.`, nullish `??`, spread in parameter lists, and `for...of` are unreliable or missing
+  on the bundled Rhino — flag them and use `+`/`.join()`, callbacks/retries, plain `try/catch`, `||`, and
+  indexed/`.forEach()` loops instead. (Support is language-version dependent — OIE defaults Rhino to ES6, so
+  `let`/`const`/arrow functions *do* work; confirm borderline features against the server in use.)
+- **Use the channel `logger`, not `System.out`/`System.err`** — output goes through the provided `logger` at
+  the right level, never `println`. And never log HL7 content, PHI, or credentials (see `[SECURITY]`).
+- **Unclosed database connections**: a connection from `DatabaseConnectionFactory` must be closed in a
+  `finally`. One leak per message drains the pool and eventually wedges the channel.
+- **Scope-map misuse**: `channelMap`/`connectorMap`/`sourceMap` are per-message and don't persist to the next
+  message; `globalMap`/`globalChannelMap` live in memory until restart, so large/unbounded objects parked
+  there are a slow leak. Pick the narrowest scope that still carries where you need it. (`globalChannelMap`
+  can't store `null` — guard reads with `|| {}`.)
+- **E4X / HL7 access assumes fields exist**: a missing segment/field comes back as an empty `XMLList`, not
+  `null`, so `if (field == null)` never fires and a naive `.toString()` yields `""` that reads like
+  real-but-empty data. Guard with an existence/length check before trusting a value.
+- **Java/JavaScript type traps**: values from `msg`, the maps, or Java calls are often Java objects, not JS
+  primitives. `==` between a Rhino string and a Java `String`, or between two E4X nodes, doesn't compare what
+  you expect; coerce explicitly (`String(x)`, `.toString()`) and compare deliberately.
+- **Per-message expense**: compiling a regex, constructing a `SimpleDateFormat`, or opening a connection on
+  *every* message runs at full channel throughput. Hoist what you safely can (mind thread-safety for anything
+  shared — a `SimpleDateFormat` is not thread-safe).
+
+## Channel data access `[DATA]`
+- **Query in a loop / N+1**: transformers iterate HL7 segments and messages, so a per-item DB lookup turns
+  one message into hundreds of queries. Batch or set-based instead.
+- **No statement timeout**: a query with no `setQueryTimeout` can hang the channel's worker thread.
+- **Long/wide work inside a live connection**: keep DB work short; close in `finally` (above).
+
+## Security `[SECURITY]`
+Channels carry HL7/PHI outward by design.
+- **Logging PHI/secrets**: never log message content, patient identifiers, credentials, or tokens — not even
+  at debug. Healthcare data does not belong in logs.
+- **SQL by string concatenation**: use parameterized queries via `DatabaseConnectionFactory`, never
+  message-interpolated SQL.
+- **Unreviewed PHI egress**: any HTTP/API/LLM/analytics call that ships message content to an external
+  service needs a BAA-covered destination or a non-PHI/redacted model. OIE routes data outward by design, so
+  unreviewed egress is a real HIPAA exposure.
+- **PHI in outbound URLs**: identifiers in a path/query string land in proxy/server logs — use the body.
+
+## Tests `[TEST]`
+- **Untestable-by-design logic**: Rhino code can't run under a Node test runner directly. Pure logic should
+  live in functions a test can call with **mocked OIE globals** (`msg`, maps, `logger`); flag transformers
+  that bury testable logic inline with no seam.
+- **Happy-path only**: no test for the missing-segment, empty-`XMLList`, or error branch — exactly where the
+  E4X and type traps above bite.
+- **A bug fix with no regression test that fails without the fix.**
+
+## Six-month regrets `[REGRET]`
+Decisions whose cost arrives later — name the future moment each bites.
+- **Forced-deviation with no "why" comment**: a `var`/`let`-in-loop or `.join()`-instead-of-template-literal
+  that a future reader will "clean up" back into a bug because nothing says it was deliberate.
+- **Reasoning that lives outside the repo/channel**: why a value is coerced, why a scope map is used — if it's
+  only in someone's memory, the next editor breaks it. Put it in a comment or a committed note.
+- **"Temporary" channel hacks with no removal trigger**: a hardcoded endpoint, a disabled filter, a `// TODO`
+  with no ticket — temporary plus no trigger equals permanent.
+
+## Reporting findings
+Open with a one-line verdict: **ship / ship with caveats / block**, and the one or two risks that drive it.
+Then the findings, sorted worst-first. Tag each with its category — `[RHINO]`, `[DATA]`, `[SECURITY]`,
+`[TEST]`, `[REGRET]` — a **severity** (`[Blocker]`/`[Should]`/`[Nit]`), and a **confidence**
+(`[Confirmed]` = you read/ran it; `[Suspected]` = needs a closer look; labeling a guess `[Suspected]` is
+required). Auto-`[Blocker]`: PHI/secrets in logs or outbound URLs, unreviewed PHI egress, a `const`-in-loop
+reuse bug, or SQL built by concatenation.
+
+If a category is clean, say so briefly — "clean on scope-maps and E4X guards" is a useful result; don't pad.