Skip to content

feat(release): fragment phase 3 prerequisites: Dependabot fragments, gate hints, em-dash guard - #6419

Merged
kyle-sexton merged 2 commits into
mainfrom
feat/fragments-phase-3-dependabot-gates
Oct 5, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
feat/fragments-phase-3-dependabot-gates

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Part of #6006 (ADR 0048 phase 3, part A). This lands before phase 3 adds about 10 high-churn plugins to scripts/fragment-plugins.txt, so those plugins keep working on Dependabot PRs, in the bump gates and in the release.

Changes

Dependabot (scripts/dependabot-plugin-bump.sh, .github/workflows/pr-bump-plugin-version.yml)

  • For a plugin in fragment mode, the script writes a patch fragment through scripts/new-changelog-fragment.sh (so it gets the same name), with ### Changed followed by the same update lines. It does not bump plugin.json or CHANGELOG.md.
  • It skips a plugin whose branch already added a fragment with a real bump (changelog_fragments::bump_delivered, compared against the fork point's version), so a rerun does nothing.
  • The self-check also runs check-changelog-fragments.sh --check on the fragment it wrote. --check-required reads only the committed change set, so it cannot see the new file yet.
  • The workflow also stages .changes/, because git add -u skips new files. I kept git add -u -- plugins as it was and added .changes/ with -A only when that directory exists. Running git add -A -- plugins .changes fails with fatal: pathspec '.changes' did not match any files when there is no fragment, which would break every legacy-only run.
  • harness-ops (the dependabot.yml npm entry for plugins/harness-ops/skills/inventory/scripts/js) is the case this is for.

Gate messages (sync-shared-copies.sh, check-vendor-version-bump.sh, check-standards-contract-bump.sh)

  • When a stale plugin is in fragment mode, the gate says Run scripts/new-changelog-fragment.sh <plugin> patch instead of telling you to bump the version. Plugins not in fragment mode get exactly the same messages as before.

Em dash guard (scripts/lib/changelog-fragments.sh, scripts/release-plugins.sh)

  • changelog_fragments::validate now rejects an em dash (U+2014) anywhere in a release fragment, code included, when plugins/<p>/CHANGELOG.md matches an entry in scripts/em-dash-purged-paths.txt. It reads list entries the way check-purged-em-dashes.sh does, where * matches within one path component. It does not try to copy the detector's fence parsing: a second parser missed fence shapes the detector flags, and rejecting too much is the safe direction. Without this, the release copies the em dash into the CHANGELOG and fails the release PR for every plugin. bump: none reasons are exempt because no release copies them.
  • release-plugins.sh: if one of a plugin's fragments is invalid, the script skips that plugin, reports it on stderr and leaves all of its fragments in place. The other plugins still release. If every plugin is skipped, it exits 0 and writes nothing.

Before the flip

Dependabot PRs opened before phase 3 lists a plugin were built from a branch with the old list, so they bump the version, and --check-bump then rejects that as a FRAGMENT-MODE RELEASE. Comment @dependabot rebase on each one after the flip.

Tests

Each new case failed against the origin/main scripts and passes now:

  • scripts/check-changelog-fragments.test.sh: 43 pass
  • scripts/release-plugins.test.sh: 19 pass. The two cases that expected exit 2 on an invalid fragment now expect the plugin to be skipped.
  • scripts/sync-shared-copies.test.sh, scripts/check-vendor-version-bump.test.sh (25 pass), scripts/check-standards-contract-bump.test.sh (14 pass)
  • scripts/dependabot-plugin-bump.test.sh: 18 pass. This includes a base-side release after the fork, and a branch older than the fragment library (that branch is treated as all legacy; before this fix the script exited 1)

The unchanged suites scripts/lib/changelog-fragments.test.sh, scripts/new-changelog-fragment.test.sh and scripts/check-changelog-parity.test.sh also pass. shellcheck, shfmt and actionlint are clean. The 3 executable-bit cases in sync-shared-copies.test.sh fail on Windows NTFS only; they fail the same way on origin/main.

Follow-ups (outside this PR's file scope)

  • release-plugins.yml runs scripts/release-plugins.sh | tee. With no shell: bash, the step has no pipefail, so the script's exit 2 never failed the job, either before or after this change. Skipped plugins are reported only on stderr, not as ::warning:: and not in the release PR body.
  • ADR 0048 still names dependabot-plugin-release.yml. Another lane is fixing that.

🤖 Generated with Claude Code

…ent-mode plugins

ADR 0048 phase 3 lists high-churn plugins in scripts/fragment-plugins.txt.
Before that flip:

- dependabot-plugin-bump.sh writes a patch fragment (named by
  new-changelog-fragment.sh, the update under ### Changed) for a
  fragment-mode plugin instead of bumping plugin.json and CHANGELOG.md,
  skips a plugin whose branch already added one, and self-checks it with
  check-changelog-fragments.sh --check. pr-bump-plugin-version.yml stages
  .changes/, which git add -u skips.
- sync-shared-copies.sh, check-vendor-version-bump.sh and
  check-standards-contract-bump.sh tell a fragment-mode plugin to run
  scripts/new-changelog-fragment.sh <plugin> patch instead of bumping its
  version. Legacy messages are unchanged.
- changelog_fragments::validate rejects an em dash outside code in a
  release fragment whose plugin CHANGELOG.md is in
  em-dash-purged-paths.txt, which the release would copy in and fail.
- release-plugins.sh skips a plugin with an invalid fragment, reports it
  and leaves its fragments in place, and releases the rest.

Part of #6006

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body does not yet satisfy the issue-linkage contract:

  • Missing a "## Summary" section. Describe what this PR changes and why, in a sentence or two.
  • Missing a "## Fix" section. State the concrete change and how it addresses the problem.
  • Missing a "## Verification" section. Record concrete evidence the change works (commands, gates, output).
  • Missing a "## Related" section. List related PRs, ADRs, or decision-log entries this PR does not close.
  • Missing a native closing keyword (Closes/Fixes/Resolves #N). If this PR references an issue it must not close, put "Refs: #N" (or "Relates to: #N") on its own line. If it relates to no GitHub issue at all, state "No linked issue" (or "No related issue:") in the body instead.

Edit the body and this comment updates itself on the next run.

…nd em-dash checks

- The em-dash guard rejects U+2014 anywhere in a release fragment for a
  purged CHANGELOG.md, code included. A second fence parser missed four
  shapes the detector flags (tab or deep indent, mixed fence characters,
  list-item fences), and a miss fails the release PR.
- dependabot-plugin-bump.sh treats a branch without
  scripts/lib/changelog-fragments.sh as all legacy instead of exiting 1,
  since the workflow runs main's script against the PR branch's scripts/.
- In fragment mode it compares the fork point's version, so a release of
  the plugin on main after the fork no longer suppresses the fragment.
- The fragment-count assertions no longer pass on an unmatched glob.

Part of #6006

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 5, 2026 01:14
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-05T01:18:40.917445Z 4597d96 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 5m 0s —— View job


Review complete

  • Gather context (PR diff, body, CLAUDE.md/AGENTS.md instructions)
  • Understand the request (invoke /review:code-review per custom instructions)
  • Run /review:code-review against the merge base (05cf4e4, this branch's actual fork point — see note below)
  • Post findings as inline PR comments
  • Post summary comment

Note on base ref: the checkout is shallow and origin/main locally resolves to a commit (0f2e6e0) with no common ancestor with this branch in the fetched history, so git diff origin/main...HEAD failed with "no merge base." I instead diffed against 05cf4e4 (the commit this branch's two PR commits are built on), which reproduces the PR's stated stat exactly (461 insertions / 49 deletions / 13 files), so the reviewed diff matches this PR's actual content.

No findings to report. I read every changed script and its paired test file:

  • scripts/lib/changelog-fragments.sh's new em_dash_purged/em_dash_lines helpers reimplement check-purged-em-dashes.sh's component-aware glob matching via a different technique (comparing slash counts via ${s//[^\/]/} rather than splitting on /). I worked through why this is equivalent (a */? can only match path separators that aren't otherwise literal in the glob, and the slash-count equality check forces that match count to zero), and the existing test added in scripts/check-changelog-fragments.test.sh ("--check keeps a glob's * inside one path component") confirms the cross-component case is actually exercised. No bug.
  • scripts/release-plugins.sh's skip-whole-plugin-on-invalid-fragment logic, scripts/dependabot-plugin-bump.sh's fragment-mode branch (including the merge_base/fork-version handling for bump_delivered and the legacy-branch fallback when lib/changelog-fragments.sh doesn't exist yet), and the three gate scripts' new fragment-mode-aware messaging all traced correctly end to end, and each matches its accompanying new test cases.
  • The one odd-looking wording I checked closely — check-standards-contract-bump.sh's footer line conditionally appending " not in fragment mode" to the generic bump instruction — is asserted verbatim by its own new test case, so it's intentional phrasing, not a bug.

Unverified (author-claimed): I was not able to execute the test suites or shellcheck/shfmt in this sandboxed environment (script execution was denied), so the PR's stated test-pass counts and lint-clean claims are author-reported and I did not independently confirm them. Everything above is from reading the diff and tracing the logic by hand.

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 48s —— View job


Security review complete

  • Gather context (PR diff, changed files)
  • Review Dependabot bump script changes
  • Review gate-message script changes
  • Review em-dash guard / release script changes
  • Review workflow file change (.github/workflows/pr-bump-plugin-version.yml)
  • Check instruction-surface deletions against protected-class register
  • Adversarially validate any candidate findings
  • Post findings / summary

No security issues found.

Scope reviewed (diff only, origin/main...HEAD via gh pr diff 6419):

  • scripts/dependabot-plugin-bump.sh — the new fragment-mode branch writes through scripts/new-changelog-fragment.sh, jq --arg, and quoted printf/bash calls throughout; --title/--pr values from the workflow (PR_TITLE, PR_NUMBER env vars, not inline ${{ }} interpolation in run:) only ever land as literal text inside a changelog/fragment body, never in a command or eval context, so Dependabot-controlled text has no injection path. scripts/dependabot-plugin-bump.sh#L239-L358
  • scripts/lib/changelog-fragments.sh — the new em_dash_purged/em_dash_lines/em-dash branch in validate() reads a repo-owned list (scripts/em-dash-purged-paths.txt), matches it with a bounded glob, and the bump: none path still returns before reaching the em-dash check, consistent with the PR description's claim that none fragments are exempt because no release copies them. scripts/lib/changelog-fragments.sh#L187-L206
  • scripts/release-plugins.sh — the skip-invalid-plugin logic correctly removes every fragment belonging to a skipped plugin before the bump/write passes run, so a plugin with one bad fragment can't have its other fragments partially applied. No path-traversal or privilege widening introduced; .changes/<plugin>/ membership is still validated by the unchanged validate() prefix checks. scripts/release-plugins.sh#L58-L85
  • check-vendor-version-bump.sh, check-standards-contract-bump.sh, sync-shared-copies.sh — only add a parallel "fragment missing" failure branch alongside the existing "version not bumped" failure; both still exit/gate_entry::finish 1 on staleness, so the supply-chain-relevant bump gate isn't weakened or bypassable.
  • .github/workflows/pr-bump-plugin-version.yml — the diff only changes step bodies (adds staging .changes/ and a comment); it does not touch the trigger (pull_request, not pull_request_target), the dependabot[bot]-only if: gate, permissions:, or secret usage. No new Actions trust-boundary change; GitHub Actions hardening concerns are zizmor's advisory lane per this review's scope and are not re-reported here.

Instruction-surface check: this PR touches no CLAUDE.md/AGENTS.md/rules/skill/agent/hook file, so the instruction-exception-register lens (docs/conventions/instruction-exception-register/README.md, present in this checkout) has no candidate deletion to test against it.

No command/path/template injection, no secret-handling issue, and no authorization gap found in the changed files.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

claude-security-review has reviewed this pull request through 4597d96; a later push is reviewed from there.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

claude-review has reviewed this pull request through 4597d96; a later push is reviewed from there.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 5, 2026
Merged via the queue into main with commit d02954e Oct 5, 2026
52 of 62 checks passed
@kyle-sexton
kyle-sexton deleted the feat/fragments-phase-3-dependabot-gates branch October 5, 2026 01:27
kyle-sexton added a commit that referenced this pull request Oct 5, 2026
…face skipped plugins

The step piped release-plugins.sh into tee without pipefail, so a failed
release script never failed the job. Set pipefail and keep the script's
exit code. release-plugins.sh now skips a plugin whose fragments fail
validation (#6419); emit a ::warning:: for each skipped plugin and list
them in the release pull request body. The body's merge note now says the
workflow queues the pull request itself.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.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.

1 participant