From 68ca24a508c9485e97cced5e7e54991170c69e75 Mon Sep 17 00:00:00 2001 From: Erny Sans Date: Sat, 22 Aug 2026 16:49:20 -0500 Subject: [PATCH 1/4] ci: gate on stale build output and private markers This package commits its build output and its exports point directly at it, and there is no prepare script, so installing it straight from git performs no build. Consumers therefore execute the committed output while reviewers read the TypeScript source. Those are different files, and nothing in a pull request diff can reveal a mismatch between them: the source diff looks correct because the source is exactly what it claims to be. Add a build step that fails when the committed output differs from a clean build of the source, with a message naming the fix. It uses `git status --porcelain` rather than `git diff --exit-code` because git diff only sees tracked files, so a newly added source module compiles to untracked output files and git diff exits 0 -- the case that arises precisely when someone adds a module. Also add a check for private markers and attribution trailers covering tracked files, commit messages and commit authorship. Commit messages are included because they never appear in a file diff. The script proves its own patterns against synthetic markers on every run, so a pass means the patterns were demonstrated rather than assumed, and it holds no exemption for itself. --- .github/scripts/check-private-markers.sh | 198 +++++++++++++++++++++++ .github/workflows/nodejs.yml | 52 ++++++ 2 files changed, 250 insertions(+) create mode 100755 .github/scripts/check-private-markers.sh diff --git a/.github/scripts/check-private-markers.sh b/.github/scripts/check-private-markers.sh new file mode 100755 index 0000000..4f6338e --- /dev/null +++ b/.github/scripts/check-private-markers.sh @@ -0,0 +1,198 @@ +#!/usr/bin/env bash +# +# check-private-markers.sh +# +# This is a PUBLIC repository. This package is consumed by other services that +# are not public, and the operational details of those services must not appear +# here. The leak vector is not only documentation: it is every code comment, +# config comment, test fixture, commit message, pull request title and pull +# request body. A marker written in good faith by someone who simply did not +# know a repository was public is the realistic failure mode, so this check is +# automated rather than left to review. +# +# It scans, in order: +# 1. tracked files in the working tree +# 2. commit messages in the range under review +# 3. commit authorship (attribution trailers are not permitted here) +# +# The marker patterns are ASSEMBLED FROM FRAGMENTS below rather than written as +# literals. If they were written literally this script would match itself, and +# the only ways out of that are to exclude the script from its own scan — which +# turns it into a blind spot — or to publish the very strings it exists to +# block. Assembling them keeps the file both self-consistent and clean. +# +# CONSEQUENTLY THIS FILE IS **NOT** EXEMPT FROM ITS OWN SCAN, and must never be +# made exempt. There is no allow-list and no skipped path, so this file is not a +# blind spot: a real marker written here is caught exactly like a marker written +# anywhere else. That was verified by planting a synthetic finding-style +# identifier in this file and observing the check fail, citing this file's own +# line number. An exempted path is somewhere a real marker could be hidden, and +# an undocumented exempted path is how that rots quietly — so the correct design +# is no exemption at all, and the fragment assembly above is what makes that +# possible. +# +# Usage: +# check-private-markers.sh --self-test prove the patterns detect a synthetic marker +# check-private-markers.sh [RANGE] scan files, commit messages and authorship +# RANGE defaults to origin/main..HEAD +set -uo pipefail + +# --- pattern assembly (no private literal appears in this file) -------------- +ORG='fur'"cata" + +# Case-insensitive: names of non-public sibling repositories and environments. +PAT_NAMES="${ORG}/(functions|app|config)|${ORG}-(production|staging)" + +# Case-sensitive and word-bounded: internal tracker / finding identifiers. +# Deliberately narrow. A looser form (an unbounded, case-insensitive "W[0-5]") +# was measured against this repository and produced 105 false positives — 101 +# from base64 integrity hashes in package-lock.json and 4 from a video id in a +# test fixture — against 0 true positives. A check that cries wolf on every +# lockfile refresh is one that gets commented out, which is the same inert +# outcome as having no check at all. +PAT_IDS='\bORCH-[0-9]+\b|\bFN-M-[0-9]+\b|\b[HM]-0[0-9]\b|\bW[0-5] wave\b' + +ATTRIB='[Cc]o-authored-by:' + +fail=0 + +note() { printf '%s\n' "$*"; } +err() { printf '::error::%s\n' "$*" >&2; } + +# --- self test --------------------------------------------------------------- +# A guard that has never been observed rejecting anything is a hypothesis, not a +# control. This runs on every CI invocation, so the patterns are demonstrated +# live rather than trusted. The probe strings are built at runtime from split +# fragments: nothing greppable is written to disk and nothing is published. +self_test() { + local rc=0 tmp + tmp="$(mktemp -d)" + + # Positive control: each class must match a synthetic instance. + { + printf '%s/%s\n' "$ORG" "functions" + printf '%s-%s\n' "$ORG" "production" + printf 'ORCH-%s\n' "42" + printf 'FN-M-%s\n' "7" + printf 'H-%s\n' "01" + printf 'W%s wave\n' "0" + } > "$tmp/positive.txt" + + local want=6 got_names got_ids got + got_names=$(grep -ciE "$PAT_NAMES" "$tmp/positive.txt" || true) + got_ids=$(grep -cE "$PAT_IDS" "$tmp/positive.txt" || true) + got=$(( got_names + got_ids )) + if [ "$got" -ne "$want" ]; then + err "self-test FAILED: patterns matched $got/$want synthetic markers" + rc=1 + else + note "self-test: positive control OK ($got/$want synthetic markers detected)" + fi + + # Attribution trailer control. + printf 'Co-authored-by: Someone \n' > "$tmp/attrib.txt" + if ! grep -qE "$ATTRIB" "$tmp/attrib.txt"; then + err "self-test FAILED: attribution pattern did not match a synthetic trailer" + rc=1 + else + note "self-test: attribution control OK" + fi + + # Negative control: ordinary text must NOT match, otherwise a pass is noise. + printf 'An ordinary sentence about a TypeScript model library.\n' > "$tmp/negative.txt" + if grep -qiE "$PAT_NAMES" "$tmp/negative.txt" || grep -qE "$PAT_IDS" "$tmp/negative.txt"; then + err "self-test FAILED: patterns matched clean text (false positive)" + rc=1 + else + note "self-test: negative control OK (clean text not matched)" + fi + + rm -rf "$tmp" + return "$rc" +} + +# --- file scan --------------------------------------------------------------- +scan_files() { + local hits hits_ids + hits="$(git ls-files -z | xargs -0 grep -nIiE "$PAT_NAMES" 2>/dev/null || true)" + hits_ids="$(git ls-files -z | xargs -0 grep -nIE "$PAT_IDS" 2>/dev/null || true)" + + if [ -n "$hits" ] || [ -n "$hits_ids" ]; then + err "private markers found in tracked files" + [ -n "$hits" ] && printf '%s\n' "$hits" + [ -n "$hits_ids" ] && printf '%s\n' "$hits_ids" + fail=1 + else + note "files: clean" + fi +} + +# --- commit message scan ----------------------------------------------------- +# Commit messages never appear in a file diff, so they are the easiest place for +# a marker to survive review untouched. +scan_commits() { + local range="$1" msgs + if ! git rev-parse --quiet --verify "${range%%..*}" >/dev/null 2>&1; then + note "commits: range '$range' unavailable, scanning HEAD only" + msgs="$(git log -1 --format='%B' 2>/dev/null || true)" + else + msgs="$(git log "$range" --format='%H%n%B' 2>/dev/null || true)" + fi + + if [ -z "$msgs" ]; then + note "commits: no commit messages in range" + return + fi + + if printf '%s' "$msgs" | grep -qiE "$PAT_NAMES" || printf '%s' "$msgs" | grep -qE "$PAT_IDS"; then + err "private markers found in commit messages in range $range" + printf '%s' "$msgs" | grep -niE "$PAT_NAMES" || true + printf '%s' "$msgs" | grep -nE "$PAT_IDS" || true + fail=1 + else + note "commits: messages clean" + fi + + if printf '%s' "$msgs" | grep -qE "$ATTRIB"; then + err "attribution trailer found in a commit message in range $range" + fail=1 + else + note "commits: no attribution trailer in messages" + fi +} + +# --- authorship scan --------------------------------------------------------- +# A clean message check is necessary but not sufficient. A squash merge can +# synthesise an attribution trailer server-side from the AUTHORSHIP of the +# squashed commits, so a branch whose every message greps clean still produces +# one if an author differs from the merging identity. No local message hook can +# intercept that, because it never runs. +scan_authorship() { + local range="$1" ids + git rev-parse --quiet --verify "${range%%..*}" >/dev/null 2>&1 || return 0 + ids="$(git log "$range" --format='%an <%ae> | %cn <%ce>' 2>/dev/null | sort -u || true)" + if [ -n "$ids" ]; then + note "commits: distinct author|committer identities in range:" + printf '%s\n' "$ids" | sed 's/^/ /' + fi +} + +# --- main -------------------------------------------------------------------- +if [ "${1:-}" = "--self-test" ]; then + self_test || exit 1 + exit 0 +fi + +RANGE="${1:-origin/main..HEAD}" + +self_test || fail=1 +scan_files +scan_commits "$RANGE" +scan_authorship "$RANGE" + +if [ "$fail" -ne 0 ]; then + err "private-marker check FAILED. Describe what is true about this package; do not name where else it is used or what internal work motivated a change." + exit 1 +fi + +note "private-marker check passed." diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index e75067d..fef1f1c 100755 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -24,6 +24,10 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + # Full history so the private-marker check can scan commit messages + # and authorship across the range under review, not just the tip. + fetch-depth: 0 - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4 @@ -39,6 +43,54 @@ jobs: env: CI: true + # `lib/` is committed and `exports` points at it, and there is no `prepare` + # script — so installing this package straight from git performs no build, + # and consumers execute the committed output. That makes `lib/` the code + # that runs while `src/` is the code that gets reviewed. Without this check + # a stale `lib/` passes CI green, and an approved change to `src/` silently + # never reaches consumers: the diff looks perfectly correct, because `src/` + # is exactly what it claims to be. Nothing in the review surface can expose + # that, which is why it is enforced here rather than left to reviewers. + # + # `git status --porcelain` is used deliberately instead of the more obvious + # `git diff --exit-code`. `git diff` only sees TRACKED files, so a NEW + # module added under `src/` compiles to UNTRACKED files under `lib/` and + # `git diff` exits 0 — measured, not assumed. That is precisely the case + # that arises whenever someone adds a module, so the obvious form would + # have been inert exactly when it was most needed. + - name: Verify committed build output is current + run: | + drift="$(git status --porcelain -uall -- lib/)" + if [ -n "$drift" ]; then + echo "::error::lib/ is stale. Run 'npm run build' and commit the regenerated lib/ in the SAME commit as your src/ change." + echo "Drift between the committed lib/ and a clean build of src/:" + echo "$drift" + echo "--- diff of tracked build output ---" + git --no-pager diff -- lib/ + exit 1 + fi + echo "OK: committed lib/ matches a clean build of src/." + env: + CI: true + + # This repository is public. Scans tracked files, commit messages and + # authorship. Commit messages are included because they never appear in a + # file diff and are therefore the vector least likely to be caught by + # review. The script self-tests its own patterns on every run, so a pass + # means the patterns were demonstrated working rather than merely trusted. + - name: Check for private markers and attribution trailers + run: | + BASE="${{ github.event.pull_request.base.sha || github.event.before }}" + if [ -n "$BASE" ] && git rev-parse --quiet --verify "$BASE" >/dev/null 2>&1; then + RANGE="$BASE..HEAD" + else + RANGE="origin/${{ github.event.repository.default_branch }}..HEAD" + fi + echo "Scanning range: $RANGE" + ./.github/scripts/check-private-markers.sh "$RANGE" + env: + CI: true + - name: Test run: npm test env: From 9963256fa5f7d48accf22518b8ad52653da0c7d8 Mon Sep 17 00:00:00 2001 From: Erny Sans Date: Sat, 22 Aug 2026 16:58:34 -0500 Subject: [PATCH 2/4] docs: add instructions directory; repair the type gate; add zod Adds .github/instructions/ with six task-scoped guides and rewrites .github/copilot-instructions.md to point at them. The most important content is the public-repository constraint. The "private" flag in package.json means "do not publish to the npm registry" and says nothing about visibility; this repository is public while the services consuming it are not. Every guide leads with that, because the leak vector is code comments, fixtures, commit messages and PR bodies rather than documentation alone. Repairs the type gate, which was inert in two separate ways: - tsconfig.test.json could not resolve the vitest globals types because typeRoots was narrowed to node_modules/@types by the base config, so `tsc -p tsconfig.test.json` failed on a clean tree and had evidently never run. Widening typeRoots fixes it. - `vitest run --typecheck` is not a substitute: its typecheck.include defaults to **/*.test-d.ts and this project has no such files, so it checked zero files and reported "no errors" regardless. This matters because the runtime suite cannot fail on a type change -- interfaces are erased, so a test that declares a literal and reads a property back still passes once the field is deleted from the source. Deleting a field outright left 480/480 tests green. The repaired typecheck catches that, a wrong-typed assignment, and a removed enum member. Exposed as `npm run typecheck` and gated in CI. Adds zod as a runtime dependency for the schema layer that will follow, and pins the existing git dependency to the exact commit the lockfile already resolved. That spec carried no ref, so it floated to whatever the default branch pointed at on any plain `npm install`; the lockfile hid this from CI, which runs `npm ci`. The pin records existing behaviour -- the resolved commit is unchanged. --- .github/copilot-instructions.md | 75 ++++- .../instructions/cross-repo.instructions.md | 284 ++++++++++++++++++ .../documentation.instructions.md | 87 ++++++ .github/instructions/readme.instructions.md | 63 ++++ .github/instructions/security.instructions.md | 175 +++++++++++ .../serialized-models.instructions.md | 128 ++++++++ .github/instructions/tests.instructions.md | 125 ++++++++ .github/workflows/nodejs.yml | 12 + package-lock.json | 12 +- package.json | 4 +- tsconfig.test.json | 4 + 11 files changed, 956 insertions(+), 13 deletions(-) create mode 100644 .github/instructions/cross-repo.instructions.md create mode 100644 .github/instructions/documentation.instructions.md create mode 100644 .github/instructions/readme.instructions.md create mode 100644 .github/instructions/security.instructions.md create mode 100644 .github/instructions/serialized-models.instructions.md create mode 100644 .github/instructions/tests.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d07286e..75349b8 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -6,8 +6,38 @@ > `package.json`, `tsconfig.json`, `tsconfig.test.json`, `eslint.config.js`, `vitest.config.ts`, > and `.github/workflows/nodejs.yml` of *this* repository — not from boilerplate. +> ## 🔴 THIS IS A PUBLIC REPOSITORY +> +> `"private": true` in `package.json` means **"never publish to the npm registry."** It says +> **nothing** about GitHub visibility, and misreading it as though it did is the single most +> expensive mistake available in this repository. Source, build output, commit messages, pull +> request titles and bodies are all world-readable. +> +> This package is consumed by services that are **not** public. **Describe what is true about this +> package. Never name where else it is used, never name environments or internal identifiers, and +> never describe an unfixed weakness in another system.** References may flow private → public, +> never the reverse. +> +> This applies to code comments, config comments, test fixtures, commit messages and PR +> descriptions — not just documentation. Enforced by +> `.github/scripts/check-private-markers.sh` in CI; run it locally before pushing. + +## 📁 Detailed instructions live in `.github/instructions/` + +This file is the global summary. The detailed, task-scoped rules are: + +| File | Covers | +|---|---| +| [`cross-repo.instructions.md`](instructions/cross-repo.instructions.md) | **Read first.** Public-repo rules, the trust model, why types are a security control, the committed-output hazard, evidence standards, agent conduct. | +| [`security.instructions.md`](instructions/security.instructions.md) | This repo's actual measured state, sweep recipes with positive controls, and what is deliberately left alone. | +| [`serialized-models.instructions.md`](instructions/serialized-models.instructions.md) | Model/interface conventions. **This package *is* the serialized model layer.** | +| [`tests.instructions.md`](instructions/tests.instructions.md) | Vitest conventions, and why `npm test` alone cannot fail on a type change. | +| [`documentation.instructions.md`](instructions/documentation.instructions.md) | JSDoc conventions. | +| [`readme.instructions.md`](instructions/readme.instructions.md) | README / CONTRIBUTING maintenance. | + --- + ## 1. Project Stack Reality & Workspace Bounding ### Core Environment (detected in this repository) @@ -21,24 +51,37 @@ | **Linting** | `eslint@^10` flat config (`eslint.config.js`) using `typescript-eslint@^8`, composing `eslint.configs.recommended` + `tseslint.configs.recommended` + `tseslint.configs.stylistic`. Scoped to `src/**/*.ts`. `max-len` is `200` (`ignoreComments`, `ignoreUrls`). | | **Testing** | `vitest@^4` in the `node` environment, with type-checking via `tsconfig.test.json`. | | **Cloud / Firebase context** | This package targets **Firebase Cloud Functions** and **Firestore** as its *consumers*. There is intentionally **no** `firebase.json`, emulator config, or Cloud Functions runtime in this repo, and **no** `firebase-admin` / `firebase-functions` dependency installed here. Firebase is downstream context, not a local dependency — do not add Firebase packages unless the task explicitly requires it. | -| **Sole runtime dependency** | `@fabricelements/shared-helpers` (e.g. the `User` type used by the `Account` model). | +| **Runtime dependencies** | `@fabricelements/shared-helpers` (public; e.g. the `User` type used by the `Account` model), pinned to an exact commit SHA — and `zod` `^4.4.3` for runtime schemas. | | **Public entry points** | `exports` map: `"./model" → "./lib/model/index.js"` and `"./interface" → "./lib/interface/index.js"`. | | **Source layout** | `src/model/` (entity namespaces: `Account`, `Block`, `EventData`, `MessagingEvent`, `Post`, `Price`) and `src/interface/` (`base_db.ts`, `queue.ts`, `place.ts`). Each folder has a barrel `index.ts`. | -### 🔴 CRITICAL `/lib` BLACKLIST +### 🔴 CRITICAL `/lib` RULES — it is COMMITTED, and consumers execute it + +`/lib` is an auto-generated build target produced exclusively by the TypeScript compiler +(`tsc`, `outDir: lib`), wiped and regenerated on every build by the `clear` script (`rm -rf ./lib`). -`/lib` is an **immutable, auto-generated build target**. It is produced exclusively by the -TypeScript compiler (`tsc`, `outDir: lib`) and is wiped and regenerated on every build by the -`clear` script (`rm -rf ./lib`). +**But it is also committed to git — 22 tracked files, not ignored — and it is what consumers +actually run.** `package.json` `exports` points straight at `./lib/model/index.js` and +`./lib/interface/index.js`, and there is **no** `prepare`/`prepack` script, so installing this +package directly from git performs **no build**. + +> **Consequence: reviewers read `src/`, consumers execute `lib/`. Those are different files.** +> A change to `src/` can be authored, reviewed, approved and merged and still never run, because +> the compiled output was never regenerated. The PR diff looks perfectly correct — `src/` is +> exactly what it claims to be — so nothing in the review surface can expose it. **System mandate — every AI agent MUST obey all of the following:** - **NEVER read or take context from `/lib`.** It is generated output and is not a source of truth. Use `src/` for all understanding. -- **NEVER edit, create, or delete any file inside `/lib`.** This includes every `.js` and `.d.ts` file there. -- **NEVER direct, suggest, or apply modifications to `/lib`.** Any change there is silently overwritten on the next compile. +- **NEVER edit, create, or delete any file inside `/lib` by hand.** Any such change is destroyed by the next build. - **NEVER edit compiled `.js` artifacts anywhere.** All development happens exclusively in `.ts` source files under `src/`. -- The local `/lib` build is updated **solely** by running `npm run build`. To change runtime behaviour, edit the matching `.ts` source in `src/` and recompile. -- Tooling already enforces this boundary: ESLint ignores `lib/*` (alongside `node_modules/*`, `.github/*`, `functions/*`). +- **After ANY change to `src/`, run `npm run build` and commit the regenerated `/lib` in the SAME + commit.** A source change and its compiled output are one atomic unit. Splitting them lets a + partial landing leave consumers executing code nobody reviewed. +- **Verify before you push:** `git status --porcelain -- lib/` must be empty after a build. CI + enforces this and fails on drift. +- Tooling already enforces the read boundary: ESLint ignores `lib/*` (alongside `node_modules/*`, `.github/*`, `functions/*`). + --- @@ -149,8 +192,18 @@ Any update to the root `README.MD` must: | Build (watch) | `npm run build:watch` | | Compile only | `npm run compile` (`tsc -p ./tsconfig.json`) | | Test (CI mode) | `npm test` (`vitest run`) | +| **Typecheck (required)** | `npm run typecheck` (`tsc -p ./tsconfig.test.json`) | | Test (direct / watch / coverage) | `npx vitest run` · `npx vitest` · `npx vitest run --coverage` | +| Private-marker check | `./.github/scripts/check-private-markers.sh` | +| Build-output drift check | `npm run build && git status --porcelain -- lib/` (must be empty) | + +> **`npm test` and `npm run typecheck` check different things and both are required.** TypeScript +> interfaces are erased at runtime, so the Vitest suite **cannot fail** on an interface change: +> deleting a field outright from `src/interface/queue.ts` leaves all 480 tests passing. Type-level +> regressions are caught only by `npm run typecheck`. Note also that `npx vitest run --typecheck` +> is **not** the gate — Vitest's `typecheck.include` defaults to `**/*.test-d.ts`, and this repo +> has none, so it checks zero files and always reports "no errors". > **CI gate:** `.github/workflows/nodejs.yml` runs on `push`/`pull_request` to `main` across Node -> `22.x` and `24.x`, executing `npm ci` → `npm run build` → `npm test`. Changes must keep all of -> these green. +> `22.x` and `24.x`, executing `npm ci` → `npm run build` → build-output drift check → +> private-marker check → `npm test` → `npm run typecheck`. Changes must keep all of these green. diff --git a/.github/instructions/cross-repo.instructions.md b/.github/instructions/cross-repo.instructions.md new file mode 100644 index 0000000..93f1265 --- /dev/null +++ b/.github/instructions/cross-repo.instructions.md @@ -0,0 +1,284 @@ +--- +description: Security & agent playbook for this shared public type package — trust model, evidence standards, agent conduct. +applyTo: "**" +--- + +> **Adapted from the canonical FabricElements public playbook.** The generic security +> engineering below is replicated across public packages; the sections describing *this* +> package's build and consumption model are specific to this repository. Fix generic rules at +> the canonical source and re-replicate rather than editing only this copy. + +--- + +# Security & Agent Playbook — `@furcata/core-node` + +**Read this before making any change.** It applies to AI agents and to humans. + +> ## 🔴 THIS IS A PUBLIC REPOSITORY +> +> `"private": true` in `package.json` means **"never publish to the npm registry."** It says +> **nothing** about GitHub visibility, and it is easy to misread as though it did. This +> repository is public: its source, its build output, its commit messages, its pull request +> titles and bodies, and its issue threads are all world-readable. +> +> This package is consumed by services that are **not** public. Their operational details must +> never appear here: +> +> - **never name a non-public consumer repository**, or quote a line from its manifest; +> - **never name environments, project identifiers, service accounts or internal collection names**; +> - **never reference an internal tracker or finding identifier**; +> - **above all, never describe an unremediated weakness in a non-public system.** That is not a +> note for colleagues, it is a public disclosure of a live vulnerability with the exploitation +> path attached. +> +> **Describe what is true about *this package*. Never where else it is used, and never what +> internal work motivated a change.** If a finding is really about a consumer, hand it to a +> maintainer to route privately — do not write it down here, not even in a commit message. +> +> The leak vector is not only documentation. It is every code comment, config comment, test +> fixture, commit message and pull request description. The realistic failure is not malice: it +> is someone writing a technically correct note in good faith without knowing the repository is +> public. `.github/scripts/check-private-markers.sh` enforces this in CI, but a check that has to +> fire has already been written — the habit is the real control. +> +> References may flow **private → public**. Never the reverse. + +--- + +## 1. What this package is, and why that makes types a security control + +This package ships **types**: namespaces of interfaces and enums under `src/model/`, and shared +interfaces under `src/interface/`. It contains no runtime logic, performs no I/O, and makes no +network calls. It is tempting to conclude that it therefore has no security surface. That +conclusion is wrong, and the reasoning is worth stating plainly: + +> **A type here is the shape that a consumer's backend is written against.** It is the thing +> reviewers reason about, the thing autocompletion suggests, and the thing a validation layer is +> generated from. A permissive type does not merely fail to catch a bug — it actively tells every +> consumer that the permissive shape is legitimate. + +Three consequences follow. + +- **A widened type is a widened trust boundary.** `field?: any` does not describe a field whose + type is unknown; it describes a field about which no consumer can be warned. Every downstream + `if` that would have been a compile error becomes a runtime possibility nobody wrote a branch + for. +- **`T | any` collapses to `any`.** TypeScript absorbs the union: `Type | any` *is* `any`, so a + discriminated enum annotated that way silently stops discriminating while still reading like it + constrains something. This is worse than a bare `any`, because it looks careful. + Use `T | string` when a raw stored value must genuinely be tolerated. +- **An index signature is an unbounded accept.** `[x: string]: any` on a base document interface + means no extra field is ever a type error, anywhere, for anyone. That may be a deliberate and + correct choice for a sparse document store — but it is a decision with a blast radius, and it + must be a decision rather than an accident. + +### The rule that follows + +> **Model the narrowest shape the domain genuinely permits, and widen deliberately with a comment +> saying why.** Narrow-by-default is not pedantry here: this package's whole job is to be the +> contract, and a contract that permits everything is not a contract. + +Balance this against the regression rule in §4: narrowing an *existing* published type is a +breaking change for consumers. Widen inputs freely; narrow only with intent and a version bump. + +--- + +## 2. The build output is the code that runs + +This repository has an unusual and load-bearing property that every contributor must know: + +- `lib/` is **committed to git** — it is generated by `tsc`, but it is tracked, not ignored. +- `package.json` `exports` points directly at `./lib/model/index.js` and `./lib/interface/index.js`. +- There is **no** `prepare`, `prepack`, `prepublishOnly` or `postinstall` script. + +Installing this package directly from git therefore **performs no build**. Consumers execute the +**committed `lib/`**, while reviewers read **`src/`**. Those are different files. + +> 🔴 **This is the most dangerous property of this repository.** A change to `src/` can be +> authored, reviewed, approved and merged, and still never run — because the compiled output that +> consumers actually execute was never regenerated. The pull request diff looks perfectly correct, +> because `src/` is exactly what it claims to be. **Nothing in the review surface can expose it.** +> +> It is a sharper instance of the inert-control problem in §5: the usual inert control is missing +> an external precondition, but here the *reviewed artifact and the executed artifact are +> different files*. + +### The rules that follow + +- **`src/` is the only editable source.** Never hand-edit anything under `lib/`. Any edit there is + destroyed by the next build, because `npm run build` begins with `clear` (`rm -rf ./lib`). +- **Never read `lib/` as a source of truth.** Use `src/` for all understanding. +- **After any change to `src/`, run `npm run build` and commit the regenerated `lib/` in the SAME + commit.** A source change and its output belong to one atomic unit. Splitting them lets a + partial landing leave consumers executing code nobody reviewed. +- CI enforces this: a build step regenerates `lib/` and fails if it differs from what was + committed. The check uses `git status --porcelain`, not `git diff --exit-code`, because + `git diff` sees only *tracked* files — a newly added module compiles to *untracked* output and + would slip through the obvious form of the check. + +--- + +## 3. Security rules for a shared type package + +### 3.1 Prefer precise shapes to escape hatches +No banned types (`Function`, `Object`, bare `{}`). Prefer a concrete interface, +`Record`, or `unknown`. `unknown` forces the consumer to narrow; `any` forces +nothing and silently disables checking at every call site downstream. + +### 3.2 Model absence honestly +Stored documents are sparse, so optional (`?`) is usually right for persisted shapes. But +"optional" and "nullable" are different claims: use `T | null` when the field is explicitly +absent in storage and must survive a JSON round-trip, and `?` when it may simply not be present. +Document the meaning and the default per field. + +### 3.3 Keep the models transport-agnostic +`src/model/` and `src/interface/` are pure data contracts. Do not embed trigger wiring, side +effects, I/O, credentials, environment lookups or business logic. A type package that reaches out +at runtime becomes a dependency that every consumer must audit. + +### 3.4 Never encode a secret, identifier or environment detail in a type +Enum members, default values, JSDoc examples and test fixtures are all published. An example +value that looks realistic is a leak if it *is* realistic. Use obviously synthetic values. + +### 3.5 Validation belongs at the boundary, and a type is not validation +A TypeScript type is erased at runtime. It constrains what a consumer *writes*, never what +arrives. Where a runtime schema is added to this package, it must **reject unknown keys rather +than silently dropping them**: a silent drop leaves the caller believing their input was honoured +and the recipient believing it was filtered. + +### 3.6 Dependencies are part of the contract +A dependency of this package becomes a transitive dependency of every consumer. + +- **Pin what you depend on.** A git dependency written without a `#ref` floats to whatever the + default branch happens to be at install time. The lockfile masks this: `npm ci` installs the + locked commit and CI stays green, so nothing surfaces the float until someone runs + `npm install` and silently moves. +- **Prefer a resolvable, public transport.** A dependency resolved over SSH requires every + consumer and every CI runner to hold a key. +- Re-run `npm audit` when changing dependencies, and record what you could not fix and why. + +--- + +## 4. Breaking-change discipline + +This package ships `.d.ts` declarations and its consumers compile against them, so its types are a +published contract. + +- **Additive changes are safe:** new **optional** fields, new enum members appended, new exports. +- **Never remove or rename a published field, type or export casually.** Keep it, mark it + `@deprecated` naming the replacement, and add the new field alongside. +- **Widen inputs, keep outputs stable.** Narrowing a union is breaking for readers; widening one + is breaking for writers. Know which side of the contract you are moving. +- **A change that removes capability is a REGRESSION, even when the motive is security.** A closed + grammar, allow-list or schema must be **complete, not minimal**. The risk comes from accepting + *arbitrary* input, not from the *number* of legitimate options. Shrinking the option set is a + product change wearing a security costume. + +--- + +## 5. Evidence standards + +### 5.1 The citation rule + +> **A negative claim needs a citation to the artifact that would have contained the positive.** +> A `file:line`, a test assertion, a command's exit code, a query result. +> *"I looked and didn't see it"* is not a citation. + +At review time it reduces to one question: *what would this have shown if the thing existed, and +did you actually look at that?* The recurring failure is **inferring absence from an incomplete +source** — a grep that structurally cannot match the pattern being hunted, an ignore rule that +does not apply to already-tracked files, a stale ref, or memory of a file instead of the file. + +### 5.2 Every probe needs a positive control + +> **A probe that cannot demonstrate it can return a true positive is not evidence.** It produces a +> confident, well-formed argument for the wrong action. + +Before trusting a zero, make the probe report a one. Point the same grep at a synthetic string +that *must* match; inject the thing you are checking for into a scratch copy and confirm +detection. A probe that comes back negative in **both** the positive and the negative case has no +control and is inconclusive — which is easy to misread as confirmation. + +This applies to investigative probes, not only to committed tests. + +### 5.3 A green suite can be vacuous + +A test asserting *"X is rejected"* passes vacuously if a change removed the precondition that made +X reachable. **Prove a test can fail:** mutate the thing it checks, confirm red, revert. Report +the mutations. A suite nobody has watched fail is a hypothesis about a suite. + +This matters more than usual here, because much of what this package asserts is structural. A test +that only reads properties off an object literal it declared itself in the same file can be +incapable of failing for any interesting reason. + +### 5.4 Prove before/after, don't assert it +Run the identical probe against the pre-change state and the post-change state and report both. A +measured transition is evidence; an assertion that a fix works is not. + +### 5.5 Record negative results +A refuted finding is a real deliverable. Disproving a suspected problem prevents wasted effort and +wrong fixes. Never quietly drop a claim you disproved — write down what you checked and why it was +clean. + +### 5.6 Order staged work so a partial landing is inert, not misleading +Code first, then the documentation describing it. If docs land first, the repository carries +instructions describing a control it does not yet have — the failure mode nobody notices later, +because a reader trusts the documented behaviour and the behaviour is absent. +**False confidence is worse than a documented gap:** a documented gap gets fixed, a false one gets +relied upon. + +--- + +## 6. Working as an agent in this repository + +- **One session ≈ one branch ≈ one PR.** Scope to a single unit of work. +- **Assign file ownership explicitly** when several sessions edit this repo in parallel, and state + which paths are off-limits. +- **Push back on instructions that are wrong.** Treat a coordinator's suggestion that touches a + security invariant as a *question about the invariant* rather than an instruction — the question + form is self-cancelling when it turns out to be wrong. Some of the most valuable outcomes come + from a session declining a request and explaining why. +- **State-dependent claims carry an implicit timestamp.** A hash, a test count, a dependency + version or a merge check must be re-measured, not re-quoted. +- **A control can exist, pass its tests, and still be inert** because an **external precondition + owned by someone else** is missing. For any control you ship, name what must be true *outside + this repository* for it to actually run, and check that too. Beware best-effort paths that + degrade to a warning: their failure is indistinguishable from "nothing to do". +- **An exempted path is a blind spot.** If a check must skip something, make the exemption as + narrow as possible, document it where a reader will meet it, and prefer a design with no + exemption at all. +- **A green pipeline cannot see a functionality regression.** Every automated gate is code-facing. + When a change **narrows** an interface, the test that matters asserts what must *still* be there + — an **inventory test**, the capability equivalent of a positive control. +- **A change owns the failure paths it makes more frequent, even ones it never edited.** If a + change converts a rare error path into a routine one, a latent bug on that path becomes an + active one. A diff-based review structurally cannot surface this, because the offending code is + unchanged and therefore absent from the diff. +- **Never add co-authorship attribution.** No `Co-authored-by:` trailer on any commit, pull request + or merge, regardless of tooling defaults. A rebase, amend or squash re-creates commit messages, + so re-check after one: + `git log ..HEAD --format='%B' | grep -ci 'co-authored-by'` must print `0` — and + positive-control that grep, because a zero from an untested probe is not evidence. + **A clean message check is necessary but NOT sufficient.** A squash merge generates + `Co-authored-by:` trailers **server-side, from the authorship of the squashed commits**. A branch + whose every message greps clean still produces a merge commit carrying the trailer if any + commit's *author* differs from the merging identity, and a local `commit-msg` hook cannot + intercept it — it never runs. So also assert authorship: + `git log ..HEAD --format='%an|%cn'` must show only expected identities. + +--- + +## 7. Before you merge + +- [ ] Did you change `src/`? Then run `npm run build` and commit the regenerated `lib/` **in the + same commit**. +- [ ] `npm run build`, `npm test` and `npm run lint` all pass — exit codes observed, not assumed. +- [ ] Does any new or changed type use `any`, `T | any`, `Function`, `Object` or bare `{}`? Justify + it in a comment or narrow it. +- [ ] Are you **narrowing** an existing published type? That is a breaking change for consumers. +- [ ] Do new tests have positive controls, or could they be passing vacuously? Did you watch one + fail? +- [ ] Does anything you wrote — code, comment, fixture, commit message, PR title, PR body — name a + non-public consumer, an environment, an internal identifier, or an unfixed weakness + elsewhere? +- [ ] No `Co-authored-by:` trailer, checked in **both** the messages and the authorship. diff --git a/.github/instructions/documentation.instructions.md b/.github/instructions/documentation.instructions.md new file mode 100644 index 0000000..5010157 --- /dev/null +++ b/.github/instructions/documentation.instructions.md @@ -0,0 +1,87 @@ +--- +description: JSDoc conventions for the model and interface sources. +applyTo: "src/**/*.ts" +--- + +# Documentation Instructions — `@furcata/core-node` + +This package ships `.d.ts` declarations, so **its JSDoc is the documentation consumers read** in +their editor. A field whose meaning is only obvious from its name is undocumented. + +--- + +## 1. Format + +- **Block comments only.** Use `/** … */` for every namespace, enum, enum member, interface and + property. Never use `///` triple-slash or a `//` line as the doc comment. Plain `//` is fine for + an incidental trailing note (`area?: string; // AKA: region`), never as the documentation itself. +- **Summary sentence first**, capitalised, ending in a period. +- **License header** at the top of every file, in the established form: + + ```ts + /** + * @license + * Copyright Furcata. All Rights Reserved. + */ + ``` + +- **`@return`, not `@returns`.** `eslint.config.js` sets the JSDoc `tagNamePreference` to rewrite + `returns` → `return`. +- `max-len` is 200 with `ignoreComments` and `ignoreUrls`, so long prose and long URLs are allowed. + +--- + +## 2. Content + +- **Document the *why*, not the syntax.** `/** The account id. */` on `accountId` adds nothing. + Say what it points at, who writes it, and what happens when it is absent. +- **State units and formats explicitly.** Minutes vs seconds, ISO 8601 vs epoch, decimal degrees, + minor currency units, `[longitude, latitude]` ordering. These are the details that cause real + defects and they are invisible in the type. +- **Say who owns the field.** Server-authored, client-supplied, or derived. For a package whose + whole purpose is to describe stored documents, provenance is the most valuable thing the comment + can carry. +- **Document what "absent" means.** Optional (`?`) and nullable (`| null`) are different claims; + say which applies and what the reader should assume when the value is missing. +- **Cross-reference with `{@link Name}`** to connect an interface to the enum that constrains it. +- **Never alter existing links or URLs.** Do not shorten, "tidy" or strip markdown links, + `{@link …}` references or external URLs in existing comments. + +--- + +## 3. Types in documentation + +- Preserve any existing `@param {type}` structure exactly. +- If a parameter has no documented type, add the one that matches the TypeScript declaration. +- **Never document a banned type.** No `Function`, no `Object`, no bare `{}`. Use a concrete + signature, a precise interface, `Record` or `unknown`. +- Where a field is deliberately permissive, the comment must **say why**, because the type no + longer explains itself. A bare `any` with no rationale is indistinguishable from an oversight — + and this repository has fields where the permissiveness is a considered decision, so the comment + is what preserves that distinction for the next reader. + +--- + +## 4. Deprecation + +Consumers compile against these declarations, so removal is a breaking change. To retire a field: + +```ts +/** @deprecated Use `startTime` instead; removed in the next major. */ +start?: string; +``` + +Keep the old field, mark it, add the replacement alongside, and let a major version remove it. + +--- + +## DO NOT + +- ❌ Write a doc comment that restates the field name. +- ❌ Use `///` or `//` as the documentation form. +- ❌ Drop or rewrite an existing URL or `{@link …}`. +- ❌ Document a type as `Function`, `Object` or `{}`. +- ❌ Put a realistic identifier, key, endpoint or internal collection name in an example — this + repository is public, and an example that looks real is a leak if it is real. Use obviously + synthetic values. +- ❌ Leave a permissive type undocumented. diff --git a/.github/instructions/readme.instructions.md b/.github/instructions/readme.instructions.md new file mode 100644 index 0000000..7038e3e --- /dev/null +++ b/.github/instructions/readme.instructions.md @@ -0,0 +1,63 @@ +--- +description: Rules for maintaining README.MD and CONTRIBUTING.md in this public repository. +applyTo: "README.MD,CONTRIBUTING.md,*.md" +--- + +# README Instructions — `@furcata/core-node` + +The README is the first thing a contributor and a consumer read, and — because this repository is +public — it is also the most widely read file in it. + +--- + +## 1. Public-repository constraint + +Everything in §"THIS IS A PUBLIC REPOSITORY" of +[`cross-repo.instructions.md`](cross-repo.instructions.md) applies with full force here, because +prose is where it is most tempting to be helpful: + +- Describe **what this package is and how to work on it**. +- Do **not** name non-public consumer repositories, environments, project identifiers, service + accounts, internal collection names, tracker IDs, or any unremediated weakness elsewhere. +- It is fine to say the package is consumed by other services. It is not fine to say **which**. + +--- + +## 2. What the README must cover + +- **What this package is** — a shared TypeScript type package: Firestore document shapes, enums + and queue contracts. Not a deployable service; it has no runtime logic and no I/O. +- **Architecture** — the `src/model/` and `src/interface/` split, the two entry points + (`./model`, `./interface`), and the `src/` → `lib/` build relationship. +- **🔴 The committed build output.** `lib/` is committed and consumers execute it; there is no + `prepare` script. Any change to `src/` requires `npm run build` and the regenerated `lib/` + committed **in the same commit**. This must be prominent, not a footnote — it is the single + easiest way for a well-reviewed change to silently not take effect. +- **Tech stack** — Node `>=22`, TypeScript `^6` (ESM, `Node16` resolution, `ES2020` target), + ESLint `^10` flat config, Vitest `^4`, Zod for runtime schemas. +- **Commands** — install, lint, build, test, **typecheck**, and the private-marker check. + `npm test` and `npm run typecheck` must both be listed, with a note that they check different + things: the suite cannot fail on an erased type. +- **Repository URL** — `https://github.com/furcata/core-node.git`. + +## 3. What CONTRIBUTING.md must cover + +- The `src/`-only editing rule and the rebuild-and-commit-output requirement. +- The full local verification sequence, in the order CI runs it. +- The evidence expectations: positive controls on probes, and mutation-proof for tests. +- The no-attribution-trailer rule, checked in **both** commit messages and authorship. +- The public-repository constraint on comments, commit messages and PR descriptions. + +--- + +## 4. Style + +- Keep existing external links, badges and the repository URL intact — never "tidy" a URL away. +- Prefer a table for commands; keep every command copy-pasteable and **verified against + `package.json`** rather than remembered. A README that documents a script that does not exist is + worse than one that documents nothing. +- Where a rule exists because of a real hazard, state the hazard in one sentence. A rule with a + reason survives; a bare prohibition gets worked around. +- Do not document a control that is not yet in place. Land the code first, then the prose (see + §5.6 of the cross-repo playbook) — otherwise the repository advertises a guarantee it does not + provide, and a reader acts on it. diff --git a/.github/instructions/security.instructions.md b/.github/instructions/security.instructions.md new file mode 100644 index 0000000..f1690ab --- /dev/null +++ b/.github/instructions/security.instructions.md @@ -0,0 +1,175 @@ +--- +description: Repository-specific security rules, current known state, and sweep recipes for @furcata/core-node. +applyTo: "src/**/*.ts,test/**/*.ts,package.json,.github/**" +--- + +# Security Instructions — `@furcata/core-node` + +Companion to [`cross-repo.instructions.md`](cross-repo.instructions.md), which holds the generic +playbook and the public-repository rules. This file holds what is true about **this** repository: +its actual current state, the sweeps that establish it, and the decisions behind what is +deliberately left alone. + +> **This repository is public.** Everything below describes this package only. Do not add findings +> about consumers here — see the boxed rule at the top of the cross-repo playbook. + +--- + +## 1. Threat model in one paragraph + +This package has no runtime logic, no I/O and no network access, so it cannot itself be exploited +at runtime. Its security surface is **the contract it publishes**: the types every consumer +compiles against, the build output consumers actually execute, and the dependency closure it drags +into every consumer. Attacks land through *permissiveness* (a type that stops warning anyone), +through *supply chain* (an unpinned dependency, a stale committed artifact), and through +*disclosure* (this repository is public and its consumers are not). + +--- + +## 2. Current known state + +Measured on this branch. **Re-measure rather than re-quoting** — every figure here carries an +implicit timestamp, and a claim about a dependency or a count is stale the moment something moves. + +### 2.1 Type permissiveness + +| Class | Count in `src/` | Status | +|---|---|---| +| `T \| any` union | **0** | Clean. A previous change removed these; verified not regressed. | +| bare `any` | **8** | Known and accepted for now — see below. | +| `Function` / `Object` / bare `{}` | **0** | Clean. | +| spread after a literal key | **0** | Not applicable — this package has no object literals. | +| `eval` / `new Function` | **0** | Clean. | +| `process.env` access | **0** | Clean — the package reads no environment. | +| secret-like field names | **0** | Clean. | + +The 8 bare `any` are **not** arbitrary. Six are timestamp fields +(`base_db.ts` `created`/`updated`/`expiry`, `Account.ts` `domainTimestamp`, +`EventData.ts` `startTime`/`endTime`), whose honest type is a three-way union of "read value", +"write sentinel" and "serialized string". Writing that precisely requires the server SDK's +`FieldValue` type, which **is not a dependency of this package and should not become one** — a +pure type package should not pull a server SDK into every consumer's closure. The remaining two +are `queue.ts` `counted` (an untyped diagnostic snapshot) and the deliberate catch-all index +signature `base_db.ts` `[x: string]: any`. + +> **Do not "fix" these by reflex.** Narrowing a published type is a breaking change for consumers +> (§4 of the cross-repo playbook), and the index signature is a deliberate choice for a sparse +> document store. The right resolution is a runtime schema layer that validates these shapes at +> the boundary, where the constraint can be enforced rather than merely asserted. Until then they +> are documented, not hidden. + +### 2.2 Tooling that does not enforce what it appears to + +Three settings mean the compiler and linter are **more permissive than they look**. This is +recorded so nobody mistakes a green build for a strictness guarantee: + +- `eslint.config.js` sets `@typescript-eslint/no-explicit-any: ['off']` — an explicit `any` is + **not** a lint error here. +- `tsconfig.json` sets `"strict": true` but then **overrides two of its members**: + `"noImplicitAny": false` and `"strictNullChecks": false`. `strict: true` is not the final word; + the later, narrower keys win. +- `eslint.config.js` `files` is scoped to `src/**/*.ts`, so **`test/**` is not linted**. + +Any claim that "strict mode would have caught it" must be checked against these three lines first. + +### 2.3 Dependencies + +- The single runtime dependency is a **public** package, declared as a `github:` dependency + **without a `#ref`**. It therefore floats to whatever the default branch points at when someone + runs `npm install`. `package-lock.json` pins the resolved commit, so `npm ci` — which is what CI + runs — is reproducible and green. **That is exactly what makes the float easy to miss:** nothing + surfaces it until a plain `npm install` silently moves the dependency. +- The lockfile resolves that dependency over **SSH** (`git+ssh://`), which requires every consumer + and CI runner to hold a key with access. +- `npm audit` reports vulnerabilities that are **entirely transitive** through that one dependency + and through dev tooling. This package's own code introduces none. Several have no fix available + upstream, so they are recorded rather than suppressed. + +--- + +## 3. Sweep recipes + +Run these before claiming a class is clean. **Each one is written to be positive-controlled** — +run it against a synthetic file containing the defect first and confirm a non-zero result, because +a zero from an untested probe is not evidence. + +```bash +# Bare `any` in type position +grep -rEn ':[[:space:]]*any\b' src --include='*.ts' + +# `T | any` — collapses to `any` while looking constrained +grep -rEn '\|[[:space:]]*any\b|\bany[[:space:]]*\|' src --include='*.ts' + +# Banned escape-hatch types +grep -rEn ':[[:space:]]*(Function|Object)\b|:[[:space:]]*\{[[:space:]]*\}' src --include='*.ts' + +# Dynamic execution and environment reads (should be empty in a pure type package) +grep -rEn '\beval[[:space:]]*\(|new Function[[:space:]]*\(|process\.env' src --include='*.ts' +``` + +Positive control for any of the above: + +```bash +printf 'interface Z { a: any; b: Foo | any; c: Function; d: Object; e: {}; }\n' > /tmp/ctrl.ts +grep -Ec ':[[:space:]]*any\b' /tmp/ctrl.ts # MUST be non-zero before you trust a zero +``` + +Common way to get this wrong: `grep -rc` prefixes each line with a filename, so arithmetic on its +output silently misbehaves and every control appears to fail (or, worse, appears to pass). Use +`grep -Ec` on a single file, and read the control's output rather than only its exit status. + +--- + +## 4. Build-output integrity + +See §2 of the cross-repo playbook for the full explanation. Operationally: + +```bash +npm run build # clear -> lint -> compile; rm -rf ./lib happens first +git status --porcelain -- lib/ # MUST be empty; anything here is uncommitted drift +``` + +`git status --porcelain` rather than `git diff --exit-code`: `git diff` sees only tracked files, so +a newly added module under `src/` produces **untracked** files under `lib/` that `git diff` reports +as clean. Verified by adding a module and observing `git diff` exit `0` while two untracked output +files existed. + +CI enforces this after every build. + +--- + +## 5. Disclosure hygiene + +`.github/scripts/check-private-markers.sh` scans tracked files, commit messages and commit +authorship. Run it locally before pushing: + +```bash +./.github/scripts/check-private-markers.sh # defaults to origin/main..HEAD +./.github/scripts/check-private-markers.sh --self-test # prove the patterns still detect +``` + +It self-tests on every run, so a pass means the patterns were demonstrated rather than trusted. +**It holds no exemption for itself** — a marker written inside the script is caught like any other, +verified by planting one and watching the check cite the script's own line number. Keep it that +way: an exempted path is a place a real marker can hide. + +Commit messages are scanned because they never appear in a file diff, which makes them the vector +least likely to be caught by review. + +--- + +## 6. Deliberately not done + +Recorded because a refuted or deferred finding is a real deliverable, and a silently dropped one +becomes someone's rediscovery: + +- **The 8 bare `any` are left in place.** Rationale in §2.1: precise typing needs a server SDK type + this package must not depend on, and narrowing a published type breaks consumers. The resolution + is a runtime schema layer, not a type edit. +- **`noImplicitAny` / `strictNullChecks` are left `false`.** Flipping either is not a + documentation change — it is a compile-breaking change across every model, and it belongs in its + own reviewed unit of work with the resulting diff visible. +- **`@typescript-eslint/no-explicit-any` is left `off`.** Turning it on would fail the build on the + 8 known fields above before there is anywhere for them to go. +- **Transitive advisories with no upstream fix are not suppressed.** An `overrides` entry that + forces an unrelated version can break the consumer's runtime in a way this package cannot test. diff --git a/.github/instructions/serialized-models.instructions.md b/.github/instructions/serialized-models.instructions.md new file mode 100644 index 0000000..5e56df5 --- /dev/null +++ b/.github/instructions/serialized-models.instructions.md @@ -0,0 +1,128 @@ +--- +description: Conventions for the serialized model layer — this package IS that layer. +applyTo: "src/model/**/*.ts,src/interface/**/*.ts" +--- + +# Serialized Models Instructions — `@furcata/core-node` + +Most repositories have a serialized model layer somewhere inside them. **This repository *is* that +layer.** Everything in `src/model/` and `src/interface/` crosses a serialization boundary: these +types describe documents at rest in Firestore, payloads on a queue, and the `.d.ts` contract that +consumers compile against. + +That makes the rules below load-bearing rather than stylistic. A mistake here is not a local bug; +it is a wrong shape replicated into every consumer. + +--- + +## 1. Where models live + +- **Entity namespaces** → `src/model/` — `Account`, `Block`, `EventData`, `MessagingEvent`, + `Post`, `Price`. Each is a `namespace` containing its `Interface` and its associated enums. +- **Cross-cutting shared interfaces** → `src/interface/` — `BaseFirestore` (`base_db.ts`), + `MessageQueue` (`queue.ts`), place types (`place.ts`). +- Each folder has a barrel `index.ts` that re-exports with `export * from './X.js'`. +- **Never duplicate a model.** Import and reuse. A shape defined twice will drift, and the two + copies will disagree in production before anyone notices in review. + +--- + +## 2. Conventions + +- **Namespace-scoped naming.** The canonical entity per namespace is `Interface` + (`Account.Interface`). Enums sit beside it in the same namespace. +- **Fields are optional (`?`) by default** for stored entities. Firestore documents are sparse and + partially populated; a required field in the type is a promise the datastore does not keep. +- **`?` and `| null` mean different things.** Use `?` for "may not be present" and `| null` for + "explicitly absent and must survive a JSON round-trip" — `undefined` keys are dropped by + `JSON.stringify`, `null` keys are not. `place.ts` uses `| null` deliberately; match the + surrounding convention rather than mixing. +- **Constrain string enumerations with an enum or a union.** Where a raw stored value must be + tolerated, use `T | string` — **never `T | any`**, which collapses to `any` and silently stops + discriminating while still reading as though it constrains something. +- **Document every field** with a `/** … */` block: meaning, units, provenance, and what absent + means. See [`documentation.instructions.md`](documentation.instructions.md). +- **No banned types.** No `Function`, `Object` or bare `{}`. + +--- + +## 3. Timestamps + +Timestamp fields are the hardest shape in this package and the reason most of its remaining `any` +types exist. A Firestore timestamp is genuinely three things depending on direction: + +| Direction | Runtime value | +|---|---| +| Read from the datastore | a `Timestamp` / `Date` | +| Written to the datastore | a server sentinel (`serverTimestamp()`) | +| Serialized for transport | an ISO 8601 `string` | + +Writing that union precisely requires the server SDK's `FieldValue` type, which **is not a +dependency of this package and must not become one** — a pure type package should not pull a +server SDK into every consumer's dependency closure. + +**Consequently several timestamp fields are typed `any` on purpose.** That is a considered +trade-off, not an oversight. Do not "fix" them by reflex: + +- Narrowing a published field is a **breaking change** for consumers. +- The honest resolution is a **runtime schema** that validates the shape at the boundary, where the + constraint can actually be enforced, rather than a type that merely asserts it. + +Whatever you do, keep the JSDoc that explains the permissiveness. The comment is the only thing +distinguishing a deliberate decision from an accident. + +--- + +## 4. The index signature on `BaseFirestore` + +`BaseFirestore` carries `[x: string]: any`, which means **no extra property is ever a type error on +any document that extends it**. That is a deliberate accommodation of a sparse, evolving document +store, and it is also the single most permissive line in the package. + +Know what it costs: it disables excess-property checking for every extending interface, so a typo +in a field name is not a compile error anywhere in any consumer. Treat it as the reason a **runtime +schema** is necessary rather than optional — the type system has been explicitly told to stop +helping here. + +--- + +## 5. JSON and queue payload safety + +Anything that crosses the wire must be plain-JSON-serializable: + +- ❌ No `Date` objects (use ISO strings), `undefined`, `Map`/`Set`, `BigInt`, class instances or + functions in a transported payload. +- ✅ Prefer `null` over `undefined` for values that must survive a round-trip. +- Keep secrets and PII out of transported shapes entirely. A field that exists in a type is a field + someone will populate and log. + +--- + +## 6. Compatibility + +These declarations are a published contract: + +- **Additive is safe** — new optional fields, appended enum members, new exports. +- **Never remove or rename** a published field, type or export casually. Mark `@deprecated` with + the replacement, add the new field alongside, and remove only in a major version. +- **Widen inputs, keep outputs stable.** Narrowing a union breaks readers; widening one breaks + writers. Know which side you are moving before you move it. +- **Tolerate unknown fields on read.** Do not design a shape that must throw on an extra property — + stored documents predate every change you are about to make. + +--- + +## 7. When you add a runtime schema + +Schemas are the intended resolution to §3 and §4, so they will arrive. When they do: + +- A schema must **reject unknown keys, not silently drop them.** A silent drop leaves the caller + believing their input was honoured and the recipient believing it was filtered — both wrong, in + opposite directions. +- Keep the schema and the interface **in the same file as the type they describe**, so they cannot + drift apart unnoticed. +- The schema is a runtime artifact, so it compiles into `lib/` — which means it is subject to the + committed-output rule. **A schema that is never rebuilt into `lib/` is a validation layer that + passes review and never runs.** +- Test the rejection path, not only the happy path, and positive-control it: a schema test that + only asserts a valid object parses passes identically whether the schema is strict or wide open. diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md new file mode 100644 index 0000000..543e6c5 --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,125 @@ +--- +description: Vitest conventions, positive controls, and the limits of a runtime suite over erased types. +applyTo: "test/**/*.ts,vitest.config.ts,tsconfig.test.json" +--- + +# Testing Instructions — `@furcata/core-node` + +Companion to [`cross-repo.instructions.md`](cross-repo.instructions.md) §5 (evidence standards). + +--- + +## 1. The thing you must understand before writing a test here + +> 🔴 **TypeScript interfaces are erased at runtime. A Vitest assertion cannot see them.** + +This package is almost entirely interfaces and enums. That splits the suite into two kinds of +test with completely different value, and they look identical on the page: + +```ts +// ❌ CANNOT FAIL. This declares a literal in the test file and reads it back. +// It exercises JavaScript object semantics, not the source. Deleting `pending` +// from MessageQueue entirely leaves this green. +it('should accept a positive integer', () => { + const queue: MessageQueue = { pending: 5 }; + expect(queue.pending).toBe(5); +}); + +// ✅ CAN FAIL. An enum is a real runtime value, so this compares against the source. +it('should map country to its stored value', () => { + expect(PlaceType.country).toBe('country'); +}); +``` + +This is measured, not theoretical. Deleting `pending?: number` outright from +`src/interface/queue.ts` and running `npm test` produced **480 passed, exit 0**. Changing one enum +member's value produced **3 failed, exit 1**. + +### What follows from that + +- **Runtime tests over an interface are documentation, not verification.** They are still worth + having — they record intent and catch a barrel-export mistake — but never cite one as evidence + that a type is correct. +- **Type-level correctness is enforced by `npm run typecheck`, not by `npm test`.** Both must pass. + `npm test` proves runtime values; `npm run typecheck` proves the shapes. +- **When you change a type, the test that proves it is a compile error, not an assertion.** + +--- + +## 2. Commands + +| Purpose | Command | +|---|---| +| Run the suite once (CI mode) | `npm test` → `vitest run` | +| Watch | `npx vitest` | +| Coverage | `npx vitest run --coverage` | +| **Type-level check (required)** | `npm run typecheck` → `tsc -p ./tsconfig.test.json` | + +> ⚠️ `npx vitest run --typecheck` is **not** the type gate. Vitest's `typecheck.include` defaults +> to `**/*.test-d.ts`, and this repository has no such files, so it type-checks **zero files** and +> reports "no errors" no matter what is broken. Verified: with an interface field deleted it still +> reported `Type Errors no errors` and exited `0`. Use `npm run typecheck`. + +--- + +## 3. Conventions + +- **Framework.** Vitest, `node` environment. `globals: true` is set, but the convention is to + import explicitly: `import { describe, it, expect } from 'vitest';`. Two existing files rely on + the implicit globals, which is why `tsconfig.test.json` still needs the `vitest/globals` types. +- **Directory mapping.** Every test mirrors its source path and name: + `src/model/Account.ts` → `test/model/Account.test.ts`, + `src/interface/queue.ts` → `test/interface/queue.test.ts`. + Only `.test.ts` and `.spec.ts` under `test/` are collected. +- **Import with the ESM `.js` specifier**, matching `Node16` resolution: + `import type { MessageQueue } from '../../src/interface/queue.js';` +- **Structure.** Arrange–Act–Assert inside descriptive nested `describe()` / `it()` blocks. +- **Isolation.** Zero network, zero disk I/O, zero cloud or emulator access. The suite must be + safe to run anywhere, against anything. It currently is; keep it that way. +- **No lint escape hatches.** No `// eslint-disable*`. Note that `eslint.config.js` scopes `files` + to `src/**/*.ts`, so **`test/` is not currently linted** — do not read a green `npm run lint` as + a statement about test files. + +--- + +## 4. Positive controls and mutation testing + +> **A suite nobody has watched fail is a hypothesis about a suite.** + +Before claiming a test protects something, prove it can go red: + +1. Mutate the thing the test checks — delete the field, change the enum value, remove the export. +2. Run the relevant command and **observe the failure**, including the exit code. +3. Revert, and confirm green again. +4. **Report the mutation and what you saw.** "Tests pass" is not evidence; "deleting X turned N + assertions red, reverting restored them" is. + +Choose the mutation to match the gate you are validating: + +| Mutation | Caught by `npm test`? | Caught by `npm run typecheck`? | +|---|---|---| +| Change an enum member's **value** | ✅ yes | no | +| Remove an enum member | ✅ yes | ✅ yes | +| Delete an **interface field** | ❌ **no** | ✅ yes | +| Assign a wrong type in a test | ❌ no | ✅ yes | +| Remove a barrel `export *` | ✅ yes | ✅ yes | + +The two ❌ rows are precisely why both commands are required, and why a green `npm test` on its own +must never be reported as proof that a type change is safe. + +--- + +## 5. Writing a test that can actually fail + +When the thing under test is a type, assert against something with runtime existence: + +- **Enums** — assert member values and the full member set. An **inventory test** over + `Object.keys(SomeEnum)` is the capability equivalent of a positive control: it fails loudly when + a member is quietly dropped, which a per-member test cannot do. +- **Barrel exports** — import the barrel and assert the expected names are present. This catches a + dropped `export *`, which is otherwise invisible until a consumer breaks. +- **Runtime schemas**, once present — assert that a valid object parses, that an invalid one is + **rejected**, and that an unknown key is **rejected rather than silently dropped**. A schema test + asserting only the happy path is vacuous in the most dangerous way: it passes identically whether + the schema is strict or wide open. Always include the rejection case, and positive-control it by + confirming the valid case still parses. diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index fef1f1c..c54f6df 100755 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -95,3 +95,15 @@ jobs: run: npm test env: CI: true + + # `npm test` alone cannot fail on a type-level change: TypeScript interfaces + # are erased at runtime, so a test that declares an object literal and reads + # a property back still passes after the field is deleted from the source. + # Verified by deleting a field from an interface and observing 480/480 tests + # still pass. This step is what makes type-level regressions detectable, and + # it is positive-controlled: deleting an interface field, assigning a wrong + # type, and removing a referenced enum member each make it exit non-zero. + - name: Typecheck (tests against source types) + run: npm run typecheck + env: + CI: true diff --git a/package-lock.json b/package-lock.json index e110927..46aabc1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,7 +8,8 @@ "name": "@furcata/core-node", "version": "1.0.0", "dependencies": { - "@fabricelements/shared-helpers": "github:FabricElements/shared-helpers" + "@fabricelements/shared-helpers": "github:FabricElements/shared-helpers#8c17a299c20402dcc9f93f281608071be58e08cd", + "zod": "^4.4.3" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -6991,6 +6992,15 @@ "funding": { "url": "https://github.com/sponsors/sindresorhus" } + }, + "node_modules/zod": { + "version": "4.4.3", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", + "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } } } } diff --git a/package.json b/package.json index dd480e3..200d696 100755 --- a/package.json +++ b/package.json @@ -23,7 +23,8 @@ "./interface": "./lib/interface/index.js" }, "dependencies": { - "@fabricelements/shared-helpers": "github:FabricElements/shared-helpers" + "@fabricelements/shared-helpers": "github:FabricElements/shared-helpers#8c17a299c20402dcc9f93f281608071be58e08cd", + "zod": "^4.4.3" }, "devDependencies": { "@eslint/js": "^10.0.1", @@ -39,6 +40,7 @@ }, "scripts": { "test": "vitest run", + "typecheck": "tsc -p ./tsconfig.test.json", "build": "npm run clear && npm run lint && npm run compile", "build:watch": "npm run clear && npm run lint && npm run compile:watch", "clear": "rm -rf ./lib", diff --git a/tsconfig.test.json b/tsconfig.test.json index dac3bc0..7fc080f 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -6,6 +6,10 @@ "types": [ "node", "vitest/globals" + ], + "typeRoots": [ + "node_modules/@types", + "node_modules" ] }, "include": [ From ffd54777f84d3ff847d2946654fadfcbfffe66c7 Mon Sep 17 00:00:00 2001 From: Erny Sans Date: Sat, 22 Aug 2026 17:00:54 -0500 Subject: [PATCH 3/4] docs: rewrite CONTRIBUTING and refresh README CONTRIBUTING.md described a different project entirely -- it instructed contributors to run `bower install` and `polymer test`, neither of which exists here, and none of its guidance matched this toolchain. Replaced with the real workflow. Both documents now lead with the two things most likely to cause a silently wrong change: that this repository is public while its consumers are not, and that the committed build output is what consumers execute, so a src/ change that is not rebuilt is reviewed, approved, merged and never run. Also documents that `npm test` and `npm run typecheck` check different things and that both are required, since interfaces are erased at runtime and the suite cannot fail on a type change; records the CI step order; and corrects the dependency list. --- CONTRIBUTING.md | 166 ++++++++++++++++++++++++++++++++++++++---------- README.MD | 112 ++++++++++++++++++++++++++------ 2 files changed, 224 insertions(+), 54 deletions(-) mode change 100755 => 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md old mode 100755 new mode 100644 index ddb4cf4..0f6d2be --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,54 +1,152 @@ -## How to contribute to FabricElements +# Contributing to `@furcata/core-node` -* Fork the repository and clone it locally. -* Install dependencies with `npm install` and `bower install`. -* Create your feature branch running `git checkout -b my-new-feature`. -* Check that tests are passing running `polymer test`. -* Commit your changes running `git commit -m 'Add some feature'`. -* Push to the branch running `git push origin my-new-feature`. -* Submit a pull request. -* Wait for response from one of the team members. +Thanks for contributing. This package is a shared **TypeScript type library** — Firestore document +shapes, enums and queue contracts — consumed by backend services. It has no runtime logic and is +not a deployable service. -## Filing Issues +Please read [`.github/copilot-instructions.md`](.github/copilot-instructions.md) and +[`.github/instructions/`](.github/instructions/) before your first change. This file is the short +version. -**If you are filing an issue to request a feature**, please provide a clear description of the feature. It can be helpful to describe answers to the following questions: +--- -* Who will use the feature? -* When will they use the feature? -* What is the user’s goal? +## ⚠️ Two things that will catch you out -Or... If you are filing an issue to report a bug, be sure to provide: +### 1. This is a public repository -* A clear description of the bug and related expectations. -* A reduced test case that demonstrates the problem. +`"private": true` in `package.json` means *"never publish to the npm registry."* It says **nothing** +about GitHub visibility. Everything here — source, build output, commit messages, pull request +titles and bodies — is world-readable. -## Submitting Pull Requests +This package is consumed by services that are not public. **Describe what is true about this +package. Never name where else it is used, its environments, its internal identifiers, or an +unfixed weakness in another system.** This applies to code comments, config comments, test fixtures, +commit messages and PR descriptions — not just documentation. -**Before creating a pull request**, ensure that an issue exists for the corresponding change in the PR that you intend to make. If an issue does not exist, please create one providing: +```shell +./.github/scripts/check-private-markers.sh # run before you push; CI runs it too +``` -* A reference to the corresponding issue or issues that will be closed by the pull request. -* A succinct description of the design used to fix any related issues. -* At least one test for each bug fixed or feature added as part of the pull request. +### 2. The build output is committed, and consumers execute it -If a proposed change contains multiple commits, please **squash commits to as few as is necessary** to succinctly express the change. +`lib/` is generated by `tsc`, but it is **committed to git**, `exports` points directly at it, and +there is no `prepare` script — so installing this package from GitHub performs **no build**. -We really appreciate your interest in contributing and improving the organization. +**Reviewers read `src/`. Consumers execute `lib/`.** If you change `src/` without rebuilding, your +change is reviewed, approved, merged — and never runs. The diff looks perfectly correct, so nothing +in review can catch it. -## Squashing commits +```shell +npm run build # regenerates lib/ (rm -rf ./lib, then lint, then tsc) +git status --porcelain -- lib/ # MUST be empty before you push +``` -To squash four commits into one, do the following: +**Commit the regenerated `lib/` in the same commit as your `src/` change.** Never hand-edit `lib/`. - $ git rebase -i HEAD~4 +--- -In the text editor that comes up, replace the words "pick" with "squash" next to the commits you want to squash into the commit before it. Save and close the editor, and git will combine the "squash"'ed commits with the one before it. Git will then give you the opportunity to change your commit message to something like, "Issue #100: Fixed retweet bug." +## Setup -**Important**: If you've already pushed commits to GitHub, and then squash them locally, you will have to force the push to your branch. +```shell +npm install # CI uses `npm ci` +``` - $ git push origin branch-name --force +Requires Node `>=22`. -Helpful hint: You can always edit your last commit message, before pushing, by using: +## Verify your change - $ git commit --amend +Run these in the order CI does. **Report exit codes rather than impressions.** -### See also: -[Git Book Chapter 6.4: Git Tools - Rewriting History](http://git-scm.com/book/en/Git-Tools-Rewriting-History) +| Step | Command | +|---|---| +| Build (clear → lint → compile) | `npm run build` | +| Build-output drift | `git status --porcelain -- lib/` (must be empty) | +| Private markers | `./.github/scripts/check-private-markers.sh` | +| Tests | `npm test` | +| **Typecheck** | `npm run typecheck` | +| Lint | `npm run lint` (auto-fix: `npm run lint:fix`) | + +### `npm test` and `npm run typecheck` are not interchangeable + +TypeScript interfaces are **erased at runtime**, so the Vitest suite cannot fail on a type change. +Deleting a field outright from an interface leaves all 480 tests green. Type-level regressions are +caught **only** by `npm run typecheck`. Both are required. + +(`npx vitest run --typecheck` is *not* the type gate — its `typecheck.include` defaults to +`**/*.test-d.ts` and this repo has none, so it checks zero files and always reports "no errors".) + +--- + +## Making a change + +- **Edit only `.ts` files under `src/`.** Keep `src/model/` and `src/interface/` as pure, + transport-agnostic data contracts — no I/O, side effects or business logic. +- **Mirror tests.** `src/model/Account.ts` → `test/model/Account.test.ts`. Import with the ESM + `.js` specifier: `from '../../src/model/Account.js'`. +- **Document every definition** with a `/** … */` block: meaning, units, provenance, and what + absent means. Use `@return` (singular). Preserve the license header and never alter existing URLs + or `{@link …}` references. +- **No banned types** — no `Function`, `Object`, or bare `{}`. Never `T | any`: it collapses to + `any` while still reading as though it constrains something. +- **Narrowing a published type is a breaking change** for consumers. Widen inputs freely; narrow + only deliberately. +- **No lint escape hatches** (`// eslint-disable*`). + +--- + +## Evidence standards + +These are not ceremony; each one exists because its absence caused a real mistake. + +- **Positive-control every probe.** A grep that cannot demonstrate it can return a true positive is + not evidence — it produces a confident, well-formed argument for the wrong action. Before you + trust a zero, make the same probe report a one against a synthetic string. +- **Prove a test can fail.** Mutate what it checks, confirm red, revert, and say which mutation you + ran. A suite nobody has watched fail is a hypothesis about a suite. +- **Prove before/after; don't assert it.** Run the identical check on both states and report both. +- **Record negative results.** A refuted finding is a real deliverable. Never quietly drop a claim + you disproved — write down what you checked and why it was clean. +- **A control can pass its tests and still be inert** because a precondition outside this repo is + missing. For anything you add, name what must be true elsewhere for it to actually run. + +--- + +## Commits and pull requests + +- One branch, one pull request, one unit of work. +- Write a clear message explaining **why**, not just what. +- **Never add a `Co-authored-by:` trailer**, or any attribution trailer, regardless of tooling + defaults. + + A clean message check is necessary but **not sufficient**: a squash merge generates the trailer + **server-side from the authorship of the squashed commits**, which no local `commit-msg` hook can + intercept. Check both: + + ```shell + git log origin/main..HEAD --format='%B' | grep -ci 'co-authored-by' # must be 0 + git log origin/main..HEAD --format='%an|%cn' # expected identities only + ``` + + Positive-control that first grep — a zero from an untested probe is not evidence. +- If a change lands in stages, **order them so stopping halfway leaves the tree safe rather than + dishonest**: code first, then the docs describing it. Docs landing first means the repository + advertises a control it does not have, and a reader will act on it. + +--- + +## Filing issues + +**For a bug**, include a clear description of expected versus actual behaviour and a reduced case +that demonstrates it. + +**For a feature**, describe who needs it, when, and what goal it serves. + +Remember that issues are public too — do not paste internal identifiers, environment details, or +anything about a non-public consumer. + +--- + +## License + +By contributing you agree that your contributions are licensed under the +[BSD 3-Clause License](LICENSE.md). diff --git a/README.MD b/README.MD index 8875ce1..d9926c6 100644 --- a/README.MD +++ b/README.MD @@ -9,6 +9,18 @@ queue contracts used across the system. > **Repository:** +> [!IMPORTANT] +> **This is a public repository.** The `"private": true` flag in `package.json` means *"never +> publish to the npm registry"* — it says nothing about GitHub visibility. Source, build output, +> commit messages and pull request descriptions here are all world-readable. +> +> This package is consumed by services that are not public. **Describe what is true about this +> package; never name where else it is used, its environments, its internal identifiers, or an +> unfixed weakness elsewhere.** That applies to code comments, test fixtures, commit messages and +> PR bodies, not just documentation. `.github/scripts/check-private-markers.sh` enforces it in CI — +> run it locally before pushing. + + ------ ## Project Overview & Cloud Architecture @@ -62,8 +74,8 @@ and Google Cloud are **downstream consumers** of this package, not local depende | **Module system** | Native ESM — `"type": "module"`, with `Node16` `module` and `moduleResolution`. | | **Data layer** | Models describe **Firestore** documents. `BaseFirestore` standardises identity, audit timestamps, and TTL (`expiry`) fields for every collection document. | | **Linting** | ESLint `^10` flat config (`eslint.config.js`) via `typescript-eslint` (`recommended` + `stylistic`), scoped to `src/**/*.ts`; `max-len` is `200` (`ignoreComments`, `ignoreUrls`). | -| **Testing** | Vitest `^4` (`node` environment) with type-checking through `tsconfig.test.json`. | -| **Dependency** | `@fabricelements/shared-helpers` — the sole runtime dependency (e.g., the `User` type used by the `Account` model). | +| **Testing** | Vitest `^4` (`node` environment) for runtime assertions, plus `npm run typecheck` (`tsc -p ./tsconfig.test.json`) for type-level checks. Both are required — see [Type-checking](#type-checking). | +| **Dependencies** | `@fabricelements/shared-helpers` (public; e.g. the `User` type used by the `Account` model), pinned to an exact commit SHA, and `zod` `^4.4.3` for runtime schemas. | ### Active development source tree @@ -98,21 +110,33 @@ core-node/ > The `src/model/` ↔ `src/interface/` split mirrors the public `exports` map in `package.json`: > `"./model" → "./lib/model/index.js"` and `"./interface" → "./lib/interface/index.js"`. -### 🔴 Strict blacklist notice — `/lib` is auto-generated build output +### 🔴 `/lib` is generated build output — AND it is committed > [!CAUTION] -> **The `/lib` directory contains auto-generated build targets. NEVER edit it manually, and -> completely ignore it during development.** +> **`/lib` is auto-generated. Never edit it by hand. But it *is* committed to git, and it is the +> code consumers actually execute — so it must be rebuilt and committed whenever `src/` changes.** +> +> `package.json` `exports` points directly at `./lib/model/index.js` and `./lib/interface/index.js`, +> and there is **no** `prepare` or `prepack` script. Installing this package straight from GitHub +> therefore performs **no build**: the consumer runs the committed `/lib`. +> +> **Reviewers read `src/`. Consumers execute `lib/`. Those are different files.** A change to +> `src/` can be authored, reviewed, approved and merged and still never run, because the compiled +> output was never regenerated. The pull request diff looks perfectly correct — `src/` is exactly +> what it claims to be — so nothing in the review surface can reveal it. +> +> The rules that follow: > -> - `/lib` is produced **exclusively** by the TypeScript compiler (`tsc`, `outDir: lib`) and is -> wiped and regenerated on every build by the `clear` script (`rm -rf ./lib`). -> - **Never** edit, create, or delete any file under `/lib` — including every `.js` and `.d.ts` -> artifact. Any change there is silently overwritten on the next compile. -> - **Never** edit compiled `.js` artifacts anywhere. All development happens in `.ts` source -> files under `src/`. -> - To change runtime behaviour, edit the matching `.ts` source in `src/` and recompile. -> - Tooling already enforces this boundary: ESLint ignores `lib/*` (alongside `node_modules/*`, -> `.github/*`, and `functions/*`). +> - `/lib` is produced **exclusively** by `tsc` (`outDir: lib`) and is wiped and regenerated on +> every build by the `clear` script (`rm -rf ./lib`). +> - **Never** hand-edit any `.js` or `.d.ts` under `/lib`; the next build destroys it. +> - **Never** read `/lib` as a source of truth — use `src/`. +> - ✅ **After any change to `src/`, run `npm run build` and commit the regenerated `/lib` in the +> same commit.** Source and output are one atomic unit. +> - Verify before pushing: `git status --porcelain -- lib/` must be empty after a build. +> **CI fails the job on drift.** +> - ESLint ignores `lib/*` (alongside `node_modules/*`, `.github/*`, and `functions/*`). + ## Local Setup & Firebase Emulation @@ -219,8 +243,25 @@ $ npx vitest run --coverage # coverage report ### Type-checking -Vitest is configured to type-check the test tree using `tsconfig.test.json` (see `typecheck` in -`vitest.config.ts`), so type errors in tests surface during runs. +> [!IMPORTANT] +> **`npm test` cannot fail on a type change, and `npm run typecheck` is a separate, required gate.** +> +> TypeScript interfaces are **erased at runtime**. A test that declares an object literal and reads +> a property back exercises JavaScript, not the source: deleting `pending?: number` outright from +> `src/interface/queue.ts` still leaves **480/480 tests passing, exit 0**. Only enum-backed +> assertions can fail at runtime, because an enum is a real runtime value. + +```shell +$ npm run typecheck # tsc -p ./tsconfig.test.json — type-checks test/ against src/ +``` + +This catches what the suite cannot: a deleted interface field, a wrong-typed assignment, and a +removed enum member each make it exit non-zero. + +> ⚠️ `npx vitest run --typecheck` is **not** this gate. Vitest's `typecheck.include` defaults to +> `**/*.test-d.ts` and this repository has no such files, so it type-checks **zero** files and +> reports `Type Errors no errors` regardless of what is broken. Use `npm run typecheck`. + ### Safe testing of Firebase-dependent code @@ -239,8 +280,19 @@ network requests, disk I/O, or live cloud/Firestore calls. ### Continuous Integration Pushes and pull requests targeting `main` run the **Node CI** workflow -(`.github/workflows/nodejs.yml`) on Node `22.x` and `24.x`. Each job runs `npm ci`, then -`npm run build` (lint + compile), then `npm test`. Changes must keep all of these green. +(`.github/workflows/nodejs.yml`) on Node `22.x` and `24.x`. Each job runs, in order: + +| Step | Command | Fails when | +|---|---|---| +| Install | `npm ci` | lockfile and manifest disagree | +| Build | `npm run build` | lint or compile error | +| **Build-output drift** | `git status --porcelain -- lib/` | committed `/lib` is stale | +| **Private markers** | `.github/scripts/check-private-markers.sh` | a private marker or attribution trailer appears in a file, commit message or authorship | +| Test | `npm test` | a runtime assertion fails | +| **Typecheck** | `npm run typecheck` | a type-level regression | + +Changes must keep all of these green. + ## AI-Assisted Engineering Rules @@ -251,8 +303,20 @@ contribute: > **Read [`.github/copilot-instructions.md`](.github/copilot-instructions.md) before contributing — > with code or with an AI assistant.** +Task-scoped detail lives in [`.github/instructions/`](.github/instructions/): + +| File | Covers | +|---|---| +| [`cross-repo.instructions.md`](.github/instructions/cross-repo.instructions.md) | **Read first.** Public-repository rules, why types are a security control here, the committed-output hazard, evidence standards, agent conduct. | +| [`security.instructions.md`](.github/instructions/security.instructions.md) | Measured current state, sweep recipes with positive controls, and what is deliberately left alone. | +| [`serialized-models.instructions.md`](.github/instructions/serialized-models.instructions.md) | Model and interface conventions — this package **is** the serialized model layer. | +| [`tests.instructions.md`](.github/instructions/tests.instructions.md) | Vitest conventions, and why the suite cannot fail on an erased type. | +| [`documentation.instructions.md`](.github/instructions/documentation.instructions.md) | JSDoc conventions. | +| [`readme.instructions.md`](.github/instructions/readme.instructions.md) | README / CONTRIBUTING maintenance. | + It is the source of truth for the standards summarised below: + - **Strict typing.** Use precise TypeScript types and explicit declarations; never rely on banned or unsafe types such as `Function` or `Object` — use a concrete signature, `unknown`, or the actual interface instead. @@ -269,8 +333,16 @@ It is the source of truth for the standards summarised below: - **Separation of concerns.** Keep `src/model/` and `src/interface/` as pure, transport-agnostic data contracts — no trigger wiring, side effects, or I/O. Prefer `async`/`await` over raw `.then()/.catch()` chains. -- **Respect the `/lib` blacklist.** Implement only in `.ts` files under `src/`; never edit `/lib` - or compiled `.js` artifacts (see the blacklist notice above). +- **Respect the `/lib` rules.** Implement only in `.ts` files under `src/`; never hand-edit `/lib` + or compiled `.js` artifacts — **and rebuild and commit `/lib` in the same commit as any `src/` + change** (see the caution above). +- **Evidence standards.** Positive-control every probe: a grep that cannot demonstrate a true + positive is not evidence. Prove a test can fail by mutating what it checks and watching it go + red. Record negative results rather than dropping them. +- **Never add a `Co-authored-by:` trailer** to any commit or pull request. Check both the messages + and the **authorship** — a squash merge synthesises the trailer server-side from commit + authorship, which no local hook can intercept. + **Before opening a Pull Request,** ensure your AI chat assistant is grounded in `.github/copilot-instructions.md` so generated code, comments, and tests stay 100% consistent with From 5795ab2d096e2dbad434085010e1dfb1035ab35c Mon Sep 17 00:00:00 2001 From: Erny Sans Date: Sat, 22 Aug 2026 17:01:55 -0500 Subject: [PATCH 4/4] chore(deps): apply non-breaking audit fixes `npm audit fix` without --force, so every change is semver-compatible. Takes the advisory count from 17 (1 low, 11 moderate, 5 high) to 13 (11 moderate, 2 high). Only package-lock.json changes. Verified afterwards: npm ci, build, test, typecheck and lint all exit 0, and the build output is unchanged. The remaining 13 all arrive transitively through the single runtime dependency and are not reachable from this package's own code, which has no runtime logic. None can be resolved from here -- they need an upstream release. They are left visible rather than suppressed with an `overrides` entry, since forcing an unrelated version could break a consumer's runtime in a way this package cannot test. --- package-lock.json | 48 +++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index 46aabc1..76be6e7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -693,9 +693,9 @@ } }, "node_modules/@google-cloud/storage": { - "version": "7.21.0", - "resolved": "https://registry.npmjs.org/@google-cloud/storage/-/storage-7.21.0.tgz", - "integrity": "sha512-l+IFTkd+6Y5LoAuXyYCKNAKtw/Ci+rAMqgdTB1jv4iZiLhw0rtq+0qjIRbBizXkNzEFmXiXUW0H7sZQQvk1ffA==", + "version": "7.22.0", + "resolved": "https://registry.npmjs.org/@google-cloud/storage/-/storage-7.22.0.tgz", + "integrity": "sha512-W98gTQOAntEeEQ7/pZxSQXxfUO5CiQcXJRsxBpUa6UU25n8YN/VtogPBi0H+MAmUrn/6bY/sBhFansoWEsr2/g==", "license": "Apache-2.0", "optional": true, "dependencies": { @@ -715,7 +715,7 @@ "teeny-request": "^9.0.0" }, "engines": { - "node": ">=14" + "node": ">=18" } }, "node_modules/@google-cloud/storage/node_modules/@google-cloud/paginator": { @@ -2838,9 +2838,9 @@ } }, "node_modules/body-parser": { - "version": "1.20.5", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.5.tgz", - "integrity": "sha512-3grm+/2tUOvu2cjJkvsIxrv/wVpfXQW4PsQHYm7yk4vfpu7Ekl6nEsYBoJUL6qDwZUx8wUhQ8tR2qz+ad9c9OA==", + "version": "1.20.6", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.6.tgz", + "integrity": "sha512-p5tAzS57i5MV9fZFDj9LeIiTZEufbSe2eDozP+ElheSUq1m74CRq1jI4mYNDdVs9vQztXFLuk/Gd6BWTdwRJ5g==", "license": "MIT", "dependencies": { "bytes": "~3.1.2", @@ -2877,16 +2877,16 @@ "license": "MIT" }, "node_modules/brace-expansion": { - "version": "5.0.6", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", - "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/buffer-equal-constant-time": { @@ -4221,9 +4221,9 @@ "license": "MIT" }, "node_modules/glob/node_modules/brace-expansion": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.1.tgz", - "integrity": "sha512-WR1cURNjuvBLMZBMbqM0UoE+WAfdUcEV1ccD8PVBVOI+Z3ND4+SZbN8RsfT2bMuG1qwz5RFvPukSZm5fF2D5eA==", + "version": "2.1.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", + "integrity": "sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==", "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" @@ -5289,9 +5289,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.12", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.12.tgz", - "integrity": "sha512-ZB9RH/39qpq5Vu6Y+NmUaFhQR6pp+M2Xt76XBnEwDaGcVAqhlvxrl3B2bKS5D3NH3QR76v3aSrKaF/Kiy7lEtQ==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "dev": true, "funding": [ { @@ -5591,9 +5591,9 @@ } }, "node_modules/postcss": { - "version": "8.5.15", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.15.tgz", - "integrity": "sha512-FfR8sjd4em2T6fb3I2MwAJU7HWVMr9zba+enmQeeWFfCbm+UOC/0X4DS8XtpUTMwWMGbjKYP7xjfNekzyGmB3A==", + "version": "8.5.26", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.26.tgz", + "integrity": "sha512-u82N74LFzG8ca+dD8puPnplTXoGH4fTPpVGuIbt36G3qvNlkvfD0lEAZSxaly3KX8TS/L1A1gsCEmvKmBcVbkQ==", "dev": true, "funding": [ { @@ -5611,7 +5611,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.17", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, @@ -5642,9 +5642,9 @@ } }, "node_modules/protobufjs": { - "version": "7.6.4", - "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-7.6.4.tgz", - "integrity": "sha512-RJJPTTpvFfHcWLkIa2JFWK4XvtSzS0yEWDmunqHXli1h3JlkbcQZXDZdcWxv+JK3Xsl5/UFDPZ0iGm7DAengYw==", + "version": "7.6.5", + "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-7.6.5.tgz", + "integrity": "sha512-/FPD0nUc9jH6rfFjji9IBqOz4pcSE3CsT1m7Ep6Mdb0LxSUMj8hgl6GomOvZzpNpAqqGaXA0P3VSrZLFzIhQrw==", "hasInstallScript": true, "license": "BSD-3-Clause", "dependencies": {