fix(actionlint): align emitted telemetry hook id with schema (#959)

[kyle-sexton]

Summary

The actionlint-check hook emitted a telemetry envelope whose hook id was "actionlint", but the published schema and discovery convention key on the hook-script basename, actionlint-check. A consumer sink resolving data/<hook>.schema.json from the envelope's hook value would look up data/actionlint.schema.json (which does not exist) instead of the real data/actionlint-check.schema.json. This is a producer-conformance bug: three authoritative artifacts (the schema file, the README Implementers table, and the schema's own title/description) already agree on actionlint-check; only the producer and its own test diverged.

Fix

Producer-side only — 4 lines across 2 files:

• plugins/actionlint/hooks/actionlint-check.sh — "actionlint" → "actionlint-check" at all three emit_tel sites (skipped / findings / clean paths).
• plugins/actionlint/hooks/actionlint-check.test.sh — flipped the .hook assertion and its success-message string to expect "actionlint-check".

No change to docs/conventions/hook-telemetry/ schema or README — they are already canonical. No schema_version bump (the published envelope/data contract is unchanged). The plugin's own version is bumped 0.5.0 → 0.5.1 (patch, producer-conformance bugfix) with a matching CHANGELOG entry.

Verification

Acceptance grep (expect empty):

$ git grep 'emit_tel "actionlint"' plugins/actionlint/
$ echo $?
1        # no match — all three sites converted

Test suite (actionlint was on PATH, so it ran fully — not skipped):

$ bash plugins/actionlint/hooks/actionlint-check.test.sh
...
ok: envelope: hook is actionlint-check
...
PASS=35 FAIL=0

Closes #959

Related

Implementation follows the triage-lane planning comment on #959 (posted 2026-07-22T07:00:13Z), "Implementation plan — #959: align actionlint telemetry hook id", which resolved the canonical-direction fork from evidence (envelope hook id is the hook-script basename, proven by the guardrails plugin shipping many hooks each emitting its own basename).

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

[claude]

Claude finished @kyle-sexton's task in 1m 35s —— View job

---

Code Review

• Gather diff and context
• Read changed files in full
• Verify schema/convention alignment
• Post findings

---

Verdict: Approve — correct, minimal, well-documented producer-conformance fix.

What the PR does

All three emit_tel call sites in actionlint-check.sh were passing "actionlint" as the hook envelope field. The published schema file is data/actionlint-check.schema.json and the README Implementers table maps actionlint-check as the canonical id. The schema's own description makes the expectation explicit: "Discovered from the envelope hook value \"actionlint-check\"". A consumer resolving data/<hook>.schema.json would land on a non-existent path. This is a genuine conformance bug.

Changes reviewed

File	Assessment
actionlint-check.sh — L105, L136, L142	All three emit sites corrected. Verified with grep — no bare "actionlint" string remains at any emit_tel call. ✓
actionlint-check.test.sh — L204	Assertion updated to "actionlint-check". The stub-sink + violation path now correctly guards the hook field. ✓
plugin.json	Version bumped 0.5.0 → 0.5.1. Patch is correct — no envelope/data contract change. ✓
CHANGELOG.md	Entry is accurate and well-written; consistent with existing entries (no comparison link footer, matching prior convention). ✓

One minor test coverage gap (non-blocking)

The "actionlint-absent" telemetry block (L262–L265) asserts .status == "skipped" but does not assert .hook. Similarly the clean-file telemetry block (L221–L226) only checks .status and data.findings.length. The violation-path test catches the hook id on two of three code paths; the skipped and clean paths are not exercised by the .hook assertion. The fix itself is correct — this is just a future-proofing gap. Fix this →

Summary

The root cause, the fix scope, the version bump, and the CHANGELOG entry are all correct. The change is precisely scoped — only the producer is touched; the schema and convention docs are already canonical and untouched. No security, correctness, or architecture concerns.

[claude]

Claude finished @kyle-sexton's task in 1m 23s —— View job

---

Security Review

• Gather context and understand the request
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found in this PR.

The diff is narrow and low-risk:

• actionlint-check.sh L105, L136, L142 — the only change is replacing the hardcoded string literal "actionlint" with "actionlint-check" in three emit_tel calls. Both values contain only alphanumeric characters and a hyphen. The string is passed to hook::emit_telemetry as hook_id, which feeds it to jq via --arg hook "$hook_id" — a safe, injection-proof path regardless.
• actionlint-check.test.sh L204 — a test assertion string comparison updated to match. No execution surface.
• plugin.json and CHANGELOG.md — metadata/documentation only.

No new code paths, no new external inputs consumed, no trust-boundary changes, no permission widening, no supply-chain modifications.

[claude]

Claude finished @kyle-sexton's task in 1m 0s —— View job

---

Code Review

• Gather diff and context
• Read changed files in full
• Verify schema/convention alignment
• Post findings

---

Verdict: Approve — correct, minimal, well-documented producer-conformance fix.

What the PR does

All three emit_tel call sites in actionlint-check.sh were emitting "actionlint" as the envelope hook field. The published schema at data/actionlint-check.schema.json makes the expectation explicit in its own description: "Discovered from the envelope hook value \"actionlint-check\"". A consumer resolving data/<hook>.schema.json from the envelope would try data/actionlint.schema.json, which does not exist. This is a genuine conformance bug, and the fix is in exactly the right place.

Changes reviewed

File	Assessment
actionlint-check.sh L105, L136, L142	All three emit sites corrected — skipped, findings, and clean paths. No bare "actionlint" string remains at any emit_tel call. ✓
actionlint-check.test.sh L204	Violation-path assertion flipped to "actionlint-check". Stub-sink test now guards the hook field correctly. ✓
plugin.json	Version bumped 0.5.0 → 0.5.1. Patch is correct — no envelope/data contract change. ✓
CHANGELOG.md	Accurate, well-written entry; consistent with existing convention (no comparison link footer). ✓

One minor test coverage gap (non-blocking)

The clean-file telemetry block (L221–L226) and the actionlint-absent block (L262–L265) each assert .status but neither asserts .hook. The fix is correct in source across all three code paths, but only the violation path has a test that would catch a regression on the hook id field. Fix this →

Summary

Root cause, fix scope, version bump, and CHANGELOG entry are all correct. The schema, convention docs, and README are already canonical and are correctly left untouched. No security, correctness, or architecture concerns.

[claude]

Claude finished @kyle-sexton's task in 1m 11s —— View job

---

Security Review

• Gather context and understand the request
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found in this PR.

The diff is extremely narrow — all substantive changes are swapping one hardcoded string literal for another.

Analysis by file

actionlint-check.sh L105, L136, L142
The only change at each of the three emit_tel sites is replacing the string literal "actionlint" with "actionlint-check". Both values consist solely of lowercase ASCII letters and a hyphen. The argument is passed into emit_tel → hook::emit_telemetry as hook_id and ultimately into jq via --arg hook "$hook_id" — a parameter-safe binding, not string interpolation. No injection surface exists regardless of the value, and there is no user-controlled or attacker-reachable input on this path.

actionlint-check.test.sh L204
A string comparison in a test assertion is updated to match the new hook id. No execution surface; no security impact.

plugin.json and CHANGELOG.md
Metadata and documentation only.

Areas specifically checked

Area	Verdict
Command / template injection	Not applicable — changed value is a hardcoded literal with no shell-special characters
Untrusted input reaching new code paths	No new code paths introduced
Secret / credential handling	No secrets touched
Permission widening	No permissions fields modified
GitHub Actions patterns (pull_request_target, script injection via github context)	No workflow files changed
Supply-chain / dependency pin changes	No pins modified; version bump is in plugin.json only
Trust-boundary changes	None

[claude]

Claude finished @kyle-sexton's task in 1m 56s —— View job

---

Code Review

• Gather diff and context
• Read changed files in full
• Verify schema/convention alignment
• Check test coverage
• Post findings

---

Verdict: Approve — correct producer-conformance fix, and the version bump is cleanly resolved after the main merge.

What changed since the last review pass

The previous two review passes reviewed the original commit (367dc86). Since then, the branch picked up a merge commit (ec203ad) to incorporate origin/main. That merge brought in 0.5.1 from a separate fix (#903 — guardrails git-option parser). The conflict was correctly resolved by bumping this PR's version to 0.5.2 with its own CHANGELOG entry. The core fix is unchanged.

Changes reviewed

File	Assessment
actionlint-check.sh L105, L136, L142	All three emit_tel sites emit "actionlint-check". git grep 'emit_tel "actionlint"' returns empty — no bare id remains. ✓
actionlint-check.test.sh L204	Violation-path assertion updated to "actionlint-check". ✓
plugin.json	Version 0.5.1 → 0.5.2. Correct — 0.5.1 was already taken on main by the guardrails sync (#903). ✓
CHANGELOG.md	## [0.5.2] entry is accurate; ## [0.5.1] for the guardrails sync is preserved correctly below it. ✓

One note: PR body is now slightly stale (non-blocking)

The PR description says "version is bumped 0.5.0 → 0.5.1" but the actual diff against main is 0.5.1 → 0.5.2. The code and CHANGELOG are correct; only the PR body description is behind. Worth a squash-message update when merging, but not a code issue. Fix PR description →

Persistent test coverage gap (non-blocking, previously noted)

The clean-file telemetry block (L221–L226) and actionlint-absent block (L262–L265) each assert .status but not .hook. The fix is correct in source across all three code paths; only the violation path has a regression guard on the hook id. Previously flagged in both earlier review passes — still non-blocking. Fix this →

Summary

Root cause, fix scope, and version resolution are all correct. The merge from main was handled cleanly — 0.5.2 is the right version, and the CHANGELOG reflects both this fix and the prior 0.5.1 guardrails sync. No security, correctness, or architecture concerns.

[claude]

Claude finished @kyle-sexton's task in 1m 8s —— View job

---

Security Review

• Gather context
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found in this PR.

The diff is the narrowest possible: swapping one hardcoded string literal for another at three call sites plus one test assertion. Analysis by file:

actionlint-check.sh L105, L136, L142

Each of the three emit_tel sites changes the first positional argument from "actionlint" to "actionlint-check". Both values consist solely of lowercase ASCII letters and a hyphen — no shell-special characters. The argument flows to hook::emit_telemetry as hook_id and into jq via --arg parameter binding (not string interpolation), so there is no injection surface regardless of the value. No new code paths are introduced; the control flow is unchanged on all three branches (skipped / findings / clean).

actionlint-check.test.sh L204

A string comparison [[ "$(jq -r '.hook' "$TEL")" == "actionlint-check" ]] — the RHS is a hardcoded literal. No execution surface; no security impact.

plugin.json and CHANGELOG.md

Metadata and documentation only. No permission fields, no dependency pins, no trust-boundary changes.

Areas specifically checked

Area	Verdict
Command / template injection	Not applicable — changed value is a hardcoded literal with no shell-special characters
Untrusted input reaching new code paths	No new code paths introduced
Secret / credential handling	No secrets touched
Permission widening	No permissions fields modified
GitHub Actions patterns (pull_request_target, script injection via github context)	No workflow files changed
Supply-chain / dependency pin changes	No pins modified; version bump is in plugin.json only
Trust-boundary changes	None