Skip to content

Forward Express app extension points through HalEngineConfig - #85

Open
lore-agent[bot] wants to merge 10 commits into
mainfrom
lore/implementation-loop/issue-38
Open

lore-agent[bot] wants to merge 10 commits into
mainfrom
lore/implementation-loop/issue-38

Conversation

@lore-agent

@lore-agent lore-agent Bot commented Sep 17, 2026

Copy link
Copy Markdown

createApp already declared additionalRoutes and errorHandler on HalAppOptions, but createHalEngine never forwarded them to createApp, and there was no way for a consumer to mount routes at / — every forwarded route landed under basePath. The practical consequence: a consumer who wanted to serve a front-end bundle beside the API either ran a second service or hand-built their own Express app, bypassing the transport layer entirely. Adding a route after createHalEngine returned also did not work because createApp registers a terminal 404 catch-all before returning, and Express matches routes in registration order.

This change adds rootRoutes?: (router: Router) => void to HalAppOptions in src/transport/createApp.ts. The callback receives a fresh Router mounted after the basePath router and before the catch-all 404, so a handler there can serve GET / while GET {basePath}/health still answers independently. src/config.ts is extended to declare additionalRoutes, rootRoutes, and errorHandler on HalEngineConfig['transport'] and forward all three through to createApp. The additionalRoutes and errorHandler fields existed on HalAppOptions before this branch; the only runtime gap was that createHalEngine never passed them. rootRoutes is new at both layers.

Neither callback is gated by auth.http; both inherit the CORS, JSON body-parsing, and cookie-parsing middleware that createApp mounts unconditionally. The post-construction limitation is preserved and now tested: a route added to engine.app after createHalEngine returns always answers 404 because the catch-all was registered first.

The ticket's strategy called for direct implementation — createApp and createHalEngine are both callable today, and fixing the forwarding gap is mechanical. No deviation from that strategy.

Three acceptance tests define done, all in the "Done when these pass" list in .lore/dod.md:

  • createApp.test.ts line 128: mounts a rootRoutes handler at / before the catch-all 404 — a rootRoutes callback serving GET / coexists with the engine's GET /hal/health.
  • createApp.test.ts line 143: lets rootRoutes at /health and the basePath health answer independently — the same path prefix at root and under basePath routes to each handler separately.
  • config.test.ts line 109: forwards transport.additionalRoutes to createApp so the route mounts under basePathcreateHalEngine with transport.additionalRoutes registering GET /ping makes GET /hal/ping answer 200.

All three cite their statements in specs/hal-engine-architecture/spec.md with inline ([validated by ...]) links. The spec also gained a new ## Engine Lifecycle section that cites the six pre-existing config.test.ts tests that previously lacked spec links, closing the re-lint/require-spec-link gap those tests had opened.

specs/hal-engine-architecture/spec.md gains ### App extension points stating the two mount points, that neither is covered by auth.http, that both inherit CORS and body/cookie parsing, that engine.app cannot be extended after createHalEngine returns, and the default error behaviour and GDPR / NIS-2 notes the ticket required.

Documentation changes: README.md and docs/getting-started.md show all three new transport fields with one-line comments. The getting-started.md listing is generated from example/full-config.ts under <!-- doc-block: example/full-config.ts#full-config -->; the three fields were added there and the doc block regenerated with npm run docs:fix. CHANGELOG.md carries an [Unreleased] entry written for an npm reader rather than a git log reader.

Items from the ticket's acceptance criteria that are out of scope for this branch and remain open: a safe default JSON error handler inside the engine ("Answer unhandled HTTP errors with a safe JSON response"), per-route authentication in the engine, and deferring the 404 or any post-hoc extension of engine.app.

Refs #38
Lore-Task: 8eb920ee-7d28-4b4d-a591-f8a0865a7f69

Three failing acceptance tests for issue #38:

- createApp rootRoutes: GET / via rootRoutes handler returns 404 (should be 200)
  because rootRoutes is absent from HalAppOptions.
- createApp rootRoutes coexistence: rootRoutes /health returns 404 (should be
  {source:'root'}) for same reason.
- createHalEngine forwarding: GET /hal/ping returns 404 (should be 200) because
  createHalEngine does not pass transport.additionalRoutes to createApp.

Adds App Extension Points section to specs/hal-engine-architecture/spec.md with
the three cited statements and DoD in .lore/dod.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown

🔍 Lore Spec Impact — advisory

This PR touches 17 statement(s) across 2 spec(s), and changes the validating tests alongside every one of them.

HAL Engine on npm · 7 statement(s)

specs/hal-engine-npm-release/spec.md

The logger a consumer supplies is the one that gets used
✓ this PR also changes the tests that validate it

A logger passed to createHalEngine receives the package's own log lines ([validated by: delivers the package's own lo…

validated by src/config.test.ts:94

The logger a consumer supplies is the one that gets used
✓ this PR also changes the tests that validate it

Supplying none leaves the built-in console logger in place ([validated by: leaves an already-supplied logger in place w…

validated by src/config.test.ts:94

The logger a consumer supplies is the one that gets used
✓ this PR also changes the tests that validate it

createHalEngine installs a logger only when the config names one, so a second engine naming none keeps the first one'…

validated by src/config.test.ts:94

Two config options that were declared and dropped
✓ this PR also changes the tests that validate it

orchestrator.hooks is declared and forwarded, so a hook passed through the factory fires ([validated by: forwards an …

validated by src/config.test.ts:94

Two config options that were declared and dropped
✓ this PR also changes the tests that validate it

transport.port reaches the server, and the resolution order is the start(port) argument, then transport.port, the…

validated by src/config.test.ts:94

Two config options that were declared and dropped
✓ this PR also changes the tests that validate it

An explicit start(port) still wins over the configured one ([validated by: lets an explicit start(port) win over the …

validated by src/config.test.ts:94

Two config options that were declared and dropped
✓ this PR also changes the tests that validate it

A port that cannot be bound rejects the promise start() returned, rather than surfacing as an unhandled error event…

validated by src/config.test.ts:94

Weaker signals (10) — linked by a spec, not proven by a test run

HAL Engine chat routes · 10 statement(s)

specs/hal-engine-chat-routes/spec.md

How the routes are mounted
✓ this PR also changes the tests that validate it

With no authMiddleware, POST /hal/chats is refused ([validated by: refuses to create a chat](../../src/transport/cr…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

Health still answers 200 alongside that refusal ([validated by: still answers health, so a denied chat route is not a…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

A path off the base path still answers 404 ([validated by: still answers 404 off the base path](../../src/transport/c…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

A supplied middleware is invoked ([validated by: invokes the middleware it was given](../../src/transport/createApp.tes…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

Once it attaches a user, the route is served ([validated by: serves the route once that middleware attaches a user](../…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

The chat routes mount only when a sessionStore is supplied; without one the path is 404 ([validated by: mounts no c…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

The store is still empty after a chat is created ([validated by: leaves the session store empty after a chat is created…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

Two apps built on one store do not see each other's chats ([validated by: does not share chats between two apps built o…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

An Express RequestHandler is accepted as an HttpAuthMiddleware ([validated by: accepts a plain express RequestHandl…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

How the routes are mounted
✓ this PR also changes the tests that validate it

An HttpAuthMiddleware is usable as an Express RequestHandler ([validated by: is itself usable as an express Request…

validated by src/transport/createApp.test.ts:82
via changed file src/transport/createApp.test.ts

28 new statement(s) have no test link yet.

Deterministic · graph baseline unknown (no ingested test run has stamped this repo) — line-precise coupling skipped · no tests run by this check

Lore Agent and others added 9 commits September 17, 2026 16:07
…t fields

Add `rootRoutes?: (router: Router) => void` to `HalAppOptions` in
`createApp.ts`, mounted after the basePath router and before the 404
catch-all. Forward `additionalRoutes`, `rootRoutes`, and `errorHandler`
from `HalEngineConfig.transport` to `createApp` in `createHalEngine`.
Remove now-unnecessary `as any` cast from config.test.ts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…est uncited tests

Six tests added in the previous round had no spec-link citation; the
re-lint/require-spec-link rule fails the build on any uncited test.
Add an Engine Lifecycle section to the architecture spec with one
statement per test, each carrying the required ([validated by ...]) link.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previous round added tests to src/config.test.ts, shifting seven
existing line-number anchors in specs/hal-engine-npm-release/spec.md
by one. CI's spec-anchor check reported all seven as stale/rotten.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
check-doc-blocks.mjs requires each <!-- doc-block: ... --> comment to be
immediately followed by a ```typescript fence (no blank line). Eight
fences in specs/hal-engine-architecture/spec.md had an intervening blank
line, causing the Documented code blocks CI step to fail with 16 findings.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add CHANGELOG.md entry for the three new transport options
(additionalRoutes, rootRoutes, errorHandler). Update README.md
transport interface listing with the three new fields. Update
example/full-config.ts with commented examples of all three, then
regenerate the full-config doc-block in docs/getting-started.md via
npm run docs:fix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
re-lint/max-comment-lines treats consecutive trailing // comments as a
block; reduce to a single line on errorHandler only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
check-doc-blocks.mjs requires the marker comment and the typescript
fence to be on consecutive lines; blank lines between them were causing
10 findings (5 marker/fence mismatches, 5 unmarked fences).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lore-agent lore-agent Bot changed the title Forward the Express app extension points through HalEngineConfig Forward Express app extension points through HalEngineConfig Sep 17, 2026
@lore-agent
lore-agent Bot marked this pull request as ready for review September 17, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants