fix: harden migration command execution

Two problems in the migration commands: no timeouts, and SQL split on literal
bytes.

`migrations:plan` and `migrations:apply` constructed their connection with
neither a connection deadline nor an operation deadline — a real server reported
`statement_timeout = 0` on the migration connection. Both now default to 10
seconds for the connection deadline, the client query timeout, and PostgreSQL's
own statement_timeout, applied at the one shared construction boundary so a
future call site cannot silently omit them. History reads, advisory-lock
queries, transaction control, migration SQL, and history writes are all covered.
The existing lock-wait deadline, the Pass 2 TLS policy, and Doctor's and the
bootstrap CLI's own timeouts are untouched, and no environment variable or flag
was added.

Explicit client fields do not win under the pinned driver. [email protected].0 builds
parameters as `Object.assign({}, config, parse(config.connectionString))`, so
the URL overrides passed options — verified directly, where an explicit
statement_timeout of 10000 alongside `?statement_timeout=0` produced a
server-reported 0, while a clean URL produced 10s. A DATABASE_URL carrying
connect_timeout, query_timeout, statement_timeout, or an `options` value setting
statement_timeout is therefore refused with the existing fixed configuration
error, before any client is constructed and without echoing the URL, parameter,
or value. Unrelated parameters are left alone.

The runner split each migration on the literal `--> statement-breakpoint` bytes.
Those bytes inside a string literal, a dollar-quoted function body, a quoted
identifier, or a comment tore the migration into fragments that were no longer
valid SQL — each case verified to produce two fragments where one was required.
Each migration's exact SQL is now passed to the client once, with no values and
no prepared-statement name, inside the existing transaction, and its history row
inserted only after that batch succeeds. One transaction still spans all pending
migrations; advisory locking, checksum validation, approval preflight, ordering,
rollback, cleanup, and sanitized error identity are unchanged. The port method
is renamed executeStatement to executeMigrationSql so the contract says what it
receives.

PostgreSQL parses the batch because it owns the only correct parser. A homegrown
splitter would have to track single quotes, dollar quoting with arbitrary tags,
quoted identifiers, line comments, nested block comments, and escape syntax to
answer a question the server already answers, and a parser dependency would add
supply-chain surface for the same reason. Drizzle's marker lines are ordinary
`--` comments when the batch is sent whole. No custom SQL parser exists here.

Co-Authored-By: Claude Opus 5 <[email protected]>

docs/architecture/decisions/0020-module-runtime-and-migration-execution-hardening.md

