Skip to content

feat: ship MCP prompts and resources, not just tools (#371) - #392

Merged
StuMason merged 2 commits into
mainfrom
feat/371-prompts-and-resources
Sep 10, 2026
Merged

StuMason merged 2 commits into
mainfrom
feat/371-prompts-and-resources

Conversation

@StuMason

@StuMason StuMason commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Closes #371.

The server has exposed one of MCP's three primitives. This adds the other two: three prompts a client shows as slash commands, and two resources a client can attach. No new Coolify endpoint, no new dependency — registerPrompt and ResourceTemplate are already in SDK v2.

Prompts

Prompt Argument Workflow
troubleshoot_application query (name, UUID or domain) diagnose_app, then logs, then env sanity, ending in a cause and the smallest fix
explain_failed_deploy deployment_uuid build log through deployment get, which stage broke, quoting the evidence
estate_health none find_issues plus the overview, worst first

Fleet mode adds the same optional instance the tools take.

They make no API call

prompts/get is synchronous, has no elicitation channel and no error affordance, so a prompt that pre-fetched would turn a slash command into a multi-second stall that can fail with something the human cannot act on.

The bigger reason is security. Embedding container or build output in the returned message would place attacker-influenceable text in a user-role message, outside the asUntrustedLogs boundary the tool layer puts around exactly that text (evals/FINDINGS.md #4). Naming the tool and letting the model call it keeps the log on the one path that frames it as data. The builders are pure string functions and a test spies on the client to prove it.

A prompt whose tools are missing is not listed

This was the design's one real wall. Read-only mode (#303) registers no mutating tool, and consolidation puts two pure reads under destructive tools: env_vars list and deployment get. So "which tools exist" is a per-mode fact, and a prompt naming env_vars names a tool a read-only server does not have.

Resolved structurally rather than with a comment nobody maintains. defineTool records what it registered; definePrompt takes requires and skips the whole prompt when a required tool is absent; the builders take ctx.has() and drop an individual step.

Concretely, on a read-only server:

  • explain_failed_deploy is not listed at all — the build log is reachable only through deployment, so the workflow cannot run, and a dead-end slash command is worse than none.
  • troubleshoot_application is listed with its env step dropped.
  • estate_health is unchanged.

Resources

coolify://overview and coolify://application/{uuid}, instance-scoped in fleet mode (coolify://staging/overview), because reading production's overview while believing it is staging's is the mistake a second instance invents. Every application is listed as a concrete entry so a human can pick one instead of knowing a UUID; an unreachable instance drops out of the listing rather than blanking it.

Masking. Reads go through the same client as every tool call, so deepSanitize applies. The contract suite asserts the resource payload equals get_application's byte for byte rather than assuming it, with a planted fixture secret so the paired assertion can actually fail. No URI takes a reveal: get_application has one because a caller justifies it in the moment, whereas a resource URI is a durable handle a client may cache, re-read or paste.

get_infrastructure_overview moved into a shared private method, so the tool and the resource cannot drift into two answers.

Contract

tools/list is untouched — these are separate lists over separate methods, which is what #371 asked for. The evals harness now lists all three surfaces from one boot, with _prompts.json / _resources.json snapshots and fleet variants, their own budgets, and a check that no prompt names a tool the server did not register.

Verification

  • npm test — 844 passing, 15 suites. prompts.ts at 100% statements.
  • npm run snapshots — 23 contract tests passing, single and fleet.
  • npm run lint, npx tsc --noEmit, evals typecheck + lint clean. No new warnings.
  • site build and tests pass; the new doc lands in /llms.txt and /llms-full.txt.

Notes

  • Client support is uneven. Prompts surface as slash commands in Claude Code and Claude Desktop; resources appear as attachments in Desktop and by mention in Claude Code. A client supporting neither loses nothing.
  • Instance-scoped resource URIs are shipped here rather than deferred, since the alternative was a fleet reading the default instance under an unscoped URI.
  • Like the other open 3.2 branches this adds a line at the same ### Added anchor in CHANGELOG.md, so expect a conflict there on the second and later merges.

https://claude.ai/code/session_01THRG6UFPUdMSARgJLcGf9a

@StuMason StuMason added this to the 3.2 milestone Sep 9, 2026
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review: prompts and resources (#392)

Read the whole diff. This is a genuinely well-argued PR — the "prompts never fetch" rule with the FINDINGS #4 reasoning, requires/ctx.has() making the read-only invariant structural rather than a comment, and infrastructureOverview() extracted so the tool and the resource cannot drift are all the right calls. The planted FIXTURE_WEBHOOK_SECRET, so the byte-equality assertion can actually fail, is the detail that shows the test suite was designed rather than assembled.

Findings below, most-significant first.


1. Resource list entries inherit the template's title, so every application renders as "Application detail"

src/lib/mcp-server.ts:1151-1175 — the list callback overrides name and description per entry but not title, and the registration metadata's title merges into every listed resource. Your own snapshots show it:

{ "name": "api-gateway",   "title": "Application detail", "uri": "coolify://application/app-api" },
{ "name": "log-viewer",    "title": "Application detail", "uri": "coolify://application/app-logviewer" },
{ "name": "shop-frontend", "title": "Application detail", "uri": "coolify://application/app-shop" }

(evals/src/contract/__toolsnaps__/_resources.json, and the same three-times-over twice in _resources.fleet.json)

Per the MCP spec title is the human display name and takes precedence over name in clients that implement it — so an estate of 26 applications renders as 26 rows all reading "Application detail". That defeats the stated reason for shipping the list callback at all ("so a human can pick one instead of knowing a UUID"). Same for the fleet overview entries: prod overview and staging overview both carry title: "Infrastructure overview".

Fix is one line per entry — a per-entry title of the application name (plus instance in fleet mode), and "<instance> overview" in the overview list. Worth adding an assertion that no two listed resources share a title, since the snapshot happily encoded the collision.

2. Fleet mode: an empty {instance} segment silently reads the default instance

src/lib/mcp-server.ts:1093-1096. first() maps a missing or empty variable to '', and InstanceRegistry.get() treats '' exactly like undefined (src/lib/instances.ts:76 — if (name === undefined || name === '') return this.default). So any URI that matches the template without capturing a non-empty instance resolves to the default instance and returns its data under a URI that names no instance.

That is precisely the invariant evals/src/contract/fleet.test.ts asserts two tests later ("an unknown instance in a URI is an error, not the default instance"). Whether coolify:///overview actually matches depends on the SDK's UriTemplate regex, so this may be unreachable today — but it is unreachable by accident, and it is an SDK-internal detail away from being a silent cross-instance read on the one surface this PR argues hardest about. Cheap to close:

const onInstance = <T>(name: string | undefined, body: () => Promise<T>): Promise<T> => {
  if (fleet && !name) throw new Error('Resource URI is missing its instance segment');
  return this.instanceContext.run(this.registry.get(name), body);
};

Same shape for {uuid}: first(variables.uuid) returning '' sends getApplication('') at GET /applications/, which Laravel resolves to the index route. A read of coolify://application/ would return the whole application list under a URI claiming one application. Masked, so not a leak — but the wrong shape silently, where a thrown "missing uuid" is free.

3. src/lib/prompts.ts:6 references a file that does not exist

read as instructions where instructions.ts reads as orientation

There is no src/lib/instructions.ts on this branch, and no instructions field passed to super() in mcp-server.ts:776. If that file is landing on a sibling 3.2 branch the comment is just early; otherwise the contrast it draws is with something imaginary, which is a shame in a file whose comments are otherwise doing real work.

4. registeredPrompts is written and never read

mcp-server.ts:637 and :981. Nothing consumes the set — PROMPT_NAMES's doc comment says it exists "so a test can assert the registered prompts and this list stay 1:1", but the test that does that (prompts.test.ts:133) goes through listPrompts() instead. TS will not flag it because .add() counts as a read of the property. Either delete it, or make it earn its keep: assert in the constructor that every registered prompt is in PROMPT_NAMES and that a full (non-readonly) server registers all of them. That turns "somebody added a prompt and forgot the roster" into a boot failure rather than a snapshot diff nobody looks at.

5. The evals "names only real tools" check is fail-open

evals/src/contract/toolsnaps.test.ts:597-623. The TOOL_LIKE allowlist means a prompt naming a tool that is neither registered nor in the list passes silently — exactly the case a future prompt author hits. The comment argues this is deliberate ("a new tool named in a prompt should have to be added here, which is the moment to ask..."), but nothing makes them add it; forgetting the list is indistinguishable from passing.

Inverting it fails closed for the same effort: flag every backticked [a-z_]+ token that is not a registered tool, with an explicit NOT_A_TOOL allowlist for the argument names and action values the prose backticks (lines, page, instance, list, get, application). Then a new tool name in prompt text is caught by default, and a new argument name is the thing that needs a list entry — the safer direction to be wrong in.

Smaller second point: this check only ever runs against the full server, so the read-only path — the case the entire requires / has() design exists for — is never exercised at the contract layer. src/__tests__/prompts.test.ts:137-164 does cover it, so this is about where the guard lives, not whether it is covered at all.

6. Coverage gaps

  • Fleet application-detail routing is untested. fleet.test.ts reads coolify://staging/overview and asserts the unknown-instance error, but nothing ever reads coolify://{instance}/application/{uuid}. mcp-server.ts:1189-1192 is the line where a routing mistake serves prod's configuration under a staging URI, and it is the one resource path with no read test. A fleet test asserting that staging's client was the spy that fired would close it.
  • estateHealthPrompt's plural branch (prompts.ts:129) never runs. Every fixture has exactly two instances, so otherInstances.length === 1 always. Statement coverage stays at 100% because it is a ternary; the "The others configured here are ..." sentence has never been rendered. A three-instance case in the snapshot would cost two lines.
  • No test for a failing resource read. getApplication rejecting propagates as a JSON-RPC error rather than the wrap()-style text envelope the tools return. That is a defensible choice for resources, but it is an undocumented, unasserted behavioural difference between two surfaces reading the same data — worth one test pinning it, since it is what a client actually has to handle.

7. Performance: resources/list fans out per instance, uncached

mcp-server.ts:1151-1175 makes one listApplications call per configured instance on every resources/list. Clients call that on connect and again on every list_changed. Two consequences worth at least a comment:

  • On a large estate this is the summary payload for every application, built and discarded, on each listing.
  • When an instance is unreachable the allSettled degradation is right, but each listing still pays that instance's full connect/read timeout before the other instances' results can be returned — a fleet with staging down makes every resources/list slow, not just incomplete. A short TTL cache, or a listing-specific deadline, would make the degradation cheap as well as correct.

8. Minor

  • No complete callbacks on the ResourceTemplate variables or the prompt instance argument. {instance} in particular is a closed set the server already knows (registry.names), and completion is the affordance that makes a template usable in Desktop. Natural follow-up rather than a blocker.
  • troubleshootApplicationPrompt interpolates query into (query: "...") unescaped — a name containing a double quote produces mis-quoted text. Cosmetic, but JSON.stringify(query) is the same character count.
  • Prompt arguments land verbatim in a user-role message. The threat model reasonably assumes a human typed them, and I agree — but it is worth one line in prompts.ts saying so, given that the file's opening argument is precisely about what may and may not occupy a user-role message. It documents the assumption rather than leaving it inferred.

Nothing here is a merge blocker except arguably #1, which is a visible-in-the-product bug the snapshots already captured. #2 is cheap insurance on the claim the PR makes most loudly.

@StuMason

StuMason commented Sep 9, 2026

Copy link
Copy Markdown
Owner Author

Review round applied in 83a69a8. Every finding actioned.

Two were real bugs my own snapshots had encoded:

  1. Listed resources inherited the template title, so every application rendered as the identical row "Application detail". Each entry now carries its own title, with a test asserting no two listed resources share one.
  2. An empty {instance} capture fell through to the default instance, since registry.get('') returns the default exactly as get(undefined) does. Now throws, as does an empty {uuid} (which would have reached Laravel's index route and returned every application under a URI claiming one).

The rest: the instructions.ts reference was to an unmerged sibling branch and is reworded; registeredPrompts now backs a boot-time roster check instead of being write-only; the "names only real tools" check is inverted to fail closed with a NOT_A_TOOL list for argument names; coverage added for fleet application reads, the estate_health plural branch, a rejecting read and a quoted application name; prompt arguments are JSON.stringifyd; {instance} gains completion on both templates; the user-role assumption about prompt arguments is now stated.

Not taken: the resources/list fan-out cache. A short TTL helps the second listing onwards but not the first, and it buys staleness on a surface whose job is to be current. The timeout half may be the more valuable one. Filed as #393 with the reasoning, and the cost is named in the code.

The rejecting-read assertion sits in the unit tests rather than the contract suite: the eval fixture answers an unknown uuid with HTTP 200 and a not-found body where real Coolify sends a 404. Worth knowing separately.

The contract gate failure was mine and is fixed: evals/ has its own prettier config, so those files must be formatted from inside evals/.

The three build (N.x) failures are not from this branch. They are GHSA-7w5x-hrqm-74c2 in smol-toml, which fails on all five open PRs. Fixed separately in #394, which is fully green. Merge that first and this goes green on rebase.

Now: 847 jest tests, 26 contract tests, tsc / lint / evals typecheck clean.

Three guided workflows and two attachable reads, so a client shows
workflows rather than only a list of 45 tools.

Prompts (`troubleshoot_application`, `explain_failed_deploy`,
`estate_health`) are pure text builders that make no API call.
`prompts/get` has no elicitation channel and no error a human can act
on, so a prompt that pre-fetched would turn a slash command into a
stall that can fail. Worse, embedding build output in the returned
message would put attacker-influenceable text in a user-role message,
outside the `asUntrustedLogs` boundary the tool layer puts around
exactly that text (FINDINGS #4). Naming the tool and letting the model
call it keeps the log on the one path that frames it as data.

A prompt may only name tools that exist in the current mode. Read-only
mode drops every mutating tool, and consolidation puts two pure reads
under destructive ones (`env_vars` list, `deployment` get), so this is
per-mode rather than constant. `definePrompt`'s `requires` drops a
whole prompt whose workflow cannot run — `explain_failed_deploy` is
simply not listed on a read-only server — and `ctx.has()` drops a
single step, so read-only troubleshooting keeps its log walk without
naming `env_vars`. Structural, not a list to remember.

Resources (`coolify://overview`, `coolify://application/{uuid}`) read
through the same client as every tool call, so `deepSanitize` applies
and a resource read can never be a masking bypass; the contract suite
asserts byte equality with `get_application` rather than assuming it.
No URI takes a `reveal`: a resource URI is a durable handle a client
may cache or paste, which is the last place for an opt-in to
plaintext. Fleet mode scopes every URI to its instance, because
reading prod's overview believing it is staging's is the mistake a
second instance invents.

`get_infrastructure_overview` moved to a shared method so the tool and
the resource cannot answer differently.

Prompts and resources are separate lists over separate methods, so the
tools/list token budget is unchanged. Contract snapshots, budgets and
the masking eval extend to both surfaces in single and fleet mode.

Claude-Session: https://claude.ai/code/session_01THRG6UFPUdMSARgJLcGf9a
Formatting: evals has its own prettier config, so `evals/**` must be
formatted from inside `evals/`. That is what failed the contract gate.

Review findings:

1. Listed resources inherited the template's `title`, so every
   application rendered as the identical row "Application detail" —
   `title` takes precedence over `name` in clients that implement it,
   which defeated the only reason the listing makes API calls. Each
   entry now carries its own title, and a test asserts no two listed
   resources share one, since the snapshot happily encoded the
   collision.

2. An empty `{instance}` capture fell through to the default instance,
   because `registry.get('')` returns the default exactly as
   `get(undefined)` does — a silent cross-instance read on the surface
   this design argues hardest about. Now throws. Same for an empty
   `{uuid}`, which would have reached `GET /applications/`, Laravel's
   index route, returning every application under a URI claiming one.

3. `prompts.ts` referenced `instructions.ts`, which lives on an
   unmerged sibling branch. Reworded to contrast with tool
   descriptions, which are on this branch.

4. `registeredPrompts` was written and never read. It now backs a
   boot-time roster check: a prompt missing from PROMPT_NAMES, or a
   PROMPT_NAMES entry that never registered on a full server, throws
   rather than showing up as a snapshot diff nobody reads.

5. The "names only real tools" contract check was fail-open — a prompt
   naming a tool that was neither registered nor in the allowlist
   passed silently. Inverted: every backticked token that is not a
   registered tool is a finding, with a NOT_A_TOOL list for the
   argument names the prose backticks. Forgetting to list an argument
   now fails loudly; forgetting a tool no longer passes quietly.

6. Coverage: fleet application-detail reads (the one resource path
   where a routing mistake serves prod under a staging URI), the
   estate_health plural branch, a rejecting resource read, and an
   application name containing a double quote.

7. `resources/list` fan-out cost documented and filed as #393 rather
   than papered over with an unmeasured cache.

8. Prompt arguments are `JSON.stringify`d into the text, `{instance}`
   gains a completion callback on both templates, and the user-role
   assumption about prompt arguments is stated rather than inferred.

The rejecting-read assertion lives in the unit tests, not the contract
suite: the eval fixture answers an unknown uuid with HTTP 200 and a
not-found body where real Coolify sends a 404.

Claude-Session: https://claude.ai/code/session_01THRG6UFPUdMSARgJLcGf9a
@StuMason
StuMason force-pushed the feat/371-prompts-and-resources branch from 83a69a8 to b5e8b6a Compare September 10, 2026 08:15
@StuMason
StuMason merged commit ce02d77 into main Sep 10, 2026
8 checks passed
@StuMason
StuMason deleted the feat/371-prompts-and-resources branch September 10, 2026 08:16
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.35%. Comparing base (af09b76) to head (b5e8b6a).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #392      +/-   ##
==========================================
+ Coverage   90.24%   90.35%   +0.10%     
==========================================
  Files          13       14       +1     
  Lines        1538     1555      +17     
  Branches      469      476       +7     
==========================================
+ Hits         1388     1405      +17     
  Misses         58       58              
  Partials       92       92              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

3.1: ship MCP prompts and resources, not just tools

1 participant