Skip to content

fix(core): remove Node-only code to restore browser compat [closes #281]#283

Merged
EmersonBraun merged 1 commit intomainfrom
foundation/core-browser-compat
Apr 15, 2026
Merged

fix(core): remove Node-only code to restore browser compat [closes #281]#283
EmersonBraun merged 1 commit intomainfrom
foundation/core-browser-compat

Conversation

@EmersonBraun
Copy link
Copy Markdown
Owner

Summary

P0 for Phase 0 launch. Restores browser compatibility of @agentskit/core, fulfilling Manifesto principle 1 ("core works in any environment — Node, Deno, edge, browser").

Closes #281. Unblocks the 5 browser-side E2E tests currently skipped in #282.

The problem

Since v0.x, packages/core/src/config.ts had:

import { readFile } from 'node:fs/promises'

at the top of the file. Browser bundlers externalize node:* but the import still runs at module load time, crashing every browser consumer with:

Module "fs/promises" has been externalized for browser compatibility. Cannot access "fs/promises.readFile" in client code.

Result: apps/example-react never mounted. <div id="root"> stayed empty. The 5 skipped tests in #282 were detecting this.

The fix

Relocate two Node-only helpers out of @agentskit/core:

Before After
createFileMemory (core) fileChatMemory (@agentskit/memory)
loadConfig + AgentsKitConfig + LoadConfigOptions (core) same exports, moved to @agentskit/cli

What stays in core (all universal):

  • createInMemoryMemory — pure JS
  • createLocalStorageMemory — already guards typeof localStorage

Breaking changes

Declared in .changeset/core-browser-compat.md:

- import { createFileMemory } from '@agentskit/core'
+ import { fileChatMemory } from '@agentskit/memory'

- import { loadConfig } from '@agentskit/core'
+ import { loadConfig } from '@agentskit/cli'

Bumps:

  • @agentskit/core major (exports removed)
  • @agentskit/memory minor (fileChatMemory added)
  • @agentskit/cli minor (loadConfig now public)
  • @agentskit/react minor (dropped re-export)
  • @agentskit/ink minor (dropped re-export)

The removed re-exports in @agentskit/react and @agentskit/ink were always Node-only — browser consumers got a runtime error from them anyway, so this is making the shape honest.

Verification

  • grep fs/promises packages/core/dist/*.js → zero matches
  • grep "from 'node:" packages/core/dist/*.js → zero matches
  • pnpm --filter "./packages/*" build → all green
  • ✅ Manual browser check on apps/example-react: root now mounts with the full chat UI, no pageerrors
  • pnpm test → only the pre-existing ink@7 failure (fixed in foundation: manifesto, origin, governance templates, and Phase 0 planning docs #265), unrelated to this change

Test plan

  • Core dist is Node-builtin-free
  • Example-react mounts in a real browser
  • Tests relocated to the right packages
  • Changeset describes the migration
  • Reviewer: CI confirms bundle size gate still passes (core should be slightly smaller now)
  • Follow-up PR: re-enable the 5 skipped E2E tests in test(e2e): Playwright coverage for all 4 example apps [closes #251] #282 and adjust placeholder selector

Refs #281 #282 #265 #211

EmersonBraun added a commit that referenced this pull request Apr 15, 2026
Removes the test.skip wrappers in example-react and example-multi-agent
specs that were blocked by #281 (core importing fs/promises). With the
fix from foundation/core-browser-compat (PR #283) merged in, the React
apps mount and the tests pass.

Selector update: switched from getByPlaceholder(/type|message/i) to
locator('[data-ak-input]'). The example apps use custom placeholders
('Ask about the weather in any city...', 'Give the planner a task to
delegate...'), so the regex never matched. The data-ak-input attribute
is the headless contract — stable across placeholder changes.

Result locally:
  Before: 4 passed, 5 skipped, 0 failed
  Now:    10 passed, 0 skipped, 0 failed

Coverage by suite:
  example-ink           1 test
  example-runtime       1 test
  example-react         5 tests (mount, typed input, send, tool call, server)
  example-multi-agent   3 tests (server, mount, send)

Builds on: PR #283 (core fix) merged into this branch.
Closes the unblock task tracked alongside #251 and #281.

Refs #251 #281 #283 #211
…oses #281]

Manifesto principle 1 requires @agentskit/core to work in any environment
(Node, Deno, edge, browser). The previous implementation imported
node:fs/promises at module load via config.ts, which crashed every
browser consumer at runtime with 'Module fs/promises has been
externalized for browser compatibility.'

This PR relocates the two Node-only helpers:

  createFileMemory  (packages/core/src/memory.ts)
    -> fileChatMemory  (packages/memory/src/file-chat.ts)

  loadConfig + AgentsKitConfig + LoadConfigOptions  (packages/core/src/config.ts)
    -> @agentskit/cli (packages/cli/src/config.ts)

What stays in @agentskit/core (all universal):
  createInMemoryMemory     works in every environment
  createLocalStorageMemory already guards on typeof localStorage

Consumer migrations:
- Re-exports of createFileMemory from @agentskit/react and @agentskit/ink
  are dropped (they were always Node-only; browser consumers got a
  broken export)
- apps/example-ink switched to fileChatMemory from @agentskit/memory
- @agentskit/cli internals now import loadConfig from its own ./config

Tests relocated:
- packages/core/tests/config.test.ts  -> packages/cli/tests/config.test.ts
- packages/core/tests/memory.test.ts createFileMemory block split out
  -> packages/memory/tests/file-chat.test.ts

Changeset declares:
  @agentskit/core          major (breaking — exports removed)
  @agentskit/memory        minor (new fileChatMemory export)
  @agentskit/cli           minor (new loadConfig public export)
  @agentskit/react         minor (dropped re-export)
  @agentskit/ink           minor (dropped re-export)

Verified:
- packages/core/dist/*.js contains zero fs/path imports
- apps/example-react mounts in a real browser (was empty root before)
- pnpm build succeeds for every package
- Ink test failure is pre-existing (ink@7 incompat fixed in #265),
  not caused by this change

Unblocks the 5 browser-side E2E tests currently skipped in #282.

Closes #281
@EmersonBraun EmersonBraun force-pushed the foundation/core-browser-compat branch from c2716a5 to 9816a10 Compare April 15, 2026 12:23
@EmersonBraun EmersonBraun merged commit 451fe5e into main Apr 15, 2026
0 of 2 checks passed
@EmersonBraun EmersonBraun deleted the foundation/core-browser-compat branch April 15, 2026 12:23
EmersonBraun added a commit that referenced this pull request Apr 15, 2026
Removes the test.skip wrappers in example-react and example-multi-agent
specs that were blocked by #281 (core importing fs/promises). With the
fix from foundation/core-browser-compat (PR #283) merged in, the React
apps mount and the tests pass.

Selector update: switched from getByPlaceholder(/type|message/i) to
locator('[data-ak-input]'). The example apps use custom placeholders
('Ask about the weather in any city...', 'Give the planner a task to
delegate...'), so the regex never matched. The data-ak-input attribute
is the headless contract — stable across placeholder changes.

Result locally:
  Before: 4 passed, 5 skipped, 0 failed
  Now:    10 passed, 0 skipped, 0 failed

Coverage by suite:
  example-ink           1 test
  example-runtime       1 test
  example-react         5 tests (mount, typed input, send, tool call, server)
  example-multi-agent   3 tests (server, mount, send)

Builds on: PR #283 (core fix) merged into this branch.
Closes the unblock task tracked alongside #251 and #281.

Refs #251 #281 #283 #211
EmersonBraun added a commit that referenced this pull request Apr 15, 2026
Removes the test.skip wrappers in example-react and example-multi-agent
specs that were blocked by #281 (core importing fs/promises). With the
fix from foundation/core-browser-compat (PR #283) merged in, the React
apps mount and the tests pass.

Selector update: switched from getByPlaceholder(/type|message/i) to
locator('[data-ak-input]'). The example apps use custom placeholders
('Ask about the weather in any city...', 'Give the planner a task to
delegate...'), so the regex never matched. The data-ak-input attribute
is the headless contract — stable across placeholder changes.

Result locally:
  Before: 4 passed, 5 skipped, 0 failed
  Now:    10 passed, 0 skipped, 0 failed

Coverage by suite:
  example-ink           1 test
  example-runtime       1 test
  example-react         5 tests (mount, typed input, send, tool call, server)
  example-multi-agent   3 tests (server, mount, send)

Builds on: PR #283 (core fix) merged into this branch.
Closes the unblock task tracked alongside #251 and #281.

Refs #251 #281 #283 #211
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Phase 0] @agentskit/core contains fs/promises import that breaks browser consumers

1 participant