@@ -0,0 +1,216 @@
+# 0020: Module runtime and migration execution hardening
+
+- Status: Accepted
+- Date: 2026-07-27
+- Amends: [0009](0009-transactional-module-migrations.md), [0015](0015-static-module-http-routing-and-runtime-lifecycle.md)
+- Extends: [0017](0017-public-request-perimeter-hardening.md), [0019](0019-durable-authorization-history-and-grant-management.md)
+
+## Context
+
+Phase 5D passes 1–3 hardened the public perimeter, the identity/database transport, and
+authorization integrity. Pass 4 addresses the module runtime and the migration commands underneath
+them. Seven defects were reproduced against the accepted pass-3 tree before anything changed.
+
+**Registration order decided which permission governed a request.** A module may register both
+`GET /events/:id` and `GET /events/new`. Driven through the pinned router, `/events/new` reached the
+parameter handler with `demo.events.read` when the parameter route registered first, and the static
+handler with `demo.events.create` when it registered second. Nothing rejected the pair. A permission
+policy was being decided by array order.
+
+**Decoded parameters looked safer than they are.** The declaration grammar admits only lowercase
+kebab-case literals and `:name` segments, but that constrains declarations, not requests. Verified
+through real requests: `%2F..%2Fadmin` arrives as `/../admin`, `a%5Cb` as `a\b`, `a%00b` with an
+embedded NUL, `a%0D%0Ab` with CR/LF, `1%3B%20drop` as `1; drop`, and `%C3%A9v%C3%A9nement` as
+`événement`.
+
+**A module that failed to start was never cleaned up.** `startModules` appended to its rollback set
+only *after* `start()` resolved, so a hook that acquired a resource and then threw had its own
+`stop()` skipped while every previously completed module was rolled back.
+
+**Nothing bounded shutdown.** `RunningApi.shutdown()` awaited `server.close()` with no deadline, so
+one open connection could hold it forever, and each module `stop()`/rollback hook was awaited
+without a deadline, so a hook that never settled prevented pool closure indefinitely.
+
+**Migration commands had no timeouts.** `migrations:plan` and `migrations:apply` constructed their
+connection with neither a connection deadline nor an operation deadline; a real server confirmed
+`statement_timeout = 0` on the migration connection.
+
+**Migration SQL was split on literal bytes.** The runner did
+`migration.sql.split("--> statement-breakpoint")`. Those bytes inside a string literal, a
+dollar-quoted body, a quoted identifier, or a comment split the migration into fragments that were
+no longer valid SQL — each verified to produce two fragments where one was required.
+
+## Decision
+
+### Overlapping same-module routes are rejected, not ranked
+
+Two routes of one module overlap when they share an HTTP method and a segment count, and at every
+position either both segments are the same static literal or at least one is a parameter. Under
+this grammar — static literals or `:name` parameters, with no wildcard, regex, optional, or
+catch-all segment — that condition is exactly "some concrete request path matches both".
+
+`moduleRoutesOverlap` (`packages/contracts/src/module-http.ts`) implements it, kept framework-neutral
+because path semantics already live in that package and Hono stays confined to `apps/api`. Core's
+staged registration checks it after the existing duplicate check, so identical shapes — including
+paths differing only in parameter name — keep returning `MODULE_ROUTE_DUPLICATE`, and a genuine
+non-duplicate intersection returns the new `MODULE_ROUTE_OVERLAP`. Detection happens during the
+registering module's staged registration, so the module publishes **none** of its staged routes or
+capabilities, no start hook runs, and the HTTP listener never binds.
+
+Rejection rather than specificity ordering is the point. A specificity rule ("static beats
+parameter") would silently pick a winner, and the thing being picked is which permission guards the
+path — that is a policy decision, not a routing preference, and it must not be inferred. Disjoint
+routes keep their registration and publication order untouched; different methods, different
+modules (each has its own platform-owned mount prefix), and different segment counts remain legal.
+No priority, specificity, wildcard, regex, catch-all, optional-segment, mount-prefix, or manifest
+field was added.
+
+### Decoded route parameters are untrusted, module-owned input
+
+`context.params` continues to carry the router's decoded string values in a frozen context.
+Decoding is not validation, and validating a *declaration* says nothing about what a *request* may
+send. Every handler must validate each parameter it uses against its own exact domain grammar —
+UUID, slug, date, numeric id, or whatever that module owns — before any database, filesystem,
+command, identifier, or other side-effecting use, and reject missing, repeated, malformed, or
+semantically invalid values with its own sanitized response. Re-encoding or a decode/re-encode round
+trip is not a substitute.
+
+No generic parameter grammar, validator service, path-normalization layer, or new route field was
+added: module parameters are module-owned domain types, and one platform-wide pattern would either
+reject legitimate values or approve meaningless ones. The platform keeps owning authentication,
+authorization, `Origin`, CORS, body limits, cache behavior, and uncaught-handler sanitization, and
+permission enforcement always completes before a handler — and therefore any parameter validation —
+can run.
+
+### A module that fails to start rolls itself back first
+
+A module joins the rollback set the moment its `start` hook is *invoked*. Its own `stop` hook runs
+first, then previously completed modules in reverse order. The original start issue stays first in
+`ModuleRuntimeError.issues`; a failure or timeout stopping the failing module is appended and never
+prevents earlier modules from being rolled back. Every applicable hook is invoked at most once, a
+runtime that fails to start is never returned, and every hook receives the same frozen
+`ModuleRuntimeServices` identity its start hook received. A module with no stop hook gets no
+synthetic callback.
+
+The consequence for module authors is explicit: **`stop()` may run after a partially completed
+`start()`** and must be safe and idempotent about resources it may or may not have acquired. The
+platform cannot undo arbitrary side effects a trusted module performed outside its stop hook, and
+does not claim to.
+
+### Fixed, finite deadlines for hooks and for HTTP drain
+
+- Per-module `stop`/rollback hook: `MODULE_STOP_HOOK_TIMEOUT_MS` = **10 000 ms**.
+- Graceful HTTP drain: `HTTP_DRAIN_TIMEOUT_MS` = **30 000 ms**.
+
+Neither is configurable in this pass — no environment variable, manifest field, runtime service, or
+flag. Narrow test-only overrides exist so tests need not wait out production durations.
+
+A hook that reaches its deadline records `MODULE_STOP_TIMEOUT` or `MODULE_ROLLBACK_TIMEOUT` and the
+sequence continues with the next module in reverse order. The whole module-stop sequence is finite
+because the module count is fixed at discovery: its worst case is
+`startedModuleCount × 10 000 ms`, **not** one global ten-second bound. Timers that lose the race are
+cleared, the losing promise gets a permanent rejection handler so a late throw can never become an
+unhandled rejection, and a timed-out hook is never awaited again. `stop()` remains at-most-once and
+memoizes one shared promise for concurrent and later callers.
+
+Shutdown stops accepting connections with `server.close()`, waits up to the drain deadline, and at
+the deadline closes idle and then active connections through Node's own public
+`closeIdleConnections`/`closeAllConnections` — after `server.close()`, never touching a private
+socket or server field — and stops waiting for the close callback. It then runs the bounded module
+stop sequence and closes the one shared pool. The accepted order
+`HTTP drain/forced close → bounded module stop → pool close` is unchanged, and a drain timeout or
+module-stop failure never skips later cleanup. Both outcomes are reported together as
+`ApiShutdownError`, carrying fixed booleans and the already-sanitized module error — never a
+request, socket, URL, credential, SQL, or stack detail. A forced drain is a shutdown *failure*, not
+a success; signal shutdown exits `0` only after a clean shutdown and `1` otherwise, after cleanup
+has been attempted.
+
+**A deadline bounds waiting, not execution.** An in-process promise cannot be canceled: a timed-out
+hook keeps running and may settle later, and force-closing a connection ends the transport without
+proving that arbitrary handler JavaScript was canceled. What is guaranteed is that shutdown no
+longer waits, and that the pool closes afterwards, so late database work fails. Graceful ordering
+holds until the drain deadline; after it, the platform closes connections and proceeds so process
+shutdown stays bounded.
+
+### Finite migration connection and operation defaults, enforced against driver precedence
+
+`createPostgresMigrationDatabase` — the one construction boundary every migration command shares —
+now defaults to `MIGRATION_CONNECTION_TIMEOUT_MS` = **10 000 ms** (`connectionTimeoutMillis`) and
+`MIGRATION_OPERATION_TIMEOUT_MS` = **10 000 ms** for both `query_timeout` (client-side) and
+`statement_timeout` (server-side). Defaults live there so a future CLI call site cannot silently
+omit them; explicit test overrides may still narrow them. History reads, advisory-lock queries,
+transaction control, migration SQL, and history writes are all covered. The existing migration
+lock-wait deadline and error codes, and the Pass 2 TLS/hostname/certificate policy, are unchanged,
+as are Doctor's and the bootstrap CLI's own timeouts. No timeout environment variable or flag was
+added.
+
+Explicit constructor fields do **not** win under the pinned driver. `[email protected]` builds parameters as
+`Object.assign({}, config, parse(config.connectionString))`, so the connection string overrides
+passed options — verified directly: an explicit `statement_timeout: 10000` alongside
+`?statement_timeout=0` produced a server-reported `statement_timeout` of `0`, while a clean URL
+produced `10s`. A `DATABASE_URL` carrying `connect_timeout`, `query_timeout`, `statement_timeout`,
+or an `options` value that sets `statement_timeout` is therefore refused with the existing fixed
+`POSTGRES_MIGRATION_CONFIGURATION_INVALID`, before any client is constructed and without echoing the
+URL, parameter, or value. The rejection is deliberately narrow — only settings that actually conflict
+with the selected policy; unrelated parameters are untouched.
+
+### Each migration is one PostgreSQL-parsed batch
+
+The runner no longer splits migration SQL. Each pending migration's exact string is passed once to
+the client — no values, no prepared-statement name, no trimming, normalizing, or rewriting — inside
+the existing transaction, and its history row is inserted only after that batch succeeds. One
+transaction still spans all pending migrations and their history records, and advisory locking,
+checksum validation, approval preflight, ordering, rollback, cleanup, and sanitized error identity
+are unchanged. The transaction port method was renamed `executeStatement` → `executeMigrationSql` so
+the contract states what it actually receives.
+
+PostgreSQL was chosen as the parser because it is the only correct one. A homegrown splitter would
+need to track single quotes, dollar quoting with arbitrary tags, quoted identifiers, line comments,
+block-comment nesting, and escape syntax — reimplementing a SQL lexer to answer a question the
+server already answers — and a parser dependency would add supply-chain surface for the same
+reason. Drizzle's `--> statement-breakpoint` lines are ordinary `--` comments when the batch is sent
+whole, so the same bytes inside a literal, dollar-quoted body, quoted identifier, or comment retain
+their ordinary PostgreSQL meaning. **No custom SQL parser exists in this repository.**
+
+## Alternatives
+
+- **Ordering static routes ahead of parameter routes** was rejected: it silently resolves an
+  ambiguity whose resolution is a permission decision, and it would make the governing permission a
+  function of an implementation detail rather than of a declaration.
+- **A platform-wide route-parameter grammar** was rejected: parameters are module-owned domain
+  types, so any single pattern is simultaneously too strict and too permissive.
+- **Normalizing or re-encoding decoded parameters** was rejected: it produces a value the module
+  never declared and still does not answer whether the value is a valid domain identifier.
+- **A second, dedicated rollback hook** was rejected: it would double the lifecycle surface to
+  express something `stop` already means, and every module would have to implement both.
+- **Making the deadlines configurable** was rejected for this pass: a configurable shutdown bound
+  invites setting it to infinity, which is the defect being fixed.
+- **Claiming a timed-out hook is terminated** was rejected as untrue; the honest guarantee is that
+  the platform stops waiting.
+- **Trusting explicit `pg` client fields over the connection string** was rejected once the pinned
+  driver was shown to do the opposite.
+- **A timeout environment variable** was rejected as out of scope; a fixed default that cannot be
+  weakened is the safer starting point.
+- **Writing a SQL statement splitter, or adding a SQL parser dependency** — see above.
+
+## Consequences
+
+An ambiguous same-method route pair fails module registration with a stable code instead of
+resolving by array order, and no request can reach a match whose permission depended on registration
+sequence. Decoded parameters are documented as untrusted input that handlers must validate, with no
+platform grammar implied. A module whose `start` fails is cleaned up through its own `stop` hook
+before earlier modules. Shutdown is finite in every stage, observable when it is not clean, and
+always reaches pool closure. Migration commands have finite connection, query, and statement
+deadlines that a connection string cannot weaken, and each migration reaches PostgreSQL byte-for-byte
+in a single parsed batch.
+
+This pass adds no production schema, migration, manifest field, `manifestVersion` change,
+`ModuleRuntimeServices` field, `ModuleDatabase` operation, route-declaration field, handler-context
+field, second runtime, route registry, or runtime pool; the three accepted Core migrations and their
+checksums are byte-identical, one API runtime still owns one physical pool, migrations remain
+explicit operator work that never runs at application startup, and Hono remains confined to
+`apps/api`. Modules remain trusted compiled backend code: they are **not** isolated by process,
+PostgreSQL role, schema, or tenant, and these deadlines do not sandbox them.
+
+**Phase 5D is not complete.** Pass 5 — deployment/supply-chain work and an independent
+whole-repository re-audit — remains outstanding, and Phase 6 remains blocked.