Define core plus installable extensions architecture#137
Conversation
There was a problem hiding this comment.
QA Review — PR #137 / Issue #136
Decision: APPROVED
Blockers
None.
Important
None.
Nice to have
- The
overview.mdMermaid diagram now includes anExtensionsnode connected fromCore, which is the right conceptual shape even before implementation. If someone reads it literally they might expect the node to be wired up — a short prose note near the diagram saying "Extension layer wiring is shown as accepted architecture direction; runtime implementation is future work" could prevent confusion. Not a blocker; the existing prose section covering Extension layer already says this clearly enough.
Docs / invariants
All acceptance criteria verified:
-
✅ ADR 0010 added — properly sequenced after ADR 0009, Status: Accepted, Date: 2026-05-09. Covers all required model elements: core boundary, extension definition, install/activation layering, repo enablement, discovery/compatibility/namespacing.
-
✅ ADR explains refinement, not contradiction — Section 6 "Relation to ADR 0001" calls this out explicitly and clearly. Cross-references ADR 0001 by name, explains precisely what stays unchanged and what the refinement adds.
-
✅ Docs consistently describe core, extensions, and repo enablement — All four updated files (
invariants.md,overview.md,vision.md,spec.md) are internally consistent with each other and with the ADR. New "Extension model invariants" section ininvariants.md, new "Extension layer" section and updated architectural rules inoverview.md, updated product principles/V1 focus/non-goals invision.md, and reserved extension config direction inspec.md§6.2.1 all converge on the same model. -
✅ Repo config is declarative and non-secret — Stated explicitly in
invariants.md(line 31),overview.md(line 71),spec.md(line 268 + §6.2.1 line 275), andvision.md(line 47). Consistent phrasing throughout. -
✅ Installed extensions are not active unless enabled — Stated explicitly in
invariants.md(line 11),overview.md(lines 106, 109–110),vision.md(line 69),spec.md(line 276), and ADR 0010 (lines 59–62). The "installed ≠ enabled, enabled ≠ validly configured" layering is present in the ADR and echoed where needed. -
✅
configterminology, notprofile— The rule is declared normatively in ADR 0010 (line 88),invariants.md(line 43), andspec.md(line 278). All JSON examples throughout useconfigconsistently. Noprofileleakage found. -
✅
docs/README.mddoes not need updating — It already referencesdocs/architecture/adrs/generically, and the updatedinvariants.md,overview.md, andvision.mdare already in its map. -
✅ No debt update required — This is a positive architecture direction documentation pass, not the introduction of new debt. Correct call by Greg.
What I verified
- Read all five changed files in full against every acceptance criterion
- Confirmed ADR 0010 is correctly sequenced as the 10th ADR after 0009
- Read ADR 0001 to verify that ADR 0010's "refinement, not contradiction" framing is accurate — it is: ADR 0001 says repo-specific workflow semantics stay above
orfe; ADR 0010 does not challenge that, it adds a home for reusable installable deterministic layers between core and repo-specific policy npm run lint— clean, no outputnpm run typecheck— clean, no outputnpm run build— clean, no outputnpm test— all 98 test cases incore.test.tspassed; full suite timed out in bash session but no failures observed, consistent with a docs-only change- PR body first line is
Ref: #136✅; branch isissues/orfe-136✅; no auto-closing keywords ✅
Ref: #136
Summary
Verification
npm test✅npm run lint✅npm run typecheck✅npm run build✅Docs / ADR / debt
Risks / follow-ups