Release: require the version's changelog section, and explain the missing Dependabot bumps by polewskm · Pull Request #62 · NCodeGroup/code-review

polewskm · 2026-10-31T04:12:16Z

release.yml now runs tools/release-notes.ts before it tags: the run fails when CHANGELOG.md has no dated ## [<version>] - YYYY-MM-DD section with entries for the version, and that section becomes the release notes instead of --generate-notes. test/release-notes.test.ts covers it hermetically (node:test).
The README's Versions section explains why Dependabot proposed no bumps: since dependabot-core#13731 (December 2025), the github-actions updater skips any tag whose GitHub release is marked as a pre-release. That applies to SHA pins too. A 0.x version on its own isn't treated as a pre-release. The section links to the source and gives the manual step to use until this is fixed.

Decisions

release.yml still marks versions below 1.0.0 as pre-releases, so Dependabot will keep skipping new releases until that changes. Whether to drop --prerelease is left to you.
tools/ was added to tsconfig.json's include so the script is typechecked.
A section's date must be a real YYYY-MM-DD date but doesn't have to be the release day.

Remaining

Test the pre-release finding on a real release: publish one without the pre-release flag, or run gh release edit v0.3.1 --prerelease=false, which an immutable release allows. Then run "Check for updates" in a consumer and confirm Dependabot proposes the bump.
Auto-merge workflow for Dependabot bumps in consumers. This belongs in project-template.

Refs #33

…issing Dependabot bumps The Release workflow now runs tools/release-notes.ts before tagging: it fails when CHANGELOG.md has no dated section with entries for the version, and its output becomes the release notes instead of generated ones. The README records that Dependabot's github-actions updater skips releases marked as pre-releases, with the manual step meanwhile. Refs #33

ncode-review

🤖 NCode Review at db8499a: 3 new findings on the changed lines.

ncode-review · 2026-10-31T04:13:39Z

🤖 Review complete

Verdict: 💬 Comment — This PR adds tools/release-notes.ts, which makes a release require a dated CHANGELOG.md section with entries and uses that section as the release notes. It also documents why Dependabot hasn't proposed bumps to consumers and how to bump them in the meantime. The parser is small and fails safe: every gap found ends in a blocked release, never a bad one. What remains is misleading error messages, untested edge cases and documentation hygiene. One thing to check: the diff shown has no change to release.yml, and the reviewers cite a test/release-notes.test.ts that isn't in the diff either. Please confirm the workflow actually calls the script and that the tests are included, since the changelog entry says the Release workflow now enforces this.

8 reviewers run — 5 findings (run details)

Reviewer	Model	Findings	Top
synthesis	opus-5-5	5	🟡
architecture	sonnet-5-5	0	🟢
completeness	sonnet-5-5	0	🟢
conventions	sonnet-5-5	0	🟢
correctness	sonnet-5-5	0	🟢
hygiene	haiku-5-5	2	🟡
intent	haiku-5-5	1	🟡
rule-check	sonnet-5-5	1	🟡
tests	haiku-5-5	2	🟡

Skipped: api-compat, data-model, infrastructure, performance, security

Findings

🟡 F1 tools/release-notes.ts:16: Fence tracking misreads unclosed and mismatched fences, which hides the section behind a misleading error
🟡 F2 tools/release-notes.ts:54: Numbered entries are rejected with 'lists no changes'
🟡 F3 CHANGELOG.md:14: Changelog entry mixes the workflow change with a summary of the README investigation
🟡 F4 README.md:91: Upstream analysis relies on line-range permalinks into dependabot-core
🟡 F5 tsconfig.json:23: Typecheck now covers tools/, and nothing says so

_{Trial: findings are posted as review threads and in the check run. $0.39 on Amazon Bedrock, 68s, at a3fc122. (how to interact)}

…as a process

…twin in the workflow

polewskm · 2026-10-31T05:00:07Z

/review

ncode-review

🤖 NCode Review at 08ffcde: 5 new findings on the changed lines.
2 more were already posted on this pull request.

…n, which Node 24.0 and 24.1 lack

polewskm · 2026-10-31T06:26:10Z

Autopilot: answering the findings in #62 (comment) that have no thread:

