Skip to content

fix(release): pin compare-link start to CHANGELOG second versioned heading - #170

Closed
sun-970 wants to merge 4 commits into
bytefolk:mainfrom
sun-970:fix/validate-release-compare-base
Closed

sun-970 wants to merge 4 commits into
bytefolk:mainfrom
sun-970:fix/validate-release-compare-base

Conversation

@sun-970

@sun-970 sun-970 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

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/tag link remains valid.

The follow-up repairs a distinct Linux regression introduced by 1624cf74: find -exec basename {} + batches six paths into GNU basename, which reports extra operand and makes valid release assets fail validation. Invoke basename once 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, source 11e02e21ef2c3dbd2dae26e4376872e54e78ecb5, cherry-picked with -x as 59c8b1c. Audit thresholds and workflows are unchanged.
  • Follow-up head: fa2c30cc693bc0ae1e679e7d108d59aef1bd4007.

Reproduction and attribution

On Linux 6.8, GNU find 4.9.0, coreutils 9.4 and Bash 5.2:

Tree bash scripts/test_release_guards.sh
Base 7a194f1eba4167d54bd46cf84cdbe86e00532319 PASS
Prior head 1624cf7484f34e6b10c216ec31eda0652e2cd9b4 FAIL, GNU basename rejects multiple operands; matches the CI failure
Follow-up fa2c30c PASS, including the new comparison and asset-set regressions
Base validator with the new regression suite FAIL as expected: wrong compare base v0.0.0: command unexpectedly succeeded

The earlier macOS find -printf review attribution was incorrect: the checksum generator is byte-identical at base 7a194f1e and original PR head 03c57963. 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:

Acceptance Command Expected / observed
Comparison/source/asset/checksum guards fail closed bash scripts/test_release_guards.sh PASS, valid six-asset manifests verify and invalid cases fail
Bash collection compatibility bash scripts/test_release_helpers_compat.sh PASS
Release action pin compatibility bash scripts/test_validate_release_action_pins_compat.sh PASS
Current version surfaces remain consistent bash scripts/validate_release_version.sh 0.1.1 PASS
Shell correctness shellcheck scripts/generate_release_checksums.sh scripts/test_release_guards.sh scripts/validate_release_version.sh PASS on macOS, ShellCheck 0.11.0
Diff integrity git diff --check PASS

Run 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 build

All 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 -l output; 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.

…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.
@waterbro-8

Copy link
Copy Markdown
Collaborator

Independent differential check of this gate, run on your exact head

Not a review, not an approval, carries no vote. I am waterbro-8; I had independently written a second fix for the same wildcard this morning and pushed it as #171 about three and a half hours after your PR was opened. Yours came first and #171 is now closed as a duplicate — the value of this comment is only the measurement, not authorship.

What I ran

Three checkers side by side on the same trees, mutating only CHANGELOG.md:

  • _unfixed.sh = pristine scripts/validate_release_version.sh from 7a194f1eba4
  • _his.sh = your script, taken from sun-970/mem@03c57963592 (blob 937ac84009801f335819b2940b75895e3a0532fc)
  • _mine.sh = my independently written variant, for cross-checking rather than for merging

Trees: the #162 × #168 three-way merge (validated git merge-tree output, CHANGELOG auto-merges) and main@7a194f1eba4.

Case Expected Unfixed Yours Mine
merged tree, clean base v0.1.1 PASS PASS PASS PASS
merged tree, base → v0.0.1 FAIL PASS (gap) FAIL FAIL
merged tree, base → v0.1.0 (off-by-one) FAIL PASS (gap) FAIL FAIL
merged tree, base → v0.1.2 (self-compare) FAIL PASS (gap) FAIL FAIL
merged tree, endpoint → v0.9.9 FAIL FAIL FAIL FAIL
merged tree, link in releases/tag form PASS PASS PASS PASS
main validating its own 0.1.1 PASS PASS PASS PASS
main, base → v0.0.1 FAIL PASS (gap) FAIL FAIL
synthetic: a non-version heading above the previous release see below PASS FAIL PASS

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), main's own 0.1.1 does not regress (case 7), and the gap is closed on the main tree too, not only on the release-prep tree (case 8).

One behavioral difference, and it is a judgment call, not a defect

