Skip to content

refactor(skills): read the managed slash-command marker from its writer - #7683

Merged
huangruiteng merged 1 commit into
loopx-project:mainfrom
lll-hhh:refactor/managed-marker-single-owner
Oct 30, 2026
Merged

huangruiteng merged 1 commit into
loopx-project:mainfrom
lll-hhh:refactor/managed-marker-single-owner

Conversation

@lll-hhh

@lll-hhh lll-hhh commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

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 the unit row below.

  • Issue/task and intended base: intended base main at 7cdf887fa.

Author Declaration

  • Written by: model_agent (an agent in the Qoder product family).

Implemented against

  • Specification and revision: the writer's own contract in loopx/slash_command_files.py (managed_marker() builds the facade header from MANAGED_MARKER_PREFIX) and the readback check in loopx/skill_install_readback.py, both at main 58205c62a, re-verified against 7cdf887fa; the single-owner idiom already recorded in tests/control_plane/test_compact_identifier_shape_owner.py (census by literal, empty allow-list).
  • Criteria:
Criterion (spec clause) Disposition Symbol / path Test or command
The marker is written from one constant implemented MANAGED_MARKER_PREFIX, loopx/slash_command_files.py python -m pytest tests/architecture/test_managed_slash_command_marker_owner.py
The readback asks the same owner rather than restating it implemented skill_install_readback.LOOPX_MANAGED_SLASH_COMMAND_MARKER (kept as the module's public name) test_the_readback_imports_the_owner_object
A new restatement fails rather than passing review implemented census over loopx/**/*.py with no allow-list entries test_the_marker_body_is_stated_only_by_the_owner_in_python
Writer and reader agree for every facade surface the installer emits implemented managed_marker(command=..., surface=...) test_the_writer_and_the_reader_agree_on_every_facade_surface
  • Self-check before submission: verified — the new guard, the 20 test files that reference these modules, ruff check on both changed files, three source mutations against the guard (see the manual row), 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 — the tests/ files that embed the marker string; those are fixtures stating hostile or host-shaped input and read better literal than imported.

Scope And Continuation

  • Completed scope and remaining work: both Python statement sites are down to one owner, and the marker's non-Python carriers are now checked against it. Remaining: none needed for this defect. If the marker is ever versioned, the asset-header test names the four files that must move in the same change, which is the point of listing them.
  • Slice boundary / successor: complete within this scope; no successor.

Validation

  • Tested revision: 6deca25a1 (base 7cdf887fa). The branch was first built on 58205c62a as a3ff8160c and rebased when main advanced; git patch-id --stable is unchanged (b458312e7de95c56f6908e00efd5d2a6a1f93248), and the guard plus its failing-before control were re-run on the new base.
  • Run state: finished
  • Input classes: public_fixture | synthetic
Check kind Result Public-safe evidence / limitation
unit passed python -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.
integration passed The 20 test files that reference skill_install_readback, slash_command_files or slash_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_parity passed The identical 19-file selection (without the new guard) run on the base revision: 396 passed with the same 2 failures. The failure sets are equal; head adds 4 passing tests and removes nothing, so no net regression.
static passed ruff check on both changed files, clean. No TypeScript change, so the control-plane typecheck row is not_applicable.
manual passed Three mutations against the real guard, each killed: the read side restating the literal verbatim, the read side building an equal-but-distinct object, and renaming the owner's prefix while the shipped assets are left behind. The tree was restored after each and the guard re-ran green. Python caches a same-length restore within one second, so __pycache__ was cleared between runs — the first restore looked red until it was.
  • Coverage and gaps: the changed path is one import plus the readback's two uses, exercised by the install, doctor, delivery-parity and host-surface suites above. The 2 failures in tests/capabilities/test_native_codex_profile_skill_versions.py reproduce 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.py is 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

  • UI impact: none
  • Before: N/A
  • After: N/A
  • First viewport and preview: N/A
  • States and viewports shown: N/A
  • Source data: none
  • Attention review: N/A
  • Craft review: N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Benchmark boundary (adapters, runners, verifiers, scoring, evidence)
  • Capability or extension (providers, adapters, skills)
  • Public docs or presentation surface (README, protocols, dashboard)
  • Build, packaging, installer, or CI
  • Host or runtime integration

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

  • N/A: no promotion, runtime-routing or compatibility-projection change; no fixture schema or provider conformance arm is touched.

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths (including .loopx/, .codex/goals/, and live ACTIVE_GOAL_STATE.md).
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • Existing English/Chinese RFC pairs (including ledger entries) both have a diff, and the local premerge check python3 scripts/check_rfc_bilingual.py --base origin/main --head HEAD passes, or no paired RFC changed.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer (git commit -s).

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 loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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。关键符号如下:

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.

@huangruiteng
huangruiteng merged commit 547766f into loopx-project:main Oct 30, 2026
1 check passed
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.

3 participants