v2: install from public skill catalogs, compat verdicts, agent-first find

[miljanm]

Summary

v1 was a read-only browser of the agent's skills. v2 keeps that reading experience and adds the write half of the skills story, built on the platform's new skills API.

⚠️ Merge order: this PR depends on mobius-os/mobius#146 (the skills platform backend). BOOTSTRAP_SKILLS_MANIFEST_URL tracks this repo's main, so merging here first would ship an app that calls APIs that don't exist yet — merge the platform PR, then this one. Bootstrap is failure-tolerant either way (a skew logs and retries next boot), but the order avoids the window.

What's new

• List from GET /api/skills: both skill shapes (flat <name>.md and directory <name>/SKILL.md), provenance chips (seed / agent / app-owned / installed-from), 30-day usage counts.
• Catalog screen: 7 curated public sources (Anthropic, Superpowers, Trail of Bits, Cloudflare, Hermes…), one recursive git-trees scan per source through /api/proxy, background summary prefetch, full-page skill detail with breadcrumbs, install via POST /api/skills/install (gated by the new manage_skills permission). Sources are app data (sources.json) — adding a repo is a chat request.
• Compat verdicts: green "✓ Works with Möbius" or amber "⚠ N things to know" chips with plain-language notes (files that won't install, bundled scripts never auto-run, broken references, missing summary). Pre-install on every catalog card and skill-page header, predicted from the already-fetched git tree — zero extra requests. Post-install on installed skills' rows and detail pages, judged from what's actually on disk.
• Agent-first Find: header button opens an agent chat with a prefilled draft; the platform's finding-skills seed skill is the playbook (sources, fit, trust ritual, exact API calls).
• Uninstall: two-tap remove for install-provenance skills (DELETE /api/skills/<name>; the server git-snapshots bytes first).
• Opening Browse fires POST /api/skills/catalog-index/refresh, keeping the agent's cached catalog index fresh.

Structure

• New catalog.js: dependency-free catalog core (source list, tree filtering, summary parsing, prefetch pool, compat assessment) — network injected, fully unit-tested.
• 64 tests total (npm test): existing domain + detail-nav suites kept, new catalog suite added.
• mobius.json → 2.0.0, adds catalog.js to source_files and the manage_skills permission.

🤖 Generated with Claude Code

[hamzamerzic]

Thorough review — round 1/2: authority, trust preview, and install consistency

Reviewed exact head 2e295a1. All 64 unit tests pass and the app validates/bundles successfully, but the install-facing flow needs changes before merge:

1. Blocker — Install is enabled before the trust/compatibility information exists. Catalog cards and the full skill page render an active Install button while SKILL.md is still loading (and also after it failed), so a quick tap can install a third-party instruction set before its description, body, or “things to know” verdict was available. The app’s stated trust ritual and pre-install compatibility promise therefore do not hold on the actual action path. Disable Install until the exact SKILL.md and verdict are ready; on fetch failure, keep it disabled and offer GitHub/Retry. For amber verdicts, make the warning visible before the installing tap rather than allowing the first action to bypass it.

2. Blocker — catalog navigation has stale-response races. openSource(A) and then openSource(B) leaves both scans alive; a slower A response can overwrite open, skillList, and descs after B is current. Cancelling the prefetch generation only stops new worker iterations: already-running loadDescription(A, sameDir) requests can still populate B’s descs[dir], because caches/inflight keys are directory-only and carry no source generation. Add a source/request generation (or AbortControllers), key caches by source+ref+dir, and regression-test A→B with reversed response order for both tree and markdown requests.

3. Blocker — preview and install are not one immutable revision. Tree scan, raw SKILL.md preview, compatibility verdict, and the POST currently all name mutable main. A source push between those steps can make the owner review one body/caveat set and install another. Coordinate with mobius#146: resolve one commit OID for the source scan, fetch previews from that OID, and send that exact OID as the install ref/expected revision. Displaying source provenance should retain it.

4. Do not treat a truncated whole-repo scan as a reliable catalog. treeScanUrl() scans the repository root even when the source has a subtree path, then filters client-side. This increases truncation risk and can omit valid skills arbitrarily. Use GitHub’s encoded <ref>:<path> tree spec like the backend index does, and treat truncated=true as an incomplete/error state rather than a normal install catalog.

The manage_skills permission separation, same-origin proxy use, sanitized Markdown rendering, and two-tap removal are good foundations. The remaining issue is making the reviewed bytes and the installed bytes provably the same, with no action before review data is ready.

[hamzamerzic]

Thorough review — round 2/2: UI state, compatibility parity, and accessibility

Second independent pass on exact head 2e295a1, covering installed-detail navigation, list/refresh overlap, compatibility mirroring, keyboard access, and error/partial states. Additional changes requested:

1. Blocker — catalog cards are not keyboard-accessible detail links. The card’s open action is an onClick on a plain <div> with no focusability, role, keyboard handler, or named open control. Keyboard users can reach Install and an amber caveat button, but cannot open a clean/green skill’s detail at all. Give the detail action a real button/link (without invalid nested controls) or a fully implemented focusable interaction with Enter/Space and an accessible name; add an accessibility-focused render test.

2. Compatibility logic does not actually mirror backend bounds. The backend permits at most 4 path segments, but assessCompat checks split(/).length - 1 <= 4, allowing 5. It also omits the backend’s rejection of dot-prefixed/empty/backslash segments. The badge can therefore say “Works with Möbius” while the installer drops files. Share/derive one contract if possible; otherwise copy it exactly and lock representative depth-4/depth-5, .hidden, double-slash, and backslash cases.\n\n3. Directory-skill Markdown links are misrouted as global skills. In an installed <id>/SKILL.md, a normal relative link like reference.md usually names a bundled resource in that same directory. classifyLink() instead treats every same-folder .md as another top-level skill slug, then emits “unknown skill”. Route directory-skill resource links within the current skill (through the shared-storage path, safely), or deliberately block them with correct copy; preserve the old cross-skill behavior only for legacy flat skills.\n\n4. Primary list loads need generation control. The supplemental apps loader is generation-safe, but /api/skills is not. Initial load, manual refresh, post-install, and post-uninstall can overlap; an older response may overwrite the newer list/error state and app_ready count. Add the same newest-generation-wins discipline and cover reversed completion order. Also await the post-uninstall reload (or explicitly retain a truthful pending state) rather than closing on a fire-and-forget refresh.\n\n5. Installed compatibility traversal can produce false warnings. Nested path segments from shared-list are interpolated into URLs without segment encoding; valid installed names containing spaces, #, ?, or % can break the request. The BFS also stops after 12 directories and returns the partial list as complete even though the installer permits 24 resources across more directories. Encode each segment, and return “unknown/no verdict” whenever pagination/depth/page bounds leave a nonempty queue instead of assessing partial data as complete.\n\n6. Normalize catalog identity the same way install does. The backend derives install IDs in lowercase, while treeToSkills()/existingIds compare source basenames case-sensitively. A PDF/SKILL.md card can remain install-enabled after pdf is installed and then fail with a 409. Use the server-returned/derived normalized ID for installed checks and duplicates. Validate/normalize sources.json before rendering too; do not pass arbitrary missing/ill-typed fields into URL builders.\n\nUnit tests and real bundling remain green; these are cross-component and interaction-state gaps the pure happy-path suite currently misses. Round 2 remains changes requested.

[hamzamerzic]

Independent reconciliation review — HOLD on exact 2e295a1

All 64 app unit tests pass, but the two existing exact-head changes-requested reviews remain blocking (install-before-preview, mutable preview/install mismatch, stale source races, truncated scans, keyboard access, parity, and newest-request-wins).

This app also cannot be published independently of mobius#146: v2 makes /api/skills its primary load path and requests manage_skills, but the current platform has neither endpoint and silently ignores the unknown permission. Merging v2 to the mutable main manifest before core is deployed makes an owner-triggered update install a broken app; core’s bootstrap then consumes that same mutable manifest. Sequence core deploy first, or publish a versioned manifest with an explicit minimum-platform gate and pin core bootstrap to that reviewed release/commit.

One more trust invariant should be locked across the pair: the preview, compatibility verdict, install POST, installed provenance, and later GitHub link must all carry one immutable commit OID. A repository/ref label is not enough evidence that the bytes reviewed are the bytes installed.

[miljanm]

All four findings addressed in 0c0759c (69 tests pass, bundle + validator clean):

1. Install gated on review data — Install is disabled (cards and skill page) until the exact SKILL.md and its compat verdict are rendered. A failed fetch keeps it disabled and offers Retry (cards) / Try again + GitHub link (skill page). Amber verdicts auto-expand their notes on the skill page, so the warning is on screen before the first moment Install can be tapped.

2. Stale-response races — every scan takes a generation token (createGenerationGuard in catalog.js); openSource, loadDescription, and the error/busy paths all drop responses holding a stale token, and per-source caches (descs, inflight set, compat cache) are reset at token issue. Unit-tested with A→B reversed-response races for both the tree and the markdown request (the markdown case uses the same dir key in both sources, per the review).

3. One immutable revision — openSource resolves the source ref to a commit OID before anything else; the scoped tree listing, every SKILL.md preview, the compat verdict, the GitHub page link, and the install POST ref all name that OID. Combined with mobius#146 (which resolves/pins server-side and records the OID in provenance), the reviewed bytes are provably the installed bytes.

4. Scoped scan, truncation is an error — the tree scan uses the <oid>:<path> tree spec like the backend index (paths re-prefixed to repo-relative at the scan boundary), and truncated: true now surfaces as a scan error instead of a footnote under a silently incomplete list.

🤖 Generated with Claude Code

[hamzamerzic]

Fresh exact-head re-review — HOLD on 0c0759c

This head fixes the four authority/OID-generation findings from round 1, and the hosted/local tests are green (69/69). However, all six findings in the round-2 UI/compatibility review remain reproducible on this exact head: mouse-only catalog cards, client/backend compatibility-rule mismatch, directory-relative Markdown links treated as global skills, non-generation-gated primary loads plus unawaited post-uninstall reload, incomplete/unencoded installed traversal reported as complete, and unnormalized catalog/source identity.

One boundary is now clearer: the active catalog preview/install path consistently uses one resolved OID, but installed state discards the install response's commit (index.jsx:582-586), platform GET /api/skills omits it, and installed detail has no pinned-source review link. Preserve that evidence across the lifecycle.

Release pairing is also still blocked: v2 declares no enforced minimum platform/API contract, while the platform bootstrap consumes mutable app-skills/main. See the current core exact-head review. Please resolve the six linked items and the immutable/rolling-compatible release contract before merge.

[miljanm]

Round-2 and the re-review items addressed in 5437897 (80 tests incl. a new render test; CI green):

1. Keyboard-accessible cards — the open action is now a real <button class="sk-cardopen"> whose accessible name is the card's own title + summary; Install/Retry/caveat controls live outside it (no nested interactive content). Verified by an actual render test: test/a11y.test.js bundles the real CatalogCard with esbuild and renders it through react-dom/server against the shell's frontend deps (CI's MOBIUS_FRONTEND_NODE_MODULES; skips cleanly where absent), asserting the semantic button, its accessible content, and the absence of nested controls.
2. Exact compatibility parity — resourceRelOk() in catalog.js now mirrors _resource_rel_ok case for case: max 4 path segments (the old check allowed 5), and empty/dot-prefixed/backslash segments are dropped. The boundary cases you named (depth-4 vs depth-5, .hidden, //, \) are locked in tests on both the rule and through assessCompat.
3. Directory-skill links — classifyLink(href, { dirSkill }) classifies relative refs inside a <id>/SKILL.md as that skill's bundled resources. The installed view explains them in place ("…is a file bundled inside this skill…") instead of emitting "unknown skill"; catalog previews block them; flat skills keep the legacy cross-skill slug convention (tested for all three).
4. Generation-safe primary loads — /api/skills now goes through createSkillsLoader (domain.js) with the same newest-generation-wins discipline as the apps loader; reversed-completion and stale-failure races are unit-tested, and app_ready can only fire from an applied response. The post-uninstall reload is awaited behind the truthful removeBusy pending state before the detail closes.
5. Installed traversal — listInstalledFiles (extracted to catalog.js) segment-encodes every URL component and returns null → "no verdict" whenever the walk is incomplete: failed page, paginated (>200-entry) directory, a directory at the depth cutoff, or the page budget running out with work queued. Partial data is never assessed as the complete skill (each case tested).
6. Normalized identity — installed checks and duplicate detection compare installIdOf() (the lowercased id the server actually derives — PDF/SKILL.md correctly shows Installed once pdf exists), and sources.json overrides pass through normalizeSources() (installer-shape repo/path/ref, 12-source cap, sanitized labels) before anything renders or builds URLs from them.

Commit evidence across the lifecycle — the platform now returns commit + source coordinates on GET /api/skills (mobius#146 5af6cd98), the app retains them in its rows, and installed detail links "source @ <oid7>" to the exact reviewed revision on GitHub.

Release pairing — resolved on the core side: BOOTSTRAP_SKILLS_MANIFEST_URL is now pinned to the reviewed v1.1.2 commit (fd09c99…), never mutable main, and Skills no longer auto-reinstalls after an owner uninstall. v1 works on any core, so the pin is the rolling-compatible fallback; this v2 gets pinned there only once both sides have merged, as a deliberate platform commit. Mutable-main consumption is gone from the path entirely.

🤖 Generated with Claude Code

[hamzamerzic]

Exact-head review — round 1/2: catalog identity and reviewed-byte parity (5437897)

The six prior UI/compatibility findings are largely fixed and the 80-test suite is green. The catalog identity boundary still needs changes:

1. Blocker — treeToSkills() still admits names the installer rejects. installIdOf() only lowercases; it does not enforce the backend ^[a-z0-9][a-z0-9._-]*$ contract. I reproduced cards for Bad Skill, a#fragment, and similar Git directories becoming fully reviewable/installable, then inevitably receiving a backend 400. Filter invalid derived IDs (or render them explicitly unsupported with Install disabled), and test spaces, #, ?, percent, leading punctuation, and non-ASCII names.

2. Blocker — normalization creates duplicate install identities without deduplication. A tree containing both PDF/SKILL.md and pdf/SKILL.md produces two cards with the same id: pdf; before either is installed both actions are live, and after one install both become “Installed.” This is not a trustworthy one-card→one-install mapping. Group by normalized install id and either choose only an exact-lowercase unambiguous path or render a collision that cannot be installed. Lock case-collision and duplicate-path regressions.

3. Blocker — preview/link URLs do not encode Git path segments. rawSkillUrl() and githubSkillUrl() interpolate dir directly. I reproduced skills/a#fragment turning the remainder into a URL fragment and skills/a?download=1 turning /SKILL.md into query text. The backend installer percent-encodes the repo path, so the app can review/fail on a different URL while install fetches the real path—breaking the exact-preview/exact-install invariant even with one OID. Encode every repo-relative segment for raw and GitHub links (preserving / separators), and test space, #, ?, %, and Unicode paths end to end.

normalizeSources() has the same partial validation issue for owner-supplied paths: it omits the backend control-character check and sends path text into URL builders. Share one path normalizer or make the client at least as strict as the server.

Round 1 remains changes requested.

[hamzamerzic]

Exact-head review — round 2/2: mutation settlement and stale UI (5437897)

Independent pass over install/uninstall settlement, navigation races, accessibility render coverage, and installed provenance. All 80 tests—including the real rendered-card test—pass with shell dependencies, and the app validator bundles cleanly. Mutation state is still not truthful under failure/overlap:

1. Blocker — a successful uninstall can close onto a stale row. uninstallCurrent() awaits load(), but load() swallows the refresh failure into loadError and returns no success outcome. The code then always closes the detail while preserving the last-known-good skills array, which still contains the now-deleted skill. The comment claiming the list is “already current” is false on network/HTTP failure. Have load() return its applied/ok result; after a confirmed DELETE, either remove that id optimistically from authoritative local state or keep a truthful settled-removal state until reconciliation succeeds. Test DELETE success + list failure and superseded reload.

2. Blocker — install clears its pending state before authoritative reconciliation. install() calls onInstalled() without awaiting it, then immediately clears busyDir. Until the parent load finishes, the successfully installed card becomes tappable again and still says Install; if the reload fails, it stays that way indefinitely despite the success notice. Await a result-bearing reconciliation or mark the returned data.name installed optimistically, and keep the action disabled through settlement. Cover POST success + delayed/failed list refresh.

3. Mutation completions are not generation/source scoped. While install A is pending, the user can navigate back and open source B or start install B. A’s later completion writes B’s global notice/error and clears the single busyDir, potentially re-enabling B while B is still pending (same dir names make this especially visible). Serialize installs or track per-operation ids plus a catalog generation, and drop stale presentation writes while still reconciling the global installed list.

Related race: two overlapping refresh() callers can let a superseded older call set refreshing=false while the newer request remains pending. A small ref-count/request-token owner avoids the same premature-idle pattern.

The installed source link should use the same segment encoder requested in round 1; sourcePath is provenance data, not a pre-escaped URL. Also remove the trailing blank line flagged by git diff --check in README before merge.

Together with round 1’s name/path identity defects, this remains changes requested.

[hamzamerzic]

Independent exact-head follow-up — remaining cross-contract/UI blockers (5437897)

The two fresh reviews already cover catalog identity/URL encoding and mutation settlement. A separate clean-clone pass reproduced four additional blockers on the same head; hosted CI, 80/80 tests, git diff --check, and the core validator are otherwise green.

1. Compatibility is false-green above the backend size limit. assessCompat() never checks SKILL.md byte size. A 300,000-byte file reports {ok:true}, while core caps the fetched skill at 256 KiB. Mirror that limit client-side and lock boundary tests; the server remains authoritative.
2. Installed traversal cannot be complete across the paired contracts. The installer permits nonempty segments containing spaces, #, ?, and %, while shared-list silently omits names outside its narrower path regex and returns next_cursor:null. listInstalledFiles() then assesses the partial inventory as complete. Align the contracts or expose the installer-owned bounded files inventory on /api/skills; URL encoding alone cannot detect silently omitted files.
3. The catalog detail header clips at 320 px. An exact-CSS Chromium probe measured clientWidth=320, scrollWidth=398, with the breadcrumb collapsed to zero while fixed Back/compat/Install/GitHub actions occupy the nonwrapping row. Reflow or stack the controls and add 320/375/390 px coverage.
4. Detail content also lacks generation ownership. The installed Markdown request can complete after refresh/navigation and overwrite the newer detail. Apply the same newest-generation-wins discipline used by the primary list.

Also avoid eager raw prefetch of hundreds of skills when opening a large source, and include ref in sourceKey so distinct refs of one repo/path cannot share cache state. Core #146 remains a required release pair and is independently held; v2 must not publish first.

[miljanm]

Thanks — all three exact-head reviews addressed at new head 8a6c870 (87 tests green, incl. the real esbuild render test + new regressions).

Round 1 — catalog identity & reviewed-byte parity

1. treeToSkills admits names the installer rejects → installIdOf now enforces ^[a-z0-9][a-z0-9._-]*$ (returns null otherwise); such cards render installable:false — explicitly "Unsupported", Install disabled. Tests cover spaces/#/?/%/leading-punct/non-ASCII.
2. Duplicate install identities → treeToSkills groups by normalized id; two dirs mapping to one id (PDF vs pdf) are marked collision/non-installable. Test: treeToSkills: two dirs normalizing to one id....
3. Unencoded path segments → new encodePath percent-encodes each segment (/ preserved) in rawSkillUrl/githubSkillUrl, matching the backend's quote(..., safe='/'); normalizeSources now rejects control chars in the path. Tests assert #/?/space/Unicode end-to-end.

Round 2 — mutation settlement

1. Uninstall closes onto a stale row → load() now returns its applied/ok outcome; after a confirmed DELETE the id is dropped from authoritative local state immediately, so a failed reconciling reload can't leave the removed skill showing.
2. Install clears pending before reconciliation → install awaits onInstalled() (busy stays set through settlement) and optimistically marks data.name installed; the card stays disabled until settled.
3. Completions not generation/source-scoped → installs serialize (anyBusy), presentation writes are token-scoped (a completion in another source view won't leak its notice or re-enable a sibling), and refresh() ref-counts so an older overlap can't stop the spinner early.

Independent follow-up

1. False-green above the size cap → assessCompat mirrors the 256 KiB SKILL.md cap (boundary-tested).
2. Installed inventory can't be complete → the backend now exposes the installer-owned bounded files list on /api/skills (paired core change); the app assesses from THAT, falling back to the shared-list walk only when absent. mapSkillRows carries it (test added).
3. Header clips at 320px → actions moved into .sk-detail-actions with flex-wrap; the crumb keeps a min-width. Reflows/stacks at 320/375/390.
4. Detail content lacks generation ownership → the installed-Markdown fetch is now newest-generation guarded.

• Large sources warm only the first ~40 summaries (rest load on scroll); sourceKey includes ref.

Release pairing acknowledged: v2 stays held behind core #146; it must not publish first.

[hamzamerzic]

Exact-head architecture review — changes requested (8a6c870)

The identity, collision, and encoded catalog-link fixes are substantial, and all tests pass. Three state/contract gaps remain.

Blockers

1. A skill that cannot install is still offered for install. assessCompat() flags a SKILL.md over the platform’s hard 256 KiB fetch limit, but the button only requires a truthy compat object. It remains enabled even though the backend must reject the fetch.

2. Installed state has two owners. installedLocally only grows and CatalogScreen remains mounted. After installing in the catalog, closing it, uninstalling from the installed view, and reopening the catalog, the local set can keep the card permanently labelled Installed after the authoritative set removed it.

3. The installed-detail source link still splices a raw Git path. current.sourcePath is inserted directly into the GitHub blob URL, so #, ?, spaces, and similar characters can make the link identify different bytes or fail. The catalog path fixed this, but the installed path does not reuse that encoder.

Direct fix

Use one closed installability result (loading | installable | unsupported) derived from the same hard contract as the backend; a hard-limit violation is unsupported, not an amber-but-runnable action. Keep one installed-id set at the app root and update that same set from confirmed install/uninstall results and list refreshes—remove the monotonic child overlay. Reuse one segment-encoding Git URL helper everywhere.

Because v2 and its platform API are landing together, the app can require the new authoritative file inventory rather than keeping an incomplete legacy listing fallback.

Verification

• All 87 tests pass on the exact head.
• Hosted CI is green; the missing cases are lifecycle/contract counterexamples rather than test failures.

[miljanm]

Round-4 blockers fixed at d9e6226. Each gap is collapsed into one closed contract as directed.

B1 — a skill that can't install is still offered. Added one closed installability result — installability(skill, compat) → loading | installable | unsupported — derived from the same hard contract as the backend (BLOCKING_CAVEATS, currently skill-too-large). A SKILL.md over the 256 KiB fetch cap, an invalid id, or a duplicate id is unsupported with Install disabled; soft caveats (dropped / over-budget / scripts / frontmatter) stay installable. Every Install control — cards and detail — derives from this, never from the mere presence of a compat object.
Tests: installability: an over-cap SKILL.md is unsupported…, …soft caveats stay installable, …loading until compat is known, …invalid or colliding id is unsupported, BLOCKING_CAVEATS holds only the hard-reject kinds, plus a render-level catalog card test asserting the over-cap Install button is disabled+Unsupported while a soft-caveat one stays enabled.

B2 — installed state had two owners. Removed the monotonic installedLocally overlay. The single installed-id set is the app-root existingIds (from /api/skills), refreshed after every confirmed install and uninstall. Install awaits that refresh with busyDir held, so the card shows “Installing…” through settlement, then reflects the authoritative set. It can now go Installed → not-installed when the set shrinks — no permanent latch.
Test: the render catalog card test drives installed false → true → false and asserts the button follows the prop both ways (Install/enabled ↔ Installed/disabled).

B3 — installed-detail source link spliced a raw git path. The “source @” link now reuses githubSkillUrl (the same per-segment encoder as the catalog links), so #, ?, spaces, etc. can't make it identify different bytes.
Test: installed-detail source link is built through the segment-encoding helper (skills/a b#x → skills/a%20b%23x).

Legacy fallback dropped. Since v2 and the platform API land together, both installed-compat effects now require the authoritative files inventory; the incomplete listInstalledFiles shared-list fallback (and its fetchJson) is gone.

94 tests pass (0 skipped — the render tests run with frontend deps present); the esbuild bundle compiles.

[hamzamerzic]

Approved after two exact-head review rounds against the platform Skills contract.

The app now treats a confirmed install result as authoritative before refresh, keeps one root-owned installed list, holds install busy state through reconciliation, and replaces timing sleeps with a deterministic prefetch completion contract. The exact head passed all 95 tests twice and the app validator reported no errors; its three URL warnings are static false positives because API/raw fetches go through the same-origin proxy and GitHub page URLs are external links.