From c7ab56ce644e1f3d5dce0a2ffbb9a48017e7bebe Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Thu, 3 Sep 2026 05:13:33 +0000 Subject: [PATCH] =?UTF-8?q?fix(docs,test):=20closed-issue=20audit=20G4=20?= =?UTF-8?q?=E2=80=94=20correct=20stale=20claims=20and=20pin=20the=20handsh?= =?UTF-8?q?ake=20queue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - packages/agent-bundle/README.md: the Claude adapter no longer emits cwd for plugin-root stdio servers (#347 removed it because Claude's placeholder table excludes cwd); the README still claimed it did (#47). - docs/preview-packages.md: the --peerDeps rewrite now applies to the optional @agent-bundle/runtime peer declared by agent-bundle, not an agent-bundle peer inside the (renamed) runtime package (#45). - docs/framework-mode.md: restore the rendered-skills (SKILL.tsx) power tier and the skills precedence rule (AB4734/AB4735/AB4736, AB3003-AB3005) that #150's rewrite dropped from the RFC #63 newcomer doc. - runtime-client-surface-proxy.test.ts: unit-pin the #23 fix — host requests relayed during the App handshake are queued (bounded at 32) and flushed on ui/notifications/initialized instead of dropped; only the mcp-app-real e2e covered it before. --- .changeset/closed-audit-g4-docs.md | 11 +++ docs/framework-mode.md | 39 +++++++++ docs/preview-packages.md | 14 ++-- packages/agent-bundle/README.md | 8 +- .../runtime-client-surface-proxy.test.ts | 79 ++++++++++++++++++- 5 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 .changeset/closed-audit-g4-docs.md diff --git a/.changeset/closed-audit-g4-docs.md b/.changeset/closed-audit-g4-docs.md new file mode 100644 index 000000000..0f9b14d00 --- /dev/null +++ b/.changeset/closed-audit-g4-docs.md @@ -0,0 +1,11 @@ +--- +"agent-bundle": patch +--- + +Docs-only corrections from the closed-issue audit (#23, #45, #47, #63). The +published README no longer claims the Claude adapter emits +`cwd: "${CLAUDE_PLUGIN_ROOT}"` for source-built stdio servers — that emission +was deliberately removed because Claude Code's placeholder table excludes +`cwd`; the absolute entry path plus the `AGENT_BUNDLE_PLUGIN_ROOT` env anchor +carry the working-directory guarantee. No runtime code or export surface +changes. diff --git a/docs/framework-mode.md b/docs/framework-mode.md index 2910fcf94..22feaee21 100644 --- a/docs/framework-mode.md +++ b/docs/framework-mode.md @@ -69,6 +69,45 @@ results and never renders JSX. Routed `src/cli/**` commands and Agent renderer (TTY progress, piped Markdown, `--json`, `--ndjson`); `.ts` is plain. +## Skills: convention, override, and rendered documents + +`src/skills//SKILL.md` ships with no declaration. Config wins, +conventions fill: declaring `skills:` replaces the directory convention +entirely, and validation reports `AB4734` for any conventional skill directory +the explicit list leaves uncovered. Skills at the removed top-level +`skills//` location are an `AB4736` error unless explicit `skills` +config claims them. + +A skill whose document is generated (power tier, never required) puts +`SKILL.tsx` (or `SKILL.ts`) in the skill directory instead of `SKILL.md`. The +module default-exports a component (sync or async) and exports a `frontmatter` +record; the build renders the tree to Markdown and emits the same +`skills//SKILL.md` artifact every host consumes. + +```tsx +// src/skills/deploy-checklist/SKILL.tsx +export const frontmatter = { + description: 'Deployment checklist.', + name: 'deploy-checklist', +}; + +export default () => ( + <> +

Deploy checklist

+

Verify each step in order.

+ +); +``` + +The renderer supports a documented element subset (`h1`–`h6`, `p`, +`ul`/`ol`/`li`, `strong`/`b`, `em`/`i`, `code`, `pre`, `blockquote`, `a`, +`hr`, `br`, fragments, strings and numbers) and rejects anything outside it by +name (`AB3005`), never a silent approximation; a module that fails to load or +lacks the two exports reports `AB3003`/`AB3004`. Components may import +project code, so the document can be computed from the same sources the +plugin ships. A hand-authored `SKILL.md` in the same directory always wins +(`AB4735`). + ## Config reference ### `output` diff --git a/docs/preview-packages.md b/docs/preview-packages.md index da52920cc..ee31bdbf9 100644 --- a/docs/preview-packages.md +++ b/docs/preview-packages.md @@ -49,14 +49,18 @@ npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@5685 pnpm and yarn accept the same URLs (`pnpm add `, `yarn add agent-bundle@`). Previews carry the version string `0.0.0-preview-`, and the publish -rewrites the `agent-bundle` peer range inside the `@agent-bundle/runtime` -preview tarball to that exact preview version (`--peerDeps`). Installing both +(`--peerDeps`) rewrites every peer range that points at a sibling workspace +package to that exact preview version inside the preview tarballs. Today that +is the optional `@agent-bundle/runtime` peer declared by `agent-bundle` +(`@agent-bundle/runtime` itself no longer declares an `agent-bundle` peer; +its peers are `react`, `react-dom`, and `@rspack/core`). Installing both packages from the same sha therefore works with stock npm — no `--legacy-peer-deps` needed. Mixing two different shas fails with `ERESOLVE` by design; use one sha (or one PR number) for both URLs. Previews published -before the peer rewrite landed (PR #46) still carry the original -`agent-bundle@^0.1.0` range, so pair-installing those older shas with npm -still requires `--legacy-peer-deps`. +before the peer rewrite landed (PR #46, fixing #45) still carry the original +`agent-bundle@^0.1.0` range on the then-named `@agent-bundle/rsc-runtime` +package, so pair-installing those older shas with npm still requires +`--legacy-peer-deps`. ## Where previews come from diff --git a/packages/agent-bundle/README.md b/packages/agent-bundle/README.md index 60b2dda62..681fd7801 100644 --- a/packages/agent-bundle/README.md +++ b/packages/agent-bundle/README.md @@ -32,9 +32,11 @@ against the entry's plugin-root `cwd`. Codex has no path-token interpolation, so server without a plugin-root working directory omits the anchor; source-built (`entry:`) servers always have one on every target. Server runtime code should resolve persistent state and bundled assets against this anchor rather than the process working directory: Claude Code currently -launches stdio servers from the host's own working directory and ignores the emitted `cwd` field -(the Claude adapter still emits `cwd: "${CLAUDE_PLUGIN_ROOT}"` as documented, schema-valid -future-proofing). A server's own `env` entries win over the injected value, so declaring +launches stdio servers from the host's own working directory and ignores stdio `cwd` at runtime, +and its placeholder-substitution table excludes `cwd`, so the Claude adapter emits no `cwd` for a +plugin-root working directory (the absolute `${CLAUDE_PLUGIN_ROOT}/mcp/...` entry path plus this +env anchor carry the guarantee) and rejects token-bearing `cwd` values outright. A server's own +`env` entries win over the injected value, so declaring `env: { AGENT_BUNDLE_PLUGIN_ROOT: ... }` replaces the anchor. The `pluginRootEnvAnchor` export names the variable for consumer code. diff --git a/packages/agent-bundle/tests/runtime-client-surface-proxy.test.ts b/packages/agent-bundle/tests/runtime-client-surface-proxy.test.ts index 22d9b803f..ccbc99194 100644 --- a/packages/agent-bundle/tests/runtime-client-surface-proxy.test.ts +++ b/packages/agent-bundle/tests/runtime-client-surface-proxy.test.ts @@ -125,11 +125,14 @@ const runtimeProxyShellHarness = async ( timers.clear(); for (const callback of pending) callback(); }; - const appPosts: unknown[] = []; + const appPosts: Array> = []; const parentPosts: Array> = []; const child = Object.freeze({ postMessage: (message: unknown, targetOrigin: string) => { appPosts.push(Object.freeze({ message, targetOrigin })); }, }); + const parentWindow = Object.freeze({ + postMessage: (message: unknown, targetOrigin: string) => { parentPosts.push(Object.freeze({ message, targetOrigin })); }, + }); const entries: string[] = []; const app = Object.create(null) as { readonly contentWindow: typeof child; srcdoc: string }; Object.defineProperty(app, 'contentWindow', { enumerable: true, value: child }); @@ -181,7 +184,7 @@ const runtimeProxyShellHarness = async ( ) => void; execute( Object.freeze({ getElementById: () => app }), - Object.freeze({ postMessage: (message, targetOrigin) => { parentPosts.push(Object.freeze({ message, targetOrigin })); } }), + parentWindow, Object.freeze({ origin: binding.origin }), FakeWebSocket, TextEncoder, @@ -195,6 +198,9 @@ const runtimeProxyShellHarness = async ( return Object.freeze({ appPosts, emitChild: (data: unknown) => { emit('message', Object.freeze({ data, origin: 'null', ports: Object.freeze([]), source: child })); }, + emitParent: (data: unknown, origin = foregroundOrigin) => { + emit('message', Object.freeze({ data, origin, ports: Object.freeze([]), source: parentWindow })); + }, entries, pagehide: () => { emit('pagehide'); }, parentPosts, @@ -265,6 +271,75 @@ it('keeps malformed opaque-child initialize messages from advancing the trusted } }); +it('queues host requests relayed during the App handshake and flushes them once the App reports initialized', async () => { + // Regression for #23: a ui/resource-teardown relayed between the initialize + // response and ui/notifications/initialized used to be dropped, so the host + // burned its bounded teardown grace waiting for an acknowledgement that could + // never arrive. The shell must queue (bounded) instead of dropping. + const upstream = createServer((request, response) => { + if (serveBootstrapEntry(request, response)) return; + response.writeHead(404).end(); + }); + const origin = await listen(upstream); + const binding = await RuntimeClientSurfaceProxy.open({ + entryPath: '/app/index.html', + httpOrigin: origin, + httpPathPrefixes: ['/app/'], + surfaceId: 'app.weather', + subscribeReload: noopSubscribeReload, + }, () => undefined); + try { + const shell = await runtimeProxyShellHarness(binding); + const initialize = { + id: 'init', jsonrpc: '2.0', method: 'ui/initialize', + params: { appCapabilities: {}, appInfo: { name: 'app', version: '1' }, protocolVersion: '2026-01-26' }, + }; + const initializeResponse = { id: 'init', jsonrpc: '2.0', result: { hostContext: {} } }; + const teardown = { id: 'teardown', jsonrpc: '2.0', method: 'ui/resource-teardown', params: {} }; + const initialized = { jsonrpc: '2.0', method: 'ui/notifications/initialized' }; + + // Host traffic before the App has even asked to initialize is queued too. + shell.emitParent({ id: 'early', jsonrpc: '2.0', method: 'ui/notifications/host-context-changed', params: {} }); + shell.emitChild(initialize); + expect(shell.parentPosts.map((entry) => entry.message)).toEqual([initialize]); + shell.emitParent(initializeResponse); + expect(shell.appPosts.map((entry) => entry.message)).toEqual([initializeResponse]); + + // The race from #23: a teardown request lands inside the handshake window. + shell.emitParent(teardown); + // Foreign-origin and malformed host traffic is still rejected, never queued. + shell.emitParent({ id: 'foreign', jsonrpc: '2.0', method: 'ui/resource-teardown', params: {} }, 'http://evil.example'); + shell.emitParent({ id: 'not-rpc', method: 'ui/resource-teardown' }); + // The queue is bounded: the 32-entry window already holds `early` and + // `teardown`, so exactly 30 more fit and the rest are dropped. + const filler = Array.from({ length: 40 }, (_, index) => ({ id: `fill-${String(index)}`, jsonrpc: '2.0', method: 'ui/notifications/host-context-changed', params: { index } })); + for (const message of filler) shell.emitParent(message); + expect(shell.appPosts).toHaveLength(1); + + shell.emitChild(initialized); + expect(shell.parentPosts.map((entry) => entry.message)).toEqual([initialize, initialized]); + const flushed = shell.appPosts.slice(1).map((entry) => entry.message); + expect(flushed).toHaveLength(32); + expect(flushed[0]).toEqual({ id: 'early', jsonrpc: '2.0', method: 'ui/notifications/host-context-changed', params: {} }); + expect(flushed[1]).toEqual(teardown); + expect(flushed.slice(2)).toEqual(filler.slice(0, 30)); + expect(shell.appPosts.every((entry) => entry.targetOrigin === '*')).toBe(true); + + // Once initialized, host traffic relays immediately and nothing is replayed twice. + const late = { id: 'late', jsonrpc: '2.0', method: 'ui/resource-teardown', params: {} }; + shell.emitParent(late); + expect(shell.appPosts).toHaveLength(34); + expect(shell.appPosts.at(-1)?.message).toEqual(late); + const acknowledgement = { id: 'teardown', jsonrpc: '2.0', result: {} }; + shell.emitChild(acknowledgement); + expect(shell.parentPosts.at(-1)?.message).toEqual(acknowledgement); + } finally { + await binding.close(); + upstream.closeAllConnections(); + await close(upstream); + } +}); + it('refreshes only for owned reload frames with advancing generations while reconnecting the channel', async () => { const upstream = createServer((request, response) => { if (serveBootstrapEntry(request, response)) return;