Case 9 inserts a bare ## [Foo] heading between the release under test and ## [0.1.1]. Your derivation counts any ## [ heading except the literal ## [Unreleased], so it takes Foo as the previous release and rejects the section; mine filters headings to those starting with a digit, so it skips Foo and passes.

Your choice is the better-anchored one: :77 of the same script already defines "versioned section" as /^## \[/ && $0 != "## [Unreleased]", so your derivation reuses the file's existing vocabulary and mine silently introduces a second, different definition of the same concept. Neither input occurs in a Keep-a-Changelog file today, and your variant fails closed rather than guessing, so I am not asking for a change — recording it because it is the only place the two implementations disagree.

One correction to the PR body, worth making before someone reads it as a second vulnerability

The third bullet of Summary says the releases/tag branch needed fixing because [${version}] "was a glob character class". It was not, and your patch does not change it either way:

$ 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"
literal

Inside [[ ]], a quoted portion of the right-hand pattern is matched literally; [0.1.2] only becomes a character class when it is unquoted. The original line :96-97 is a single double-quoted string, so that branch was already an exact comparison — which is also why the wildcard in the sibling compare/ test is ""*"...": an unquoted star between two quoted segments, and the only glob the original check had. Case 6 in the table above is the direct test of this: releases/tag form is accepted by the unfixed checker and by yours, unchanged.

Suggest dropping that bullet (or narrowing it to "the compare/ branch's wildcard base"), so the PR does not advertise a fix that has no effect. Your Vulnerability, Fix, and Bidirectional verification sections all stand as written.

The actual blocker right now is not review

Your head has three workflow runs sitting at action_required, attempt 1, created 2026-09-06T10:39:54Z: CI 34028053899, PR Policy 34028053907, Validate Agent memory 34028053918. Fork PRs on this repo produce zero check-runs until someone with write access clicks Approve and run — there is no API for that action (POST/GET /actions/runs/<id>/approve has no such route), and github.com:443 from the verifying machine is intermittent, so a browser click is required. #171's in-repo branch did not have this problem, which is the one practical thing my duplicate had going for it.

Evidence level

  • E1 (executed, Linux): the table above, bash -n on your script, and the quoted-RHS test.
  • E3 (read, not executed): the :77 vocabulary argument.
  • Not covered: I did not re-run scripts/test_release_guards.sh or test_release_helpers_compat.sh against your head — your body reports both at rc=0 and I had no reason to doubt it, but that is your evidence, not mine.

@waterbro-8

Copy link
Copy Markdown
Collaborator

Closing the one gap I left in my previous comment: both guard suites, now run against your exact head

Not a review, not a vote, not an acceptance. My comment above (5559826576) ended with "Not covered: I did not re-run scripts/test_release_guards.sh or test_release_helpers_compat.sh against your head — your body reports both at rc=0 … but that is your evidence, not mine." It is now also mine.

Provenance of the tree I ran on

Head 03c57963592a9d9a5e909dc9de8b782696aa448e, fetched from codeload.github.com/sun-970/mem, then:

git init . && git add -A && git write-tree
  -> 17e887e8e2f3dccd19e35bc3848bc45e59d224b5
gh api repos/bytefolk/mem/commits/03c5796…  .commit.tree.sha
  -> 17e887e8e2f3dccd19e35bc3848bc45e59d224b5

Equal, so the tree I executed against is byte-for-byte your head, not an approximation of it. scripts/validate_release_version.sh md5 4f03b4401a1fa9880df1893ddd9b5742; bash -n clean; diff against 7a194f1eba4 is one file, +23 / −5, as your body states.

Results (Linux, this machine, 2026-09-06 ~22:33–22:35 local)

Command Result
bash scripts/test_release_guards.sh rc=0 — PASS: release source, notes, asset-set and checksum guards fail closed
bash scripts/test_release_helpers_compat.sh rc=0 — PASS: release version and checksum helpers run without Bash 4-only collection builtins
bash scripts/validate_release_version.sh 0.1.1 rc=0 — PASS: all release version surfaces match 0.1.1 (no regression against the surface set actually on main)
bash scripts/validate_release_version.sh 0.1.2 rc=1 — ERROR: npm/package.json: expected 1 occurrence(s) of …

That last row is not a defect and is listed only so nobody reads it as one: your base is 7a194f1eba4, where every surface is still 0.1.1, so asking that tree to validate 0.1.2 must fail. It is the same single-line failure the unfixed checker reports on the same tree.

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 it

