fix(core): remove Node-only code to restore browser compat [closes #281]#283
Merged
EmersonBraun merged 1 commit intomainfrom Apr 15, 2026
Merged
fix(core): remove Node-only code to restore browser compat [closes #281]#283EmersonBraun merged 1 commit intomainfrom
EmersonBraun merged 1 commit intomainfrom
Conversation
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
5 tasks
…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
c2716a5 to
9816a10
Compare
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
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
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.
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.tshad:at the top of the file. Browser bundlers externalize
node:*but the import still runs at module load time, crashing every browser consumer with:Result:
apps/example-reactnever 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:createFileMemory(core)fileChatMemory(@agentskit/memory)loadConfig+AgentsKitConfig+LoadConfigOptions(core)@agentskit/cliWhat stays in core (all universal):
createInMemoryMemory— pure JScreateLocalStorageMemory— already guardstypeof localStorageBreaking changes
Declared in
.changeset/core-browser-compat.md:Bumps:
@agentskit/coremajor (exports removed)@agentskit/memoryminor (fileChatMemoryadded)@agentskit/climinor (loadConfignow public)@agentskit/reactminor (dropped re-export)@agentskit/inkminor (dropped re-export)The removed re-exports in
@agentskit/reactand@agentskit/inkwere 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 matchesgrep "from 'node:" packages/core/dist/*.js→ zero matchespnpm --filter "./packages/*" build→ all greenapps/example-react: root now mounts with the full chat UI, no pageerrorspnpm test→ only the pre-existing ink@7 failure (fixed in foundation: manifesto, origin, governance templates, and Phase 0 planning docs #265), unrelated to this changeTest plan
Refs #281 #282 #265 #211