opal-frontend-common-node-review-guidelines

name	opal-frontend-common-node-review-guidelines
description	Automated review rules for opal-frontend-common-node-lib PRs. Use when asked to review code, run @codex review, or provide PR feedback; apply severity levels and the required comment format.

Opal Frontend Common Node Review Guidelines

Overview

Apply these rules when reviewing changes in this Node.js library; focus on P0/P1 blockers and use the required comment format.

Scope and Process

• Apply rules to changes in the PR only.
• Prefer specific, line-anchored feedback with rationale and concrete fixes.
• Treat P0 and P1 as blocking; treat P2 as advisory.

Repo Scope

• This is an ESM TypeScript library built with tsc and published via package.json exports.
• Runtime baseline is Node.js 18+.
• Consumers are Node/Express services in the OPAL ecosystem.

P0 Rules (Blockers)

Security and Safety

• Avoid credentials, tokens, secrets, or PII in code, logs, comments, or tests.
• Avoid disabling security middleware defaults (Helmet/CSRF/session) without explicit justification and tests.
• Do not introduce insecure defaults (e.g., weak cookie/session settings, relaxed CORS, disabling CSRF) without clear opt-in.
• Do not log sensitive request/session data.

Build and API Integrity

• Do not break the published API surface without a version bump and release notes.
• Keep package.json exports, src/index.ts, and tsconfig.json paths in sync for any public module change.
• Do not introduce deep-import-only usage that bypasses the exports map.
• Every package.json export target (import and types) must resolve to real files in dist/.
• yarn check:pack-shape (or npm pack --dry-run + equivalent validation) must pass for package-surface changes.
• Packed tarball must include all files referenced by exports, types, and typesVersions, including copied ambient .d.ts.
• Do not split publish metadata across multiple manifests (for example by generating dist/package.json).
• Avoid TypeScript errors, failing lint/prettier, or build regressions.

P1 Rules (High Priority)

Node/Express Correctness

• Ensure middleware is ordered correctly (e.g., body parser before routes, CSRF after session).
• Avoid mutating shared state across requests; keep per-request state in req/res.
• Use async error handling (pass to next) and avoid unhandled promise rejections.

TypeScript Quality

• Prefer explicit types for exported APIs; avoid any in public interfaces.
• Keep functions small and single-purpose; extract helpers for readability.
• Use consistent error shapes and typed config interfaces.

Packaging and ESM Compatibility

• For exported ESM files, ensure runtime-resolved relative imports remain Node-compatible after emit (no broken extensionless paths).
• If a PR tightens exports/subpaths, require a clear consumer impact note and migration import examples.
• Prefer local consumer validation via yarn pack:local tarball installs; flag raw-repo-link workflows that bypass publish behavior.

Dependency and Performance Risk

• Avoid introducing heavy dependencies unless justified; note impact in review.
• Prefer streaming for large payloads; avoid loading large buffers in memory without need.

Testing

• Add or update tests for new logic and error paths when feasible.
• Ensure changes are reflected in README usage if public APIs change.

P2 Rules (Advisory)

• Keep exported modules focused; avoid side effects at import time.
• Prefer clear naming over abbreviations; keep folder structure consistent with src/ layout.
• Keep .d.ts files minimal and aligned to actual runtime behavior.

Comment Format

Use this exact shape:

[Severity]: <Rule name>
Problem: <what is wrong in one sentence>
Why: <risk/impact>
Fix: <specific change>
Example: <code snippet or link to guideline>