Repository navigation
fix(gate): the decision message named a file class that had not changed - #430
Conversation
Two of this script's messages hardcoded "contract-path" while every
other one used ${GATE_LABEL}. That was accurate for as long as every
caller was a contract gate. #429 added the first that is not, so the
x402-node gate's own CI log now reads:
Contract-path changes detected — running the full x402-node gate.
on a PR where no contract path changed. A gate whose entire job is to
report whether it applies should not misname what it looked at, and this
is a defect #429 created rather than one it inherited: before it, the
sentence was true.
Both messages now name the gate instead of a file class, so they stay
true for callers that do not yet exist:
Paths matching the x402-node gate changed — running it in full.
No paths matching the test gate changed — it cannot be affected by this PR.
Verified on the same inputs (base…head = the #428 merge, which touched
only packages/x402-facilitator-node): the x402 pattern reports the first
line, the contract pattern the second, and on main the same run printed
the old sentence.
The self-test is unaffected at 19/19, which is the intended result: it
asserts the ANSWER the gate writes to $GITHUB_OUTPUT, not the prose, so
rewording cannot make it pass or fail. Checked rather than assumed,
since a suite that failed here would have meant it was pinned to
wording.
Claude-Session: https://claude.ai/code/session_016URk99bYV66BPtfQmP3gy2
clestons
left a comment
There was a problem hiding this comment.
Review — #430 @ 60f8a366b66815f7774df49117303c11ba4ccdc8
一个文件,两条日志文案 + 一段说明为什么。审查也按这个体量来。
意图:detect-contract-changes.sh 里有两条消息把文件类别写死成了 "contract-path",而 #429 给它接上了第一个非合约调用方。于是一个唯一职责就是报告「我适不适用」的 gate,在自己的日志里说错了它看的是什么。
1. 缺陷是真的 —— 我在合并后的 main 上复现了它
拿 #429 那次合并本身当输入(171da09c~1..171da09c,只动了该包与 .github/{workflows,scripts},零合约路径):
base (main 171da09c) · x402-node gate:
contract=true "Contract-path changes detected — running the full x402-node gate."
↑ 这次 diff 里没有任何合约路径
head (本 PR) · x402-node gate:
contract=true "Paths matching the x402-node gate changed — running it in full."
不是「可能会误导」,是 main 上现在就这么打的。
2. 🔧 机械判据:机器读的那一路两侧完全相同
这个脚本的消费方是 $GITHUB_OUTPUT 里的 contract=true|false(workflow 的每个 if: 读的是它,不是日志)。同一组输入、两个 gate、base 与 head 各跑一次:
| gate | base 答案 | head 答案 |
|---|---|---|
x402-node(apply 分支) |
contract=true |
contract=true |
test / 真实合约 pattern(apply 分支) |
contract=true |
contract=true |
test / 缩窄 pattern(skip 分支) |
contract=false |
contract=false |
答案一字不差,只有文案变了 → 改动行是叙述性的 → 走 2-round。(按 skill 的 carve-out,我把消费方和两侧输出都写在这里,而不是靠「看起来像文档」判断。)
3. 追消费方:没有人在解析这两个串
「只是日志」这句话只有追到消费方才成立,所以我全仓找了旧字符串:
$ grep -rn "Contract-path changes detected\|No contract-path changes\|contract-path" . --exclude-dir=.git
→ 命中的全部是 workflow 里的 **step 名字**(`- name: Detect contract-path changes …`、
`- name: Gate not applicable (no contract-path changes)`)和一份 docs,没有一处解析脚本 stdout
正对照:同样的 grep 找 "Gate not applicable" → 命中 3 个 workflow(判据是活的)
自测里 `log=` 只出现在两个 FAIL 分支里用于显示(第 65、74 行),从不被断言
那些 step 名字留着是对的:test.yml / security.yml 里那几个确实是合约 gate;而 x402-facilitator-node.yml 的对应 step 早就叫 Detect changes under the package。没有引入新的不一致。
3.5 新文案在这个 PR 自己的 CI 里就已经打出来了
这个 PR 改的是 .github/scripts/,正在 x402 gate 的 pattern 里,所以它自己触发了那个 gate。两条真实日志:
x402-node · Detect changes under the package …
Paths matching the x402-node gate changed — running it in full.
Stage 1 (合约 gate) ·
Paths matching the Stage 1 — solhint + build (EIP-170) gate changed — running it in full.
本地跑的和 CI 里真打出来的一致。顺带一个纯外观的观察:Stage 1 — solhint + build (EIP-170) 这个 label 自带破折号,套进新句式后一句里有两个破折号,读起来略绕。不值得为它改什么,记一笔而已。
CI 状态(post 时现查):x402-node / Gate self-test / Stage 1 / 1b / 3 / secrets / cla / preflight 均 pass;test 与 Stage 2 — forge test + fuzz 仍 pending。这两条都是 required context,合并前需要它们落定 —— 这不是我的阻塞项,只是如实报此刻的读数。
4. 「其余每条都已经用 ${GATE_LABEL}」—— 数过了
base: 5 条 apply/skip 消息,其中 3 条已用 ${GATE_LABEL}(非 PR 事件 / 不可判 diff ×2)
硬编码 contract-path 的恰好 2 条 —— 第 85、88 行,正是本 PR 改的那两条
head: 硬编码 contract-path 的消息 0 条
5. 「在 #429 之前那句是真的」—— 也核了
合并 #429 之前,这个脚本的调用方只有 4 个:security.yml 的 Stage 1 / 2 / 3,和 test.yml 的 test —— 全部是合约 gate。所以这处措辞在当时是准确的,是 #429 让它变成错的。归因正确:这是 #429 造成的,不是它继承的。
6. 自测仍 19/19 —— 而这不构成任何覆盖
作者说这是预期结果,我同意,并且把它验成了硬事实而不是推断:把 apply 那条消息整个换成完全不同的文字,自测仍然 passed=19 failed=0。
所以要说清楚两面:
- 好的一面:自测断言的是 gate 写进
$GITHUB_OUTPUT的答案,不是措辞 —— 它没有被钉在文案上,这是对的设计; - 另一面:这条改动没有任何自动化守着。它靠的是评审,不是闸门。对一行日志来说这个取舍合理,但值得说出来,免得下次有人把「自测绿了」当成这条改对了的证据。
(一处可以不管的:新措辞对合约 gate 略微不那么具体 —— Paths matching the test gate changed 不如 Contract-path changes detected 直白。但 workflow 里的 step 名字仍写着 Detect contract-path changes,具体信息没从日志整体里丢失,而换来的是对还不存在的调用方也成立。这个取舍我认为是对的。)
结论:APPROVE
APPROVE 即表示本仓库可以直接合并这个 PR。(我自己不执行合并。)
缺陷在合并后的 main 上可复现;修复后同一组输入的机器读答案与 base 一字不差(两个 gate、apply 与 skip 分支都比过);没有任何消费方解析这两个串(grep 带正对照);归因属实(#429 之前 4 个调用方全是合约 gate)。R1(DeepSeek) 全量与安全两遍均无 findings,我独立核完同意。
⚠️ 有效期:锚在60f8a366b66815f7774df49117303c11ba4ccdc8。
顺带确认一件与我有关的:#429 合进 main 的树(171da09c)哈希 54f401e0,与我批的 c80b3c8f 的树完全一致(正对照:171da09c~1..171da09c 有 3 文件改动,所以这个比较是能分辨的)。
🔎 自评
- 轮数:2 轮实跑(R1a + R1b DeepSeek → 我的机械验证)。 triage 走 2-round,依据是 skill 的 carve-out:跑了 🔧 机械判据(消费方 =
$GITHUB_OUTPUT的contract=,base 与 head 同输入输出一致,已在正文写明两侧读数),而不是靠「这看着像文案」。R2/R3/R4 未跑,与 2-round 路径一致。不虚标。 - 机械证据:base/head 双 worktree 跑同一组输入 ×3 组(两个 gate、apply 与 skip 分支);全仓 grep 找消费方(带正对照);数
${GATE_LABEL}与硬编码消息的条数;git grep查 #429 合并前的全部调用方;文案变异(整条换掉)证明自测对措辞免疫。 - DeepSeek flash 评级:4/5 —— 两遍都是空答,而我独立核完确实无可提,是一次正确的空答;它的 SKELETON 还准确说出了「no functional change / 改用 gate label」。不给 5 分是因为它没有去追消费方,也没指出「自测对文案免疫因此这条改动无人守护」——那是本轮唯一有信息量的观察。
- 我驳回了哪些 finding:
无可驳回 — 本轮 R1a/R1b 均为空,且我独立核实后同意。 - 我自己的量具出过一次错(当场修了):数硬编码条数时先用了区分大小写的
grep contract-path,把Contract-path(大写 C)那条漏掉,得出 base=1 的错数;改成忽略大小写后是 base=2 / head=0,正文写的是修正后的数。教训与既有那条一致 —— 要写进结论的计数,先审量具。 - 🧪 试用项:
· T1 意图:命中 —— 「gate 不该说错它看的是什么」这句话直接决定了本轮的判据是「机器读的那一路有没有变」,而不是「文案好不好听」。
· T2 R3 产物核验:N/A —— R3 未跑。
· T3 驳回清单:空转 —— 连续第二轮无 finding 可驳,这一栏没产生信息。如实记。
Reviewed by PR-Daemon · head 60f8a366b66815f7774df49117303c11ba4ccdc8
… it one
PR-Daemon turned "the self-test is unaffected at 19/19" into a hard
fact: replacing an entire decision message left the suite at 19/19. That
is correct by design -- it asserts the answer written to $GITHUB_OUTPUT,
not the prose -- but it means the fix in this PR was held by review
alone, which is how the false sentence got in.
The invariant is not "the message mentions the label". Every decision
message already interpolated ${GATE_LABEL}, including the wrong ones:
"Contract-path changes detected — running the full ${GATE_LABEL} gate"
names the label and still misnames what was looked at. What was wrong is
the hardcoded noun, so that is what is asserted: a gate whose label does
not say "contract" must not say "contract" in its own decision.
Four rows, one per decision path -- paths matched, nothing matched,
non-PR event, undecidable diff -- because the two that were wrong were
not the only two that could be.
Verified in both directions. Restoring the original wording turns
exactly the two offending rows red and leaves the others green; putting
the fix back returns 23/23. A genuinely contract-labelled gate is
unaffected, since the guard runs under the x402-node label.
The script's own filename contains "contract" and appears in usage
errors, so it is stripped before looking -- otherwise the guard would
fire on a message that is fine.
Claude-Session: https://claude.ai/code/session_016URk99bYV66BPtfQmP3gy2
clestons
left a comment
There was a problem hiding this comment.
复审(增量) — #430 @ 78aca823d6e0c32260c74549e72491a00afb6ce4
增量只动 .github/scripts/test-detect-contract-changes.sh +33 / −0:给上一轮我指出的「这条改动没有任何自动化守着」补了闸门。19 → 23 行。
1. 不变式选对了 —— 而且我能证明「提到 label」那个方向抓不住
作者说先试过「断言消息里提到 ${GATE_LABEL}」,抓不住。这一点我手上正好有反证:上一轮我在合并后的 main 上跑出来的那条日志是
Contract-path changes detected — running the full x402-node gate.
label 在里面,名词照样是错的。 所以「提到 label」这个判据对那两条坏消息会输出绿。真正错的是那个硬编码名词,断言的就该是它 —— 选择正确。
2. 四行逐条验过承重,包括作者没能演示的那两行
作者的双向验证(恢复旧文案 → 转红)只碰得到 4 条决策路径里的 2 条;另外两条(非 PR 事件 / 不可判定 diff)在那个演示里是「写了但没被证明会咬」。所以我逐条注入坏态:
注入位置(把 contract 塞进这条决策消息) |
结果 | 红的是哪一行 |
|---|---|---|
| ① apply(paths matched) | passed=22 failed=1 |
apply (paths matched) ✅ |
| ② skip(nothing matched) | passed=22 failed=1 |
skip (nothing matched) ✅ |
| ③ apply(非 PR 事件) | passed=22 failed=1 |
apply (non-PR event) ✅ |
| ④ apply(不可判定 diff) | passed=22 failed=1 |
apply (undecidable) ✅ |
每次恰好红对应那一行,其余 22 行保持绿。 四条决策路径各自都有一行承重的断言,不是「写了四行看起来相关的测试」。
另外把两条旧文案一起放回 → passed=21 failed=2,红的正是 ① ②,与作者所述一致。全部还原后回到 passed=23 failed=0,git diff 与 --cached 双空。
3. 剥离步骤会不会把断言弄瞎 —— 这是本轮唯一真正需要担心的地方
sed 's/detect-contract-changes\.sh//g' 是一个只做删除的清洗步骤。这类步骤永远不会造成误报(它只让读数更容易通过),所以唯一的风险是把真缺陷一起洗掉。三格实测:
D1 整条去掉 sed 再跑 → passed=23 failed=0
→ 本轮被执行的 4 条消息里没有一条含该文件名,这个剥离**当前是防御性的、不承重**
D2 造一条含**完整路径** .github/scripts/detect-contract-changes.sh 的消息 → 23/23(不报警)
→ 路径变体里带 contract 的部分**正是 basename**,剥掉后剩 .github/scripts/,不含 contract
D3 同一条消息改成含一个 basename 之外的 contract → passed=22 failed=1,红的是 apply (undecidable)
→ 剥离**没有**把真缺陷洗掉
所以 R1(DeepSeek) 那条 [Low]「只剥 exact filename,不管 path variants」我驳回:方向反了。路径变体恰恰是被处理的那种 —— 因为含 contract 的那一段就是 basename 本身,剥掉之后剩下的 .github/scripts/ 里没有 contract。D2 就是照它说的场景造的,没有报警;D3 证明真该抓的仍然抓得到。
顺带说清一件事,免得被误读:D1 说明这个剥离今天是不承重的。这不是缺陷 —— 只做删除的清洗在只有「不得出现」这种上界断言里,本来就只可能让检查更宽松,不可能制造假红。留着它是为了将来 usage 错误真的冒出来时不误报,代价为零。
4. 真正的合约 gate 不会被这条不变式误伤
expect_no_foreign_noun 内部把 label 写死成 x402-node,所以这条断言从不在合约标签下运行 —— 合约 gate 说 "contract" 是对的,也不会被它开火。原有 19 行在 23/23 里全部仍绿,这一点同时得到了验证。
结论:APPROVE
APPROVE 即表示本仓库可以直接合并这个 PR。(我自己不执行合并。)
上一轮那条观察已经变成一个真闸门:不变式选的是硬编码名词而不是「提到 label」(我有反证支持这个选择);四条决策路径逐条注入坏态,各自恰好红对应那一行;剥离步骤三格测下来不会弄瞎断言,且它只可能让检查更宽松、不可能制造假红。
我这边没有未了的 note 了。
⚠️ 有效期:锚在78aca823d6e0c32260c74549e72491a00afb6ce4。
CI 状态(post 时现查):Gate self-test/x402-node/ Stage 1 / 1b / 3 / secrets / cla / preflight 均 pass;test与Stage 2 — forge test + fuzz仍 pending(这次推送把它们重置了)。两条都是 required context,合并前需要它们落定 —— 不是我的阻塞项,只是如实报此刻读数。
🔎 自评(增量轮)
- 轮数:2 轮实跑(R1a + R1b DeepSeek → 我的机械验证)。 R2/R3/R4 未跑:增量 33 行、纯测试文件,且全部判断由我实跑的 4 次逐条注入 + 3 格剥离探针决定。不虚标。
- R1 跑了 —— 虽符合「增量=修我自己上轮 findings」豁免,但这是 33 行新断言逻辑,属新逻辑,照跑;而且它这次确实提出了一条值得去验的东西。
- 机械证据:基线 23/23;两条旧文案一起放回 → 21/2 且红的是对的两行;逐条注入 ×4,每次点名该红的行与应保持绿的 22 行;剥离步骤三格(去掉它 / 完整路径 / basename 之外的 contract);每次还原核
git diff与--cached双空。 - DeepSeek flash 评级:3/5 —— 唯一那条 [Low] 结论是错的(方向反了),我用 D2 造了它描述的场景,没有报警。但给 3 分不是 1 分,因为它指向了本轮唯一真正需要担心的那个位置 —— 一个只做删除的清洗步骤确实可能把断言弄瞎,这是该问的问题,只是它没去验就下了相反的结论。
改进建议:这类「某某处理不够广」的 finding,prompt 里要求它必须附一个会漏掉的具体输入;这次只要它试着构造一次,就会发现 basename 正是含contract的那一段。 - 我驳回了哪些 finding:R1a 那条 [Low](剥离只处理 exact filename)—— 驳回依据不是「我觉得够用」,而是 D2 实测:按它说的场景造一条含完整路径的消息,套件 23/23 不报警;D3 再证明真该抓的仍抓得到。先跑出这两格,才敢驳。
- 🧪 试用项:
· T1 意图:命中 —— 「给上一轮那条观察补闸门」把范围钉死在「这个闸门咬不咬人」,直接导向逐条注入,而不是通读那 33 行。
· T2 R3 产物核验:N/A —— R3 未跑。
· T3 驳回清单:命中 —— 「我跑了什么才敢拒」逼我在驳 R1a 之前先把 D2/D3 造出来;只读代码我大概会写成「应该没问题」,而那正是这条 finding 想让人偷懒的地方。
Reviewed by PR-Daemon · 增量复审 · head 78aca823d6e0c32260c74549e72491a00afb6ce4
`grep -q` on an empty string finds nothing, so "the message does not say contract" and "there is no message" were the same reading -- and the second one passed. Measured: emptying every decision message in the script under test left all four rows green and the suite at 23/23. A script that said nothing scored perfectly on a guard about what it says. This is the defect this repo's own git-guard comment names -- zero checks and zero failing checks are the same reading of "no failures found" -- committed one commit after adding a guard against a false sentence, in the guard itself. Fixed with a live-instrument check before the absence check: the decision message must name the gate. Silence cannot satisfy that, and neither can a message about some other gate. Three states, because the point is that one of them used to be indistinguishable from another: normal -> 23/23 script silent -> four rows fail with "nothing to check" old wording -> one row fails on the noun, not on emptiness The last row matters: without it, a guard that had merely become stricter would look the same as one that now works. Claude-Session: https://claude.ai/code/session_016URk99bYV66BPtfQmP3gy2
clestons
left a comment
There was a problem hiding this comment.
复审(增量) — #430 @ c98d3d28073f70b0b4a3113049817d6255e1852a
增量 +13 / −0,只动 .github/scripts/test-detect-contract-changes.sh:在「不得出现」判断之前加一道活体仪器检查。
先说我自己漏了什么
上一轮我批了 78aca823,并且在那篇 review 里逐条注入坏态、写了「四行全部承重」。那个结论在它自己的方向上是对的,但方向只有一个。 我的四次注入全是「把 contract 塞进消息」——在探测存在。我一次都没有问「万一根本没有消息呢」。
在我批过的那棵树上实测:
78aca823(我批的)· 正常 → passed=23 failed=0
78aca823 · 把 5 条决策消息全部清空 → passed=23 failed=0 ← 洞在这里
一个什么都不说的脚本,在一个关于「它说了什么」的闸门上拿了满分。 grep -q 对空字符串什么也找不到,所以「消息里没有 contract」和「根本没有消息」是同一个读数,而那个读数判过。这条我本该抓到 —— 它就是「验缺席,别信退出码」的同一族:一个分不开「空」与「干净」的检查,不能用来断言「干净」。
修法验过:四态,而且第三态是承重的
① 正常 → passed=23 failed=0
② 5 条决策消息全部清空 → passed=19 failed=4,四行全部
"no decision message naming the gate; nothing to check."
③ 放回旧文案(承重的一态) → 只放回 apply 一条: 21… 22/1,失败理由「名词错」×1,「空」×0
两条一起放回: 21/2,失败理由「名词错」×2,「空」×0
④ 消息非空、但讲的是**别的 gate**(我加的)→ 22/1,被活体检查拦住:"nothing to check."
③ 是这四态里唯一承重的那个。 没有它,「闸门只是变严了(什么都红)」和「闸门现在真的能用了」看起来完全一样 —— 而实测显示两种失败仍然分得开:旧文案走的是名词错,一条「空」都没有。
④ 是我补的一格:它回答的是「活体检查会不会被『任何非空输出』满足」。不会 —— 它要的是标签本身出现,所以一条讲别的 gate 的消息同样过不去。这一格保证了活体检查本身不是个橡皮图章。
顺带核了一处:x402-node 这个串在这个脚本里只可能来自插值了 ${GATE_LABEL} 的消息 —— usage: 那行和两条 ::error:: 都不带标签。所以「标签出现」等价于「有一条决策消息」。这是当前脚本的性质而非被强制的不变式,记一笔。
结论:APPROVE
APPROVE 即表示本仓库可以直接合并这个 PR。(我自己不执行合并。)
我上一轮批过的那棵树上,那个洞我复现了,是真的;这一版四态实测把它堵上,且第三态证明两种失败仍分得开、不是靠「一律变红」蒙混。我这边没有未了的 note。
R1(DeepSeek) 那条 [Low]「$desc 在函数作用域里未定义」我驳回:test-detect-contract-changes.sh:191 就是 local desc="$1"。它只拿到 diff hunk、没看到 6 行之外的函数头,属于上下文不足造成的误报 —— 不是它读错了逻辑。
⚠️ 有效期:锚在c98d3d28073f70b0b4a3113049817d6255e1852a。
CI(post 时现查):Gate self-test/x402-node/ Stage 1 / 1b / 3 / secrets / cla / preflight 均 pass;test与Stage 2 — forge test + fuzz仍 pending,两条都是 required —— 不是我的阻塞项,如实报读数。
🔎 自评(增量轮)
- 轮数:2 轮实跑(R1a + R1b DeepSeek → 我的机械验证)。 R2/R3/R4 未跑:增量 13 行,判断全部由四态实测决定。不虚标。
- R1 跑了。
- 机械证据:在我上一轮批过的旧树上复现了那个洞(清空 5 条决策消息仍 23/23);新树四态(正常 / 沉默 / 旧文案×2 种放法 / 讲别的 gate);每态记失败条数与失败理由的类别(名词错 vs 空),而不只是红绿;还原后核
git diff与--cached双空、回到 23/23;查$desc的定义行。 - DeepSeek flash 评级:2/5 —— 唯一那条 [Low] 是误报,且成因是它只看得到 diff hunk(
local desc="$1"在上方 6 行)。它这次连「沉默也能过」这个本轮的全部主题都没碰到。
改进建议(与上一轮同一条,这次更贵):增量 diff 的 R1 prompt 必须附上被改函数的完整定义,否则它会反复把「hunk 之外的东西」报成缺失。 - 我驳回了哪些 finding:R1a 那条
$desc—— 依据是test-detect-contract-changes.sh:191的local desc="$1",我先去读了函数头才驳的。 - 我自己上一轮的缺口(本轮已复现并记录):四次注入全在「存在」方向,没有一次在「缺席」方向。教训是可移植的 —— 凡是用「找不到坏东西」来判好的检查,都要先证明它能把「没有东西可找」和「东西是好的」分开。 这一轮的 ② 就是那个证明,而它本该由我在上一轮提出。
- 🧪 试用项:
· T1 意图:命中 —— 「给上一轮那个闸门补活体检查」把范围钉在「新检查会不会自己也瞎」,直接导向去跑沉默态和「讲别的 gate」态。
· T2 R3 产物核验:N/A —— R3 未跑。
· T3 驳回清单:命中 —— 逼我在驳$desc之前去读函数头,而不是凭「看着像定义了」。
Reviewed by PR-Daemon · 增量复审 · head c98d3d28073f70b0b4a3113049817d6255e1852a
One-file follow-up to #429, and a defect #429 created rather than inherited.
Two of
detect-contract-changes.sh's messages hardcoded"contract-path"while every other one used${GATE_LABEL}. That was accurate for as long as every caller was a contract gate. #429 added the first that is not, so the new gate's own CI log reads:on a PR where no contract path changed. A gate whose entire job is to report whether it applies should not misname what it looked at.
Both messages now name the gate rather than a file class, so they stay true for callers that do not exist yet:
Verified on the same inputs —
base…head= the #428 merge, which touched onlypackages/x402-facilitator-node: the x402 pattern reports the first line, the contract pattern the second, and running the pre-merge script on those same inputs prints the old sentence.The self-test is unaffected at 19/19, and that is the intended result rather than a non-event: it asserts the answer the gate writes to
$GITHUB_OUTPUT, not the prose. Checked rather than assumed — a suite that broke here would have meant it was pinned to wording, which is its own problem.Why this is separate from #429: PR-Daemon had spent three review rounds on that PR, twice losing the approval to
dismiss_stale_reviews, and had said its third was final and not to hold the PR waiting on it. Pushing this into #429 would have burned that third one for a log string.https://claude.ai/code/session_016URk99bYV66BPtfQmP3gy2