refactor: apply the over-engineering audit — delete withApp, Port.many, and the drifted CLAUDE.md surface copy - #46
Merged
Merged
Conversation
…aller exports `bootFixture` already starts, stops on every exit path and rethrows a shutdown Defect, and every example and starter used it; `withApp` had no consumer but core's own four invariant specs, now on the `boot` fixture. `createDeferred`'s `settled` flag guarded nothing — a promise's own `resolve` is idempotent by spec. `logLevel` is un-exported (one caller, eight lines below it), `flush` is `node:timers/promises`, and run-main's comment names a file that exists.
Set ports had no consumer in any of the eight packages or ten examples — only their own specs, a type test and a how-to page. The exemption they needed had rippled into `plan`'s levelling, which kept two count maps and a provider-identity Set to stop a set port's siblings being dropped when the first member landed; readiness is now one membership test. Gone with them: the MANY brand, ManyPortClass, MemberOf, the both-kinds wiring defect, and unsafeAddAll's grouping pass.
…d it The root CLAUDE.md carried a copy of all eight packages' public surfaces alongside the per-package CLAUDE.md, the README, docs/reference and the generated TypeDoc. The copy with no gate is the one that drifted: it described Logger.error/fatal as taking (message, cause?, attributes?) while logger.ts ships (message, attributes?, cause?) on all six methods and argues for that ordering in its own TSDoc. The kernel's surface moves to packages/core/CLAUDE.md, where the other seven already keep theirs; the root keeps the theses, the footgun, the two runtime contracts and the conventions, and a table saying where each surface lives. 1212 -> 801 lines.
The stated rule is that rationale lives in CLAUDE.md and a comment stays only where it guards a specific line against a plausible simplification. Applied to the nine largest inline blocks in core and testing: the drain's three beats, runMain's exit-code precedence, the probe-close drop, the ready() latch, the unit-fork placement and the tap's namespaced id all keep the guard and lose the essay. di's blocks are left alone deliberately — its CLAUDE.md records them as regression guards measured against a specific TypeScript version. TSDoc is untouched throughout: it is what TypeDoc renders into the API reference.
There was a problem hiding this comment.
Pull request overview
Repo-wide refactor to remove unused/over-engineered surface area across the DI container and test harness, and to relocate/align public-surface documentation so it doesn’t drift.
Changes:
- Deleted
withAppfrom@btravstack/testingand migrated remaining usages (notably core invariants) to thebootFixture-based workflow. - Removed set-port support from
@btravstack/di(Port.many/Provider.member) and simplified container planning/context plumbing accordingly. - Updated docs/specs (CLAUDE + VitePress reference/how-to pages + examples) to match the reduced API surface; small internal cleanups (e.g.
createDeferred, fake clock flush).
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/testing/src/with-app.ts | Removes the withApp callback harness implementation (deleted). |
| packages/testing/src/with-app.spec.ts | Removes withApp harness behavior tests (deleted). |
| packages/testing/src/tapped.ts | Trims/rewrites the Tap port rationale comment. |
| packages/testing/src/index.ts | Removes withApp from the public barrel exports. |
| packages/testing/src/fake-clock.ts | Replaces custom macrotask flush with node:timers/promises delay. |
| packages/testing/src/boot-fixture.ts | Updates fixture docs/comments after withApp removal; retains teardown semantics. |
| packages/testing/README.md | Removes withApp from README surface docs and updates teardown description. |
| packages/testing/CLAUDE.md | Updates harness surface/spec to remove withApp references and adjust wording. |
| packages/observability/src/index.ts | Stops re-exporting logLevel from the main entry point. |
| packages/di/src/provider.ts | Removes Provider.member and simplifies Provider export shape. |
| packages/di/src/port.ts | Removes Port.many + set-port types/brands; simplifies Port export. |
| packages/di/src/many.test-d.ts | Removes type-level tests for set-port behavior (deleted). |
| packages/di/src/many.spec.ts | Removes runtime tests for set-port accumulation/collisions (deleted). |
| packages/di/src/index.ts | Updates exported types to drop ManyPortClass etc. and updates comments. |
| packages/di/src/context.ts | Simplifies unsafeAddAll now that set-port accumulation is gone. |
| packages/di/src/build.ts | Simplifies plan() leveling/readiness now that set ports are gone. |
| packages/di/README.md | Removes mention of Port.many as a feature. |
| packages/di/CLAUDE.md | Updates DI package spec to reflect removal of set ports. |
| packages/core/src/test-fixtures.ts | Adds bootFixture to core test fixtures and exposes boot fixture. |
| packages/core/src/start.ts | Comment simplification + replaces withApp mentions with Boot. |
| packages/core/src/run-main.ts | Comment simplification + replaces withApp mention with bootFixture. |
| packages/core/src/probes.ts | Comment simplification around probe server close semantics. |
| packages/core/src/invariants.spec.ts | Migrates invariant tests from withApp to boot fixture usage. |
| packages/core/src/drain.ts | Reworks the high-level drain comment block for clarity/maintainability. |
| packages/core/src/docs-examples.test-d.ts | Updates compiled docs samples to use bootFixture instead of withApp. |
| packages/core/src/deferred.ts | Removes the internal settled-flag and relies on promise resolve idempotence. |
| packages/core/CLAUDE.md | Moves/expands the kernel public-surface spec into packages/core/CLAUDE.md. |
| examples/README.md | Updates narrative around removed set-port example coverage. |
| examples/hexagonal-order-api/src/emit-guards.ts | Removes set-port emit guards and adjusts commentary/tests accordingly. |
| docs/reference/testing.md | Removes withApp from the reference page and updates examples. |
| docs/reference/packages.md | Updates package entry-point list (still needs follow-up per comments). |
| docs/reference/di/wiring-defects.md | Removes wiring-defect text that only applied to set ports. |
| docs/reference/di/providers.md | Removes Provider.member section from reference docs. |
| docs/reference/di/ports.md | Removes Port.many section from reference docs. |
| docs/reference/di/entry-points.md | Removes set-port-specific Context.get description. |
| docs/reference/core/start.md | Updates StartGate docs to remove withApp mention. |
| docs/reference/core/probes.md | Updates probes doc to refer to bootFixture defaults instead of withApp. |
| docs/reference/core/events.md | Updates events doc to refer to bootFixture instead of withApp. |
| docs/how-to/test-an-application.md | Removes withApp how-to section and updates examples (still needs wording fix per comments). |
| docs/how-to/swap-an-adapter.md | Updates references to testing harness usage (removes withApp). |
| docs/how-to/open-a-per-request-scope.md | Updates guidance to remove withApp mention. |
| docs/how-to/embed-without-run-main.md | Updates cross-links to testing docs after withApp removal. |
| docs/how-to/build-a-plugin-registry.md | Removes the plugin-registry how-to page (deleted). |
| docs/explanation/nothing-throws.md | Updates “exceptions” list to remove withApp and focus on bootFixture. |
| docs/explanation/design-decisions.md | Updates design notes around the test harness (still needs a small fix per comments). |
| docs/explanation/compile-time-wiring.md | Updates explanation text to remove withApp mention. |
| docs/examples/hexagonal-order-api.md | Updates example narrative after ManyPortClass/set-port removal. |
| docs/.vitepress/config.ts | Removes sidebar link to the deleted plugin registry how-to page. |
| CLAUDE.md | Deletes the drift-prone surface copy and replaces with per-package surface pointers. |
| .changeset/ponytail-audit.md | Adds a changeset documenting removal of set ports and withApp. |
Suppressed comments (1)
packages/observability/src/index.ts:6
logLevelis no longer exported from@btravstack/observability’s main entry point, but multiple docs still list it as part of the surface (e.g.docs/api/index.md,docs/reference/observability.md, andpackages/observability/CLAUDE.md). Those should be updated to match the new export set.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… mangled
logLevel goes back on @btravstack/observability's entry point. Un-exporting it
was judged on internal caller count, but this is a library: docs/reference
teaches an application composing its own config schema with
`logLevel({ default: "debug" })`, which is a real external consumer.
The rest were string replacements that left bad prose: 'Five tools' with an
orphaned withApp clause, bootFixture listed twice, and an events.md sentence
claiming bootFixture forces probes off — it defaults them off, and a call may
still ask for a port. Invariant 4 had grown a second expect, which the Test
conventions forbid; both instants are now one projection.
docs/api/index.md and typedoc.di.json drop the removed di symbols.
packages/di/CHANGELOG.md keeps its Port.many entry: 0.1.0 did ship it, and a
changelog that edits its own history is worse than one that reads as dated.
This was referenced Aug 16, 2026
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
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.
A repo-wide over-engineering audit, applied. 50 files, −1,238 net lines.
Five of the fourteen findings were wrong, and reading the files is what
found that out — those are listed below too, because the reasons are worth
keeping.
Applied
withAppdeleted (@btravstack/testing)bootFixturealready does the job — start, stop on every exit path, rethrow a shutdownDefect— inside thetest.extendprotocol the Test conventions mandate. Every example and starter already used it; only the kernel's four invariant specs still calledwithApp, now on thebootfixture. −171 src.Port.many/Provider.memberremoved (@btravstack/di)plan's levelling, which kept two count maps and a provider-identitySetso a set port's later members were not dropped once the first landed; readiness is now one membership test. Gone with it: theMANYbrand,ManyPortClass,MemberOf,unsafeAddAll's grouping pass, and the "registered as both a set port and an ordinary port" wiring defect. −290 src.createDeferred'ssettledflagresolveis idempotent by spec, so a second SIGTERM could never rewrite the reason an application stopped with or without it.CLAUDE.md's surface copyCLAUDE.md, its README,docs/referenceand generated TypeDoc. The copy with no gate is the one that drifted (below). The kernel's surface moves topackages/core/CLAUDE.md, where the other seven already keep theirs.CLAUDE.mddescribedLogger.error/fatalas taking(message, cause?, attributes?);logger.tsships(message, attributes?, cause?)on all six methods and argues for that ordering in its own TSDoc.logLevelun-exported (one caller, eight lines below it);flush→node:timers/promises;run-main.tsciteduncaught.ts, a file that does not exist.coreandtesting, applying the repo's own stated test: rationale lives inCLAUDE.md, a comment stays only where it guards a specific line against a plausible simplification.di's comment blocks are left alone deliberately — itsCLAUDE.mdrecordsthem as regression guards measured against a specific TypeScript version.
TSDoc is untouched throughout: it is what TypeDoc renders into
/api/.Rejected on inspection
Each of these looked like one-caller indirection from a line count and is
load-bearing from inside — the deletion would have made an existing test
unwritable.
httpModule'shandlerparameter — has a second real implementation, abare node listener, and it is the only thing exercising the
500andsocket-destroy fallbacks that oRPC's surface cannot reach.
packages/http/src/handler.ts(24 lines, one importer) — acycle-breaking leaf:
orpc.tsneeds the port,http-runtime.tsneedsorpc. Folding it puts a circular import around a class declaration.order-*-contractworkspaces — each carries a client-half specbuilding a typed client from the contract alone (no
@orpc/server) plus alayering.test-d.tswhose entire proof is the package boundary. The secondconsumer was inside the workspace I proposed deleting.
examples/hexagonal-order-api— the only workspace that compiles twice(repo TS 7.0.2 +
typescript-consumer5.9.3) and re-checks the emitted.d.ts.packages/dicannot be its own downstream consumer, so movingemit-guards.tsthere deletes the guard rather than relocating it.docs/reference/(3,650 lines) — the signature blocks duplicate TypeDoc,but the tables, defaults, precedence rules and glossary do not, and 57
inbound links from the surviving Diátaxis tiers depend on them.
The comment finding was also mis-sized: of the 44% comment ratio, two thirds is
TSDoc. Real cuttable pool was ~430 lines, not ~900.
Not done
Promise.withResolvers()would deletedeferred.tsoutright, butlibispinned to
ES2023by@btravstack/tsconfig— a different repo — so it needs alocal
liboverride in eight tsconfigs to save 20 lines. Bumping the publishedenginesfloor was tried and reverted: an unrelated breaking change to acompatibility promise, smuggled in for a tidy-up.
Gate
format --check,lint,typecheck(28/28),knip,build(9/9) green.test: 24/24 tasks pass.packages/amqpandexamples/order-amqp-workerwere not run — they need a Docker daemon for their RabbitMQ container and
none was available locally. CI covers them.
Changeset:
minoron@btravstack/diand@btravstack/testingfor the twoAPI removals.