Repository navigation
refactor(skills): read the managed slash-command marker from its writer - #7683
huangruiteng merged 1 commit into
Conversation
The writer of a managed slash-command facade and the readback that decides whether a file is LoopX-managed each stated the same marker literal under their own name, so a version bump had two places to land and a drift between them would silently treat a managed file as user-owned. Point the readback at the writer's constant and census the literal, including the shipped host assets that state it in their own runtime. Signed-off-by: lll-hhh <169072815+lll-hhh@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=max
动机 / Motivation
安装或升级 LoopX 工作流技能、同时保留自己技能文件的用户,会经过这条清理路径。旧代码在生成端与读回端分别写入同一标记;升级标记时若漏改读回端,生成副本会被当作未托管文件而留在旧目录。现在读回端直接复用生成端的常量,当前清理行为保持一致。可验证交付是消除标记定义漂移,并在真实安装命令中保留用户文件、附件和不可安全替换的副本。本 PR 只统一既有托管标记来源;不升级标记、改变技能正文、扩大删除权限或宣称完成所有安装宿主资格。
未找到针对这次内部常量去重的独立书面规范(no_spec);维护判断依据是现有 writer/readback 的真实保护行为。
改动思路 / Approach
生成端 slash_command_files 已经拥有标记,读回端导入其轻量常量即可;保留兼容别名比增加配置、迁移技能或建立第二个标记注册表更合适。本 PR 只统一既有托管标记来源;不升级标记、改变技能正文、扩大删除权限或宣称完成所有安装宿主资格。正向路径是 workflow-skills --install 退役已托管的重复技能,保留完整 reconciliation 和其他文件内容;再次用正常 reader 可完成同一夹具清理。 反向路径是 用户技能、附件、未托管 metadata、symlink 和无替代副本保持原字节;故意把 reader 标记改错会让独立退役断言失败,恢复后通过。 这两条真实 caller 路径用于判断保持了哪些行为;单个 helper 的相等断言不能替代安装或完整规划结果。
具体改动 / Changes
审阅 head:6deca25a19731e8c605503057f3f2a5af0646e60;不可变比较基线:7cdf887faa68d5527a196147696d8d33ac79d5cf。已逐行读取全 PR,范围是 2 个路径、+63/-1。关键符号如下:
- _managed_command_facade:读回端用同一个生成标记识别 facade。
- retire_duplicate_managed_skills:保留 canonical 替代、附件、symlink 和 manifest/hash 限制,再退役副本。
- managed_marker:生成既有跨宿主文件标记的唯一来源。
398 项相关测试与新增 4 项 owner guard 通过;8 项独立安装探针覆盖托管副本、用户内容、附件、metadata、symlink、无替代副本及错误标记后的恢复。原有 6 项 Windows/PowerShell 条件测试在 macOS 跳过,未称其通过。
对主干的风险 / Risks
主要风险是 共享标记后错误识别用户文件;因此比较完整 reconciliation 与实际文件字节,并把两版源码版本/路径元数据单独核验,未宣称整个安装 manifest 字节相等。既有 marker 子串检测也不构成身份认证,manifest、替代文件和附件保护未被放宽。
精确 scope 的 change-quality 回执有效,Ruff、编译、diff 与 RFC 双语检查通过。当前标准 premerge 仍失败:最终6项直接检查通过、5项选中检查中4项通过;唯一失败是 install-local-smoke.py:84 拒绝 loopx-project/SKILL.md 的 ../loopx-self-repair/references/targeted-diagnostics.md 链接。不可变基线7cdf887同命令在同一行、同一断言和目的地失败,完整规范化末段相等;相关 smoke 与技能文件字节未改。这是本次提案前已存在的问题,仍保留合并 hold。首轮词表120秒超时、基线86.71秒、受控head110.48秒及最终完整复核中词表检查通过的整个记录均保留。 首轮失败和当前复核分开记录;不查询或等待远端 CI。初次无 Git 元数据的 archive 安装探针在 provenance 断言提前失败,不能用来归因当前安装检查;后续基线观察单独保存。
我的整体评价 / Verdict
未发现 diff 引入的可执行阻塞问题,可以批准这一有界重构;当前安装检查仍有单独的合并 hold。生成端 slash_command_files 已经拥有标记,读回端导入其轻量常量即可;保留兼容别名比增加配置、迁移技能或建立第二个标记注册表更合适。本 PR 只统一既有托管标记来源;不升级标记、改变技能正文、扩大删除权限或宣称完成所有安装宿主资格。长程工作与用户操作路径在本次范围内保持:保留原权限、停止、恢复和结果读回,不增加确认轮次;没有把一次成功读回或测试数当成长程模型收益。已检查全部变更及其安装 CLI 调用方,没有渲染页面变化;实际结果通过 reconciliation 与文件存留读回。原生 Windows/PowerShell 和安装态宿主采用未在本次资格化;全树检查的合并阻塞单独保留。
English verdict: APPROVE the bounded refactor; the pre-existing install check remains a separate merge hold.
Goal And Delivered Outcome
Outcome basis / optional anchor: self-contained maintenance outcome (no issue anchor).
Goal/source and gap: the module that writes a managed slash-command facade (
loopx/slash_command_files.py) and the module that reads one back to decide whether a file is LoopX-managed (loopx/skill_install_readback.py) each stated the same marker literal,<!-- loopx-managed-slash-command:v1, under their own name. A marker version bump therefore had two places to land, and drifting only one of them makes the readback treat a LoopX-managed file as user-owned — silently, because both sides still see a well-formed comment.Observable before → after, with the validation row that proves it: before, the literal appeared in two Python modules and no test related them; after, exactly one Python module states it and the census asserts the found set is
{loopx/slash_command_files.py: 1}, while the readback holds the writer's object (is, not equality). The same guard file run against the base revision fails those two tests and passes the other two, which is the failing-before half of theunitrow below.Issue/task and intended base: intended base
mainat7cdf887fa.Author Declaration
Implemented against
loopx/slash_command_files.py(managed_marker()builds the facade header fromMANAGED_MARKER_PREFIX) and the readback check inloopx/skill_install_readback.py, both atmain58205c62a, re-verified against7cdf887fa; the single-owner idiom already recorded intests/control_plane/test_compact_identifier_shape_owner.py(census by literal, empty allow-list).MANAGED_MARKER_PREFIX,loopx/slash_command_files.pypython -m pytest tests/architecture/test_managed_slash_command_marker_owner.pyskill_install_readback.LOOPX_MANAGED_SLASH_COMMAND_MARKER(kept as the module's public name)test_the_readback_imports_the_owner_objectloopx/**/*.pywith no allow-list entriestest_the_marker_body_is_stated_only_by_the_owner_in_pythonmanaged_marker(command=..., surface=...)test_the_writer_and_the_reader_agree_on_every_facade_surfaceruff checkon both changed files, three source mutations against the guard (see themanualrow), and the same guard run against the base revision. Assumed, not verified — that the four shipped host assets keep their first-line marker as a literal: they are JavaScript/TypeScript files the host loads directly and cannot import a Python constant, so they are pinned by name in the test against the owner's value instead of being folded. Deliberately left out — thetests/files that embed the marker string; those are fixtures stating hostile or host-shaped input and read better literal than imported.Scope And Continuation
Validation
6deca25a1(base7cdf887fa). The branch was first built on58205c62aasa3ff8160cand rebased when main advanced;git patch-id --stableis unchanged (b458312e7de95c56f6908e00efd5d2a6a1f93248), and the guard plus its failing-before control were re-run on the new base.unitpassedpython -m pytest tests/architecture/test_managed_slash_command_marker_owner.py— 4 passed. The same file copied onto the base revision fails exactly the two tests that encode this change (census, owner-object identity) and passes the other two, so the guard discriminates rather than restating the diff.integrationpassedskill_install_readback,slash_command_filesorslash_command_install— install, doctor-freshness, skill delivery parity, discovery reconciliation, slash-command install and the host-surface suites for Kiro, Zcode, Ark, Agy and Windows install: 400 passed, 2 failed, 6 skipped.regression_paritypassedstaticpassedruff checkon both changed files, clean. No TypeScript change, so the control-plane typecheck row isnot_applicable.manualpassed__pycache__was cleared between runs — the first restore looked red until it was.tests/capabilities/test_native_codex_profile_skill_versions.pyreproduce identically on the base revision in this local selection and raise from the capability's own host-runtime precondition, so they are environmental and not attributable to this change. The Windows PowerShell CI lane and the browser lanes were not run locally;tests/test_windows_install.pyis in the selection above but on this host it only exercises the POSIX-side install logic, so the native Windows path stays with CI.Frontend / Visual Evidence
Type of Change
LoopX Area
The marker identifies a LoopX-managed slash-command facade on a host CLI, which is what the readback consults to decide whether it may rewrite that file.
Shared-authority RFC fixture impact
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).python3 scripts/check_rfc_bilingual.py --base origin/main --head HEADpasses, or no paired RFC changed.none.Signed-off-bytrailer (git commit -s).