feat: extract @codeoid/core — framework-agnostic client core (P0, final) - #115
Merged
Conversation
The second half of the packages extraction (design §4): everything a frontend needs except the pixels, shared by web and the upcoming mobile app so transcript semantics and transport reliability can't drift per-client. @codeoid/core (packages/core, source-only, MIT, peer-deps on @codeoid/protocol): - CodeoidClient — the transport moved from web/src/lib/ws.ts: auth handshake, request/response correlation, heartbeat, jittered backoff reconnect. Now host-parameterized: capabilities + clientName are ConnectOptions (web passes parts/replay.chunked/replay.resume + "codeoid-web"), all timing (heartbeat, backoff, request timeout) is injectable (React Native backgrounding; real-timer tests), and the browser resume listeners are typed via a feature-detected globalThis view so the package compiles without the DOM lib. Native hosts call reconnectNow() from their own resume signal. - Message semantics: mergeDeltaInto + dedupeReplay kernels (the web Solid store now delegates to them inside produce(), so the merge semantics live in exactly one place) and a batteries-included MessageStore for hosts without a bespoke reactive layer — including ingest(), the full broadcast routing table (live/delta/snapshot/ chunked/incremental replay + resume-cursor advancement) that is easy to get subtly wrong when reimplemented. - ResumeCursors (class; web binds a singleton), display helpers (formatters, identity/provenance labels, approval scan, slash parser, URI sanitizers, UTC day buckets). Tailwind class mappers stay in web; ctxWindowColorClass now maps core's ctxWindowSeverity. Web: eight modules become re-export shims (zero import churn), the Solid store keeps its tuned fine-grained reactivity but shares the kernels, and the client construction site declares capabilities. Tests move where the code lives: 113 core tests under bun:test — including the CodeoidClient suite rewritten from vitest fake timers to injected real timings, plus new MessageStore/kernel/ingest coverage. Web keeps its Solid-store and colour-mapper tests (78). Release: release.yml publishes @codeoid/core (version-guarded) after protocol; needs its own npm Trusted Publisher + one-time bootstrap publish, same as protocol. Root suite 956 pass; web tsc/lint/tests/build green; daemon build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughA new ChangesCore package extraction and web integration
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
==========================================
+ Coverage 76.39% 77.76% +1.36%
==========================================
Files 70 79 +9
Lines 11605 12438 +833
==========================================
+ Hits 8866 9672 +806
- Misses 2739 2766 +27
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Bun (latest, in CI) installs file: deps as symlinks, so module resolution for @codeoid/core runs from the real packages/core/src path and up-walks to the repo-root node_modules for its @codeoid/protocol peer — which the web job never populated (it only installed inside web/). Locally this passed because the root workspace was installed. The release workflow already root-installs before build:web; CI now matches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/client.test.ts (1)
217-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exposing a synchronous status getter.
Tests rely on subscribing via
onStatusjust to read the current status synchronously (it's invoked immediately on subscribe). Aget status()onCodeoidClientwould make this and similar downstream consumers (e.g. debug tooling) simpler than the subscribe-then-read pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/client.test.ts` around lines 217 - 224, Add a synchronous status accessor on CodeoidClient so callers do not need to subscribe just to read the current state. Expose a get status() (or similarly named getter) that returns the client’s current ClientStatus, and have it read from the same internal state used by onStatus; update client.test.ts expectations or usage to prefer the getter where the status is only being read synchronously.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/client.ts`:
- Around line 417-513: The initial auth path in `#connectOnce` can hang forever
because authResolved is never time-bounded, so add a dedicated handshake timeout
around the WebSocket open/auth exchange and fail the promise if neither auth.ok
nor response.error arrives in time. Use the existing `#connectOnce` and
`#connectWithBackoff` flow to clear the timer on resolve/reject, close the socket,
and surface a clear timeout error so reconnect/backoff can proceed normally.
---
Nitpick comments:
In `@packages/core/src/client.test.ts`:
- Around line 217-224: Add a synchronous status accessor on CodeoidClient so
callers do not need to subscribe just to read the current state. Expose a get
status() (or similarly named getter) that returns the client’s current
ClientStatus, and have it read from the same internal state used by onStatus;
update client.test.ts expectations or usage to prefer the getter where the
status is only being read synchronously.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: de83c719-d908-456f-8def-ebc589ae6504
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lock,!**/*.lock,!bun.lockweb/bun.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (38)
.github/workflows/release.ymlpackage.jsonpackages/core/README.mdpackages/core/package.jsonpackages/core/src/approvals.test.tspackages/core/src/approvals.tspackages/core/src/client.test.tspackages/core/src/client.tspackages/core/src/format.test.tspackages/core/src/format.tspackages/core/src/identity.test.tspackages/core/src/identity.tspackages/core/src/index.tspackages/core/src/messages.test.tspackages/core/src/messages.tspackages/core/src/resume.test.tspackages/core/src/resume.tspackages/core/src/sanitize-url.test.tspackages/core/src/sanitize-url.tspackages/core/src/slash.test.tspackages/core/src/slash.tspackages/core/src/usage-days.test.tspackages/core/src/usage-days.tspackages/core/tsconfig.jsonweb/package.jsonweb/src/components/prompt/slash.tsweb/src/lib/approvals.tsweb/src/lib/format.tsweb/src/lib/identity.test.tsweb/src/lib/identity.tsweb/src/lib/sanitize-url.tsweb/src/lib/usage-days.tsweb/src/lib/ws.test.tsweb/src/lib/ws.tsweb/src/state/connection.tsweb/src/state/messages.tsweb/src/state/resume.test.tsweb/src/state/resume.ts
💤 Files with no reviewable changes (2)
- web/src/lib/ws.test.ts
- web/src/state/resume.test.ts
Address CodeRabbit review on #115: - Bound the initial auth handshake with requestTimeoutMs: a peer (or middlebox) that accepts the socket but never answers the auth frame previously hung connect() forever — the heartbeat only starts after `connected` and request timeouts don't cover the pre-auth frame. The deadline closes the socket and rejects into the normal backoff loop (double-loop safe: the post-close reconnect kick is gated on #connecting). This hang predates the extraction (web's ws.ts had it); fixed in the now-shared transport. All handshake exits clear the timer. - Add `get status()` so hosts/tests can read the connection state synchronously instead of subscribe-then-read. Tests: never-answered handshake fails with "auth handshake timed out" and lands in `failed` under bounded attempts; reconnect test simplified via the getter. Core suite 113 pass; root + web green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…request-id guard Second audit pass over @codeoid/core before it becomes the mobile foundation. Five findings, all fixed: - connect() during a background reconnect threw "connect already in progress" — real API bug for mobile hosts (an AppState foreground handler calling connect() mid-backoff got an exception). Every entry point (connect, reconnectNow, close- and heartbeat-triggered kicks) now shares one memoized loop promise: callers JOIN the in-flight attempt. Rejected/finished loops clear via finally, so recovery from `failed` starts fresh. - connect() after shutdown() surfaced a confusing "shutdown during reconnect" — now throws a clear terminal-instance error, and the contract (shut-down clients are not reusable) is documented. - request() with a duplicate id silently clobbered the earlier pending entry, hanging its caller until a misleading timeout — duplicates now reject immediately; the original request is unharmed. - Subscriber-fault isolation: a throwing onMessage/onStatus handler (or MessageStore listener) broke the fan-out for later subscribers from inside the WS event context. Each handler is now isolated with the error logged; transport dispatch survives consumer bugs. - sideEffects:false on @codeoid/core and @codeoid/protocol (tree-shaking for Vite/Metro); MessageStore.messagesFor returns a stable frozen empty slice instead of allocating per miss. Audited and left deliberately unchanged: sanitize-url (scheme allowlist + remote-image exfil blocking is careful, correct work), approvals (turn-bounded backward scan), slash (fully DI'd), resume, format/identity, MessageStore kernels — reviewed line-by-line, no defects found. Tests: 6 new regressions (concurrent-connect joins one socket; post-shutdown connect error; duplicate-id rejection with original unharmed; throwing handler doesn't block dispatch, both client and store; stable empty slice). Core 119, root 963, web 78 + build — green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The second half of the packages extraction (design doc §4) and the last prerequisite for mobile P1: everything a frontend needs except the pixels, shared by web and the upcoming mobile app so transcript semantics and transport reliability live in exactly one place.
@codeoid/core(packages/core, source-only, MIT, peer-dep on@codeoid/protocol)CodeoidClient— the transport moved fromweb/src/lib/ws.ts, host-parameterized:capabilities+clientNameare nowConnectOptions(web declaresparts/replay.chunked/replay.resume+"codeoid-web"at its construction site) — no web-isms baked into the transport.globalThisview, so the package compiles without the DOM lib and no-ops cleanly on native hosts (which callreconnectNow()from their own resume signal, e.g. RNAppState).Message semantics —
mergeDeltaInto+dedupeReplaykernels, plus a batteries-includedMessageStorefor hosts without a bespoke reactive layer. Itsingest()encodes the full broadcast routing table — live message / delta / snapshot replay / chunked replay (#84) / incremental resume (never resets!) + resume-cursor advancement — the part that's easy to get subtly wrong when reimplemented per client.ResumeCursors(now a class; web binds a singleton) and the display helpers: formatters, identity/provenance labels, approval scanning, slash parsing (already DI'd viaSlashContext), URI sanitizers, UTC day buckets. Tailwind class mappers stay web-local;ctxWindowColorClassmaps core's newctxWindowSeverity.Web
file:../packages/core(same mechanics as feat: web consumes @codeoid/protocol — retire the hand-maintained mirror #105; core's protocol peer-dep resolves against web's copy).Tests move where the code lives
bun:test: the ported lib suites, theCodeoidClientstate machine (rewritten with injected real timings; new coverage for capability declaration, request correlation incl.waitForResult, in-flight rejection on drop, shutdown semantics), and new MessageStore/kernel/ingest suites.Release wiring
release.ymlpublishes@codeoid/core(version-guarded) after@codeoid/protocol. Like protocol, it needs a one-time manual bootstrap publish + its own npm Trusted Publisher before/with the next release.Verification
Root: lint ✓ typecheck (root + both packages) ✓ 956 tests ✓ build ✓ · Web: tsc ✓ eslint ✓ 78 tests ✓ vite build ✓ (fresh
file:installs +--frozen-lockfilepath).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores