fix(ratchet): read a JSON key's $comment marker on the line below it

[ghost]

What broke

The release PR for plugin v2.23.3 (#1722) failed the required Legacy-name ratchet check, and a fixup commit was needed before it could ship.

• Why: JSON has no comments, so the legacy-name-ok marker for the MEMCLAW_AGENT_ID alias lives in a "$comment" member of the object the key opens. On main that member shares the key's line. release-please re-serialises every JSON file it bumps, which put the member on the line below.
• What the ratchet saw: it reads markers per line, so the key read as a newly minted, unmarked legacy name, and the marked line read as removed. Nothing had changed except the layout.

Any JSON formatter does the same, so this would come back at every plugin release.

The fix

For JSON files, the engine now joins two lines, with one space:

• a key that is alone on its line and opens an object, and
• a "$comment" member directly below it that carries one of this tool's markers.

That joined text is exactly the one-line form, so both layouts read as the same line. The key's marker has to be in its object's first member, directly below the key. A "$comment" further down, or the same layout in a file that isn't .json, does not exempt the key. The module docstring now says where a JSON marker goes.

A "$comment" without a marker is not joined at all. It documents the key, and joining it made every edit to that documentation read as a newly minted line. Adding or rewording one below a branded key failed the gate, where main's engine passes. No JSON file in the tree has such a pair today, so this was latent.

The pair is one line everywhere else too:

• Counted once. When the comment's reason names the brand as well, git grep also returns the comment line on its own. That counted one alias twice in the exempt-line reports. A comment read with its key is no longer counted again. Only a comment actually joined to a matched key is skipped, so a branded comment below an unbranded key still counts.
• Listed when it changes. The exempt-line report picks lines by git's diff. When a marker is written below an existing key, git calls only the comment line added, so the key was exempted without being listed. A change to the comment now counts as a change to its key.

The manifest stays as it is. The next plugin release will split the line, and that now changes nothing.

Evidence

• Real commit: release-please's original chore: release main #1722 commit (9d65875e), checked against its base:

  engine	result
  main's	fails: plugin/openclaw.plugin.json (1 -> 2)
  this PR's	No new lines.

  It still lists the rewritten line under "exempt line(s) written". That is a report, not a failure.

• The first review's scenarios, where the comment's reason names the brand. Exempt lines reported, on this PR's first revision and now; the gate passes in all four runs:

  scenario	written	removed
  release-please's move from one line to two	2 → 1	0 → 0
  the reason edited, below its key	1 → 1	2 → 1

• New tests: eleven, one of them run for both layouts.

  • Five cases for the new behaviour fail against main's engine: a marker directly below its key; a layout change from one line to two; a comment that also names the brand, counted once; a marker added below an existing key, listed; a reworded reason below its key, reported.
  • A reworded reason is reported the same way on the key's line, and that case passes on main too. Rewording a reason is a change to the marker's claim, so both layouts report one line removed and one written.
  • Two pin the fix above: adding, and rewording, an unmarked "$comment" below a branded key mint nothing. They pass on main and failed on this PR's previous revision.
  • Three bound it and pass either way, as they should: a comment further down, a comment with no marker, a non-JSON file.
  • One guards the de-duplication: a branded comment below an unbranded key still fails the gate. It passes either way, and fails against a variant that skips comment lines by layout alone.

• Mutation check: four mutants each fail exactly their own tests: no de-duplication, no diff mapping, de-duplication by layout alone, and joining any comment.

• Other checks:

  • the ratchet test suite: 204 passed, 1 skipped;
  • ruff check and ruff format --check on scripts/ and tests/, and mypy on scripts/. Nothing was reformatted, since this file is byte-sensitive;
  • the ratchet on this branch: no new lines;
  • the sentinel: all 35 protected strings survive.

After merging

The org-required check runs the engine pinned at ratchet-stable, which is currently ccd11eb8, identical to main's engine before this PR. Move it to this PR's merge commit, or the next plugin release fails the same way.

🤖 Generated with Claude Code

[github-actions]

🤖 Review by Claude Code

Double-counts a JSON alias when its $comment reason text also names the legacy brand

Severity: Medium
File: scripts/legacy_name_ratchet.py:901-930, 993-995
Problem: _with_json_comment joins a JSON key line with the "$comment" line below it, but git grep still returns the "$comment" line as its own separate match whenever its reason text itself mentions the legacy name (e.g. "$comment": "legacy-name-ok: alias for memclaw compatibility"), so one alias ends up counted/reported twice.

🤖 Claude Code Prompt

In scripts/legacy_name_ratchet.py, `_grep` (around lines 933-996) calls
`_with_json_comment(tree, path, lineno, text)` (defined at lines 921-930) for
every matched line independently. When a JSON key-opening line such as
`"MEMCLAW_AGENT_ID": {` is immediately followed by a `"$comment"` member whose
own reason text also contains the legacy literal (a very natural way to write
an alias justification, e.g. `"$comment": "legacy-name-ok: alias for legacy
memclaw clients"`), `git grep` returns TWO separate records for the same
logical entity:

1. the key line, joined by `_with_json_comment` with the comment line below it
   into one combined "text" (correctly exempt), and
2. the standalone `"$comment"` line itself, which independently contains the
   legacy literal in its own reason text and is therefore returned again as
   its own match, unmodified by `_with_json_comment` (since
   `_JSON_KEY_OPENING_OBJECT.fullmatch` does not match a bare "$comment" line).

Both records get classified via `_kind()` and both are exempt, so they both
land in `Scan.exempt_by_file`/`Scan.by_file` as two different Counter keys
(the joined text vs. the raw comment text) for what is really one alias
declaration. This inflates:
- the "N compat alias(es)" counts printed by `_report_new_exemptions` and
  tallied by `_marker_inventory`,
- the per-file exempt/gated Counters that `_minted`/`_mint_budget` and the
  moved/removed-exemption reports (`_report_removed_exemptions`,
  `_report_excused_moves`) rely on for text-identity matching across trees,
  which can produce spurious "moved"/"removed" entries when the duplicate
  structure changes between base and head (e.g. the key or comment is edited,
  not just relocated).

Fix by deduplicating: when `_with_json_comment` successfully joins a key line
with the comment line at `lineno + 1`, have `_grep` remember that
(path, lineno + 1) was consumed as part of the joined record, and skip
emitting a standalone record for that same (path, line) if `git grep` also
returns it as an independent match later in the same pass. Add a regression
test (in tests/test_legacy_name_ratchet.py) using a `$comment` reason that
itself contains the legacy literal, asserting the report/counts show exactly
one exemption rather than two.

---

Reviewed by claude-sonnet-5 · cost $1.1457315

[ghost]

@claude please re-review 45e07c2.

The double count was real. With a $comment reason that names the brand, the exempt-line report listed two lines for one alias, and editing that reason reported two removed. A comment line is now skipped when git grep returns it again, but only when it was actually joined to a matched key, so a branded comment below an unbranded key still counts.

Skipping it alone would have hidden reason edits from the exempt-line report. That report picks lines by git's diff, and the diff names the comment's line, not the key's. The same gap already let a marker added below an existing key go unlisted. _added_lines now counts a changed comment as a change to its key.

Three new tests. Each one fails against a mutant that removes its part of the fix.

[github-actions]

🤖 Review by Claude Code

Summary

The PR extends legacy_name_ratchet.py to treat a JSON "$comment" member on the line directly below a matching key as part of that key's text, so the marker convention works in JSON (which has no inline comments). The mechanism is well documented and has solid test coverage for the intended alias-marker scenarios, but the join is performed unconditionally on any "$comment" member — not only ones that actually carry one of this tool's recognized markers — which creates a realistic false-positive/false-negative footgun.

Critical/High Issues

JSON $comment joining is unconditional, causing spurious "new mint" failures on unrelated schema comments

Severity: High
File: scripts/legacy_name_ratchet.py:924-940 (_json_comment_below), scripts/legacy_name_ratchet.py:998-1004 (_grep)
Problem: _json_comment_below merges any "$comment" member below a matching key into that key's counted text regardless of whether the comment contains a recognized marker (legacy-name-ok/-floor/-absent/-deferred), so adding, editing, or removing an ordinary JSON-Schema $comment (a standard, generic keyword, not exclusive to this tool) next to an already-tracked legacy-named key changes the line's compared text and can make the ratchet report a brand-new "mint" for a line whose legacy-name usage never actually changed.

🤖 Claude Code Prompt

In scripts/legacy_name_ratchet.py, `_json_comment_below` (around lines 913-935) joins
any "$comment" JSON member found directly below a key-opening line into that key's
grepped text, and `_grep` (around lines 998-1004) unconditionally performs this join
before computing `_kind(text)`. Because the join happens regardless of whether the
comment text actually contains one of this tool's markers (EXEMPT_MARKER,
FLOOR_MARKER, ABSENT_MARKER, DEFERRED_MARKER), any edit to an ordinary,
marker-unrelated "$comment" field (a standard JSON Schema keyword used broadly for
documentation) sitting below a key whose own name contains the legacy brand will
change the compared "text" for that key between base and head. Since the ratchet's
core algorithm (`_mint_budget`/`_minted` in the same file) fails a file when it gains
text the repo did not have before and there is no byte-identical predecessor
elsewhere to pair the change with as a "move", this will spuriously fail the gate for
a change that has nothing to do with renaming or aliasing work — e.g. someone
tidying up an unrelated "$comment" description under a
"MEMCLAW_LEGACY_PORT"-style key.

Fix this by only treating the "$comment" line as belonging to the key when its
content actually resolves to one of this tool's markers — e.g. check
`_kind(comment) is not None` (or `EXEMPT_RE`/`DEFERRED_RE` match on the comment text)
inside `_json_comment_below` before returning the comment line, so an unrelated
"$comment" is left as ordinary standalone text (matched only if it independently
contains the legacy pattern) rather than being fused into the key's line and
changing its identity for the base/head text comparison. Add regression tests
mirroring the existing repo() fixture pattern (not a brand-new file) that: (1) commit
a legacy-named key with no comment, then in a later change add an unrelated
non-marker "$comment" below it, asserting the gate still passes with no reported
mint; and (2) commit a marked key+comment pair, then edit only unrelated wording in
an already-marked comment while keeping the marker literal untouched, asserting no
spurious "exempt line(s) removed"/"written" churn is reported for content that carries
no marker at all.

---

Reviewed by claude-sonnet-5 · cost $1.6007505

[ghost]

@claude please re-review ded93f6.

The unconditional join was real. Adding or rewording an unmarked "$comment" below a branded key failed the gate, where main's engine passes. _json_comment_below now returns the comment only when it carries one of this tool's markers (EXEMPT_RE or DEFERRED_RE). The join and the diff mapping both go through it, so they cannot disagree. Two new tests cover your case (1): they pass on main and failed on the previous head.

I have not adopted case (2). A reworded reason in a marked comment is still reported, as one line removed and one written. The reason is the marker's claim, and a reworded reason on the key's own line is reported the same way on main today. A new test runs both layouts and asserts the same report. The one-line case passes on main as well, so this keeps the two layouts consistent rather than adding churn.

[github-actions]

🤖 Review by Claude Code

Consistent with the design. Given the depth of analysis, the implementation appears correct and well-tested. I did not find genuine correctness or security defects introduced by this diff.

Claude Code Review ✅ No issues found.

---

Reviewed by claude-sonnet-5 · cost $1.0647167499999999

[github-actions]

Claude Code Review — skipped: PR author 'eldad-caura-ai' is not a public member of the 'caura-ai' org