Skip to content

feat: expose bundle instructions to LLM + add nb__read_resource tool (#3)#25

Open
mgoldsborough wants to merge 1 commit intomainfrom
fix/issue-3-read-resource
Open

feat: expose bundle instructions to LLM + add nb__read_resource tool (#3)#25
mgoldsborough wants to merge 1 commit intomainfrom
fix/issue-3-read-resource

Conversation

@mgoldsborough
Copy link
Copy Markdown
Contributor

Summary

Two gaps caused the agent to repeatedly fail on bundles that publish their usage guidance via MCP:

  • `McpSource` connected to the server but discarded the `initialize.instructions` field — so per-bundle guidance never reached the system prompt.
  • No system tool let the LLM read an MCP resource on demand. `readResource()` existed but was internal-only. The agent had no path from the instructions hint to the actual skill content.

The Solar5 demo showed the concrete symptom: 17+ failed tool calls per conversation guessing parameter names and units, even though the bundle author had published the correct workflow at `skill://solar5estrella/usage`.

Changes

1. Surface bundle `instructions` in the apps list

  • `McpSource` now captures `client.getInstructions()` after connect, exposes via `getInstructions()`, clears in `stop()`.
  • `ToolRegistry` gets a `getSource(name)` accessor.
  • `Runtime.buildAppsList` looks up the source for each bundle instance and includes its instructions in the `PromptAppInfo`.
  • `compose.formatAppsSection` renders `…` with containment-tag escaping — any `` in the payload is HTML-encoded so a malicious bundle cannot close the tag early and inject a forged system section.

2. Add `nb__read_resource` system tool

  • Iterates the workspace registry's sources, calls `readResource()` on each that implements `ResourceReader`, returns the first resolved payload as text.
  • Output capped at 12,000 chars (matches the focused-app skill budget) with a truncation marker.
  • Source errors are aggregated into the final "not found" message so failures aren't swallowed.
  • Workspace-scoped via the existing `getRegistry()` accessor, so workspace isolation invariants hold.

Test plan

  • New `nb__read_resource` tests: resolves from the first matching source, returns error for unknown URIs, rejects empty URIs, truncates oversize content, aggregates per-source errors, skips non-ResourceReader sources.
  • New prompt-injection tests: `` renders correctly and cannot be escaped by a payload containing the closing tag.
  • `ToolRegistry.getSource` unit test.
  • `bun test test/unit/` — 1734 pass, 0 fail.
  • `bun run check` / `bun run lint` — clean.

README updated to list `nb__read_resource` in the system tools table.

Closes #3

)

When a bundle's FastMCP server sets `instructions` pointing at a
`skill://` resource (the idiomatic way to publish per-bundle usage
guidance), the agent never saw either piece:

  - McpSource connected to the server but discarded the
    `initialize.instructions` field.
  - No system tool let the LLM read an MCP resource on demand;
    readResource() existed only for internal use.

Result: the agent guessed parameter names, guessed units, looped
17+ failed tool calls per conversation on the Solar5 demo, and
had no path to discover the correct workflow that the bundle
author had published.

Fix (two parts, independent but ship together):

1. Surface bundle instructions
   - McpSource captures client.getInstructions() after connect,
     exposes via getInstructions().
   - Runtime.buildAppsList looks up the matching source by name in
     the workspace registry and includes `instructions` in the
     PromptAppInfo.
   - compose.formatAppsSection renders `<app-instructions>…</app-instructions>`
     with containment-tag escaping: any `</app-instructions>` in the
     payload is HTML-encoded so a malicious bundle author cannot
     close the tag early and inject a forged system section.

2. nb__read_resource system tool
   - Iterates the workspace registry's sources, calls readResource()
     on each that implements ResourceReader, returns the first
     resolved payload as text.
   - Output capped at 12_000 chars (same budget as the focused-app
     skill injection path) with a truncation marker.
   - Sources that throw are reported in the aggregated "not found"
     error, never swallowed silently.
   - Non-ResourceReader sources are skipped.

Workspace-scoped via getRegistry() (routes through the existing
request-scoped registry accessor), so isolation invariants are
preserved.
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.

LLM cannot read bundle skill resources on demand

1 participant