Skip to content

feat: issue labeler logic moves to scripts/issue-labeler.js with a durable test #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TzuH-Hsu merged 1 commit into from
Sep 16, 2026
Merged

feat: issue labeler logic moves to scripts/issue-labeler.js with a durable test #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

# Example: Require infrastructure ownership
# /scripts/bootstrap.sh @infra-owner
# /scripts/issue-labeler.js @infra-owner

# Example: Solo dev auto-request
# * @your-username
229 changes: 23 additions & 206 deletions .github/workflows/issue-labeler.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
name: Issue labeler

# Syncs priority:*, area:* and the four type:* SUBTYPE labels from
# the issue form fields on open/edit. The issue body is untrusted input — it is
# only ever read inside the actions/github-script JS sandbox
# (context.payload.issue.body), never interpolated into a shell `run:` step.
# Syncs priority:*, area:* and the four type:* SUBTYPE labels from the issue
# form fields on open/edit. The logic lives in scripts/issue-labeler.js
# (ADR-0008); this file only checks out the default branch and calls it, so
# it carries no adopter values and can be replaced whole on upgrade. The
# script is tested by `make check` (scripts/check-issue-labeler.sh).
#
# "the four type:* subtype labels", not "type:*": the type: namespace also
# carries labels no form produces, so this workflow owns an exact list rather
# than a prefix. See FORM_MANAGED_TYPES below.
# The issue body is untrusted input — it is only ever read inside the
# actions/github-script JS sandbox (context.payload.issue.body), never
# interpolated into a shell `run:` step, and every label the script adds is
# validated against an allowlist the body cannot influence.
#
# area:* is the family adopters are told to rename, so its allowlist is not
# written here: it is read from .github/labels.yml on the default branch at
# run time (see loadAllowedAreas below). This file carries no adopter values
# and can be replaced whole on upgrade.
# Why a checkout on an issue event: the script and .github/labels.yml (the
# run-time source of the area:* allowlist) live in the repository. On `issues`
# events GITHUB_SHA is the head of the default branch, so this checks out
# exactly the code and label declarations that were merged — never a PR
# branch's copy.

on:
issues:
Expand All @@ -34,203 +37,17 @@ jobs:
timeout-minutes: 5
permissions:
issues: write
contents: read # getContent on .github/labels.yml
contents: read # checkout of the default branch
steps:
- name: Check out the default branch
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
fetch-depth: 1

- name: Sync form-managed labels
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
// Parses the GitHub issue-form markdown body (### <Label> headings)
// and syncs the form-managed labels to match. Only ever touches
// priority:*, area:* and the four type:* subtypes listed
// in FORM_MANAGED_TYPES — every other label on the issue is left
// alone, including any type:* label OUTSIDE that list (a coarse
// type:bug/type:feature, or an adopter's own). The four form-owned
// subtypes are still removed when the form no longer selects them;
// that is the point of the sync.
const body = context.payload.issue.body || '';

// Split the body into sections keyed by "### <Heading>".
function parseSections(text) {
const lines = text.split(/\r?\n/);
const sections = {};
let current = null;
for (const line of lines) {
const heading = line.match(/^###\s+(.+?)\s*$/);
if (heading) {
current = heading[1].trim();
sections[current] = [];
continue;
}
if (current) {
sections[current].push(line);
}
}
return sections;
}

const sections = parseSections(body);

function sectionText(name) {
const raw = sections[name];
if (!raw) return '';
return raw.join('\n').trim();
}

// Dropdown answers render as a plain text line under the heading.
// Missing/optional answers render as "_No response_".
function dropdownPrefix(name) {
const text = sectionText(name);
if (!text || text === '_No response_') return null;
const firstLine = text.split(/\r?\n/)[0].trim().toLowerCase();
return firstLine;
}

// Allowlists — the issue body is untrusted input, so every label
// this script adds must be validated against a list the body cannot
// influence, never derived from body text alone (addLabels
// auto-creates unknown labels, so an unconstrained regex match
// would let a crafted body mint arbitrary labels). Priorities and
// subtypes are fixed constants; area:* is read from labels.yml
// (below), which only a merged PR can change.
const ALLOWED_PRIORITIES = ['p0', 'p1', 'p2', 'p3'];
// Deliberately NOT read from labels.yml: a repo running the
// coarse-Type label fallback (ADR-0006) declares type:bug and
// type:feature there, and a crafted "### Subtype" line must not be
// able to mint a coarse Type on a Task. scripts/check-label-forms.sh
// keeps this list and task.yml's dropdown equal.
const ALLOWED_SUBTYPES = ['chore', 'ops', 'docs', 'security'];
// The exact type:* labels this workflow owns. Deliberately NOT
// `label.startsWith('type:')`: only task.yml has a Subtype dropdown,
// so on a bug report or feature request `desired` never holds a
// type:* label — and a prefix match would put every hand-applied
// type:* label into `toRemove` and strip it on the next body edit.
const FORM_MANAGED_TYPES = ALLOWED_SUBTYPES.map((s) => `type:${s}`);

// area:* allowlist = every `- name: area:...` entry in
// .github/labels.yml on the DEFAULT branch. The default branch, not
// the event ref, so a PR branch's copy cannot widen the list. The
// parse mirrors parse_labels_yml in scripts/bootstrap.sh: an entry
// is a `- name:` line, and a commented-out entry starts with # and
// never matches. A read failure THROWS — an empty allowlist is not
// a safe default, because `desired` would then hold no area:* and
// every area label already on the issue would land in toRemove.
async function loadAllowedAreas() {
const ref = context.payload.repository.default_branch;
const { data } = await github.rest.repos.getContent({
owner: context.repo.owner,
repo: context.repo.repo,
path: '.github/labels.yml',
ref,
});
if (!data || data.type !== 'file' || typeof data.content !== 'string') {
throw new Error(`.github/labels.yml is not a readable file on ${ref}`);
}
const yml = Buffer.from(data.content, 'base64').toString('utf8');
const areas = [];
for (const line of yml.split(/\r?\n/)) {
// Same reading as parse_labels_yml: the name is everything after
// `name:` with surrounding whitespace trimmed — no quote or
// comment stripping, so area:c# and area:team's survive intact
// and a quoted or inline-commented entry is wrong here exactly
// as it is wrong for bootstrap (labels.yml's header rules both out).
const m = line.match(/^\s*-\s*name:\s*(area:\S.*?)\s*$/);
if (m) areas.push(m[1]);
}
if (areas.length === 0) {
core.warning(`no area:* entries in .github/labels.yml on ${ref} — Area checkboxes will apply nothing`);
}
return areas;
}
const ALLOWED_AREAS = await loadAllowedAreas();

const desired = new Set();

// Option text is "<value> — <description>". Rather than guessing
// where the value ends, match the text against the allowed values:
// the longest allowed value the text starts with, followed by the
// end of the text or a dash (—, – or -) with whitespace on both
// sides. A value may therefore contain spaces or a spaced dash, as
// GitHub label names can; the allowlist alone decides, and there
// is no second regex spelling out the same values
// (scripts/check-label-forms.sh keeps the forms and labels.yml on
// this convention).
function matchAllowed(text, allowed) {
const candidates = [...allowed].sort((a, b) => b.length - a.length);
for (const name of candidates) {
if (text === name) return name;
if (text.startsWith(name) && /^\s+[—–-]\s+/.test(text.slice(name.length))) return name;
}
return null;
}

// ### Priority -> priority:pX (dropdown, e.g. "p0 — Critical, drop everything")
const priorityLine = dropdownPrefix('Priority');
if (priorityLine) {
const value = matchAllowed(priorityLine, ALLOWED_PRIORITIES);
if (value) desired.add(`priority:${value}`);
}

// ### Subtype -> type:X (task form only; dropdown, e.g. "chore — Maintenance")
const subtypeLine = dropdownPrefix('Subtype');
if (subtypeLine) {
const value = matchAllowed(subtypeLine, ALLOWED_SUBTYPES);
if (value) desired.add(`type:${value}`);
}

// ### Area -> area:* (checkboxes, e.g. "- [x] area:docs — Documentation and guides")
const areaText = sectionText('Area');
if (areaText) {
const checkedLines = areaText.split(/\r?\n/).filter((line) => /^-\s*\[[xX]\]/.test(line));
for (const line of checkedLines) {
// "- [x] area:<name> — <description>": same rule as the
// dropdowns; the allowlist decides, not a character class.
const value = matchAllowed(line.replace(/^-\s*\[[xX]\]\s*/, '').trim(), ALLOWED_AREAS);
if (value) desired.add(value);
}
}

// Only these labels are form-managed; every other label is left
// untouched. area: stays a prefix match on purpose — that is what
// cleans up a stale area:old label after an adopter renames their
// area taxonomy in .github/labels.yml.
function isFormManaged(label) {
return (
label.startsWith('priority:') ||
FORM_MANAGED_TYPES.includes(label) ||
label.startsWith('area:')
);
}

const { owner, repo } = context.repo;
const issue_number = context.payload.issue.number;

const currentLabels = (context.payload.issue.labels || []).map((l) => l.name);
const currentManaged = currentLabels.filter(isFormManaged);

const toAdd = [...desired].filter((label) => !currentLabels.includes(label));
// On 'edited', drop previously form-managed labels the user un-selected.
// On 'opened' this is a no-op (nothing managed exists yet).
const toRemove = currentManaged.filter((label) => !desired.has(label));

if (toAdd.length > 0) {
await github.rest.issues.addLabels({
owner,
repo,
issue_number,
labels: toAdd,
});
}

for (const label of toRemove) {
await github.rest.issues.removeLabel({
owner,
repo,
issue_number,
name: label,
});
}

core.info(`Desired: ${[...desired].join(', ') || '(none)'}`);
core.info(`Added: ${toAdd.join(', ') || '(none)'}`);
core.info(`Removed: ${toRemove.join(', ') || '(none)'}`);
const { run } = require(`${process.env.GITHUB_WORKSPACE}/scripts/issue-labeler.js`);
await run({ github, context, core });
4 changes: 2 additions & 2 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Agent-specific entry files (`CLAUDE.md`, `GEMINI.md`, `.github/copilot-instructi

## Build and validation

The Makefile is the only executable contract in this repository. CI calls make targets; customize the Makefile, never the workflows. The single exception is runner selection, which GitHub resolves before any make target exists to be called: set the `RUNNER_LABELS` repository variable instead of editing `runs-on` (see `docs/setup/runners.md`).
The Makefile is the only executable contract in this repository. CI calls make targets; customize the Makefile, never the workflows. Two exceptions, neither of which puts adopter values in the YAML: runner selection, which GitHub resolves before any make target exists to be called — set the `RUNNER_LABELS` repository variable instead of editing `runs-on` (see `docs/setup/runners.md`); and event handlers that need the token and payload, whose logic lives in `scripts/*.js` behind a thin `github-script` caller and is tested by `make check` (the issue labeler; ADR-0008).

| Level | Name | Command | When required |
| --- | --- | --- | --- |
Expand All @@ -39,7 +39,7 @@ The Makefile is the only executable contract in this repository. CI calls make t
| `skills/` | Reusable knowledge modules (one directory per skill, `SKILL.md` inside) |
| `docs/adr/` | Architecture Decision Records |
| `docs/setup/` | Bootstrap and GitHub configuration guides |
| `scripts/` | Bootstrap and self-consistency check scripts |
| `scripts/` | Bootstrap, self-consistency checks, and the event-handler logic workflows call (`issue-labeler.js`; ADR-0008) |
| `Makefile` | Canonical target contract (validation ladder entry points) |

## Workflow
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Install the lint tools used by `make lint-docs`:
| Tool | Install |
| --- | --- |
| markdownlint-cli2 | `npm install -g markdownlint-cli2` |
| node (any LTS) | `brew install node` — runs `scripts/issue-labeler.test.js` in `make check` |
| yamllint | `brew install yamllint` (or `pip install yamllint`) |
| lychee | `brew install lychee` |
| actionlint | `brew install actionlint` |
Expand Down
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ check: ## Run repo self-consistency scripts (skips scripts not yet added)
@if [ -x scripts/check-local-md.sh ]; then scripts/check-local-md.sh; else echo "skip: scripts/check-local-md.sh not present yet"; fi
@if [ -x scripts/check-license-marker.sh ]; then scripts/check-license-marker.sh; else echo "skip: scripts/check-license-marker.sh not present yet"; fi
@if [ -x scripts/check-label-forms.sh ]; then scripts/check-label-forms.sh; else echo "skip: scripts/check-label-forms.sh not present yet"; fi
@if [ -x scripts/check-issue-labeler.sh ]; then scripts/check-issue-labeler.sh; else echo "skip: scripts/check-issue-labeler.sh not present yet"; fi

lint: lint-docs lint-actions lint-secrets check ## L0 - aggregate all lint/consistency checks

Expand Down
30 changes: 30 additions & 0 deletions docs/adr/ADR-0008-event-workflow-logic-in-scripts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# ADR-0008: Event-driven workflow logic lives in `scripts/`, behind a thin `github-script` caller

- **Status**: Accepted
- **Date**: 2026-09-16
- **Issue**: #49

## Context

`AGENTS.md` and `skills/github-actions-hygiene` rule 1 say workflows call `make` targets and never contain logic. The issue labeler was always the exception: an event handler that needs the `GITHUB_TOKEN` and the `issues` payload, which no `make` target can own, so ~150 lines of JavaScript lived inline in `.github/workflows/issue-labeler.yml`. The exception was undocumented, and the only way to test the script was to extract it from the YAML by hand — which is exactly how #48 was validated. A repository whose rule is "the template obeys its own rules" cannot leave its one stateful workflow untestable and unnamed.

## Decision

1. **The logic moves to `scripts/issue-labeler.js`**, a plain Node module with pure functions (`parseSections`, `matchAllowed`, `parseAllowedAreas`, `computeChanges`) and one I/O entry point (`run({github, context, core})`). The workflow checks out the default branch (`actions/checkout`, pinned, `persist-credentials: false`, `fetch-depth: 1`) and its `github-script` step is two lines: `require` the module, `await run(...)`.
2. **`labels.yml` is read from the checkout, not the API.** On `issues` events `GITHUB_SHA` is the head of the default branch, so the checkout holds exactly the merged declarations — the same trust boundary the API read had, without the extra request.
3. **The script is tested by `make check`** (`scripts/check-issue-labeler.sh` → `node --test scripts/issue-labeler.test.js`, node built-ins only). A missing `node` fails with an install hint, the way every other tool `make` needs does — never a green skip (validation-ladder rule 7).
4. **Rule 1 names the shape.** Event-driven workflows that genuinely need the token and payload keep their logic in `scripts/*.js`, called through `github-script` after a checkout; the workflow stays a thin caller and carries no adopter values.

## Consequences

- The labeler is readable in one file and has a durable test; a change to its matching rules is a normal PR with a failing test first.
- The workflow file stays replaceable whole on upgrade — the property adopters already rely on — but now travels with `scripts/issue-labeler.js`; `docs/template/upgrading.md` says so.
- One more pinned action to keep bumped (Dependabot covers it) and a checkout on every issue event (a few seconds).
- `node` becomes a dependency of `make check`; CI already has it for markdownlint.
- The logic is now protected by whatever protects `scripts/` on the default branch, not by GitHub's rule that tokens without the `workflows` scope cannot write under `.github/workflows/`. The blast radius is unchanged (the job holds `issues: write` and nothing else), and `main` already requires a PR plus green `ci`; adopters who want a review barrier add `/scripts/issue-labeler.js` and `/.github/workflows/` to `.github/CODEOWNERS` (the commented example there shows the line) and set `require_code_owner_review` in the ruleset.

## Alternatives considered

- **Record the exception and keep the script inline.** Rejected: it leaves the only stateful workflow untestable, and the next change to it repeats the extract-by-hand validation.
- **Keep reading `labels.yml` through the API and skip the checkout.** Rejected: the checkout is needed for the script anyway, and a second copy of the same data by a second route is a second thing to keep consistent.
- **A `make labeler` target invoked from a `run:` step.** Rejected: the payload would have to reach the shell through the environment, and the token through `gh` — more surface, no gain over `github-script`'s sandbox.
1 change: 1 addition & 0 deletions docs/adr/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ Routine choices (a library patch bump, a wording tweak) do not get ADRs. When in
| [ADR-0005](ADR-0005-runner-selection-variable.md) | Runner selection is an adopter variable, not a workflow edit | Accepted |
| [ADR-0006](ADR-0006-coarse-type-fallback.md) | Coarse Type on accounts without native issue types | Accepted |
| [ADR-0007](ADR-0007-retire-agent-labels.md) | Retire the `agent-ok` / `by-agent` label mechanism | Accepted |
| [ADR-0008](ADR-0008-event-workflow-logic-in-scripts.md) | Event-driven workflow logic lives in `scripts/`, behind a thin `github-script` caller | Accepted |
Loading