Skip to content

refactor(review): read the review-digest team config from docs conventions review-digest.yaml - #9890

Merged
melodic-automation-lanes[bot] merged 5 commits into
mainfrom
work-resolve-issue/issue-9734-40135899798-1
Oct 25, 2026
Merged

melodic-automation-lanes[bot] merged 5 commits into
mainfrom
work-resolve-issue/issue-9734-40135899798-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #9734

Summary

Opened by the work-resolve-issue lane. The run's own summary:

/review:explain-change now reads its team layer from docs/conventions/review-digest.yaml, validated by a new plugins/review/schemas/review-digest.schema.json. The old json config block and .claude/review-digest.json are read only when the YAML is absent at the base commit. This repository keeps its old block until the cleanup slice. I could not run any test suite in this lane, so the new tests are unexecuted.

Not verified

  • scripts/run-test-file.sh plugins/review/tests/explain-change.test.mjs was not run: this lane runs no test runner. The new and changed tests have never been executed.
  • scripts/check-convention-yaml.sh --check was not run (check-affected lists it as CI only), nor its test script. The new YAML against the new schema is unvalidated here.
  • scripts/check-changelog-fragments.sh --check-required origin/main was not run; the no-argument check-changelog-fragments passed. HEAD was detached, so new-changelog-fragment could not run and the fragment was written by hand in the same format.
  • Setup does not write the YAML through the shared setup-apply.mjs: it reports array-typed keys as unchecked, which would make --check fail on this file, and fixing that touches 8 synced plugin copies. The setup skill instead tells the agent to write the file by hand and read it back through digest-po
  • The 'git grep -n' check that no skill body names the old location was not run. plugins/review/skills/explain-change/SKILL.md still points to docs/conventions/review-digest.md as the owner doc for prose, not as a config location.
  • The PR body must list each migrated setting with its new home and schema: all six keys, in docs/conventions/review-digest.yaml, validated by plugins/review/schemas/review-digest.schema.json.
  • The CI-only checks listed by check-affected (convention-yaml-schema, evals-quality, cant-fail-scan, portability-lint and the others) are left to the PR's CI.
  • The new test file plugins/review/skills/setup/scripts/ was left unchanged; no executable-bit changes were possible in this lane.

Fix

  • docs/conventions/review-digest.yaml: new; carries every value of the old config block (digest_policy, max_files, max_changed_lines, blast_radius, risk_paths, opt_in_label)
  • plugins/review/schemas/review-digest.schema.json: new schema for those six keys (plus $schema), additionalProperties false
  • plugins/review/skills/explain-change/scripts/digest-policy.mjs: team layer reads the YAML first (via lib/yaml-subset.awk) and the old locations only when it is absent; the YAML path joins CONFIG_PATHS; new exported parseYamlLayer
  • plugins/review/tests/explain-change.test.mjs: tests that the YAML equals the defaults and the old block, YAML wins when both exist, old block is read when the YAML is absent, source names the layer, invalid or unparsable YAML drops the layer without consulting the old block, schema keys match reader
  • scripts/check-convention-yaml.sh: third schema lookup, plugins/*/schemas/<concern>.schema.json, so a plugin's second concern validates
  • scripts/check-convention-yaml.test.sh: two cases for that lookup
  • docs/conventions/review-digest.md: layers and the old block's status described
  • docs/conventions/config-cascade/README.md: review-digest row names the YAML as the team layer
  • plugins/review/skills/setup/SKILL.md: documents writing docs/conventions/review-digest.yaml
  • .changes/review/refactor-review-digest-yaml-6f3a9c21.md: minor changelog fragment

Verification

The runner ran each check from the base copy against d7d8711:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • /home/runner/work/_temp/lane-checks/bin/check-changelog-fragments: all 1 fragment valid
  • /home/runner/work/_temp/lane-checks/bin/check-affected: 10 passed, 6 skipped, the rest CI-only (see not_done)
  • A fresh-context subagent reviewed the diff and found two issues. One, a quoted number for opt_in_label reading as a number, is fixed. The other (the extra setup entry point refused files holding list keys) is fixed by dropping that entry point.
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40135899798

Left out: the items under Not verified.

Related

#9734

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 57dbf613916e02d3aa5b6a63e548e19793381007: 0 finding(s), 0 as inline comments.

Incremental security review of 9750096..57dbf61 (two files). I found no security issues.

  • scripts/check-convention-yaml.sh:189: the change only rewrites the "no schema" diagnostic so it names the third lookup location. $stem and the paths are passed to printf as %s arguments, not in the format string, so there is no format-string or injection exposure. Control flow is unchanged: the same findings increment and continue still run.
  • plugins/review/skills/setup/SKILL.md:54: the diff drops the sentence "The old json config block and .claude/review-digest.json are never written." It is a legacy-location note, not a guardrail. It guards no protected class such as secrets, destructive actions or trust gates. The surrounding text still tells the setup skill to write only docs/conventions/review-digest.yaml. I did not read the instruction exception register; I judged the removal by its consequence.
  • No manifest files changed in this increment, so there are no manifest <field> lines. None of the changed files match a trusted path.
  • The PR context file holds no embedded directives aimed at the review lane.

critical-findings: 0

@github-actions github-actions Bot left a comment

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.

claude-review: 1 inline finding(s) on d7d871192f0ae568bdbbe63f0d74ed1b28155b73; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread plugins/review/skills/setup/SKILL.md Resolved
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 57dbf613916e02d3aa5b6a63e548e19793381007: 0 finding(s), 0 as inline comments.

Incremental review of 9750096..57dbf61, covering plugins/review/skills/setup/SKILL.md and scripts/check-convention-yaml.sh. I found no issues.

  • setup/SKILL.md: The change drops the sentence saying the old json config block and .claude/review-digest.json are never written. A grep of plugins/review/skills shows the old location now appears only in digest-policy.mjs, the compatibility reader. That meets the closing issue's "No skill body tells an agent to read or write the old location" criterion for this file. I did not run git grep over the whole of plugins/review.
  • check-convention-yaml.sh: The change only edits the "no schema at" error message, so it names the third lookup, plugins/*/schemas/<stem>.schema.json. $stem is set at lines 168-169, before the message prints, so the format string and its arguments match. No test or other file asserts on the old message text.

I did not run any tests or scripts. The PR body's claims about unexecuted tests are author-claimed and I have not verified them.

Manifest checks: this incremental change touches no plugin manifest.

Closing issue #9734 was read and has criteria. This increment affects none of them except the skill-body one above. Whether the earlier-reviewed files meet the rest was covered at the previous review.

Nothing here can cause a real charge, so I set no hold.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of 3 on 3ee4ca3.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor Author

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

pr-merge's gate holds it on its risk score (evidence-unbound:claude-review: not bound: no-run; evidence-unbound:claude-security-review: not bound: no-run)

@github-actions github-actions Bot left a comment

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.

claude-review: 1 inline finding(s) on 0b7df5dd91d5e4f5b3efc7809c631f3909d917cc; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread plugins/review/skills/setup/SKILL.md Resolved
Comment thread scripts/check-convention-yaml.sh Resolved
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 2 of 3 on 9750096.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 25, 2026
…gin schema lookup

The setup body no longer names .claude/review-digest.json, which the
done-when grep forbids. check-convention-yaml's missing-schema message
now names the plugins/*/schemas lookup it also tries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 25, 2026
@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 25, 2026
@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 25, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 57dbf613916e02d3aa5b6a63e548e19793381007.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Oct 25, 2026
Merged via the queue into main with commit a4337d1 Oct 25, 2026
79 of 81 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-9734-40135899798-1 branch October 25, 2026 20:56
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.

refactor(review): move the review-digest fenced config block to docs/conventions/review-digest.yaml

1 participant