You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

260 lines
11 KiB

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# 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**.

Powered by TurnKey Linux.