test(ci): pin the label-writing workflows with contract tests - #3664

hack/label-workflows-contract.bats (8)

87-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The grep -v '^typeToKind\|^scopeToArea' filter is dead code.

Line 88 selects only toAdd.add('<literal>') occurrences, because the pattern requires single quotes. Calls of the form toAdd.add(typeToKind[type]) never match. After sed, every remaining value is a label string, so no line can start with typeToKind or scopeToArea. Remove the filter, or keep it and add a comment that states it is a guard against a future literal-string regression.

♻️ Proposed change

   literal_labels="$(code_lines < "$PR_LABELER" | script_lines \
-    | grep -o "toAdd.add('[^']*')" | sed "s/toAdd.add('//; s/')//" \
-    | grep -v '^typeToKind\|^scopeToArea' || true)"
+    | grep -o "toAdd.add('[^']*')" | sed "s/toAdd.add('//; s/')//" || true)"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 87 - 89, Remove the
redundant grep -v filter from the literal_labels pipeline after extracting
toAdd.add(...) values, leaving the existing literal-label matching and sed
transformation unchanged.

---

62-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The local keyword contradicts the "POSIX-only" comment.

Line 63 states the helpers stay POSIX because hack/cozytest.sh runs the file in plain sh. Line 64 then uses local, which POSIX does not define. dash and busybox ash accept it, so this is not a runtime failure today. Either drop the POSIX claim, or remove local. The same pattern appears at lines 22 and 28.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 62 - 69, Remove the
non-POSIX local declarations from the helper functions containing the
all/missing logic and the matching declarations near lines 22 and 28. Preserve
the existing variable assignments and behavior while keeping the POSIX-only
claim accurate for hack/cozytest.sh.

---

112-114: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

The edited guard misses the YAML block-sequence form.

Line 114 inspects only lines that match ^[[:space:]]*types:. If someone rewrites the trigger as a block sequence, edited appears on its own line and the guard passes:

    types:
      - opened
      - reopened
      - edited

Line 110 would also fail in that case, so the test still fails overall. However the negative assertion itself gives no protection. Scan the whole on: section for edited instead.

♻️ Proposed change

-  ! code_lines < "$PR_LABELER" | grep -E '^[[:space:]]*types:' | grep -q 'edited'
+  # Covers both the flow-sequence and block-sequence spellings of `types:`.
+  ! code_lines < "$PR_LABELER" | grep -qw 'edited'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 112 - 114, Update the
negative assertion in the label-workflow contract test to scan the entire on:
section for edited rather than only types: lines, so both inline and YAML
block-sequence forms are detected. Preserve the existing intent that edited must
not appear as an executable trigger value.

---

48-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Hard-coded indentation makes these helpers brittle.

stale_input and stale_exempt_list require exactly 10 spaces before the key and exactly 12 spaces for folded continuation lines. A pure reformat of stale.yaml then produces an empty value, and the failure message does not explain the cause. This is acceptable for a pinning contract test. Consider matching a looser indentation prefix, or add a guard that fails with an explicit message when the key is not found.

♻️ Optional: loosen the indentation requirement

 stale_input() {
-  code_lines < "$STALE" | awk -v key="          $1: " '
-    index($0, key) == 1 { sub(key, ""); print; exit }'
+  code_lines < "$STALE" | awk -v key="$1" '
+    $1 == key ":" { sub(/^[[:space:]]*[^:]+:[[:space:]]*/, ""); print; exit }'
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 48 - 59, Update stale_input
and stale_exempt_list to avoid relying on fixed indentation when parsing STALE,
while preserving their current extraction behavior; alternatively, add explicit
guards that fail with a clear message when the expected key or folded list is
not found.

---

35-40: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Strip // comments before block detection, not after.

js_object_block finds the opening line with index($0, name) on the raw file. A commented line such as // const typeToKind = { also opens the block. The script_lines filter runs after awk, so it cannot prevent that. This weakens the stated contract that commented-out code must never satisfy an assertion.

♻️ Proposed change

 js_object_block() {
-  awk -v name="const $1 = {" '
+  script_lines < "$2" | awk -v name="const $1 = {" '
     index($0, name) { inside = 1; next }
     inside && /^[[:space:]]*};[[:space:]]*$/ { exit }
-    inside' "$2" | script_lines
+    inside'
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 35 - 40, Update
js_object_block so each input line has // comments removed before evaluating the
opening-pattern match and block boundaries. Ensure commented-out declarations
such as // const typeToKind = { never set inside, while preserving extraction of
active object blocks for subsequent script_lines processing.

---

142-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider pinning the stale.yaml trigger list.

The stated objective is to pin the trigger list for both workflows. The suite pins the pr-labeler.yaml triggers at line 110. It does not pin the stale.yaml triggers. stale.yaml runs on schedule with cron 37 4 * * * and on workflow_dispatch. A change to the cron, or the addition of a push trigger, would go undetected. Add an assertion for the cron value and for the absence of event triggers that could cause repeated writes.

♻️ Proposed additional test

`@test` "stale runs only on a daily schedule and manual dispatch" {
  triggers="$(code_lines < "$STALE" | awk '/^on:/ { inside = 1; next }
    inside && /^[^[:space:]]/ { exit }
    inside')"
  printf '%s\n' "$triggers" | grep -qE "^    - cron: '37 4 \* \* \*'"
  printf '%s\n' "$triggers" | grep -qE '^  workflow_dispatch:'
  ! printf '%s\n' "$triggers" | grep -qE '^  (push|pull_request|pull_request_target|issues):'
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 142 - 164, Extend the
contract tests around STALE workflow trigger validation, alongside the existing
pr-labeler trigger assertions, with a test that extracts the stale.yaml on block
and asserts cron remains “37 4 * * *” and workflow_dispatch is present. Also
assert push, pull_request, pull_request_target, and issues triggers are absent
so only the daily schedule and manual dispatch can run the workflow.

---

160-163: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

stale_input returns trailing inline comments as part of the value.

code_lines removes only whole-line # comments. stale_input prints the rest of the line after the key. If a maintainer documents a value inline, for example days-before-stale: 60 # two months, the value becomes 60 # two months and line 160 fails. The rest of stale.yaml uses inline comments already, such as the cron line, so this edit is likely.

♻️ Proposed change: strip trailing comments and whitespace

 stale_input() {
   code_lines < "$STALE" | awk -v key="          $1: " '
-    index($0, key) == 1 { sub(key, ""); print; exit }'
+    index($0, key) == 1 {
+      sub(key, "")
+      sub(/[[:space:]]+#.*$/, "")
+      sub(/[[:space:]]+$/, "")
+      print
+      exit
+    }'
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 160 - 163, Update
stale_input to remove trailing inline comments and surrounding whitespace from
the extracted value, not just whole-line comments handled by code_lines. Ensure
entries such as days-before-stale remain parsed as the bare value when followed
by an inline # comment, while preserving existing behavior for uncommented
values.

---

43-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Use a YAML-safe label parser.

The static $3 extraction works now, but it still rejects quoted labels and labels containing spaces. This affects labels already in .github/labels.yml such as quality-of-life, help wanted, and good first issue, so the contract test can miss future label changes. Update defined_labels() to parse the name: value instead of splitting by spaces.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/label-workflows-contract.bats` around lines 43 - 45, Update
defined_labels() to use a YAML-safe parser that extracts each name: value rather
than relying on awk’s whitespace-delimited $3 field. Ensure quoted labels and
labels containing spaces, including existing entries such as “help wanted” and
“good first issue,” are returned intact.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.