From 965e4a2bd6d4f20483692f9251890891883e93d8 Mon Sep 17 00:00:00 2001 From: ModeIO Local Date: Wed, 15 Apr 2026 14:59:31 +0800 Subject: [PATCH 1/3] Simplify legacy storage and marketplace cache schemas --- README.md | 21 +++++- frontend/src/__tests__/App.test.tsx | 12 +-- .../components/MarketplaceCard.test.tsx | 19 +---- .../components/MarketplaceDetailView.test.tsx | 38 ++-------- .../screens/MarketplacePage.loading.test.tsx | 20 +---- .../screens/MarketplacePage.test.tsx | 74 ++++++------------- .../src/features/marketplace/test-fixtures.ts | 59 +++++++++++++++ .../skills/screens/ManagedSkillsPage.tsx | 2 +- skill_manager/application/container.py | 3 +- .../application/marketplace/cache.py | 9 +-- .../application/marketplace/catalog.py | 17 +++-- .../application/marketplace/queries.py | 2 +- .../application/marketplace/repo_snapshots.py | 12 +-- .../application/marketplace/resolver.py | 14 ++-- .../application/read_model_service.py | 5 +- skill_manager/harness/resolution.py | 2 - skill_manager/sources/github.py | 5 -- skill_manager/storage_paths.py | 62 ++++++++++++++++ skill_manager/store/__init__.py | 5 +- skill_manager/store/harness_support.py | 14 +--- skill_manager/store/shared_store.py | 39 ---------- tests/support/marketplace_fixture.py | 7 +- tests/unit/test_github_repo_metadata.py | 37 +++++++++- tests/unit/test_marketplace_service.py | 16 ++-- tests/unit/test_storage_paths.py | 67 +++++++++++++++++ tests/unit/test_store.py | 38 +--------- 26 files changed, 311 insertions(+), 288 deletions(-) create mode 100644 frontend/src/features/marketplace/test-fixtures.ts create mode 100644 skill_manager/storage_paths.py create mode 100644 tests/unit/test_storage_paths.py diff --git a/README.md b/README.md index 75cca00..b99425a 100644 --- a/README.md +++ b/README.md @@ -185,7 +185,26 @@ Default local URLs: ## Configuration -By default, `skill-manager` resolves harness paths from `HOME` and `XDG_CONFIG_HOME`. You can override individual roots with environment variables. +`skill-manager` resolves its own app paths through: + +- config: `app_config_dir(env)` +- data: `app_data_dir(env)` +- state: `app_state_dir(env)` + +On macOS, those all resolve under `~/Library/Application Support/skill-manager`. + +App-owned storage layout: + +- shared managed store: `~/Library/Application Support/skill-manager/shared` +- marketplace cache: `~/Library/Application Support/skill-manager/marketplace` +- harness support settings: `~/Library/Application Support/skill-manager/settings.json` + +Compatibility behavior is intentionally narrow: + +- managed shared skills still fall back to an initialized legacy shared store at `~/.local/share/skill-manager/shared` +- marketplace cache does not use a legacy fallback because it is disposable and rehydrates cleanly + +You can still override individual harness roots with environment variables when you need to test or relocate a specific global skill directory. ### Codex diff --git a/frontend/src/__tests__/App.test.tsx b/frontend/src/__tests__/App.test.tsx index 54bbd1b..2ac80f6 100644 --- a/frontend/src/__tests__/App.test.tsx +++ b/frontend/src/__tests__/App.test.tsx @@ -3,6 +3,7 @@ import { MemoryRouter } from "react-router-dom"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { App } from "../App"; +import { createMarketplaceItem } from "../features/marketplace/test-fixtures"; const fetchMock = vi.fn(); @@ -196,22 +197,15 @@ function mockSkillsPage(options?: { codexSupportEnabled?: boolean }) { ok: true, json: async () => ({ items: [ - { + createMarketplaceItem({ id: "skillssh:mode-io/shared-audit:shared-audit", name: "Shared Audit", description: "Shared audit workflow", installs: 44, stars: 33, repoLabel: "mode-io/shared-audit", - repoUrl: "https://github.com/mode-io/shared-audit", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", - skillsDetailUrl: "https://skills.sh/mode-io/shared-audit/shared-audit", installToken: "token-shared-audit", - installation: { - status: "installable", - installedSkillRef: null, - }, - }, + }), ], nextOffset: null, hasMore: false, diff --git a/frontend/src/features/marketplace/components/MarketplaceCard.test.tsx b/frontend/src/features/marketplace/components/MarketplaceCard.test.tsx index 312c9e3..511ce82 100644 --- a/frontend/src/features/marketplace/components/MarketplaceCard.test.tsx +++ b/frontend/src/features/marketplace/components/MarketplaceCard.test.tsx @@ -1,25 +1,10 @@ import { fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; -import type { MarketplaceItemDto } from "../api/types"; +import { createMarketplaceItem } from "../test-fixtures"; import { MarketplaceCard } from "./MarketplaceCard"; -const baseItem: MarketplaceItemDto = { - id: "skillssh:mode-io/skills:mode-switch", - name: "Mode Switch", - description: "Switch between supported skill execution modes.", - installs: 128, - stars: 512, - repoLabel: "mode-io/skills", - repoUrl: "https://github.com/mode-io/skills", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", - skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", - installToken: "token-mode-switch", - installation: { - status: "installable", - installedSkillRef: null, - }, -}; +const baseItem = createMarketplaceItem(); describe("MarketplaceCard", () => { it("renders repo identity, installs, and stars", () => { diff --git a/frontend/src/features/marketplace/components/MarketplaceDetailView.test.tsx b/frontend/src/features/marketplace/components/MarketplaceDetailView.test.tsx index a232b64..a75dd7b 100644 --- a/frontend/src/features/marketplace/components/MarketplaceDetailView.test.tsx +++ b/frontend/src/features/marketplace/components/MarketplaceDetailView.test.tsx @@ -2,6 +2,7 @@ import { render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import { useMarketplaceDetailQuery, useMarketplaceDocumentQuery } from "../api/queries"; +import { createMarketplaceDetail } from "../test-fixtures"; import { MarketplaceDetailView } from "./MarketplaceDetailView"; vi.mock("../api/queries", () => ({ @@ -15,26 +16,9 @@ const useMarketplaceDocumentQueryMock = vi.mocked(useMarketplaceDocumentQuery); describe("MarketplaceDetailView", () => { it("shows the backend refresh error message when detail loading fails", () => { useMarketplaceDetailQueryMock.mockReturnValue({ - data: { - id: "skillssh:mode-io/skills:mode-switch", - name: "Mode Switch", + data: createMarketplaceDetail({ description: "Mode Switch description", - installs: 128, - stars: 512, - repoLabel: "mode-io/skills", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", - sourceLinks: { - repoLabel: "mode-io/skills", - repoUrl: "https://github.com/mode-io/skills", - folderUrl: null, - skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", - }, - installation: { - status: "installable", - installedSkillRef: null, - }, - installToken: "token-mode-switch", - }, + }), isPending: false, isFetching: false, error: new Error("Marketplace is temporarily unavailable. Check your network connection or reinstall skill-manager if the problem persists."), @@ -66,26 +50,14 @@ describe("MarketplaceDetailView", () => { it("does not render a refresh spinner for background detail refetches", () => { useMarketplaceDetailQueryMock.mockReturnValue({ - data: { - id: "skillssh:mode-io/skills:mode-switch", - name: "Mode Switch", - description: "Switch between supported skill execution modes.", - installs: 128, - stars: 512, - repoLabel: "mode-io/skills", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", + data: createMarketplaceDetail({ sourceLinks: { repoLabel: "mode-io/skills", repoUrl: "https://github.com/mode-io/skills", folderUrl: "https://github.com/mode-io/skills/tree/main/skills/mode-switch", skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", }, - installation: { - status: "installable", - installedSkillRef: null, - }, - installToken: "token-mode-switch", - }, + }), isPending: false, isFetching: true, error: null, diff --git a/frontend/src/features/marketplace/screens/MarketplacePage.loading.test.tsx b/frontend/src/features/marketplace/screens/MarketplacePage.loading.test.tsx index 3d8a017..6f78aa9 100644 --- a/frontend/src/features/marketplace/screens/MarketplacePage.loading.test.tsx +++ b/frontend/src/features/marketplace/screens/MarketplacePage.loading.test.tsx @@ -3,6 +3,7 @@ import { MemoryRouter } from "react-router-dom"; import { describe, expect, it, vi } from "vitest"; import { useMarketplaceController } from "../model/use-marketplace-controller"; +import { createMarketplaceItem } from "../test-fixtures"; import MarketplacePage from "./MarketplacePage"; vi.mock("../model/use-marketplace-controller", () => ({ @@ -19,24 +20,7 @@ describe("MarketplacePage loading ownership", () => { errorMessage: "", selectedItemId: null, selectedItem: null, - items: [ - { - id: "skillssh:mode-io/skills:mode-switch", - name: "Mode Switch", - description: "Switch between supported skill execution modes.", - installs: 128, - stars: 512, - repoLabel: "mode-io/skills", - repoUrl: "https://github.com/mode-io/skills", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", - skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", - installToken: "token-mode-switch", - installation: { - status: "installable", - installedSkillRef: null, - }, - }, - ], + items: [createMarketplaceItem()], feedQuery: { isFetching: true, fetchNextPage: vi.fn(async () => undefined), diff --git a/frontend/src/features/marketplace/screens/MarketplacePage.test.tsx b/frontend/src/features/marketplace/screens/MarketplacePage.test.tsx index 1d3fa3a..9c2486b 100644 --- a/frontend/src/features/marketplace/screens/MarketplacePage.test.tsx +++ b/frontend/src/features/marketplace/screens/MarketplacePage.test.tsx @@ -3,6 +3,7 @@ import { act, fireEvent, render, screen, waitFor, within } from "@testing-librar import { MemoryRouter } from "react-router-dom"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createMarketplaceDetail, createMarketplaceItem } from "../test-fixtures"; import MarketplacePage from "./MarketplacePage"; const fetchMock = vi.fn(); @@ -160,26 +161,16 @@ describe("MarketplacePage", () => { }); } if (url.includes("/api/marketplace/items/skillssh%3Amode-io%2Fskills%3Amode-switch")) { - return okJson({ - id: "skillssh:mode-io/skills:mode-switch", - name: "Mode Switch", - description: "Switch between supported skill execution modes.", - installs: 128, - stars: 512, - repoLabel: "mode-io/skills", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", - sourceLinks: { - repoLabel: "mode-io/skills", - repoUrl: "https://github.com/mode-io/skills", - folderUrl: "https://github.com/mode-io/skills/tree/main/skills/mode-switch", - skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", - }, - installation: { - status: "installable", - installedSkillRef: null, - }, - installToken: "token-mode-switch", - }); + return okJson( + createMarketplaceDetail({ + sourceLinks: { + repoLabel: "mode-io/skills", + repoUrl: "https://github.com/mode-io/skills", + folderUrl: "https://github.com/mode-io/skills/tree/main/skills/mode-switch", + skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", + }, + }), + ); } throw new Error(`Unhandled URL ${url}`); }); @@ -231,26 +222,16 @@ describe("MarketplacePage", () => { await act(async () => { pendingDetail.resolve( - okJson({ - id: "skillssh:mode-io/skills:mode-switch", - name: "Mode Switch", - description: "Switch between supported skill execution modes.", - installs: 128, - stars: 512, - repoLabel: "mode-io/skills", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", - sourceLinks: { - repoLabel: "mode-io/skills", - repoUrl: "https://github.com/mode-io/skills", - folderUrl: "https://github.com/mode-io/skills/tree/main/skills/mode-switch", - skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", - }, - installation: { - status: "installable", - installedSkillRef: null, - }, - installToken: "token-mode-switch", - }), + okJson( + createMarketplaceDetail({ + sourceLinks: { + repoLabel: "mode-io/skills", + repoUrl: "https://github.com/mode-io/skills", + folderUrl: "https://github.com/mode-io/skills/tree/main/skills/mode-switch", + skillsDetailUrl: "https://skills.sh/mode-io/skills/mode-switch", + }, + }), + ), ); }); @@ -299,20 +280,11 @@ function deferred() { } function baseItem(id: string, name: string, installs: number) { - return { + return createMarketplaceItem({ id: `skillssh:mode-io/skills:${id}`, name, description: `${name} description`, installs, - stars: 512, - repoLabel: "mode-io/skills", - repoUrl: "https://github.com/mode-io/skills", - repoImageUrl: "https://avatars.githubusercontent.com/u/424242?v=4", - skillsDetailUrl: `https://skills.sh/mode-io/skills/${id}`, installToken: `token-${id}`, - installation: { - status: "installable", - installedSkillRef: null, - }, - }; + }); } diff --git a/frontend/src/features/marketplace/test-fixtures.ts b/frontend/src/features/marketplace/test-fixtures.ts new file mode 100644 index 0000000..28cc580 --- /dev/null +++ b/frontend/src/features/marketplace/test-fixtures.ts @@ -0,0 +1,59 @@ +import type { MarketplaceDetailDto, MarketplaceItemDto } from "./api/types"; + +function repoOwner(repoLabel: string): string { + return repoLabel.split("/", 1)[0] || repoLabel; +} + +function skillIdFromItemId(itemId: string): string { + const [, skillId = "mode-switch"] = itemId.match(/^[^:]+:[^:]+:(.+)$/) ?? []; + return skillId; +} + +export function marketplaceRepoImageUrl(repoLabel: string): string { + return `https://github.com/${repoOwner(repoLabel)}.png?size=96`; +} + +export function createMarketplaceItem(overrides: Partial = {}): MarketplaceItemDto { + const repoLabel = overrides.repoLabel ?? "mode-io/skills"; + const id = overrides.id ?? `skillssh:${repoLabel}:mode-switch`; + const skillId = skillIdFromItemId(id); + + return { + id, + name: overrides.name ?? "Mode Switch", + description: overrides.description ?? "Switch between supported skill execution modes.", + installs: overrides.installs ?? 128, + stars: overrides.stars ?? 512, + repoLabel, + repoUrl: overrides.repoUrl ?? `https://github.com/${repoLabel}`, + repoImageUrl: overrides.repoImageUrl ?? marketplaceRepoImageUrl(repoLabel), + skillsDetailUrl: overrides.skillsDetailUrl ?? `https://skills.sh/${repoLabel}/${skillId}`, + installToken: overrides.installToken ?? `token-${skillId}`, + installation: overrides.installation ?? { + status: "installable", + installedSkillRef: null, + }, + }; +} + +export function createMarketplaceDetail(overrides: Partial = {}): MarketplaceDetailDto { + const item = createMarketplaceItem(overrides); + + return { + id: item.id, + name: item.name, + description: item.description, + installs: item.installs, + stars: item.stars, + repoLabel: item.repoLabel, + repoImageUrl: item.repoImageUrl, + sourceLinks: overrides.sourceLinks ?? { + repoLabel: item.repoLabel, + repoUrl: item.repoUrl, + folderUrl: null, + skillsDetailUrl: item.skillsDetailUrl, + }, + installation: overrides.installation ?? item.installation, + installToken: overrides.installToken ?? item.installToken, + }; +} diff --git a/frontend/src/features/skills/screens/ManagedSkillsPage.tsx b/frontend/src/features/skills/screens/ManagedSkillsPage.tsx index 435eca6..f74e133 100644 --- a/frontend/src/features/skills/screens/ManagedSkillsPage.tsx +++ b/frontend/src/features/skills/screens/ManagedSkillsPage.tsx @@ -75,7 +75,7 @@ export default function ManagedSkillsPage() {

No managed skills yet

Your shared inventory is empty.

-

Review detected local skills or install something from the marketplace to start managing coverage here.

+

Review unmanaged skills found in supported global roots or install something from the marketplace to start managing coverage here.

diff --git a/skill_manager/application/container.py b/skill_manager/application/container.py index a0fc601..0385f89 100644 --- a/skill_manager/application/container.py +++ b/skill_manager/application/container.py @@ -3,6 +3,7 @@ from dataclasses import dataclass import os +from skill_manager.storage_paths import default_harness_support_path from .marketplace import ( MarketplaceCatalog, MarketplaceDocumentService, @@ -13,7 +14,7 @@ from .settings import SettingsMutationService, SettingsQueryService from .skills import SkillsMutationService, SkillsQueryService from .source_fetch_service import SourceFetchService -from skill_manager.store import HarnessSupportStore, default_harness_support_path +from skill_manager.store import HarnessSupportStore @dataclass(frozen=True) diff --git a/skill_manager/application/marketplace/cache.py b/skill_manager/application/marketplace/cache.py index 1870490..8e84a95 100644 --- a/skill_manager/application/marketplace/cache.py +++ b/skill_manager/application/marketplace/cache.py @@ -6,12 +6,7 @@ import time from pathlib import Path - -def default_marketplace_cache_root(env: dict[str, str] | None = None) -> Path: - active_env = env or {} - home = Path(active_env.get("HOME", str(Path.home()))) - xdg_data_home = Path(active_env.get("XDG_DATA_HOME", home / ".local" / "share")) - return xdg_data_home / "skill-manager" / "marketplace" +from skill_manager.storage_paths import canonical_marketplace_cache_root @dataclass(frozen=True) @@ -37,7 +32,7 @@ def __init__(self, root: Path | None = None) -> None: @classmethod def from_environment(cls, env: dict[str, str] | None = None) -> "MarketplaceCache": - return cls(default_marketplace_cache_root(env)) + return cls(canonical_marketplace_cache_root(env)) def read(self, namespace: str, key: str, *, ttl_seconds: int) -> CachedPayload | None: stored = self.load(namespace, key) diff --git a/skill_manager/application/marketplace/catalog.py b/skill_manager/application/marketplace/catalog.py index a7d44ce..5dc7233 100644 --- a/skill_manager/application/marketplace/catalog.py +++ b/skill_manager/application/marketplace/catalog.py @@ -43,6 +43,7 @@ class MarketplaceCatalog: DETAIL_MISSING_FALLBACK = "No summary available on skills.sh." _LEADERBOARD_TTL_SECONDS = 3600 _DETAIL_TTL_SECONDS = 86400 + _DETAIL_NAMESPACE = "details-v2" _SEARCH_TTL_SECONDS = 900 _SEARCH_FETCH_FLOOR = 40 _SEARCH_CACHE_LIMIT = 24 @@ -172,10 +173,10 @@ def detail_enrichment(self, record: SkillsShSkill) -> DetailEnrichment: detail = DetailEnrichment( description=summary.description, - github_folder_url=folder_url, + folder_url=folder_url, folder_resolution_complete=True, ) - self._cache.write("details", record.detail_url, detail.to_dict()) + self._cache.write(self._DETAIL_NAMESPACE, record.detail_url, detail.to_dict()) return detail @staticmethod @@ -283,14 +284,14 @@ def _description_for_card(self, record: SkillsShSkill, *, prefer_description_hin return detail.description def _cached_detail(self, record: SkillsShSkill) -> DetailEnrichment | None: - detail_cache = self._cache.read("details", record.detail_url, ttl_seconds=self._DETAIL_TTL_SECONDS) + detail_cache = self._cache.read(self._DETAIL_NAMESPACE, record.detail_url, ttl_seconds=self._DETAIL_TTL_SECONDS) if detail_cache is None or not isinstance(detail_cache.payload, dict): return None return DetailEnrichment.from_dict(detail_cache.payload) @staticmethod def _needs_folder_refresh(detail: DetailEnrichment) -> bool: - return bool(detail.github_folder_url and "/tree/HEAD/" in detail.github_folder_url) + return bool(detail.folder_url and "/tree/HEAD/" in detail.folder_url) def _resolve_summary_enrichment(self, record: SkillsShSkill) -> DetailEnrichment: description = record.description_hint.strip() if self._is_usable_description(record.description_hint) else "" @@ -306,10 +307,10 @@ def _resolve_summary_enrichment(self, record: SkillsShSkill) -> DetailEnrichment description = self.DETAIL_MISSING_FALLBACK detail = DetailEnrichment( description=description, - github_folder_url=None, + folder_url=None, folder_resolution_complete=False, ) - self._cache.write("details", record.detail_url, detail.to_dict()) + self._cache.write(self._DETAIL_NAMESPACE, record.detail_url, detail.to_dict()) return detail def _resolve_summary_enrichment_best_effort(self, record: SkillsShSkill) -> DetailEnrichment: @@ -318,10 +319,10 @@ def _resolve_summary_enrichment_best_effort(self, record: SkillsShSkill) -> Deta except MarketplaceUpstreamError: detail = DetailEnrichment( description=self._fallback_description(record), - github_folder_url=None, + folder_url=None, folder_resolution_complete=False, ) - self._cache.write("details", record.detail_url, detail.to_dict()) + self._cache.write(self._DETAIL_NAMESPACE, record.detail_url, detail.to_dict()) return detail def _warm_leaderboard_async(self) -> None: diff --git a/skill_manager/application/marketplace/queries.py b/skill_manager/application/marketplace/queries.py index 1fbb06e..6e08188 100644 --- a/skill_manager/application/marketplace/queries.py +++ b/skill_manager/application/marketplace/queries.py @@ -45,7 +45,7 @@ def get_item_detail(self, item_id: str) -> dict[str, object] | None: "sourceLinks": { "repoLabel": record.repo, "repoUrl": github_repo_url(record.repo), - "folderUrl": enrichment.github_folder_url, + "folderUrl": enrichment.folder_url, "skillsDetailUrl": record.detail_url, }, "installation": installation, diff --git a/skill_manager/application/marketplace/repo_snapshots.py b/skill_manager/application/marketplace/repo_snapshots.py index f29dc37..51d693a 100644 --- a/skill_manager/application/marketplace/repo_snapshots.py +++ b/skill_manager/application/marketplace/repo_snapshots.py @@ -19,7 +19,6 @@ @dataclass(frozen=True) class GitHubRepoSnapshot: repo: str - repo_url: str stars: int | None default_branch: str | None @@ -27,7 +26,6 @@ def to_dict(self) -> dict[str, object]: return { "status": "success", "repo": self.repo, - "repoUrl": self.repo_url, "stars": self.stars, "defaultBranch": self.default_branch, } @@ -37,16 +35,12 @@ def from_dict(cls, payload: dict[str, object]) -> "GitHubRepoSnapshot | None": if payload.get("status") != "success": return None repo = payload.get("repo") - repo_url = payload.get("repoUrl") stars = payload.get("stars") default_branch = payload.get("defaultBranch") if not isinstance(repo, str) or not repo: return None - if not isinstance(repo_url, str) or not repo_url: - return None return cls( repo=repo, - repo_url=repo_url, stars=stars if isinstance(stars, int) else None, default_branch=default_branch if isinstance(default_branch, str) and default_branch else None, ) @@ -81,8 +75,8 @@ class GitHubRepoSnapshotService: _TRANSIENT_FAILURE_SECONDS = 15 * 60 _PERMANENT_FAILURE_SECONDS = 24 * 60 * 60 _REFRESH_WORKERS = 4 - _SUCCESS_NAMESPACE = "repo-metadata" - _FAILURE_NAMESPACE = "repo-metadata-failures" + _SUCCESS_NAMESPACE = "repo-metadata-v2" + _FAILURE_NAMESPACE = "repo-metadata-failures-v2" def __init__( self, @@ -185,11 +179,9 @@ def _refresh_repo(self, repo: str) -> None: @staticmethod def _snapshot_from_metadata(metadata: GitHubRepoMetadata) -> GitHubRepoSnapshot: repo = metadata.repo or "" - repo_url = metadata.repo_url or "" stars = metadata.stars if metadata.stars > 0 else None return GitHubRepoSnapshot( repo=repo, - repo_url=repo_url, stars=stars, default_branch=metadata.default_branch, ) diff --git a/skill_manager/application/marketplace/resolver.py b/skill_manager/application/marketplace/resolver.py index 2af17c4..d1834a4 100644 --- a/skill_manager/application/marketplace/resolver.py +++ b/skill_manager/application/marketplace/resolver.py @@ -15,29 +15,25 @@ @dataclass(frozen=True) class DetailEnrichment: description: str - github_folder_url: str | None + folder_url: str | None folder_resolution_complete: bool = False def to_dict(self) -> dict[str, object]: return { "description": self.description, - "githubFolderUrl": self.github_folder_url, + "folderUrl": self.folder_url, "folderResolutionComplete": self.folder_resolution_complete, } @classmethod def from_dict(cls, payload: dict[str, object]) -> "DetailEnrichment": description = payload.get("description") - github_folder_url = payload.get("githubFolderUrl") + folder_url = payload.get("folderUrl") folder_resolution_complete = payload.get("folderResolutionComplete") return cls( description=description if isinstance(description, str) else "", - github_folder_url=github_folder_url if isinstance(github_folder_url, str) and github_folder_url else None, - folder_resolution_complete=( - folder_resolution_complete - if isinstance(folder_resolution_complete, bool) - else bool(github_folder_url) - ), + folder_url=folder_url if isinstance(folder_url, str) and folder_url else None, + folder_resolution_complete=folder_resolution_complete if isinstance(folder_resolution_complete, bool) else False, ) diff --git a/skill_manager/application/read_model_service.py b/skill_manager/application/read_model_service.py index 9a4f201..e16ccf7 100644 --- a/skill_manager/application/read_model_service.py +++ b/skill_manager/application/read_model_service.py @@ -7,7 +7,8 @@ from skill_manager.domain import HarnessScan, StoreScan from skill_manager.errors import MutationError from skill_manager.harness import HarnessDriver, HarnessManager, HarnessStatus, collect_harness_statuses, create_default_drivers, scan_all_harnesses, supported_harness_ids -from skill_manager.store import HarnessSupportStore, SharedStore, default_harness_support_path, default_shared_store_root +from skill_manager.storage_paths import default_harness_support_path, resolve_shared_store_root +from skill_manager.store import HarnessSupportStore, SharedStore @dataclass(frozen=True) @@ -46,7 +47,7 @@ def from_environment( support_store: HarnessSupportStore | None = None, ) -> "ReadModelService": active_env = env or {} - store = SharedStore(default_shared_store_root(active_env)) + store = SharedStore(resolve_shared_store_root(active_env)) active_support_store = support_store or HarnessSupportStore(default_harness_support_path(active_env)) drivers = create_default_drivers(active_env) return cls(store=store, harness_drivers=drivers, support_store=active_support_store) diff --git a/skill_manager/harness/resolution.py b/skill_manager/harness/resolution.py index fbea86a..31073f5 100644 --- a/skill_manager/harness/resolution.py +++ b/skill_manager/harness/resolution.py @@ -9,7 +9,6 @@ class ResolutionContext: env: dict[str, str] home: Path xdg_config_home: Path - xdg_data_home: Path def resolve_context(env: dict[str, str] | None = None) -> ResolutionContext: active_env = dict(env or {}) @@ -18,5 +17,4 @@ def resolve_context(env: dict[str, str] | None = None) -> ResolutionContext: env=active_env, home=home, xdg_config_home=Path(active_env.get("XDG_CONFIG_HOME", home / ".config")), - xdg_data_home=Path(active_env.get("XDG_DATA_HOME", home / ".local" / "share")), ) diff --git a/skill_manager/sources/github.py b/skill_manager/sources/github.py index b2a163a..b375b52 100644 --- a/skill_manager/sources/github.py +++ b/skill_manager/sources/github.py @@ -17,7 +17,6 @@ @dataclass(frozen=True) class GitHubRepoMetadata: repo: str | None - repo_url: str | None stars: int default_branch: str | None = None @@ -119,12 +118,9 @@ def _default_metadata_fetcher(repo: str) -> GitHubRepoMetadata | None: except OSError as error: raise GitHubRepoMetadataError(repo, str(error)) from error - repo_url = payload.get("html_url") stars = payload.get("stargazers_count", 0) default_branch = payload.get("default_branch") - if not isinstance(repo_url, str) or not repo_url: - repo_url = github_repo_url(repo) try: star_count = int(stars) except (TypeError, ValueError): @@ -134,7 +130,6 @@ def _default_metadata_fetcher(repo: str) -> GitHubRepoMetadata | None: return GitHubRepoMetadata( repo=repo, - repo_url=repo_url, stars=star_count, default_branch=default_branch, ) diff --git a/skill_manager/storage_paths.py b/skill_manager/storage_paths.py new file mode 100644 index 0000000..fd99603 --- /dev/null +++ b/skill_manager/storage_paths.py @@ -0,0 +1,62 @@ +from __future__ import annotations + +import os +from pathlib import Path + +from .app_paths import app_config_dir, app_data_dir + + +SETTINGS_PATH_ENV = "SKILL_MANAGER_SETTINGS_PATH" + + +def canonical_shared_store_root(env: dict[str, str] | None = None) -> Path: + active_env = _active_env(env) + return app_data_dir(active_env) / "shared" + + +def legacy_shared_store_root(env: dict[str, str] | None = None) -> Path: + active_env = _active_env(env) + home = Path(active_env.get("HOME", str(Path.home()))) + return home / ".local" / "share" / "skill-manager" / "shared" + + +def resolve_shared_store_root(env: dict[str, str] | None = None) -> Path: + active_env = _active_env(env) + canonical_root = canonical_shared_store_root(active_env) + legacy_root = legacy_shared_store_root(active_env) + if canonical_root == legacy_root: + return canonical_root + if _store_location_initialized(canonical_root): + return canonical_root + if _store_location_initialized(legacy_root): + return legacy_root + return canonical_root + + +def canonical_marketplace_cache_root(env: dict[str, str] | None = None) -> Path: + active_env = _active_env(env) + return app_data_dir(active_env) / "marketplace" + + +def default_harness_support_path(env: dict[str, str] | None = None) -> Path: + active_env = _active_env(env) + override = active_env.get(SETTINGS_PATH_ENV) + if override: + return Path(override) + return app_config_dir(active_env) / "settings.json" + + +def _store_location_initialized(root: Path) -> bool: + manifest_path = root.parent / "manifest.json" + if manifest_path.is_file(): + return True + if not root.is_dir(): + return False + return any(root.iterdir()) + + +def _active_env(env: dict[str, str] | None) -> dict[str, str]: + active_env = dict(os.environ) + if env is not None: + active_env.update(env) + return active_env diff --git a/skill_manager/store/__init__.py b/skill_manager/store/__init__.py index 336afcf..ce0733f 100644 --- a/skill_manager/store/__init__.py +++ b/skill_manager/store/__init__.py @@ -2,10 +2,9 @@ HarnessSupportPreferences, HarnessSupportStore, SETTINGS_PATH_ENV, - default_harness_support_path, ) from .manifest import ManifestEntry, StoreManifest, load_manifest, write_manifest -from .shared_store import SharedStore, default_shared_store_root +from .shared_store import SharedStore __all__ = [ "HarnessSupportPreferences", @@ -14,8 +13,6 @@ "SETTINGS_PATH_ENV", "SharedStore", "StoreManifest", - "default_harness_support_path", - "default_shared_store_root", "load_manifest", "write_manifest", ] diff --git a/skill_manager/store/harness_support.py b/skill_manager/store/harness_support.py index 3ee2140..439607d 100644 --- a/skill_manager/store/harness_support.py +++ b/skill_manager/store/harness_support.py @@ -4,10 +4,7 @@ import json from pathlib import Path -from skill_manager.app_paths import app_config_dir - - -SETTINGS_PATH_ENV = "SKILL_MANAGER_SETTINGS_PATH" +from skill_manager.storage_paths import SETTINGS_PATH_ENV, default_harness_support_path @dataclass(frozen=True) @@ -17,15 +14,6 @@ class HarnessSupportPreferences: def is_enabled(self, harness: str) -> bool: return harness not in self.disabled_harnesses - -def default_harness_support_path(env: dict[str, str] | None = None) -> Path: - active_env = env or {} - override = active_env.get(SETTINGS_PATH_ENV) - if override: - return Path(override) - return app_config_dir(active_env) / "settings.json" - - class HarnessSupportStore: def __init__(self, path: Path) -> None: self.path = path diff --git a/skill_manager/store/shared_store.py b/skill_manager/store/shared_store.py index e29fc60..493a8a2 100644 --- a/skill_manager/store/shared_store.py +++ b/skill_manager/store/shared_store.py @@ -1,10 +1,8 @@ from __future__ import annotations -import os import shutil from pathlib import Path -from skill_manager.app_paths import app_data_dir from skill_manager.domain import ( CheckIssue, SourceDescriptor, @@ -17,22 +15,6 @@ from .manifest import ManifestEntry, StoreManifest, load_manifest, write_manifest - -def default_shared_store_root(env: dict[str, str] | None = None) -> Path: - active_env = _active_env(env) - default_root = app_data_dir(active_env) / "shared" - if active_env.get("XDG_DATA_HOME"): - return default_root - legacy_root = _legacy_shared_store_root(active_env) - if legacy_root == default_root: - return default_root - if _store_location_initialized(default_root): - return default_root - if _store_location_initialized(legacy_root): - return legacy_root - return default_root - - class SharedStore: def __init__(self, root: Path, manifest_path: Path | None = None) -> None: self.root = root @@ -133,24 +115,3 @@ def check_integrity(self) -> tuple[CheckIssue, ...]: ) ) return tuple(issues) - - -def _legacy_shared_store_root(env: dict[str, str]) -> Path: - home = Path(env.get("HOME", str(Path.home()))) - return home / ".local" / "share" / "skill-manager" / "shared" - - -def _store_location_initialized(root: Path) -> bool: - manifest_path = root.parent / "manifest.json" - if manifest_path.is_file(): - return True - if not root.is_dir(): - return False - return any(root.iterdir()) - - -def _active_env(env: dict[str, str] | None) -> dict[str, str]: - active_env = dict(os.environ) - if env is not None: - active_env.update(env) - return active_env diff --git a/tests/support/marketplace_fixture.py b/tests/support/marketplace_fixture.py index 8b94be8..3fc137b 100644 --- a/tests/support/marketplace_fixture.py +++ b/tests/support/marketplace_fixture.py @@ -29,11 +29,11 @@ def create_fixture_marketplace_service() -> MarketplaceCatalog: cache.write("leaderboard", "all-time", [skill_to_dict(item) for item in skills]) for skill in skills: cache.write( - "details", + "details-v2", skill.detail_url, DetailEnrichment( description=next(item["description"] for item in FIXTURE_SKILLS if item["skillId"] == skill.skill_id), - github_folder_url=FIXTURE_FOLDER_URLS[skill.skill_id], + folder_url=FIXTURE_FOLDER_URLS[skill.skill_id], folder_resolution_complete=True, ).to_dict(), ) @@ -57,19 +57,16 @@ def _fixture_repo_metadata(repo: str) -> GitHubRepoMetadata | None: metadata = { "mode-io/skills": GitHubRepoMetadata( repo=repo, - repo_url="https://github.com/mode-io/skills", stars=512, default_branch="main", ), "vercel-labs/skills": GitHubRepoMetadata( repo=repo, - repo_url="https://github.com/vercel-labs/skills", stars=314, default_branch="main", ), "microsoft/github-copilot-for-azure": GitHubRepoMetadata( repo=repo, - repo_url="https://github.com/microsoft/github-copilot-for-azure", stars=271, default_branch="main", ), diff --git a/tests/unit/test_github_repo_metadata.py b/tests/unit/test_github_repo_metadata.py index 9bcd321..a00f6ef 100644 --- a/tests/unit/test_github_repo_metadata.py +++ b/tests/unit/test_github_repo_metadata.py @@ -24,7 +24,6 @@ def metadata_fetcher(repo: str) -> GitHubRepoMetadata | None: calls.append(repo) return GitHubRepoMetadata( repo=repo, - repo_url=f"https://github.com/{repo}", stars=512, default_branch="main", ) @@ -128,7 +127,6 @@ def test_stale_success_survives_transient_refresh_failure(self) -> None: metadata_client=GitHubRepoMetadataClient( metadata_fetcher=lambda repo: GitHubRepoMetadata( repo=repo, - repo_url=f"https://github.com/{repo}", stars=512, default_branch="main", ), @@ -144,7 +142,7 @@ def test_stale_success_survives_transient_refresh_failure(self) -> None: ) try: seed.refresh_repo_now("mode-io/skills") - _age_cache_entry(cache, "repo-metadata", "mode-io/skills", seconds_ago=2 * 24 * 60 * 60) + _age_cache_entry(cache, "repo-metadata-v2", "mode-io/skills", seconds_ago=2 * 24 * 60 * 60) service.refresh_repo_now("mode-io/skills") metadata = service.metadata_for_repo("mode-io/skills") finally: @@ -163,7 +161,6 @@ def test_success_cache_is_persistent_across_service_instances(self) -> None: metadata_client=GitHubRepoMetadataClient( metadata_fetcher=lambda repo: GitHubRepoMetadata( repo=repo, - repo_url=f"https://github.com/{repo}", stars=271, default_branch="main", ), @@ -187,6 +184,38 @@ def test_success_cache_is_persistent_across_service_instances(self) -> None: self.assertEqual(metadata.image_url, github_owner_avatar_url("vercel-labs/skills")) self.assertEqual(metadata.stars, 271) + def test_success_snapshot_cache_persists_only_runtime_fields(self) -> None: + with TemporaryDirectory() as temp_dir: + cache = MarketplaceCache(Path(temp_dir)) + service = GitHubRepoSnapshotService( + cache=cache, + metadata_client=GitHubRepoMetadataClient( + metadata_fetcher=lambda repo: GitHubRepoMetadata( + repo=repo, + stars=271, + default_branch="main", + ), + ), + ) + try: + service.refresh_repo_now("vercel-labs/skills") + finally: + service.close() + + stored = cache.load("repo-metadata-v2", "vercel-labs/skills") + + assert stored is not None + assert isinstance(stored.payload, dict) + self.assertEqual( + stored.payload, + { + "status": "success", + "repo": "vercel-labs/skills", + "stars": 271, + "defaultBranch": "main", + }, + ) + def _raise_transient(calls: list[str], repo: str) -> GitHubRepoMetadata | None: calls.append(repo) diff --git a/tests/unit/test_marketplace_service.py b/tests/unit/test_marketplace_service.py index d03d908..36d1497 100644 --- a/tests/unit/test_marketplace_service.py +++ b/tests/unit/test_marketplace_service.py @@ -104,7 +104,6 @@ def searcher(query: str, limit: int) -> list[SkillsShSkill]: github_resolver=_resolver( metadata_fetcher=lambda repo: GitHubRepoMetadata( repo=repo, - repo_url=f"https://github.com/{repo}", stars=42, default_branch="main", ), @@ -133,7 +132,6 @@ def test_popular_page_fetches_real_descriptions_on_cold_cache(self) -> None: github_resolver=_resolver( metadata_fetcher=lambda repo: GitHubRepoMetadata( repo=repo, - repo_url=f"https://github.com/{repo}", stars=42, default_branch="main", ), @@ -145,7 +143,7 @@ def test_popular_page_fetches_real_descriptions_on_cold_cache(self) -> None: payload = service.popular_page()["items"] self.assertEqual(payload[0]["description"], "Switch between supported skill execution modes.") - cached = cache.read("details", record.detail_url, ttl_seconds=3600) + cached = cache.read("details-v2", record.detail_url, ttl_seconds=3600) self.assertIsNotNone(cached) def test_search_page_falls_back_to_detail_when_hint_is_missing(self) -> None: @@ -242,7 +240,7 @@ def test_detail_enrichment_returns_summary_when_folder_resolution_fails(self) -> detail = service.detail_enrichment(record) self.assertEqual(detail.description, "Build React components from Stitch designs.") - self.assertIsNone(detail.github_folder_url) + self.assertIsNone(detail.folder_url) self.assertTrue(detail.folder_resolution_complete) def test_detail_enrichment_falls_back_when_summary_fetch_fails(self) -> None: @@ -267,7 +265,7 @@ def test_detail_enrichment_falls_back_when_summary_fetch_fails(self) -> None: detail = service.detail_enrichment(record) self.assertEqual(detail.description, MarketplaceCatalog.DETAIL_MISSING_FALLBACK) - self.assertIsNone(detail.github_folder_url) + self.assertIsNone(detail.folder_url) self.assertTrue(detail.folder_resolution_complete) def test_detail_enrichment_fetches_and_caches_on_first_access(self) -> None: @@ -291,7 +289,6 @@ def test_detail_enrichment_fetches_and_caches_on_first_access(self) -> None: github_resolver=_resolver( metadata_fetcher=lambda repo: GitHubRepoMetadata( repo=repo, - repo_url=f"https://github.com/{repo}", stars=42, default_branch="main", ), @@ -304,11 +301,8 @@ def test_detail_enrichment_fetches_and_caches_on_first_access(self) -> None: detail = service.detail_enrichment(record) self.assertEqual(detail.description, "Switch between supported skill execution modes.") - self.assertEqual( - detail.github_folder_url, - "https://github.com/mode-io/skills/tree/main/skills/mode-switch", - ) - cached = cache.read("details", record.detail_url, ttl_seconds=3600) + self.assertEqual(detail.folder_url, "https://github.com/mode-io/skills/tree/main/skills/mode-switch") + cached = cache.read("details-v2", record.detail_url, ttl_seconds=3600) self.assertIsNotNone(cached) diff --git a/tests/unit/test_storage_paths.py b/tests/unit/test_storage_paths.py new file mode 100644 index 0000000..b0f07de --- /dev/null +++ b/tests/unit/test_storage_paths.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +from pathlib import Path +from tempfile import TemporaryDirectory +import unittest +from unittest import mock + +from skill_manager.storage_paths import ( + canonical_marketplace_cache_root, + canonical_shared_store_root, + legacy_shared_store_root, + resolve_shared_store_root, +) + +from tests.support.fake_home import seed_skill_package + + +class SharedStorePathResolutionTests(unittest.TestCase): + def test_prefers_legacy_store_when_canonical_location_is_uninitialized(self) -> None: + with TemporaryDirectory() as temp_dir: + home = Path(temp_dir) / "home" + legacy_root = home / ".local" / "share" / "skill-manager" / "shared" + legacy_root.mkdir(parents=True, exist_ok=True) + seed_skill_package(legacy_root, "audit", "Audit") + canonical_data_dir = home / "Library" / "Application Support" / "skill-manager" + + with mock.patch("skill_manager.storage_paths.app_data_dir", return_value=canonical_data_dir): + resolved = resolve_shared_store_root({"HOME": str(home)}) + + self.assertEqual(resolved, legacy_root) + self.assertEqual(canonical_shared_store_root({"HOME": str(home)}), canonical_data_dir / "shared") + self.assertEqual(legacy_shared_store_root({"HOME": str(home)}), legacy_root) + + def test_prefers_initialized_canonical_store_over_legacy_store(self) -> None: + with TemporaryDirectory() as temp_dir: + home = Path(temp_dir) / "home" + legacy_root = home / ".local" / "share" / "skill-manager" / "shared" + legacy_root.mkdir(parents=True, exist_ok=True) + seed_skill_package(legacy_root, "legacy-audit", "Legacy Audit") + canonical_data_dir = home / "Library" / "Application Support" / "skill-manager" + canonical_root = canonical_data_dir / "shared" + canonical_root.mkdir(parents=True, exist_ok=True) + seed_skill_package(canonical_root, "audit", "Audit") + + with mock.patch("skill_manager.storage_paths.app_data_dir", return_value=canonical_data_dir): + resolved = resolve_shared_store_root({"HOME": str(home)}) + + self.assertEqual(resolved, canonical_root) + + +class MarketplaceCachePathResolutionTests(unittest.TestCase): + def test_marketplace_cache_uses_canonical_app_data_root_only(self) -> None: + with TemporaryDirectory() as temp_dir: + home = Path(temp_dir) / "home" + canonical_data_dir = home / "Library" / "Application Support" / "skill-manager" + old_cache_root = home / ".local" / "share" / "skill-manager" / "marketplace" + old_cache_root.mkdir(parents=True, exist_ok=True) + + with mock.patch("skill_manager.storage_paths.app_data_dir", return_value=canonical_data_dir): + resolved = canonical_marketplace_cache_root({"HOME": str(home)}) + + self.assertEqual(resolved, canonical_data_dir / "marketplace") + self.assertNotEqual(resolved, old_cache_root) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_store.py b/tests/unit/test_store.py index c4682da..4c98c0c 100644 --- a/tests/unit/test_store.py +++ b/tests/unit/test_store.py @@ -3,9 +3,8 @@ from pathlib import Path from tempfile import TemporaryDirectory import unittest -from unittest import mock -from skill_manager.store import SharedStore, default_shared_store_root, load_manifest +from skill_manager.store import SharedStore, load_manifest from tests.support.fake_home import create_fake_home_spec, seed_skill_package @@ -134,40 +133,5 @@ def test_delete_refuses_package_missing_from_manifest(self) -> None: self.assertIn("missing from manifest", str(ctx.exception)) self.assertTrue((spec.shared_store_root / "audit").is_dir()) - - -class SharedStorePathResolutionTests(unittest.TestCase): - def test_prefers_legacy_store_when_default_location_is_uninitialized(self) -> None: - with TemporaryDirectory() as temp_dir: - home = Path(temp_dir) / "home" - legacy_root = home / ".local" / "share" / "skill-manager" / "shared" - legacy_root.mkdir(parents=True, exist_ok=True) - seed_skill_package(legacy_root, "audit", "Audit") - default_data_dir = home / "Library" / "Application Support" / "skill-manager" - default_root = default_data_dir / "shared" - - with mock.patch("skill_manager.store.shared_store.app_data_dir", return_value=default_data_dir): - resolved = default_shared_store_root({"HOME": str(home)}) - - self.assertEqual(resolved, legacy_root) - self.assertFalse(default_root.exists()) - - def test_prefers_initialized_default_location_over_legacy_store(self) -> None: - with TemporaryDirectory() as temp_dir: - home = Path(temp_dir) / "home" - legacy_root = home / ".local" / "share" / "skill-manager" / "shared" - legacy_root.mkdir(parents=True, exist_ok=True) - seed_skill_package(legacy_root, "legacy-audit", "Legacy Audit") - default_data_dir = home / "Library" / "Application Support" / "skill-manager" - default_root = default_data_dir / "shared" - default_root.mkdir(parents=True, exist_ok=True) - seed_skill_package(default_root, "audit", "Audit") - - with mock.patch("skill_manager.store.shared_store.app_data_dir", return_value=default_data_dir): - resolved = default_shared_store_root({"HOME": str(home)}) - - self.assertEqual(resolved, default_root) - - if __name__ == "__main__": unittest.main() From e2afbb9fb0f665a0479ce825dd4d4ee1973d5aea Mon Sep 17 00:00:00 2001 From: ModeIO Local Date: Wed, 15 Apr 2026 15:04:12 +0800 Subject: [PATCH 2/3] Tighten README user-facing configuration docs --- README.md | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index b99425a..48ee176 100644 --- a/README.md +++ b/README.md @@ -185,26 +185,17 @@ Default local URLs: ## Configuration -`skill-manager` resolves its own app paths through: +`skill-manager` stores its own app data in standard per-user locations. -- config: `app_config_dir(env)` -- data: `app_data_dir(env)` -- state: `app_state_dir(env)` +On macOS, app-owned files live under `~/Library/Application Support/skill-manager`. -On macOS, those all resolve under `~/Library/Application Support/skill-manager`. - -App-owned storage layout: +Useful paths: - shared managed store: `~/Library/Application Support/skill-manager/shared` - marketplace cache: `~/Library/Application Support/skill-manager/marketplace` -- harness support settings: `~/Library/Application Support/skill-manager/settings.json` - -Compatibility behavior is intentionally narrow: - -- managed shared skills still fall back to an initialized legacy shared store at `~/.local/share/skill-manager/shared` -- marketplace cache does not use a legacy fallback because it is disposable and rehydrates cleanly +- app settings: `~/Library/Application Support/skill-manager/settings.json` -You can still override individual harness roots with environment variables when you need to test or relocate a specific global skill directory. +You can override individual harness roots with environment variables when you need to relocate a supported global skill directory. ### Codex @@ -232,11 +223,11 @@ You can still override individual harness roots with environment variables when - `SKILL_MANAGER_MARKETPLACE_BASE_URL` -These overrides are useful when you need to relocate the canonical global skill roots in a controlled environment. `SKILL_MANAGER_MARKETPLACE_BASE_URL` is an advanced override for deterministic tests and release validation; normal installs should continue to use the production `skills.sh` marketplace. +`SKILL_MANAGER_MARKETPLACE_BASE_URL` is an advanced override for custom or local marketplace endpoints. Normal installs should keep using the default `skills.sh` marketplace. ## Troubleshooting -- If Marketplace requests fail with `Marketplace is temporarily unavailable`, verify your network connection first. Packaged installs use the bundled CA bundle automatically; if failures persist after a reinstall, check whether your environment overrides `SSL_CERT_FILE`. +- If Marketplace requests fail with `Marketplace is temporarily unavailable`, verify your network connection first and reinstall if the problem persists. - If `npm install -g @mode-io/skill-manager` reports that Homebrew already owns `skill-manager`, uninstall the Homebrew formula first. The inverse also applies: uninstall the npm package before switching back to Homebrew. ## Development From fd2bfb3b4688c888ac1a1422aac7d81ee2c34a4e Mon Sep 17 00:00:00 2001 From: ModeIO Local Date: Wed, 15 Apr 2026 15:12:19 +0800 Subject: [PATCH 3/3] Fix storage path test portability --- README.md | 10 ++-------- tests/unit/test_storage_paths.py | 3 ++- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 48ee176..28e61b7 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ Useful paths: - marketplace cache: `~/Library/Application Support/skill-manager/marketplace` - app settings: `~/Library/Application Support/skill-manager/settings.json` -You can override individual harness roots with environment variables when you need to relocate a supported global skill directory. +Most users do not need to change these locations. If you manage skills in a custom environment, you can override individual harness roots with environment variables. ### Codex @@ -219,15 +219,9 @@ You can override individual harness roots with environment variables when you ne - global scope defaults to `~/.openclaw/skills` -### Marketplace - -- `SKILL_MANAGER_MARKETPLACE_BASE_URL` - -`SKILL_MANAGER_MARKETPLACE_BASE_URL` is an advanced override for custom or local marketplace endpoints. Normal installs should keep using the default `skills.sh` marketplace. - ## Troubleshooting -- If Marketplace requests fail with `Marketplace is temporarily unavailable`, verify your network connection first and reinstall if the problem persists. +- If Marketplace requests fail with `Marketplace is temporarily unavailable`, verify your network connection and try reinstalling `skill-manager` if the problem persists. - If `npm install -g @mode-io/skill-manager` reports that Homebrew already owns `skill-manager`, uninstall the Homebrew formula first. The inverse also applies: uninstall the npm package before switching back to Homebrew. ## Development diff --git a/tests/unit/test_storage_paths.py b/tests/unit/test_storage_paths.py index b0f07de..730e4e3 100644 --- a/tests/unit/test_storage_paths.py +++ b/tests/unit/test_storage_paths.py @@ -26,9 +26,10 @@ def test_prefers_legacy_store_when_canonical_location_is_uninitialized(self) -> with mock.patch("skill_manager.storage_paths.app_data_dir", return_value=canonical_data_dir): resolved = resolve_shared_store_root({"HOME": str(home)}) + canonical_root = canonical_shared_store_root({"HOME": str(home)}) self.assertEqual(resolved, legacy_root) - self.assertEqual(canonical_shared_store_root({"HOME": str(home)}), canonical_data_dir / "shared") + self.assertEqual(canonical_root, canonical_data_dir / "shared") self.assertEqual(legacy_shared_store_root({"HOME": str(home)}), legacy_root) def test_prefers_initialized_canonical_store_over_legacy_store(self) -> None: