Skip to content

chore(CI): lint, Bats tests, CI, and Dependabot#120

Merged
mxriverlynn merged 4 commits into
testdouble:mainfrom
taminomara:chore/setup-prek-precommit
Jul 14, 2026
Merged

chore(CI): lint, Bats tests, CI, and Dependabot#120
mxriverlynn merged 4 commits into
testdouble:mainfrom
taminomara:chore/setup-prek-precommit

Conversation

@taminomara

@taminomara taminomara commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an npm-managed lint and test toolchain, CI, and Dependabot for the han repo. One npm install pins every tool
locally (no global installs, no cross-project version drift) and installs the git hook via the prepare script.

  • prek (@j178/prek) runs the pre-commit hooks (drop-in for pre-commit).
  • Prettier formats Markdown, JSON, YAML, and JS: 120-column prose wrap, ordered-list numbering preserved, .github PR/issue templates unwrapped (GitHub renders newlines as <br>), and docs/plans, docs/research, and the vendored assets excluded.
  • ShellCheck (via shellcheck-py, no Docker) lints shell scripts.
  • The pre-commit-hooks hygiene checks cover the rest; 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 validates those types.
  • Bats runs test/*.bats in CI (not on commit); a sanity test proves the harness.
  • CI: a lint job (npm run lint) and a test job (npm test), both on the pinned lockfile.
  • Dependabot: one monthly PR bundling npm + GitHub Actions + pre-commit updates via a multi-ecosystem group, with a 30-day cooldown on new releases to reduce supply-chain risk (security updates are exempt from the cooldown).

Mitigates #113 for this repo. Should be merged before #119.

Note on the lint CI job

This PR adds the tooling only. Running npm run lint reflows the existing Markdown across the repo; that mechanical reflow is intentionally not included here, so the lint job will report changes until it is applied in a separate pass. The test job is green.

Follow-ups for a maintainer

  • Run npm run lint and commit the repo-wide Prettier reflow (and re-run so the lint job goes green).
  • Resolve two ShellCheck findings, or add # shellcheck disable directives: init-guidance.sh:55 (SC2016) and upload-screenshots.sh:136 (SC2034). Both look benign.
  • Fix the broken symlink .claude/rules/plugin-entity-taxonomy.md that check-symlinks flags.
  • Enable Settings → Code security → Dependabot security updates so security patches bypass the 30-day cooldown.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KFKuiUmQ9QinkSt9LbE7hc

taminomara and others added 3 commits July 13, 2026 01:51
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 taminomara changed the title chore: npm-managed lint (prek + Prettier + ShellCheck), Bats tests, CI, and Dependabot chore(CI): lint, Bats tests, CI, and Dependabot Jul 12, 2026
@taminomara taminomara marked this pull request as ready for review July 12, 2026 21:57
@mxriverlynn

Copy link
Copy Markdown
Collaborator

@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 git commit --no-verify calls, which gets to be tedious. i often commit changes knowing things are currently in a bad state, and having to wait for a pre-commit hook interrupts that. if someone has an outdated tools version, or there's a known failure that would prevent the commit, it ends up adding both cognitive overhead and time to correct. best case scenario is that commits take longer, as well.

what i would prefer to see is CI rules that validate all the things via npm run ... calls, making it easy for someone to see that they should run these linting and formatting commands before doing a git push. being able to easily reproduce the CI failure locally, with a simple command, is important to me so that i don't end up relying on CI to tell me what's broken before i can fix it. but being forced to through this pre-commit tends to cause me more annoyance than benefit.

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

@taminomara

Copy link
Copy Markdown
Contributor Author

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 npm install. Contributors who want pre-commit lint check can install manually using npx prek install. They can also change hooks to be pre-push using prek's flags.

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 package-lock.json.

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.

@taminomara

Copy link
Copy Markdown
Contributor Author

Anyways, npm run lint and npm run test are available locally, and lint auto-fixes everything it can. So, there's full CI vs local parity.

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.
@taminomara taminomara force-pushed the chore/setup-prek-precommit branch from 00fc7d2 to 6872546 Compare July 13, 2026 18:58
@mxriverlynn

Copy link
Copy Markdown
Collaborator

love it! just tested this out with and without prek installed, and it works great. 🥳

@mxriverlynn mxriverlynn merged commit 7dcba3b into testdouble:main Jul 14, 2026
mxriverlynn added a commit that referenced this pull request Jul 14, 2026
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.
mxriverlynn added a commit that referenced this pull request Jul 14, 2026
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.
mxriverlynn added a commit that referenced this pull request Jul 14, 2026
## 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.
mxriverlynn added a commit that referenced this pull request Jul 14, 2026
# 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants