Phase 1A: introduce runtime container composition root + integration coverage#8
Merged
ethanj merged 8 commits intoarchitecture2from Apr 17, 2026
Merged
Conversation
Introduce an explicit, isolated composition root for Atomicmemory-core
and remove the last module-level mutable wiring (setEntityRepository).
Part of the rearchitecture plan at
atomicmemory-research/docs/atomicmemory-core-rearchitecture-plan-2026-04-16.md.
New composition layer (src/app/):
- runtime-container.ts: createCoreRuntime({ pool, config? }) composes
repositories and services from explicit deps. Pool is required — the
composition root is a pure function with no side-effectful singleton
imports at module load. Config defaults to the module-level value
because it is pure env-derived data.
- startup-checks.ts: checkEmbeddingDimensions() returns a structured
result instead of calling process.exit.
- create-app.ts: createApp(runtime) builds the Express app, wiring
routers and middleware onto a runtime container.
- __tests__/runtime-container.test.ts: 9 composition tests covering
container wiring, startup-check branches, and the app factory.
server.ts: thinned from 101 → 71 lines. Imports the singleton pool once
and passes it to createCoreRuntime({ pool }). Shutdown closes
runtime.pool so a custom bootstrap closes the right pool. Preserves all
original named exports (app, service, repo, claimRepo, trustRepo,
linkRepo) plus the new `runtime` export.
search-pipeline.ts: removed module-level entityRepo and the
setEntityRepository() setter. Threaded entityRepo through as an
explicit parameter to runSearchPipelineWithTrace and all ten internal
helpers that consume it. The one call site in memory-search.ts passes
deps.entities.
Test fixtures and current-state-retrieval-regression dropped their
references to the removed setEntityRepository export.
Preserved:
- Endpoint behavior unchanged, routes unchanged, MemoryService kept as
a thin compatibility wrapper.
- No DI framework, no package split, no config/index/route churn.
Validation:
- npx tsc --noEmit: clean
- Targeted run (composition, current-state retrieval regression,
route-validation, smoke): 19/19 pass
- Full test suite: 884 pass / 1 pre-existing fail (deployment-config
regex predates this change; confirmed via stash against main)
- fallow --no-cache: 0 above threshold, maintainability 90.9
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…me (phase 1a.5) Codex stop-time review flagged the runtime container's `config?` override as dishonest: the override only influenced repo construction (entityGraphEnabled, lessonsEnabled), while routes/, services/, and the search pipeline still read config directly from the module singleton at 25+ call sites. A consumer passing a custom config would silently get split-brain behavior — repos honoring the override, everything else ignoring it. Scope decision: - Preferred path (thread runtime.config through createMemoryRouter and MemoryService deps) was scoped and rejected for Phase 1A.5: any honest thread requires reaching memory-search.ts (4 reads) which delegates to search-pipeline.ts (35 reads), which is Phase 1B work. Stopping short would preserve the split-brain condition the reviewer flagged. - Safest honest fallback: remove the override. Keep runtime.config as a stable reference to the module singleton so consumers still have a single named entry point for config, but drop the promise that `createCoreRuntime` can take a different config. Changes: - CoreRuntimeDeps: drop `config?: CoreRuntimeConfig`. Only `pool` is accepted. JSDoc documents why: most config reads happen below this layer, so an override here would be silently ignored. - createCoreRuntime: read `config` from the module singleton directly for repo-construction flags. Single source of truth. - CoreRuntimeConfig type kept as `typeof config` with a comment flagging Phase 1B as the place proper config splitting + per-runtime override will land. - Tests: the flag-branch coverage (`entityGraphEnabled`, `lessonsEnabled`) now uses a small `withConfigFlag` helper that temporarily mutates the module singleton and restores on cleanup. That is an honest reflection of how the app actually works — config mutation via the live singleton is already the mechanism behind `PUT /memories/config`. Added one explicit test that `runtime.config` references the module singleton, so future drift is caught. Validation: - npx tsc --noEmit: clean - src/app/__tests__/runtime-container.test.ts: 9/9 pass - Full suite: 884 pass / 1 pre-existing fail (deployment-config regex) - fallow --no-cache: 0 above threshold, maintainability 90.9 Follow-ups deferred to Phase 1B: - Thread config through createMemoryRouter, MemoryService deps, and the search pipeline so per-runtime config overrides become honest. - Split config into CoreRuntimeConfig (public) + InternalPolicyConfig (experimental), per the rearchitecture plan. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR #6 changed docker-compose.yml ports from "3050:3050" to "${APP_PORT:-3050}:3050" for side-by-side CI safety. The regex in deployment-config.test.ts:105 was written for the bare-digit shape and silently failed on the env-var substitution form, leaving the app-port exposure assertion broken since that PR landed. Update the assertion to use a new composePortBindingRegex(internalPort) helper that accepts either a literal external port or a ${VAR:-default} shell-variable substitution. The helper makes the pattern intent explicit and prevents the same drift from recurring on future compose changes. Restores the deployment-config test suite to a clean baseline (15/15 pass). Recommended in the Phase 1A follow-up analysis as the smallest move to restore baseline integrity before Phase 1B. Known follow-up: src/__tests__/route-validation.test.ts "skip_extraction (storeVerbatim)" returns 500 in some local environments. Out of scope for this fix; investigate separately before declaring full suite green. Validation: - npx tsc --noEmit: clean - npx vitest run src/__tests__/deployment-config.test.ts: 15/15 pass - fallow --no-cache: 0 above threshold, maintainability 90.9 unchanged Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Memo-only artifact in docs/design/. Documents the source-grounded audit that informed the Phase 1A refactor (ff57540, 0da468e) and the minimum safe sequence for Phase 1B onward. Contents: - Hazards that existed before Phase 1A: search-pipeline module-level entityRepo with setter, server.ts as singleton composition root with bootstrap-on-import, parallel DI paths for the entity repo - What Phase 1A resolved: runtime-container composition root, createApp factory, startup-checks extraction, module-global removal, search-pipeline functions now take entityRepo as a param - What Phase 1A deliberately did NOT do: config override on createCoreRuntime (53 call sites still import config directly), process-global handler extraction, server.ts re-export surface - Remaining hazards post-Phase 1A, with exact file:surface pointers - Files and tests to watch during Phase 1B+ churn - Phase 1B through 1E follow-on sequence with independently testable and reversible exit criteria Source-grounded throughout — every reference is either a commit SHA or a path within src/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…composed boot Memo-only artifact in docs/design/. Records the one integration test still missing after 0da468e: boot createApp(createCoreRuntime(...)) and prove config-facing HTTP behavior matches the singleton-backed runtime end-to-end. Contents: - What is missing, in one sentence - Why runtime-container.test.ts is insufficient — source-grounded at src/app/__tests__/runtime-container.test.ts lines 17, 42-80, 119-126 - Why route-validation.test.ts is insufficient — it bypasses createCoreRuntime and createApp by design - Test shape (not implementation): boots app.listen(0), at least one write + read HTTP round-trip through the composed graph, parity assertion against a reference - Narrow acceptance criteria (6 items, each binary) - Explicit non-goals (route-validation, per-endpoint business logic, config-threading parity across 53 sites — those are other tests) No code, no test file. This memo only defines what "done" looks like so the test can be written in a focused follow-up. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Closes the gap identified in docs/design/phase-1a-composed-boot-parity-test.md
after 0da468e: no live-HTTP test existed proving that
createApp(createCoreRuntime({ pool })) produces a server whose
observable behavior matches the singleton-backed reference.
The existing runtime-container.test.ts is composition-shape only
(stubbed pool, no SQL, no requests issued). The existing
route-validation.test.ts is route-logic only (deliberately bypasses
the composition seam, hand-wires its own MemoryService). Neither
covered the seam itself.
This test boots two ephemeral Express listeners — one composed via
the Phase 1A seam, one hand-wired in the legacy shape — and asserts:
1. GET /memories/health returns identical config payloads from both,
proving config flows correctly through composition into routes.
2. GET /memories/stats returns identical stats from both, proving the
full route → service → repo → pool graph is connected end-to-end
through the composition seam (this query actually hits the DB).
Two requests are sufficient: more would shade into per-endpoint
behavior coverage, which is route-validation.test.ts territory and
explicitly out of scope per the design doc's acceptance criteria.
Validation:
- npx tsc --noEmit: clean
- vitest run src/app/__tests__/composed-boot-parity.test.ts: 2/2 pass
- vitest run src/app/__tests__/: 11/11 pass (no regression in
runtime-container.test.ts)
- fallow --no-cache: 0 above threshold, maintainability 90.9 unchanged
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
codex2 review of ba7fd6e flagged the remaining gap: composed-boot
parity covered config-facing READS (GET /memories/health,
GET /memories/stats) but not the config-facing WRITE seam
(PUT /memories/config), which is the one route that mutates the
module-level config singleton end-to-end.
Add a single test case to the existing composed-boot-parity file:
1. PUT /memories/config on the composed app with a sentinel
max_search_results value (current + 17, safely past validation
floor).
2. Assert the PUT response surfaces { applied, config } with the
sentinel applied.
3. GET /memories/health on BOTH the composed app and the reference
app and assert both surface the sentinel — proving the composed
write seam mutates the same singleton the reference app reads.
4. finally{} restores the original value via direct config mutation
(not a follow-up PUT) so cleanup does not depend on either server
still being healthy at teardown.
Test ordering is deliberate: this test runs LAST in the file so any
finally{} hiccup cannot bleed sentinel state into the existing GET
parity tests above.
Validation:
- npx tsc --noEmit: clean
- vitest run src/app/__tests__/composed-boot-parity.test.ts: 3/3 pass
- vitest run src/app/__tests__/{composed-boot-parity,runtime-container}.test.ts: 12/12 pass (no regression)
- fallow --no-cache: 0 above threshold, maintainability 90.9 unchanged
- git diff --stat: 1 file changed, +39/-0
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
ethanj
added a commit
that referenced
this pull request
Apr 19, 2026
Cuts over `main` to the Phase 1A–7 rearchitecture (composition root, explicit scope/observability contracts, store-narrowed repository access, public consumption seams, config split, leaf-module config threading, retrieval orchestration polish) plus the OSS-release-prep on top.⚠️ Breaking changes for HTTP / SDK consumers - All API endpoints are now mounted under `/v1` (e.g. `POST /v1/memories/ingest`, `PUT /v1/agents/trust`). The unversioned `/health` liveness probe is unchanged. - Workspace `GET /memories/list`, `GET /memories/:id`, and `DELETE /memories/:id` now require `agent_id` when `workspace_id` is present; missing returns 400 (no visibility-unsafe fallback). - `PUT /memories/config` returns 410 Gone in production. Provider/model fields (embedding_provider, embedding_model, llm_provider, llm_model) are rejected with 400 — they were never honored mid-flight (provider caches are fixed at first use). Set via env at process start. - npm package renamed `@atomicmemory/atomicmemory-engine` → `@atomicmemory/atomicmemory-core`. Tarball now ships `dist/` (built via `tsc`); `main`/`types`/`exports` point at compiled output. - Deep-importers of `services/embedding` and `services/llm` must call `initEmbedding(config)` / `initLlm(config)` before hot-path APIs. Consumers using `createCoreRuntime({ pool })` are auto-initialized. Rearchitecture (Phases 1A–7) - Phase 1A: composition root via `createCoreRuntime` + `createApp` (#8) - Phase 2A: canonical search scope contract (#9) - Phase 2B: explicit retrieval observability contract (#10) - Phase 3: runtime-config seam cleanup to Phase 4 boundary (#11) - Phase 4: ingest pipeline decomposition (475 → 215 lines) (#13) - Post-Phase 4: unify scope contract via `scopedSearch`/`scopedExpand`, document schema scoping gaps as deferred (#15) - Phase 5: narrow repository access behind 8 domain-facing stores; workspace ingest now flows through canonical lineage (#16) - Phase 6: publish stable consumption seams (HTTP, in-process, docker) with two-direction parity contract test (#17) - Phase 7 Steps 3a–3c: split runtime config into supported/internal partitions; deprecate `PUT /memories/config` for production (#18) - Phase 7 Step 3d: thread config through 5 leaf modules (33→28 singleton audit) (#21) - Phase 7 Item 4: retrieval polish — `memory-search.ts` reduced to pure orchestration (374 → 248 lines, -34%) (#22) - Chore: reduce fallow duplication 367 → 234 lines (#12) OSS release prep - `"private": true` removed; package renamed to `@atomicmemory/atomicmemory-core`. `files` field scopes the tarball. - `tsconfig.build.json` + `prepublishOnly` so `npm publish` always ships compiled `dist/`. Bare-import smoke test passes. - `release.yml` publishes to public npm on tag push (NPM_TOKEN secret). - SuperMem codename scrubbed from `src/`, tests, docker-compose, and `.env.example` (DB user/name/password renamed to `atomicmemory`). - Private-research-repo URLs unlinked from public docs. - README links to docs.atomicmemory.ai. - `/v1` API prefix on all routes; mount-coverage test added. - CI workflow: set `CORE_RUNTIME_CONFIG_MUTATION_ENABLED=true` to match `.env.test` (gitignored) and unblock the composed-boot-parity test on `PUT /v1/memories/config`. Verification - 966/966 tests pass (100 files) - npx tsc --noEmit clean - fallow --no-cache: 0 above threshold (maintainability 91.0) - npm publish --dry-run succeeds
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Atomicmemory-coreserver.tsand remove the old global entity-repository wiring pathGET /memories/health,GET /memories/stats, andPUT /memories/configValidation
npx tsc --noEmit27/27passing across runtime-container, composed-boot parity, and deployment-config coveragefallow --no-cachewith0above-threshold issues (maintainability90.9)Notes / non-claims
src/__tests__/route-validation.test.tsstill has a pre-existingstoreVerbatim500 failure outside this slice