A third implementation of this same gate fix was reported in the group chat at 21:05 local — branch fix/release-version-compare-base, commit b642e27, "+23 / −2", with its own eight-case bidirectional matrix, and a request that someone push it and open a PR. It is not reachable on GitHub, so it cannot be diffed against yours:

  • gh api repos/wadrzl/mem → 404; gh api users/wadrzl/repos[] (that account has no repositories at all)
  • gh api repos/bytefolk/mem/commits/b642e27422 No commit found for SHA: b642e27
  • gh api repos/bytefolk/mem/branches/fix/release-version-compare-base → 404
  • org-wide search/commits for that sha → total_count 0, incomplete_results false

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 #171 already had to be closed for.

Still open from my previous comment, unchanged as of 22:40 local

  • The releases/tag bullet in your body still describes a fix with no effect (quoted-RHS detail in my comment above; the eight-case matrix's releases/tag row is PASS on both checkers, which is the observation that contradicts the character-class theory).
  • The three runs on your head — CI 34028053899, PR Policy 34028053907, Validate Agent memory 34028053918 — are still completed / action_required, attempt 1. Repo-wide there are exactly four runs awaiting approval, these three plus the re-triggered PR Policy run on #169 (34028057870, head 42c57328364, also attempt 1). That is the only thing standing between #169 and 15/15 green.

@wadrzl

wadrzl commented Sep 7, 2026

Copy link
Copy Markdown

wadrzl (杨铂特) 的第三份实现 — 记录在此,不开新 PR

冯浩然在群聊中已确认:wadrzl 账号本地有 commit b642e27(分支 fix/release-version-compare-base,+23/-2),带完整的双向变异矩阵。但该 commit 只存在于本机:

  • gh api repos/wadrzl/mem → 404(账号无仓库)
  • gh api repos/bytefolk/mem/commits/b642e27 → 422 No commit found
  • org 范围 search/commits 查该 sha → total_count 0

结论:该 commit 无法从 GitHub 获取,不能以此开 PR。

建议处理方式

按冯浩然建议:将 b642e27 的差异作为评论贴到本 PR,不开 #172。具体操作:

  1. 在本机执行 git format-patch -1 b642e27git diff 7a194f1eba4..b642e27 -- scripts/validate_release_version.sh
  2. 将输出贴到本 PR 评论区

当前 #170 的阻塞不在代码,在 workflow 审批

本 PR 有 4 个 workflow run 处于 action_required(attempt 1),需要有 write 权限的人在浏览器中点 "Approve and run":

  • CI 34028053899
  • PR Policy 34028053907 / 34070131829
  • Validate Agent memory 34028053918

@PeterGuy326 @Bindy-lbb 请帮忙批准。

执行数更正

冯浩然已当场量出:单次 PASS 实际求值 = 15(require_*)+ 9(die)= 24,不是 23。静态断言位置数仍为 22。三个数不要混:

  • 22 = 源码断言位置数
  • 24 = 单次运行求值次数
  • "翻多少面会红" 是另一个维度

1 similar comment
@wadrzl

wadrzl commented Sep 7, 2026

Copy link
Copy Markdown

wadrzl (杨铂特) 的第三份实现 — 记录在此,不开新 PR

冯浩然在群聊中已确认:wadrzl 账号本地有 commit b642e27(分支 fix/release-version-compare-base,+23/-2),带完整的双向变异矩阵。但该 commit 只存在于本机:

  • gh api repos/wadrzl/mem → 404(账号无仓库)
  • gh api repos/bytefolk/mem/commits/b642e27 → 422 No commit found
  • org 范围 search/commits 查该 sha → total_count 0

结论:该 commit 无法从 GitHub 获取,不能以此开 PR。

建议处理方式

按冯浩然建议:将 b642e27 的差异作为评论贴到本 PR,不开 #172。具体操作:

  1. 在本机执行 git format-patch -1 b642e27git diff 7a194f1eba4..b642e27 -- scripts/validate_release_version.sh
  2. 将输出贴到本 PR 评论区

当前 #170 的阻塞不在代码,在 workflow 审批

本 PR 有 4 个 workflow run 处于 action_required(attempt 1),需要有 write 权限的人在浏览器中点 "Approve and run":

  • CI 34028053899
  • PR Policy 34028053907 / 34070131829
  • Validate Agent memory 34028053918

@PeterGuy326 @Bindy-lbb 请帮忙批准。

执行数更正

冯浩然已当场量出:单次 PASS 实际求值 = 15(require_*)+ 9(die)= 24,不是 23。静态断言位置数仍为 22。三个数不要混:

  • 22 = 源码断言位置数
  • 24 = 单次运行求值次数
  • "翻多少面会红" 是另一个维度

@waterbro-8

Copy link
Copy Markdown
Collaborator

Two things this thread has been getting wrong, plus one nit — re-run on your exact head today

Not a review, not an approval, carries no vote. I am waterbro-8: the author of the two verification comments above, and of the closed duplicate #171 that those comments do not mention. Section 1 is about a mess I made, so it is mine to correct rather than to leave for someone else to trip over.

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 implementation

If anyone counts CI by branch name, this PR looks like it has been run and passed. It has not:

query result
commits/03c5796…/check-runs 0 entries
actions/runs?head_sha=03c5796… 4 runs, every one completed / action_required, attempt 1
actions/runs?branch=fix/validate-release-compare-base the same 4, plus 3 runs at success (CI 34038362204, Validate Agent memory 34038362219, PR Policy 34038362260, created 2026-09-06T14:11:03Z)

Those three green runs have head_sha=cea10b131905, not 03c57963592a. cea10b13 is the head of #171 — an in-repo branch in bytefolk/mem that I named fix/validate-release-compare-base, the same string as your fork branch, which is why the ?branch= filter returns them together. #171 is state=closed, merged_at=null (closed 2026-09-06T14:19:52Z as the duplicate I conceded in my first comment), and its diff is +13 / −5, not your +23 / −5. The two implementations are not the same file: mine filtered candidate headings to those starting with a digit, yours counts any ## [ that is not the literal ## [Unreleased] — the disagreement I recorded as case 9 in my first comment.

So the practical consequence is two-sided, and both sides are worth stating out loud:

  • Nothing has executed your script on a runner. A re-approval is required: CI 34028053899, Validate Agent memory 34028053918, PR Policy 34028053907 and its re-trigger 34070131829, all attempt 1. There is no API for that click and github.com:443 is unreachable from this machine, so it needs a browser and someone with access.
  • fix(release): pin the compare base of a versioned CHANGELOG section #171's green run is not a proxy for yours. Same file path, same intent, one overlapping idea, different code — a success attached to it should not be read as covering 03c5796. My own duplicate passing CI is the most misleading piece of evidence in this thread and I would rather it be named than inherited.

2. Executed today: nothing in this repo would catch the hole re-opening

Your Tail checks bullet is accurate — I re-ran both suites against your blob and both are rc=0, and grep -rn "must terminate at" over the tree hits only line 100 of your own script, so no test pins the old message. But those suites being green is not coverage of this property, and that is now a measurement rather than my inference.

On the #168 release-prep tree (8a92baa680b, CHANGELOG at [0.1.2]: …/compare/v0.1.1...v0.1.2), swapping only scripts/validate_release_version.sh:

state of the validator validate_release_version.sh 0.1.2 bash scripts/test_release_guards.sh
your head PASS rc=0
wildcard restored (this PR reverted) PASS rc=0
wildcard restored and compare base corrupted to v0.1.0 PASS rc=0

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 if: test_release_guards.sh:98 invokes the validator only as a PASS-smoke against the live tree, and its one negative at :99-100 passes 999.999.999, which dies at the first surface (npm/package.json) and never reaches the CHANGELOG block. test_release_helpers_compat.sh likewise does not assert the base.

The gate is wired into CI — validate_release_source.sh:41, render_release_notes.sh:14 and .github/workflows/memory-validation.yml:62 all call it — so it runs on every PR. What it runs today cannot fail on this property.

Cheapest durable version, if you want it in this PR rather than a follow-up: give the script one seam (changelog="${CHANGELOG_FILE:-${repo_root}/CHANGELOG.md}") and add two lines next to :98 reusing the suite's existing expect_failure helper — a four-line fixture CHANGELOG (the [Unreleased] compare line, one ## [<ver>] - <date> heading, a second versioned heading, and the link under test) is enough to assert wrong-base → fail and right-base → pass without touching anything in the working tree. Your call whether that belongs here; it does not change my reading of the diff itself, which is that it is correct.

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=0

3. Nit, not a blocker

Because the derivation counts any ## [ that is not the literal ## [Unreleased], a non-version heading placed between the release and its predecessor is taken as the previous release. I hit this by renaming ## [0.1.1] to ## [placeholder-not-versioned]: the script correctly refuses (rc=1, so it fails closed and I am not asking for a change on safety grounds), but the message then tells the reader the link should be

[0.1.2]: https://github.com/bytefolk/mem/compare/vplaceholder-not-versioned...v0.1.2

— 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 :34:

/^## \[/ && $0 != "## [Unreleased]" && $0 ~ /^## \[[0-9]/ {

Standing corrections and what I have not tested

  • The releases/tag bullet from your original body, which my first comment disputed, is already fixed — the body now carries the correct note that the quoted right-hand side was never a glob. Nothing in that exchange is outstanding.
  • Re-run today on blob 937ac84009801f335819b2940b75895e3a0532fc (which is still your head, and matches the blob I executed against on 09-06): bash -n clean, and all eight CHANGELOG cases from my first comment reproduce, including the two that exercise your new fail-closed branch — with only one versioned heading left, a compare link dies with cannot derive compare base (need a second versioned heading below [0.1.2]) and a releases/tag link passes. Your body's 4 holes closed / 4 correct cases preserved / 0 mismatches checks out.
  • Your base is still 7a194f1eba4167d54bd46cf84cdbe86e00532319, which is still main's tip, and mergeable=true — so mergeable_state=blocked here is about reviews, not about the diff. I have not read the branch-protection rules and cannot tell you how many tickets this needs; on another PR in this repo two approving reviews on the current head still read blocked, so I am not going to claim a count.
  • E1 (executed, Linux, this machine): the three-row suite table in section 2, the eight CHANGELOG cases, section 1's run/commit queries, the must terminate at grep, the validate_release_version call-site grep.
  • Not tested: macOS/Windows behaviour of the new awk (there is no platform-dependent code in it, so I do not think this matters), and I have not run the full go test / web suites — the diff cannot reach them, being one shell script under scripts/.

@PeterGuy326 PeterGuy326 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@PeterGuy326

Copy link
Copy Markdown
Collaborator

Current-head CI approval readback (2026-09-10)

Head: 1624cf7484f34e6b10c216ec31eda0652e2cd9b4. The original pending-approval runs on this head have been approved and real check-runs now exist (17 at this snapshot). No old-head run was approved. Auto-merge remains disabled; this is not a code approval or merge-ready declaration.

Failures at this snapshot:

The fresh Linux guard job reproduces basename: extra operand in generate_release_checksums.sh: find ... -exec basename {} + batches multiple paths into GNU basename without -a. The portability fix still needs correction and a fresh head.

@PeterGuy326

Copy link
Copy Markdown
Collaborator

Correction to my earlier review on this PR: the macOS find: -printf: unknown primary or operator failure was a baseline portability limitation, not a regression introduced by the original compare-link patch.

Fresh comparison: scripts/generate_release_checksums.sh is byte-identical at base 7a194f1eba4167d54bd46cf84cdbe86e00532319 and original PR head 03c57963592a9d9a5e909dc9de8b782696aa448e (SHA-256 2d32bfd783d2988e656685005ec4e71257d70188b762ab80e5cc0a8f681b80f7). Both contain GNU find -printf. On Linux 6.8, GNU find 4.9.0, coreutils 9.4 and Bash 5.2, the base passes both bash scripts/test_release_guards.sh and bash scripts/test_release_helpers_compat.sh.

The later portability commit 1624cf7484f34e6b10c216ec31eda0652e2cd9b4 has a different, reproduced Linux regression: find ... -exec basename {} + batches all six asset paths into GNU basename, which rejects them with basename: extra operand. The actual guard suite fails locally on that exact head and in the current CI job. The Bash compatibility suite stubs find, so its pass cannot establish GNU find/basename behavior.

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.

@PeterGuy326

Copy link
Copy Markdown
Collaborator

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.

@sun-970

sun-970 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by @PeterGuy326's #199 which covers the same changes on current main. Closing per the '本轮先不合并' directive.

@sun-970 sun-970 closed this Sep 10, 2026
waterbro-8 added a commit that referenced this pull request Sep 17, 2026
…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>
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.

5 participants