F1 (import.meta.main needs Node 24.2): fixed in a3fc122. The script now compares import.meta.url with process.argv[1], as training/run.ts does, so on Node 24.0 or 24.1 it no longer exits 0 with empty notes; a new process test runs it by the relative path the workflow uses.
F4 (version pattern copied between the script and release.yml): a repeat, answered in Release: require the version's changelog section, and explain the missing Dependabot bumps #62 (comment).

polewskm · 2026-10-31T06:26:16Z

Autopilot: Round 2 answered. No blocker or should-fix is left open; the remaining threads are nits, answered above. Recommend merging once you've resolved them.

ncode-review

🤖 NCode Review at a3fc122: 4 new findings on the changed lines.
1 more was already posted on this pull request.

ncode-review · 2026-10-31T06:30:47Z

+function* linesWithFences(lines: readonly string[]): Generator<[index: number, line: string, inFence: boolean]> {
+  let inFence = false;
+  for (const [index, line] of lines.entries()) {
+    if (/^\s*(```|~~~)/.test(line)) inFence = !inFence;


🟡 Nit: Fence tracking misreads unclosed and mismatched fences, which hides the section behind a misleading error _{F1 · tests, rule-check}

linesWithFences flips state on any line that starts with or ~~~. So an unclosed fence in an earlier section marks every later heading as fenced, and a fence can be closed by a ~~~ line.

Consequence: A stray or mismatched fence makes the release fail with 'has no ... section; add one' even though the section exists, or cuts the section in the wrong place. The tests cover only a closed ``` fence, so a regression here would go unnoticed.

Fix: Record the opening marker and close the fence only on the same marker. If the scan ends inside a fence, say so in the error. Add tests for an unclosed fence before the version's heading and for a ~~~ fence.

Evidence

if (/^\s*(```|~~~)/.test(line)) inFence = !inFence;. The only fence test is 'releaseNotes keeps a heading inside a fenced block', which uses a closed fence.

Confidence 0.35.

Autopilot: fixed in #85 (ca461a7): a fence now closes only on a run of its own character at least as long as the opener, with nothing after it, as in CommonMark. When an unclosed fence hides the version's heading, the error names the line the fence opened on, and a section that ends inside an unclosed fence fails. New tests cover a mismatched marker, a ~~~ fence and both unclosed cases.

ncode-review · 2026-10-31T06:30:47Z

+  const notes = body.join('\n').trim();
+  if (!/^\s*[-*+]\s+\S/m.test(notes)) throw new Error(`CHANGELOG.md's section for ${version} lists no changes.`);
+  return `${notes}\n`;
+}


🟡 Nit: Numbered entries are rejected with 'lists no changes' _{F2 · tests}

The entries check accepts only -, * and + bullets, so a section made only of 1. ... items fails as if it were empty.

Consequence: A maintainer who writes numbered entries has the release blocked by an error that misstates the problem, and no test pins down which behavior is intended.

Fix: Choose one: accept ordered items (/^\s*(?:[-*+]|\d+[.)])\s+\S/m), or keep rejecting them and say in the error that bullets are required. Add a test either way.

Evidence

if (!/^\s*[-*+]\s+\S/m.test(notes)) throw new Error(CHANGELOG.md's section for ${version} lists no changes.);

Confidence 0.30.

Autopilot: fixed in #85 (ca461a7): 1. and 1) items now count as entries, and a test checks it.

ncode-review · 2026-10-31T06:30:47Z

+
+### Dependabot and pre-releases
+
+No consumer got a Dependabot pull request for v0.2.0 to v0.3.1, because each is marked as a pre-release. Since


🟡 Nit: Upstream analysis relies on line-range permalinks into dependabot-core _{F4 · hygiene}

The 'Dependabot and pre-releases' section backs its claims about upstream behavior with commit-pinned line ranges (#L418-L430, #L722-L757, #L73-L97).

Consequence: Once upstream changes, the links still resolve but stop describing current behavior. That leaves a usage guide holding source analysis that readers can't easily recheck.

Fix: Keep the link to dependabot-core#13731 and move the line-level analysis to issue #33, leaving the README with the practical steps.

Evidence

README.md:91-109 holds three permalinks to git_commit_checker.rb and package_details_fetcher.rb with line anchors.

Confidence 0.30.

Autopilot: declined: the links are pinned to commit 85379652e on purpose, so they keep showing the exact code the analysis read even after upstream changes. The section already says the second route hasn't been tried on a consumer. Testing the pre-release theory is tracked in #84 (the #33 item), and the line-level analysis can move to #33 once that test settles it.

ncode-review · 2026-10-31T06:30:47Z

    "skipLibCheck": true
  },
-  "include": ["src", "test", "evals", "training"]
+  "include": ["src", "test", "evals", "training", "tools"]


🟡 Nit: Typecheck now covers tools/, and nothing says so _{F5 · intent}

Adding "tools" to include puts every TypeScript file under tools/ into npm run typecheck, and neither the changelog nor the PR mentions it.

Consequence: Other TypeScript files under tools/ now fall under the strict checks, and readers won't know why typecheck coverage widened.

Fix: Confirm this is intended, and mention it in the CHANGELOG entry or the PR description.

Evidence

"include": ["src", "test", "evals", "training", "tools"]. The CHANGELOG diff adds only the Release workflow bullet.

Confidence 0.30.

Autopilot: declined: this is intended. tools/ holds TypeScript the Release workflow runs (tools/release-notes.ts) and tools/report.ts, so it belongs under the strict typecheck like src/ and test/, and it passes. Typecheck coverage is internal and consumers never see it, so the CHANGELOG doesn't record it.

Answer late review findings on #62, #78 and #79

polewskm deployed to code-review October 31, 2026 04:12 — with GitHub Actions Active

ncode-review Bot reviewed Oct 31, 2026

View reviewed changes

Comment thread tools/release-notes.ts Resolved

Comment thread tools/release-notes.ts Resolved

Comment thread tools/release-notes.ts Resolved

polewskm added 3 commits October 30, 2026 22:52

Merge remote-tracking branch 'origin/main' into issue-33

7336c7b

Test the release-notes script's usage error and unreadable changelog …

c0652c7

…as a process

Share the release-notes fence tracking, and note the version check's …

08ffcde

…twin in the workflow

ncode-review Bot reviewed Oct 31, 2026

View reviewed changes

Comment thread test/release-notes.test.ts Resolved

Comment thread .github/workflows/release.yml Resolved

Comment thread README.md Resolved

Comment thread CHANGELOG.md Resolved

Comment thread test/release-notes.test.ts Resolved

polewskm added 2 commits October 30, 2026 23:09

Merge remote-tracking branch 'origin/main' into issue-33

e47b0aa

Detect the release-notes script's entry point without import.meta.mai…

a3fc122

…n, which Node 24.0 and 24.1 lack

polewskm marked this pull request as ready for review October 31, 2026 06:29

polewskm deployed to code-review October 31, 2026 06:29 — with GitHub Actions Active

polewskm enabled auto-merge October 31, 2026 06:29

polewskm merged commit ebea926 into main Oct 31, 2026
6 checks passed

polewskm deleted the issue-33 branch October 31, 2026 06:30

ncode-review Bot reviewed Oct 31, 2026

View reviewed changes

polewskm mentioned this pull request Oct 31, 2026

Answer late review findings on #62, #78 and #79 #85

Merged

polewskm added a commit that referenced this pull request Oct 31, 2026

Merge pull request #85 from NCodeGroup/late-feedback

0398d5a

Answer late review findings on #62, #78 and #79

polewskm mentioned this pull request Oct 31, 2026

Roll out the 2026-10-31 changes: release, consumer pins, app permission, CodeQL #97

Closed


		### Dependabot and pre-releases

		No consumer got a Dependabot pull request for v0.2.0 to v0.3.1, because each is marked as a pre-release. Since

Conversation

polewskm commented Oct 31, 2026

Uh oh!

ncode-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ncode-review Bot commented Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 Review complete

Uh oh!

polewskm commented Oct 31, 2026

Uh oh!

ncode-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

polewskm commented Oct 31, 2026

Uh oh!

polewskm commented Oct 31, 2026

Uh oh!

Uh oh!

ncode-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ncode-review Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

polewskm Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

ncode-review Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

polewskm Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

ncode-review Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

polewskm Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

ncode-review Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

polewskm Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ncode-review Bot commented Oct 31, 2026 •

edited

Loading