ci: add pre-commit hook and CI validation for generated skill rules by ekrako · Pull Request #148 · avivsinai/bitbucket-cli

ekrako · 2026-04-10T13:00:10Z

Summary

Add staleness detection so generated skill rules stay in sync with the Cobra command tree
This is PR 4 of 4 (final) in the auto-generate skill series

Full Plan (4 PRs) - Complete

PR 1 (merged): Add detailed docstrings + platform markers to all commands
PR 2 (merged): Generator core (cmd/docgen/ + internal/docgen/) + make generate-skill
PR 3 (merged): Commit generated rules/*.md, rewrite SKILL.md
PR 4 (this): Pre-commit hook + CI validation for staleness detection

Changes

scripts/check-generated-skill.sh — regenerates into temp dir and diffs against committed rules + SKILL.md References section
Makefile — check-generated-skill target
.pre-commit-config.yaml — local hook triggers on changes to pkg/cmd/, cmd/docgen/, internal/docgen/, or skills/bkt/
.github/workflows/ci.yml — check-skills job now runs check-generated-skill with Go toolchain

Test plan

make check-generated-skill passes on clean tree
Modifying a command's Short/Long causes make check-generated-skill to fail with clear error
make generate-skill && make check-generated-skill recovers from stale state
go test ./... passes
CI workflow runs both check-skills and check-generated-skill

sentry · 2026-04-10T13:02:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

avivsinai

Correct staleness check — rulesDir derivation matches, SKILL.md copied into $TMPDIR before regen (matches generator contract at internal/docgen/generate.go:80), trap-based cleanup is solid.

One thing worth fixing: scripts/check-generated-skill.sh:19 — drop the 2>/dev/null on go run ./cmd/docgen. It suppresses the real generator failure. With set -e, if docgen can't build or errors at runtime, the script aborts opaquely with no useful output — reproduced by running the script with go missing from PATH: exit 127, nothing on stderr, nothing telling the dev what actually broke. Drop the redirect or capture stderr to a log that's dumped on failure.

Nits (take or leave):

diff -rq runs twice in the failure path (script:22,37) — single invocation into a var.
❌ emoji in error output — project CLAUDE.md discourages; plain ERROR: is more portable across CI log viewers.
.pre-commit-config.yaml:22 — consider adding Makefile + the script itself to the files: pattern so gen-plumbing edits also trigger re-verification.
.github/workflows/ci.yml:24 — new setup-go step has no module cache; existing build job should share it.

Happy to approve once the 2>/dev/null is dropped.

Add staleness detection so generated skill rules stay in sync with the Cobra command tree: - scripts/check-generated-skill.sh: regenerates rules into a temp dir and diffs against committed files; also validates the SKILL.md References section - Makefile: add check-generated-skill target - .pre-commit-config.yaml: local hook triggers on changes to pkg/cmd/, cmd/docgen/, internal/docgen/, or skills/bkt/ - .github/workflows/ci.yml: check-skills job now also runs check-generated-skill with Go toolchain This is PR 4 of 4 in the auto-generate skill series: 1. Add detailed docstrings to all commands (merged) 2. Generator core + make generate-skill (merged) 3. Generated rules, SKILL.md rewrite (merged) 4. (this) Pre-commit hook + CI validation for staleness detection

- Drop 2>/dev/null on docgen invocation to surface real failures - Deduplicate diff -rq into a single variable - Replace emoji with ERROR: prefix for CI log portability - Ignore manually authored rule files (e.g. headless.md) in staleness check - Normalize trailing whitespace for end-of-file-fixer compatibility - Add Makefile and script to pre-commit trigger pattern - Regenerate skill rules

ekrako · 2026-04-12T09:20:54Z

Addressed all points:

2>/dev/null dropped — stderr flows through naturally now.
diff -rq deduplicated — single invocation captured into a variable, reused in error output.
❌ → ERROR: — plain prefix, portable across CI log viewers.
Pre-commit files: pattern — added Makefile and scripts/check-generated-skill.sh to trigger re-verification on plumbing edits.
Module cache — setup-go v6 caches modules by default (keyed on go.sum), so both jobs already benefit. Cross-job sharing within a single run isn't possible since they run on separate runners.

Additional fixes in the same commit:

Check script now ignores manually authored rule files (e.g. headless.md) — only compares files the generator produces.
Trailing whitespace normalized before diff to avoid false positives from end-of-file-fixer.

avivsinai

Blocking issue in scripts/check-generated-skill.sh:23-31.

The new staleness check only iterates over $TMPDIR/rules/*.md, so it catches missing/generated mismatches for files docgen currently emits, but it never notices extra committed auto-generated files that docgen would remove.

Why this matters:

docgen.GenerateAll explicitly deletes stale generated markdown files before regenerating (internal/docgen/generate.go:149-173).
There is already a dedicated regression test asserting that behavior (internal/docgen/generate_test.go:481-514).
The current shell loop does not model that contract. If skills/bkt/rules/old-command.md is a stale auto-generated file and docgen now only emits pr.md, the script still passes because it only walks the generated side.

I reproduced the loop with:

committed: old-command.md + pr.md
generated: pr.md

and STALE remained empty.

For a pristine OSS gate, this needs to fail on orphaned generated files too, while still ignoring hand-maintained files like headless.md. The simplest fix is to compare both sides, but only treat committed .md files with the docgen auto-generated header as candidates for orphan detection.

ekrako · 2026-04-12T13:08:47Z

Added a reverse loop (scripts/check-generated-skill.sh:33-43) that iterates over committed $RULES_DIR/*.md, checks for the <!-- auto-generated by cmd/docgen header prefix (matching the contract in removeMarkdownFiles), and flags any file docgen no longer emits as orphaned:. Hand-maintained files like headless.md are skipped.

avivsinai

One issue still left here for a pristine OSS repo.

The new generated-skill check path ignores the repo's existing GO override contract. Makefile defines GO ?= go, and generate-skill uses $(GO), but scripts/check-generated-skill.sh:18 hardcodes go run ./cmd/docgen .... That means make generate-skill GO=... and make check-generated-skill GO=... can run under different Go binaries / wrappers, which is inconsistent for contributors and CI setups that pin GO.

Please thread the same GO override through the check path as well, so generation and verification use the same toolchain contract.

ekrako · 2026-04-12T14:57:49Z

Done — script now reads ${GO:-go} and the Makefile target passes GO="$(GO)" to it. make check-generated-skill GO=/path/to/go and make generate-skill GO=/path/to/go use the same binary.

avivsinai

Re-reviewed the latest head after the GO override fix. check-generated-skill now honors the same GO contract as generate-skill, both through the Makefile and direct script invocation. LGTM.

avivsinai reviewed Apr 10, 2026

View reviewed changes

avivsinai mentioned this pull request Apr 10, 2026

fix: alias pr update to pr edit #149

Merged

ekrako force-pushed the feat/generate-skill-ci branch from bf10a9a to 1884fcc Compare April 12, 2026 09:09

avivsinai requested changes Apr 12, 2026

View reviewed changes

ekrako requested a review from avivsinai April 12, 2026 13:05

fix: detect orphaned auto-generated rule files in staleness check

18329f4

avivsinai requested changes Apr 12, 2026

View reviewed changes

fix: thread GO override through check-generated-skill script

a9c7109

ekrako requested a review from avivsinai April 12, 2026 14:59

avivsinai approved these changes Apr 12, 2026

View reviewed changes

avivsinai merged commit f69b3ea into avivsinai:master Apr 12, 2026
7 checks passed

avivsinai mentioned this pull request Apr 12, 2026

docs(changelog): capture unreleased v0.23.0 changes #158

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: add pre-commit hook and CI validation for generated skill rules#148

ci: add pre-commit hook and CI validation for generated skill rules#148
avivsinai merged 4 commits intoavivsinai:masterfrom
ekrako:feat/generate-skill-ci

ekrako commented Apr 10, 2026

Uh oh!

sentry Bot commented Apr 10, 2026

Uh oh!

avivsinai left a comment

Uh oh!

ekrako commented Apr 12, 2026

Uh oh!

avivsinai left a comment

Uh oh!

ekrako commented Apr 12, 2026

Uh oh!

avivsinai left a comment

Uh oh!

ekrako commented Apr 12, 2026

Uh oh!

avivsinai left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ekrako commented Apr 10, 2026

Summary

Full Plan (4 PRs) - Complete

Changes

Test plan

Uh oh!

sentry Bot commented Apr 10, 2026

Codecov Report

Uh oh!

avivsinai left a comment

Choose a reason for hiding this comment

Uh oh!

ekrako commented Apr 12, 2026

Uh oh!

avivsinai left a comment

Choose a reason for hiding this comment

Uh oh!

ekrako commented Apr 12, 2026

Uh oh!

avivsinai left a comment

Choose a reason for hiding this comment

Uh oh!

ekrako commented Apr 12, 2026

Uh oh!

avivsinai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants