Summary
Follow-ups to #1589 (merged as #1591). One is a real defect the review surfaced indirectly; the rest
are the review's own non-blocking findings, which landed after the merge.
1. The telemetry payload can be falsified, not merely lost
build_data_json passes the findings array to jq -n as an --argjson value. Windows caps a process
command line at 32767 characters. Reproduced with the same jq the hooks use:
n=100 serialized 6401 B rc=0 findings preserved
n=300 serialized 19201 B rc=0 findings preserved
n=600 serialized 38401 B rc=126 <- argument list too long
n=1200 serialized 76801 B rc=126
jq never runs, and the || printf '{"tool":"","file":"","findings":[]}' fallback emits an envelope
claiming zero findings, with tool and file blanked — for the noisiest files in the repository,
which are the ones a sink is most likely wired for.
data.findings is deliberately uncapped (that is stated in the plugin's README and CHANGELOG), so this
is not a corner case: a real 324-finding file serializes well past the limit.
Telemetry is documented best-effort and lossy, so a dropped envelope is inside contract. An
envelope that arrives reporting a finding-dense file as clean is not. The fix is the same one
already applied to the classification path: feed jq on stdin. tool and file stay as arguments —
both are bounded by a path length.
The shared hook::emit_telemetry hands the finished payload to jq the same way and is tracked
separately in #1595; until that lands, an oversized envelope is dropped rather than delivered. That is
the correct failure direction and the one this fix establishes.
The existing 50-finding telemetry assertion cannot catch this — 50 findings is about 6 KB, comfortably
under the limit.
2. Carriage returns leak into the report
markdownlint-cli2 is a Node process whose stdout is CRLF-terminated on Windows, and command
substitution strips only the trailing newline. Every retained violation line therefore carries a CR
that survives JSON-escaping into additionalContext as a literal \r. Pre-existing, but it shows up
in the report format #1591 restructured.
3. Review findings from #1591 that landed after the merge
- Prune frequency.
find … -mtime +7 -delete on the digest store ran on every Markdown edit. It
should run only when a new digest file is created — the steady state for a repeatedly-edited file
already has one, so the common path need not walk the directory at all.
-maxdepth 1 on that prune. CLAUDE_PLUGIN_DATA is shared with the trust-approvals tree and
with whatever a future version of this plugin puts there. A recursive age-based -delete should not
be able to reach into a sibling's state.
- Silent histogram truncation. The rule histogram names the top five rules and stops. On a file
where twelve more rules are firing, MD013 x48 reads as the whole story. It should carry the count
of the rules it omitted, the same way the finding list already reports its own remainder.
Summary
Follow-ups to #1589 (merged as #1591). One is a real defect the review surfaced indirectly; the rest
are the review's own non-blocking findings, which landed after the merge.
1. The telemetry payload can be falsified, not merely lost
build_data_jsonpasses the findings array tojq -nas an--argjsonvalue. Windows caps a processcommand line at 32767 characters. Reproduced with the same jq the hooks use:
jqnever runs, and the|| printf '{"tool":"","file":"","findings":[]}'fallback emits an envelopeclaiming zero findings, with
toolandfileblanked — for the noisiest files in the repository,which are the ones a sink is most likely wired for.
data.findingsis deliberately uncapped (that is stated in the plugin's README and CHANGELOG), so thisis not a corner case: a real 324-finding file serializes well past the limit.
Telemetry is documented best-effort and lossy, so a dropped envelope is inside contract. An
envelope that arrives reporting a finding-dense file as clean is not. The fix is the same one
already applied to the classification path: feed
jqon stdin.toolandfilestay as arguments —both are bounded by a path length.
The shared
hook::emit_telemetryhands the finished payload tojqthe same way and is trackedseparately in #1595; until that lands, an oversized envelope is dropped rather than delivered. That is
the correct failure direction and the one this fix establishes.
The existing 50-finding telemetry assertion cannot catch this — 50 findings is about 6 KB, comfortably
under the limit.
2. Carriage returns leak into the report
markdownlint-cli2is a Node process whose stdout is CRLF-terminated on Windows, and commandsubstitution strips only the trailing newline. Every retained violation line therefore carries a CR
that survives JSON-escaping into
additionalContextas a literal\r. Pre-existing, but it shows upin the report format #1591 restructured.
3. Review findings from #1591 that landed after the merge
find … -mtime +7 -deleteon the digest store ran on every Markdown edit. Itshould run only when a new digest file is created — the steady state for a repeatedly-edited file
already has one, so the common path need not walk the directory at all.
-maxdepth 1on that prune.CLAUDE_PLUGIN_DATAis shared with thetrust-approvalstree andwith whatever a future version of this plugin puts there. A recursive age-based
-deleteshould notbe able to reach into a sibling's state.
where twelve more rules are firing,
MD013 x48reads as the whole story. It should carry the countof the rules it omitted, the same way the finding list already reports its own remainder.