feat: 支持共享 Skill 编辑并收敛安装服务边界 - #1088
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be2e27ac0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| os.close(child_fd) | ||
| elif stat.S_ISREG(mode): | ||
| with open_regular_file_fd(directory_fd, (name,)) as (file_fd, _file_stat): | ||
| with archive.open(path, "w") as output: |
There was a problem hiding this comment.
When a Skill contains an executable script, streaming it through archive.open() creates a ZIP entry with the default Unix mode (typically 0600) rather than the source file's mode. The previous ZipFile.write() path preserved 0755, so users who export and extract such a Skill now receive non-executable scripts; construct each ZipInfo from file_stat or explicitly copy its permission bits into external_attr.
Useful? React with 👍 / 👎.
| temporary = f".{filename}.{uuid.uuid4().hex}.tmp" | ||
| file_fd = os.open(temporary, os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, mode, dir_fd=staging_fd) |
There was a problem hiding this comment.
For an existing valid file whose basename is near the filesystem's 255-byte limit, prefixing and suffixing that basename with a UUID makes temporary exceed NAME_MAX, so os.open() raises ENAMETOOLONG and the online editor returns a 500 even though the target itself is valid. Use a short UUID-only staging name so every file accepted by the owning filesystem remains editable.
Useful? React with 👍 / 👎.
| while pending: | ||
| slug = pending.pop(0) | ||
| if slug in shadowed or slug not in visible or slug in locked: | ||
| continue | ||
| item = await repo.get_by_slug_for_read(slug) |
There was a problem hiding this comment.
During a rolling startup, a run selecting built-in Skills in B, A order can retain a FOR SHARE lock on B here while waiting for A, while init_builtin_skills() retains its FOR UPDATE lock on A and proceeds to update B before its final commit. PostgreSQL then detects a deadlock and aborts either run preparation or the required startup component; derive the closure before locking and coordinate every multi-row reader and initializer through one deterministic row order or common advisory lock, with real PostgreSQL concurrency coverage.
AGENTS.md reference: backend/AGENTS.md:L37-L37
Useful? React with 👍 / 👎.
变更说明
共享 Skill 详情页支持在线编辑文件及运行依赖。保存根级
SKILL.md同步 PostgreSQL 的名称、描述与依赖索引；过期修订值返回 409，页面保留草稿。安装流程区分获取/解析失败与安装失败，部分成功后可以只重试失败项，用户移除的条目不会被重新选中。feature、architecture、simplification。工程主张与 Owner
services/skills/edit.py校验管理权限、内置来源、文件路径和 SHA-256 修订值；共享行锁覆盖读取/写入与根文件索引更新。文件发布后提交数据库，普通异常恢复旧文件。repositories/skill_repository.py拥有可见性查询和行锁；agents/skills/runtime.py与services/skills/projection.py在锁后重查权限，并只读取对应来源。services/skills/personal.py拥有个人 Skill 文件及安装用例；draft.py拥有安装快照、选择和成功条目消费；package.py统一包格式及相对路径校验。路由与工具只调用对应用例。SkillDetailView.vue、AgentFilePreview.vue拥有编辑草稿与冲突交互；SkillInstallFlowModal.vue按实际可复用失败项选择重试路径。验证情况
提交
79b071ba的远端 CI 已全部通过：后端单测、PowerShell 安全契约、Web lint/unit/build、工程契约、Ruff、文档构建，以及 Durable Task / PostgreSQL / readiness / 确定性 Agent 与沙盒链路。PR 环境的 Pages 部署按 workflow 条件跳过。保存、授权与运行时加载
docker compose exec -T api env SANDBOX_RUNTIME_PROFILE=core uv run --no-sync --group test pytest test/unit -m 'not slow' -q -p no:cacheprovider --timeout=60：2454 passed、58 skipped。docker compose exec -T api uv run --no-sync --group test pytest test/integration/api/test_shared_skill_edit_router.py test/integration/api/test_skill_artifact_authorization.py test/integration/services/test_user_skill_projection.py -q -p no:cacheprovider --timeout=120：7 passed，真实 HTTP/PostgreSQL。docker compose exec -T api uv run --no-sync --group test pytest test/e2e/test_shared_skill_edit_e2e.py -q -p no:cacheprovider --timeout=360：1 passed，确定性 replay provider，经实际 API/worker 回读文件投影和 Run manifest。docker compose exec -T api uv run --no-sync --group test pytest test/unit/services/skills/test_skill_service.py::test_unchanged_skill_projection_does_not_create_staging -q -p no:cacheprovider：1 passed。草稿保存与安装失败重试
docker compose exec -T web pnpm run lint:check、docker compose exec -T web pnpm run test:unit（394 passed）、docker compose exec -T web pnpm run build；真实浏览器验证保存/HTTP 回读、取消切页保留草稿、真实 409 后保留编辑内容；重试组件在真实 Vue DOM 配合模拟远程响应中验证部分成功后的重新获取、确认按钮可用与已移除项不重新选中。结构、文档与静态门禁
python3 scripts/verify_engineering_contracts.py、python3 -m unittest scripts.test_verify_engineering_contracts（63 passed）、uvx ruff==0.16.4 check backend/package、uvx ruff==0.16.4 format backend/package --check、uvx ruff==0.16.4 check --select I backend/package、pnpm --dir docs run build、git diff --check。简化 / 删除验收
移除原
agents/skills/service.py、旧 repository/remote 模块路径和重复准备逻辑，实际调用方迁至 service/repository Owner，不提供无 consumer 的转发层。保留共享行锁、文件补偿、个人覆盖和草稿消费语义。旧模块导入在源码、测试和当前文档中的搜索无残留；HTTP、持久数据和部署配置无需迁移。只有出现明确兼容消费者时才重新评估旧 Python 路径兼容。独立语义 Review
全新上下文 Reviewer 审查完整需求、70 文件 diff、规范、验证结果及远端既有修复。发现安装重试死路，补先红后绿回归并修复；投影测试 mock 同步到真实 Owner。复审独立执行 4 项重试测试并核对追加差异，批准提交，无剩余阻塞。对接远端历史后核对 Git tree 与已审查快照完全一致。
未验证范围与风险
uv run --group test因挂载目录不可写而无法构建 editable 包，使用--no-sync验证当前挂载源码。文档原跨工作树 node_modules 链接缺依赖，已在本工作树按 frozen lockfile 安装后构建通过，未更改依赖清单或锁文件。test_model_provider_uid_header.py长行，以及 scripts 文件已有的隐式字符串拼接告警；本 PR 不顺手修改，实际 CI package 门禁通过本地同版本检查。事故反馈
新功能及提交前发现的问题，不涉及已逃逸高影响事故。
界面变更
已完成实际页面交互检查并保存浅色、深色、移动端及 409 冲突截图，位于本地验证产物，未提交到仓库或上传到 PR。
关联事项
无。
补充说明
旧客户端保存文件需先读取修订值；无数据库 Schema 迁移。远端原有三个提交历史通过合并保留，未强推分支；同步 main 的文档重组后重新通过工程契约和文档构建，应用源码不变。