Skip to content

refactor(plugin-quality): fold repeated resets and patterns in the audit collectors - #12763

Open
melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
chore/tidy-adhoc-plugins-plugin-quality-2026-11-03
Open

melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
chore/tidy-adhoc-plugins-plugin-quality-2026-11-03

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

No related issue: code-tidying upkeep sweep.

Summary

Ad hoc tidy sweep of plugins/plugin-quality/** from c8b364bcf (anchor: none, ad hoc). Structure-only edits to the two audit collector scripts; no behavior change.

Fix

  • skills/audit/scripts/collect-categories.sh (Beck feat: markdown-formatter plugin (pilot migration) #3, Normalize Symmetries): the per-heading marker / items / finding_open reset now runs once after close_section() instead of being repeated in both heading branches (-3 lines).
  • skills/audit/scripts/collect-standards.sh (Beck chore: initialize marketplace scaffold #1, Guard Clauses): cite()'s missing-file branch (printf '' + bare return) becomes [[ -f "$file" ]] || return 0.
  • skills/audit/scripts/collect-standards.sh (Beck feat: markdown-formatter plugin (pilot migration) #3): the windows-path-emit probe regex factors its three declaration forms into one alternation, (export|declare[[:space:]]+-x|typeset[[:space:]]+-x)[[:space:]]+(MSYS_NO_PATHCONV|MSYS2_ARG_CONV_EXCL); old and new patterns were diffed over a sample of matching and non-matching lines with identical results.
  • .changes/plugin-quality/ patch fragment.

Verification

  • bash plugins/plugin-quality/skills/audit/scripts/collect-categories.test.sh and collect-standards.test.sh: pass before and after (via scripts/affected-tests.sh --run <changed files>: 2 of 2 suites pass).
  • scripts/check-affected.sh --base c8b364bcf --no-fetch: changelog-fragments and the other static checks pass. The shellcheck step failed only because this runner's shellcheck 0.9.0 has no --rcfile flag. Running shellcheck on both files from the repo root, which picks up .shellcheckrc automatically, is clean.

Related

  • Deferred by the scope budget: none.
  • Withheld (not a pure tidying): scripts/packet-seal.sh builds the generation manifest and the primary manifest with two near-identical digest loops (around lines 329-343 and 377-391). The loops differ in their manifest-name filter and the symlink error text, so merging them would change behavior unless those differences are preserved deliberately. Evidence that would settle it: a helper that keeps both differences, with packet-seal.test.sh green.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed fe36dc42ade5a08f2485290ca6d9916900dc5032: read 3 of 3 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12763 (head fe36dc42)

I found no security issues.

All 3 changed files were read in full in .claude-lane/pr.diff. None of them matches a trusted-path glob or touches a trust control, a merge hold, a plugin manifest, or an instruction surface.

  • plugins/plugin-quality/skills/audit/scripts/collect-categories.sh: the awk per-heading reset (marker, items, finding_open) moves to just after close_section(). The awk code between the new reset and the old resets (problem() and the section assignments) never reads those variables, so the move doesn't change what the script does. It is a local report parser with no new input, command, or trust boundary.
  • plugins/plugin-quality/skills/audit/scripts/collect-standards.sh: cite()'s missing-file branch becomes [[ -f "$file" ]] || return 0. It still prints nothing and still returns 0, the same as the old printf '' followed by a bare return. The probe regex (export|declare[[:space:]]+-x|typeset[[:space:]]+-x)[[:space:]]+(MSYS_NO_PATHCONV|MSYS2_ARG_CONV_EXCL) is the same match as the old three-branch alternation. $COMPONENT is still quoted and passed to grep as a file argument, so the change adds no injection surface.
  • .changes/plugin-quality/...md: a changelog fragment (prose only).

The PR context file has no embedded instructions aimed at the lane. Prior findings: 0 rounds, and the round cap did not apply.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed fe36dc42ade5a08f2485290ca6d9916900dc5032: read 3 of 3 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings

I read every hunk of all 3 changed files. None of them match a trusted-path glob.

collect-categories.sh. The marker, items and finding_open reset used to appear once in each ## heading branch. It now runs once, right after close_section() (L224-227). Behavior is the same:

  • close_section() still reads the old values before they are cleared.
  • Both branches used to clear the same three variables right before next.
  • None of the code between the old and new reset positions reads those variables. That code is only name =, the skip-list check and the section assignment.

collect-standards.sh cite(). [[ -f "$file" ]] || return 0 behaves the same as the old printf ''; return. Both print nothing and return 0, and the new form is safe under set -e.

collect-standards.sh windows-path-emit regex. (export|declare[[:space:]]+-x|typeset[[:space:]]+-x)[[:space:]]+(MSYS_NO_PATHCONV|MSYS2_ARG_CONV_EXCL) is the old three-way alternation with the shared [[:space:]]+(…) suffix pulled out. It matches exactly the same lines. The surrounding head/cut/|| true handling is unchanged.

Changelog fragment. bump: patch fits a change that only restructures code. No plugin manifest changed, so there are no manifest lines.

Other checks:

  • There is no closing issue: the PR says it is an ad hoc tidy sweep, so no acceptance criteria apply.
  • The .claude/rules/ files whose globs match these paths (skill-bodies-state-current-rules.md, cost-claims.md) cover prose, and this change edits none.
  • There are no prior findings, and the round cap does not apply.
  • The PR's verification (the test suites passing before and after, shellcheck being clean) is the author's claim. This lane has no shell, so I did not run it.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head fe36dc42ade5a08f2485290ca6d9916900dc5032. The lanes clear this reason on their own; the next scheduled run judges it again.

a pr-unblock lane stopped its head (pr-unblock/address-feedback is failure), which clears when the
  lane runs again

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The pr-verify verifier failed head fe36dc42ade5a08f2485290ca6d9916900dc5032. What it wrote, as data:

Summary: PR #12763 has no spec to judge against. spec-text.json is an empty array: the PR closes no
  issue, has no `Refs:` line, and no trusted pull-request entry was recorded. The rules require
  `fail` when no spec exists. The diff itself, a no-behavior-change tidy of two plugin-quality audit
  collectors, looks correct, and test-bash (0) and lint-shell passed on the judged head.
- pr-verify-data/spec-text.json: holds no entry. The PR body says "No related issue", so there is no
  outcome or done-when list to judge against. To fix, link a trusted issue (Closes or Refs) that
  asks for this tidy, or give the PR a title and body from a trusted author or script so the spec
  file records it.
- CI evidence: test-bash (0)
  https://github.com/melodic-software/claude-code-plugins/actions/runs/41590658483/job/125818376163
  and lint-shell
  https://github.com/melodic-software/claude-code-plugins/actions/runs/41590658483/job/125818424314
  both concluded success on the judged head.
- plugins/plugin-quality/skills/audit/scripts/collect-categories.sh:225: moving the
  marker/items/finding_open reset to straight after close_section() resets the same state on every
  `## ` heading path before `next`. It looks equivalent. No defect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pr-unblock / address-feedback: UNCERTAIN, left open

Evidence: plugins/plugin-quality/skills/audit/scripts/collect-categories.sh:224

No code change made. The verifier failed this head only because the pull request has no spec to judge against (no Closes or Refs line, no trusted spec entry). It names no defect in this file. I read the code: at collect-categories.sh lines 223-227, every heading line runs close_section() and then resets marker, items and finding_open before any branch reaches next. That covers the Blindspots path too, so the reset is equivalent to the earlier per-branch resets. The fix the verifier names is to link a trusted issue or give the pull request a trusted title and body. That is a change to the pull request's metadata, not to a file, so a person needs to decide it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Settled in owner-login replacement #12811. The only failure here was the missing spec; #12811 carries a trusted title and body that state it (a structure-only tidy with unchanged output). The verifier found no defect at collect-categories.sh:225, and the reset still runs after close_section() on every heading path. #12811 also adds tests that pin the folded suppressor regex in collect-standards.sh. This PR is held as superseded.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The pr-verify verifier failed head fe36dc42ade5a08f2485290ca6d9916900dc5032. What it wrote about this file, as data:

- plugins/plugin-quality/skills/audit/scripts/collect-standards.sh:117: `[[ -f "$file" ]] || return
  0` matches the old empty printf plus bare return (exit 0, no output). No defect.
- plugins/plugin-quality/skills/audit/scripts/collect-standards.sh:275: the folded regex
  `(export|declare[[:space:]]+-x|typeset[[:space:]]+-x)[[:space:]]+(MSYS_NO_PATHCONV|MSYS2_ARG_CONV_EXCL)`
  matches the same lines as the old three-way alternation. No defect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pr-unblock / address-feedback: UNCERTAIN, left open

Evidence: plugins/plugin-quality/skills/audit/scripts/collect-standards.sh:117, plugins/plugin-quality/skills/audit/scripts/collect-standards.sh:275

No code change made. The verifier's notes on this file report no defect, and the code agrees. At line 117, cite() returns 0 with no output when the file is missing, which matches the old empty printf followed by a bare return. At line 275, the folded ERE matches export, declare -x or typeset -x, then whitespace, then MSYS_NO_PATHCONV or MSYS2_ARG_CONV_EXCL. Those are the same lines the old three-way alternation matched. The head failed because the pull request has no linked spec. That needs a person to link a trusted issue or supply a trusted title and body, and no edit to this file can fix it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Settled in owner-login replacement #12811. The verifier found no defect at collect-standards.sh:117 or :275. #12811 adds collect-standards.test.sh cases that flag declare -x and typeset -x suppressors at their line and leave a per-command MSYS_NO_PATHCONV=1 ungraded, so dropping a form from the folded alternation now fails the suite (checked by removing typeset from the regex: two assertions fail). It also supplies the trusted spec this head lacked. This PR is held as superseded.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the pull request has an unresolved review thread the lane does not take (a reviewer it does not act on, a review of an older head, or a thread it already answered), so no lane resolves it and a session builder takes it. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=12763 -f activity=address-feedback

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on fe36dc4.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 3, 2026
@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 3, 2026
@kyle-sexton

Copy link
Copy Markdown
Member

Hold: superseded by #12811, an owner-login replacement. Releases when: #12811 merges, then this PR closes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Hard merge gate: do not merge while applied. needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant