Skip to content

refactor: single-source hash-avoid prompts + official npm registry#181

Merged
Svtter merged 2 commits into
mainfrom
refactor/shared-hash-avoid-prompts
Jun 3, 2026
Merged

refactor: single-source hash-avoid prompts + official npm registry#181
Svtter merged 2 commits into
mainfrom
refactor/shared-hash-avoid-prompts

Conversation

@Svtter

@Svtter Svtter commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses the two remaining architecture/infrastructure suggestions from PR #178 review.

Changes

1. Single-source hash-avoid prompts (shared/prompts/)

  • Added shared/prompts/hash-avoid-zh.txt and shared/prompts/hash-avoid-en.txt as the single source of truth
  • Consistency tests now compare both TS (reviewers.ts) and Python (run-github-opencode.py) inline definitions against these shared files
  • Previously: tests parsed TS/Python source with regex and compared to each other (fragile — format changes break tests)
  • Now: tests parse each source and compare to the canonical file (format-independent assertions)

2. Official npm registry (multi-review/.npmrc)

  • Added multi-review/.npmrc with registry=https://registry.npmjs.org/
  • Previously: package-lock.json resolved against the developer's local npmmirror.com config (138 references)
  • Now: lockfile uses official registry regardless of local config (0 mirror references)

Testing

  • 14 TS unit tests pass
  • 52 Python tests pass (including 2 updated consistency tests)
  • TypeScript build passes (tsc --noEmit)

- Add shared/prompts/hash-avoid-zh.txt and hash-avoid-en.txt as the
  single source of truth for hash-number avoidance instructions.
- Consistency tests now compare both TS (reviewers.ts) and Python
  (run-github-opencode.py) inline definitions against the shared files,
  eliminating fragile cross-language regex comparison.
- Add multi-review/.npmrc to pin registry.npmjs.org, removing
  dependency on local npmmirror configuration. Regenerated package-lock.json.
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

有条件合并

本 PR 将 hash-avoid 提示文案集中到 shared/prompts/ 作为测试侧单一真源,并切换 .npmrc 至官方 npm registry。功能正确,无安全问题,性能/架构层面无阻塞项。有一项测试断言质量回退需修复后才能合并。

阻塞项:

  • tests/test_all.py:1079-1080, 1101-1102self.assertIsNotNone(...) 被降级为裸 assert。前者失败时输出更清晰,且不会被 python -O 优化掉。需恢复为 unittest 风格(仅 quality reviewer 明确标记阻塞,但属测试质量回退,按"取最严重发现"原则保留为阻塞项)。

建议项:

  1. 已确认(2 reviewer 同意):.npmrcnpmmirror.com 切到 npmjs.orgpackage-lock.json 内的 rollup/js-yaml/@opencode-ai/sdk/tinyglobby 版本变更与"提示词单一来源重构"无直接耦合,应拆为独立 PR;security reviewer 视其为正向供应链改进,但 quality/performance reviewer 认为混入会模糊改动焦点。npmmirror.com 在国内安装更快,若主要 CI 在大陆,建议保留 .npmrc 并通过环境变量允许覆盖。
  2. 已确认(2 reviewer 同意):shared/prompts/hash-avoid-{en,zh}.txt 首行为空行,依赖 _read_sharedrstrip("\n") 与源码抠出字符串隐式对齐,契约脆弱。建议要么删除首行空行,要么在 _read_shared 中显式 lstrip("\n") + rstrip("\n"),并在文件中加注释说明用途。
  3. 已确认(2 reviewer 同意):tests/test_all.py 仍以正则从源码中提取字符串字面量做断言。重构后已具备"事实源"基础,更彻底方案是让 TS/Python 在 build/import 时嵌入共享文件内容并直接比较。属过渡方案,建议后续 PR 完成。
  4. shared/ 作为新增顶层目录缺少 README.md 说明职责与准入规则(是否仅限 prompt,是否允许其他共享数据等)。
  5. "single source of truth" 措辞与实际行为有落差:shared/prompts/*.txt 仅被测试读取,TS/Python 运行时仍硬编码相同字符串,未真正消除重复。建议在 docstring 中明确这是"测试侧"单一事实源,或改用打包时内联方式。
  6. _read_shared() 在两个测试方法中各调用一次,可考虑 setUpClass 缓存(收益微乎其微,但成本也低)。
  7. CHANGELOG.md 未记录本次文件结构与测试逻辑变化。
  8. 预存在的 TestDogfoodWorkflow 5 个用例因 review.yml 缺失而失败,与本 PR 无关,无需处理。

📋 各 Reviewer 详细审查结果
quality

我需要先查看实际文件来验证 PR 的内容。
有条件合并

本 PR 将 hash-avoid 提示文案提取到 shared/prompts/ 下作为单一来源,并在测试中校验 TS/Python 中的内嵌字符串与之一致。功能上正确(两个新测试均通过),但存在几处需要关注的问题。

阻塞项:

  1. 测试断言降级tests/test_all.py:1079-1080, 1101-1102 将原来的 self.assertIsNotNone(...) 改为裸 assertunittest 的断言方法在断言失败时给出更清晰的诊断信息,且不会被 python -O 优化掉。建议恢复为 self.assertIsNotNone(...),保持与文件中其他测试一致的 unittest 风格。

建议项:

  1. 混入的依赖变更未与重构解耦.npmrcregistry.npmmirror.com 切换到 registry.npmjs.org,同时 package-lock.jsonrollup 4.60→4.61js-yaml 4.1→4.2@opencode-ai/sdk 1.15.10→1.15.13tinyglobby 0.2.16→0.2.17 等也一起变更。这些与"单一来源重构"无直接关系,建议拆分为独立 PR,并说明 registry 切换的理由(影响国内安装速度)。
  2. 测试仍依赖脆弱的正则解析tests/test_all.py:1069-1105 仍然用 regex 从源码中抠出字符串常量。更彻底的方案是让 TS/Python 在运行时直接读取 shared/prompts/*.txt(消除重复维护)。当前 PR 是过渡方案,建议在后续 PR 中完成这步。
  3. 共享文件首部空行shared/prompts/hash-avoid-{en,zh}.txt 首行为空。这是因为源字符串以 \n 开头,但作为独立文件看起来不自然,且依赖 _read_sharedrstrip("\n") 才能和源码抠出的字符串匹配。如果未来改成运行时读取,行为上是一致的;但建议在文件内加一行注释说明首部空行的用途。
  4. CHANGELOG 未更新:本次重构属于可见的行为变化(文件结构、测试逻辑),建议在 CHANGELOG.md 中记录。
  5. 预存在的测试失败(与本 PR 无关):TestDogfoodWorkflow 5 个用例因 .github/workflows/review.yml 不存在而失败,是历史遗留问题,无需在本 PR 中处理。
security

安全无虞

本 PR 为依赖与配置优化类变更,不涉及业务代码改动:

  • multi-review/.npmrc 将 npm registry 从 registry.npmmirror.com(第三方镜像)切换至官方 registry.npmjs.org,属于供应链安全改进,降低对第三方镜像的信任依赖。
  • multi-review/package-lock.json 中所有 resolved URL 同步切换、版本小幅升级(@opencode-ai/sdk 1.15.10→1.15.13、rollup 4.60.4→4.61.0、@types/estree 1.0.8→1.0.9、js-yaml 4.1.1→4.2.0、tinyglobby 0.2.16→0.2.17),integrity 字段完整保留,未引入可疑来源。
  • shared/prompts/hash-avoid-{en,zh}.txt 仅为静态提示词文本,无可执行代码,不构成注入向量。
  • tests/test_all.py 仅为测试结构重构(提取单一真相源),不影响运行安全。

阻塞项:无
建议项:无

performance

性能良好

本 PR 主要是:

  1. 测试重构tests/test_all.py):从"TS vs Python 双向比对"改为"以 shared/prompts/*.txt 为单一真源",新增读取两个 ~30 字节的提示文件。文件极小,I/O 开销可忽略。
  2. 新增共享提示文件shared/prompts/hash-avoid-{en,zh}.txt):静态小文本,无运行时影响。
  3. package-lock.json 升级:rollup 4.60.4→4.61.0、@opencode-ai/sdk 1.15.10→1.15.13、js-yaml 4.1.1→4.2.0、tinyglobby 0.2.16→0.2.17、tsx 内嵌 esbuild 0.27.7→0.28.0,均为补丁/次版本号,不涉及运行时热点。
  4. .npmrc 切换 registry:从 registry.npmmirror.com 切到 registry.npmjs.org,仅影响 npm install 阶段,属开发/构建期配置,不影响 action 运行时。

阻塞项:无

建议项:

  • 两个测试方法各自调用 _read_shared() 读取同一文件,可考虑在 setUpClass 中读取一次并缓存到类属性,以进一步减少 I/O 次数(但因文件极小、仅执行一次,实际收益微乎其微)。
  • .npmrc 切换到官方 registry 后,在中国大陆/部分亚洲地区执行 npm install 可能比 registry.npmmirror.com 慢;如团队主要在大陆 CI 环境运行,可考虑保留 .npmrc 但允许在 CI 中通过环境变量覆盖(仅为体验建议,非性能阻塞项)。
architecture

架构有疑虑

本次 PR 的核心改动是在仓库根新增 shared/prompts/ 目录,把 hash-avoid 提示词集中成单一事实源,并由测试统一校验 TS(multi-review/src/reviewers.ts:36-42)和 Python(github-run-opencode/run-github-opencode.py:459-468)两侧副本与共享文件一致。其余 .npmrc 切换到 npmjs 官方源、随附的 package-lock.json 版本/registry 同步属于配套变更,不涉及架构。

架构层面观察:

  • 模块位置与命名shared/prompts/ 落在仓库根,与 multi-review/github-run-opencode/ 等并列,表达了"跨多个 action 共享的纯数据资源"这一意图,位置恰当。命名 hash-avoid-{en,zh}.txt 与已有 multi-review/reviewers/*-{en,zh}.md 的 i18n 命名风格一致,使用 .txt 也合理(避免被当作独立 Markdown 文档)。
  • 关注点分离:共享目录只放纯文本 prompt,无逻辑、无依赖,分层清晰;tests/test_all.py 中的 TestCrossLanguageHashInstructionConsistency 负责单一职责的跨语言一致性校验。
  • 接口/契约shared/prompts/hash-avoid-*.txt 隐含的契约是"行内容 + 末尾换行",测试用 .rstrip("\n") 抹平末尾换行差异,但要求首字符为 \n 以匹配现有 HASH_AVOID_* 前导 \n 的拼接风格。契约没显式文档化,依赖两端隐式约定。
  • 一致性:新增 helper _read_shared / 重排静态方法顺序、assertIsNotNoneassert,与既有 unittest 风格基本一致。
  • Shotgun surgery 缓解:原本需要同时改 TS 和 Python 两处字符串、还要同步修改两端的正则提取器;现在只在共享 .txt 中改一处,测试能立刻发现漂移。这是一个明显的架构改进。
  • .npmrc + package-lock.json:仅是构建/分发侧配置,跨包不影响运行时架构。

阻塞项:

建议项:

  1. "single source of truth" 实为"测试锚点",未消除运行时重复reviewers.ts:36-42run-github-opencode.py:459-468 仍硬编码相同字符串,shared/prompts/*.txt 仅被测试读取——这避免了漂移检测失败,但并没有真正消除重复,未来的"改一处生效"承诺并不成立。考虑到 multi-review 通过 tsup 打包、github-run-opencode 是独立分发脚本,直接运行时读文件不一定可行,但当前 PR 命名/注释("single source of truth")与实际行为存在落差。建议在 docstring 中明确这是"测试侧"单一事实源,或在 reviewers.ts 中提供基于 import.meta.url/打包时内联的 fallback 方案以收敛两处副本。
  2. 共享 prompt 文件的契约没有显式说明。文件首行是空行(hash-avoid-en.txt:1 仅为 \n),靠测试里的 .rstrip("\n") 抹平末尾、靠首字符 \n 匹配——两端隐式约定较脆弱。建议要么在 shared/prompts/README.md 写明"每个文件内容 = prompt 文本本身,不含前后额外换行"并删除首行空行,要么在 _read_shared 中显式 .lstrip("\n") + .rstrip("\n"),让契约不依赖文件首字符。
  3. shared/ 目录缺少顶层说明。新顶层目录对外部贡献者而言意图不明显(shared 包含哪些类别?是否允许放非 prompt 的共享数据?)。建议补一个简短的 shared/README.md 说明该目录的职责与准入规则。
  4. test_all.py 仍以正则解析源文件字符串字面量。这是历史问题(README 注释中已说明),新 PR 没有改善也未恶化,但既然新增了共享文件,可考虑改成直接 import/读取并断言两侧副本即来自该文件(例如 TS/Python 在 build/import 时把内容嵌入为常量,再用属性检查),从而把"事实源"从测试断言升级为运行时契约。

- Restore assertIsNotNone semantics via raise AssertionError (not stripped by -O)
- Remove leading empty line from shared prompt files; use .strip("\n")
  and .lstrip("\n") to decouple file format from inline text
- Fix docstring: clarify this is a test-side anchor, not runtime SSOT
- Add shared/README.md documenting directory purpose and conventions
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

审计分析

PR #XXX 规范要求(来自 PR 标题与正文,无关联 issue):

  1. shared/prompts/ 新增 hash-avoid-zh.txthash-avoid-en.txt 作为单一真源
  2. 一致性测试改为将 TS(reviewers.ts)与 Python(run-github-opencode.py)的内联定义与共享文件比对
  3. multi-review/.npmrc 添加 registry=https://registry.npmjs.org/
  4. 配套的 package-lock.json 切换为官方 registry

实现核对

  • shared/prompts/hash-avoid-{en,zh}.txt:已新增,内容与 reviewers.ts:36-42run-github-opencode.py:459-468 中字面量一致
  • shared/README.md:已新增,解释了 shared/prompts/ 的职责(reviewer 关注的"目录缺少说明"也已顺带覆盖)
  • multi-review/.npmrc:已新增,单行 registry=https://registry.npmjs.org/
  • multi-review/package-lock.json:已更新,所有 resolved URL 均切换至 registry.npmjs.org/
  • tests/test_all.py:新增 TestCrossLanguageHashInstructionConsistency 类(tests/test_all.py:1055-1126),含 _read_shared_extract_py_hash_avoid_extract_ts_hash_avoid 三个静态方法,以及 test_zh_matches_shared_file / test_en_matches_shared_file 两个测试用例(与 PR 描述的"2 updated consistency tests"对应)。测试锚点从"TS vs Python 双向比对"改为"以 shared/prompts/*.txt 为事实源"

关于 review 评论:评论中提及的若干项(assertIsNotNoneraise AssertionErrorshared/*.txt 首行空行契约脆弱、未改用运行时读取、CHANGELOG 未更新、_read_shared 重复 I/O 等)均属于代码风格 / 架构建议 / 文档遗漏,不属于 PR 规范中描述的功能需求;按审计规则不纳入 gap 报告。


无遗漏

PR 规范中的全部功能项(单一真源文件、跨语言一致性测试改造、.npmrc 切换、package-lock.json 同步更新)均已在当前代码中完整实现,无功能、集成、边界或配置层面的缺口。

New%20session%20-%202026-06-03T13%3A19%3A25.294Z
opencode session  |  github run

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

可合并

总结:本 PR 将 multi-review/.npmrc 从第三方镜像切回官方 npmjs.org,bump 了若干 patch/minor 依赖,并新增 shared/prompts/ 目录作为跨语言 hash 指令一致性测试的共享锚点,tests/test_all.py 相应重构为三方比对。三位 reviewer 均未发现阻塞项。Security 确认切换官方 registry 反而降低供应链攻击面、所有 integrity 哈希保留、升级版本无已知高危 CVE;Performance 确认无热路径改动;Architecture 指出 shared/ 当前形态下仅作"测试侧锚点"而非运行时 single source of truth,存在文档措辞不一致与覆盖用户级 npm 配置的隐患。

阻塞项:无

建议项

  1. 统一 shared/ 角色的文档措辞(架构):shared/README.md 称其为"canonical reference",而 tests/test_all.py docstring 称其为"test-side anchor",措辞不一致且前者容易误导维护者认为应以 shared/ 为唯一真源。建议明确该目录当前仅是测试侧参考、运行时仍由 TS/Python 各自硬编码。
  2. 后续真正消除提示词硬编码(架构):如"消除重复"是真实诉求,建议未来 PR 将 multi-review/src/reviewers.tsgithub-run-opencode/run-github-opencode.py 改为运行时从 shared/prompts/ 读取,让 shared/ 真正承担 single source of truth;当前形态下新增该目录的收益有限。
  3. multi-review/.npmrc 固定 registry 覆盖了用户/CI 级配置(架构 + 性能):会覆盖用户级 .npmrcnpm config,对中国大陆/使用镜像的开发环境与 CI 拉取延迟有负面影响。建议改为通过 CI 环境变量注入,或在文件内加注释说明覆盖原因。Security 视角则认为切回官方 registry 反而降低了供应链攻击面,需权衡后决策。
  4. AGENTS.md/CONTRIBUTING.md 中约定 .npmrc 不得提交私有凭据(安全):当前 .npmrc 仅含 registry 设置无敏感信息,但应将"不得提交 _authToken 等凭据"明确写入仓库约定文档。
  5. CI 中加入 npm audit --omit=dev 定期复核(安全):js-yaml、@rollup 等从镜像解析后版本号有小幅变化,建议 CI 增加 audit 步骤持续监控依赖告警。
  6. 测试中共享文件读取提取到 setUpClass/模块级缓存(性能,可忽略):test_en_matches_shared_filetest_zh_matches_shared_file 各自重复读取 3 个文件并跑正则,文件极小,影响可忽略但更整洁。

📋 各 Reviewer 详细审查结果
quality

(审查失败: fetch failed)

security

安全无虞

本 PR 的主要变更:

  • 新增 multi-review/.npmrc 将 registry 从第三方镜像 npmmirror.com 切换为官方 registry.npmjs.org
  • multi-review/package-lock.json 重新解析并更新若干依赖:@opencode-ai/sdk 1.15.10→1.15.13、@rollup/* 4.60.4→4.61.0、@types/estree 1.0.8→1.0.9、js-yaml 4.1.1→4.2.0、tinyglobby 0.2.16→0.2.17
  • 新增 shared/README.mdshared/prompts/hash-avoid-{en,zh}.txt 纯文本提示文件
  • tests/test_all.py 重构 hash 指令一致性测试,改为以 shared/prompts/*.txt 为参考基准

安全分析:

  • .npmrc 仅包含 registry=https://registry.npmjs.org/,无 _authToken 等敏感凭据,不存在密钥泄露风险;从第三方镜像切回官方 registry 实际降低了供应链攻击面
  • 所有 integrity(sha512)哈希在 lockfile 中完整保留,npm 仍会校验包内容,registry 切换不会绕过完整性检查
  • 升级的依赖均为 minor/patch,未引入已知高危 CVE 的新版本(js-yaml 4.2.0 已修复早期版本相关问题)
  • 测试代码使用 read_text() 与 regex,无 shell/SQL/eval 调用,无路径穿越风险
  • 共享 prompt 仅为纯文本,不含可执行内容或敏感数据

阻塞项:无

建议项:

  • 建议在仓库的 CONTRIBUTING.mdAGENTS.md 中明确约定:.npmrc 中不得提交 _authToken 等私有凭据,避免未来误提交泄露
  • 由于 js-yaml@rollup 等从镜像解析后版本号有小幅变化,建议在 CI 中加入 npm audit --omit=dev 步骤定期复核依赖告警
performance

性能良好

本 PR 主要内容是:1) 创建 shared/prompts/ 目录存放共享 prompt 文本;2) 重构 tests/test_all.py 中的 TestCrossLanguageHashInstructionConsistency 让其以共享文件为锚点对比 TS/Python 实现;3) 将 multi-review/.npmrc 指向官方 registry.npmjs.org 并 bump 若干 lockfile 依赖版本(rollup 4.60.4→4.61.0、js-yaml 4.1.1→4.2.0、tinyglobby 0.2.16→0.2.17、@opencode-ai/sdk 1.15.10→1.15.13 等 patch/minor 更新)。无算法、无运行时热路径改动,性能层面无风险。

阻塞项:无

建议项:

  • multi-review/.npmrc 将 registry 从 registry.npmmirror.com 切到 registry.npmjs.org。对中国大陆开发环境/CI 的 npm install 拉取延迟可能显著增加(属于构建期网络 I/O,不影响 action 运行时性能,但建议评估)。
  • TestCrossLanguageHashInstructionConsistency.test_zh_matches_shared_filetest_en_matches_shared_file 各自都会调用 _read_shared_extract_ts_hash_avoid_extract_py_hash_avoid,即每个 test 都重新读 3 个文件 + 跑 3 套正则。可考虑把共享文件读取提取到 setUpClass/模块级缓存,或在 test_en_matches_shared_file 复用上一 test 的结果,进一步减少重复 I/O(文件很小,影响可忽略,但更整洁)。
  • js-yaml 4.1.1→4.2.0、tinyglobby 0.2.16→0.2.17、rollup 4.60.4→4.61.0 均为上游 patch/minor 升级,未引入额外 bundle 体积或运行时开销,无需处理。
architecture

架构有疑虑

架构分析:
本次 PR 主要包含三部分变更:1) multi-review/.npmrc 新增及 package-lock.json 批量更新(registry 由 npmmirror 切换到 npmjs.org、若干依赖版本小幅 bump);2) 新增顶层 shared/prompts/ 目录存放 hash-avoid-en.txt / hash-avoid-zh.txt,作为跨 action 的"canonical reference";3) tests/test_all.py 中的 TestCrossLanguageHashInstructionConsistency 重构为三方比对(TS / Python / shared 文件)替代原先的双方比对(TS ↔ Python)。

shared/ 目录的引入在分层与命名上是合理的(顶层目录、单一职责、按语言分子目录),整体改动范围局部化、无散弹式修改,依赖版本 bump 与 registry 切换属于运维/构建层面而非架构问题。

但"shared 文件仅供测试使用、运行时仍由 TS/Python 各自硬编码"这一设定在架构上存在疑问:原实现是"两份实现 + 双向一致性测试",改动后变成"两份运行时实现 + 一份测试侧参考文件 + 三方一致性测试",共享文件并没有消除重复,反而引入了第三个需要手动同步的副本;README 又将其称为"canonical reference",容易给后续维护者造成"应以 shared/ 为准"的误解。建议要么真正将 TS/Python 改造为运行时从 shared/prompts/ 读取(消除硬编码重复),要么在 README 和测试注释里明确说明"shared/ 是测试侧冗余参考,非运行时 single source of truth",避免认知偏差。

阻塞项:无

建议项:

  • shared/README.mdtests/test_all.py 的 docstring 对 shared/prompts/*.txt 的角色描述不一致(前者称"canonical reference for consistency tests",后者称"test-side anchor"),建议统一措辞,明确该目录在当前阶段仅为测试锚点、运行时仍由各 action 硬编码,避免后续维护者误以为应以 shared/ 为唯一真源而做无谓改动。
  • 若"消除提示词硬编码重复"是真实诉求,建议后续 PR 将 TS(multi-review/src/reviewers.ts)与 Python(github-run-opencode/run-github-opencode.py)改为在运行时从 shared/prompts/ 读取,使 shared/ 真正承担 single source of truth 的角色;当前形态下新增 shared/ 目录的架构收益有限。
  • multi-review/.npmrc 显式将 registry 固定为 https://registry.npmjs.org/,会覆盖潜在的用户/CI 级 .npmrcnpm config 设置,建议评估是否改为在 CI 中通过环境变量注入,或在文件内补充注释说明为何需要覆盖默认行为,降低对国内/镜像环境开发者的隐性影响。

@Svtter Svtter merged commit 2bd6fe5 into main Jun 3, 2026
3 checks passed
@Svtter Svtter deleted the refactor/shared-hash-avoid-prompts branch June 3, 2026 13:41
@github-actions github-actions Bot mentioned this pull request Jun 4, 2026
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