Skip to content

Architecture audit: enforce domain purity, derive the conformance population, document Mongo atomicity - #14

Open
FlashyLabs wants to merge 6 commits into
mainfrom
claude/flashy-ledger-architecture-audit-79az1h
Open

FlashyLabs wants to merge 6 commits into
mainfrom
claude/flashy-ledger-architecture-audit-79az1h

Conversation

@FlashyLabs

@FlashyLabs FlashyLabs commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

What changed

An architecture audit of @flashylabs/ledger against its own stated design, closing the real enforcement gaps it surfaced — each with tests and documentation, no behaviour changed. (1) Domain purity was only half-enforced: eslint guards clocks/randomness in src/domain, but nothing guarded the dependency direction, so a domain module importing an adapter, a port, or node:fs would pass lint, typecheck and every test while silently dissolving the ports-and-adapters seam. Now pinned as invariant I-9 and enforced by tests/domain-purity.test.ts (reads the real import graph via ts.preProcessFile). (2) The conformance population was hand-kept — the shape of failure this estate has hit before — so it is now derived: tests/adapters.catalog.ts is the single source of truth, conformance.test.ts builds its harnesses from it, and tests/adapter-coverage.test.ts checks it against the package's real exports. The audit also filled the deliberately-blank CLAUDE.md section, documented the Mongo concurrency/atomicity design as ADR 0004, added the one missing Mongo safety test (refusing to tear a transfer without a client), and fixed a pre-existing lint break in charter-served.test.ts that had npm run check (and CI) red before this branch. This branch also carries one prior unmerged commit (directory/1 re-vendor).

Ledger invariants

  • Entries remain append-only — nothing updates or deletes history (unchanged)
  • Amounts are signed integers in minor units — no floats, one convention (unchanged)
  • Every write is idempotent under a stable key (unchanged)
  • Balances stay derived — no new authoritative balance column (unchanged)
  • The domain stays pure — no clock, randomness or I/O under src/domain — strengthened: the import-graph half is now enforced (I-9), not only the runtime half

Verification

  • npm run check passes locally — typecheck + lint + coverage all green (205 tests, 17 files; coverage 98.24% stmts / 92.61% branch / 100% func / 100% line, above the 90/85/90/90 thresholds). Lint was red before this branch on a pre-existing any-access in charter-served.test.ts (TS 5.9.3 per the lockfile); fixed here by typing the parse, no behaviour change.
  • New behaviour is covered by tests, including its failure cases — the purity test, both coverage guards, and the Mongo refusal were each verified to go red when their invariant is violated, then restored

Migration impact

None. No entry format change and no change to the hashEntry input, so every chain written before this PR verifies unchanged. The work is tests, documentation, one test-support module (tests/adapters.catalog.ts), and a typing fix in a test; src/ is untouched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X8LDnsexvNjSkqc2ihZR7U

The vendored copy of @flashyos/directory's checker had drifted behind the
authoritative EDGE_REQUIRES: it required settled edges to carry only
['sealed'], where packages/directory/src/types.ts (origin/main) requires
['sealed', 'capability']. A lagging copy does not fail — it disagrees
silently, validating a settled edge the estate's own spec rejects.

Re-vendored byte-identical to flashyos origin/main
packages/directory/vendor-check-directory.mjs (the shipping canonical, not a
stale working tree). Verified: the re-vendored checker still validates this
repo's own directory.fragment.json (16 nodes, 22 edges, 0 problems). The
estate-level differential tools/vendored-directory.test.mjs in flashyos is
the guard that catches this class of drift.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8LDnsexvNjSkqc2ihZR7U
The ledger's headline architectural claim — the domain reads no database,
calls no clock, and generates no randomness, which is the whole reason it can
move onto a chain without its rules changing — was only half enforced. eslint
guards the runtime half (Date, Date.now, Math.random banned in src/domain),
but nothing guarded the dependency direction: a domain module importing an
adapter, a port, node:fs, or a third-party package would pass lint, typecheck
and every behavioural test, silently breaking the seam the package is built on.

- tests/domain-purity.test.ts reads the real import graph of src/domain with
  ts.preProcessFile (not a regex) and refuses any specifier that is not a
  domain sibling or node:crypto. Verified non-vacuous: it goes red when an
  adapter import or node:fs is injected into a domain file.
- docs/INVARIANTS.md gains I-9, making purity a numbered, cited invariant;
  tests/invariants.test.ts holds the doc and the suite together as before.
- CLAUDE.md gains the "what makes this repository different" section the file
  template asked for: what it is, that it ships as a library with no deploy
  target, the ports-and-adapters shape, and the invariants an agent must not
  break.

Full suite 198 passing, typecheck clean, new test lint-clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8LDnsexvNjSkqc2ihZR7U
…city

Two audit findings from the ports-and-adapters layer, each closed with tests
and documentation.

1. The conformance suite's worth is the set of adapters it is pointed at, and
   that set was a hand-kept list — the shape of failure this estate has hit
   before (pulse.yml over a population of zero). The population is now derived:
   - tests/adapters.catalog.ts declares every writable LedgerStore the package
     ships, as the single source of truth.
   - conformance.test.ts builds its harnesses from the catalog (Mongo skipped
     without a database but never dropped from the count) and asserts the
     harnessed classes equal the catalog.
   - tests/adapter-coverage.test.ts checks the catalog against the package's
     real exports, recognising a writable store by structure (an append method)
     so the read-only GoldLedgerReader is excluded by that same fact. A new
     adapter exported-but-uncataloged, or cataloged-but-unharnessed, now fails
     the build. Documented in docs/INVARIANTS.md, "Coverage".

2. The Mongo adapter's refusal to tear a transfer — reject a multi-entry append
   when constructed without a client — had no test, because conformance always
   passes one. tests/mongo-adapter.test.ts covers it with a fake Db that runs on
   any machine, no infrastructure. The concurrency and atomicity design (indexes
   as the guarantee, the duplicate-key disambiguation, the session requirement)
   is recorded in docs/adr/0004-mongo-concurrency-and-atomicity.md.

Full suite 205 passing, typecheck clean, new files lint-clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8LDnsexvNjSkqc2ihZR7U
…d test

npm run lint (hence npm run check, hence CI) was red before this branch:
charter-served.test.ts did `const parsed = JSON.parse(served)` — an `any` —
and then `parsed.slug` / `parsed.roles`, three no-unsafe-member-access errors
under the repo's strictTypeChecked config (typescript 5.9.3, as the lockfile
pins). Not introduced here, but it blocks the repo's own gate and this PR.

Typed the parse as Record<string, unknown> and narrowed `roles` through
Array.isArray before reading .length — no behaviour change, the two
assertions are identical. npm run check now passes: typecheck + lint +
coverage (98.24% stmts, above the 90/85/90/90 thresholds).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8LDnsexvNjSkqc2ihZR7U

Copy link
Copy Markdown
Owner Author

This PR is currently in conflict with main (which has moved since the branch point). The conflicting files are narrow — scripts/check-directory.mjs and tests/charter-served.test.ts, both of which main also changed — and everything else (the purity invariant, the derived conformance population, the Mongo ADR + test) is clean.

Resolving it is a routine merge origin/main + regenerate-via-tooling, but that git operation is currently blocked in my execution environment pending a permission decision from the maintainer. Once unblocked I'll merge main into this branch, resolve the two files (regenerating any generated files with their emitters rather than by hand), re-run npm run check, and push — no force, no history rewrite.


Generated by Claude Code

…architecture-audit-79az1h

# Conflicts:
#	tests/charter-served.test.ts

Copy link
Copy Markdown
Owner Author

Conflict resolved — merged origin/main into the branch (02da28d). The only conflict was tests/charter-served.test.ts, where main had independently fixed the same any-access lint break with a cleaner typed interface Charter; I took main's version, so my earlier as Record<string, unknown> is gone. No force-push, no history rewrite.

The branch is now conflict-free and npm run check is green except for one pre-existing failure on main that this PR did not cause:

  • tests/releases.test.ts — package.json is 1.0.0 (set by cf4d9ba, "Bump to 1.0.0 for npm publication") but RELEASES.md has no 1.0.0 row, which is exactly the gap that test guards. 218 of 219 tests pass; typecheck and lint are clean.

I'm not adding the ledger row from here: RELEASES.md is a measured trail, and the one field that matters — whether the 1.0.0 publish actually landed — I can't verify from this environment. The v1.0.0 and v1.0.0-ledger tags both exist on origin, which suggests the release is mid-flight and the row is yours to add with the measured publish result. Proposed shape for whoever has the run data:

| 1.0.0 | `cf4d9ba` | ✅ v1.0.0 | <landed? run N / E401 / assumed> | 1.0.0 for npm publication |

Once that row lands on main, this PR goes fully green with no further change from me.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

CI on 02da28d is red on two checks, both pre-existing on main and neither introduced by this PR (which adds only test files, docs, and a test-support module — no dependencies, no src/ change):

  1. Typecheck, lint, test — this is the releases.test.ts failure already detailed above (1.0.0 bumped in cf4d9ba without a RELEASES.md row). Typecheck and lint pass; 218/219 tests pass; only that one fails. Needs the measured publish result I can't fabricate.

  2. Dependency audit — npm audit --audit-level=high flags a transitive brace-expansion advisory (high, quadratic-expansion / recursion DoS; GHSA-q2hr-2g5m-vwhr, -qhr7-859c-m2p7, -6j4f-fj2g-mc7p), reached via @typescript-eslint/typescript-estree and the root tree. It comes entirely from the lockfile this branch inherited from main, so main and every open PR fail this check identically until the lockfile is bumped. npm audit fix resolves it (a brace-expansion patch bump, lockfile-only). I'm not folding that into this architecture-audit PR — a lockfile bump is unrelated to the diff and belongs on main, where it clears the check for every PR at once.

Both clear from this PR with zero further change once main carries (a) the 1.0.0 RELEASES.md row and (b) the brace-expansion bump. Happy to open a separate small PR for the npm audit fix lockfile bump if you'd like it handled now — just say so.


Generated by Claude Code

…elease row

Both were pre-existing on main and red on this PR; the user asked for the
brace-expansion fix and for the record to reflect production reality.

- Dependency audit: `npm audit fix --package-lock-only` bumps the transitive
  brace-expansion past the quadratic-expansion / recursion DoS advisories
  (GHSA-q2hr-2g5m-vwhr and siblings). Lockfile-only, package.json untouched;
  `npm ci` clean, `npm audit --audit-level=high` reports 0 vulnerabilities, and
  the full suite still passes (the bump is transitive under eslint's matcher).

- releases.test: `package.json` declares 1.0.0 (cf4d9ba) with no ledger row.
  Measured the publish rather than assuming it: all three publish.yml runs for
  v1.0.0 (2026-09-25) failed at "Publish to npm" with `npm error code
  ENEEDAUTH` — the tarball built but never authenticated to npm.pkg.github.com,
  so 1.0.0 is NOT on the registry. A `v1.0.0` git tag was pushed anyway, which
  the ledger's own rule forbids (a tag must back a landed publish), so the row
  records the tag as unbacked, not ✅. The 2026-10-05 note explains the
  regression and that fixing the publish auth is an operator action.

npm run check green: typecheck + lint + coverage (98.24% stmts), 219 tests,
0 vulnerabilities.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8LDnsexvNjSkqc2ihZR7U
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants