fix(skills): stop the bundled skill using :iid, which is not a placeholder

Why

Reported in #8531 (closed). The bundled glab skill used :iid in five glab api examples, for both merge requests and issues:

$ glab api projects/:id/merge_requests/:iid
{"error":"merge_request_iid is invalid"}glab: HTTP 400

glab api expands a fixed set of placeholders, and iid is not in it:

var placeholderRE = regexp.MustCompile(`:(group/:namespace/:repo|namespace/:repo|fullpath|id|user|username|group|namespace|repo|branch)\b`)

Because the regexp never matches, the token is not rejected either — it is passed through and sent to the API verbatim, which is why the failure surfaces as a 400 from GitLab rather than as an error from glab.

A skill is instructions an agent follows literally, so this is not a cosmetic typo. An agent following it as written gets HTTP 400 on every merge request and issue call and, as the reporter notes, typically falls back to guessing.

What

Not by adding :iid as a placeholder. Three of the five uses are issue endpoints (projects/:id/issues/:iid/notes), and an iid resolved from the current branch's merge request is meaningless there — it would turn a loud 400 into a request against the wrong issue.

  • the five examples use a real number
  • a short Placeholders section lists the tokens that are expanded, says anything else is sent verbatim, and gives glab mr view -F json | jq .iid for getting a real one

Stopping it recurring

api.Placeholders is now the single definition of the set, with placeholderRE built from it, and a test checks every glab api line in every bundled skill against that list.

The test reads the exported list, not a copy, so a placeholder added or removed cannot silently disagree with the documentation.

The refactor is behaviour-preserving, and that is checked rather than assumed — a temporary test compared placeholderRE.String() against the original literal and passed, so the compiled pattern is byte-identical. Alternation order matters here (a longer token must precede any token that prefixes it), and the slice preserves the original order.

Verification

check result
go test ./internal/commands/skills/... ./internal/commands/api/... ok, all packages
go build ./... clean
make gen-docs then git status clean

Mutation-tested. Putting one :iid back:

--- FAIL: TestBundledSkillsOnlyUseRealPlaceholders
    Messages: SKILL.md:184 uses ":iid", which glab api does not expand and sends verbatim:
              glab api projects/:id/merge_requests/:iid -X PUT -f "assignee_id=1"

It names the file, the line, the token and the offending command, so the next person hits a usable message rather than a bare assertion.

The scan only looks at lines containing glab api, so https://, a -H "Content-Type: ..." header and {"position_type":"text"} in JSON are all untouched — and the two remaining :iid mentions in the new prose, which exist to say it is not a placeholder, are correctly ignored.

Closes #8531 (closed)

🤖 Generated with Claude Code

Merge request reports

Loading
Loading