feat(go): Go coverage — batch ecosystem entry + go-format hook plugin - #910
Conversation
Adds `go.yaml` to the toolchain plugin's bundled ecosystem defaults (build/test unconditional; golangci-lint lint/fix gated behind an opt-in key, since golangci-lint v2 with no config file still applies its own fixed "standard" linter preset — the same imposed-unconfigured-opinion risk PR #890 fixed for dotnet; project-discovery for nested-module coverage; a go-mod-tidy-drift gate) and a new `plugins/go-format/` hook plugin that runs goimports unconditionally on every .go edit (no consumer-config gate — goimports has no meaningful config-divergence axis when left unconfigured, unlike ruff/dotnet format), skipping generated files. Formatter pick (goimports over gofmt/gofumpt/golangci-lint fmt) resolved via the field survey issue #832 calls for; full rationale, stress-test findings, and empirical verification notes in docs/topics/832-go-ecosystem/PLAN.md. Closes #832 (epic #830, sub-item 2 of docs/topics/lint-static-analysis-gaps/PLAN.md). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Independent review found the guard checked only the file's first
non-blank line, so a common real-world shape — a copyright/license
header (addlicense/goheader-style tooling) preceding the generated-code
marker by a few `//` comment lines — defeated it, silently rewriting
DO-NOT-EDIT files. Also fixes two related defeat vectors caught by the
same review: a trailing CRLF `\r` and a leading UTF-8 BOM.
Scans the file's full leading comment/blank-line run instead of only
line one, matching Go's own stated convention ("before the first
non-comment, non-blank text in the file"). Adds five regression cases
(license-header preamble, CRLF, BOM, and a negative case confirming a
marker appearing after the leading block does not suppress a real edit).
Also simplifies the stderr-capture idiom (drops an unnecessary mktemp
file, matching every sibling hook's plain command-substitution capture)
and corrects an inaccurate PLAN.md claim about an existing schema-check
script that does not exist in this repo's CI.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 5m 30s —— View job PR #910 Review: feat(go): Go coverage — batch ecosystem entry + go-format hook pluginStatus: Review complete ✓ Tasks:
SummaryThis is a well-executed, thoroughly-researched PR. The design decisions are well-reasoned and the implementation is clean. The pre-PR review history (stress-test + independent code review) folded in the critical issues correctly — the generated-file guard is notably thorough and handles all the real-world edge cases (license-header preambles, CRLF, UTF-8 BOM, marker after leading block). Test coverage is excellent. I have one pre-merge action item, one documentation gap worth fixing, and two cosmetic observations. FindingsMUST-FIX before merge:
|
|
Claude finished @kyle-sexton's task in 3m 45s —— View job Security Review — PR #910Scope: No GitHub Actions files were modified. No credential handling, no SQL surfaces, no remote MCP servers, no supply-chain pin changes. FindingsSUGGESTION — Missing
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b7180e0a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CI's comment-hygiene gate flags tracker back-references in source comments (version control owns history, not source comments) and the referenced path was a topic PLAN.md slated for pruning at close-out anyway. Rewrote the rationale to stand on its own. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 34s —— View job PR #910 Review: feat(go): Go coverage — batch ecosystem entry + go-format hook pluginTasks:
ReviewOverall, this is a solid implementation. The independent code review and security review prior to opening caught the two substantive issues (CRITICAL guard completeness, SUGGESTION Two findings below — one documentation inaccuracy and one confirmation of the already-noted FINDING 1 — Documentation inaccuracy in README: "first non-blank line" vs. full leading blockSeverity: Low The README currently reads:
But the implementation (and its own header comment at lines 19–27) correctly scans the full leading comment/blank-line run, not just the first non-blank line. A user reading the README to decide whether their protobuf or stringer output (which often carries a copyright header before the marker) would be protected could conclude it won't be — when in fact it is. The CRITICAL finding from the independent pre-PR review was precisely this mismatch, and it was fixed in the code. The README description was not updated to match. The header comment in Suggested fix: -- **Skips generated files.** A file whose first non-blank line matches Go's
- canonical `// Code generated ... DO NOT EDIT.` marker is left untouched.
+- **Skips generated files.** A file whose leading comment/blank-line block
+ contains Go's canonical `// Code generated ... DO NOT EDIT.` marker is
+ left untouched — this includes files where a copyright/license header
+ precedes the marker, which is common for `addlicense`/`goheader` output.FINDING 2 — Missing
|
|
Claude finished @kyle-sexton's task in 3m 36s —— View job Security Review — PR #910Tasks:
Scope: FindingsNo CRITICAL or IMPORTANT security issues found. SUGGESTION — Missing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 097cfdb872
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review round (Codex + claude-review + security-review) surfaced two
real correctness gaps beyond the pre-PR fixes:
- The generated-file guard only treated `//` lines as comment
continuation, so a `/* ... */` block-comment license header before
the marker caused premature loop termination — confirmed against
`go help generate`'s own text ("must appear before the first
non-comment, non-blank text"), which doesn't restrict "comment" to
`//` style. Fixed by tracking open block comments during the scan.
- goimports ran with no `-local` grouping prefix, so a repo already
formatting with `-local` would have every edit re-collapse its
local-import grouping — empirically confirmed this changes output
when a third-party import is present. Fixed by auto-deriving
`-local` from the edited file's own module path (`go list -m`),
requiring no new consumer-config surface.
Also: added `--` end-of-flags separator (security review, both
passes); widened the go-mod-tidy-drift gate's trigger-globs to
include `*.go` so source-only tidy drift isn't missed, and noted the
Go 1.23+ floor in its remediation text; added `go` to
`/toolchain:lint`'s per-project walking list; corrected README
wording describing the guard; aligned go.yaml/example key ordering.
Six new regression cases added (43/43 passing).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 55s —— View job PR #910 Review: feat(go): Go coverage — batch ecosystem entry + go-format hook pluginStatus: Review complete ✓ Tasks:
ContextThis review covers HEAD What the latest commit fixedAll substantive findings are now addressed:
Remaining item before mergeMUST-FIX:
|
|
Claude finished @kyle-sexton's task in 3m 44s —— View job Security Review — PR #910Scope: No security issues found.The two findings from prior security passes on older commits have both been resolved:
Areas reviewedCommand injection — All tool-input variables (
Path traversal — JSON construction in Telemetry sink execution ( Session ID → marker file — Sanitized to
GitHub Actions — No |
|
Addressed the findings from both review passes (claude-review × 2, security-review × 2, Codex × 4 inline) across two follow-up commits:
Left as-is, with reasoning:
Six new regression cases added (43/43 passing). Full detail and the "confirmed valid, not accepting the dismissal" reasoning on two of the Codex items: Pruning the |
Close-out per the plan's own procedure: the full PLAN.md content is already pasted into PR #910's description (kept in sync through the review-fix rounds), so the topic slice is redundant on main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 31s —— View job PR #910 Final Review — feat(go): Go coverage — batch ecosystem entry + go-format hook pluginStatus: Review complete ✓ Tasks:
SummaryAll findings from the prior three review passes (claude-review ×2, security-review ×2, Codex ×4) have been addressed. The PR is ready to merge. All Prior Findings: Verified Addressed
One Residual Cosmetic Note (no action required)
What's GoodThe block-comment guard implementation is correct: |
|
Claude finished @kyle-sexton's task in 3m 10s —— View job Security Review — PR #910 (HEAD:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bba8dd682f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The go per-project-walking bullet cited context/go.md as a backtick-quoted skill-internal path, but that file lives under the check skill's directory, not lint's — CI's skill-quality-gate correctly flagged it as unresolvable. Reworded to prose, matching this file's existing cross-skill-reference pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 21s —— View job PR #910 Review — feat(go): Go coverage — batch ecosystem entry + go-format hook pluginStatus: Review complete ✓ Tasks:
ContextThis review covers HEAD New Codex P2 — Indented block comments before generated markersSeverity: Very Low (theoretical gap; does not occur in real-world generated Go files) File: if [[ "$_line" == /\** ]]; thenThe Assessment: The Codex reviewer's
The fix would be a trivial 1-line change — use Recommendation: Leave as-is. The prior review explicitly dismissed the block-comment concern before the column-0 fix was added; this indented variant is a further step removed from practice. Document this as a known theoretical gap in a comment if desired. Residual cosmetic (from prior round, unchanged)# this hook skips any file whose
# first non-blank line matches that markerStill says "first non-blank line" — stale shorthand from before the leading-block-scan fix. The inline comment at lines 121–135 and the README are both accurate. No behavior affected; purely cosmetic. Latest commit (9f6bd4f) — confirmed correctThe This matches the existing cross-skill-reference pattern in the file and eliminates the unresolvable skill-internal backtick path that CI flagged. All prior findings — confirmed resolved
VerdictReady to merge. The two remaining items are cosmetic (stale header comment) and theoretical (indented |
|
Claude finished @kyle-sexton's task in 3m 22s —— View job Security Review — PR #910 (HEAD:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f6bd4f535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex caught that comment-shape classification (// and /* checks) matched only column-0, so a generated file with an indented block- comment license header before the marker would defeat the guard — same failure class as the two prior block-comment findings, just indented. Fixed by trimming leading whitespace before classifying a line as comment-continuation; the marker regex itself stays column-0-anchored, matching Go's own convention exactly. New regression case (5g) added, 44/44 passing. Also corrects the top-of-file summary comment, which still described the pre-fix "first non-blank line" behavior. A third Codex finding (a marker literally nested inside an open /* */ span, e.g. "// Code generated..." as text within a block comment) was evaluated and declined — no real Go generator or license-header tool produces that shape. A fourth finding (the ecosystem-commands `gates` schema array has no execution path in check/SKILL.md or lint/SKILL.md, for any ecosystem) is a real, pre-existing, cross-cutting toolchain-plugin gap unrelated to Go specifically — filed as #926 rather than folded into this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
One more review round landed on `225fee6` (Codex, post-prune):
All threads replied to, reacted, and resolved. Waiting on this commit's CI before merge. |
|
Claude finished @kyle-sexton's task in 3m 19s —— View job Claude finished @kyle-sexton's task —— PR #910 Review — feat(go): Go coverage — batch ecosystem entry + go-format hook pluginStatus: Review complete ✓ Tasks:
ContextThis review covers HEAD 225fee6 change: indented block-comment guard fixCorrect. The prior commit ( The fix at _trimmed="${_line#"${_line%%[![:space:]]*}"}"
...
if [[ "$_trimmed" == /\** ]]; then
[[ "$_trimmed" == *'*/'* ]] || IN_BLOCK=1
continue
fi
Case 5g in the test suite exercises The guard logic trace for Case 5g:
Top-level header comment: The stale All prior findings — confirmed resolved at HEAD
No-action items
VerdictReady to merge. All blocking and substantive findings across seven prior review rounds are resolved. The generated-file guard now correctly handles all four real-world leading-block shapes: bare marker, |
|
Claude finished @kyle-sexton's task in 2m 33s —— View job Security Review — PR #910 (HEAD:
|
| Finding | Commit | Status |
|---|---|---|
Missing -- before $FILE in goimports invocation |
43f9233 |
✅ go-format.sh:213 |
/* */ block-comment preamble defeated generated-file guard |
43f9233 |
✅ IN_BLOCK tracking, go-format.sh:142–144 |
Indented ////* lines defeated guard (column-0 only match) |
225fee6 |
✅ _trimmed classification, go-format.sh:150–163 |
Areas reviewed with no findings
Command injection (go-format.sh): $FILE, $GOIMPORTS_BIN, and $LOCAL_PREFIX are double-quoted throughout. LOCAL_PREFIX (from go list -m) is stored as a separate GOIMPORTS_ARGS array element and expanded via "${GOIMPORTS_ARGS[@]}" — Go module path characters carry no shell-special meaning and the quoting prevents word-splitting regardless. build_data_json uses jq -n --arg/--argjson exclusively. Goimports stderr is serialized through jq -R . | jq -s . before entering the findings array.
go list -m → -local pipeline: Executed in a subshell (cd "$(dirname "$FILE")" && go list -m) against the already-validated file's directory. Output stored in $LOCAL_PREFIX, quoted at use. command-line-arguments sentinel is explicitly filtered. Resolution failure degrades to LOCAL_PREFIX="".
Path traversal (hook::read_file_path): Validates [[ -f "$file" ]] (regular file, no FIFOs/devices), resolves symlinks via realpath/readlink -f, enforces a path-segment-anchored prefix check against CLAUDE_PROJECT_DIR. The generated-file guard's read <"$FILE" loop and goimports -- "$FILE" both operate on paths that have passed this validation.
_trimmed derivation in generated-file guard: Pure bash parameter expansion — ${_line%%[![:space:]]*} captures leading whitespace, ${_line#...} strips it. No subprocess, no eval. Safe regardless of _line content.
Telemetry sink execution (hook-utils.sh:455): "$sink" is a consumer-configured path from settings.json — consumer-controlled by design. Envelope built by jq -n --arg/--argjson. Background subshell stdout/stderr redirected to /dev/null.
Session ID → marker files: Sanitized to [A-Za-z0-9_-] before filename use. find … -type f -mtime +7 -delete scoped to ${CLAUDE_PLUGIN_DATA}/skip-notices.
go.yaml commands: All build-cmd, test-cmd, check-cmd, fix-cmd, and gates.cmd values are fixed strings — no variable interpolation, no injection surface.
plugin.json userConfig: Single boolean field mirrored to CLAUDE_PLUGIN_OPTION_GO_FORMAT_ENABLED. No serialization or exec surface.
GitHub Actions: No .github/workflows/ files changed in this PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 225fee627e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The blank-line check (${_line// /}) only stripped literal spaces, so
a tab-only separator line between a comment header and the marker
fell through to the "first non-comment, non-blank line" break case,
defeating the guard — Codex caught this against a real ast.IsGenerated
comparison. Fixed: strips all [[:space:]], not just spaces. New
regression case (5h) added, 45/45 passing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 14s —— View job PR #910 Review — feat(go): Go coverage — batch ecosystem entry + go-format hook pluginStatus: Review complete ✓ Tasks:
ContextThis review covers HEAD
|
| Finding | Fixed in |
|---|---|
MUST-FIX: docs/topics/832-go-ecosystem/ not pruned |
bba8dd6 |
CRITICAL: /* */ block-comment preamble defeated guard |
43f9233 |
P2: no -local grouping prefix → re-collapsed import groups |
43f9233 |
SUGGESTION: missing -- before $FILE (security) |
43f9233 |
| P2: tidy-drift gate missed source-only drift | 43f9233 |
P2: lint/SKILL.md per-project walking list omitted go |
43f9233 |
| LOW: README guard wording said "first non-blank line" | 43f9233 |
| LOW: tidy-drift remediation silent on Go 1.23+ floor | 43f9233 |
| COSMETIC: key ordering between bundled/example YAML | 43f9233 |
CI: unresolvable skill-internal ref in lint/SKILL.md |
9f6bd4f |
P2: indented /* block-comment preamble defeated guard |
225fee6 |
| COSMETIC: top-level header said "first non-blank line" | 225fee6/396587a |
| Codex P2: tab-only separator line defeated guard | 396587a |
No-action items
- BOM stripping on every loop iteration —
_line="${_line#$'\xEF\xBB\xBF'}"runs per-line; a UTF-8 BOM can only appear at byte offset 0. Harmless no-op after line 1; no fix needed. - Marker nested inside an open
/* */span —/* \n// Code generated ...\n*/— not accepted. No real Go generator or license-header tool produces this deliberately mixed-comment-style shape; left as a documented theoretical gap (consistent with prior rounds). gatesexecution gap — thego-mod-tidy-driftgate has no execution path incheck/SKILL.md/lint/SKILL.md(pre-existing cross-cutting gap affecting all ecosystems). Filed as fix(toolchain): wire the ecosystem-commandsgatesschema array into an actual execution step #926; thego.yamlentry stays schema-correct and will activate once fix(toolchain): wire the ecosystem-commandsgatesschema array into an actual execution step #926 lands.
Verdict
Ready to merge. The generated-file guard now correctly handles all real-world leading-block shapes: bare marker, //-style license-header preamble, column-0 /* */ block-comment preamble, indented /* */ block-comment preamble, and tab-only separator lines between any of the above. The 45-case test suite covers all shapes with regression cases. All substantive findings from eight prior review rounds are resolved.
|
Claude finished @kyle-sexton's task in 2m 22s —— View job Security Review — PR #910 (HEAD:
|
| Finding | Fixed in | Verified at HEAD |
|---|---|---|
Missing -- before $FILE in goimports invocation |
43f9233 |
✅ go-format.sh:213: "$GOIMPORTS_BIN" "${GOIMPORTS_ARGS[@]}" -- "$FILE" |
/* */ block-comment preamble bypassed generated-file guard |
43f9233 |
✅ IN_BLOCK tracking at go-format.sh:142–144 |
Indented ////* lines bypassed guard (column-0 match only) |
225fee6 |
✅ _trimmed whitespace-strip at go-format.sh:150 |
| Tab-only separator line bypassed blank-line check | 396587a |
✅ go-format.sh:146: [[:space:]] |
Areas reviewed with no findings
Command injection: $FILE, $GOIMPORTS_BIN, and $LOCAL_PREFIX are double-quoted throughout. LOCAL_PREFIX is derived from go list -m (Go module path characters are syntactically restricted and carry no shell-special meaning), stored as a separate array element (GOIMPORTS_ARGS+=(-local "$LOCAL_PREFIX")), and expanded via "${GOIMPORTS_ARGS[@]}". build_data_json uses jq -n --arg/--argjson exclusively. Goimports stderr is serialized through jq -R . | jq -s . before entering the findings array.
go list -m → -local pipeline (go-format.sh:194–197): Executed in a subshell (cd "$(dirname "$FILE")" && go list -m) against the already-validated file's directory. dirname "$FILE" is safe given $FILE is validated. No output reaches a shell interpreter — stored in $LOCAL_PREFIX and quoted at use. command-line-arguments sentinel is explicitly filtered; resolution failure degrades to LOCAL_PREFIX="".
Generated-file guard (go-format.sh:137–166): Entirely pure bash string operations — [[ ... ]] tests, parameter expansions, =~ regex against a hard-coded pattern. No subprocess, no eval. The _trimmed derivation at line 150 (${_line#"${_line%%[![:space:]]*}"}) is textbook leading-whitespace strip with no subprocess. Safe regardless of _line content.
Path traversal (hook::read_file_path): Validates [[ -f "$file" ]] (regular file, not FIFO/device), resolves symlinks via hook::physical_path, and enforces a path-segment-anchored prefix check against CLAUDE_PROJECT_DIR. The guard's read <"$FILE" loop and goimports -- "$FILE" both operate on a path that has passed this validation.
Telemetry sink (hook-utils.sh:455): "$sink" is consumer-configured in settings.json — consumer-controlled by design. Envelope built entirely by jq -n --arg/--argjson. Background subshell stdout/stderr redirected to /dev/null.
Session ID / marker files: Sanitized to [A-Za-z0-9_-] before filename use. find … -type f -mtime +7 -delete scoped to ${CLAUDE_PLUGIN_DATA}/skip-notices.
go.yaml commands: All build-cmd, test-cmd, check-cmd, fix-cmd, and gates.cmd values are fixed strings — no variable interpolation, no injection surface.
plugin.json userConfig: Single boolean field mirrored to CLAUDE_PLUGIN_OPTION_GO_FORMAT_ENABLED. No serialization or exec surface.
hooks.json: Hook command is "${CLAUDE_PLUGIN_ROOT}"/hooks/go-format.sh — references the plugin-root env var as documented; no injection surface in the manifest itself.
GitHub Actions: No .github/workflows/ files changed in this PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 396587aa9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…Generated gaps (#937) ## Summary `plugins/go-format/hooks/go-format.sh`'s generated-file guard is a bash string-match approximation of Go's own `ast.IsGenerated` (a parsed-AST classifier via `go/parser`). Across PR #910's review, an automated Codex pass progressively surfaced — and each time got a real fix for — a bare marker, a `//`-header before the marker, a `/* */` block-comment header, an *indented* block-comment header, and a tab-only blank separator (now covered by regression cases 5–5h in `go-format.test.sh`). That is the expected shape of chasing string-match parity with an AST-based detector: the gap is **structural, not a single fixable bug**, so further narrow edge cases can keep surfacing indefinitely. The guard is advisory only — the hook never blocks an edit; the worst case for an unhandled shape is goimports reformatting a generated file, visible in the diff, no gate broken. This PR documents that deliberate scope boundary at the guard's location so the known-limitation is explicit rather than implicit, and re-litigating the tradeoff isn't needed each time a bot or human surfaces a new exotic shape. ## Fix Documentation-only. No behavior change, no logic change, no test change. - **`plugins/go-format/hooks/go-format.sh`** — extended the existing `GENERATED-FILE GUARD` inline comment block (at the guard's scan logic) with an `APPROXIMATION:` note capturing: (1) the scan is a deliberate string-match stand-in for `ast.IsGenerated`, which can't be made equal by adding patterns because the gap is structural; (2) that's tolerable because the guard is advisory (worst case = a generated file gets reformatted, visible in the diff); (3) no further pattern patches unless a **real-world** generated file is observed defeating it (a synthetic counter-example is not enough); (4) the actual structural fix — a `go`-toolchain shell-out (`go/parser` + real `ast.IsGenerated`) — is deliberately not taken, to avoid per-edit latency and a hard `go` dependency this hook otherwise keeps optional. Placed as a maintainer-facing code comment (single home) rather than duplicated into the user-facing README, whose "Skips generated files" bullet already covers the behavior. The comment references no issue/PR number — it stands on its own. - **`plugins/go-format/CHANGELOG.md`** — new `## [0.1.1]` `### Added` entry. - **`plugins/go-format/.claude-plugin/plugin.json`** — patch bump `0.1.0` → `0.1.1`. ## Verification Docs-only: verification is "no code/behavior change; existing suite still passes." Ran the full go-format hook test suite (a real `goimports` v0.48.0 on PATH) before and after the comment edit: ``` $ GOIMPORTS_TEST_BIN="$HOME/go/bin/goimports.exe" bash plugins/go-format/hooks/go-format.test.sh ... PASS=45 FAIL=0 ``` `shellcheck plugins/go-format/hooks/go-format.sh` → clean. Closes #927 ## Related - #910 — the PR whose review (a progressive Codex edge-case pass) produced this issue and the 5a–5h regression cases. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#867) ## Summary Adds `lychee-offline` to the `toolchain` plugin's bundled `cross-cutting.yaml` ecosystem default, alongside the existing `typos`/`gitleaks`/editorconfig-checker tools — on-disk link/anchor integrity checking with no network dependency. ## Fix - `reference/ecosystems/cross-cutting.yaml`: `check-cmd` gains `lychee --offline --no-progress './**/*.md'` (`--offline` = "Only check local files and block network requests" per lychee's own CLI help — external URLs are skipped, only local file and fragment targets are verified); `opt-in` and `install-hint` documented following the same per-tool-config pattern as the existing tools (optional `lychee.toml` at repo root customizes the ruleset; absent means lychee's own defaults). - `skills/setup/SKILL.md`: cross-cutting inference-candidate line mentions `lychee-offline`, and all four cross-cutting tool names backtick-wrapped consistently. - `skills/lint/evals/evals.json`: eval 3's `expected_output` updated to include `lychee-offline` in the enumerated cross-cutting tool set. - `plugins/toolchain/.claude-plugin/plugin.json` + `CHANGELOG.md`: version bump. **Version note**: sibling PR #859 (issue #834) landed first and took the intervening `0.6.0`/`0.7.0` versions; PR #910 (issue #832) then landed `0.8.0`. Rebased onto current `main` and renumbered to `0.9.0`, sequential CHANGELOG history intact. Collision with #859 is resolved (merged); `do-not-merge` label removed. ## Verification - Confirmed `--offline` and `--no-progress` are real lychee CLI flags, and the winget package id (`lycheeverse.lychee`) / brew formula (`lychee`) are correct, against lychee's own upstream README (`lycheeverse/lychee`, `master` branch, Installation and Commandline usage sections). - Smoke-tested the actual `check-cmd` (`lychee --offline --no-progress './**/*.md'`) against this repo: 0 errors across 2645 links (1079 unique, 1056 excluded). - `evals.json` still parses as valid JSON; structural shape (required `id`/`prompt` fields) unchanged — only `expected_output` prose edited. - `skills/setup/SKILL.md` re-checked against the 500-line skill-quality hard cap. - Rebased onto current `origin/main`; `claude plugin validate --strict`, `node scripts/validate-plugin-contracts.mjs`, and full `bash scripts/run-plugin-tests.sh` all green. Closes #833 ## Related - Epic: #830 - Brief item 3, `docs/topics/lint-static-analysis-gaps/PLAN.md` - Sibling PR on the same plugin: #859 (issue #834, pyright) — merged, serialization resolved --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
#926) (#1020) ## Summary `docs/conventions/ecosystem-commands/ecosystem.schema.json` defines a structured `gates` array (`name`/`cmd`/`trigger-globs`/`remediation`) that any ecosystem config — bundled default or consumer-tracked — can declare, and both `plugins/toolchain/reference/ecosystems/go.yaml` (`go-mod-tidy-drift`) and `docs/conventions/ecosystem-commands/examples/dotnet.yaml` (`nuget-lockfile-drift`) ship one. The array was resolved as part of each ecosystem's command surface (`check/SKILL.md` §1.5) but **never invoked by any workflow step** — so every declared gate was inert. The only gate-execution mechanism `check/SKILL.md` documented was a prose, consumer-CLAUDE.md-only step whose closing sentence ("this plugin ships none of its own") was itself stale once `go.yaml` shipped a bundled gate. ## Fix Scoped to `plugins/toolchain/skills/check/SKILL.md`, precise and additive (no restructuring of unrelated parts): - **New structured gate-execution step** in §2 "Run checks", replacing the stale prose step at the old `:121`. After an affected ecosystem's build → test → lint, iterate its resolved `gates` array (§1.5 ladder — bundled default or consumer file). Per gate: - **Fire condition** — `trigger-globs` present → run only when ≥1 changed file matches, against the **full** changed-files set (a gate's trigger files need not classify into the ecosystem's own `globs`); omitted → run whenever the ecosystem runs; present-but-no-match (e.g. `check all` on a clean tree) → does not fire. - **Independent of the build/test/lint short-circuit** — a fired gate runs even when an earlier phase failed and stopped (a lockfile / `go mod tidy` gate is meaningful regardless of build success). - **Tool presence** — missing tool → `skip` with the ecosystem's `install-hint`, never `FAIL`. - **Outcome** — `pass`/`FAIL` by name; on `FAIL` surface `gate.remediation`; a fired-and-failed gate counts toward the run's FAIL verdict. - **Overall verdict** now covers gate-only failures — a failed fired gate flips Overall to `FAIL` and is counted even when every ecosystem's build/test/lint cell passed (e.g. `Overall: FAIL (0 of 2 ecosystems failed, 1 gate failed)`). - **Stale sentence removed** — the "this plugin ships none of its own" framing is replaced with an accurate ladder pointer (bundled default e.g. `go.yaml`'s `go-mod-tidy-drift`; consumer-declared in `.claude/ecosystems/<ecosystem>.yaml`). - **Cross-reference reconciled** at the old `:119` ("CI-parity gates … run independent of `check-cmd`") and the report scaffold (`:140`). **`lint/SKILL.md` deliberately unchanged** (the one maintainer-vetoable fork, per the planning comment): every schema-described gate and both shipped examples are CI-parity checks beyond build/test/lint (lockfile drift, module drift, generated-artifact freshness) — none is a lint/format op. They belong with the heavier `check` step and `/verification:confirm` (the sanctioned pre-commit gate); adding them to `/toolchain:lint` would break its "fast" contract. Veto before merge if a maintainer wants gates on the lint path. **Version bump (rule 6d):** fresh `origin/main` had `plugins/toolchain` at `0.9.0` (claimed by the lychee PR #867); this is a new executable behavior, so bumped to `0.10.0` with a matching `## [0.10.0]` → `### Added` CHANGELOG entry. (The planning comment's `0.8.0 → 0.9.0` predated #867 landing on main.) ## Verification No skill-behavior test harness exists (`find plugins/toolchain -iname '*test*'` and a repo-level search for `toolchain:check`/`gates` tests both empty), so verification is static gate + hand-trace: - **`/skill-quality:check` on the edited `check` skill:** PASS — 0 errors, markdownlint clean, 165/500 lines, all base-ref trigger phrases preserved (frontmatter `description` untouched). The lone WARN (description lacks `Use when:` phrasing) is pre-existing and out of scope. - **Hand-trace — bundled default (`go.yaml`, a real rung-4 default that runs for this repo):** a change set including `go.sum` (or any `*.go`) makes `go` affected; §1.5 resolves `go.yaml`; the new §2 step reaches its `gates` and matches `go-mod-tidy-drift`'s `trigger-globs: ["go.mod","go.sum","*.go"]` against the full changed-files set → gate fires → runs `go mod tidy -diff` from `$REPO_ROOT`. Drift → `FAIL` + remediation + Overall FAIL; clean → `pass`; missing Go toolchain → `skip` with `install-hint`. - **Hand-trace — consumer-declared shape (`examples/dotnet.yaml`, an example fixture under `docs/`, not a bundled default):** for a consumer whose tracked `.claude/ecosystems/dotnet.yaml` carries the `nuget-lockfile-drift` gate, a changed `*.csproj` matches `trigger-globs: ["*.csproj","Directory.Packages.props","packages.lock.json"]` → gate fires → runs `dotnet restore --locked-mode`. Same pass/FAIL/skip semantics. Both gate entries are invoked by the new step given a matching changed-files set; neither required a per-ecosystem change. Closes #926 ## Related - PR #910 (Go ecosystem) — **merged** `2026-07-22`; added `go.yaml`'s `go-mod-tidy-drift` gate whose Codex review surfaced this wiring gap. This change is the cross-cutting follow-up that makes that gate (and every other declared gate) actually run — additive, no `go.yaml` change. - Planning-subagent comment on #926 (`2026-07-22T03:10:48Z`) — the primary implementation plan followed here; the only deviation is the version target (`0.10.0`, re-derived from fresh `origin/main` per rule 6d, since `0.9.0` was taken by #867 after the plan was written). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019xSPACJvbsqGsjYCP91DoX Work-class: C3 (bug-fix-shaped) — attended triage 2026-07-23, operator-ratified. 🤖 --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 participant
Summary
Two deliverables per issue #832's "Go coverage, both lanes":
plugins/toolchain/reference/ecosystems/go.yamlbatch ecosystem default (+ matchingdocs/conventions/ecosystem-commands/examples/go.yamlfixture) —go build ./.../go test ./...unconditional;golangci-lint run [--fix] ./...gated behind anopt-inkey (.golangci.yml/.golangci.yaml/.golangci.toml/.golangci.jsonpresence);project-discovery: ["go.mod"]for nested-module coverage; ago-mod-tidy-driftgate viago mod tidy -diff.plugins/go-format/— runsgoimports -won every.goWrite/Edit, unconditionally (no consumer-config opt-in gate — the one deliberate shape difference from siblingruff-format/typos-format), skipping files carrying Go's// Code generated ... DO NOT EDIT.marker.Design decisions
Issue #832 calls for an "implementation-time field survey" to pick the per-file formatter (gofmt/goimports/gofumpt/golangci-lint fmt) against three criteria: official/authoritative, maintained, feature-fit.
goimportspicked: its own docs state it "formats your code in the same style as gofmt so it can be used as a replacement for your editor's gofmt-on-save hook" — a direct official statement of intent for this exact per-file-hook scenario. Actively maintained (golang/tools, v0.48.0). Handles import add/remove, which baregofmtnever does — necessary since an LLM edit changing symbol usage can leave a broken import list.gofumptrejected as the unconditional default: third-party opinionated superset ("a stricter gofmt"), same risk class as whyruff-format/dotnet formatgate behind consumer config. No toggle added in v1 (no consumer has asked; can be added later if requested).golangci-lint fmtrejected for the per-file hook: with no config present it has zero formatters enabled by default (does nothing) — corroborates the issue's own "golangci-lint is batch-only by design" framing, for the correct reason (verified via the tool's own behavior, not just restated from the brief).go-formatruns unconditionally —goimportshas no meaningful config-divergence axis when left unconfigured (unlike ruff/dotnet-format, whose underlying tools genuinely diverge by config), so gating it would be inert ceremony.golangci-lint's lint step IS gated — empirically verified golangci-lint v2 with no config file still applies its own fixed "standard" linter preset unconditionally, the same imposed-unconfigured-opinion risk PR feat(toolchain): dotnet opt-in gate on .editorconfig, visible skip status #890 (issue feat(toolchain): dotnet batch guard — runtime analyzer/editorconfig detection #835) fixed fordotnet format. Mirrors the existingpython.yaml/dotnet.yamlopt-inpattern.Full rationale, stress-test findings, and empirical verification notes:
docs/topics/832-go-ecosystem/PLAN.md(pasted below).Review history
Two independent fresh-context passes ran before this PR, findings folded in:
Plan stress-test (before implementation) — one HIGH finding: the "goimports has no config-divergence axis" premise missed that
goimportshas zero awareness of Go's generated-file convention, and would silently rewrite generated files. Fixed by adding a marker-skip guard to the hook before implementation began. Also found a MEDIUM multi-module gap (./...doesn't cross a nestedgo.modboundary — fixed by addingproject-discovery: ["go.mod"]) and corrected two citation errors (a misattributedgolangci-lint fmtquote; an inaccurate "every ecosystem has a matching example fixture" claim).Independent code review (after implementation, before PR) — one CRITICAL finding: the generated-file guard checked only the file's first non-blank line, missing the common real-world shape where a copyright/license header (
addlicense/goheader-style tooling) precedes the marker by a few//comment lines — reviewer empirically reproduced the miss against a real generated-file shape and confirmed the hook silently rewrote it. Fixed: the guard now scans the file's full leading comment/blank-line run per Go's own stated convention ("before the first non-comment, non-blank text in the file"), and two related defeat vectors caught by the same review (trailing CRLF, leading UTF-8 BOM) are also fixed. Five new regression cases added. Two SUGGESTION-level findings (an unnecessarymktempfor stderr capture; an inaccurate PLAN.md claim about a nonexistent schema-check script) were also fixed.Test plan
plugins/go-format/hooks/go-format.test.sh— 41/41 passing (FAKEBIN-pattern contract test, realgoimportsv0.48.0 binary), including 5 generated-file-guard regression cases (bare marker, license-header preamble, CRLF, BOM, marker-after-leading-block negative case).check-jsonschemaagainstecosystem.schema.json— bothgo.yamlfiles (bundled default + example fixture) validate; against the plugin manifest schema forplugins/go-format/.claude-plugin/plugin.json.scripts/check-changed-skills.sh/ directcheck-skill.shinvocation —go-format:setup,toolchain:check,toolchain:lintall PASS.scripts/sync-hook-utils.sh --check,scripts/check-cross-plugin-source-drift.sh,scripts/check-silent-skips.sh,scripts/check-changelog-parity.sh --check,node scripts/validate-plugin-contracts.mjs,bash scripts/validate-plugins.sh(strict catalog validation) — all pass.typos,markdownlint-cli2,shellcheckon all new/changed files — clean.node scripts/generate-catalog.mjs --check— in sync.go mod tidy -diffand golangci-lint's unconfigured-defaults behavior against real installed toolchains (Go 1.26.5, golangci-lint v2) — see PLAN.md.Related
docs/topics/lint-static-analysis-gaps/PLAN.md, contract landed via PR docs: add lint/static-analysis gaps brief #829).PLAN.md (implementation plan, decisions, and review history)
Brief
Issue #832 (epic #830, sub-item 2 of
docs/topics/lint-static-analysis-gaps/PLAN.mdlines 26-30). Two deliverables, one PR:plugins/toolchain/reference/ecosystems/go.yamlbatch ecosystem entry + matchingdocs/conventions/ecosystem-commands/examples/go.yamlfixture.plugins/go-format/— per-filegoimportsautofix on Write/Edit.Scope boundaries: batch additions are rung-4 defaults only (consumer
.claude/ecosystems/*.yamloverride ladder unchanged).
govulncheckis explicitly "optional" per the brief — NOT shipped as adefault; documented as a consumer-addable local gate only. No
gofumpttoggle in v1 (YAGNI — noconsumer has asked).
Success criteria: both new surfaces pass the plugin contract gate + fleet conformance audit
(acceptance criteria line 61 of the epic Brief); CI/local parity restored for the Go toolchain gap
identified 2026-07-21 (line 66-67).
Open Decisions (resolved this session, recorded here for the approval gate)
Formatter pick for the per-file hook:
goimports. Field survey against the Brief's owncriteria (official/authoritative, maintained, feature-fit), researched fresh this session:
gofmt— non-configurable by design (go.dev/blog/gofmt, go.dev/doc/effective_go), but doesn'tmanage imports; an LLM edit that adds/removes symbol usage leaves a broken file.
goimports(golang.org/x/tools, v0.48.0, 2026-07-09; ~7,983 commits, last updated2026-07-20) — its own docs state it "formats your code in the same style as gofmt so it can be
used as a replacement for your editor's gofmt-on-save hook." Direct official statement of
intent for exactly this scenario. Picked.
gofumpt(mvdan/gofumpt v0.10.0, 2026-05-04) — self-branded "a stricter gofmt," third-partyopinionated superset. Rejected as the unconditional default (same class of risk as why
ruff-format/dotnet-format gate behind consumer config); not adding a toggle in v1.
golangci-lint fmt— rejected for the per-file hook. Stress-test correction: the plan'soriginal citation was imprecise —
golangci-lint fmt --stdinIS a genuine single-file,non-package-scoped invocation (empirically confirmed: no
go/packagesloading, formats pipedstdin instantly). The "directories are NOT analyzed recursively... files must come from the
same package" quote governs
golangci-lint run(linters), notfmt(formatters) — don'tmisattribute it. The correct, empirically-confirmed rejection reason: with no config file
present,
golangci-lint fmthas zero formatters enabled by default (unlikerun's fixed"standard" 5-linter preset) — i.e. it silently does nothing, the opposite of a usable default.
Conclusion (rejected for the hook) is unchanged; only the stated reason is corrected.
Independently corroborates the Brief's own line 30 ("golangci-lint is batch-only by design")
for the
run/lint half of the tool.go-formatrunsgoimportsunconditionally — no consumer-config opt-in gate.This is the one deliberate shape difference from ruff-format/typos-format/dotnet-format's
opt-in-gated pattern. Rationale: goimports has no meaningful config-divergence axis when left
unconfigured, so gating it would be inert ceremony. Qualifies for
docs/PLUGIN-PHILOSOPHY.mdlines 143-147 lane-1 treatment (non-conflicting good-practicedefault) where gofumpt/golangci-lint-fmt would not.
was incomplete — empirically confirmed goimports rewrites files carrying the canonical
// Code generated ... DO NOT EDIT.marker with zero awareness of that convention, whilegolangci-lint's own linters/formatters default to
issues.exclude-generated: strictin v2.Generated Go files (protobuf, mockgen, sqlc, stringer, wire output) are common; an
unconditional hook would silently rewrite them on any edit. Fix, not a re-open of the
unconditional-default decision:
go-format.shadds a generated-file marker guard — skip(emit_skipped) when the marker appears anywhere in the file's leading
comment/blank-line run (scanning stops at the first line that is neither blank nor a
//comment), matching Go's own stated convention ("before the first non-comment, non-blank text
in the file"), not just the first non-blank line. Independent-review correction (CRITICAL,
folded in post-stress-test): the original implementation checked ONLY the first non-blank
line, on the premise that a preamble before the marker is "uncommon" — that premise was
false; a license/copyright header (common
addlicense/goheadertooling output) routinelyprecedes the marker by several
//lines, and this shape is empirically present in real Gostdlib-adjacent generated files. Fixed by scanning the full leading comment/blank block
instead of only line one; also fixed two related defeat vectors caught by the same review
(trailing CRLF
\rnot stripped before the$anchor; a leading UTF-8 BOM defeating the^anchor) and added five regression test cases (license-header preamble, CRLF, BOM, and a
negative case confirming a marker appearing AFTER the leading block does NOT suppress a real
edit). This is precision-scoping (same category as
--force-excludegiving Ruff per-file skipprecision), not a consumer-config
walk-up, so it does not undermine Open Decision 1's "unconditional" framing.
golangci-lintlint step gated behind anopt-inkey requiring.golangci.yml/.golangci.yaml/.golangci.toml/.golangci.jsonpresent (ancestor walk torepo root, mirroring
dotnet.yaml'sroot = trueboundary if an analogous concept exists forgolangci-lint — verified during Phase 1 to NOT exist, so ceiling at repo root only). Verified
via WebFetch/WebSearch this session (golangci-lint.run/docs/configuration,
github.com/golangci/golangci-lint discussions, 2026): golangci-lint v2 with no config file
present still runs its own fixed "standard" linter preset (
linters.default: standard)rather than erroring or running nothing — the same imposed-unconfigured-opinion risk class
PR feat(toolchain): dotnet opt-in gate on .editorconfig, visible skip status #890 (issue feat(toolchain): dotnet batch guard — runtime analyzer/editorconfig detection #835) just fixed for
dotnet format's unconfigured Roslyn defaults, andconsistent with why
python.yamlalready gates ruff behind[tool.ruff]/ruff.tomlpresence.Mirrors
python.yaml:10anddotnet.yaml:34'sopt-inkey shape exactly.Single PR, closes feat(go): Go coverage — batch ecosystem entry + format hook plugin #832. Both deliverables are one Brief line ("Go coverage, both lanes"),
share the same formatter research, and match the one-issue-one-PR precedent from feat(typos-format): per-file typos autofix hook plugin #831/feat(toolchain): dotnet batch guard — runtime analyzer/editorconfig detection #835.
Plan
Phase 1:
go.yamlecosystem batch entry [DONE]Files:
plugins/toolchain/reference/ecosystems/go.yaml(new) — flat top-level keys, bundled-fallbackdisclaimer header (mirrors
dotnet.yaml:1-27style):globs: ["*.go", "go.mod", "go.sum"]project-discovery: ["go.mod"]— added per stress-test MEDIUM finding: empiricallyconfirmed
go build ./.../go test ./.../go list ./...run from a repo root silently skipa nested module's packages (a nested
go.modbounds./...expansion; a rootgo.workfiledoes NOT cross module boundaries for this either). Without
project-discovery, a monorepo witha nested Go module gets silent incomplete build/test/lint, directly undercutting this issue's
own "CI/local parity" success criterion. Mirrors
python.yaml/typescript.yaml's existingproject-discoveryhandling for the same class of problem.build-cmd: "go build ./..."test-cmd: "go test ./..."check-cmd: "golangci-lint run ./..."fix-cmd: "golangci-lint run --fix ./..."opt-in: text per Open Decision 2 above.install-hint: resolved per stress-test finding — do NOT bake in a pinnedcurl|sh -s -- -b ... vX.Y.Zone-liner (upstream's own install docs explicitly statego install/go get"aren't guaranteed to work" for golangci-lint, and a version-pinnedcurl|sh command drifts immediately). Use a durable pointer instead:
"Install golangci-lint: https://golangci-lint.run/docs/welcome/install/ | Go toolchain: https://go.dev/dl/"— same durable-pointer style asdotnet.yaml's"Install .NET SDK from https://dot.net".gates: one entry,go-mod-tidy-drift,trigger-globs: ["go.mod", "go.sum"],cmd: "go mod tidy -diff"— confirmed live viago help mod tidy(Go 1.26.5 installed thissession): "-diff causes tidy not to modify go.mod or go.sum but instead print the necessary
changes as a unified diff. It exits with a non-zero code if the diff is not empty." Introduced
Go 1.23 (2024) — note this as an implicit minimum-Go-version prerequisite in
context/go.md.remediation: "Run go mod tidy and commit the updated go.mod/go.sum."notes: mentiongovulncheckas an available consumer-addable local gate (not shipped bydefault per Open Decision brief-wording), pointing at
.claude/ecosystems/go.local.yaml.docs/conventions/ecosystem-commands/examples/go.yaml(new) — richer worked-example fixturemirroring
examples/dotnet.yaml's structure (same keys as above plus thegatesblock spelledout). Correction (stress-test finding): only 3 of the 8 existing bundled ecosystem yamls
(
bash,python,dotnet) actually have a matching example fixture —markdown,powershell,typescript,cross-cutting,yamldo not, and no CI gate enforces 1:1 coverage. Addexamples/go.yamlanyway (it's good practice and dotnet/python both have one), but don't claimin the PR body that this closes a universal-coverage gap — it doesn't exist as a gap.
plugins/toolchain/skills/check/context/go.md(new) — Go-specific gotchas:go test ./...module-root requirement (must run from the module root or a path containing
go.mod;project-discoveryabove handles nested-module walking), GOFLAGS interactions,golangci-lint's default-"standard"-preset caveat tied to the opt-in gate, AND (stress-test
MEDIUM finding) golangci-lint's own config discovery falls back to the user's home directory
with no
root = true-equivalent stop marker when no repo-level config is found — a stray~/.golangci.ymlon a developer's machine makes local runs diverge from a clean CI container;document this as a known local/CI divergence source. Mirrors the existing
context/dotnet.md/context/python.mdshape.plugins/toolchain/skills/check/SKILL.md— addgoto the covered-ecosystems list (currently:dotnet, python, typescript, bash, powershell, markdown) and its alias table if one applies (no
common alias needed — "go" is already short).
plugins/toolchain/skills/lint/SKILL.md— same covered-ecosystems list addition iflintalsoenumerates them explicitly (verify during implementation; python's opt-in-gated lint precedent
from feat(toolchain): dotnet batch guard — runtime analyzer/editorconfig detection #835 touched both
check/SKILL.mdandlint/SKILL.md, sogolikely needs the sametwo-file touch).
plugins/toolchain/.claude-plugin/plugin.json— version bump (minor: new ecosystem capability).Verify current version live (
git show origin/main:plugins/toolchain/.claude-plugin/plugin.jsonat rebase time — do not assume 0.6.0 is still current, feat(toolchain): add lychee-offline to cross-cutting ecosystem default #833/feat(toolchain): add pyright to python ecosystem check-cmd #834 may have already bumped it).
plugins/toolchain/CHANGELOG.md—[Unreleased]/new version entry under Added.Sanity Check:
check-jsonschema --schemafile docs/conventions/ecosystem-commands/ecosystem.schema.json <file>passes for bothplugins/toolchain/reference/ecosystems/go.yamlanddocs/conventions/ecosystem-commands/examples/go.yaml. Independent-review correction: no CIjob or repo script actually validates
reference/ecosystems/*.yaml/examples/*.yamlagainst thisschema today (confirmed by grepping
.github/workflows/ci.yml— its fourcheck-jsonschemastepscover only marketplace/plugin manifests, dependabot, and workflow files); the check above is a
manual
check-jsonschemaCLI run, not an existing repo script being reused. Both files validatedclean this way. This is a pre-existing gap in the repo's own CI coverage, out of scope for this
issue — noted here rather than silently left as an inaccurate claim.
Phase 2:
plugins/go-format/hook plugin [DONE]Files (full new plugin directory, mirroring
plugins/typos-format/structure):.claude-plugin/plugin.json—userConfig.go_format_enabled(boolean, defaulttrue), version0.1.0, keywords["go","golang","goimports","formatter","hook"].hooks/hooks.json—PostToolUse, matcherWrite|Edit, command"${CLAUDE_PLUGIN_ROOT}"/hooks/go-format.sh, timeout 15.hooks/hook-utils.sh— initial copy viascripts/sync-hook-utils.sh(never hand-copy).hooks/go-format.sh— control flow mirrorsruff-format.sh's shape:*.go(jq-free, before requiring jq) — like ruff-format, unliketypos-format's no-filter shape.
hook::check_enabled "GO_FORMAT",hook::buffer_stdin,hook::require_jq,hook::read_file_path,hook::repo_root.document this simplification explicitly in a short header comment referencing this PLAN's
rationale, not just silently omitting the walk-up ruff-format/typos-format both have.
command -v goimports) — no.venv-style walk (Go has noper-project virtualenv concept; mirrors typos-format's PATH-only resolution).
binary):
goimports -w -l "$FILE"in one pass —-wwrites the fix,-l(list-only)combined with
-wstill lists the filename if changes were needed, giving a single-passfix+detect (simpler than ruff-format's two-pass shape). Exit-code semantics confirmed:
-lALWAYS exits 0, even when it lists a file needing changes — there is no exit-1-style
"findings" signal like ruff/typos have. Detect "changes were made" from non-empty stdout
(the listed filename), not from exit code. Non-zero exit (confirmed: exit 2, parseable message
on stderr) occurs only on a genuine parse/syntax error — surface that as a finding-text
message (mirrors how ruff-format surfaces a mid-edit syntax error as a finding, not a
tool-break), matching the doctrine even though the underlying signal shape differs from ruff.
Skip entirely (before invoking goimports) when the file matches the generated-file marker guard
from Open Decision 1's stress-test correction above.
hook::emit_telemetry "go-format" "PostToolUse" <status> "$start" "$data_json" "$REPO_ROOT";statussemantics per the shared doctrine (ok= ran to judgment,skipped=tool broke/missing prerequisite).
hooks/go-format.test.sh— FAKEBIN-pattern contract test mirroringruff-format.test.sh/typos-format.test.sh: gate-off, non-.goextension skip, clean file,import-added-by-edit auto-added, import-removed-by-edit auto-removed, generated-file marker
skip (new case per the Open Decision 1 stress-test correction), missing-binary dim-9 visibility
(once-per-session), missing-jq dim-9 visibility, kill-switch, telemetry envelope shape assertions
(
schema_version/timestamp/hook/hook_event/status/duration_ms/data), asyntax-error-mid-edit case surfaced as a finding (per the confirmed
-lexit-2/stderr behaviorabove, not a tool-break).
skills/setup/SKILL.md—check/apply,disable-model-invocation: true.checkprobes Bash,jq,
goimportson PATH, thego_format_enabledtoggle, hook registration.apply: resolvedper stress-test finding — guidance-only, no write path, matching
typos-format's pattern(
plugins/typos-format/skills/setup/SKILL.md:57-64), notruff-format's.venv-install path.go install golang.org/x/tools/cmd/goimports@latestwrites to the machine-global$GOBIN/$GOPATH/bin(not project-scoped) and@latestis not idempotent-pinned (silentlydrifts over time) — structurally the same "no per-repo dependency-manager, machine-level binary"
case as typos-format, not ruff-format's project-
.venvcase. Document the goimports installpointer (
go install golang.org/x/tools/cmd/goimports@latest— a plain, uncaveatedgo install,unlike golangci-lint's own install docs which explicitly warn
go install/go get"aren'tguaranteed to work" for that tool specifically — don't let that caveat bleed across into this
SKILL.md's goimports guidance).
README.md— mirror typos-format's structure; explicitly document the "no config-gate,unconditional default" design choice (the one plugin in the family without an opt-in section) —
say so plainly, don't silently omit the section.
CHANGELOG.md—[0.1.0]initial release entry, telemetry-conformant from day one (matchestypos-format's precedent, not ruff-format's later-follow-up pattern).
docs/conventions/hook-telemetry/data/go-format.schema.json(new) — findings shape decided fromPhase 2's live verification of goimports' actual diagnostic output (flat-string per ruff-format's
shape unless goimports emits something structured — verify, don't assume).
docs/conventions/hook-telemetry/README.md— add Implementers table row forgo-format..claude-plugin/marketplace.json— new entry:category: "development",tags: ["go","golang","goimports","formatter","hook"],relevance: {topic: "Go", signals: {filesRead: ["**/*.go"], cli: ["goimports"]}}(mirrorsruff-format/typos-format's relevance-block shape exactly).
README.md(repo root) — regenerate catalog block vianode scripts/generate-catalog.mjs.Sanity Check:
plugins/go-format/hooks/go-format.test.shpasses 100% when run directly(
bash plugins/go-format/hooks/go-format.test.sh), andscripts/sync-hook-utils.sh --checkreports no drift for the new copy.
Phase 3: Cross-cutting verification + PR [DOING]
origin/main(expect a benign conflict onplugins/toolchain/.claude-plugin/plugin.json+plugins/toolchain/CHANGELOG.mdagainstfeat(toolchain): add lychee-offline to cross-cutting ecosystem default #833/feat(toolchain): add pyright to python ecosystem check-cmd #834's already-merged or still-in-flight changes — resolve by reapplying this lane's version
bump on top of theirs, not by discarding either).
hook-utils-sync, cross-plugin-source-drift, silent-skip-gate, changelog-parity-gate,
skill-quality-gate + portability-lint, plugin-gate (
scripts/validate-plugin-contracts.mjs+both new/changed test suites),
scripts/generate-catalog.mjs --check.gh pr create— closes feat(go): Go coverage — batch ecosystem entry + format hook plugin #832, body includes the two locked Open Decisions above (formatter pick +opt-in-gate rationale) so reviewers see the reasoning, not just the diff.
resolveReviewThreadforbot-authored addressed threads — this repo's ruleset requires
required_review_thread_resolution), merge, remove worktree, delete branch, confirm issueauto-closed.
/planning:plan close-out: paste this PLAN.md into the PR body<details>block, prunedocs/topics/832-go-ecosystem/before merge.Sanity Check:
gh pr view <N> --json state -q .statereturnsMERGED;gh issue view 832 --json state -q .statereturnsCLOSED.Review history
An independent fresh-context code review ran before PR creation and found one CRITICAL and two
lower-severity items, all addressed and re-verified before opening the PR:
license-header-then-marker layout (and two related defeat vectors: CRLF, UTF-8 BOM).
Reviewer empirically reproduced the miss against a real generated-file shape (a copyright
header plus a
stringer-style marker) and confirmed the hook silently rewrote it. Fixed: the guard nowscans the file's full leading comment/blank-line run (stopping at the first non-comment,
non-blank line) per Go's own stated convention, strips a trailing CRLF and leading BOM per
line before matching. Five new regression cases added (license-header preamble, CRLF, BOM,
and a negative case proving a marker appearing after the leading block does NOT suppress a
real edit) — see Open Decision 1 above for the full before/after.
mktempfile, inconsistent with everyother hook in the repo's simpler command-substitution idiom. Simplified to match.
ecosystem yaml against
ecosystem.schema.json; no such CI job or script exists. Corrected theclaim (see Phase 1's Sanity Check above) — the schema validation itself was and remains correct
(verified manually via
check-jsonschema), only the "reuse an existing script" framing waswrong. This is a pre-existing repo-wide CI-coverage gap, out of scope here.
On-PR review round (
claude-review,security-review, and Codex, both pushes) surfaced eightmore findings. Six confirmed and fixed, two evaluated and consciously left as-is:
go help generatelive) —the generated-file guard still missed a
/* ... */block-comment preamble. The authoritativeconvention text ("This line must appear before the first non-comment, non-blank text in the
file") does not restrict "comment" to
//style; the guard only treated//lines ascomment-continuation, so a block-comment license header (e.g.
/* Copyright ... */before// Code generated) caused premature loop termination — the same failure class as thepre-PR CRITICAL finding, just a different comment syntax. Not accepted as "zero practical
risk" (one review pass argued this) — fixed properly: the guard now tracks open
/* */blocks and continues scanning through them. New regression case (5f) added.
goimportsran with no-localgrouping prefix, so arepo already formatting with
-local(a common Go convention wired into CI/editor config)would have every edit re-collapse its local-import grouping back into the third-party group —
empirically confirmed this materially changes output (verified with a real third-party import
present). This directly undercut Open Decision 1's "no config-divergence axis" premise a
second time. Fixed: the hook now derives
-localfrom the edited file's own module path(
go list -m, walks to the nearestgo.modvia Go's own resolution) when agotoolchain ispresent — zero new consumer-config surface, covers the single most common
-localuse case(self-grouping), gracefully degrades to goimports' plain default when
gois absent or thefile isn't in a resolvable module. New regression case (4b) added.
--end-of-flags separatorbefore
$FILE. Low-confidence defense-in-depth; fixed alongside the-localchange.go-mod-tidy-driftgate only triggered ongo.mod/go.sum, missing source-onlytidy drift (e.g. removing the last usage of a dependency leaves
go.modover-declared whilego build/go teststill pass). One review pass called this an acceptable tradeoff citing thepython.yamlprecedent; not accepted — the epic's own stated success criterion is"CI/local parity," and leaving this gap directly contradicts it. Fixed:
trigger-globswidenedto include
*.go. Remediation text also now notes the Go 1.23+ floor forgo mod tidy -diff(a separate LOW finding from the first review pass).
/toolchain:lint's "Per-project walking" list enumerated python/typescript only,omitting
godespitego.yamldeclaringproject-discovery: ["go.mod"]— a monorepo with anested Go module would have
/toolchain:lint gorungolangci-lint run ./...from the wrongroot. Fixed: added a
gobullet.stale after the pre-PR CRITICAL fix. Corrected to describe the actual leading-block scan.
first. A UTF-8 BOM can only appear at byte 0, so this is a harmless no-op after line one, not
a bug. Left unchanged (the fix would add branching complexity for zero behavioral gain).
go.yaml(gatesbeforeinstall-hint) and the example fixture (install-hintbeforegates). Aligned the bundledfile to the example's ordering (which matches the
dotnet.yamlexample precedent).Blast radius
MEDIUM. New plugin + new ecosystem entry, but both are close pattern-replications of two
already-merged, already-reviewed precedents (typos-format PR #872, dotnet opt-in gate PR #890) in
this same epic. The two genuine judgment calls (goimports-unconditional, golangci-lint-opt-in) are
each backed by fresh primary-source research and a direct analogy to an already-accepted precedent
in this repo — not novel territory. No cross-module architectural change, no data-model change, no
multi-tenant concern. Several implementation-time facts (exact goimports flags, exact golangci-lint
install command,
go mod tidydry-run flag syntax) are flagged as verify-live, don't assume —this is where a stress-test/implementation-time slip is most likely, not in the two locked design
decisions.
Stress-test summary
Blast radius MEDIUM — no CRITICAL/HIGH triggers (no cross-module integration, no data-model change,
no multi-tenant posture) per
context/stress-test-triggers.md's criteria, but the two load-bearingjudgment calls (Open Decisions 1 and 2) warrant the mandatory Step 3 fresh-context plan-reviewer
pass before implementation, specifically pressure-testing: (a) is "goimports unconditional, no
opt-in" actually safe, or does goimports have a config-divergence axis this session's research
missed; (b) is the golangci-lint opt-in gate correctly scoped (does it match how
check-cmdcomposes with
fix-cmdthe same way python/dotnet's gates do, and is the ancestor-walk boundarychoice — repo-root-only, no
root = true-equivalent — actually correct for golangci-lint's ownconfig discovery, which may differ from EditorConfig's semantics).
(Dispatched separately as the mandatory Step 3 fresh-context plan-reviewer sub-agent — findings
folded in before implementation begins.)
Execution shape
Sequential — Phase 1 (ecosystem yaml) and Phase 2 (hook plugin) touch disjoint files and share no
data dependency (Phase 2 doesn't consume Phase 1's output), so they are parallel-safe by the
file-overlap test, but the combined scope is small enough (~14 files total) that the coordination
overhead of a two-agent split isn't worth it for a MEDIUM-blast-radius, largely-mechanical
replication lane. Single main-session sequential execution, Phase 1 then Phase 2, then Phase 3
cross-cutting verification gates both. Phase 3 is fully sequential-dependent on both.
Open questions
None blocking — all flagged "verify live during implementation" items are execution-time fact
lookups (exact CLI flags/install commands), not design decisions requiring further user input.
Handoff to implementation
User-approval gates
None beyond initial plan approval — no
[FALLBACK]tags, no scope-expansion proposals anticipated.If live verification during Phase 1/2 surfaces that
goimportsorgolangci-lintbehavematerially differently than researched (e.g., goimports turns out to have a real config-divergence
axis), STOP and re-open Open Decision 1 rather than silently proceeding.
Execution shape (
[EXEC-SHAPE]tagged)[EXEC-SHAPE]Single PR bundling both deliverables (Open Decision 3).[EXEC-SHAPE]Sequential single-main-session execution (Execution shape section above).[EXEC-SHAPE]go-formathook plugin structural simplification (no ancestor config walk-up) —Open Decision 1's consequence.
Mechanical work
Commit boundaries: one commit per phase is reasonable (Phase 1, Phase 2, then fixup commits for
review findings) but not mandated — squash-merge means the final PR history is one commit anyway.
Verification checkpoints: run the full local gate set after each phase, not just once at the end,
to catch cross-phase interactions (e.g.,
plugin.json/CHANGELOG.mdversion-bump conflictsbetween the toolchain-plugin edit in Phase 1 and any repo-root catalog regen in Phase 2) early.
Sequential fallback: N/A (already sequential).
🤖 Generated with Claude Code