feat: browser skill management console (spec 003 Phase 3)#215
Merged
Conversation
…ad (spec 003 phase 3)
Clean rewrite of the embedded static UI (index.html/app.js/styles.css):
status-first load with capability gating (writable/embeddingProvider),
table rebound from dead source_type/source_url to metadata.origin,
inline install bar (raw-string origin, no client-side detection),
per-row Update gated on a check:true preflight badge (Up to date /
Updatable / Pinned) plus Update-all, Remove with confirm(), a right-side
detail drawer reading GET /skills/{id}/content into an escaped <pre>,
non-blocking toasts replacing alert(), and a provider-gated Reindex
button. Dead CSS from earlier card-based UI iterations removed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0153XpJc4DrMRAJJqVDchxRm
The textContent/innerHTML escaper left " and ' intact, allowing untrusted skill content (id/name/origin/reason) to break out of quoted attributes (title=, href=, data-id=) and inject event handlers. Escape all five HTML metacharacters explicitly (SEC-7/SEC-8). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153XpJc4DrMRAJJqVDchxRm
…int (spec 003 phase 3) Add FastSkillService::infer_origin(&str) -> Result<Origin, ServiceError> in a new core::origin_infer module: the single canonical string->Origin classification path (git/http(s) URL, .zip URL, local path, or scope/skill[@Version] id), porting the CLI's detect_skill_source/ parse_git_url/is_skill_id logic into core so the HTTP install endpoint and the CLI's `add` command share one implementation instead of two drifting copies. - POST /api/v1/skills/install now takes { origin: string, groups?: [] } and resolves it via infer_origin before add_from_origin(Fresh); a bad ref or missing default repository is a 400, not a 500. - New GET /api/v1/skills/{id}/content (always-on read route, not write-gated) returns { path, content } for a skill's SKILL.md, path-confined to the configured skills directory via security::path::validate_path_within_root. - fastskill-cli's add command now delegates its common (no --source-type) Origin construction to service.infer_origin, applying --branch/--tag/ --editable on top of the inferred Origin. The --source-type override path keeps its own narrow per-variant construction. utils::is_skill_id/ parse_git_url/parse_skill_id/GitUrlInfo are now thin wrappers over the core seam instead of duplicating it. BREAKING CHANGE: InstallSkillRequest.origin changes from a typed Origin object to a plain ref string; there is no back-compat shim (greenfield API break per spec 003). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153XpJc4DrMRAJJqVDchxRm
…ase 3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0153XpJc4DrMRAJJqVDchxRm
Avoid disclosing the server's absolute filesystem layout in the
GET /skills/{id}/content response; the UI only needs the logical
location. Falls back to the bare file name. (opus review, Low)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0153XpJc4DrMRAJJqVDchxRm
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Final phase of spec 003 (browser skill management). Backend HTTP surface (Phases 0–2) shipped in #212/#213/#214; this delivers the web UI plus the two backend seams it needs. Grilled + opus-reviewed (verdict: SHIP).
What's new
Frontend — clean rewrite of the three embedded static files (vanilla JS, no build step). The old table bound dead pre-
Originfields (source_type/source_url) and hit a stale endpoint; this replaces it with a working console:.zipURL, local path, orscope/skill; the server infers the type (no client-side detection).SKILL.mdas HTML-escaped text (no Markdown; SEC-7/SEC-8)./status(writable+embedding_provider), fetched before first render — write controls and the Reindex button hide when unavailable; read-only/no-provider banners explain why.alert()), confirm-on-remove only.Backend
FastSkillService::infer_origin(&str) -> Origin— the single string→Origininference path. The CLI'sdetect_skill_source/parse_git_url/is_skill_idare now thin wrappers over it (drift eliminated).POST /skills/installnow accepts{ origin: "<ref string>", groups? }and resolves it server-side (bad ref / no-default-repo →400).GET /skills/{id}/content -> { path, content }, path-confined to the skills-dir viavalidate_path_within_root(canonicalizes, rejects escapes — never a traversal primitive). Returns the skills-dir-relative path, not the absolute server path.Vocabulary
Adds the Origin ref glossary term (the textual form of an
Origin; resolved byinfer_origin). "source" remains banned.Security
Opus review confirmed clean: path-confinement (symlink/
../absolute all rejected, no pre-canonicalization read), XSS escaping (theescapeHtmlhelper now covers& < > " '— an attribute-breakout hole found by the commit security review was fixed here), write-gating, and 400-not-500 error mapping. Regression tests cover traversal rejection, unknown-id/missing-file 404s, and the new install string shape.Testing
Full workspace
cargo test+clippy -D warningsgreen on the integrated branch.Notes
InstallSkillRequest.originis now a string (no shim), per ADR-0005.specs/003-...remains gitignored (local working artifact); the Phase 3 detail lives there.