Skip to content

feat(skills): serve skill list from the daemon so MCP prompt skills are visible - #1891

Merged
Aaronontheweb merged 3 commits into
devfrom
fix/surface-mcp-prompt-skills
Aug 12, 2026
Merged

feat(skills): serve skill list from the daemon so MCP prompt skills are visible#1891
Aaronontheweb merged 3 commits into
devfrom
fix/surface-mcp-prompt-skills

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

What

netclaw skill list scanned the skills directory on disk, so it could never show dynamic MCP prompt skills — those exist only in the daemon's in-memory SkillRegistry (alongside file and server-feed skills) and load live via prompts/get. Operators saw "no prompts" while the agent already had them in its [skills] index, which read as a bug.

Change

  • GET /api/skills (SkillEndpointRouteBuilderExtensions.MapSkillEndpoints) — .RequireAuthorization(), same default policy as /api/stats/skills and /api/sessions. Returns the live registry as a SkillInventory wire DTO.
  • Metadata — the DTO carries Name, DisplayName, Description, Source (system/native/external/mcp), Version, UserInvocable/ModelInvocable, ArgumentHint, and for MCP prompts the ServerName, PromptName, and declared Arguments.
  • netclaw skill list — calls the endpoint through DaemonApi.GetSkillsAsync. It requires the daemon: when the daemon is unreachable, times out, or returns an unusable response (non-success status, non-JSON body, null body), it prints Daemon unavailable: … and exits non-zero. No disk fallback — a disk scan would silently omit the MCP prompts, which AGENTS.md's no-silent-fallbacks rule forbids.
  • skill_load — its not-found message no longer implies MCP prompts are unavailable; it points the model at the [skills] index without listing (leaking) audience-denied prompt names.

Design notes

  • MCP prompts stay in-memory and are fetched live (prompts/get); nothing new is written to disk. The registry is the source of truth the model's index already reads, so the operator surface reads it too.
  • The operator list serves the registry (full metadata), not the compressed [skills] index (truncated, audience-scoped) the model consumes.
  • The endpoint's trust tier (loopback operator / paired device bearer, identical to /api/mcp/statuses) is full-trust, so returning every server's prompts is not an audience leak — audience filtering is a per-session/model control, not an operator-endpoint one.

Tests

  • Endpoint — 401 unauthenticated; with a seeded MCP-prompt skill returns source=mcp plus its arguments, and a file skill as native.
  • CLI — renders skills from the daemon; and for each daemon-failure mode (unreachable, a non-JSON 200 body, a {"skills":null} body) reports Daemon unavailable and exits non-zero rather than crashing or degrading.
  • Regression: McpCommand/DaemonApi, SkillRegistry, SkillToolTests, and the full Cli skill area all green.

Adversarial review rounds

Two review passes ran against this PR; every finding is addressed in-branch:

  • Round 1 — the original disk-scan fallback could crash on a malformed daemon response. Resolved by removing the fallback entirely (no-silent-fallbacks rule): skill list requires the daemon and fails loudly.
  • Round 2 — pre-HTTP failures (malformed endpoint string, undecryptable device token) and corrupt local config still produced stack traces; help text/docs still claimed offline support; a 404 from a version-skewed daemon gave wrong advice; the skill_load note leaked an existence bit from the unfiltered registry. All fixed in the follow-up commit, with tests pinning each failure mode (exit 1 + Daemon unavailable) and the empty-inventory success case (exit 0).

Known follow-ups (out of scope here): skill show/skill remove still disk-scan and cannot address MCP prompt skills; scanner issues are not part of the daemon inventory (use skill issues).

