Skip to content

Update: unify paths, lock state writes, Pydantic + OpenAPI codegen#18

Merged
SI-RUI-ZHANG merged 2 commits intomainfrom
skillmgr-arch-cleanup
Apr 17, 2026
Merged

Update: unify paths, lock state writes, Pydantic + OpenAPI codegen#18
SI-RUI-ZHANG merged 2 commits intomainfrom
skillmgr-arch-cleanup

Conversation

@SI-RUI-ZHANG
Copy link
Copy Markdown
Contributor

原问题

  • 路径解析分散在 app_paths.py + storage_paths.py + runtime/paths.py 三个模块中,存在重复的 XDG / darwin 分支以及永久性的 legacy fallback 热路径。
  • launcher.py 是死代码,没有任何模块导入它。
  • manifest.jsonsettings.json 写入既不是 atomic 也未上锁,两个并发 CLI 调用可能在交错的 read-modify-write 中破坏 JSON 状态。
  • API 全部使用 Body(dict) 而非 Pydantic 模型,OpenAPI 模式无法体现请求结构,前端 frontend/src/features/*/api/types.ts 只能手抄并随后端漂移。

本次修复

  • 引入唯一的 skill_manager/paths.py:frozen AppPaths dataclass + resolve_app_paths() 工厂;删除 app_paths.pystorage_paths.pyruntime/paths.pylauncher.py;放弃 ~/.local/share/skill-manager legacy fallback。
  • 新增 skill_manager/store/_atomic.pyatomic_write_text()(写入 sibling temp + os.replace)和 file_lock()fcntl.LOCK_EX)。SharedStore.ingest/update/deleteHarnessSupportStore.set_enabledwrite_manifest 全部用上。
  • 新增 skill_manager/api/schemas.pyEnableSkillRequestDisableSkillRequestInstallMarketplaceSkillRequestSetHarnessSupportRequest,所有 POST/PUT 路由切到 Pydantic 校验。RequestValidationError handler 输出 <field>: <msg>,对外仍是 {"error": ...} 契约。
  • 新增 scripts/dump_openapi.py + openapi-typescriptnpm run codegen:openapi 生成 frontend/src/api/openapi.jsonfrontend/src/api/generated.tsnpm run codegen:check 在 CI 中 diff 漂移。前端 features/*/api/types.ts 重新导出请求类型,client.ts 拿来给请求体上类型。
  • frontend/src/api/http.ts 兼容 Pydantic 的 detail(字符串或数组)作为 fallback。
  • CI frontend-validate 任务新增 "OpenAPI codegen drift check" 步骤。
  • 新增测试:tests/unit/test_paths.pytests/unit/test_atomic.pytests/unit/test_shared_store_concurrent.pytests/integration/test_request_validation.py;同步更新 tests/integration/test_marketplace_api.py

升级提示:若曾在 ~/.local/share/skill-manager/ 路径下初始化过 skill-manager,请在升级前手动迁移到 ~/Library/Application Support/skill-manager/

改动文件

.github/workflows/ci.yml
frontend/src/api/generated.ts (new)
frontend/src/api/http.ts
frontend/src/api/openapi.json (new)
frontend/src/features/marketplace/api/client.ts
frontend/src/features/marketplace/api/types.ts
frontend/src/features/settings/api/client.ts
frontend/src/features/settings/api/types.ts
frontend/src/features/skills/api/client.ts
frontend/src/features/skills/api/types.ts
package-lock.json
package.json
scripts/dump_openapi.py (new)
skill_manager/api/errors.py
skill_manager/api/routers/marketplace.py
skill_manager/api/routers/settings.py
skill_manager/api/routers/skills.py
skill_manager/api/schemas.py (new)
skill_manager/app_paths.py (deleted)
skill_manager/application/container.py
skill_manager/application/marketplace/cache.py
skill_manager/application/read_model_service.py
skill_manager/cli/main.py
skill_manager/launcher.py (deleted)
skill_manager/paths.py (new)
skill_manager/runtime/paths.py (deleted)
skill_manager/runtime/state.py
skill_manager/storage_paths.py (deleted)
skill_manager/store/init.py
skill_manager/store/_atomic.py (new)
skill_manager/store/harness_support.py
skill_manager/store/manifest.py
skill_manager/store/shared_store.py
tests/integration/test_marketplace_api.py
tests/integration/test_request_validation.py (new)
tests/unit/test_atomic.py (new)
tests/unit/test_paths.py (new)
tests/unit/test_shared_store_concurrent.py (new)
tests/unit/test_storage_paths.py (deleted)

Reviewer: @SI-RUI-ZHANG

