Skip to content

fix(hide-official): mask refused official tools in PTC scopes - #568

Closed
tietoumiao wants to merge 1 commit into
T-Auto:mainfrom
tietoumiao:fix/415-ptc-scope-restrict
Closed

tietoumiao wants to merge 1 commit into
T-Auto:mainfrom
tietoumiao:fix/415-ptc-scope-restrict

Conversation

@tietoumiao

@tietoumiao tietoumiao commented Nov 1, 2026 •

Copy link
Copy Markdown

Closes #415

What was wrong

In PTC presentation the hide-official component computed its deny list from nativeProviders(),
which is empty for a PTC scope because src/publication/providers.ts:30 withholds native
publication there. The result was deny: [], so tools.restrict was never called and the tools
the plugin refuses to serve stayed advertised in tools:sdk (the SDK ToolArgsMap) while every
call to them was refused at dispatch — a surface that disagrees with itself.

Measured live before the change (real host, PTC scope): tools:sdk declared
["read","glob","grep","foreign"] and all three read/glob/grep refused at dispatch.

The change

  • src/official-tool-hiding.ts — derive the mask from the names the scope can actually see
    (names.filter(hideable)), using the public transport visibility (run_code) as the collapse
    signal instead of the native-provider readiness path. A scope that refuses a tool now stops
    advertising it; the Native presentation keeps its readiness-gated mask (a failed plan still hides
    nothing there).
  • test/lib/official-tool-hiding.mjs, test/presentation-readiness.test.mjs — assertions for the
    PTC surface, the Native sibling, and a legal nested sub-call after tools/change.
  • docs/component-reference.md + docs/组件参考.md — the "never masked" claim is replaced by the
    actual rule; docs/CHANGELOG.md entry.

Deliberately not done: no private host API, no change to which tools the plugin hides, and the
Studio/native readiness path is untouched.

Validation

$ node test/run.mjs official-tool-hiding
  ok   read must not be declared in tools:sdk
  ok   a collapsed scope masks the names it refuses
dsh-ops official-tool-hiding: all 33 checks passed

$ node test/run.mjs presentation-readiness
dsh-ops presentation-readiness: all 6 checks passed

$ node test/run.mjs mount
dsh-ops mount: all 51 checks passed

$ node test/run.mjs official-tool-hiding      # after reverting only src/official-tool-hiding.ts
dsh-ops official-tool-hiding: 2 of 33 check(s) failed
  (read must not be declared in tools:sdk @ test/lib/official-tool-hiding.mjs:212
   a collapsed scope masks the names it refuses @ :445)
  • 撤掉本次实现后变红的检查:上面两条 + presentation-readiness 的 4 条
  • docs/CHANGELOG.md ## [Unreleased]:已加(EN+ZH)
  • 面向模型的描述 / 参数:无新增字节;只改变"已拒绝的工具是否仍出现在 tools:sdk 声明里",
    提示词段与工具 schema 均未改变
  • CI:待维护者批准后由 pull_request 事件触发,届时回填 CI required 的 run URL

Environment

  • OS / 架构:Windows 11 x64
  • Node:24.16.0
  • 基于上游 main 的 commit:065cd7f
  • 安装通道:源码 tarball + npm ci --ignore-scripts

未验证 / 风险披露

CI

$ gh run view 41338712169 --json url,event,conclusion --jq '"(.url) (.event) (.conclusion)"'
https://github.com/T-Auto/dsh-ops/actions/runs/41338712169 pull_request action_required

A run_code presentation resolves no native provider for its selected
counterparts, so the plan's readiness check could never pass, `deny`
stayed empty, and `tools.restrict` was never called. The host renders
`tools:sdk` (ToolArgsMap) from the same scope view, so the model was
told to call read/glob/grep inside a program while the guard refused
them, and a later surface change refused the program's legal sub-calls
too, because the failed plan re-set the readiness error.

A scope that presents the reserved transport is now masked on its
refusal contract instead: exactly the selected official names still
visible in that scope are hidden, a program's other SDK sub-calls keep
dispatching, and the native presentation keeps the readiness-gated
mask, where a failed plan still hides nothing. The assembled-surface
check now verifies collapsed scopes too.
@Haoming9527

Copy link
Copy Markdown
Collaborator

The changed tests deliberately enable retained legacy rows through legacyComponent. In the normal single-row bundle, official-tool-hiding.apply returns through retiredLegacyRow, and supported integration selects file replacements only for Native presentation.

The recorded actual Node PTC check executes visible read/glob/grep/foreign SDK callbacks after tools/change in that supported composition. This patch instead changes the retained legacy opt-in contract, including PR #366's PTC masking policy; it is not missing publication behavior from #542.

Please clarify whether the intended target is an explicit legacy/backport surface. Leaving the PR open for the maintainer's policy decision, as its risk section requests. This review does not claim the old 0.4.4 implementation was fixed by retirement, and does not approve fork CI.

@Nicholas023 Nicholas023 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.

审查结论:REQUEST_CHANGES — 建议关闭

这个修复在 0.5.0 上已经没有目标路径,而 0.4.4 也不再维护。

关键事实

0.5.0 的单行 bundle 不挂载 official-tool-hiding:

text src/index.ts: mountIntegration / mountRuntimeSettings / panel / tui / custom preset —— 没有 hide-official src/official-tool-hiding.ts:29: if (retiredLegacyRow(root, 'hide-official')) return

被修改的测试通过 test/lib/legacy-component.mjs 显式设置 DSH_OPS_LEGACY_ROWS=1 才进入该路径。正常安装的 0.5.0 用户永远走不到这段代码。

既然 0.4.4 不再维护、0.5.0 又没有该路径,本 PR 既没有 main 的发布面价值,也没有 backport 的目标分支。

附带问题:文档与 issue 关联

  • docs/component-reference.md、docs/CHANGELOG.md 把 legacy-only 的行为写成发布行为
  • Closes #415 与事实不符:#415 的关闭理由是「被单行架构替代」,不是被本 PR 修复

已核实为真的部分

  • official-tool-hiding 33/33、presentation-readiness 6/6、mount 51/51
  • revert-red(撤 src/official-tool-hiding.ts)→ 2/33 红,与声明一致
  • 代码逻辑本身自洽、revert 真红,不存在实现缺陷

处置建议

关闭。 如果未来仍需维护 0.4.x,这份补丁应去对应的维护分支;在 main 上若保留,必须显式标注 legacy-only,并改用 Related to #415。

@T-Auto T-Auto closed this Nov 2, 2026
@tietoumiao

Copy link
Copy Markdown
Author

Closing — the review is right that official-tool-hiding is not mounted by the 0.5.0 bundle (it is behind the retired legacy row), so this patch had no reachable target on main and no maintained branch to accept it. The code-level checks in the review (33/33, revert-red true) are noted, but reachability decides it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants