Conversation
…t ran The release gate refused 0.2.16 with "published tests=2026 but the measured suite reported 2271". That was an honest report: the README badge had not moved since the suite passed 2,026, and the gate binds the published number to the measured one. Skills and hooks cannot drift like this because both are counted from the payload. A test total has no such source -- it exists only after a suite runs -- so readPublished() could prove the three READMEs agreed with each other and nothing more. A wrong number written to all three was self-consistent and passed. check() now takes an optional expectedTests, exposed as --check --tests <measured-total>, and CI passes the total from the npm test run it already performs. Same TAP grammar as the release workflow, so one parser serves both callers. The capture step is pinned to bash: the windows runners default to PowerShell, which would report success over a failing suite once npm test is piped into a log. Badges move 2,026 -> 2,273, measured on this tree in a clean worktree so an uncommitted local edit could not inflate it.
CI run 33286176438 failed both windows jobs at "Run the suite" while the suite itself reported tests 2273, fail 0, skipped 7. The suite passed; the parser did not. node --test prefixes its summary with the three-byte UTF-8 glyph, and grep -Eo '^. tests [0-9]+' only spans it where the locale is multibyte-aware. Git bash on the windows runners is C, so the pattern matched nothing, the total came back empty, and set -e killed the step at the grep before the emptiness check could name the problem. Ubuntu's UTF-8 locale is why the release workflow has carried the same expression without ever showing it. LC_ALL=en_US.UTF-8 grep -Eo '^. tests [0-9]+' -> tests 2273 LC_ALL=C grep -Eo '^. tests [0-9]+' -> no match LC_ALL=C grep -Eo 'tests [0-9]+$' -> tests 2273 Both workflows now anchor on the value, which matches exactly one line of a real 2,300-line log because a test title never ends that way, and both add || true so a miss reaches the explicit check instead of dying anonymously. release.yml is fixed too rather than left latent. suite-summary-parse.test.mjs extracts every summary pattern from both workflow files and runs it under C and en_US.UTF-8, asserting identical matches and that the digits read belong to the summary rather than to a test title. Restoring the old anchor fails it on any machine. Badges move to 2,276, measured on this tree in a clean worktree.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0445e50a49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| function grepUnder(locale, pattern, logPath) { | ||
| const r = spawnSync("grep", ["-Eo", pattern, logPath], { |
There was a problem hiding this comment.
When the documented npm test is run in a native Windows PowerShell/cmd environment where grep is not on PATH, this spawn returns ENOENT; because the error is ignored, grepUnder() returns an empty array and the two new parser tests fail. CI masks the regression by explicitly running the suite under Git Bash, while the test harness is documented as requiring only Node's built-in toolchain (docs-site/src/content/docs/development/build-test.md:3-7). Implement the pattern check in JavaScript or otherwise avoid making the root suite depend on an external Unix executable.
Useful? React with 👍 / 👎.
1 participant
Summary
The 0.2.16 GitHub Release was blocked by its own gate:
That was an honest report. The README tests badge had not moved since the suite passed
2,026, and the release gate binds the published number to the measured one. Skills and
hooks cannot drift this way because both are counted from the payload; a test total has
no such source, so nothing compared it and the drift survived three versions before
surfacing at the last step before publication.
Two fixes plus their guards:
inventory.mjs check()takes an optionalexpectedTests, exposed as--check --tests <total>. CI passes the total from thenpm testrun it alreadyperforms, so this drift now fails on a pull request.
grep -Eo '^. tests [0-9]+'.node --testprefixes that line with a three-byte UTF-8 glyph, and
^.only spans it in amultibyte-aware locale. Git bash on the windows runners is
C, so the first CI runof the new step failed both windows jobs on a suite that had passed with
fail 0.Both workflows now anchor on the value, and
release.ymlis fixed rather than leftlatent.
Badges move 2,026 -> 2,276.
Testing
0445e50a: 4/4 SUCCESS, including both windows jobs. Packed installlifecycle: success.
the count:
EXIT=0,tests 2276.inventory.mjs --check --tests 2276andgate.mjs: both OK.inventory.test.mjs; restoring the old grep anchor failssuite-summary-parse.test.mjswith the C-locale message.
Not done here
Publishing the release itself. That is a workflow dispatch against
mainonce thislands.