Comment thread src/Netclaw.Cli.Tests/Skills/SkillCommandTests.cs Fixed
"Demo File",
"A file-backed skill.",
new FileSkillSource(
Path.Combine(paths.SkillsDirectory, "demo-file", "SKILL.md"),
"A file-backed skill.",
new FileSkillSource(
Path.Combine(paths.SkillsDirectory, "demo-file", "SKILL.md"),
Path.Combine(paths.SkillsDirectory, "demo-file")),
@Aaronontheweb Aaronontheweb added mcp Model context protocol server / client issues. skills Skill loading, handling, authoring, indexing, and evals. remote-access Network exposure, device pairing, tunnels, webhooks, and remote ingress labels Aug 12, 2026
…re visible

netclaw skill list scanned the skills directory on disk, so it never showed
dynamic MCP prompt skills. Those live only in the daemon's in-memory
SkillRegistry, next to file and server-feed skills, and load live via
prompts/get. Operators saw no prompts while the agent already had them in its
[skills] index, which read as a bug.

- Add GET /api/skills (RequireAuthorization, same policy as the other read
  endpoints), returning the live registry as a SkillInventory wire DTO. The DTO
  carries name, display name, description, source (system/native/external/mcp),
  version, invocability, argument hint, and for MCP prompts the server name,
  prompt name, and declared arguments.
- skill list now calls that endpoint. It requires the daemon: when the daemon is
  unreachable, times out, or returns an unusable response, it reports "Daemon
  unavailable" and exits non-zero, rather than silently degrading to a disk scan
  that would drop the MCP prompts (AGENTS.md: no silent fallbacks).
- skill_load's not-found message no longer implies MCP prompts are unavailable;
  it points the model at the [skills] index without leaking audience-denied
  prompt names.

Tests cover the endpoint auth gate and the MCP-prompt projection, the CLI
render, and the daemon-unavailable cases (unreachable, non-JSON body, null body).
@Aaronontheweb
Aaronontheweb force-pushed the fix/surface-mcp-prompt-skills branch from efea505 to 0876dd1 Compare August 12, 2026 00:49
Round-2 review findings, all addressed:

- A malformed daemon endpoint (no scheme, bad URI) or an undecryptable stored
  device token threw before any HTTP happened and crashed skill list with a
  stack trace. RunListAsync now ends with a trailing catch, like
  McpCommand.RunListAsync, so every pre-HTTP failure reports "Daemon
  unavailable" and exits 1.
- A 404 from a reachable daemon means the daemon predates /api/skills (the
  normal window after a CLI update). The message now says to restart the
  daemon instead of telling the operator to start it.
- Building the DI host for skill list parses netclaw.json and secrets.json; a
  corrupt file produced a stack trace. The route now catches config-load
  failures and prints the file problem plus a fix hint.
- The skill help text and the netclaw-operations reference doc claimed every
  subcommand works offline. Both now say list needs the daemon.
- Four stale comments still described the deleted disk fallback; all removed.
- skill_load's not-found note was gated on the unfiltered registry, which
  leaked whether any MCP prompts exist to a session denied all of them. The
  pointer at the [skills] index is now unconditional, so no registry state
  crosses the audience boundary.
- New tests pin the contract: empty inventory renders "No skills found" with
  exit 0; unreachable, 500, 404 (restart wording), non-JSON body, null body,
  pre-HTTP failure, and a missing daemon API all report "Daemon unavailable"
  with exit 1. Test temp dirs now live under the fixture dir so they are
  cleaned up.

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

/// prompt skills a disk scan cannot see. Throws <see cref="HttpRequestException"/>
/// (or a timeout) when the daemon is unreachable, so callers can fall back.
/// </summary>
public async Task<SkillInventory.Response?> GetSkillsAsync(CancellationToken ct = default)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) August 12, 2026 01:33
Comment on lines +114 to +124
catch (Exception ex)
{
// The request can fail BEFORE any HTTP happens: a malformed endpoint
// string (UriFormatException, NotSupportedException,
// InvalidOperationException) or a stored device token that no longer
// decrypts (CryptographicException). The endpoint and token are
// operator-editable configuration, so these are "daemon unavailable"
// reports too, not stack traces. Mirrors McpCommand.RunListAsync's
// trailing catch.
unavailable = $"the daemon request failed ({ex.Message})";
}
{
var configuration = new ConfigurationBuilder().Build();
// Nested under the test's own temp dir so Dispose cleans it up.
var paths = new NetclawPaths(Path.Combine(_dir.Path, $"api-{Guid.NewGuid():N}"));
@Aaronontheweb
Aaronontheweb merged commit 3d60f76 into dev Aug 12, 2026
23 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/surface-mcp-prompt-skills branch August 12, 2026 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mcp Model context protocol server / client issues. remote-access Network exposure, device pairing, tunnels, webhooks, and remote ingress skills Skill loading, handling, authoring, indexing, and evals.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant