Skip to content

fix(lib): strict parse-concern-value reports a duplicate key as a parse error - #8175

Merged
kyle-sexton merged 4 commits into
mainfrom
work-resolve-issue/issue-7331-39106099748-1
Oct 19, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
work-resolve-issue/issue-7331-39106099748-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #7331

Hold: a human merges this

Summary

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

lib/yaml-subset.awk gains an opt-in STRICT variable that makes a key repeated at the same full dotted path a parse error at the second occurrence's line. parse-concern-value.sh --strict sets it, so it exits 3 on such a file. Without STRICT, output is unchanged. Copies were regenerated and a patch fragment added for each of the 14 carrying plugins.

Not verified

  • parse-concern-value.test.sh was not re-run after the document-marker change to the awk reader; the change is gated on STRICT and the awk test file passes
  • Fragments were written by hand because new-changelog-fragment refuses a detached HEAD; check-changelog-fragments accepted them
  • Fresh-context review found one false positive (document markers); fixed. Build/lint and class audits beyond the above were not run separately (shellcheck not run)
  • sync-shared-copies --check-bump runs only in CI

Fix

  • lib/yaml-subset.awk: STRICT=1 duplicate-key check for block and flow mappings, reset at document markers
  • lib/parse-concern-value.sh: passes STRICT from --strict; header documents it
  • lib/yaml-subset-awk.test.sh: tests for duplicate, nested, flow, two parents, two sequence items, document marker, within one item, and no-STRICT behavior
  • lib/parse-concern-value.test.sh: tests that --strict exits 3 naming the second line and non-strict keeps the first value
  • plugins/*: 31 generated copies regenerated by sync-shared-copies
  • .changes/<plugin>/fix-strict-duplicate-key-7331a1b2.md: patch fragment for each of the 14 plugins

Verification

The run reports:

  • run-test-file lib/yaml-subset-awk.test.sh: all 25 checks passed
  • run-test-file lib/parse-concern-value.test.sh: all 110 checks passed (run before the document-marker fix, which touched only the awk reader and its test)
  • sync-shared-copies --check: all 493 registered copies match
  • check-changelog-fragments: all 16 fragments valid (run before the last commit; that commit added none)
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/39106099748

Left out: the items under Not verified.

Related

#7331

melodic-automation-lanes Bot and others added 2 commits October 18, 2026 09:47
…se error

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

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

No security issues found in the incremental change to plugins/docs-hygiene/skills/compress/scripts/articles-setting.sh (since 52b7d3a).

The diff moves the duplicate-key check ahead of the strict reader call and removes the later count > 1 check, which that earlier exit made dead.

  • Injection: KEY and REL are constants. $file comes from git rev-parse --show-toplevel joined to REL. All arguments are quoted, and file content is only printed or passed through sed, never evaluated.
  • Control flow: a duplicate key still yields invalid<TAB>set N times, now without reaching the reader. Output for the other cases is unchanged. No bypass or new trust boundary.
  • Instruction surface: the diff removes no guardrail or standing instruction. I did not consult the instruction exception register, because this change doesn't touch an instruction surface.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 8f93e6e0b177fd0b28dbeee40a61b79e208e4da6: 0 finding(s), 0 as inline comments.

No findings. I reviewed only the incremental change to plugins/docs-hygiene/skills/compress/scripts/articles-setting.sh since 52b7d3a.

The change moves the duplicate-key check ahead of the strict reader call and deletes the later count > 1 check. The deleted check could not fire. A count above 1 already exits earlier. The count=1 fallback only runs when the count was 0. For a duplicated key the output is the same as before (invalid<TAB>set N times), and the script no longer runs the reader in that case. The other paths (does not parse, unset, keep/cut, and the raw-value invalid line) are untouched.

I did not run articles-setting.test.sh, because the sandbox blocked the command. The behavior claim above comes from reading the diff, not from a test run. The PR description's verification claims are the author's and I did not confirm them. They also predate this commit.

I did not re-review the other files, which are unchanged since the last review. The closing issue (#7331) is about --strict in the awk reader, and this commit does not touch that.

…e parse error

The strict reader now rejects a duplicate key with exit 3, so
articles-setting.sh printed "does not parse" where its contract and
test say "set N times". Check the duplicate count first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AQ9amjLiLKnwj4WGMRcofR
Comment thread plugins/docs-hygiene/skills/compress/scripts/articles-setting.sh
…strict read

Move the duplicate-key check above the strict reader call and drop the
unreachable second count > 1 branch, leaving one place that reports
"set N times".

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

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

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit f29ffff Oct 19, 2026
28 checks passed
@kyle-sexton
kyle-sexton deleted the work-resolve-issue/issue-7331-39106099748-1 branch October 19, 2026 02:17
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.

lib/yaml-subset.awk keeps the first duplicate key where the CI parser rejects the file

1 participant