chore(CI): lint, Bats tests, CI, and Dependabot#120
Conversation
Manage the dev toolchain as npm devDependencies so one `npm install` pins
every tool locally (nothing global, no cross-project version drift) and
installs the git hook via the "prepare" script:
- @j178/prek runs the pre-commit hooks (drop-in for pre-commit).
- Prettier formats Markdown, JSON, YAML, and JS. .prettierrc.json sets a
120-column prose wrap with ordered-list numbering preserved and .github
PR/issue templates unwrapped (GitHub renders newlines as <br>);
.prettierignore excludes docs/plans, docs/research, and vendored assets.
- ShellCheck (via shellcheck-py, no Docker) lints shell scripts.
- The pre-commit-hooks hygiene checks cover what Prettier does not: their
whitespace/EOL fixers exclude md/json/yaml/js so Prettier owns those with
no overlap, and check-yaml/check-json are dropped because Prettier already
parses and validates those types.
- Bats runs shell tests from test/; a sanity test proves the harness.
Add a CI workflow with a lint job (npm run lint) and a test job (npm test),
both on the pinned lockfile. Formatting the existing Markdown is left to
maintainers; this change only adds the tooling.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KFKuiUmQ9QinkSt9LbE7hc
Add a "Setting up your environment" section covering the one-command `npm install` setup (pinned prek, Prettier, and Bats, with the git hook installed automatically), the everyday commands (npm run lint, npm test), and how Prettier treats each path. Add a TL;DR pointer to it and an "npm run lint passes" item to the self-review checklist. Left in the existing line style; the repo-wide Prettier reflow is for maintainers to apply. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFKuiUmQ9QinkSt9LbE7hc
Bundle npm devDependencies (prek, Prettier, Bats), the GitHub Actions used by the workflows, and the pre-commit hook revisions in .pre-commit-config.yaml (shellcheck-py, pre-commit-hooks) into a single monthly pull request via a Dependabot multi-ecosystem group. Each ecosystem sets a 30-day cooldown, so a newly published version is not proposed until it has been out a month, lowering supply-chain risk from fresh or compromised releases. Cooldown does not apply to security updates, so enabling Dependabot security updates in the repository settings still lands those immediately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFKuiUmQ9QinkSt9LbE7hc
|
@taminomara this is great! one bit of discussion, though: i'm not a fan of pre-commit hooks. in my experience, they tend to a lot of what i would prefer to see is CI rules that validate all the things via am definitely interested in feedback and other opinions on this, though. there's a lot of great things in this PR, too - just want to make sure we're making it easy to contribute without breaking the way someone might already work with commits |
|
Yeah, I get it. I don't use pre-commit hooks as well. But I use prek as a lint orchestrator: it only runs on changed files, and it has a bunch of useful tools like symlink integrity check. I've disabled automatic hook installation on Regarding versions, all tools are installed by npm, and all versions are pinned. So, even if someone has outdated prek/bats, han's tests are hermetic, and use versions from Also, I can add a CI stage that auto-merges dependabot PRs (those can be a nuisance to merge, even if they appear every month). I won't be able to debug it, though, as that requires admin access to the repo. |
|
Anyways, |
Not all developers prefer using pre-commit hooks. Disable automatic hook install so that each developer can choose for themselves. CI provides backstop and gates bad merges to main.
00fc7d2 to
6872546
Compare
|
love it! just tested this out with and without prek installed, and it works great. 🥳 |
The lint workflow added in #120 surfaced three pre-existing failures. This commit fixes the two shellcheck findings and the broken symlink. - init-guidance.sh: the first sed expression matches the literal text "${CLAUDE_PLUGIN_ROOT}" as it appears in the vendored SKILL.md, so the single quotes are required and SC2016 is a false positive. Disable the check at that call with a comment explaining why. - upload-screenshots.sh: verify_on's retry loop never reads its counter, so SC2034 fired on it. Use the conventional throwaway `_` instead. - .claude/rules/plugin-entity-taxonomy.md: the symlink still pointed at docs/guidance/, which ad6ee13 moved to han-plugin-builder/skills/guidance/references/. That commit rewrote the adjacent .claude/rules/ text references but could not catch a symlink target. Repoint it at the file's current home.
The lint workflow added in #120 runs Prettier over all files, but the repo had never been formatted with it, so the hook rewrote 279 files and failed the job. This commit lands that formatting so the hook is a no-op. Formatting only: prose is rewrapped to the 120-column width set in .prettierrc.json, emphasis markers are normalized, and table cells are padded. No prose was changed. The one non-whitespace change is in han-coding/skills/coding-standard/SKILL.md, where an ordered list was misnumbered 1, 2, 4, 5; Prettier owns ordered-list numbering per .pre-commit-config.yaml and renumbered it to 1, 2, 3, 4.
## Why The `lint` job has failed on every branch, including `main`, since CI landed in #120. Two of its three failures are small and fixed here. These fixes were originally pushed to the `fix/dependabot-multi-ecosystem-patterns` branch, but #123 merged at `408ff77` before they were picked up, so they never reached `main`. This re-lands them. ## What - **`init-guidance.sh` (SC2016).** The `sed` expression matches the literal string `${CLAUDE_PLUGIN_ROOT}`, which must *not* expand, so the single quotes are correct. Silences the check rather than changing behavior. - **`upload-screenshots.sh` (SC2034).** The retry loop's counter is never read. Binding it to `_` states that in the code instead of suppressing the warning. - **`.claude/rules/plugin-entity-taxonomy.md`.** The symlink pointed at `docs/guidance/`, where the taxonomy no longer lives. Repointed at its real home under `han-plugin-builder/`. ## Verification `shellcheck` and `check-symlinks` both pass; `npm test` passes. ## Still red: prettier The lint job's third failure is prettier, and it is **not** fixed here. It is not a mechanical reformat — running it corrupts instructional content: - It **renumbers ordered lists**. In `coding-standard/SKILL.md` step `4.` becomes `3.`, while that file's prose still refers to "Step 6" and "Step 7" — silently misdirecting instructions Claude executes. - It **trims inline code spans**. The issue-heading spec `` ` — ` `` (em-dash *with surrounding spaces*) becomes `` `—` ``, while the prose beside it still reads "em-dash with surrounding spaces". Neither is configurable away: both persist under `proseWrap: preserve` + `embeddedLanguageFormatting: off`. They are core CommonMark normalizations. Left for a separate decision.
# Prettier reformat: failure modes Han's `lint` job has failed on every branch, including `main`, since CI landed in #120. The prettier config (`proseWrap: "always"`, printWidth 120) was added but never actually run against the repo. Applying it touches 281 files (+28,218 / -12,660). This report categorizes what that reformat actually does, based on the diffs verified against the working tree. The thing to hold onto: prettier keeps the _rendered_ page the same, but han's markdown is read as raw text by agents. Three of these categories change the raw text in ways that matter. | # | Category | Files | Harmful? | Fixable by config? | | --- | --------------------------- | ----- | ------------------- | ------------------ | | 1 | Harmless reformatting | ~271 | No | n/a | | 2 | Frontmatter re-quoting | 103 | No (verified) | n/a | | 3 | Renumbered steps | 1 | **Yes** | **No** | | 4 | Mangled code snippets | 9 | **Yes** | **No** | | 5 | Rewritten code examples | 3 | Yes | Yes | | 6 | A file prettier can't read | 1 | Not in CI | n/a | ## 1. Harmless reformatting Rewraps paragraphs at 120 characters, swaps `*word*` for `_word_`, pads out tables, tidies blockquotes. This is most of the 28k-line diff. The words don't change, so nothing breaks. It just makes every future doc diff noisy. ## 2. Frontmatter re-quoting Rewrites the `description:` field at the top of each skill file from double quotes to single quotes. This one warranted checking, since that field is what Claude reads to decide when to run a skill. All 103 frontmatter blocks were parsed before and after with a YAML parser: the values come out identical. Safe. ## 3. Renumbered steps Prettier renumbers ordered lists. In `han-coding/skills/coding-standard/SKILL.md`, step 4 becomes step 3 — but the sentences around it still say "Step 6" and "Step 7". Those now point at the wrong steps, in a file Claude follows as instructions. Nothing errors; the skill just quietly does the wrong step. This is the worst one. Note that `.pre-commit-config.yaml` lists "ordered-list numbering" as something prettier is meant to own, which is the direct conflict. ## 4. Mangled code snippets Prettier strips the spaces inside `` ` — ` ``, turning it into `` `—` ``. In two work-item templates that snippet _is_ the heading format, and the sentence next to it still says "em-dash with surrounding spaces." The source now shows the opposite of what the text says. It also garbles backticks in docs that explain markdown itself, including `readability-editor.md` — the doc whose job is to say "leave code fences byte-for-byte unchanged." Affected: `work-items-to-issues/references/issue-template.md`, `plan-work-items/references/work-item-template.md`, `markdown-to-confluence/SKILL.md`, `code-overview/references/overview-template.md`, `han-core/agents/readability-editor.md`, `project-documentation/SKILL.md`, and three guidance references under `han-plugin-builder/`. ## 5. Rewritten code examples Prettier reaches inside fenced code blocks and restyles the code: collapsing JSON arrays, flipping `'base'` to `"base"`, shifting HTML spacing. Those examples are specs, so restyling them is wrong — but this one turns off with `embeddedLanguageFormatting: "off"`. Affected: `plugin-json-options.md`, `html-summary/SKILL.md`, `html-summary/references/report-style.md`. ## 6. A file prettier can't read `han-reporting/skills/html-summary/references/html-template.html` is a schematic sketch, not real HTML, and prettier hard-errors on it (exit 2). Not a CI problem today, because the prek hook only hands prettier markdown, JSON, YAML, and JS. It would break if anyone widens that list. ## Bottom line Categories 1 and 2 are just noise. Category 5 has a config switch. **Categories 3 and 4 are the blockers, and neither can be turned off** — both still happen under the safest config available (`proseWrap: preserve` plus `embeddedLanguageFormatting: off`), because they are core CommonMark normalizations. No prettier setting formats this repo's markdown without breaking it. Recommendation: take markdown out of prettier's scope and keep it on JSON, YAML, and JS — a clean 10-file diff that turns lint green. ## Related - [PR #126](#126) — fixes the other two lint failures (shellcheck, broken symlink). Its `lint` job is still red on prettier alone, pending this decision.
Summary
Adds an npm-managed lint and test toolchain, CI, and Dependabot for the han repo. One
npm installpins every toollocally (no global installs, no cross-project version drift) and installs the git hook via the
preparescript.@j178/prek) runs the pre-commit hooks (drop-in forpre-commit)..githubPR/issue templates unwrapped (GitHub renders newlines as<br>), anddocs/plans,docs/research, and the vendored assets excluded.shellcheck-py, no Docker) lints shell scripts.pre-commit-hookshygiene checks cover the rest; their whitespace/EOL fixers exclude md/json/yaml/js so Prettier owns those with no overlap, andcheck-yaml/check-jsonare dropped because Prettier already validates those types.test/*.batsin CI (not on commit); a sanity test proves the harness.lintjob (npm run lint) and atestjob (npm test), both on the pinned lockfile.Mitigates #113 for this repo. Should be merged before #119.
Note on the
lintCI jobThis PR adds the tooling only. Running
npm run lintreflows the existing Markdown across the repo; that mechanical reflow is intentionally not included here, so thelintjob will report changes until it is applied in a separate pass. Thetestjob is green.Follow-ups for a maintainer
npm run lintand commit the repo-wide Prettier reflow (and re-run so thelintjob goes green).# shellcheck disabledirectives:init-guidance.sh:55(SC2016) andupload-screenshots.sh:136(SC2034). Both look benign..claude/rules/plugin-entity-taxonomy.mdthatcheck-symlinksflags.🤖 Generated with Claude Code
https://claude.ai/code/session_01KFKuiUmQ9QinkSt9LbE7hc