Foundation refactor in preparation for opensource contribution:

- Collapse skill_manager.app_paths + storage_paths + runtime/paths into
  one skill_manager/paths.py with a frozen AppPaths dataclass and a single
  resolve_app_paths() factory. Drop the legacy ~/.local/share fallback.
- Remove dead skill_manager/launcher.py shim (nothing imported it).
- Add skill_manager/store/_atomic.py with atomic_write_text() and
  file_lock() helpers. Wrap manifest.json + settings.json writes in
  atomic temp-then-rename. Wrap SharedStore.ingest/update/delete and
  HarnessSupportStore.set_enabled in fcntl exclusive locks so concurrent
  CLI invocations cannot corrupt JSON state.
- Add Pydantic request body models for all POST/PUT routes
  (skill_manager/api/schemas.py). Replace ad-hoc Body(dict) +
  manual HTTPException(400) checks. RequestValidationError handler now
  emits "<field>: <msg>" so the existing {"error": ...} contract stays
  stable.
- Add scripts/dump_openapi.py + openapi-typescript codegen wired into
  package.json (codegen:openapi, codegen:check). Frontend feature
  api/types.ts re-export request schemas from frontend/src/api/generated.ts.
  client.ts uses them so request body shapes are pinned to the backend.
- Add CI step "OpenAPI codegen drift check" in frontend-validate so
  contributors must regenerate after API surface changes.
- frontend/src/api/http.ts surfaces Pydantic detail array for resilience
  if the backend handler is ever bypassed.
- Tests: tests/unit/test_paths.py (XDG/darwin/env-overrides),
  tests/unit/test_atomic.py (atomic write + lock serialization),
  tests/unit/test_shared_store_concurrent.py (threaded ingest race),
  tests/integration/test_request_validation.py (Pydantic 422 surfaces).
CI runners ship XDG_CONFIG_HOME=/home/runner/.config which leaked through resolve_app_paths' implicit os.environ merge and broke test assertions. New isolated_env helper clears XDG/HOME/SKILL_MANAGER_* before each test.
@SI-RUI-ZHANG SI-RUI-ZHANG merged commit 4476930 into main Apr 17, 2026
5 checks passed
@SI-RUI-ZHANG SI-RUI-ZHANG deleted the skillmgr-arch-cleanup branch April 17, 2026 08:58
SI-RUI-ZHANG added a commit that referenced this pull request Apr 19, 2026
* Update: unify paths, lock state writes, Pydantic + OpenAPI codegen

Foundation refactor in preparation for opensource contribution:

- Collapse skill_manager.app_paths + storage_paths + runtime/paths into
  one skill_manager/paths.py with a frozen AppPaths dataclass and a single
  resolve_app_paths() factory. Drop the legacy ~/.local/share fallback.
- Remove dead skill_manager/launcher.py shim (nothing imported it).
- Add skill_manager/store/_atomic.py with atomic_write_text() and
  file_lock() helpers. Wrap manifest.json + settings.json writes in
  atomic temp-then-rename. Wrap SharedStore.ingest/update/delete and
  HarnessSupportStore.set_enabled in fcntl exclusive locks so concurrent
  CLI invocations cannot corrupt JSON state.
- Add Pydantic request body models for all POST/PUT routes
  (skill_manager/api/schemas.py). Replace ad-hoc Body(dict) +
  manual HTTPException(400) checks. RequestValidationError handler now
  emits "<field>: <msg>" so the existing {"error": ...} contract stays
  stable.
- Add scripts/dump_openapi.py + openapi-typescript codegen wired into
  package.json (codegen:openapi, codegen:check). Frontend feature
  api/types.ts re-export request schemas from frontend/src/api/generated.ts.
  client.ts uses them so request body shapes are pinned to the backend.
- Add CI step "OpenAPI codegen drift check" in frontend-validate so
  contributors must regenerate after API surface changes.
- frontend/src/api/http.ts surfaces Pydantic detail array for resilience
  if the backend handler is ever bypassed.
- Tests: tests/unit/test_paths.py (XDG/darwin/env-overrides),
  tests/unit/test_atomic.py (atomic write + lock serialization),
  tests/unit/test_shared_store_concurrent.py (threaded ingest race),
  tests/integration/test_request_validation.py (Pydantic 422 surfaces).

* Update: scrub inherited XDG env vars in test_paths

CI runners ship XDG_CONFIG_HOME=/home/runner/.config which leaked through resolve_app_paths' implicit os.environ merge and broke test assertions. New isolated_env helper clears XDG/HOME/SKILL_MANAGER_* before each test.
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