fix(ci): portability-lint: anchor is_comment to block-level HTML comments

[kyle-sexton]

Summary

• scan_file's awk block (scripts/check-skill-portability.sh) treated any line
  containing an HTML comment-open marker (<! followed by --) as a comment
  line for pending_annot carry-forward purposes — including a CONTENT line
  that merely happens to carry an inline comment marker mid-line (e.g.
  diff against origin/main <!-- some other note -->) and is not itself
  annotated with portability-ok:.
• Because a non-annotated "comment" line left pending_annot unchanged instead
  of resetting it to 0, such a content line could silently extend a prior
  genuine annotation's coverage past itself to a later, unannotated line — a
  false-negative risk in the fail-closed gate.
• Fix: anchor the HTML-comment branch of is_comment() to require the marker
  at the start of the line (modulo leading whitespace) — block-level only —
  per the reviewer's suggested fix recorded on source-control(ci):portability-lint: pending_annot carry-forward doesn't distinguish inline vs block HTML comments #611, so only a genuine
  dedicated comment line can set or carry pending_annot.

Test plan

• Added a regression case to scripts/check-skill-portability.test.sh
  reproducing the reported scenario: an annotated comment line, followed by a
  content line with a hardcoded token and an unrelated inline comment marker
  mid-line (diff against origin/main here <!-- unrelated inline note, not an annotation -->), followed by a third unannotated content line with the same
  hardcoded token. Confirmed the new case fails against the pre-fix script
  (line 3 wrongly excused) and passes after the fix (line 3 correctly flagged,
  line 2 still excused via annotated_above).
• Full self-test suite: PASS=17 FAIL=0 (was PASS=16 before this change; the
  16 pre-existing cases still pass unchanged).
• shellcheck (repo .shellcheckrc) and shfmt -d clean on both modified
  files.

Closes #611

Related

• feat(ci): add portability-lint gate for ecosystem/forge-agnostic skills (#531) #609 — the portability-lint gate this fix hardens (post-green review
  deferred this finding out of that PR's scope to keep its diff scoped to the
  gate's initial ship).
• portability-lint: is_comment anchor doesn't recognize Markdown-list/blockquote-nested block comments (found on PR #1338, fixes #611) #1342 — follow-up filed from this PR's own post-green review (Codex bot):
  the ^[[:space:]]*<!-- anchor doesn't recognize a block comment nested
  inside a Markdown list/blockquote container (e.g. - <!-- portability-ok: ... -->). Verified not a live regression (no portability-ok: usage
  exists in the corpus today); deferred as new scope beyond source-control(ci):portability-lint: pending_annot carry-forward doesn't distinguish inline vs block HTML comments #611's literal
  acceptance criteria.
• portability gate: escape-hatch grammar rigor — standalone-comment-only matching + required nonempty reason (pair from PR #609 re-review) #624 — sibling escape-hatch grammar rigor pair from PR feat(ci): add portability-lint gate for ecosystem/forge-agnostic skills (#531) #609's re-review
  (same file/area, also deferred, needs-human).

🤖 Generated with Claude Code

[claude]

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

---

PR Review

• Gather context
• Review changed files
• Analyze correctness and test coverage
• Post findings

---

Summary

Correct, minimal fix. The one-character anchor (`` — would suppress the pending_annot = 0 reset, silently extending the prior annotation's coverage to the next line.

The new regex ^[[:space:]]* | 0 | ✓ (anchored `` | 1 | ✗ (<!-- not at start) | 0 | excused via `annotated_above` |
| 3 | `diff against origin/main again, unannotated` | 0 | ✗ | 0 | flagged ✓ |

This matches the PR description exactly.

Code comment — the inline awk comment explaining the rationale and citing #611 is helpful and appropriately scoped. No issues.

---

scripts/check-skill-portability.test.sh

The regression test is correct and well-structured.

The test fixture reproduces the exact three-line scenario from #611. The assertion logic:

elif echo "$out" | grep -q ":3:" && ! echo "$out" | grep -q ":2:"; then
  ok "..."

correctly requires line 3 to be flagged and line 2 to be clean. This is consistent with how all other test cases in the suite use the FNR:-prefixed output format.

One theoretical minor observation: grep -q ":3:" could match :3: appearing inside the token pattern or the content text rather than as the line-number prefix. In practice it is safe here because neither origin/main nor the prose in the fixture contains :3:. The existing suite uses this pattern uniformly, so it's consistent with established style.

The test's description in the comment block is thorough and accurately describes both the pre-fix failure mode and the expected post-fix behavior.

Test count — PR reports PASS=17 FAIL=0 (was 16), consistent with adding one case.

---

Overall

No issues found. The fix is correct, the test faithfully reproduces the failure scenario, and both files are clean of unrelated changes. Ready to merge.

[claude]

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

---

Security Review

• Gather PR diff and changed files
• Analyze check-skill-portability.sh changes for security issues
• Analyze check-skill-portability.test.sh changes for security issues
• Post findings

---

No security issues found.

This PR makes a single, tightly scoped fix to scripts/check-skill-portability.sh and adds a regression test. The change narrows a hardcoded awk regex inside the script itself — no user-controlled input touches the changed code path.

Surfaces examined and cleared:

• Regex hardened, not opened: The is_comment() regex changes from l ~ /<!--/ to l ~ /^[[:space:]]*<!--/. This is a purely additive constraint on a hardcoded awk pattern. It cannot be injected into or weaponised — it is not derived from file content, environment variables, or any external input.

• False-negative closure is a security improvement: The pre-fix bug let a content line containing a mid-line <!-- silently propagate pending_annot = 1 to the next line, potentially excusing an unannotated lint violation. The fix closes that gap, making the fail-closed gate correctly stricter.

• No new command injection surface: The awk invocation passes "$TOKENS" and "$file" as awk input files — not interpolated into the awk program string. The changed code adds no new shell word-splitting, eval, or $() expansion of untrusted data.

• Test scaffolding is secure: New test fixture content is passed as a single-quoted literal to tmpfile(), written via printf '%s\n' "$1" to a mktemp-created file, and cleaned up with rm -f. No predictable temp-file names, no race window, no shell expansion of fixture content.

• No workflow files modified: No pull_request_target, workflow_run, permissions, secrets, or action pin changes.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8ff28d4d4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".