Release 1.2.1 — seven advisories leave the tree - #869
Merged
Conversation
Bumps the actions group with 2 updates: [actions/upload-artifact](https://github.com/actions/upload-artifact) and [actions/download-artifact](https://github.com/actions/download-artifact). Updates `actions/upload-artifact` from 4.6.2 to 7.0.1 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@ea165f8...043fb46) Updates `actions/download-artifact` from 4.3.0 to 8.0.1 - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@d3f86a1...3e5f45b) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/download-artifact dependency-version: 8.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com>
…w per remote Two reports in #865, both about a line asserting something nobody checked. **The push line.** #632 asked "is there a local note waiting?" and #656 answered it unconditionally with yes: on a failed mirror the hook said "the records for these commits are still only local" whether or not any record existed. A field report on v1.2.0 got that line for a repository with no notes ref at all, ran `commitlore sync` on its instruction, and found nothing to send. What the remote holds cannot be known once the remote has stopped answering. Whether this clone holds anything is local and cannot time out, so the hook now measures that first and only claims records are waiting when it found some. The granularity is the ref, not the commit, and that was not the first attempt. A commit-scoped check -- "do the commits in this push carry records" -- reads the sentence's own subject and is wrong in the dangerous direction: the mirror publishes `refs/notes/commitlore` whole, so a record written against a commit the remote already has is still unsent, and the commit-scoped check calls that "nothing waiting". Three existing tests in `test/pre-push-hook.test.ts` write a record and then advance the branch, which is exactly that shape, and they failed against the first attempt. They are the reason this is ref-scoped. **The sync table.** `SyncResult.detail` promises one line and git does not: an unreachable remote answers with `remote: Repository not found.` and `fatal: repository '...' not found`, and both went into the detail column. The row broke in half and the second line read as a bare `fatal:` standing above the next remote's row rather than as that remote's result. Collapsing happens in `failure()`, the one place a git diagnostic becomes a detail, so the table, the JSON and the hook all inherit the promise. Reproduced before and after against a repository with a deleted remote. Record-Id: r-pushlinemeasures Follows: r-cdebmanifest Provenance: authored Certainty: firm Blast: module Undo: easy Ruled-out: checking only the commits in this push | it matches the sentence's subject but not the mirror's granularity -- a record for an already-pushed commit is still unsent, and this would report it as nothing waiting; three existing pre-push tests encode that shape and failed against it Ruled-out: filtering the fatal line out of the detail text | the multi-line value is the defect, not the wording; collapsing at the point of construction fixes every consumer instead of one renderer Limit: the reassuring sentence only fires when the notes ref is absent; a clone that has records but has already published them is still told they may be waiting, because that cannot be decided without reaching the remote Verified: 3162 tests passed and 4 skipped across 164 files; both behaviours reproduced through the built binary in a scratch repository -- the table now renders one row per remote, and a push with no notes ref reports nothing waiting while one with a record keeps the cautious sentence Unverified: CI has not run this branch; the canonical manifest is regenerated separately once this rebases onto the audit fix, since src/ is a source input
…ch have their own Dependabot opened #862 (js-yaml 5.2.3 -> 5.4.1, dev) and #863 (@modelcontextprotocol/sdk 1.29.0 -> 1.30.0, runtime) separately, and both failed `check` for the reason r-cdebmanifest describes: `package-lock.json` is in `SOURCE_INPUTS`, so any bump moves the manifest's source checksum. They cannot be repaired independently. Each repair regenerates `installer/canonical-artifact.json`, so whichever merged first would invalidate the other's manifest and force a second rebuild, a second review and a second 30-minute CI run for a change already made. Batching is not a convenience here; separate branches would have produced a conflict by construction. Both were verified together rather than assumed to compose: the SDK is a runtime dependency and is bundled, js-yaml is not, and the rebuilt bundle carries only the SDK's change. The artifact digest moves to 8ca06b24a111ad6e28e6f6319af53cf4140d0f31fcb791879d8844d784b0dbe0. Record-Id: r-depsbatchbump Follows: r-fasturiqsaudit Provenance: authored Certainty: firm Blast: module Undo: easy Ruled-out: repairing #862 and #863 on their own branches | each regenerates the same manifest file, so the first to merge invalidates the second; the cost is a rebuild and a full CI cycle per bump for no additional evidence Ruled-out: npm audit fix --force alongside these | the remaining advisories are dev-only and its breaking upgrades are the vitest major question, which is a separate decision with a measured cost Limit: says nothing about behaviour changes inside SDK 1.30.0 itself; the suite covers this repository's use of the SDK, not the SDK Verified: npm audit --omit=dev reports 0 vulnerabilities; the pinned linux/amd64 build produced the committed dist and artifact:verify exits 0; typecheck and check-engines clean; 3157 tests passed and 4 skipped across 164 files Unverified: CI has not run this branch yet
Bumps [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest) from 2.1.9 to 4.1.11. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.1.11/packages/vitest) --- updated-dependencies: - dependency-name: vitest dependency-version: 4.1.11 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…ner can meet
The upgrade is not currency. After the fast-uri and qs fix, every advisory left
in this tree is `esbuild <=0.24.2` reached through `vite` -> `@vitest/mocker` and
`vite-node` — vitest 2.1.9's own dependency tree, including the one rated
critical. vitest 4 resolves `vite@8.2.2` and dedupes onto the `esbuild@0.28.2`
this repository already depends on directly, and `npm audit` goes quiet.
Three things had to move with it.
**`--reporter=basic` no longer exists.** vitest 4 treats an unknown reporter name
as a module path and dies at startup: `Failed to load custom Reporter from basic`.
`ci.yml`'s PRD-F2 perf step used it. It is `--reporter=default` now.
**Changing `ci.yml` breaks the reviewed-workflow lock, deliberately.**
`EXPECTED_CI_WORKFLOW_SHA256` pins the workflow body the release gate has read,
because the Actions API can attest that a job ran but not what it ran — without
the lock, replacing every job with `true` would still look like a green release.
Its comment says to update it with the workflow when the reviewed contract
changes, so it moves to d29daccf... here. Worth knowing for anyone who sees 21
release-gate tests fail after a one-line workflow edit: that is this lock working,
not the edit's subject matter.
**The 5s default test timeout no longer fits.** vitest 4 runs each file through
vite 8's module runner and the per-file startup costs more wall clock. This suite
is unusually exposed: most tests spawn a real `node`, `git` or installer process
and wait on it. Measured on the same tree and machine:
default parallelism 35 failed 12 of them "timed out in 5000ms"
--maxWorkers=4 7 failed all 7 "timed out in 5000ms"
testTimeout 20s 2 failed one hook, two synchronous corpus sweeps
+ hookTimeout 30s and explicit timeouts on the sweeps 0 failed
Every failure was a timeout; not one was an assertion, and `guard.test.ts` passes
72/72 when run alone. The two `guard-insufficiency` sweeps take about two minutes
each and are synchronous, so vitest 2 never interrupted them; they now carry their
own 300s timeouts rather than pushing the global default up to cover them.
Record-Id: r-vitest4timeouts
Follows: r-cdebmanifest
Provenance: authored
Certainty: firm
Blast: module
Undo: easy
Ruled-out: raising the global testTimeout far enough to cover the corpus sweeps | it would let a genuinely hung test sit for three minutes everywhere to accommodate two tests that are legitimately long; those two say so themselves instead
Ruled-out: pinning vitest at 2.x and living with the advisories | the esbuild chain is only reachable through vitest's tree, so there is no fix that keeps 2.x, and one of the five is rated critical
Ruled-out: lowering parallelism instead of raising the timeout | measured, and it only moved the line — 7 failures remained, all still timeouts
Limit: the timings are from one machine; CI runners are slower on wall clock and faster on I/O, so the margins differ there even though the failures were reproducible here
Verified: npm audit reports 0 vulnerabilities on this branch; 3157 tests passed and 4 skipped across 164 files under vitest 4.1.11; the three release-gate suites pass 78/78 with the updated digest; typecheck clean
Unverified: CI has not run this branch; the perf step's output format changed with the reporter, and nothing downstream parses it
Seven advisories leave the tree with this release, and two of them were being shipped: `fast-uri` (high, four GHSAs, through `ajv`) and `qs` (moderate, two, through the MCP SDK's `express`) were both in the runtime tree — the one bundled into `dist/commitlore.mjs` and installed. The other five, one of them critical, were `esbuild <=0.24.2` reachable only through vitest 2's dependencies, and the vitest 4 upgrade on this branch is what removes them. `npm audit` is now silent with and without `--omit=dev`. The version moves in the six places `check-release-version.mjs` compares, and the script is run here rather than trusted to CI: tag, `package.json`, both plugin manifests, both `package-lock.json` fields, and `dist/commitlore.mjs --version` all read 1.2.1. `dist/` moves by six files, which is not the version bump — the bundle carries no version string, and the 1.2.0 release commit left `dist/` untouched for that reason. It is `r-pushlinemeasures` earlier on this branch: that commit changed `src/hooks/pre-push.ts` and `src/core/sync.ts` and deliberately left the canonical rebuild to the branch's single rebuild, so this is where those sources reach the artifact. The manifest is regenerated once, over everything. The four READMEs and both installers carry `v1.2.0` in their copy-paste install lines, so they move too; that is the same set the 1.2.0 release commit touched. Record-Id: r-release121 Follows: r-vitest4timeouts Provenance: authored Certainty: firm Blast: system Undo: costly Ruled-out: releasing the security fix alone and holding the rest | it was already merged and the remaining advisories are only reachable through the vitest upgrade, so a fix-only release would have left five of the seven, one critical, in a tree that says it is production ready Ruled-out: one branch per remaining pull request | main requires its checks against the current base, so each merge invalidates the others and costs a full 45-minute cycle; five sequential merges buy no evidence that one branch carrying five recorded commits does not Limit: npm audit is a claim about advisories published now, not about the code; and the canonical build was reproduced on one machine against the pinned image digest, which CI repeats twice but no one has repeated on another date Verified: check-release-version.mjs v1.2.1 passes across all six sources; npm audit reports 0 vulnerabilities with and without --omit=dev; the pinned linux/amd64 build produced the committed dist and artifact:verify exits 0 at 584bff7a...; typecheck and check-engines clean; 3162 tests passed and 4 skipped across 164 files Unverified: CI has not run this branch; the release workflow's own gates — exact-head-ci, canonical-artifact, install-gate — have not run, because they only run on the tag
…ier that was too wide Neither of these would have been caught by anything on this branch. Both came from the pre-tag review (`gpt-5.6-sol`, read-only, asked to disprove rather than approve), and both are recorded here because *review caught them* is the part worth keeping. **`server.json` still advertised 1.2.0** — its `.version`, its installer command and its release URL — while every source `check-release-version.mjs` reads said 1.2.1 and the gate passed. This is the note above `.codex-plugin` in that script, one file later and one release later: an MCP registry entry is how a stranger finds this, and it would have pointed at the previous tag. Fixed, and enrolled in the gate, its fixture harness and the repository self-check, because bumping it without enrolling it only moves the next occurrence. The success line was rewritten to build itself from `versionSources` rather than naming the sources by hand. It printed "version consistent" without mentioning `server.json` even after `server.json` was being compared — a line that can enumerate a set the loop did not check is the same defect one level up. **`REMOTE_NOT_FOUND` matched more than git's phrasing.** `repository .*not found` also takes `repository metadata not found` and `repository credentials not found`. Worse: `describeSync` reads `detail` for `ETIMEDOUT` to say *why* the mirror failed, and `execGit` appends that to whatever partial stderr the child wrote — so a message can carry both, and the replacement would have erased the only part anything parses and reported a missing fork for a remote that simply did not answer. The expression is bounded to git's two phrasings now and a timeout outranks the classification. The test that came with the original change checked one-line shape and not classification, which is why it passed; there is a classification test now. Record-Id: r-reviewcaughtserverjson Follows: r-release121 Provenance: authored Certainty: firm Blast: system Undo: easy Ruled-out: bumping server.json without adding it to the gate | the bump fixes this release and nothing else; the gate not reading it is the defect, and the Codex manifest note in that same script is the record of what happens when only the bump is done Ruled-out: anchoring REMOTE_NOT_FOUND but leaving the timeout to fall through it | a tightened expression still cannot see that ETIMEDOUT is present and load-bearing; the precedence has to be explicit or the next phrasing that slips through erases it again Ruled-out: amending the earlier commits so the branch reads as if this was right the first time | the review finding is the evidence that the gate has a hole, and rewriting it away would leave the hole documented nowhere Limit: the gate now reads server.json's .version; the installer command and release URL inside it are strings nothing compares, exactly as the four READMEs and two installers are Verified: check-release-version.mjs v1.2.1 names all seven sources and passes; classification test covers both git phrasings, a false-positive phrase, and a message carrying ETIMEDOUT; 3164 tests passed and 4 skipped across 164 files; canonical build reproduced and artifact:verify exits 0 at 26e55355... Unverified: CI has not run this branch; the release workflow's own gates run only on the tag
This was referenced Sep 7, 2026
CommitLore — record lintTrailers: clean — 7 commits in Active constraints for the paths this PR touchesLimits (434)
Truncated: 952 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
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
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.
Six recorded commits plus the release.
mainrequires its checks against the current base, so every merge invalidates every other open PR's checks — five sequential merges would have cost five full CI cycles and bought no evidence that one branch carrying five recorded commits does not. This supersedes #861, #862, #863, #864 and #868.What ships
Seven advisories leave the tree, and two of them were being shipped.
fast-urimainvia #866qsmainvia #866esbuildchainnpm auditis now silent with and without--omit=dev.The push hook no longer reports records that do not exist as unsent (#865). On a failed notes mirror the line claimed "the records for these commits are still only local" without checking. The check is ref-scoped, not commit-scoped: the mirror publishes
refs/notes/commitlorewhole, so a record against an already-pushed commit is still unsent, and a commit-scoped check would call that "nothing waiting" — wrong in the dangerous direction. Three existing tests encode that shape and rejected the first attempt.commitlore synckeeps one row per remote (#865). Git answers an unreachable remote with two lines and both went into a column that promises one, so the row broke in half and the trailingfatal:read as a bare error above the table.Upgrades. vitest 4.1.11, MCP SDK 1.30.0, js-yaml 5.4.1, upload-artifact v7.0.1, download-artifact v8.0.1.
Verified locally
check-release-version.mjs v1.2.1— seven sources agreenpm audit0 vulnerabilities, both surfaceslinux/amd64canonical build;artifact:verifyexits 0 at26e55355…tsc --noEmitandcheck-enginescleanReview found two things this branch would not have
gpt-5.6-sol, read-only, asked to disprove rather than approve. Three passes: the dependency batch, the artifact-action upgrade, and this branch before tagging.server.jsonstill advertised 1.2.0 — version, installer command, release URL — and the release gate does not read that file, so the gate passed. That is the note above.codex-plugin/plugin.jsonincheck-release-version.mjs, one file and one release later. Bumped and enrolled in the gate, its fixture harness and the repository self-check.REMOTE_NOT_FOUNDwas too wide. It also matchedrepository metadata not found, anddescribeSyncparsesdetailforETIMEDOUTwhileexecGitappends timeouts to partial stderr — so the relabel could have erased the only part anything reads and reported a missing fork for a slow remote. Bounded to git's two phrasings; a timeout now outranks the classification; a classification test replaces the shape-only one.The success line also now builds itself from
versionSourcesinstead of naming them by hand — it printed "version consistent" without mentioningserver.jsoneven onceserver.jsonwas being compared.Earlier passes recorded their own limits and those stand: neither could rerun the Docker build, and neither dispatched
canonical-merge.yml.Not changed, deliberately
#858 was measured and closed rather than fixed. At one distinctive token the corroboration strength is structurally
1.00whatever the token is — the corpus weight that separates a rareredisfrom an ordinaryreadis divided out by its own denominator. Every rule that removes the false positive also removesadd a Redis client→shared Redis cache, a documented true positive. The issue carries the measurements.One exception, recorded
#866 was merged without a cross-provider review. It was the security fix, and leaving two advisories in the runtime tree to wait for a reviewer is the worse trade. The CHANGELOG says so, the way 1.2.0's entry said it had no review at all.