Skip to content

fix(CI): repair shellcheck findings and stale rules symlink#126

Merged
mxriverlynn merged 1 commit into
mainfrom
fix/ci-shellcheck-and-symlink
Jul 14, 2026
Merged

fix(CI): repair shellcheck findings and stale rules symlink#126
mxriverlynn merged 1 commit into
mainfrom
fix/ci-shellcheck-and-symlink

Conversation

@mxriverlynn

Copy link
Copy Markdown
Collaborator

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.

The lint job fails on every branch, including main: prek's shellcheck and
check-symlinks hooks have never passed since CI landed in #120.

- init-guidance.sh: the sed expression matches the literal string
  ${CLAUDE_PLUGIN_ROOT}, which must not expand, so the single quotes are
  correct. Silence SC2016 explicitly rather than change the behavior.
- upload-screenshots.sh: the retry loop's counter is never read. Bind it to
  _ so that is stated in the code instead of suppressing SC2034.
- .claude/rules/plugin-entity-taxonomy.md: repoint at the taxonomy's real
  home under han-plugin-builder/, which it was never updated to follow.

Prettier is the lint job's third failure. It needs a repo-wide reformat of
281 files, so it lands separately to keep this branch reviewable.
@mxriverlynn mxriverlynn merged commit bdeecaf into main Jul 14, 2026
1 of 2 checks passed
@mxriverlynn mxriverlynn deleted the fix/ci-shellcheck-and-symlink branch July 14, 2026 18:19
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.

1 participant