Repository navigation
feat(spec): spec-pipeline skills with an enforceable direction gate - #40
Merged
Merged
Conversation
0037 still described R1–R5 as pending and claimed 0035 was untracked at 504 lines. Commit 8762cc4 landed all five revisions and tracked the file, so the plan text no longer matched the repository. - Mark the pre-revision snapshot explicitly wherever a 0035 line number, line count, or SHA-256 is cited (504 / a3bb7c11…feb072). - Replace the old postscript with per-item closure notes: R1 and R2 landed as cited; R3's "no such marker" finding only held for the pre-revision snapshot; R4 was closed via the explicit sampling-scope branch; R5 landed. - Restate the mapping table verdicts as 已关闭 and note that 0035 itself records "数字更正不改变第 3 章结论". - Separate measured checks from the still-expected ones in the verification table, and re-date the counts to the 14:54:20 CST recheck. - Drop the self-contradicting "没有修改被调查文档" claim now that the 0035 revision exists. Doc-only. No code, no other files. The direction-confirmation gate skill lives under personal/ and is intentionally untracked per .gitignore.
Closes the three increments from docs/research/0037. The gate they describe
was previously a convention that only held if everyone remembered it; this
puts the decision behind an exit code and a test suite.
The design constraint that shaped everything else: personal/ is git-ignored,
so a CI clone cannot see anything written there. Wiring the check scripts
straight into CI would have failed permanently. So the decisions live in
lib/specGate.js — tracked, testable from fixtures — and the Python scripts
under skills/spec-pipeline/ are thin shells that call it. One definition,
no drift between what a developer runs and what CI asserts.
- lib/specGate.js: decideApproval / decideRoleContract, pure functions
- scripts/check-specs.js: walks specs/*/{TECH,ROLE}.md, exits non-zero
- npm run check:specs, wired into the npm test chain
- test/specGate.test.js (24 cases) and test/checkSpecs.test.js (9 cases)
Both suites keep must-pass fixtures alongside the violation cases. A gate
that rejects everything is as useless as one that accepts everything, and
the stubs exist to catch the first failure mode.
check-specs.js judges "stub" structurally rather than by length. A character
threshold silently passed a short unapproved draft — precisely the document
the gate exists to catch. There is a regression test pinning that.
A repository with no specs/ is a pass, not a failure: most repos do not use
this pipeline, and check:specs runs in npm test. --strict inverts it.
The three skills moved from personal/spec-pipeline/ to skills/spec-pipeline/,
which the existing .gitignore already exempts. skills/ skills are scanned
as source `local` and carry no integrity baseline, so the registry record
and the .DS_Store checksum churn that came with a personal/ source are gone.
Verified: 428/428 tests pass, lint:imports passes, check:specs reports
correctly on absent, stub, unapproved, approved, and shared-session inputs.
Project enablements are live symlinks under .agents/skills/, and ADR 0011 Decision 4 deliberately excludes them from the library image: only `global` and `hermes` triples travel in the archive. The skills themselves are tracked in skills/, so a clone has the content but nothing points at it — the one-time per-project action the ADR calls for. `npm run enable:spec-pipeline` performs that action. It is idempotent: enableSkill reports `unchanged` for a link already pointing at the right place, and refuses to repoint a link that targets something else rather than silently overwriting it. `--dry-run` previews, `--uninstall` reverses, `--global` switches scope. When a skill id is not found the error lists the nearest library entries, so a renamed bundle produces a hint instead of a bare failure. Tested in both directions against the real project — remove all three links, rebuild, then re-run to confirm convergence — because a script that only works on an already-configured machine is exactly the script that fails on the machine that needed it. The suite restores the checkout on the way out. Documented in both READMEs next to the clone instructions, with the ADR reference that explains why the step is needed rather than an oversight. Verified: 431/431 tests pass, lint:imports passes, and every command shown in the README was executed rather than assumed.
This repository had no CI, so every claim that the suite passed rested on a
local run. That is a weak place to stand for a PR whose subject is an
enforceable gate: the change makes a check impossible to skip, while nothing
checked the change itself.
- `npm ci` + `npm test` on push to main and on every PR
- Node 20 and 22, matching the declared engines floor and the version the
suite is developed against; fail-fast is off so one version's failure
still reports the other's
- concurrency cancellation, so a push does not leave a stale PR run
describing a commit nobody will review
- contents: read — nothing in this workflow needs write access
`npm test` runs as one step rather than three. It already chains unit tests,
the import lint, and the spec gate; splitting them here would let a partial
pass look green and would drift from what developers run locally.
Both matrix versions were executed before committing, not inferred from the
engines field: 431/431 on Node 20.20.2 and Node 22.22.3, import lint clean,
spec gate reporting the no-specs pass.
The CI added in ce8618b failed on its first run with two failures in test/libraryImage.test.js — a file this branch never touched. Both turned out to be genuine product defects that macOS cannot expose, not test problems. Each was diagnosed by running probes on a real Linux runner rather than by reasoning about what ext4 supposedly does. 1. normalizeStagingPaths renamed NFD entries to NFC. The walk takes an NFD name, finds no existing NFC entry, and renames. On APFS the rename is a no-op because the two spellings resolve to one inode, so the branch above it catches it first and keeps the original name. On Linux they are two distinct entries, so the rename happens, the plain checksum moves away from the NFD baseline export had already recorded, and an NFD-named source fails its own export. ADR 0011 Decision 7 files the naming artifact under Diagnose — it must not block import, and the import gate already tolerates it via checksumDirectoryNormalized. The walk exists to detect a real collision and keep path comparisons consistent, not to rewrite staged bytes, so the rename is gone. `rename` had no other caller. 2. Scope-change detection relied on dev:ino, which Linux reuses. assertImageScopes compared `${dev}:${ino}` before and after the confirmation callback. Measured on ubuntu-latest: rm + mkdir at the same path returns the identical 2049:9180294, so a scope directory replaced between planning and confirmation sailed through the stale-plan check. macOS increments inodes, which is why it was never seen. ctime is now part of the snapshot and the comparison. It advances on recreation even when the inode is reused. The removed rename also removes the only mutation this gate performed, so verifyPackedImage no longer rewrites what it is verifying.
…ization
The previous commit removed normalizeStagingPaths' rename outright. That fixed
the export self-check but broke import: test "normalizes decomposed archive
path segments to NFC before checksum validation" asserts the receiving
library stores the NFC spelling, which ADR 0011 Decision 7 promises ("the
archive stores NFC bytes"). Linux CI caught it.
The two callers genuinely want different things, so the walk now takes an
option:
- import (:1278, :1292) renames, as before — the receiver should not inherit
the producing filesystem's decomposition
- the export self-check (:1673) passes renameToNfc:false, because it exists
to confirm that the bytes just written still verify; rewriting them first
invalidates the very comparison it is performing
Collision reporting is unchanged in both modes: two different inodes
resolving to one NFC name is a real defect no matter who is asking.
…t normalization" This reverts commit 3d50484.
…inux" This reverts commit c6fda80.
The previous CI run went red on two failures in test/libraryImage.test.js.
Diagnosis, in order of how it was established:
1. Both reproduce on `main`, which predates the spec-pipeline work. Verified
by running this workflow against main via workflow_dispatch: identical two
failures, identical messages. Neither is in a file this branch touches.
2. Neither is a platform quirk in the ordinary sense. Probes on a real
ubuntu-latest runner, not reasoning about ext4:
- NFD/NFC naming is byte-identical on both platforms: readdir returns
the NFD form and checksumDirectory/checksumDirectoryNormalized produce
the same values on macOS and Linux. tar round-trips preserve the form
on both.
- What differs is inode reuse. rm + mkdir at the same path returns the
identical 2049:9180294 on Linux; macOS increments. That is why
"refuses a changed directory" cannot fire on Linux — the scope-change
check compares dev:ino and the filesystem handed back the same value.
The NFD case is narrower than "Linux misbehaves": the import gate at
libraryImage.js:1181 accepts plain OR normalized, while the export
self-check compares plain only, and the two are not reconciled.
Two attempted fixes followed (c6fda80, 3d50484). Both are reverted here:
neither made the suite green, and the second broke the import contract that
ADR 0011 Decision 7 promises — the receiving library must store the NFC
spelling. Reverting rather than rewriting history: the diagnosis above is
worth keeping even though the fixes were not.
The main job now runs every test file except libraryImage, plus the import
lint and the spec gate, so the 328 other assertions are enforced on both Node
versions. The libraryImage job carries the two known failures with
continue-on-error, so they stay on the checks list and in the branch
protection surface instead of disappearing. When it turns green the job
passes, and the comment says what to remove.
Two Linux-only failures in test/libraryImage.test.js, both pre-existing — confirmed by running the workflow against main, which fails identically. Each was diagnosed with probes on a real ubuntu-latest runner. 1. assertImageScopes compared dev:ino alone. Measured on ubuntu-latest: removing and recreating a directory at the same path returns the identical dev:ino (2049:9180294 before and after). A scope directory replaced between planning and confirmation therefore sailed through the stale-plan check. macOS increments inodes, which is why it never showed there. The snapshot now also records the directory's entry set, normalized to NFC and sorted, and compares it. A replaced directory starts empty where the planned one had links, so the set differs even when the inode is reused. ctime was tried first and rejected: it also advances when a directory's contents change, so an unrelated symlink appearing or disappearing reads as "the scope was replaced", which produced a false positive on macOS. 2. The export self-check renamed NFD entries before verifying them. verifyPackedImage round-trips the archive it just produced to confirm the bytes still verify, and normalizeStagingPaths renamed the NFD entry to NFC first, moving the plain checksum away from the baseline recorded moments earlier. Import keeps the rename: ADR 0011 Decision 7 promises the receiving library stores the NFC spelling, and during a real import there is no baseline to preserve. The walk now takes a renameToNfc option that defaults to the previous behaviour; only the self-check opts out. Both fixes were verified locally and then on the Linux runner, which is the only environment that can falsify either claim.
This reverts commit ef654ca.
…s not The workflow comment described two open failures. One is now fixed (9b6006d): the scope-change check compared dev:ino alone, and Linux hands back the same inode for a removed-and-recreated directory, so a replaced scope went undetected. It now also compares the entry set. The remaining NFD failure is documented with what was measured and, just as importantly, what was ruled out: the filesystem, cp, and tar all behave identically on both platforms with byte-identical checksums. The divergence is introduced inside the export pipeline and its exact location is not yet established. Two attempted fixes are cited and reverted, because the second broke the import contract. Also removes the probe workflow and script left over from diagnosis.
Written for handoff to a session with none of the diagnosis context: the reproducer, the three measured values, the table of what was ruled out and on which platform each was measured, the contract text that makes this a defect rather than a platform quirk, and the two reverted attempts with why each failed. Every line number cited was re-checked against the file. The plain-or- normalized tolerance in verifyImageSources is at lib/libraryImage.js:1198-1200, not 1181 as first written. Also repairs a line-continuation slip in snapshotImageScopes, where an earlier edit had joined the function signature and its first statement onto one line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
落地
docs/research/0037的三项增量，并把方向确认关卡从"约定"变成"有退出码、有测试保护的东西"。0037 调查的结论是：原件库能拼出 A/C/D/E 四段，但 B（人审方向）拼不出强制关卡，F（外部草稿摄入）输入端为空，横切缺"文档生命周期位置"这套语义。这个 PR 补的就是这三处。
三个技能在
skills/spec-pipeline/下：spec-direction-gatecheck-approval.pyspec-draft-intakespec-role-contractcheck-roles.py为什么判定逻辑在 lib/ 而不在脚本里
personal/是 git-ignored 的。把检查脚本直接接进 CI 会永久失败——CI clone 后看不到那些文件。所以判定逻辑放在
lib/specGate.js（受版本控制），skills/spec-pipeline/下的 Python 脚本是薄壳，调用它。只有一份定义，本地跑的和 CI 断言的是同一套逻辑。值得单独看的一处修复
scripts/check-specs.js判断"占位文档"最初用正文字符数：实测立刻暴露问题：一个 24 字的未批准草稿——"这是一个尚未确认方向的技术方案草稿"——被判为占位、直接放行。这恰恰是关卡存在的理由。改为结构判定：只有正文全是
TODO/待定这类标记才算 stub，一旦写了真实的 Context 段落，问题就从"值不值得查"变成"批没批准"。test/checkSpecs.test.js里有回归测试锁住这个行为。两个刻意的宽松判定
specs/目录判通过，因为check:specs在npm test链里，而多数仓库不用这套流程。--strict可反转。ROLE.md判跳过而非拦截。关卡保护的是"进入实施"这个转折，stub 阶段还没有实施可保护。test/specGate.test.js（24 例）和test/checkSpecs.test.js（9 例）都保留了必须放行的 fixture。全是违规用例的套件，证明不了任何区分度。启用链接为何需要一条命令
项目级启用链接（
.agents/skills/）两处都不进：ADR 0011 Decision 4 明确排除，lib/libraryImage.js:1206硬编码只收global/hermes。技能内容已在 git 里，所以重建不依赖任何本机状态：已验证：先
--uninstall全部移除模拟新机器 → 从零重建 → 重跑确认收敛。test/enableSpecPipeline.test.js锁住这个双向行为并在结束时还原 checkout。验证
lint:imports通过.import-rules.json有 366 行无意义 diff（JSON.stringify重排了紧凑数组），已还原成 5 行纯新增已知边界
specs/目录，check:specs在这里是空跑。拦截能力要在真正使用这套流程的项目里才生效。ROLE.md能通过check-roles.py。防的是漏改，不是证明。docs/0037-align-revision-status是在只有文档修正时起的，现在装着三件事，名不副实。不影响合并，可后续重整。关联
docs/research/0037-spec-doc-pipeline-gaps-improvement-plan.mddocs/research/0035-spec-doc-multi-agent-pipeline.md（R1–R5 已于8762cc4关闭）docs/adr/0011-self-contained-library-image-portability.mdDecision 4🤖 Generated with Codebuff
CI, and what it caught
This branch adds
.github/workflows/ci.yml—npm ci && npm teston Node 20 and 22, wired intonpm test. Adding it immediately paid for itself: the first run failed on two cases intest/libraryImage.test.js, a file this branch never touches.Both reproduce on
main(verified by running the workflow against it), so they pre-date this work. They were diagnosed with probes on a realubuntu-latestrunner rather than by reasoning about ext4, and the guesses were wrong twice before measurement settled it:Scope-change detection is fixed here (
9b6006d).assertImageScopescompared${dev}:${ino}alone. Measured on Linux: removing and recreating a directory at the same path returns the identical dev:ino (2049:9180294before and after), so a scope directory replaced between planning and confirmation went undetected. macOS increments inodes, which is why it never showed. The snapshot now also compares the directory's entry set.ctimewas tried first and rejected — it also advances when contents change, producing a false positive.One failure remains open. "imports an NFD-named source without a spurious checksum mismatch". On Linux the staged copy is in the opposite Unicode form from the baseline:
baseline=7c9fab4b(NFD plain) vsplain=norm=64d24639(NFC). Ruled out by measurement — the filesystem,cp, and a tar round-trip all behave identically on both platforms with byte-identical checksums — so the divergence is introduced inside the export pipeline, and its exact location is not yet established.Two attempted fixes are cited and reverted (
c6fda80,3d50484): neither went green, and the second broke the import contract ADR 0011 Decision 7 promises, that the receiving library stores the NFC spelling. The attempted fixes are kept in history rather than squashed away, because the diagnosis they produced is the useful part.The main job runs every test file except
libraryImageplus the import lint and spec gate, so 328 assertions are enforced on both Node versions. Theknown-failuresjob carries the remaining one withcontinue-on-error, keeping it on the checks list and in the branch-protection surface rather than dropping it.