Repository navigation
feat(reader): load trusted component packs in isles live reader mode - #170
Merged
Merged
Conversation
Since #168, `isles live` always serves the client-rendered reader SPA, which bundled only the core agent-* components and never read the project's isles.config.json — so trusted local component packs (e.g. Quirk's `<quirk-free-text>`) rendered into the DOM but sat inert because their custom element was never registered. Bring pack support to the reader by exposing resolved pack assets over HTTP and injecting them at boot: - src/live.mjs: resolve packs once at startup (project config only, includeUserPacks: false — identical scope to the server-rendered shell path) and cache the manifest. Two new routes: - GET /__agent-isles/pack-manifest -> { assets: [{type, url}] } - GET /__agent-isles/pack-asset?pack=<i>&path=<rel> serves one asset, matching `path` exactly against a manifest-declared asset (no path arithmetic on user input -> traversal impossible by construction) and reading via readFileNoFollow (O_NOFOLLOW). Any resolution/load failure degrades to an empty manifest with one stderr note, so a broken pack never bricks the reader. - src/reader/reader-entry.js: loadPacks() fetches the manifest and, per asset, injects a stylesheet or dynamic-imports the module via Promise.allSettled, awaited before the first render so a pack island upgrades on initial paint and one failing asset degrades only that pack. Zero-pack projects see no behavior change beyond one small manifest fetch. Tests: unit coverage for manifest/asset serving, content types, empty and broken configs, the four asset-route guards (undeclared path, out-of-range/invalid pack, traversal, symlink), and multi-pack index alignment; a sanitizer regression test pinning that unknown pack elements reach the rendered DOM with attributes intact; and a Playwright spec driving the full companion loop (element upgrades, pack CSS applies, click -> proceed record -> satisfies a parked agent long-poll). Implements docs/plans/reader-pack-loading.md.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores trusted local component pack loading in isles live reader SPA mode by having the live server expose a resolved pack manifest/assets over HTTP and having the reader inject those assets before the first render, bringing behavior back in line with the pre-#168 server-rendered path.
Changes:
- Add server routes to expose a startup-resolved pack manifest and serve only manifest-declared pack assets (
/__agent-isles/pack-manifest,/__agent-isles/pack-asset). - Add reader boot logic to fetch the pack manifest and inject CSS + dynamically import JS modules before initial document render.
- Add unit/regression/browser tests to cover manifest/asset behavior, security guards (traversal, undeclared assets, symlinks), and end-to-end pack upgrade/CSS/signal loop.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/live.mjs |
Resolves packs at server startup, serves pack manifest + pack assets over HTTP for reader injection. |
src/reader/reader-entry.js |
Fetches pack manifest and injects styles/modules before first render in reader mode. |
tests/live.test.mjs |
Adds unit coverage for manifest/asset serving, guards, degradation, and multi-pack index alignment. |
tests/reader-render-browser.test.mjs |
Adds regression test ensuring unknown pack custom elements survive client render with attributes intact. |
tests/browser/reader-packs.spec.mjs |
Adds Playwright E2E verifying pack element upgrade, CSS application, and signal → agent long-poll loop. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ension The manifest permits any extension for a `module` asset (e.g. `.cjs`, extensionless), but a module must be served with a JavaScript MIME type or the reader's import() is rejected. Serve by declared asset type instead of the file suffix; add a regression test pinning a non-.js module. Addresses Copilot review feedback on #170.
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.
What & why
Since #168,
isles livealways serves the client-rendered reader SPA, which bundled only the coreagent-*components and never read the project'sisles.config.json. As a result, trusted local component packs no longer loaded: a live doc containing<quirk-free-text …>rendered the element into the DOM, butcustomElements.get('quirk-free-text')was undefined and the island sat inert. Under the pre-#168 agent-screen shell this worked because the server-side render path resolved packs per render.This PR brings that pack support to the reader: the server exposes resolved pack assets over HTTP, and the reader injects them at boot. Resolution scope is identical to the shell path — project config only,
includeUserPacks: false, no CLI packs.Implements
docs/plans/reader-pack-loading.md.Changes
Server (
src/live.mjs)resolvePackInputs({ projectDir, includeUserPacks: false })→buildPackAssetRecords()) and cache the manifest JSON.GET /__agent-isles/pack-manifest→{ assets: [{ type, url }] }.GET /__agent-isles/pack-asset?pack=<i>&path=<rel>→ serves one asset.packindexes the resolved list;pathmust exactly match a manifest-declared asset — the route never does path arithmetic on user input, so traversal is impossible by construction. Reads viareadFileNoFollow(O_NOFOLLOW, refuses symlink swaps)..js/.mjs → text/javascript,.css → text/css.PackResolutionErrororPackLoadError) degrades to{"assets": []}with a single stderr note — a broken pack never bricks the reader.Reader (
src/reader/reader-entry.js)loadPacks()fetches the manifest and, per asset, injects a<link>stylesheet or dynamic-import()s the module viaPromise.allSettled, awaited before the first render so a pack island upgrades on initial paint and one failing asset degrades only that pack.Trust model
Unchanged from the shell: packs are trusted local code declared by a config file in the directory the user launched
isles liveon — the same trust boundary as the docs and the signal endpoint. The manifest/asset routes expose only files those local packs declare; no remote fetches. Access posture matches/treeand/raw(plain GET, no token, loopback-bound server).Zero-pack projects see no behavior change beyond one small manifest fetch.
Tests
tests/live.test.mjs): manifest + asset serving with correct content types; empty manifest for zero-pack projects; broken-config degradation (server stays up); all four asset-route guards (undeclared path, out-of-range/invalidpack, traversal, symlinked asset viaO_NOFOLLOW); and multi-pack index alignment (pack=0vspack=1each serve their own declared asset).tests/reader-render-browser.test.mjs): pins that an unknown pack custom element reaches the rendered DOM with attributes intact.tests/browser/reader-packs.spec.mjs): full companion loop — pack element upgrades, pack CSS applies (computed-style assertion), click produces aproceedrecord instate/eventsand satisfies a parkedGET /__agent-isles/agent/eventslong-poll.Also ran a 4-lens adversarial review (security / correctness / plan-conformance / tests); the only confirmed finding was the multi-pack coverage gap, now closed by the alignment test above.
Reviewer notes
dist/isles-reader.jsis a build artifact —npm install && npm run buildafter pulling.npm testexits non-zero because of 2 pre-existing failures intests/cli-packs.test.mjs(user-config packs): those tests setXDG_CONFIG_HOME, butgetUserConfigDir()uses~/Library/Application Supporton darwin. Verified identical with this branch's changes stashed — not caused by this PR (unrelated to reader/pack loading). Happy to fix separately.