Skip to content

workflow-console: address the review suggestions on #64 - #74

Open
stevenjj33 wants to merge 1 commit into
MiniMax-AI:mainfrom
stevenjj33:fix/workflow-console-review-suggestions
Open

stevenjj33 wants to merge 1 commit into
MiniMax-AI:mainfrom
stevenjj33:fix/workflow-console-review-suggestions

Conversation

@stevenjj33

@stevenjj33 stevenjj33 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

背景

#64 已合并(62773d1),评审在 bd0d99e 上给出 APPROVED,并留了三条可选建议。这个 PR 逐条处理。

三条建议

一、README 两个语言版本的 Skill 一节里,关于 Git 导入接受条件的那句话,改动本包以外的规则,规则一变 README 就会过时 —— 建议删掉或中性化。

已按建议删除。宿主「从 Git 仓库导入」的接受条件属于宿主行为,不该由本包的 README 复述,
规则变了 README 就会成为噪音。Skill 本身的价值不受影响——它本来就是给 Agent 说明怎么操作控制台的,
那部分说明全部保留。删掉的理由留在这个 PR 的讨论与提交记录里,不留在 README 里。

二、「数据与边界」里的「运行记录还在引擎数据库里」,可以顺带说明指的是本插件 context.dataDir/engine/ 下的数据,避免被理解成别的数据库。

已在三处补明:

  • README.md —— the bundled engine's own database — workflows.sqlite under context.dataDir/engine/
  • README.zh-CN.md —— 内置引擎自己的数据库里(context.dataDir/engine/ 下的 workflows.sqlite)
  • skills/workflow-console/SKILL.md —— 同一句措辞

三、插件 package.json 没有 test 脚本,在插件目录执行 npm test 会报 Missing script。

已加上,并按本仓库另一个带测试脚本的插件(plugins/zch0227/disk-insight)的写法:

"scripts": {
  "test": "node --test \"tests/*.test.mjs\""
}

两份 README 的测试章节也改成以 npm test 开头,并说明它等价于 node --test "tests/*.test.mjs"。

验证

改动共 4 个文件、+17 / −10,全部是文档与 package.json,无代码与运行时行为变化。


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

Three follow-ups, one per suggestion.

Drop the restatement of the Host's Git-import rule from both READMEs. The
rule belongs to the Host and can change under us; a README that repeats it
goes stale, and this package does not need to carry it. What the Skill is
for — telling the Agent how to drive the console — stays.

Say which database "the run record stays in" means: the bundled engine's
own `workflows.sqlite` under `context.dataDir/engine/`, in both READMEs
and in SKILL.md, so it cannot be mistaken for some other store.

Add a `test` script to package.json, so `npm test` in this directory runs
the suite instead of reporting a missing script. The command matches the
one other plugin in this repository that ships a test script
(`node --test "tests/*.test.mjs"`), and the READMEs now lead with it.
@stevenjj33

Copy link
Copy Markdown
Contributor Author

@MyPrototypeWhat 这条 PR 是 #64 评审里三条可选建议的逐条处理,麻烦复审一下,当前 head 是 540e56e。

  • 建议一(Git 导入规则那句):按建议删除了。宿主「从 Git 仓库导入」的接受条件属于宿主行为,
    README 复述它会过时;Skill 本身的用途说明全部保留。
  • 建议二(引擎数据库):三处都补明了——两份 README 与 SKILL.md 都写成本插件
    context.dataDir/engine/ 下的 workflows.sqlite。
  • 建议三(test 脚本):package.json 加了 "test": "node --test \"tests/*.test.mjs\"",
    写法对齐本仓库另一个带测试脚本的插件(plugins/zch0227/disk-insight);两份 README 的测试章节
    也改成以 npm test 开头。

改动 4 个文件、+17 / −10,全是文档与 package.json,无代码与运行时行为变化。

验证:npm test 16/16;npm run check 69/69、0 errors;宿主的 plugin-system reader 复验本包
仍是 1 个有效 Skill、canImport: true。

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