Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"root": true,
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaVersion": 2022,
"sourceType": "module",
"project": "./tsconfig.json"
},
"plugins": ["@typescript-eslint"],
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/recommended",
"plugin:@typescript-eslint/recommended-requiring-type-checking"
],
"env": {
"browser": true,
"es2022": true
},
"rules": {
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],
"@typescript-eslint/explicit-function-return-type": "off",
"@typescript-eslint/no-explicit-any": "warn",
"@typescript-eslint/no-floating-promises": "error",
"@typescript-eslint/no-misused-promises": "error",
"@typescript-eslint/prefer-nullish-coalescing": "warn",
"@typescript-eslint/prefer-optional-chain": "warn",
"@typescript-eslint/strict-boolean-expressions": "off",
"no-console": "warn",
"no-debugger": "error",
"prefer-const": "error",
"no-var": "error",
"eqeqeq": ["error", "always"]
},
"ignorePatterns": ["dist/", "node_modules/", "**/*.test.ts"]
}
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ jobs:
cache: npm

- name: Install dependencies
run: npm install --no-audit --no-fund
run: npm ci --no-audit --no-fund

- name: Lint
run: npm run lint

- name: Type check
run: npm run typecheck
Expand Down
67 changes: 67 additions & 0 deletions AUDIT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Clean Code and SRP Audit

## Summary

- **Highest-leverage split:** remove WebSocket/WebTransport ownership from
`Client` into two transport implementations behind one internal transport
contract.
- `Client` currently changes for browser transport APIs, reconnect policy,
incoming wire decoding, topic subscriptions, offline persistence, and drain
orchestration.
- Incoming frame normalization is a meaningful protocol decision and should be
independently tested outside transport callbacks.
- `LocalStore` is cohesive despite managing two object stores: both exist for
one IndexedDB persistence actor and share one database lifecycle.
- CRDT and Moonshot modules are long enough to inspect but each has one actor
and should remain independent.

## Findings

| ID | Location | Category | Severity | Actors in conflict | Cost | Size | Behavior risk |
|---|---|---|---|---|---|---|---|
| BROWSER-SRP-1 | `src/client.ts:36-375` | SRP, mixed class | P1 | browser transport maintainers; reconnect policy; wire protocol; offline product; topic consumers | A browser API change and a persistence/drain change edit the same stateful client and lifecycle methods. | L | Medium |
| BROWSER-SRP-2 | `src/client.ts:226-245,378-417` | Function/module SRP | P2 | wire-contract consumers; transport callbacks | JSON parsing, validation/defaults, byte conversion, and listener dispatch are interleaved; malformed frames are silently discarded at the transport boundary. | M | Medium |
| BROWSER-CC-1 | `src/client.ts:175-196` | Hidden lifecycle side effect | P2 | application lifecycle; reconnect policy | Any close starts an unbounded reconnect loop unless the separate `reconnecting` flag was set by `close()`. | S | Medium |

## Actor and State Partition

| Partition | Fields/methods | Actor |
|---|---|---|
| Transport state | socket, WebTransport connection/reader/writer, connect/send/read/close | browser platform |
| Reconnect state | connected, reconnecting, delay policy | reliability |
| Wire codec | incoming JSON normalization, byte/header/offset conversion | protocol |
| Subscriptions | topic listener map, subscribe/unsubscribe/dispatch | SDK consumer |
| Offline drain | LocalStore, draining guard, pending replay/progress | offline-first product |

Resulting internal units:

- `WebSocketTransport` and `WebTransportTransport`, both owning real connection
state and implementing the same internal transport operations.
- `decodeIncomingRecord` in `wire.ts`, returning a typed record or `undefined`.
- `Client` retaining reconnect, subscription, offline drain, and public API
orchestration.

## Ordered Refactor Sequence

1. Characterize incoming record decoding and both transport lifecycle paths.
2. Move wire normalization unchanged into `wire.ts`.
3. Improve the decoder using explicit typed boundaries while preserving current
defaults and malformed-frame behavior.
4. Move WebSocket and WebTransport state/methods into two transport units.
5. Keep reconnect and offline draining in `Client`; do not create a forwarding
service layer.
6. Run tests, lint, typecheck, and build after every commit.

## Deferred

- Reconnect attempt limits and cancellation policy are product behavior and are
not changed by this structural pass.
- WebTransport integration requires a real browser/server environment; unit
tests use platform fakes.

## Out of Scope

- `LocalStore`: one persistence actor and one database lifecycle.
- `Topic`: one topic-handle abstraction over local history and live tailing.
- `LWWRegister`/HLC code: one CRDT actor.
- Moonshot read-only clients: one gateway-safe HTTP contract.
Loading
Loading