Skip to content

feat: agent-native discovery and compatibility - #90

Merged
mrgoonie merged 4 commits into
mainfrom
feat/agent-native-discovery-v2
Oct 15, 2026
Merged

mrgoonie merged 4 commits into
mainfrom
feat/agent-native-discovery-v2

Conversation

@mrgoonie

@mrgoonie mrgoonie commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Rebuilds #41 (agent-native discovery and compatibility) on current main. The slices that main has not already superseded are ported, and every review finding on #41 is fixed and covered by tests.

Supersedes #41.

What is ported

  • DB: skills.compatibility_json (a nullable text column), kept under its production migration name 0010_greedy_famine.sql (see "Migration decision" below).
  • CLI (skillx-sh):
    • new inspect and check commands;
    • search --compatible <runtime[@version]> and search --json;
    • shared compatibility contracts.
  • Web:
    • /skills/<slug>.md Markdown pages;
    • /llms.txt and /llms-full.txt;
    • the /api/mcp JSON-RPC endpoint with 4 read-only tools (search_skills, get_package, check_compatibility, get_release);
    • the compatible filter on search;
    • the target query param on the skill detail API.
  • Docs:
    • docs/api-reference.md gets a new "Agent Surfaces" section;
    • docs/cli-reference.md documents the new commands;
    • docs/deployment-guide.md documents the migration reconciliation;
    • the routes table in CLAUDE.md gains the new routes.

What is dropped

Item from #41 Why
CLI version.ts change Superseded by #24 and the version handling now on main
ci.yml and typecheck script changes Superseded by #45/#76/#82 (main's CI and scripts)
pnpm-lock drift fix main's lockfile is already consistent
auth-schema split Already on main
identifier.ts Superseded by main's identity parsing in use.ts
MCP get_kit tool Removed per review finding H2 (it exposed a kit surface with no access model)

Review findings and fixes

Finding Fix Test
B2: get_release bypassed release access It now goes through the release-access module as the anonymous viewer, so private, draft and quarantined releases are never served, even when the request carries a session. tests/releases/mcp-get-release.test.ts
H1: MCP slug lookups ignored aliases Lookups use fetchSkillBySlug, which resolves aliases and returns the canonical slug. tests/db/mcp-alias-lookup.test.ts
H2: get_kit exposure The tool is removed, and the docs harness is updated to match. mcp.test.ts, api.mcp.test.ts (4 tools)
H3: unbounded llms queries The query selects only the needed columns and fetches one probe row instead of a count. limit is snapped to an allowlist (index 100/250/500/1000, full 25/50/100/200), and the cache key uses the normalized limit. llms-limit.test.ts, public-catalog-index.test.ts, llms-txt.test.ts
M1: unbounded MCP body and no rate limit Requests are rate-limited (429 with Retry-After) and then size-capped: bodies over 64 KiB get 413, checked on both Content-Length and streamed bytes. api.mcp.test.ts
M2: internal errors leaked to callers Callers get a generic internal_error, and the detail is logged server-side only. An unknown tool name or method is not echoed back. mcp.test.ts, api.mcp.test.ts
M3: .md pages read the session The handler reads no session, uses the anonymous viewer, and sends Cache-Control: public, max-age=300 with no Vary. skill-markdown-handler.test.ts
M4: no write path for compatibility_json Not fixed here. It is listed under known gaps. -
M5: wrong "pool exhausted" detection Detection goes through a shared isPoolExhausted helper. compatibility-filter.test.ts
L1: loose runtime grammar Runtime IDs and versions are validated. An invalid compatible value gets a 400 with no echo, and the empty-result note text is fixed. api.search.test.ts, compatibility-filter.test.ts, mcp.test.ts
L2: skills/:slug.md partial dynamic segment React Router does not support partial segments, so the worker now handles .md paths for GET and HEAD before React Router sees them, and the route is removed from the route table. routes-config.test.ts, skill-markdown-handler.test.ts

Rate limiter decision: MCP reuses the existing RELEASE_READ_LIMITER binding (300 per 60 s) under the key prefix mcp:<caller>. Like the release API, it fails open when the binding is missing, so no new binding or deploy config is needed.

Production D1 findings (read-only: SELECT/PRAGMA only)

  • skills.compatibility_json already exists in production (PRAGMA table_info(skills), cid 28).
  • d1_migrations has 18 rows, up to 0016_personal_kit_proposals.
  • It also includes 0010_greedy_famine.sql, applied 2026-09-22 from an earlier feat(phase-1): agent-native discovery and compatibility #41 branch.

Migration decision

  • Wrangler decides which migrations are pending by file name. A new file name for this column would be treated as pending and fail on a duplicate column. The migration therefore keeps the exact name production recorded, 0010_greedy_famine.sql.
  • It sits in the drizzle journal at idx 17 with a fresh snapshot, and drizzle-kit generate now reports "No schema changes".
  • wrangler d1 migrations list skillx-db --remote on this branch reports "No migrations to apply!".
  • A test pins the file name, the SQL, the journal tag and the fresh-DB column shape.
  • Nothing was applied to remote and nothing was inserted into the remote d1_migrations.

Deploy findings

  • No workflow deploys the web worker or migrates D1. The workflows are pr-checks, release-cli (npm), publish-release (manual) and skill-tree-ci, so web deploys and migrations are manual.
  • This PR needs no migration step in prod.
  • Before deploy, a D1 Time Travel bookmark was recorded and a per-table export was taken. A full wrangler d1 export is impossible because the database has FTS5 virtual tables.

Known gaps

  • M4: there is no write path that populates compatibility_json yet (no admin or seed flow), so production returns "unknown" for every skill until one exists.
  • The search 500 envelope echoes error.message. This is pre-existing on main and out of scope here.

Test results

  • pnpm install --frozen-lockfile: ok.
  • pnpm typecheck: exit 0.
  • pnpm test: 91 files, 2021 passed, 2 skipped, 0 failed.
  • pnpm --filter skillx-sh build: ok. Smoke runs passed: --version (0.4.1), --help, inspect --help, check --help and release publish --help.
  • pnpm --filter web build: ok, and the worker bundle includes the Markdown handler.
  • There is no lint script.

Re-review of bb7776d

An isolated re-review confirmed all four fixes, with tests: danger bodies dropped from llms-full, fences a body cannot close, compatibility declarations validated on every read, MCP pinned to the anonymous viewer, and nosniff plus the v5 cache key. No blockers. Gaps outside these fixes are tracked in #91. Supersedes #41.
🤖 Generated with Claude Code

…n name

Add the nullable compatibility_json text column to the skills schema and
generate its drizzle snapshot and journal entry.

Production D1 already has the column and records 0010_greedy_famine.sql in
d1_migrations, and wrangler decides pending migrations by file name. The
migration keeps that name so production has nothing to apply and fresh
databases still create the column. A test pins the name, the SQL and the
journal tag, and the deployment guide records the reconciliation.
skillx inspect shows a skill's metadata, requirements and declared
compatibility; skillx check gates on a runtime target and answers with its
exit code (0 actionable, 4 not, 3 missing, 2 bad input). skillx search takes
--compatible and --json and prints the filter report.

Lookups reuse use.ts identity parsing, so org/repo/skill resolves through
the repo-path endpoint. The contracts package gains the catalog
compatibility resolver shared by the web app and the CLI.
Add the read-only agent surfaces:

- POST /api/mcp: MCP over JSON-RPC with search_packages, get_package,
  get_release and check_compatibility. get_release reads through the
  release access module as the anonymous viewer, so private, draft and
  quarantined releases are never served. Slug tools resolve aliases.
  Bodies over 64 KiB get 413, callers are counted on RELEASE_READ_LIMITER
  under an mcp: key, and handler failures return a generic internal_error
  while the cause is only logged.
- /skills/<slug>.md: the skill as Markdown, always anonymous and publicly
  cached. React Router has no partial dynamic segment, so the worker entry
  matches the path before routing; a route-table test pins that.
- /llms.txt and /llms-full.txt: a column-limited, bounded query with a
  probe row instead of a catalog count; ?limit= snaps to fixed sizes and
  the KV key uses the normalized size.
- compatible filter on /api/search, compatibility on the detail API and
  page, WebMCP read tools, and the harness and publisher docs pages.

Runtime ids are validated against one grammar and never echoed, and the
pool-exhausted flag now holds for any filled pool.
@mrgoonie mrgoonie self-assigned this Oct 15, 2026
- llms-full.txt states each entry's scanner risk label, omits the body of a
  danger listing behind a warning, and fences bodies with a backtick run longer
  than any inside them so a SKILL.md cannot close its own block.
- Persisted compatibility declarations go through the shared contract validator;
  an invalid shape is treated as no declaration (unknown), never as declared.
- /api/mcp runs every tool as the anonymous viewer and no longer reads the
  request's session or API key.
- The .md, llms.txt and llms-full.txt responses send X-Content-Type-Options:
  nosniff. The llms cache key is bumped for the new document shape.
@mrgoonie
mrgoonie merged commit 40217d4 into main Oct 15, 2026
1 check passed
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.

1 participant