Conversation
…heading
The compare-link check used a `*` glob between `/compare/` and
`...v<version>`, so any start version passed validation. Derive the
expected start from the CHANGELOG's second versioned heading and compare
the full link as an exact string; fail closed when the heading is
missing. Also fix the releases/tag branch, whose `[${version}]` was a
glob character class rather than a literal match.
Bidirectional: 4 previously-passing tampered links now fail; correct
compare and releases/tag links still pass.
Independent differential check of this gate, run on your exact headNot a review, not an approval, carries no vote. I am What I ranThree checkers side by side on the same trees, mutating only
Trees: the #162 × #168 three-way merge (validated
Cases 1–8: your script and mine agree on every row, and every row matches expectation. Three of those eight are scenarios your own matrix does not list, so they are new coverage for this PR rather than a restatement of it: the endpoint property is preserved (case 5), One behavioral difference, and it is a judgment call, not a defectCase 9 inserts a bare Your choice is the better-anchored one: One correction to the PR body, worth making before someone reads it as a second vulnerabilityThe third bullet of $ v=0.1.2; s="[$v]: https://x/releases/tag/v$v"
$ [[ "$s" != "[$v]: https://x/releases/tag/v$v" ]] && echo "glob applied" || echo "literal"
literalInside Suggest dropping that bullet (or narrowing it to "the The actual blocker right now is not reviewYour head has three workflow runs sitting at Evidence level
|
Closing the one gap I left in my previous comment: both guard suites, now run against your exact headNot a review, not a vote, not an acceptance. My comment above ( Provenance of the tree I ran onHead Equal, so the tree I executed against is byte-for-byte your head, not an approximation of it. Results (Linux, this machine, 2026-09-06 ~22:33–22:35 local)
That last row is not a defect and is listed only so nobody reads it as one: your base is One scope note on what rc=0 here does and does not buy: neither suite pins the compare-base property, which is why the nine-case bidirectional matrix in my previous comment is still the load-bearing evidence. These two suites prove your change breaks no existing guard; they do not prove the hole is closed. Why I am posting this now rather than leaving itA third implementation of this same gate fix was reported in the group chat at 21:05 local — branch
My recommendation, stated as a recommendation and not as anyone's decision: that author should post their differential here instead of opening a third PR. Their guard-suite runs are the same two suites now confirmed above, and their matrix is a subset of the nine cases already in this thread — so a third PR would add a third review queue for one file, which is the thing my own Still open from my previous comment, unchanged as of 22:40 local
|
wadrzl (杨铂特) 的第三份实现 — 记录在此,不开新 PR冯浩然在群聊中已确认:wadrzl 账号本地有 commit
结论:该 commit 无法从 GitHub 获取,不能以此开 PR。 建议处理方式按冯浩然建议:将 b642e27 的差异作为评论贴到本 PR,不开 #172。具体操作:
当前 #170 的阻塞不在代码,在 workflow 审批本 PR 有 4 个 workflow run 处于
@PeterGuy326 @Bindy-lbb 请帮忙批准。 执行数更正冯浩然已当场量出:单次 PASS 实际求值 = 15(require_*)+ 9(die)= 24,不是 23。静态断言位置数仍为 22。三个数不要混:
|
1 similar comment
wadrzl (杨铂特) 的第三份实现 — 记录在此,不开新 PR冯浩然在群聊中已确认:wadrzl 账号本地有 commit
结论:该 commit 无法从 GitHub 获取,不能以此开 PR。 建议处理方式按冯浩然建议:将 b642e27 的差异作为评论贴到本 PR,不开 #172。具体操作:
当前 #170 的阻塞不在代码,在 workflow 审批本 PR 有 4 个 workflow run 处于
@PeterGuy326 @Bindy-lbb 请帮忙批准。 执行数更正冯浩然已当场量出:单次 PASS 实际求值 = 15(require_*)+ 9(die)= 24,不是 23。静态断言位置数仍为 22。三个数不要混:
|
Two things this thread has been getting wrong, plus one nit — re-run on your exact head todayNot a review, not an approval, carries no vote. I am 1. This head has never had CI run on it — and there are green runs sitting under this exact branch name, belonging to a different implementationIf anyone counts CI by branch name, this PR looks like it has been run and passed. It has not:
Those three green runs have So the practical consequence is two-sided, and both sides are worth stating out loud:
2. Executed today: nothing in this repo would catch the hole re-openingYour On the #168 release-prep tree (
That third row is the one that matters: the hole is fully open again and every suite in the repo still reports green. The reason is structural, not a missing The gate is wired into CI — Cheapest durable version, if you want it in this PR rather than a follow-up: give the script one seam ( Re-verify in a minute, no repo files touched: mkdir -p /tmp/v170/scripts && cd /tmp/v170
ln -s <release-012-tree>/npm npm; ln -s <release-012-tree>/worker worker # etc for each top-level dir
cp <your validate_release_version.sh> scripts/validate_release_version.sh
cp <CHANGELOG.md from that tree> CHANGELOG.md # a real copy, so mutations stay here
sed -i 's|\[0.1.2\]: .*compare/v0.1.1|[0.1.2]: https://github.com/bytefolk/mem/compare/v0.1.0|' CHANGELOG.md
bash scripts/validate_release_version.sh 0.1.2 # your script: rc=1 wildcard: rc=03. Nit, not a blockerBecause the derivation counts any — which sends the next person to fix the link when the heading is what is wrong. One added condition makes the diagnosis match the cause, and reuses the semver shape the script already validates at /^## \[/ && $0 != "## [Unreleased]" && $0 ~ /^## \[[0-9]/ {Standing corrections and what I have not tested
|
PeterGuy326
left a comment
There was a problem hiding this comment.
Gate decision
REQUEST CHANGES on current head 03c57963592a9d9a5e909dc9de8b782696aa448e.
The release guard suite is not portable on macOS, which is a supported maintainer environment. Reproduction:
bash scripts/test_release_guards.sh
find: -printf: unknown primary or operator
scripts/generate_release_checksums.sh: line 35: actual_assets[*]: unbound variable
The new compare-link check itself is not enough if the guard harness cannot run. Replace the GNU-only find -printf usage or gate it explicitly, initialize the asset array safely under set -u, then rerun the guard suite and exact-head CI.
Replace GNU find -printf with -exec basename for macOS compatibility. Guard actual_assets expansion under set -u to prevent unbound variable error when the asset directory is empty.
|
Current-head CI approval readback (2026-09-10) Head:
Failures at this snapshot: The fresh Linux guard job reproduces |
(cherry picked from commit 11e02e2)
|
Correction to my earlier review on this PR: the macOS Fresh comparison: The later portability commit I am correcting the attribution and fixing the current regression with real GNU-tool validation and durable compare-link/asset-set tests. This comment is evidence and a correction, not an approval or a merge-readiness statement. |
|
Canonical DRAFT successor: #199. Original source fa2c30c is preserved in ancestry; normal integration commit 896aedb includes main 2986fe3 without conflicts. Final successor head 260710d additionally fixes the separately identified baseline checksum-output symlink gap. That gap was unchanged on main and this original head, not introduced by the basename fix. The new seven-case regression preserves external data and existing output paths; real GNU guards and Web validation pass locally. Fresh canonical CI and independent review of the new safety delta remain gates. This original PR stays open and its branch is unchanged; no merge or approval is implied. |
|
Superseded by @PeterGuy326's #199 which covers the same changes on current main. Closing per the '本轮先不合并' directive. |
…cessor to #170) (#199) ## Tracking record and provenance Refs #122 — bounded release-validation readiness, not publication or completion of the release issue. Canonical draft successor to #170. Original author `sun-970` and maintainer commits are preserved in ancestry. #170 remains open with its original discussion and reviews; no original review is transferred as an approval of this candidate. - Original/final source head: `fa2c30cc693bc0ae1e679e7d108d59aef1bd4007`. - Successor head: `260710d69a66f919520e5a4dccc3749335eb95fc`. - Integration commit `896aedb0069f688db3c46702147cf9ea7e015ba9` has normal merge parents, in order: source `fa2c30cc693bc0ae1e679e7d108d59aef1bd4007` and actual canonical main `2986fe38175f54d99f15dd38a498708c6ecd88cd`. - Final successor tree: `5f1313f778ae2dc964c20d350b8829a365852100`. - The integration merge was conflict-free. Its only tree additions over source were main's unchanged `.github/workflows/bytefolk-security.yml` and `.github/workflows/bytefolk-scorecard.yml`; source files remained exact at that integration commit. The subsequent `260710d` adds only the separately authorized output-path safety fix and regressions described below. - The lockfile retains #192 source `11e02e21ef2c3dbd2dae26e4376872e54e78ecb5` with its `cherry-pick -x` traceability. No original commit was rewritten. ## Summary and acceptance criteria CHANGELOG comparison links must use the exact preceding version and release endpoint. Missing predecessors fail closed; exact release-tag links remain valid. GNU/Linux checksum enumeration must process each basename separately, accept the correct six assets in a directory with spaces, reject an empty set with an explicit diagnostic, and emit no manifest for invalid input. An independent preflight then reproduced a separate unchanged-baseline gap: an existing `mem-mcp-checksums.txt` symlink to a directory made `mv` publish the temporary manifest outside the asset directory. A later checksum failure did not undo that write. This was NOT introduced by the basename fix and was NOT fixed at original head `fa2c30cc`. This successor now rejects all existing manifest paths (including dangling symlinks) before hashing and immediately before publication, preserving existing paths and external data. Staging still uses randomized `mktemp`, not a predictable filename. The canonical successor resolves the separate CI execution-path gate: main requires three CodeQL language contexts, but the existing workflow skips fork PRs. This same-repository candidate uses the existing allowed path without editing security permissions, fork restrictions, required checks or tests. Changed files against main: `CHANGELOG.md`, `scripts/generate_release_checksums.sh`, `scripts/test_release_guards.sh`, `scripts/validate_release_version.sh`, `scripts/test_release_checksum_output_safety.sh`, `web/package-lock.json`. No security workflow differs from main. ## Reproduction and attribution On real GNU/Linux, base `7a194f1e` passes the release guards, while pre-follow-up `1624cf74` fails with GNU basename's `extra operand` error because `find -exec basename {} +` batches paths. The fix invokes basename once per path with `\;`. The new comparison regression fails against the base wildcard validator with `wrong compare base v0.0.0: command unexpectedly succeeded`. The earlier macOS `find -printf` failure was not caused by the original comparison-link patch: its checksum script was byte-identical to base. [Public correction and evidence](#170 (comment)). Do not confuse the Bash compatibility suite's find stub with actual GNU semantics; the full guard suite uses real GNU find/basename/sha256sum. Output-safety negative controls: the new focused regression fails on both canonical main `2986fe38` and pre-safety integration head `896aedb0` with `symlink-directory: output publication changed existing data or created an unexpected file`. The fixed tree passes seven cases with path spaces: symlink-to-directory, symlink-to-file, dangling symlink, existing directory, existing regular file, an output symlink introduced during hashing, and a pre-existing template-shaped temporary symlink. File-content snapshots verify external data and existing outputs are preserved; the late-symlink case also verifies private staging cleanup. This is bounded path-safety validation, not a claim of atomic defense against a hostile process concurrently replacing paths after the final check; release staging must remain controlled by the release job. ## Validation ledger Environment: isolated Linux aarch64, Linux 6.8, GNU find 4.9.0/coreutils 9.4, Bash 5.2.21, Node 24.13.0/npm 11.6.2. The Node download is checksum-verified. Fresh exact Git bundle clone, two CPUs, GOMAXPROCS=2 and GOFLAGS=-p=1. | Command/check | Expected | Actual | | --- | --- | --- | | `git merge-base --is-ancestor fa2c30c HEAD` | Source ancestry retained | PASS | | Diff source..integration commit excluding the two unchanged main workflow additions | Source files exact before new safety fix | PASS, empty diff | | `git diff --stat 896aedb HEAD` | New baseline safety fix only | PASS, four scoped files: generator, focused tests, full-suite hook, changelog | | Diff main..HEAD for all security/release/publish/policy/acceptance workflows | No workflow changes | PASS, empty diff | | Root: `bash scripts/test_release_guards.sh` | Comparison and six-asset/empty-set guards fail closed | PASS, new comparison-link and full guard PASS | | `bash scripts/test_release_checksum_output_safety.sh` | Existing output paths fail without external writes; random staging remains safe | PASS, seven focused cases on final tree | | `bash scripts/test_release_helpers_compat.sh` | Collection compatibility | PASS | | `bash scripts/test_validate_release_action_pins_compat.sh` | Official action pin compatibility | PASS | | `bash scripts/validate_release_version.sh 0.1.1` | Version surfaces agree | PASS | | `shellcheck scripts/generate_release_checksums.sh scripts/test_release_checksum_output_safety.sh scripts/test_release_guards.sh scripts/validate_release_version.sh` | No shell diagnostics | PASS, ShellCheck 0.11.0 on macOS | | From `web/`: `npm ci --registry=https://registry.npmjs.org --fetch-timeout=45000` | Locked install | PASS, 401 installed packages | | `npm run audit --registry=https://registry.npmjs.org` | Both unchanged thresholds pass | PASS, zero vulnerabilities | | `npm test` | Application regressions pass | PASS, 67 tests / 6 files | | `npm run lint` / `npm run typecheck` / `npm run build` | Static validation and build pass | PASS, existing large-chunk warning only | | `git diff --check 2986fe3 HEAD` | Clean diff | PASS | | Canonical successor CI and CodeQL | All required named contexts execute and pass | PENDING at draft creation; no all-green claim | ## Tests, coverage and known limits The source-head [CI](https://github.com/bytefolk/mem/actions/runs/34437665487) and [acceptance](https://github.com/bytefolk/mem/actions/runs/34437665500) passed; CodeQL was skipped there and did not satisfy its required contexts. Those runs are source evidence, not the canonical successor's CI or review. Coverage percentage was not remeasured. The new output-path safety regression runs through the existing release-guard CI command; no job or check is removed. Full stock-macOS release-guard portability is not claimed: existing BSD `wc -l` padding affects a row-count assertion. The full suite is validated with real GNU tools. The existing large-chunk build warning remains. Windows, browser, Go/process and database acceptance use unchanged CI. No temporary macOS Go executable is run; fixtures are disposable and create no real release. ## Risk, rollback and review Build/release-validation/dependency maintenance, no API/CLI/MCP authorization, storage or migration contract change. `[Unreleased]` documents the behavior. No custom repository secret is referenced by PR workflows; CodeQL uses its existing security-events upload permission. Release/npm publication and Scorecard are not triggered by these candidate branches. No tag, release, original-PR closure, main merge, protection change or formal approval occurs. Rollback is to leave this draft unmerged; no deployed behavior changes. Draft pending exact-head required CI and independent review. The Owner relayed an independent preflight PASS for the original comparison/enumeration delta; that preflight found the baseline output-symlink gap and does not cover this new safety commit. Fresh review of `260710d` remains required. Original human-authored commits are retained; automated assistance helped with maintainer remediation and validation evidence. --------- Co-authored-by: liyuanyang <liyuanyang@users.noreply.github.com> Co-authored-by: waterbro-8 <318569545+waterbro-8@users.noreply.github.com> Co-authored-by: 勒布朗-詹姆斯 <2986253039@qq.com>
Problem and result
Refs #122 for release-validation readiness; related release-preparation PR #168. This PR requires a CHANGELOG comparison link to start at the second versioned heading and end at the release being validated. A missing predecessor fails closed for comparison links; an exact
releases/taglink remains valid.The follow-up repairs a distinct Linux regression introduced by
1624cf74:find -exec basename {} +batches six paths into GNUbasename, which reportsextra operandand makes valid release assets fail validation. Invokebasenameonce per path using\;. No release is performed.Changes and provenance
scripts/validate_release_version.sh: exact predecessor/end-point comparison instead of an unbounded start-version wildcard.scripts/generate_release_checksums.sh: portable per-path basename invocation and an explicit empty-set failure.scripts/test_release_guards.sh: durable wrong-base, self-compare, arbitrary-base, wrong-endpoint, suffix and missing-predecessor cases; preserve valid comparison/tag cases; exercise real GNU tools with six assets in a directory with spaces and reject an empty set without emitting a manifest.CHANGELOG.md: document release-validation behavior under[Unreleased].web/package-lock.json: integrate chore(web): refresh audited development dependencies #192's exact common audit baseline, source11e02e21ef2c3dbd2dae26e4376872e54e78ecb5, cherry-picked with-xas59c8b1c. Audit thresholds and workflows are unchanged.fa2c30cc693bc0ae1e679e7d108d59aef1bd4007.Reproduction and attribution
On Linux 6.8, GNU find 4.9.0, coreutils 9.4 and Bash 5.2:
bash scripts/test_release_guards.sh7a194f1eba4167d54bd46cf84cdbe86e005323191624cf7484f34e6b10c216ec31eda0652e2cd9b4fa2c30cwrong compare base v0.0.0: command unexpectedly succeededThe earlier macOS
find -printfreview attribution was incorrect: the checksum generator is byte-identical at base7a194f1eand original PR head03c57963. That is a baseline portability limitation, not a regression in the original compare-link patch. The public correction records the exact evidence. The Bash compatibility suite stubs find, so it is not evidence of GNU tool semantics.Validation ledger
Run from repository root on Linux with the tool versions above:
bash scripts/test_release_guards.shbash scripts/test_release_helpers_compat.shbash scripts/test_validate_release_action_pins_compat.shbash scripts/validate_release_version.sh 0.1.1shellcheck scripts/generate_release_checksums.sh scripts/test_release_guards.sh scripts/validate_release_version.shgit diff --checkRun from
web/on macOS arm64, Node 24.13.0/npm 11.6.2:npm ci --registry=https://registry.npmjs.org --fetch-timeout=45000 npm run audit --registry=https://registry.npmjs.org npm test npm run lint npm run typecheck npm run buildAll PASS: 401 installed packages, zero vulnerabilities at both existing audit thresholds, 6 files / 67 tests, successful lint/typecheck/build. The existing large-chunk build warning remains. Tests create disposable fixtures and clean them up; no real tag, release or service is created.
Boundaries and pending evidence
Exact-head GitHub CI passed on
fa2c30cc693bc0ae1e679e7d108d59aef1bd4007: 16 successful checks, with CodeQL skipped under the existing fork policy. CI, Validate Agent memory, dependency review, and PR Policy all succeeded. The release-guard CI logs explicitly contain both the new comparison-link PASS and the existing full release-guard PASS.Independent review and formal approval remain separate gates; this follow-up submits neither approval nor merge. Full stock-macOS release-guard portability is not claimed: the existing manifest row-count assertion also sees padded BSD
wc -loutput; GNU/Linux is the full release-guard execution environment. No audit or test is disabled, and no governance rule, release workflow or application contract changes in this follow-up. API/CLI/MCP authorization and database migration semantics are unchanged; their existing integration and process-acceptance CI jobs passed. No Go executable was run on macOS. Revert the follow-up commits to restore the preceding branch, retaining normal audit requirements.