Decouple cross-artifact runtime imports and finalize bus contract
Layer-boundary cleanup (audit-1-5.md section 1):
- fend → adom: move apply.ts to libs/dom; FrontendDom is now DomApplier;
fend falls back to bare applyChange instead of constructing ActiveDom
- sium → lang: full split — pure code (consts, types, guards, helpers,
errors, json, plural, plural_rules, diagnostics) moves to libs/lang;
arts/lang keeps engine + active wrappers and re-exports for back-compat
- conn → timr: minimum split — types and public constants move to
libs/timers; conn imports types from $libs/timers; EngineConnectionsOptions.timers
is now required (no more silent createEngineTimers fallback). Tests
updated to construct shared timers per beforeEach
Bus contract finishing touches (continued from prior session):
- subscribe() returns the unsubscribe function directly
- publishCausedBy() propagates correlationId/causationId
- invokeListener hook + Svelte adapter wraps listeners in untrack
- maxReentrancyDepth guard with BusReentrancyLimitError (fatal — bypasses
listener-error trap)
- DEV-mode structuredClone payload check raising BusInvalidPayloadError
- BusListenerFailure carries envelopeId/correlationId/causationId
- createBusRecent active wrapper, bus-context.svelte.ts, APP_EVENT_RUNTIMES
table with assertEventCanFire wired into every publishApp* helper
- App switches to createSvelteEngineBus
audit-1-5.md captures the full ecosystem audit (5 axes); section 1 is
now closed by these changes. Sections 2-5 remain open.
Verification: svelte-check 1395/0 errors; server 1230 tests passed;
client 19 tests passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 months ago
|
|
|
|
# Active framework — ecosystem audit (arts / libs / svrs)
|
|
|
|
|
|
|
|
|
|
|
|
Scope: `src/arts/`, `src/libs/`, `src/svrs/`. The `src/web/` folder is
|
|
|
|
|
|
explicitly excluded. Findings are grouped by audit axis. Severity tags:
|
|
|
|
|
|
|
|
|
|
|
|
- **bug** — incorrect runtime behaviour
|
|
|
|
|
|
- **violation** — breaks an established framework rule
|
|
|
|
|
|
- **drift** — code or docs out of sync with the rest
|
|
|
|
|
|
- **opportunity** — refactor / clean-up that is not a bug today
|
|
|
|
|
|
|
|
|
|
|
|
Each item includes file paths and line ranges so the fix can be applied
|
|
|
|
|
|
without re-searching. Items are ordered roughly by priority within each
|
|
|
|
|
|
section.
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## 1. Layer-boundary violations
|
|
|
|
|
|
|
|
|
|
|
|
Rule: `arts/<X>` may import from `$libs/*` and from its own internal
|
|
|
|
|
|
files; cross-artifact imports (`$<other-artifact>`) are reserved for the
|
|
|
|
|
|
composition root `arts/aapp`. `libs/*` may not import from any
|
|
|
|
|
|
`$<artifact>` alias. `svrs/*` is similarly restricted.
|
|
|
|
|
|
|
|
|
|
|
|
### Real coupling (runtime, not test) — **violation**
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/fend/active-frontend.svelte.ts:2` —
|
|
|
|
|
|
`import { createActiveDom, type ActiveDom } from '$adom';`
|
|
|
|
|
|
`arts/fend` reaches into `arts/adom`. The DOM port should live in
|
|
|
|
|
|
`libs/dom` and `fend` should accept an interface, leaving the DOM
|
|
|
|
|
|
engine wiring to `aapp`.
|
|
|
|
|
|
- `src/arts/sium/engine-resolver.ts:1-2` —
|
|
|
|
|
|
`import { ID_FALLBACK_SEPARATOR, parseLangRef } from '$lang';`
|
|
|
|
|
|
`arts/sium` imports runtime symbols from `arts/lang`. These two
|
|
|
|
|
|
values are pure helpers and belong in `libs/lang` so `sium` can
|
|
|
|
|
|
consume them without crossing artifacts.
|
|
|
|
|
|
- `src/arts/conn/engine-connections.ts:1` —
|
|
|
|
|
|
`import { createEngineTimers } from '$timr';`
|
|
|
|
|
|
`arts/conn` constructs its own timer engine instead of accepting a
|
|
|
|
|
|
`TimerScheduler` through options. The pattern elsewhere (sess, cach,
|
|
|
|
|
|
perm) is "App injects Timers"; conn should follow it.
|
|
|
|
|
|
|
|
|
|
|
|
### Type-only cross-artifact imports — **opportunity**
|
|
|
|
|
|
|
|
|
|
|
|
These compile away but still couple the two artifacts at the type
|
|
|
|
|
|
level. They should be promoted to `libs/<X>/types.ts` so consumers
|
|
|
|
|
|
import the contract without naming the engine.
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/auth/client.ts:2` — `import type { EngineHttp, HttpBodyInit, HttpResult } from '$http';`
|
|
|
|
|
|
- `src/arts/sess/types.ts:33` — `import type { SyncStorageAdapter } from '$stor';`
|
|
|
|
|
|
- `src/arts/sium/engine-resolver.ts:1` — `import type { EngineLang, LangParams, SupportedLocale } from '$lang';`
|
|
|
|
|
|
- `src/arts/conn/types.ts` and related — `import type { TimerScheduler, ... } from '$timr';`
|
|
|
|
|
|
|
|
|
|
|
|
### `svrs/*` and `aapp` — clean
|
|
|
|
|
|
|
|
|
|
|
|
`src/svrs/*` does not import any `$<artifact>` alias. `arts/aapp` is
|
|
|
|
|
|
the composition root and is allowed to import every engine; the
|
|
|
|
|
|
imports there are intentional.
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## 2. Naming convention drift
|
|
|
|
|
|
|
|
|
|
|
|
Rule: every module-event / method / diagnostic value must be a
|
|
|
|
|
|
**module-scoped lowercase string** (`'sess.lifecycle.adopted'`,
|
|
|
|
|
|
`'cach.delete'`, `'conn.connected'`, `'buss.event.published'`). Bare
|
|
|
|
|
|
names (`'delete'`, `'hit'`, `'auth_failed'`) collide across artifacts
|
|
|
|
|
|
once aggregated in logs or wired through the bus.
|
|
|
|
|
|
|
|
|
|
|
|
### Unscoped diagnostic event values — **violation**
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/conn/consts.ts:4-20` — `CONNECTION_DIAGNOSTIC_EVENTS` has
|
|
|
|
|
|
15 bare values: `'auth_failed'`, `'browser_reconnect'`,
|
|
|
|
|
|
`'connect_failed'`, `'frame_decode_failed'`, `'frame_encode_failed'`,
|
|
|
|
|
|
`'heartbeat_timeout'`, `'listener_threw'`, `'reauth_failed'`,
|
|
|
|
|
|
`'reconnect_exhausted'`, `'send_failed'`, `'session_changed'`,
|
|
|
|
|
|
`'session_expired'`, `'session_refreshed'`, `'session_revoked'`,
|
|
|
|
|
|
`'transport_error'`. All should carry the `'conn.'` prefix.
|
|
|
|
|
|
- `src/arts/sess/consts.ts:4-20` — `SESSION_DIAGNOSTIC_EVENTS` has 15
|
|
|
|
|
|
bare values: `'adopt_server_invalid'`, `'disposed_access'`,
|
|
|
|
|
|
`'listener_threw'`, `'refresh_*'` (5×), `'revoke_global_*'` (3×),
|
|
|
|
|
|
`'storage_*'` (3×). All need `'sess.'`.
|
|
|
|
|
|
- `src/arts/perm/consts.ts:41-45` —
|
|
|
|
|
|
`PERMISSION_CLIENT_DIAGNOSTIC_EVENTS` has `'remote_batch_failed'`,
|
|
|
|
|
|
`'remote_check_failed'`, `'remote_what_failed'`. All need `'perm.'`.
|
|
|
|
|
|
- `src/svrs/perm/consts.ts:3-7` — `PERMISSION_DIAGNOSTIC_EVENTS` has
|
|
|
|
|
|
`'decision'`, `'denied'`, `'indeterminate'`. All need a `'perm.'`
|
|
|
|
|
|
prefix (or a server-specific scope, e.g. `'perm.server.decision'`).
|
|
|
|
|
|
|
|
|
|
|
|
### Unscoped method constants — **violation**
|
|
|
|
|
|
|
|
|
|
|
|
- `src/svrs/perm/consts.ts:19-25` — `PERMISSION_METHOD_*` constants
|
|
|
|
|
|
hold bare names (`'check'`, `'can'`, `'assert'`, `'explain'`,
|
|
|
|
|
|
`'what'`, `'who'`, `'filter'`). The client-side counterpart in
|
|
|
|
|
|
`arts/perm` already uses `'perm.check'`, `'perm.can'`, … — server
|
|
|
|
|
|
must align.
|
|
|
|
|
|
|
|
|
|
|
|
### Why this matters
|
|
|
|
|
|
|
|
|
|
|
|
`Logger.warn(category, message)` aggregates across artifacts. With
|
|
|
|
|
|
unscoped diagnostic strings, a value like `'listener_threw'` appears
|
|
|
|
|
|
under both `category: 'conn'` and `category: 'sess'` and is
|
|
|
|
|
|
indistinguishable in any flat log search.
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## 3. Error-class and dispose-pattern consistency
|
|
|
|
|
|
|
|
|
|
|
|
### Hard-coded error `name` strings — **violation**
|
|
|
|
|
|
|
|
|
|
|
|
The convention is `this.name = <MOD>_ERROR_NAME_*` (read from
|
|
|
|
|
|
`consts.ts`).
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/sium/core/types.ts:302` — `SiumValidationError` sets
|
|
|
|
|
|
`this.name = 'SiumValidationError'` literally.
|
|
|
|
|
|
- `src/arts/sium/core/types.ts:322` — `SiumAsyncSchemaError` same
|
|
|
|
|
|
pattern.
|
|
|
|
|
|
|
|
|
|
|
|
### Error classes without matching `is*Error` guards — **violation**
|
|
|
|
|
|
|
|
|
|
|
|
The pattern is "one class, one guard". Missing guards make `instanceof`
|
|
|
|
|
|
checks leak into call sites.
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/conn/errors.ts:49` — `ConnChannelAlreadyExistsError` no
|
|
|
|
|
|
`isConnChannelAlreadyExistsError`.
|
|
|
|
|
|
- `src/arts/conn/errors.ts:58` — `ConnChannelNotFoundError` no
|
|
|
|
|
|
`isConnChannelNotFoundError`.
|
|
|
|
|
|
- `src/libs/auth/errors.ts:24-66` — 10 auth error classes have no
|
|
|
|
|
|
guards: `AuthAccountNotLinkedError`, `AuthSessionRevokedError`,
|
|
|
|
|
|
`AuthAssuranceRequiredError`, `AuthRateLimitedError`,
|
|
|
|
|
|
`AuthTenantBoundaryError`, `AuthOAuthStateInvalidError`,
|
|
|
|
|
|
`AuthOAuthProviderError`, `AuthOtpInvalidError`,
|
|
|
|
|
|
`AuthMfaRequiredError`, `AuthWebAuthnError`.
|
|
|
|
|
|
|
|
|
|
|
|
### `dispose()` without idempotency guard — **bug**
|
|
|
|
|
|
|
|
|
|
|
|
The convention is `if (disposed) return; disposed = true; …`. Calling
|
|
|
|
|
|
`dispose()` twice should be a no-op.
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/conn/active-connections.svelte.ts:118` — second call would
|
|
|
|
|
|
re-traverse `detachers` (empty by then) and call `engine.dispose()`
|
|
|
|
|
|
again.
|
|
|
|
|
|
- `src/arts/fend/active-frontend.svelte.ts:185` — second call would
|
|
|
|
|
|
re-invoke `ownedDom?.dispose()` if the inner DOM was created here.
|
|
|
|
|
|
|
|
|
|
|
|
### `Logger` option not defaulting to `SILENT_LOGGER` — **violation**
|
|
|
|
|
|
|
|
|
|
|
|
Pattern in the codebase: `const logger = options.logger ?? SILENT_LOGGER;`
|
|
|
|
|
|
(see `arts/buss/engine-bus.ts:54`).
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/stor/engine-storage.ts` — `createStorageDiagnostics(options.logger)`
|
|
|
|
|
|
passes the `undefined` straight through.
|
|
|
|
|
|
- `src/arts/timr/engine-timers.ts` — same issue with
|
|
|
|
|
|
`createTimerDiagnostics`.
|
|
|
|
|
|
- `src/arts/http/engine-options.ts` — same issue with
|
|
|
|
|
|
`createHttpDiagnostics`.
|
|
|
|
|
|
|
|
|
|
|
|
If diagnostics ever calls a method on the logger without guarding for
|
|
|
|
|
|
`undefined`, these three engines crash when used without a logger
|
|
|
|
|
|
(e.g. in unit tests or smoke harnesses). Verify each diagnostics
|
|
|
|
|
|
constructor; if it already null-checks internally, demote to
|
|
|
|
|
|
**opportunity** for consistency only.
|
|
|
|
|
|
|
|
|
|
|
|
### Generic `throw new Error(...)` in runtime code — **opportunity**
|
|
|
|
|
|
|
|
|
|
|
|
Typed error classes carry the discriminating `name` plus optional
|
|
|
|
|
|
fields (cause, code). Generic throws lose that.
|
|
|
|
|
|
|
|
|
|
|
|
- `src/arts/lang/engine-lang.ts:124, 133` — circular-reference message
|
|
|
|
|
|
thrown as plain `Error`.
|
|
|
|
|
|
- `src/libs/perm/runtime.ts:154, 181` — throws
|
|
|
|
|
|
`Error(PERMISSION_ERROR_MSG_FILTER_REQUIRES_ACTOR)` where a
|
|
|
|
|
|
`PermissionInvalidFilterError` would fit.
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## 4. Dead code, casts, and `any`
|
|
|
|
|
|
|
|
|
|
|
|
Codebase is largely clean. Specific findings worth acting on:
|
|
|
|
|
|
|
|
|
|
|
|
- `src/svrs/auth/adapters/db.ts:40, 64, 72, 79, 111, 134` — six
|
|
|
|
|
|
repetitions of `as unknown as Readonly<Record<string, unknown>>`.
|
|
|
|
|
|
Consolidate into a `DbRow` type alias or a `coerceRow()` helper.
|
|
|
|
|
|
**opportunity**.
|
|
|
|
|
|
- `src/libs/buss/silent-bus.ts:137` — `} as unknown as EngineBus;`.
|
|
|
|
|
|
Acceptable because the no-op surface is intentionally generic, but
|
|
|
|
|
|
worth replacing with proper generic constraints when the object
|
|
|
|
|
|
literal grows. **opportunity**.
|
|
|
|
|
|
- `src/arts/lang/types.ts:83` — `HasParams<T>` uses `any` deliberately
|
|
|
|
|
|
(contravariant variance). Comment is in place. **no action**.
|
|
|
|
|
|
- `src/libs/days/_vendor/**` — multiple `@ts-ignore` and `TODO`
|
|
|
|
|
|
comments. Vendor code, leave as-is. **no action**.
|
|
|
|
|
|
|
|
|
|
|
|
No dead exports were found in the spot checks of `libs/buss`,
|
|
|
|
|
|
`libs/auth`, `arts/conn`, `libs/aapp/events.ts`. No duplicate type
|
|
|
|
|
|
definitions detected across the audited modules.
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## 5. Documentation drift
|
|
|
|
|
|
|
|
|
|
|
|
Only the high-traffic READMEs were sampled.
|
|
|
|
|
|
|
|
|
|
|
|
### `arts/buss/README.md`
|
|
|
|
|
|
|
|
|
|
|
|
- The session-translator example (around line 563) hardcodes `cause:
|
|
|
|
|
|
APP_USER_IDENTITY_CAUSE_SESSION_ADOPTED` for every event. The actual
|
|
|
|
|
|
translator (`arts/aapp/integrations/session-translator.ts:37`) calls
|
|
|
|
|
|
`resolveAppIdentityCause(payload.event)` to map each `EVENT_*` to
|
|
|
|
|
|
the correct cause. The example should mirror that mapping or it
|
|
|
|
|
|
teaches the wrong pattern. **drift**.
|
|
|
|
|
|
- The README was already partially refreshed earlier in this session;
|
|
|
|
|
|
re-check the "Implementation gaps → Done" list now that
|
|
|
|
|
|
`subscribe()`, `publishCausedBy()`, `invokeListener`, the depth
|
|
|
|
|
|
guard, the payload-cloneable check, the Svelte adapter, the
|
|
|
|
|
|
`createBusRecent` wrapper, the `bus-context.svelte.ts`, and the
|
|
|
|
|
|
`APP_EVENT_RUNTIMES` table all landed.
|
|
|
|
|
|
|
|
|
|
|
|
### `arts/sess/README.md`
|
|
|
|
|
|
|
|
|
|
|
|
- The README still references a `publishSessIdentityChanged` helper.
|
|
|
|
|
|
Actual export is `publishSessLifecycleEvent` in
|
|
|
|
|
|
`arts/sess/bus-helpers.ts`. Either rename in code or update the
|
|
|
|
|
|
README. **drift**.
|
|
|
|
|
|
|
|
|
|
|
|
### `arts/cach/README.md`
|
|
|
|
|
|
|
|
|
|
|
|
- README written in Spanish (a deliberate choice; not flagged as a
|
|
|
|
|
|
rule break). At lines 11–12 it suggests importing
|
|
|
|
|
|
`createEngineCache` from `$svrs/cach`, but the artifact's barrel
|
|
|
|
|
|
(`arts/cach/index.ts`) only exports `createActiveCache`. Add a one-
|
|
|
|
|
|
line note clarifying the engine vs active split. **drift**.
|
|
|
|
|
|
|
|
|
|
|
|
### Other artifacts (aapp, adom, auth, conn, fend, fmts, http, lang, logr, perm, sium, stor, timr)
|
|
|
|
|
|
|
|
|
|
|
|
Spot checks did not surface drift — claims match exports.
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## Suggested fix order
|
|
|
|
|
|
|
|
|
|
|
|
1. **Coupling fixes** (Section 1, runtime violations). They constrain
|
|
|
|
|
|
every other refactor: until `fend → adom`, `sium → lang`, and
|
|
|
|
|
|
`conn → timr` are decoupled, those artifacts cannot be tested in
|
|
|
|
|
|
isolation.
|
|
|
|
|
|
2. **Naming convention pass** (Section 2). Mechanical, scoped to four
|
|
|
|
|
|
`consts.ts` files, and unblocks coherent log aggregation. Update
|
|
|
|
|
|
any test that hard-codes the literal values.
|
|
|
|
|
|
3. **Error guards + idempotent `dispose`** (Section 3). Low-risk and
|
|
|
|
|
|
localised; prevents subtle bugs if any consumer ever calls
|
|
|
|
|
|
`dispose()` twice or tries to discriminate auth errors.
|
|
|
|
|
|
4. **Logger defaults** (Section 3). Verify the three diagnostics
|
|
|
|
|
|
constructors and add `?? SILENT_LOGGER` where missing.
|
|
|
|
|
|
5. **Documentation refresh** (Section 5). Easiest after the code
|
|
|
|
|
|
changes above so the README reflects the final shape.
|
|
|
|
|
|
6. **Cast consolidation** (Section 4). Pure refactor, ship it whenever
|
|
|
|
|
|
convenient.
|
|
|
|
|
|
|
|
|
|
|
|
Items in Section 4 (vendor code, justified `any`, intentional cast)
|
|
|
|
|
|
require **no action**.
|