Inline Gemini reviewer for calibration

.github/review-layers/harper/common.md

@@ -0,0 +1,38 @@
+# Harper common conventions
+
+Version-agnostic review guidance for repos in the Harper ecosystem. The repo's own `CLAUDE.md` may duplicate or extend this — when in conflict, the repo's CLAUDE.md wins (it's closer to the source).
+
+## Non-obvious behavior
+
+- **`GenericTrackedObject` + spread.** `{ ...obj }` on a Harper _generic_ tracked object (the shape for `session.oauth`, `session.oauthUser`, and any tracked object without a declared schema) copies nothing — properties are served through a Proxy `get` trap, not as own enumerable properties. `Object.keys(obj)` returns `[]`. Use explicit property access: `{ provider: obj.provider, ... }`. Typed TrackedObjects (table rows with declared attributes) may behave differently; verify against the specific tracked type rather than assuming either way. Any test using spread on session fields is testing a plain object, not the production shape.
+- **`session.oauth` vs `session.delete()`.** Some session objects expose `.delete(id)` (Harper production shape — destroys the DB record). Some don't (in-memory test mocks). Code calling `clearOAuthSession` or similar may mutate `.oauth`/`.oauthUser` in-memory on test shapes but destroy the entire session record in production. Tests that exercise only one shape hide the divergence.
+- **`npm run build` tolerance.** Many Harper repos use `tsc || true` so the build passes even with type errors. "Build passes" ≠ "types are sound." Type correctness must be verified separately (editor diagnostics, a dedicated typecheck script, or CI step).
+- **`npm run lint` is ESLint-only**, not a typecheck.
+
+## Code conventions
+
+- TypeScript strict mode; ES modules with `.js` extensions in imports; named exports only (no default exports).
+- Logging uses the optional-chain pattern `logger?.info?.()` — the `logger` argument is frequently undefined in library code, so code must not assume it's present.
+- Error classes come from each repo's `src/errors.ts` OR the framework package (`harper` v5 / `harperdb` v4). All custom errors include a `statusCode` property.
+- Tokens, passwords, session IDs — **never** log, never return in responses, never include in error messages.
+
+## Review checks specific to Harper
+
+- **`scope.resources` / `scope.server` usage.** Declared optional in the Harper type but always assigned in the Scope constructor. Code should either guard once at entry or use narrowed locals, not sprinkle `?.` / `!` throughout.
+- **`static loadAsInstance = false` (Resource API v2).** Harper instantiates the class per request; do NOT rely on shared mutable instance state across requests. If per-request state is stored, it belongs on the context (`this.getContext()`).
+- **Unused runtime dependencies.** Harper repos target minimal runtime deps. New ones require explicit justification in the PR description (some repos maintain a `dependencies.md` for this).
+- **Dev/prod dependency mismatch.** Production code paths must only import from `dependencies` or `peerDependencies`. A `devDependencies` package referenced by runtime code (e.g. `await import('undici')` in an HTTPS path, a runtime helper imported from a test-only utility) breaks deployment for any consumer that doesn't share the dev environment. Caveat: some packages are also Node built-ins on recent Node versions (`undici` on Node 22+); using them without an explicit dep is OK ONLY if `engines.node` declares the floor that makes them built-in. When the runtime requirement and the declared engine floor disagree, flag both — the missing dep AND the permissive `engines.node`.
+
+## Documentation boundary (defer to Harper docs)
+
+Harper maintains its own documentation at [docs.harperdb.io](https://docs.harperdb.io) covering core, pro, and fabric. App, sample, and plugin repos should:
+
+- Document what is specific to the app/plugin itself — env var names, config shape, setup flow, integration API.
+- **Link** to the Harper documentation for anything not app/component-specific: deployment mechanics, runtime env var handling, Fabric configuration, core database behavior, SQL translator, replication, etc.
+
+Flag PRs that re-explain Harper behavior in-repo when a link to the authoritative Harper docs would be more maintainable. Re-explanation creates drift — Harper updates its docs, the copy doesn't.
+
+Borderline calls (judgment, not a blocker):
+
+- Short factual reminders where a link alone is too thin (e.g. "Harper reads env vars directly from the process environment — see [...]") are fine.
+- Duplicating whole Harper docs sections inline is not — link.

.github/review-layers/harper/v5.md

@@ -0,0 +1,73 @@
+# Harper v5 conventions
+
+Review guidance for repos that target Harper v5. Apply this layer when the repo's `peerDependencies` declares `harper`.
+
+> **Authoritative source.** `HarperFast/skills/harper-best-practices/` is the customer-facing guidance for building on Harper. This document is a **review lens** — what to CHECK when reviewing changes. If anything here contradicts the skills, the skills win for user-facing guidance; this file's job is reviewer discipline.
+
+## Package and CLI
+
+- **Package**: `harper`. Imports: `from 'harper'`. Peer dep range: `harper >=5.0.0`.
+- **CLI binary**: `harper` (e.g., `harper deploy_component`). PR diffs introducing v4-era `harperdb` imports or `harperdb` CLI calls are stale — flag them.
+
+## Resource API v2
+
+v5 has two co-existing dispatch patterns. A review of any Resource — or anything that wraps a Resource — must account for BOTH.
+
+- **Instance-method pattern**:
+
+  ```ts
+  class MyResource extends Resource {
+    static loadAsInstance = false;
+    async get(target) {
+      const request = this.getContext();
+      ...
+    }
+  }
+  ```
+
+  Harper's Resource base static `get` is `transactional(fn)` — it creates a per-request instance and calls the instance method.
+
+- **Static-method pattern** (v5 migration guide explicitly recommends this):
+
+  ```ts
+  class MyResource extends Resource {
+    static async get(target, request) { ... }
+  }
+  ```
+
+  Harper's REST dispatcher calls `Class.get(target, request)` at the STATIC level. The user's static shadows the Resource base's transactional static — no instance is created.
+
+- **Wrappers must cover both surfaces.** A wrapper that intercepts only instance methods is silently bypassed when the user adopts the static-method pattern. Full coverage wraps both and de-duplicates work (e.g. via a WeakSet keyed by the request context) so a single dispatch doesn't run validation twice.
+
+- **405 vs 204.** Harper's REST dispatcher returns `405 Method Not Allowed` when the method is undefined on the resource. A wrapper that defines all five verbs unconditionally — even when the parent doesn't implement them — replaces `undefined` with a function that returns `undefined`, and Harper serializes that as `204 No Content`. Only wrap verbs the parent actually implements.
+
+## Context + `getContext()`
+
+- Request-scoped state lives on the context returned by `this.getContext()` (inside an instance method). For static methods, Harper passes the request as the second arg: `static async get(target, request)`.
+- `this.getCurrentUser()` returns the authenticated user off the current context. Checks against `user.role.permission.super_user` are the standard "require superuser" guard.
+- `scope.resources` and `scope.server` are typed as optional in v5 but always assigned in the Scope constructor (`components/Scope.ts:70-71`). Plugins should guard once at `handleApplication` entry and use narrowed locals.
+
+## `config.yaml` and resource loading
+
+- `jsResource.files` controls which files Harper loads as resources. A change to this glob / pattern / directory / extension IS a meaningful review concern — it changes what actually gets served.
+- `config.yaml` also carries plugin entry point (`pluginModule`), `graphqlSchema.files`, and runtime defaults (e.g. `redirectUri`, `scope`, `defaultRole`). Treat `config.yaml` changes as code, not docs — a typo breaks the plugin/app at runtime.
+
+## Environment variables and `.env`
+
+- **Harper runtime** reads `process.env.*`. It does not itself parse a `.env` file on startup.
+- **Harper app template** (`npm create harper`) scaffolds `dotenv-cli` in `package.json` scripts. `npm run dev` / `npm run deploy` invokes `dotenv -- npm run <script>`, loading `.env` before Harper or the deploy CLI starts. `.env` IS the standard local-dev and deploy-credentials pattern for Harper v5 apps.
+- **`.env` at app root deploys with the component.** Harper Fabric ships the repo's `.env` alongside the deployed application, so the same file that provides secrets locally is available in the Fabric runtime too. This is why app-specific docs don't need to explain a separate Fabric-runtime mechanism — the local-dev `.env` IS the production `.env`. (Repo docs should still link to the Harper docs for the deployment mechanics, per the "defer to Harper docs" rule in `common.md`.)
+- **Deploy credentials vs runtime secrets.** `CLI_TARGET*` vars authenticate `harperdb deploy_component` against the cluster — they're loaded in the SHELL that runs the deploy CLI, NOT deployed with the app. Runtime app secrets (`OAUTH_GITHUB_CLIENT_ID`, etc.) come from the deployed `.env`. Don't conflate the two in docs — a GitHub Actions `env:` block used for deploy credentials does NOT propagate to runtime.
+- When reviewing docs about env-var handling, recommendations should match Harper's actual toolchain — `.env` via `dotenv-cli` for local/CI/deploy credentials, deployed `.env` for Fabric runtime — NOT generic systemd, Kubernetes secrets, or cloud-provider secrets manager patterns unless the PR author has specifically asked about non-Fabric deployment.
+
+## Deployment: Fabric
+
+Harper v5's managed production platform is Fabric.
+
+- **Authoritative references** in `HarperFast/skills`: `harper-best-practices/rules/deploying-to-harper-fabric.md`, `harper-best-practices/rules/creating-a-fabric-account-and-cluster.md`.
+- Default deployment recommendations go to Fabric. Alternatives are for users who explicitly opt out.
+
+## Typing
+
+- Imports from `harper` — v5 exports `Resource`, `Scope`, `User`, `RequestTarget`, `Context`, `SourceContext`, and others. Verify imports against the v5 `index.d.ts`.
+- Harper v5 has pre-existing TS errors in its own source that surface during consumer `tsc` runs; repos use `tsc || true` to tolerate these. Do NOT flag type errors originating from `node_modules/harper/` as blockers — they're upstream.

.github/review-layers/repo-type/plugin.md

@@ -0,0 +1,36 @@
+# Harper plugin repo
+
+Applies to repos that ship as npm packages consumed by Harper applications — OAuth, and future plugins. These repos have a distinct review surface because they extend Harper rather than being Harper itself.
+
+## Registration and lifecycle
+
+- **Plugin entry** is `handleApplication(scope)` exported from `src/index.ts`. Verify the PR preserves this signature and doesn't rename or re-shape it.
+- **Resource registration** uses `scope.resources.set('<name>', Class)`. Harper's dispatch logic requires the registered value to be a class (with `static getResource` inherited from `Resource`), not an instance, not a plain object.
+- **HTTP middleware** registration uses `scope.server.http?.(handler)`. Verify new middleware handles the "no OAuth / no auth data" passthrough case correctly — middleware runs on every HTTP request, including requests that don't need the plugin's semantics.
+- **Close handlers**: Plugins that hold resources (cache instances, timers, sockets) should register a cleanup listener on `scope.on('close', ...)`. Verify new long-lived state has a corresponding cleanup path.
+
+## Public API surface
+
+- **Everything exported from `src/index.ts` is public.** Re-exports need JSDoc and semver care.
+- **New public symbols require tests.** Silent no-test public API was a recurring gap in the OAuth plugin's review history.
+- **Breaking changes require a major version bump.** For plugins with a maintenance branch (e.g. `v1.x` for harperdb-v4 support), check whether the fix needs a backport. Surface the backport question in the PR description if non-trivial.
+
+## Dependencies
+
+- **Zero or justified runtime dependencies.** Every new runtime dep needs explicit justification in the PR description. Plugins that previously claimed zero runtime deps and then added one (e.g. `jsonwebtoken` for JWT-based providers) must update their README / docs accordingly.
+- **Peer dep version range** should match the plugin's compatibility promise. v5-only plugin → `harper >=5.0.0`. Multi-version plugin → union range or separate release lines.
+
+## Wrapping and decorating patterns
+
+Plugins frequently offer a wrapper function that user resources opt into (e.g. `withOAuthValidation`). Review such wrappers against the full dispatch surface audit in `universal.md` and the v5 static-vs-instance guidance in `harper/v5.md`. Common bypass shapes we've seen:
+
+- Proxy-based wrappers that don't intercept class-level dispatch.
+- Subclass wrappers that only override instance methods and miss user-defined statics.
+- Subclass wrappers that define all five verbs unconditionally and break Harper's 405 Method Not Allowed semantics for verbs the parent doesn't implement.
+- Wrappers whose `onValidationError` (or equivalent) callback accepts `undefined` as a "no problem" sentinel — giving an integrator's no-op logger silent auth-bypass powers.
+
+## Docs expectations
+
+- README's "Quick Start" is local-dev guidance — `.env` or shell `export`. Production guidance should point to Fabric (see `harper/v5.md`), not generic deployment patterns.
+- `docs/` covers integration patterns. Doc-only PRs still need to be consistent with the `harper/v5.md` deployment model.
+- `CLAUDE.md` captures non-obvious local gotchas. Any new wrinkle discovered during review (especially one that bit an earlier PR) should be proposed for `CLAUDE.md` in the same or follow-up PR.